aboutsummaryrefslogtreecommitdiff
path: root/monitor/qmp.c
AgeCommit message (Collapse)Author
2021-05-12monitor/qmp: fix race on CHR_EVENT_CLOSED without OOBStefan Reiter
The QMP dispatcher coroutine holds the qmp_queue_lock over a yield point, where it expects to be rescheduled from the main context. If a CHR_EVENT_CLOSED event is received just then, it can race and block the main thread on the mutex in monitor_qmp_cleanup_queue_and_resume. monitor_resume does not need to be called from main context, so we can call it immediately after popping a request from the queue, which allows us to drop the qmp_queue_lock mutex before yielding. Suggested-by: Wolfgang Bumiller <w.bumiller@proxmox.com> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com> Message-Id: <20210322154024.15011-1-s.reiter@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Cc: qemu-stable@nongnu.org Signed-off-by: Markus Armbruster <armbru@redhat.com>
2021-03-15monitor: Replaced qemu_mutex_lock calls with QEMU_LOCK_GUARDMahmoud Mandour
Removed various qemu_mutex_lock and their respective qemu_mutex_unlock calls and used lock guard macros (QEMU_LOCK_GUARD and WITH_QEMU_LOCK_GUARD). This simplifies the code by eliminating qemu_mutex_unlock calls. Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com> Message-Id: <20210311031538.5325-6-ma.mandourr@gmail.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
2021-02-15monitor/qmp: Stop processing requests when shutdown is requestedKevin Wolf
Before this patch, monitor_qmp_dispatcher_co() used to check whether shutdown is requested only when it would have to wait for new requests. If there were still some queued requests, it would try to execute all of them before shutting down. This can be surprising when the queued QMP commands take long or hang because Ctrl-C may not actually exit QEMU as soon as possible. Change monitor_qmp_dispatcher_co() so that it additionally checks whether shutdown is request before it gets a new request from the queue. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210212172028.288825-3-kwolf@redhat.com> Tested-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2021-02-04qmp: Resume OOB-enabled monitor before processing the requestMarkus Armbruster
monitor_qmp_dispatcher_co() needs to resume the monitor if handle_qmp_command() suspended it. Two cases: 1. OOB enabled: suspended if mon->qmp_requests has no more space 2. OOB disabled: suspended always We resume only after we processed the request. Which can take a long time. Resume the monitor right when the queue has space to keep the monitor available for out-of-band commands even in this corner case. Leave the "OOB disabled" case alone. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210201161504.1976989-4-armbru@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> [Trailing whitespace tidied up]
2021-02-04qmp: Add more tracepointsMarkus Armbruster
Add tracepoints for in-band request enqueue and dequeue, processing of queued in-band errors, and responses. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210201161504.1976989-3-armbru@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
2021-02-04qmp: Fix up comments after commit 9ce44e2ce2Markus Armbruster
Commit 9ce44e2ce2 "qmp: Move dispatcher to a coroutine" replaced monitor_qmp_bh_dispatcher() by monitor_qmp_dispatcher_co(), but neglected to update comments. Do that now. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20210201161504.1976989-2-armbru@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
2020-12-19qmp: Fix tracing of non-string command IDsMarkus Armbruster
Tracepoints monitor_qmp_cmd_in_band and monitor_qmp_cmd_out_of_band (commit cf869d5317 "qmp: support out-of-band (oob) execution") treat non-string "id" like absent "id". Fix that. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20201211171152.146877-10-armbru@redhat.com>
2020-12-19qobject: Change qobject_to_json()'s value to GStringMarkus Armbruster
qobject_to_json() and qobject_to_json_pretty() build a GString, then covert it to QString. Just one of the callers actually needs a QString: qemu_rbd_parse_filename(). A few others need a string they can modify: qmp_send_response(), qga's send_response(), to_json_str(), and qmp_fd_vsend_fds(). The remainder just need a string. Change qobject_to_json() and qobject_to_json_pretty() to return the GString. qemu_rbd_parse_filename() now has to convert to QString. All others save a QString temporary. to_json_str() actually becomes a bit simpler, because GString provides more convenient modification functions. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20201211171152.146877-6-armbru@redhat.com>
2020-12-19qobject: Make qobject_to_json_pretty() take a pretty argumentMarkus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20201211171152.146877-4-armbru@redhat.com>
2020-10-09qmp: Move dispatcher to a coroutineKevin Wolf
This moves the QMP dispatcher to a coroutine and runs all QMP command handlers that declare 'coroutine': true in coroutine context so they can avoid blocking the main loop while doing I/O or waiting for other events. For commands that are not declared safe to run in a coroutine, the dispatcher drops out of coroutine context by calling the QMP command handler from a bottom half. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20201005155855.256490-10-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09qmp: Call monitor_set_cur() only in qmp_dispatch()Kevin Wolf
The correct way to set the current monitor for a coroutine handler will be different than for a blocking handler, so monitor_set_cur() needs to be called in qmp_dispatch(). Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201005155855.256490-7-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09qmp: Assert that no other monitor is activeKevin Wolf
monitor_qmp_dispatch() is never supposed to be called in the context of another monitor, so assert that monitor_cur() is NULL instead of saving and restoring it. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201005155855.256490-6-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-10-09monitor: Use getter/setter functions for cur_monKevin Wolf
cur_mon really needs to be coroutine-local as soon as we move monitor command handlers to coroutines and let them yield. As a first step, just remove all direct accesses to cur_mon so that we can implement this in the getter function later. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20201005155855.256490-4-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-04-30qapi: Disallow qmp_marshal_FOO(NULL, ...)Markus Armbruster
For QMP commands without arguments, gen_marshal() laboriously generates a qmp_marshal_FOO() that copes with null @args. Turns there's just one caller that passes null instead of an empty QDict. Adjust that caller, and simplify gen_marshal(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200424084338.26803-15-armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
2020-03-06qmp: Fail gracefully if chardev is already in useKevin Wolf
Trying to attach a QMP monitor to a chardev that is already in use results in a crash because monitor_init_qmp() passes &error_abort to qemu_chr_fe_init(): $ ./x86_64-softmmu/qemu-system-x86_64 --chardev stdio,id=foo --mon foo,mode=control --mon foo,mode=control Unexpected error in qemu_chr_fe_init() at chardev/char-fe.c:220: qemu-system-x86_64: --mon foo,mode=control: Device 'foo' is in use Abgebrochen (Speicherabzug geschrieben) Fix this by allowing monitor_init_qmp() to return an error and passing any error in qemu_chr_fe_init() to its caller instead of aborting. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20200224143008.13362-18-kwolf@redhat.com> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2020-02-17qapi: Split control.json off misc.jsonKevin Wolf
misc.json contains definitions that are related to the system emulator, so it can't be used for other tools like the storage daemon. This patch moves basic functionality that is shared between all tools (and mostly related to the monitor itself) into a new control.json, which could be used in tools as well. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200129102239.31435-3-kwolf@redhat.com> [Commit message tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com>
2020-01-08chardev: Use QEMUChrEvent enum in IOEventHandler typedefPhilippe Mathieu-Daudé
The Chardev events are listed in the QEMUChrEvent enum. By using the enum in the IOEventHandler typedef we: - make the IOEventHandler type more explicit (this handler process out-of-band information, while the IOReadHandler is in-band), - help static code analyzers. This patch was produced with the following spatch script: @match@ expression backend, opaque, context, set_open; identifier fd_can_read, fd_read, fd_event, be_change; @@ qemu_chr_fe_set_handlers(backend, fd_can_read, fd_read, fd_event, be_change, opaque, context, set_open); @depends on match@ identifier opaque, event; identifier match.fd_event; @@ static -void fd_event(void *opaque, int event) +void fd_event(void *opaque, QEMUChrEvent event) { ... } Then the typedef was modified manually in include/chardev/char-fe.h. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Acked-by: Corey Minyard <cminyard@mvista.com> Acked-by: Cornelia Huck <cohuck@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <20191218172009.8868-15-philmd@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-01-07monitor/qmp: Explicit we ignore few QEMUChrEvent in IOEventHandlerPhilippe Mathieu-Daudé
The Chardev events are listed in the QEMUChrEvent enum. To be able to use this enum in the IOEventHandler typedef, we need to explicit all the events ignored by this frontend, to silent the following GCC warning: CC monitor/qmp.o monitor/qmp.c: In function ‘monitor_qmp_event’: monitor/qmp.c:345:5: error: enumeration value ‘CHR_EVENT_BREAK’ not handled in switch [-Werror=switch] 345 | switch (event) { | ^~~~~~ monitor/qmp.c:345:5: error: enumeration value ‘CHR_EVENT_MUX_IN’ not handled in switch [-Werror=switch] monitor/qmp.c:345:5: error: enumeration value ‘CHR_EVENT_MUX_OUT’ not handled in switch [-Werror=switch] cc1: all warnings being treated as errors Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20191218172009.8868-12-philmd@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2019-11-19monitor/qmp: resume monitor when clearing its queueWolfgang Bumiller
When a monitor's queue is filled up in handle_qmp_command() it gets suspended. It's the dispatcher bh's job currently to resume the monitor, which it does after processing an event from the queue. However, it is possible for a CHR_EVENT_CLOSED event to be processed before before the bh is scheduled, which will clear the queue without resuming the monitor, thereby preventing the dispatcher from reaching the resume() call. Any new connections to the qmp socket will be accept()ed and show the greeting, but will not respond to any messages sent afterwards (as they will not be read from the still-suspended socket). Fix this by resuming the monitor when clearing a queue which was filled up. Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com> Message-Id: <20191115085914.21287-1-w.bumiller@proxmox.com>
2019-08-21monitor/qmp: Update comment for commit 4eaca8de268Markus Armbruster
Commit 4eaca8de268 dropped monitor_qmp_respond()'s parameter @id without updating its function comment. Fix that. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20190816193305.12090-1-armbru@redhat.com>
2019-06-18monitor: Replace monitor_init() with monitor_init_{hmp, qmp}()Kevin Wolf
Most callers know which monitor type they want to have. Instead of calling monitor_init() with flags that can describe both types of monitors, make monitor_init_{hmp,qmp}() public interfaces that take specific bools instead of flags and call these functions directly. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20190613153405.24769-15-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2019-06-18monitor: Split Monitor.flags into separate boolsKevin Wolf
Monitor.flags contains three different flags: One to distinguish HMP from QMP; one specific to HMP (MONITOR_USE_READLINE) that is ignored with QMP; and another one specific to QMP (MONITOR_USE_PRETTY) that is ignored with HMP. Split the flags field into three bools and move them to the right subclass. Flags are still in use for the monitor_init() interface. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20190613153405.24769-14-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
2019-06-17monitor: Split out monitor/qmp.cKevin Wolf
Move QMP infrastructure from monitor/misc.c to monitor/qmp.c. This is code that can be shared for all targets, so compile it only once. The amount of function and particularly extern variables in monitor_int.h is probably a bit larger than it needs to be, but this way no non-trivial code modifications are needed. The interfaces between QMP and the monitor core can be cleaned up later. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Message-Id: <20190613153405.24769-11-kwolf@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [monitor_is_qmp() tidied up to make checkpatch.pl happy, superfluous #include dropped] Signed-off-by: Markus Armbruster <armbru@redhat.com>
generated by cgit v1.2.3 (git 2.26.2) at 2024-12-14 10:25:49 +0000