sm501: Fix support for non-zero frame buffer start address
Display updates and drawing hardware cursor did not work when frame
buffer address was non-zero. Fix this by taking the frame buffer
address into account in these cases. This fixes screen dragging on
AmigaOS. Based on patch by Sebastian Bauer.
Sebastian Bauer [Wed, 4 Jul 2018 09:40:58 +0000 (11:40 +0200)]
sm501: Log unimplemented raster operation modes
The sm501 currently implements only a very limited set of raster operation
modes. After this change, unknown raster operation modes are logged so
these can be easily spotted.
Add support for the negated destination operation mode. This is used e.g.
by AmigaOS for the INVERSEVID drawing mode. With this change, the cursor
in the shell and non-immediate window adjustment are working now.
Sebastian Bauer [Wed, 4 Jul 2018 09:40:58 +0000 (11:40 +0200)]
sm501: Use values from the pitch register for 2D operations
Before, crt_h_total was used for src_width and dst_width. This is a
property of the current display setting and not relevant for the 2D
operation that also can be done off-screen. The pitch register's purpose
is to describe line pitch relevant of the 2D operation.
Sebastian Bauer [Wed, 4 Jul 2018 09:40:58 +0000 (11:40 +0200)]
sm501: Perform a full update after palette change
Changing the palette of a color index has as an immediate effect on
all pixels with the corresponding index on real hardware. Performing a
full update after a palette change is a simple way to emulate this
effect.
sm501: Implement i2c part for reading monitor EDID
Emulate the i2c part of SM501 which is used to access the EDID info
from a monitor.
The vmstate structure is changed and its version is increased but
SM501 is only used on SH and PPC sam460ex machines that don't support
cross-version migration.
spapr/vio: quiet down the "irq" property accessors
commit efe2add7cb7f ("spapr/vio: deprecate the "irq" property")
introduced get/set accessors for the "irq" property to warn of its
usage, but the warning in the get pollutes the monitor 'info qtree'.
Commit 29f9cef39e "ppc: Include vga cirrus card into the compiling process"
changed the default display adapter for all PPC machines to cirrus. Unfortunately
it missed setting the default display type to stdvga for both Mac machines
causing the display to fail to initialise under OpenBIOS.
Update the MachineClass for both Old World and New World Macs so that the
default std(vga) display adapter is the default if no options are specified
which fixes the display for the Mac machines.
* remotes/bonzini/tags/for-upstream:
checkpatch: handle token pasting better
ioapic: remove useless lower bounds check
pr-manager-helper: fix memory leak on event
qemu-char: check errno together with ret < 0
i386: fix '-cpu ?' output for host cpu type
qtest: Use cpu address space instead of system memory
pr-helper: Rework socket path handling
pr-helper: avoid error on PR IN command with zero request size
Paolo Bonzini [Wed, 4 Jul 2018 16:05:43 +0000 (18:05 +0200)]
checkpatch: handle token pasting better
The mechanism to find possible type tokens can sometimes be confused and go into an
infinite loop. This happens for example in QEMU for a line that looks like
uint## BITS ##_t S = _S, T = _T; \
uint## BITS ##_t as, at, xs, xt, xd; \
Because the token pasting operator does not have a space before _t, it does not
match $notPermitted. However, (?x) is turned on in the regular expression for
modifiers, and thus ##_t matches the empty string. As a result, annotate_values
goes in an infinite loop.
The solution is simply to remove token pasting operators from the string before
looking for modifiers. In the example above, the string uintBITS_t will be
evaluated as a candidate modifier. This is not optimal, but it works as long
as people do not write things like a##s##m, and it fits nicely into sub
possible.
For a similar reason, \# should be rejected always, even if it is not
at end of line or followed by whitespace.
The same patch was sent to the Linux kernel mailing list.
In the tcp_chr_write function, we checked errno,
but errno was not reset before a read or write operation.
Therefore, this check of errno's actions is often
incorrect after EAGAIN has occurred.
we need check errno together with ret < 0.
Greg Kurz [Mon, 2 Jul 2018 16:56:06 +0000 (18:56 +0200)]
i386: fix '-cpu ?' output for host cpu type
Since commit d6dcc5583e7, '-cpu ?' shows the description of the
X86_CPU_TYPE_NAME("max") for the host CPU model:
Enables all features supported by the accelerator in the current host
instead of the expected:
KVM processor with all supported host features
or
HVF processor with all supported host features
This is caused by the early use of kvm_enabled() and hvf_enabled() in
a class_init function. Since the accelerator isn't configured yet, both
helpers return false unconditionally.
A QEMU binary will only be compiled with one of these accelerators, not
both. The appropriate description can thus be decided at build time.
When reviewing Paolo's pr-helper patches I've noticed couple of
problems:
1) socket_path needs to be calculated at two different places
(one for printing out help, the other if socket activation is NOT
used),
2) even though the default socket_path is allocated in
compute_default_paths() it is the only default path the function
handles. For instance, pidfile is allocated outside of this
function. And yet again, at different places than 1)
Paolo Bonzini [Mon, 2 Jul 2018 12:21:58 +0000 (14:21 +0200)]
pr-helper: avoid error on PR IN command with zero request size
After reading a PR IN command with zero request size in prh_read_request,
the resp->result field will be uninitialized and the resp.sz field will
be also uninitialized when returning to prh_co_entry.
If resp->result == GOOD (from a previous successful reply or just luck),
then the assert in prh_write_response might not be triggered and
uninitialized response will be sent.
The fix is to remove the whole handling of sz == 0 in prh_co_entry.
Those errors apply only to PR OUT commands and it's perfectly okay to
catch them later in do_pr_out and multipath_pr_out; the check for
too-short parameters in fact doesn't apply in the easy SG_IO case, as
it can be left to the target firmware even.
The result is that prh_read_request does not fail requests anymore and
prh_co_entry becomes simpler.
PPC tcg seems to be failing migration tests quite regularly;
we believe this is TCG bugs in dirty bit updating; it's
not clear why PPC fails more but lets skip for the moment.
$ ./tests/migration-test
/ppc64/migration/deprecated: OK
/ppc64/migration/bad_dest: Skipping test: kvm_hv not available OK
/ppc64/migration/postcopy/unix: Skipping test: kvm_hv not available OK
/ppc64/migration/precopy/unix: Skipping test: kvm_hv not available OK
Peter Maydell [Fri, 6 Jul 2018 09:17:51 +0000 (10:17 +0100)]
Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-pull-20180705' into staging
RISC-V: SoCify SiFive boards and connect GEM
This series has three tasks:
1. To convert the SiFive U and E machines into SoCs and boards
2. To connect the Cadence GEM device to the SiFive U board
3. Fix some device tree problems with the SiFive U board
After this series the SiFive E and U boards have their SoCs split into
seperate QEMU objects, which can be used on future boards if desired.
The RISC-V Virt and Spike boards have not been converted. They haven't
been converted as they aren't physical boards, so it doesn't make a
whole lot of sense to split them into an SoC and board. The only
disadvantage with this is that they now differ to the SiFive boards.
This series also connect the Cadence GEM device to the SiFive U board.
There are some interrupt line changes requried before this is possible.
# gpg: Signature made Fri 06 Jul 2018 02:17:21 BST
# gpg: using RSA key 21E10D29DF977054
# gpg: Good signature from "Alistair Francis <[email protected]>"
# gpg: WARNING: This key is not certified with sufficiently trusted signatures!
# gpg: It is not certain that the signature belongs to the owner.
# Primary key fingerprint: F6C4 AC46 D493 4868 D3B8 CE8F 21E1 0D29 DF97 7054
* remotes/alistair/tags/pull-riscv-pull-20180705:
hw/riscv/sifive_u: Connect the Cadence GEM Ethernet device
hw/riscv/sifive_u: Move the uart device tree node under /soc/
hw/riscv/sifive_u: Set the interrupt controller number of interrupts
hw/riscv/sifive_u: Set the soc device tree node as a simple-bus
hw/riscv/sifive_plic: Use gpios instead of irqs
hw/riscv/sifive_e: Create a SiFive E SoC object
hw/riscv/sifive_u: Create a SiFive U SoC object
Alistair Francis [Thu, 26 Apr 2018 20:54:12 +0000 (13:54 -0700)]
hw/riscv/sifive_plic: Use gpios instead of irqs
Instead of creating the interrupt in lines with qemu_allocate_irq() use
qdev_init_gpio_in() as this gives us the ability to use the qdev*gpio*()
helpers later on.
Peter Maydell [Thu, 5 Jul 2018 17:24:28 +0000 (18:24 +0100)]
Merge remote-tracking branch 'remotes/stsquad/tags/pull-code-coverage-and-build-tweaks-050718-3' into staging
Code coverage and other build tweaks
- revert 208ecb3e (and drop filter for mingw, tweak for check-tcg)
- some travis speed-ups
- modernise code coverage support
- docker image cleanups
- clean-up binfmt_misc docker infrastructure
- add debian-powerpc-user-cross image for ppc32 build
# gpg: Signature made Thu 05 Jul 2018 17:00:02 BST
# gpg: using RSA key FBD0DB095A9E2A44
# gpg: Good signature from "Alex Bennée (Master Work Key) <[email protected]>"
# Primary key fingerprint: 6685 AE99 E751 67BC AFC8 DF35 FBD0 DB09 5A9E 2A44
* remotes/stsquad/tags/pull-code-coverage-and-build-tweaks-050718-3:
docker: add linux-user powered cross builder for QEMU
docker: add special rule for deboostrapped images
docker: add special handling for FROM:debian-%-user targets
docker: debian-bootstrap.pre allow customising of variant/url
docker: drop QEMU build-dep from bootstrap
docker: Do not run tests in 'intermediate' images
docker: Clean the MXE base image
docker: ubuntu: Use SDL2
docker: ubuntu: Update the package list before installing new ones
linux-user: add gcov support to preexit_cleanup
linux-user: introduce preexit_cleanup
build-system: add coverage-report target
build-system: add clean-coverage target
travis: add gcovr summary for GCOV build
docker: add gcovr to travis image
.gitignore: add .gcov files
build-system: remove per-test GCOV reporting
travis: test out-of-tree builds
travis: do not waste time cloning unused submodules
Revert "Makefile: Rename TARGET_DIRS to TARGET_LIST"
Alex Bennée [Fri, 29 Jun 2018 19:41:26 +0000 (20:41 +0100)]
docker: add linux-user powered cross builder for QEMU
We can't use cross compilers in the current Debian stable and Debian
sid is sketchy as hell. So for powerpc fall back to dog-fooding our
own linux-user to do the build.
As we can only build the base image with a suitably configured
source tree we fall back to checking for its existence when we can't
build it from scratch. However this does mean you don't have to keep
a static powerpc-linux-user in your active configuration just to
update the cross build image.
Alex Bennée [Fri, 29 Jun 2018 16:57:57 +0000 (17:57 +0100)]
docker: add special rule for deboostrapped images
We might as well have a custom rule for this. For one thing the
dependencies are different. As the primary dependency for
docker-image-% could never be docker-image-debian-bootstrap we can
drop that test in the main rule as well.
Missing EXECUTABLE, DEB_ARCH and DEB_TYPE are treated as hard faults
now. We also error out if the EXECUTABLE file isn't there. We should
really do this with a dependency on any source rules but currently
subdir-FOO-linux-user isn't enough on a clean build.
Alex Bennée [Fri, 29 Jun 2018 16:46:49 +0000 (17:46 +0100)]
docker: add special handling for FROM:debian-%-user targets
These will have been build with debootstrap so we need to check
against the debian-bootstrap dockerfile. This does mean sticking to
debian-FOO-user as the naming conventions for boot-strapped images.
The actual cross image is built on top.
Alex Bennée [Mon, 2 Jul 2018 13:02:44 +0000 (14:02 +0100)]
docker: debian-bootstrap.pre allow customising of variant/url
We default to the buildd variant as most of our images are for
building. However lets give the user the ability to specify "minbase"
if they want to create a simple base image for experimentation.
Allowing the tweaking of DEB_URL means we can also bootstrap other
Debian based OS's. For example:
make docker-binfmt-image-debian-ubuntu-bionic-arm64 \
DEB_ARCH=arm64 DEB_TYPE=bionic \
DEB_VARIANT=minbase DEB_URL=http://ports.ubuntu.com/ \
EXECUTABLE=./aarch64-linux-user/qemu-aarch64
docker: ubuntu: Update the package list before installing new ones
Since docker caches the different layers, updating the package
list does not invalidate the previous "apt-get update" layer,
and it is likely "apt-get install" hits an outdated repository.
See https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#apt-get
This fixes:
$ make docker-image-ubuntu V=1
./tests/docker/docker.py build qemu:ubuntu tests/docker/dockerfiles/ubuntu.docker --add-current-user
Sending build context to Docker daemon 3.072kB
[...]
E: Failed to fetch http://archive.ubuntu.com/ubuntu/pool/main/m/mesa/libgles2-mesa_17.0.7-0ubuntu0.16.04.2_amd64.deb 404 Not Found
E: Failed to fetch http://archive.ubuntu.com/ubuntu/pool/main/m/mesa/libgles2-mesa-dev_17.0.7-0ubuntu0.16.04.2_amd64.deb 404 Not Found
E: Unable to fetch some archives, maybe run apt-get update or try with --fix-missing?
The command '/bin/sh -c apt-get -y install $PACKAGES' returned a non-zero code: 100
tests/docker/Makefile.include:40: recipe for target 'docker-image-ubuntu' failed
make: *** [docker-image-ubuntu] Error 1
Alex Bennée [Fri, 22 Jun 2018 16:23:44 +0000 (17:23 +0100)]
linux-user: add gcov support to preexit_cleanup
As we don't always take the normal exit path when running a guest we
can skip the normal exit destructors where gcov normally dumps it's
info. The GCC manual suggests long running programs use __gcov_dump()
to flush out the coverage state periodically so we use that here.
Alex Bennée [Fri, 22 Jun 2018 16:09:10 +0000 (17:09 +0100)]
linux-user: introduce preexit_cleanup
To avoid repeating ourselves move our preexit clean-up code into a
helper function. I figured the continuing effort to split of the
syscalls made it worthwhile creating a new file for it now.
Alex Bennée [Wed, 20 Jun 2018 13:04:24 +0000 (14:04 +0100)]
build-system: add coverage-report target
This will build a coverage report under the current directory in
reports/coverage. At the users option a report can be generated by
directly invoking something like:
Alex Bennée [Wed, 20 Jun 2018 11:00:07 +0000 (12:00 +0100)]
travis: add gcovr summary for GCOV build
This gives a more useful summary, sorted by descending % coverage,
after the tests have run. The final numbers will give an idea if our
coverage is getting better or worse.
To keep the width sane we need to post process the file that the old
gcovr tool generates. This is done with a mix of sed, awk and column
in the scripts/coverage-summary.sh script.
As quite a lot of lines don't get covered at all we filter out all the
0% lines. If the file doesn't appear it is not being exercised.
Alex Bennée [Wed, 20 Jun 2018 10:28:51 +0000 (11:28 +0100)]
build-system: remove per-test GCOV reporting
I'm not entirely sure who's using this information and certainly in a
CI environment it just washes over as additional noise. Later patches
will provide new reporting options so a user who wants to analyse
individual tests will be able to use that to get the information.
Alex Bennée [Wed, 4 Jul 2018 06:30:11 +0000 (07:30 +0100)]
Revert "Makefile: Rename TARGET_DIRS to TARGET_LIST"
This reverts commit 208ecb3e1acc8d55dab49fdf721a86d513691688. This was
causing problems by making DEF_TARGET_LIST pointless and having to
jump through hoops to build on mingw with a dully enabled config.
This includes a change to fix the per-guest TCG test probe which was
added after 208ecb3 and used TARGET_LIST.
Peter Maydell [Thu, 5 Jul 2018 14:53:04 +0000 (15:53 +0100)]
Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging
Block layer patches:
- qcow2: Use worker threads for compression to improve performance of
'qemu-img convert -W' and compressed backup jobs
- blklogwrites: New filter driver to log write requests to an image in
the dm-log-writes format
- file-posix: Fix image locking during image creation
- crypto: Fix memory leak in error path
- Error out instead of silently truncating node names
# gpg: Signature made Thu 05 Jul 2018 11:24:33 BST
# gpg: using RSA key 7F09B272C88F2FD6
# gpg: Good signature from "Kevin Wolf <[email protected]>"
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6
* remotes/kevin/tags/for-upstream:
file-posix: Unlock FD after creation
file-posix: Fix creation locking
block/blklogwrites: Add an option for the update interval of the log superblock
block/blklogwrites: Add an option for appending to an old log
block/blklogwrites: Change log_sector_size from int64_t to uint64_t
block/crypto: Fix memory leak in create error path
block: Don't silently truncate node names
block: Add blklogwrites
block: Move two block permission constants to the relevant enum
qcow2: add compress threads
qcow2: refactor data compression
qemu-img: allow compressed not-in-order writes
$ cov-make-library -of scripts/coverity-model.xmldb scripts/coverity-model.c
output file: scripts/coverity-model.xmldb
Compiling scripts/coverity-model.c with command /opt/cov-sa-2018.06/bin/cov-emit --dir /tmp/cov-armbru/930a6fb31e5f464fc1a53354b2deb66b/cov-make-library-emit -w --no_error_recovery --emit_header_functions --no_implicit_decl --preinclude /opt/cov-sa-2018.06/library/decls.h --c scripts/coverity-model.c
"scripts/coverity-model.c", line 110: error #20: identifier "replay_file" is
undefined
if (replay_file) {
^
Emit for file '/work/armbru/qemu/scripts/coverity-model.c' complete.
[ERROR] 1 error detected in the compilation of "scripts/coverity-model.c".
ERROR: cov-emit returned with code 1
Broken in commit 04a0afe5285. Fix by dumbing down.
* remotes/armbru/tags/pull-monitor-2018-07-03-v2: (32 commits)
qapi: Polish command flags documentation in qapi-code-gen.txt
monitor: Improve some comments
qmp: Clean up capability negotiation after commit 02130314d8c
qobject: Let qobject_from_jsonf() fail instead of abort
qmp: Switch timestamp_put() to qdict_from_jsonf_nofail()
qmp: Add some comments around null responses
qmp: Simplify monitor_qmp_respond()
qmp: Replace get_qmp_greeting() by qmp_greeting()
qmp: Replace monitor_json_emitter{,raw}() by qmp_{queue,send}_response()
qmp: Use QDict * instead of QObject * for response objects
qmp: De-duplicate error response building
qobject: New qdict_from_jsonf_nofail()
monitor: Peel off @mon_global wrapper
monitor: Rename use_io_thr to use_io_thread
qmp: Don't let JSON errors jump the queue
qmp: Don't let malformed in-band commands jump the queue
tests/qmp-test: Demonstrate QMP errors jumping the queue
qmp: Simplify code around monitor_qmp_dispatch_one()
qmp: Always free QMPRequest with qmp_request_free()
qmp: Revert change to handle_qmp_command tracepoint
...
Max Reitz [Wed, 4 Jul 2018 14:47:50 +0000 (16:47 +0200)]
file-posix: Fix creation locking
raw_apply_lock_bytes() takes a bit mask of "permissions that are NOT
shared".
Also, make the "perm" and "shared" variables uint64_t, because I do not
particularly like using ~ on signed integers (and other permission masks
are usually uint64_t, too).
Ari Sundholm [Wed, 4 Jul 2018 14:59:36 +0000 (17:59 +0300)]
block/blklogwrites: Add an option for the update interval of the log superblock
This is a way to ensure that the log superblock is periodically
updated. Before, this was only done on flush requests, which may
not be enough if the VM exits abnormally, omitting the final flush.
Kevin Wolf [Wed, 4 Jul 2018 11:28:29 +0000 (13:28 +0200)]
block: Don't silently truncate node names
If the user passes a too long node name string, we silently truncate it
to fit into BlockDriverState.node_name, i.e. to 31 characters. Apart
from surprising the user when the node has a different name than
requested, this also bypasses the check for duplicate names, so that the
same name can be assigned to multiple nodes.
Fix this by just making too long node names an error.
Implements a block device write logging system, similar to Linux kernel
device mapper dm-log-writes. The write operations that are performed
on a block device are logged to a file or another block device. The
write log format is identical to the dm-log-writes format. Currently,
log markers are not supported.
This functionality can be used for crash consistency and fs consistency
testing. By implementing it in qemu, tests utilizing write logs can be
be used to test non-Linux drivers and older kernels.
The driver accepts an optional parameter to set the sector size used
for logging. This makes the driver require all requests to be aligned
to this sector size and also makes offsets and sizes of writes in the
log metadata to be expressed in terms of this value (the log format has
a granularity of one sector for offsets and sizes). This allows
accurate logging of writes to guest block devices that have unusual
sector sizes.
The implementation is based on the blkverify and blkdebug block
drivers.
Do data compression in separate threads. This significantly improve
performance for qemu-img convert with -W (allow async writes) and -c
(compressed) options.
Make a separate function for compression to be parallelized later.
- use .avail_out field instead of .next_out to calculate size of
compressed data. It looks more natural and it allows to keep dest to
be void pointer
- set avail_out to be at least one byte less than input, to be sure
avoid inefficient compression earlier
Peter Maydell [Thu, 5 Jul 2018 07:21:25 +0000 (08:21 +0100)]
Merge remote-tracking branch 'remotes/mdroth/tags/qga-pull-2018-07-03-tag' into staging
qemu-ga patch queue for soft-freeze
* add systemd suspend support
* add used/total space stats for guest-get-fsinfo
* fixes for guest-get-fsinfo over PCI bridges
* MSI installer and schema doc fixes
* guard against unbounded allocations in guest-file-read
* add some additional qga test cases
* remotes/mdroth/tags/qga-pull-2018-07-03-tag:
qga: removing bios_supports_mode
qga: systemd hibernate/suspend/hybrid-sleep support
qga: removing switch statements, adding run_process_child
qga: guest_suspend: decoupling pm-utils and sys logic
qga: bios_supports_mode: decoupling pm-utils and sys logic
qga: refactoring qmp_guest_suspend_* functions
qemu-ga: make get-fsinfo work over pci bridges
qga-win: Fixing msi upgrade disallow in WiX file
qga/schema: fix documentation for GuestOSInfo
test-qga: add trivial tests for some commands
qga-win: add driver path usage to GuestFilesystemInfo
qga: add mountpoint usage info to GuestFilesystemInfo
qga: check bytes count read by guest-file-read
qga: unset frozen state if no mount points are frozen
Bitmap lock/unlock were added to bdrv_enable_dirty_bitmap in 8b1402ce80d, but some places were not updated correspondingly, which
leads to trying to take this lock twice, which is dead-lock. Fix this.
Actually, iotest 199 (about dirty bitmap postcopy migration) is broken
now, and this fixes it.
qmp: Clean up capability negotiation after commit 02130314d8c
qmp_greeting() offers capabilities to the client, and
qmp_qmp_capabilities() accepts or denies capabilities requested by the
client. The two compute the set of available capabilities
independently. Not nice.
Clean this up as follows. Compute available capabilities just once in
monitor_qmp_caps_reset(), and store them in Monitor member
qmp.capab_offered[]. Have qmp_greeting() and qmp_qmp_capabilities()
use that. Both are now oblivious of capability details.
qobject: Let qobject_from_jsonf() fail instead of abort
qobject_from_jsonf() aborts on error, unlike qobject_from_jsonv(),
which returns null. Since all remaining users of qobject_from_jsonf()
cope fine with null, change it to return null.
monitor_qmp_respond() takes both a response object and an error
object. If an error object is non-null, the response object must be
null, and the response is built from the error object.
Of the two callers, one always passes a null response object, and one
a null error object. Move building the response object from the error
object to the latter, and drop the error object parameter.
qmp: Replace monitor_json_emitter{,raw}() by qmp_{queue,send}_response()
monitor_json_emitter() and monitor_json_emitter_raw() are
unnecessarily general: they can send arbitrary JSON values, even
though we only ever use them for QMP, which may send only JSON
objects.
Specialize the argument from QObject * to QDict *, and rename to
qmp_queue_response(), qmp_send_response().
All callers but one lose an upcast. The lone exception gains a
downcast; the next commit will get rid of it.
All callers of qmp_build_error_object() duplicate the code to wrap it
in a response object. Replace it by qmp_error_response() that
captures the duplicated code, including error_free().
Many uses of qobject_from_jsonf() convert JSON objects. Create new
convenience function qdict_from_jsonf_nofail() that includes the
conversion to QDict. The next few commits will put it to use.
handle_qmp_command() reports JSON syntax errors right away. This is
wrong when OOB is enabled, because the errors can "jump the queue"
then.
The previous commit fixed the same bug for semantic errors, by
delaying the checking until dispatch. We can't delay the checking, so
delay the reporting.
qmp: Don't let malformed in-band commands jump the queue
handle_qmp_command() reports certain errors right away. This is wrong
when OOB is enabled, because the errors can "jump the queue" then, as
the previous commit demonstrates.
To fix, we need to delay errors until dispatch. Do that for semantic
errors, mostly by reverting ill-advised parts of commit cf869d53172
"qmp: support out-of-band (oob) execution". Bonus: doesn't run
qmp_dispatch_check_obj() twice, once in handle_qmp_command(), and
again in do_qmp_dispatch(). That's also due to commit cf869d53172.
The next commit will fix queue jumping for syntax errors.
tests/qmp-test: Demonstrate QMP errors jumping the queue
When OOB is enabled, out-of-band commands are executed right away,
everything else is queued. This lets out-of-band commands "jump the
queue".
However, certain errors are always reported right away, and therefore
can jump the queue even when the erroneous input does not request
out-of-band execution. These errors are pretty unlikely to occur in
production, but it's wrong all the same. Mark FIXME.
qmp: Simplify code around monitor_qmp_dispatch_one()
Change monitor_qmp_dispatch_one() to take its parameters unwrapped,
move monitor_resume() to the one caller that needs it, rename the
function to monitor_qmp_dispatch().
qmp: Always free QMPRequest with qmp_request_free()
monitor_qmp_dispatch_one() frees a QMPRequest manually, because it
needs to keep a reference to ->id. Premature optimization. Take an
additional reference so we can use qmp_request_free().
qmp: Revert change to handle_qmp_command tracepoint
Commit 71da4667db6 "monitor: separate QMP parser and dispatcher" moved
the handle_qmp_command tracepoint from handle_qmp_command() to
monitor_qmp_dispatch_one(). This delays tracing from enqueue time to
dequeue time. Revert that. Dequeue remains adequately visible via
tracepoint monitor_qmp_cmd_in_band.
qmp: Redo how the client requests out-of-band execution
Commit cf869d53172 "qmp: support out-of-band (oob) execution" added a
general mechanism for command-independent arguments just for an
out-of-band flag:
The "control" key is introduced to store this extra flag. "control"
field is used to store arguments that are shared by all the commands,
rather than command specific arguments. Let "run-oob" be the first.
However, it failed to reject unknown members of "control". For
instance, in QMP command
Instead of fixing this, revert the general "control" mechanism
(because YAGNI), and do it the way I initially proposed, with key
"exec-oob". Simpler code, simpler interface.
qmp qemu-ga: Revert change that accidentally made qemu-ga accept "id"
Commit cf869d53172 "qmp: support out-of-band (oob) execution" changed
how we check "id":
Note that in the patch I exported qmp_dispatch_check_obj() to be
used to check the request earlier, and at the same time allowed
"id" field to be there since actually we always allow that.
The part after "and" is ill-advised: it makes qemu-ga accept and
ignore "id". Revert.
qmp: Make "id" optional again even in "oob" monitors
Commit cf869d53172 "qmp: support out-of-band (oob) execution" made
"id" mandatory for all commands when the client accepted capability
"oob". This is rather onerous when you play with QMP by hand, and
unnecessarily so: only out-of-band commands need an ID for reliable
matching of response to command.
Revert that part of commit cf869d53172 for now, but have documentation
advise on the need to use "id" with out-of-band commands.
projects / qemu.git / log