Commit Graph

22 Commits

Author SHA1 Message Date
Mahmoud Mandour
a8e2ab5db2 monitor: Replaced qemu_mutex_lock calls with QEMU_LOCK_GUARD
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-03-15 20:01:55 +00:00
Kevin Wolf
b248e61652 monitor/qmp: Stop processing requests when shutdown is requested
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-15 15:10:14 +01:00
Markus Armbruster
88daf0996c qmp: Resume OOB-enabled monitor before processing the request
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-04 13:20:29 +01:00
Markus Armbruster
f680405f45 qmp: Add more tracepoints
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-04 13:20:29 +01:00
Markus Armbruster
395a95080a qmp: Fix up comments after commit 9ce44e2ce2
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>
2021-02-04 13:20:29 +01:00
Markus Armbruster
d403d92dfc qmp: Fix tracing of non-string command IDs
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-19 10:38:43 +01:00
Markus Armbruster
eab3a4678b qobject: Change qobject_to_json()'s value to GString
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-19 10:38:43 +01:00
Markus Armbruster
6589f45991 qobject: Make qobject_to_json_pretty() take a pretty argument
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20201211171152.146877-4-armbru@redhat.com>
2020-12-19 10:38:43 +01:00
Kevin Wolf
9ce44e2ce2 qmp: Move dispatcher to a coroutine
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-09 07:08:20 +02:00
Kevin Wolf
41725fa7ed qmp: Call monitor_set_cur() only in qmp_dispatch()
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-09 07:08:19 +02:00
Kevin Wolf
57d3635e42 qmp: Assert that no other monitor is active
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-09 07:08:19 +02:00
Kevin Wolf
947e47448d monitor: Use getter/setter functions for cur_mon
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-10-09 07:08:19 +02:00
Markus Armbruster
2061487bdb qapi: Disallow qmp_marshal_FOO(NULL, ...)
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-04-30 07:26:41 +02:00
Kevin Wolf
f27a9bb3e9 qmp: Fail gracefully if chardev is already in use
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-03-06 17:21:28 +01:00
Kevin Wolf
fa4dcf577e qapi: Split control.json off misc.json
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-02-17 13:53:47 +01:00
Philippe Mathieu-Daudé
083b266f69 chardev: Use QEMUChrEvent enum in IOEventHandler typedef
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-08 11:15:35 +01:00
Philippe Mathieu-Daudé
ed7c5bb7ad monitor/qmp: Explicit we ignore few QEMUChrEvent in IOEventHandler
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>
2020-01-07 16:22:56 +01:00
Wolfgang Bumiller
2895aaa139 monitor/qmp: resume monitor when clearing its queue
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-11-19 08:21:47 +01:00
Markus Armbruster
eb707eac58 monitor/qmp: Update comment for commit 4eaca8de26
Commit 4eaca8de26 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-08-21 17:33:38 +02:00
Kevin Wolf
fbfc29e3bf monitor: Replace monitor_init() with monitor_init_{hmp, qmp}()
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-18 08:14:17 +02:00
Kevin Wolf
920824165c monitor: Split Monitor.flags into separate bools
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-18 08:14:17 +02:00
Kevin Wolf
7e3c0deab1 monitor: Split out monitor/qmp.c
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>
2019-06-17 20:36:56 +02:00