Commit cf869d5317 "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.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-10-armbru@redhat.com>
Commit cf869d5317 "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 cf869d5317 for now, but have documentation
advise on the need to use "id" with out-of-band commands.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180703085358.13941-8-armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Events are broadcast to all monitors. If another monitor's client has
a command with the same ID in flight, the event will incorrectly claim
that command was dropped. This must be fixed before out-of-band
execution can graduate from "experimental".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-5-armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-3-armbru@redhat.com>
Affects documentation and a few error messages.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180703085358.13941-2-armbru@redhat.com>
It eases code review, unit is explicit.
Patch generated using:
$ git grep -n '[<>][<>]= ?[1-5]0'
and modified manually.
Suggested-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20180625124238.25339-43-f4bug@amsat.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Previously we clean up the queues when we got CLOSED event. It was used
to make sure we won't send leftover replies/events of a old client to a
new client which makes perfect sense. However this will also drop the
replies/events even if the output port of the previous chardev backend
is still open, which can lead to missing of the last replies/events.
Now this patch does an extra operation to flush the response queue
before cleaning up.
In most cases, a QMP session will be based on a bidirectional channel (a
TCP port, for example, we read/write to the same socket handle), so in
port and out port of the backend chardev are fundamentally the same
port. In these cases, it does not really matter much on whether we'll
flush the response queue since flushing will fail anyway. However there
can be cases where in & out ports of the QMP monitor's backend chardev
are separated. Here is an example:
cat $QMP_COMMANDS | qemu -qmp stdio ... | filter_commands
In this case, the backend is fd-typed, and it is connected to stdio
where in port is stdin and out port is stdout. Now if we drop all the
events on the response queue then filter_command process might miss some
events that it might expect. The thing is that, when stdin closes,
stdout might still be there alive!
In practice, I encountered SHUTDOWN event missing when running test with
iotest 087 with Out-Of-Band enabled. Here is one of the ways that this
can happen (after "quit" command is executed and QEMU quits the main
loop):
1. [main thread] QEMU queues a SHUTDOWN event into response queue.
2. "cat" terminates (to distinguish it from the animal, I quote it).
3. [monitor iothread] QEMU's monitor iothread reads EOF from stdin.
4. [monitor iothread] QEMU's monitor iothread calls the CLOSED event
hook for the monitor, which will destroy the response queue of the
monitor, then the SHUTDOWN event is dropped.
5. [main thread] QEMU's main thread cleans up the monitors in
monitor_cleanup(). When trying to flush pending responses, it sees
nothing. SHUTDOWN is lost forever.
Note that before the monitor iothread was introduced, step [4]/[5] could
never happen since the main loop was the only place to detect the EOF
event of stdin and run the CLOSED event hooks. Now things can happen in
parallel in the iothread.
Without this patch, iotest 087 will have ~10% chance to miss the
SHUTDOWN event and fail when with Out-Of-Band enabled:
--- /home/peterx/git/qemu/tests/qemu-iotests/087.out
+++ /home/peterx/git/qemu/bin/tests/qemu-iotests/087.out.bad
@@ -8,7 +8,6 @@
{"return": {}}
{"error": {"class": "GenericError", "desc": "'node-name' must be
specified for the root node"}}
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
=== Duplicate ID ===
@@ -53,7 +52,6 @@
{"return": {}}
{"return": {}}
{"return": {}}
-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
This patch fixes the problem.
Fixes: 6d2d563f8c ("qmp: cleanup qmp queues properly", 2018-03-27)
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180620073223.31964-4-peterx@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Commit message and a comment touched up]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The old names are confusing since both of the old functions are popping
an item from multiple queues rather than a single queue. In that
sense, *_pop_any() suites better than *_pop_one().
Since at it, touch up the function monitor_qmp_response_pop_any() a bit
to let the callers pass in a QMPResponse struct instead of returning a
struct. Change the return value to boolean to mark whether we have
popped a valid response instead.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180620073223.31964-3-peterx@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
soft-freeze, but I'd like these preparatory patches to be merged anyway.
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEEtIKLr5QxQM7yo0kQcdTV5YIvc9YFAls2DEgACgkQcdTV5YIv
c9ZShw/6AuDMeWpzFHAdf4lBuUdDFyAC7QFln8xdb+MDus5whAvtt7vDeY0OKbgo
w5VDTkstO7h4jQJDHkjzHK91ZdUbgu0Tj7C09x4oQpUueNWsTZGcPBvNGadjjOBt
70LdwyV2nSER3+QNjTNznrh0faxay4xuSTIY/mW6iudeWGobwXmseEeOE8gGM+w0
s1GwxMVKIfllKUmW2vx0mGfn02pKtTnan+Si+sp/AnY9xSquFfHWpZhXZlkZrfYd
mgtJOTY9IpSekr9jBBKgUlZ/QVYiliDzuh3ePDYKtsuHZZ7z2ype3DkXqYOnblOs
C+2gWUE/TC5BStjRX3RmPv21dpfkEdlxOZpgXbpP1VgKqbtnbnvcgTL89IPv9afl
Aj+q5uYR494kOL5rSDynVRdWhnUmMnkqHCZpKG+IRMHv6GlrXpxOQWenwCS/vYWK
swKqRwGj0CFugdt7qVZ+4XjXbbWEI21dHHG7nAXinfakKVOfJYIeGIQC7WfpIrxy
ApV0mHSceK0AMBJvlf1Zf0Qm0lJ7Ay7MRT/5XWDFV9Bogf+wxtGvf9Ukc2qQhwd8
mR9iN7rlWz3VSu5vS3bEdsiBXKibxIRfv7HhF5fa+mwkZA9gMbj33vVds1zA4ta1
Qw4doRq4xWui3uNO9jvtcXtW5Bq7N4p6wVFK76dLVHk5axLCIec=
=vVJr
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/gkurz/tags/for-upstream' into staging
The Darwin host support still needs some more work. It won't make it for
soft-freeze, but I'd like these preparatory patches to be merged anyway.
# gpg: Signature made Fri 29 Jun 2018 11:39:04 BST
# gpg: using RSA key 71D4D5E5822F73D6
# gpg: Good signature from "Greg Kurz <groug@kaod.org>"
# gpg: aka "Gregory Kurz <gregory.kurz@free.fr>"
# gpg: aka "[jpeg image of size 3330]"
# Primary key fingerprint: B482 8BAF 9431 40CE F2A3 4910 71D4 D5E5 822F 73D6
* remotes/gkurz/tags/for-upstream:
9p: darwin: Explicitly cast comparisons of mode_t with -1
cutils: Provide strchrnul
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
strchrnul is a GNU extension and thus unavailable on a number of targets.
In the review for a commit removing strchrnul from 9p, I was asked to
create a qemu_strchrnul helper to factor out this functionality.
Do so, and use it in a number of other places in the code base that inlined
the replacement pattern in a place where strchrnul could be used.
Signed-off-by: Keno Fischer <keno@juliacomputing.com>
Acked-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
This adds owners/parents (which are the same, just occasionally
owner==NULL) printing for memory regions; a new '-o' flag
enabled new output.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Message-Id: <20180604032511.6980-1-aik@ozlabs.ru>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Now we can cope with preconfig in HMP, reenable by reverting
commit 71dc578e11.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20180620153947.30834-8-dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Don't show the commands that aren't available.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20180620153947.30834-4-dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Allow the 'help' command in preconfig state but
make it only list the preconfig commands.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20180620153947.30834-3-dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Add a flag to command definitions to allow them to be used in preconfig
and check it.
If users try to use commands that aren't available, tell them to use
the exit_preconfig comand we're adding in a few patches.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20180620153947.30834-2-dgilbert@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
When a user incorrectly provides an hmp command, an error response will be
printed that prompts the user to try "help <command name>". However, when
the command contains multiple parts e.g. "info uuid xyz", only the last
whitespace delimited string will be reported (in this example "info" will
be dropped and the message will read "Try "help uuid" for more information",
which is incorrect).
Let's correct this by capturing the entirety of the command from the command
line -- excluding any extraneous characters.
Reported-by: Mikhail Fokin <fokin@de.ibm.com>
Signed-off-by: Collin Walling <walling@linux.ibm.com>
Message-Id: <ee680f5e-ac9a-479d-f65e-9f8ae9cfe5d4@linux.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Introduce a new global big lock for mon_fdsets. Take it where needed.
The monitor_fdset_get_fd() handling is a bit tricky: now we need to call
qemu_mutex_unlock() which might pollute errno, so we need to make sure
the correct errno be passed up to the callers. To make things simpler,
we let monitor_fdset_get_fd() return the -errno directly when error
happens, then in qemu_open() we move it back into errno.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180608035511.7439-8-peterx@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Instead, use a dynamic function to detect which clock we'll use. The
problem is that the old code will let monitor initialization depend on
configure_accelerator() (that's where qtest_enabled() start to take
effect). After this change, we don't have such a dependency any more.
We just need to make sure configure_accelerator() is called when we
start to use it. Now it's only used in monitor_qapi_event_queue() and
monitor_qapi_event_handler(), so we're good.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180608035511.7439-6-peterx@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[monitor_get_event_clock() name and comment tweaked]
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Fix typo in d622cb5879. Meanwhile move these variables close to each
other. monitor_qapi_event_state can be declared static, add that.
Reported-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180608035511.7439-5-peterx@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Add some explicit comments for both Readline and cpu_set/cpu_get helpers
that they do not need the mon_lock protection.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180608035511.7439-4-peterx@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
mon->fds were protected by BQL. Now protect it by mon_lock so that it
can even be used in monitor iothread.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180608035511.7439-3-peterx@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The out_lock is protecting a few Monitor fields. In the future the
monitor code will start to run in multiple threads. We are going to
turn it into a bigger lock to protect not only the out buffer but also
most of the rest.
Since at it, rearrange the Monitor struct a bit.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180608035511.7439-2-peterx@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Remove those unneeded includes to speed up the compilation
process a little bit.
Code change produced with:
$ git grep '#include "sysemu/blockdev.h"' | \
cut -d: -f-1 | \
xargs egrep -L "(BlockInterfaceType|DriveInfo|drive_get|blk_legacy_dinfo|blockdev_mark_auto_del)" | \
xargs sed -i.bak '/#include "sysemu\/blockdev.h"/d'
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20180528232719.4721-15-f4bug@amsat.org>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
New option will be used to allow commands, which are prepared/need
to run, during preconfig state. Other commands that should be able
to run in preconfig state, should be amended to not expect machine
in initialized state or deal with it.
For compatibility reasons, commands that don't use new flag
'allow-preconfig' explicitly are not permitted to run in
preconfig state but allowed in all other states like they used
to be.
Within this patch allow following commands in preconfig state:
qmp_capabilities
query-qmp-schema
query-commands
query-command-line-options
query-status
exit-preconfig
to allow qmp connection, basic introspection and moving to the next
state.
PS:
set-numa-node and query-hotpluggable-cpus will be enabled later in
a separate patches.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <1526057503-39287-1-git-send-email-imammedo@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[ehabkost: Changed "since 2.13" to "since 3.0"]
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Ban it for now, if someone would need it to work early,
one would have to implement checks if HMP command is valid
at preconfig state.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1525423069-61903-5-git-send-email-imammedo@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
For convenience and clarity, make it possible to call qobject_ref() at
the time when the reference is associated with a variable, or
argument, by making qobject_ref() return the same pointer as given.
Use that to simplify the callers.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180419150145.24795-5-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Useless change to qobject_ref_impl() dropped, commit message improved
slightly]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Now that we can safely call QOBJECT() on QObject * as well as its
subtypes, we can have macros qobject_ref() / qobject_unref() that work
everywhere instead of having to use QINCREF() / QDECREF() for QObject
and qobject_incref() / qobject_decref() for its subtypes.
The replacement is mechanical, except I broke a long line, and added a
cast in monitor_qmp_cleanup_req_queue_locked(). Unlike
qobject_decref(), qobject_unref() doesn't accept void *.
Note that the new macros evaluate their argument exactly once, thus no
need to shout them.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180419150145.24795-4-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Rebased, semantic conflict resolved, commit message improved]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Eric Auger reported the problem days ago that OOB broke ARM when running
with libvirt:
http://lists.gnu.org/archive/html/qemu-devel/2018-03/msg06231.html
The problem was that the monitor dispatcher bottom half was bound to
qemu_aio_context now, which could be polled unexpectedly in block code.
We should keep the dispatchers run in iohandler_ctx just like what we
did before the Out-Of-Band series (chardev uses qio, and qio binds
everything with iohandler_ctx).
If without this change, QMP dispatcher might be run even before reaching
main loop in block IO path, for example, in a stack like (the ARM case,
"cont" command handler run even during machine init phase):
#0 qmp_cont ()
#1 0x00000000006bd210 in qmp_marshal_cont ()
#2 0x0000000000ac05c4 in do_qmp_dispatch ()
#3 0x0000000000ac07a0 in qmp_dispatch ()
#4 0x0000000000472d60 in monitor_qmp_dispatch_one ()
#5 0x000000000047302c in monitor_qmp_bh_dispatcher ()
#6 0x0000000000acf374 in aio_bh_call ()
#7 0x0000000000acf428 in aio_bh_poll ()
#8 0x0000000000ad5110 in aio_poll ()
#9 0x0000000000a08ab8 in blk_prw ()
#10 0x0000000000a091c4 in blk_pread ()
#11 0x0000000000734f94 in pflash_cfi01_realize ()
#12 0x000000000075a3a4 in device_set_realized ()
#13 0x00000000009a26cc in property_set_bool ()
#14 0x00000000009a0a40 in object_property_set ()
#15 0x00000000009a3a08 in object_property_set_qobject ()
#16 0x00000000009a0c8c in object_property_set_bool ()
#17 0x0000000000758f94 in qdev_init_nofail ()
#18 0x000000000058e190 in create_one_flash ()
#19 0x000000000058e2f4 in create_flash ()
#20 0x00000000005902f0 in machvirt_init ()
#21 0x00000000007635cc in machine_run_board_init ()
#22 0x00000000006b135c in main ()
Actually the problem is more severe than that. After we switched to the
qemu AIO handler it means the monitor dispatcher code can even be called
with nested aio_poll(), then it can be an explicit aio_poll() inside
another main loop aio_poll() which could be racy too; breaking code
like TPM and 9p that use nested event loops.
Switch to use the iohandler_ctx for monitor dispatchers.
My sincere thanks to Eric Auger who offered great help during both
debugging and verifying the problem. The ARM test was carried out by
applying this patch upon QEMU 2.12.0-rc0 and problem is gone after the
patch.
A quick test of mine shows that after this patch applied we can pass all
raw iotests even with OOB on by default.
CC: Eric Blake <eblake@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Fam Zheng <famz@redhat.com>
Reported-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180410044942.17059-1-peterx@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Add new parameter to optionally enable Out-Of-Band for a QMP server.
An example command line:
./qemu-system-x86_64 -chardev stdio,id=char0 \
-mon chardev=char0,mode=control,x-oob=on
By default, Out-Of-Band is off.
It is not allowed if either MUX or non-QMP is detected, since
Out-Of-Band is currently only for QMP, and non-MUX chardev backends.
Note that the client STILL has to request 'oob' during qmp_capabilities;
in part because the x-oob command line option may disappear in the
future if we decide the capabilities negotiation is sufficient.
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180326063901.27425-4-peterx@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[eblake: enhance commit message]
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Marc-André Lureau reported that we can have this happen:
1. client1 connects, send command C1
2. client1 disconnects before getting response for C1
3. client2 connects, who might receive response of C1
However client2 should not receive remaining responses for client1.
Basically, we should clean up the request/response queue elements when:
- after a session is closed
- before destroying the queues
Some helpers are introduced to achieve that. We need to make sure we're
with the lock when operating on those queues. This also needed the
declaration of QMPRequest moved earlier.
Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180326063901.27425-3-peterx@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[eblake: drop pointless qmp_response_free(), drop queue flush on connect
since a clean queue on disconnect is sufficient]
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
When someone sends a command before QMP handshake, the error used to be
like this:
{"execute": "query-cpus"}
{"error": {"class": "CommandNotFound", "desc":
"Expecting capabilities negotiation with 'qmp_capabilities'"}}
While after cf869d5317 it becomes:
{"execute": "query-cpus"}
{"error": {"class": "CommandNotFound", "desc":
"The command query-cpus has not been found"}}
Fix it back to the nicer one.
Fixes: cf869d5317 ("qmp: support out-of-band (oob) execution", 2018-03-19)
Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180326063901.27425-2-peterx@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: commit message grammar tweaks]
Signed-off-by: Eric Blake <eblake@redhat.com>
This reverts commit 3fd2457d18.
Enabling OOB caused several iotests failures; due to the imminent
2.12 release, the safest action is to disable OOB for now. If
other patches fix the issues that iotests exposed, it may be turned
back on in time for the release, otherwise it will be 2.13 material;
either way, the framework changes not reverted now do not hurt if
they remain as part of the 2.12 release.
Additionally, revert the tests in the patch 02130314d8 ("qmp: introduce
QMPCapability", 2018-03-19), as both parts must be reverted at once
to keep 'make check' passing.
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180323140821.28957-2-peterx@redhat.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
[eblake: reorder/squash commits, enhance commit message]
Signed-off-by: Eric Blake <eblake@redhat.com>
Start to use dedicate IO thread for QMP monitors that are not using
MUXed chardev.
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-21-peterx@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
For those monitors who have enabled IO thread, we'll offload the
responding procedure into IO thread. The main reason is that chardev is
not thread safe, and we need to do all the read/write IOs in the same
thread. For use_io_thr=true monitors, that thread is the IO thread.
We do this isolation in similar pattern as what we have done to the
request queue: we first create one response queue for each monitor, then
instead of replying directly in the main thread, we queue the responses
and kick the IO thread to do the rest of the job for us.
A funny thing after doing this is that, when the QMP clients send "quit"
to QEMU, it's possible that we close the IOThread even earlier than
replying to that "quit". So another thing we need to do before cleaning
up the monitors is that we need to flush the response queue (we don't
need to do that for command queue; after all we are quitting) to make
sure replies for handled commands are always flushed back to clients.
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-20-peterx@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Having "allow-oob":true for a command does not mean that this command
will always be run in out-of-band mode. The out-of-band quick path will
only be executed if we specify the extra "run-oob" flag when sending the
QMP request:
{ "execute": "command-that-allows-oob",
"arguments": { ... },
"control": { "run-oob": true } }
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.
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.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-19-peterx@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: rebase to qobject_to(), spelling fix]
Signed-off-by: Eric Blake <eblake@redhat.com>
Set maximum QMP command queue length to 8. If the queue is full,
instead of queuing the command, we directly return a "command-dropped"
event, telling the client that a specific command is dropped.
Note that this flow control mechanism is only valid if OOB is enabled.
If it's not, the effective queue length will always be 1, which strictly
follows original behavior of QMP command handling (which never drops
messages).
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-17-peterx@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: commit message grammar, abort on failure to send event]
Signed-off-by: Eric Blake <eblake@redhat.com>
Originally QMP goes through these steps:
JSON Parser --> QMP Dispatcher --> Respond
/|\ (2) (3) |
(1) | \|/ (4)
+--------- main thread --------+
This patch does this:
JSON Parser QMP Dispatcher --> Respond
/|\ | /|\ (4) |
| | (2) | (3) | (5)
(1) | +-----> | \|/
+--------- main thread <-------+
So the parsing job and the dispatching job is isolated now. It gives us
a chance in follow up patches to totally move the parser outside.
The isolation is done using one QEMUBH. Only one dispatcher QEMUBH is
used for all the monitors.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-15-peterx@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: grammar tweaks, rebase to qobject_to()]
Signed-off-by: Eric Blake <eblake@redhat.com>
This patches allows QMP monitors to be suspended/resumed.
One thing to mention is that for QMPs that are using IOThreads, we need
an explicit kick for the IOThread in case it is sleeping.
Meanwhile, we need to take special care on non-interactive HMPs.
Currently only gdbserver is using that. For these monitors, we still
don't allow suspend/resume operations.
Since at it, add traces for the operations.
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-14-peterx@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Monitor code now can be run in more than one thread. Let it be thread
safe when accessing suspend_cnt counter.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-13-peterx@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
A tiny refactoring, preparing to split the QMP dispatcher away.
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-12-peterx@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: rebase to qobject_to() usage]
Signed-off-by: Eric Blake <eblake@redhat.com>
There were no QMP capabilities defined. Define the first capability,
"oob", to allow out-of-band messages.
After this patch, we will allow QMP clients to enable QMP capabilities
when sending the first "qmp_capabilities" command. Originally we are
starting QMP session with no arguments like:
{ "execute": "qmp_capabilities" }
Now we can enable some QMP capabilities using (take OOB as example,
which is the only capability that we support):
{ "execute": "qmp_capabilities",
"arguments": { "enable": [ "oob" ] } }
When the "arguments" key is not provided, no capability is enabled.
For capability "oob", the monitor needs to be run on a dedicated IO
thread, otherwise the command will fail. For example, trying to enable
OOB on a MUXed typed QMP monitor will fail.
One thing to mention is that QMP capabilities are per-monitor, and also
when the connection is closed due to some reason, the capabilities will
be reset.
Also, touch up qmp-test.c to test the new bits.
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-11-peterx@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: touch up commit message]
Signed-off-by: Eric Blake <eblake@redhat.com>
For each Monitor, add one field "use_io_thr" to show whether it will be
using the dedicated monitor IO thread to handle input/output. When set,
monitor IO parsing work will be offloaded to the dedicated monitor IO
thread, rather than the original main loop thread.
This only works for QMP. HMP will always be run on the main loop
thread.
Currently we're still keeping use_io_thr off always. Will turn it on
later at some point.
One thing to mention is that we cannot set use_io_thr for every QMP
monitor. The problem is that MUXed typed chardevs may not work well
with it now. When MUX is used, frontend of chardev can be the monitor
plus something else. The only thing we know would be safe to be run
outside main thread so far is the monitor frontend. All the rest of the
frontends should still be run in main thread only.
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-10-peterx@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: squash in Peter's followup patch to avoid test failures]
Signed-off-by: Eric Blake <eblake@redhat.com>
It was QLIST. I want to use this list to do monitor priority job later,
which need tail insertion ability. So switching to a tail queue.
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-9-peterx@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
There are many places where the monitor initializes its globals:
- monitor_init_qmp_commands() at the very beginning
- single function to init monitor_lock
- in the first entry of monitor_init() using "is_first_init"
Unify them a bit.
monitor_lock is not used before monitor_init() (as confirmed by code
analysis and gdb watchpoints); so we are safe delaying what was a
constructor-time initialization of the mutex into the later first call
to monitor_init().
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-8-peterx@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
In monitor_qmp_read(), we have the hack to temporarily replace the
cur_mon pointer. Now we move this hack deeper inside the QMP dispatcher
routine since the Monitor pointer can be actually obtained using
container_of() upon the parser object, just like most of the other JSON
parser users do.
This does not make much sense as a single patch. However, this will be
a big step for the next patch, when the QMP dispatcher routine will be
split from the QMP parser.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-7-peterx@redhat.com>
[eblake: rebase context of qobject_to() macro]
Signed-off-by: Eric Blake <eblake@redhat.com>
It's part of the data init. Collect it.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180309090006.10018-6-peterx@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
This patch was generated using the following Coccinelle script:
@@
expression Obj;
@@
(
- qobject_to_qnum(Obj)
+ qobject_to(QNum, Obj)
|
- qobject_to_qstring(Obj)
+ qobject_to(QString, Obj)
|
- qobject_to_qdict(Obj)
+ qobject_to(QDict, Obj)
|
- qobject_to_qlist(Obj)
+ qobject_to(QList, Obj)
|
- qobject_to_qbool(Obj)
+ qobject_to(QBool, Obj)
)
and a bit of manual fix-up for overly long lines and three places in
tests/check-qjson.c that Coccinelle did not find.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
Message-Id: <20180224154033.29559-4-mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: swap order from qobject_to(o, X), rebase to master, also a fix
to latent false-positive compiler complaint about hw/i386/acpi-build.c]
Signed-off-by: Eric Blake <eblake@redhat.com>
Replace the generated json string with a literal qobject. The later is
easier to deal with, at run time as well as compile time: adding #if
conditionals will be easier than in a json string.
The output of query-qmp-schema is not changed.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180305172951.2150-5-marcandre.lureau@redhat.com>
[eblake: fix python 3 failure]
Signed-off-by: Eric Blake <eblake@redhat.com>
