Monitor patches for 2026-07-07
Subsystems (4)
The following changes since commit 94826ec1370328375c3b6d1e80fdc94c8f46c348:
Merge tag 'accel-20260706' of https://github.com/philmd/qemu into staging (2026-07-06 18:38:14 +0200)
are available in the Git repository at:
https://repo.or.cz/qemu/armbru.git tags/pull-monitor-2026-07-07
for you to fetch changes up to 58b70f21ba6364be4f79d9f9c56b334341051bfd:
docs: mark '-mon' as deprecated in favour of -object (2026-07-07 11:16:27 +0200)
----------------------------------------------------------------
Monitor patches for 2026-07-07
----------------------------------------------------------------
Christian Brauner (6):
monitor: convert from oneshot BH to persistent BH
monitor: reject attempts to delete the current monitor
monitor: protect qemu_chr_fe_accept_input with monitor lock
monitor: implement support for deleting QMP objects tests/qtest: add tests for dynamic monitor add/remove tests/functional: add e2e test for dynamic QMP monitor hotplug
Daniel P. Berrangé (29):
qom: replace 'can_be_deleted' with 'prepare_delete'
monitor: replace 'common' with 'parent_obj' in MonitorHMP
monitor: replace 'common' with 'parent_obj' in MonitorQMP
monitor: rename monitor_init* to monitor_new*
monitor: minimal conversion of monitors to QOM
monitor: pass chardev ID into monitor constructor instead of object
monitor: add 'chardev' property to Monitor base class
monitor: add 'readline' property to HMP Monitor class
monitor: add 'pretty' property to QMP Monitor class
monitor: remove 'skip_flush' field
monitor: move monitor_data_(init|destroy) into QOM init/finalize
monitor: use class methods for monitor_vprintf
monitor: use class methods for monitor_qapi_event_emit
monitor: use class methods for monitor_accept_input
monitor: use class method for I/O thread request
monitor: use dynamic cast in monitor_qmp_requests_pop_any_with_lock
util: use dynamic cast in error vreport
monitor: drop unused monitor_cur_is_qmp
monitor: use dynamic cast in QMP commands
monitor: use dynamic cast in monitor_is_hmp_non_interactive
monitor: drop unused monitor_is_qmp method
monitor: eliminate monitor_is_hmp_non_interactive method
monitor: implement "user creatable" interface for adding monitors tests/functional: add a stress test for monitor hot unplug
qom: add trace events for user creatable create/delete APIs
monitor: add support for auto-deleting monitors upon close
tests: switch from -mon to -object monitor-qmp
qemu-options: document new monitor-hmp and monitor-qmp objects
docs: mark '-mon' as deprecated in favour of -object
MAINTAINERS | 1 + docs/about/deprecated.rst | 10 + docs/devel/writing-monitor-commands.rst | 4 +- docs/system/arm/xenpvh.rst | 4 +- docs/system/i386/xen.rst | 3 +- docs/system/i386/xenpvh.rst | 4 +- qapi/qom.json | 63 +++++ include/monitor/monitor.h | 26 +- include/qom/object_interfaces.h | 26 +- include/system/event-loop-base.h | 2 +- monitor/monitor-internal.h | 73 +++-- backends/cryptodev.c | 12 +- backends/hostmem.c | 7 +- backends/iommufd.c | 11 +- block/throttle-groups.c | 12 +- chardev/char.c | 3 +- event-loop-base.c | 8 +- gdbstub/system.c | 2 +- migration/migration-hmp-cmds.c | 5 +- monitor/hmp-cmds.c | 7 +- monitor/hmp.c | 181 ++++++++++--- monitor/monitor.c | 273 ++++++++++--------- monitor/qmp-cmds-control.c | 12 +- monitor/qmp-cmds.c | 14 +- monitor/qmp.c | 323 ++++++++++++++++++++--- net/can/can_core.c | 6 +- qom/object_interfaces.c | 20 +- storage-daemon/qemu-storage-daemon.c | 2 +- stubs/monitor-core.c | 5 - stubs/monitor-internal.c | 4 +- system/vl.c | 19 +- tests/qtest/libqtest.c | 2 +- tests/qtest/qmp-test.c | 174 ++++++++++++ tests/unit/test-util-sockets.c | 1 - tools/qemu-vnc/stubs.c | 5 - ui/ui-hmp-cmds.c | 2 +- util/error-report.c | 13 +- util/main-loop.c | 6 +- python/qemu/machine/machine.py | 4 +- qemu-options.hx | 64 ++++- qom/trace-events | 5 + tests/functional/generic/meson.build | 1 + tests/functional/generic/test_monitor_hotplug.py | 305 +++++++++++++++++++++ tests/qemu-iotests/245 | 4 +- 44 files changed, 1395 insertions(+), 333 deletions(-) create mode 100755 tests/functional/generic/test_monitor_hotplug.py
Signature · 1 line
2.54.0
[PULL 26/35] monitor: protect qemu_chr_fe_accept_input with monitor lock
From: Christian Brauner <[email protected]>
The monitor_accept_input API is called from a bottom half, and will invoke qemu_chr_fe_accept_input().
When a following patch introduces the ability to delete monitors, it will be neccesary to delete the bottom half. Protecting the call to qemu_chr_fe_accept_input with the monitor lock will allow for synchronization with the deletion process.
[DB: extracted from a larger commit and refactored to apply
to the new monitor class structure]
Message-ID: <[email protected]>
--- monitor/hmp.c | 2 ++ monitor/monitor.c | 6 +----- monitor/qmp.c | 9 +++++++++ 3 files changed, 12 insertions(+), 5 deletions(-)
monitor/hmp.c +2 -0
@@ -116,9 +116,11 @@ static void monitor_hmp_accept_input(Monitor *mon)MonitorHMP *hmp =MONITOR_HMP (mon);assert (hmp->rs);readline_restart (hmp->rs); +qemu_chr_fe_accept_input (&mon->chr);qemu_mutex_unlock (&mon->mon_lock);readline_show_prompt (hmp->rs); }else { +qemu_chr_fe_accept_input (&mon->chr);qemu_mutex_unlock (&mon->mon_lock); } }
monitor/monitor.c +1 -5
@@ -562,11 +562,7 @@ static void monitor_accept_input(void *opaque)Monitor *mon = opaque;MonitorClass *cls =MONITOR_GET_CLASS (mon); -if (cls->accept_input) { - cls->accept_input (mon); - } - -qemu_chr_fe_accept_input (&mon->chr); + cls->accept_input (mon); }void monitor_resume (Monitor *mon)
monitor/qmp.c +9 -0
@@ -107,6 +107,7 @@ static void monitor_qmp_emit_event(Monitor *mon, QAPIEvent event, QDict *qdict);static bool monitor_qmp_requires_iothread (const Monitor *mon);static void monitor_qmp_complete (UserCreatable *uc,Error **errp);static bool monitor_qmp_prepare_delete (UserCreatable *uc,Error **errp); +static void monitor_qmp_accept_input (Monitor *mon);static void monitor_qmp_class_init (ObjectClass *cls,const void *data) {
@@ -119,6 +120,7 @@ static void monitor_qmp_class_init(ObjectClass *cls, const void *data) moncls->emit_event = monitor_qmp_emit_event; moncls->requires_iothread = monitor_qmp_requires_iothread; + moncls->accept_input = monitor_qmp_accept_input; ucc->complete = monitor_qmp_complete; ucc->prepare_delete = monitor_qmp_prepare_delete;
@@ -665,3 +667,10 @@ static bool monitor_qmp_prepare_delete(UserCreatable *uc, Error **errp)error_setg (errp,"Deleting QMP monitors is not supported" );return false; } + +static void monitor_qmp_accept_input (Monitor *mon) +{ + WITH_QEMU_LOCK_GUARD(&mon->mon_lock) { +qemu_chr_fe_accept_input (&mon->chr); + } +}
Signature · 1 line
2.54.0
[PULL 30/35] tests/functional: add a stress test for monitor hot unplug
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
When unplugging a monitor there is a careful synchronization dance between the monitor handling the "object-del" command and the command processing for the monitor being deleted.
The stress test runs a busy loop of 'query-qmp-schema' on a second monitor, while the primary monitor requests its deletion.
Message-ID: <[email protected]>
--- .../generic/test_monitor_hotplug.py | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+)
tests/functional/generic/test_monitor_hotplug.py +62 -0
@@ -6,11 +6,16 @@ # # Copyright (c) 2026 Christian Brauner +import asyncio import os +import random +import threading +import time from qemu_test import QemuSystemTest from qemu.qmp.legacy import QEMUMonitorProtocol +from qemu.qmp import QMPClient class MonitorHotplug(QemuSystemTest):
@@ -163,6 +168,63 @@ def test_events_after_negotiation(self): qmp.close() self._remove_monitor() + def stress_mon(self, sock): + async def main(): + qmp = QMPClient('testvm') + await qmp.connect(sock) + # Run query-version in a tight loop so that the + # monitor thread/dispatcher is very busy at the + # time we try to delete the monitor + while True: + try: + # A command which returns a lot of data to make + # it more likely we're in the I/O reply path + # when deleting the monitor + res = await qmp.execute('query-qmp-schema') + # Some commands which generate async events + # as those can trigger different code paths + res = await qmp.execute('stop') + res = await qmp.execute('cont') + except: + # we'll get here if the monitor is terminated + # by QEMU in which case we must disconnect + # out side, but.... + try: + await qmp.disconnect() + except (ConnectionResetError, EOFError, BrokenPipeError): + # ... disconnect() will probably see + # errors too, but we must try to call it + # regardless to cleanup asyncio state + # and prevent python warnings at GC time + pass + return + asyncio.run(main()) + + def test_hotplug_stress(self): + """ + Repeatedly hotplug and unplug a monitor, while another thread + concurrently issues commands on that monitor. This stresses + the synchronization with the monitor thread during cleanup + """ + self.set_machine('none') + self.vm.add_args('-nodefaults') + self.vm.launch() + + # Each loop sleeps at most 0.5 seconds, so this should + # give an upper bound of approx 5 seconds execution + # time which is reasonable to run by default + repeat = 10 + for i in range(repeat): + # First cycle + sock = self._add_monitor() + print ("# stress cycle %02d/%02d" % (i, repeat)) + stress = threading.Thread(target=self.stress_mon, args=[sock]) + stress.start() + # Sleep upto 1/2 second to vary the races + time.sleep(random.random() / 2) + self._remove_monitor() + stress.join() + if __name__ == '__main__': QemuSystemTest.main()
Signature · 1 line
2.54.0
[PULL 08/35] monitor: add 'readline' property to HMP Monitor class
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
This determines whether a human monitor runs with readline for interactive use, or without readline for non-interactive use by the GDB stub.
Message-ID: <[email protected]>
--- monitor/hmp.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-)
monitor/hmp.c +26 -1
@@ -50,12 +50,37 @@ static void monitor_hmp_finalize(Object *obj) { } +static bool monitor_hmp_get_readline (Object *obj,Error **errp) +{ +MonitorHMP *mon =MONITOR_HMP (obj); + +return mon->use_readline; +} + +static void monitor_hmp_set_readline (Object *obj,bool val,Error **errp) +{ +MonitorHMP *mon =MONITOR_HMP (obj); + + mon->use_readline = val; +} +static void monitor_hmp_class_init (ObjectClass *cls,const void *data) { +object_class_property_add_bool (cls,"readline" , + monitor_hmp_get_readline, + monitor_hmp_set_readline); }static void monitor_hmp_init (Object *obj) { +MonitorHMP *hmp =MONITOR_HMP (obj); + +/* +* Default to common case for external HMP use, +* as opposed to non-interactive internal use +* from gdbstub +*/ + hmp->use_readline = true; } static void monitor_command_cb(void *opaque, const char *cmdline,
@@ -1550,6 +1575,7 @@ void monitor_new_hmp(const char *id, const char *chardev_id, id ? id : autoid, errp,"chardev" , chardev_id, +"readline" , use_readline ?"yes" :"no" , NULL);if (!obj) {
@@ -1566,7 +1592,6 @@ void monitor_new_hmp(const char *id, const char *chardev_id,monitor_data_init (&mon->parent_obj, false, false, false); - mon->use_readline = use_readline;if (mon->use_readline) { mon->rs = readline_init(monitor_readline_printf, monitor_readline_flush,
Signature · 1 line
2.54.0
[PULL 18/35] monitor: drop unused monitor_cur_is_qmp
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
The previous patch dropped the only remaining use of monitor_cur_is_qmp.
Message-ID: <[email protected]>
--- include/monitor/monitor.h | 1 - monitor/monitor.c | 10 ---------- stubs/monitor-core.c | 5 ----- tests/unit/test-util-sockets.c | 1 - tools/qemu-vnc/stubs.c | 5 ----- 5 files changed, 22 deletions(-)
include/monitor/monitor.h +0 -1
@@ -25,7 +25,6 @@ extern QemuOptsList qemu_mon_opts;Monitor *monitor_cur (void );Monitor *monitor_set_cur (Coroutine *co,Monitor *mon); -bool monitor_cur_is_qmp (void );void monitor_init_globals (void );void monitor_init_globals_core (void );
monitor/monitor.c +0 -10
@@ -152,16 +152,6 @@ Monitor *monitor_set_cur(Coroutine *co, Monitor *mon)return old_monitor; } -/** -* Is the current monitor, if any, a QMP monitor? -*/ -bool monitor_cur_is_qmp (void ) -{ -Monitor *cur_mon =monitor_cur (); - -return cur_mon &&monitor_is_qmp (cur_mon); -} -bool monitor_requires_iothread (const Monitor *mon) {MonitorClass *cls =MONITOR_GET_CLASS (mon);
stubs/monitor-core.c +0 -5
@@ -7,11 +7,6 @@ Monitor *monitor_cur(void)return NULL ; } -bool monitor_cur_is_qmp (void ) -{ -return false; -} -Monitor *monitor_set_cur (Coroutine *co,Monitor *mon) {return NULL ;
tests/unit/test-util-sockets.c +0 -1
@@ -74,7 +74,6 @@ int monitor_get_fd(Monitor *mon, const char *fdname, Error **errp)Monitor *monitor_cur (void ) {return cur_mon; }Monitor *monitor_set_cur (Coroutine *co,Monitor *mon) {abort (); }int monitor_vprintf (Monitor *mon,const char *fmt,va_list ap) {abort (); } -bool monitor_cur_is_qmp (void ) {abort (); };#ifndef _WIN32static void test_socket_fd_pass_name_good (void )
tools/qemu-vnc/stubs.c +0 -5
@@ -36,11 +36,6 @@ Monitor *monitor_cur(void)return NULL ; } -bool monitor_cur_is_qmp (void ) -{ -return false; -} -Monitor *monitor_set_cur (Coroutine *co,Monitor *mon) {return NULL ;
Signature · 1 line
2.54.0
[PULL 13/35] monitor: use class methods for monitor_qapi_event_emit
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
This removes the need for using monitor_is_qmp() to check the subclass type, which is an anti-pattern.
Message-ID: <[email protected]>
--- include/monitor/monitor.h | 1 + monitor/monitor-internal.h | 5 +++++ monitor/monitor.c | 15 +++------------ monitor/qmp.c | 20 ++++++++++++++++++++ 4 files changed, 29 insertions(+), 12 deletions(-)
include/monitor/monitor.h +1 -0
@@ -3,6 +3,7 @@#include "block/block.h" #include "qapi/qapi-types-misc.h" +#include "qapi/qapi-emit-events.h" #include "qemu/readline.h" #include "exec/hwaddr.h" #include "qom/object.h"
monitor/monitor-internal.h +5 -0
@@ -111,6 +111,11 @@ struct MonitorClass {*/ int (*vprintf)(Monitor *mon, const char *fmt, va_list ap) G_GNUC_PRINTF(2, 0); +/* +* If non-NULL, the monitor is able to send event +* notifications back to the client +*/ +void (*emit_event)(Monitor *mon,QAPIEvent event,QDict *qdict); };struct Monitor {
monitor/monitor.c +3 -12
@@ -348,22 +348,13 @@ static inline QEMUClockType monitor_get_event_clock(void)static void monitor_qapi_event_emit (QAPIEvent event,QDict *qdict) {Monitor *mon; -MonitorQMP *qmp_mon;trace_monitor_protocol_event_emit (event, qdict); QTAILQ_FOREACH(mon, &mon_list, entry) { -if (!monitor_is_qmp (mon)) { -continue ; +MonitorClass *cls =MONITOR_GET_CLASS (mon); +if (cls->emit_event) { + cls->emit_event (mon, event, qdict); } - - qmp_mon =container_of (mon, MonitorQMP, parent_obj); - { -QEMU_LOCK_GUARD (&mon->mon_lock); -if (qmp_mon->commands == &qmp_cap_negotiation_commands) { -continue ; - } - } -qmp_send_response (qmp_mon, qdict); } }
monitor/qmp.c +20 -0
@@ -99,11 +99,17 @@ static void monitor_qmp_set_pretty(Object *obj, bool val, Error **errp) mon->pretty = val; } +static void monitor_qmp_emit_event (Monitor *mon,QAPIEvent event,QDict *qdict); +static void monitor_qmp_class_init (ObjectClass *cls,const void *data) { +MonitorClass *moncls =MONITOR_CLASS (cls); +object_class_property_add_bool (cls,"pretty" , monitor_qmp_get_pretty, monitor_qmp_set_pretty); + + moncls->emit_event = monitor_qmp_emit_event; }static void handle_qmp_command (void *opaque,QObject *req,Error *err);
@@ -117,6 +123,20 @@ static void monitor_qmp_init(Object *obj)json_message_parser_init (&mon->parser, handle_qmp_command, mon,NULL ); } +static void monitor_qmp_emit_event (Monitor *mon,QAPIEvent event,QDict *qdict) +{ +MonitorQMP *qmp =MONITOR_QMP (mon); + + WITH_QEMU_LOCK_GUARD(&mon->mon_lock) { +if (qmp->commands == &qmp_cap_negotiation_commands) { +return ; + } + } + +qmp_send_response (qmp, qdict); +} + +static bool qmp_oob_enabled (MonitorQMP *mon) {return mon->capab[QMP_CAPABILITY_OOB ];
Signature · 1 line
2.54.0
[PULL 31/35] qom: add trace events for user creatable create/delete APIs
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
Message-ID: <[email protected]>
--- qom/object_interfaces.c | 11 +++++++++-- qom/trace-events | 5 +++++ 2 files changed, 14 insertions(+), 2 deletions(-)
qom/object_interfaces.c +9 -2
@@ -20,12 +20,14 @@#include "qapi/opts-visitor.h" #include "qemu/config-file.h" #include "qemu/keyval.h" +#include "trace.h" bool user_creatable_complete (UserCreatable *uc,Error **errp) {UserCreatableClass *ucc =USER_CREATABLE_GET_CLASS (uc);ERRP_GUARD (); +trace_user_creatable_complete (uc,object_get_typename (OBJECT (uc)));if (ucc->complete) { ucc->complete (uc, errp); }
@@ -35,11 +37,16 @@ bool user_creatable_complete(UserCreatable *uc, Error **errp)bool user_creatable_prepare_delete (UserCreatable *uc,Error **errp) {UserCreatableClass *ucc =USER_CREATABLE_GET_CLASS (uc); +bool ret = true; +trace_user_creatable_prepare_delete (uc,object_get_typename (OBJECT (uc)));if (ucc->prepare_delete) { -return ucc->prepare_delete (uc, errp); + ret = ucc->prepare_delete (uc, errp); } -return true; +trace_user_creatable_prepare_delete_result ( + uc,object_get_typename (OBJECT (uc)), + *errp ?error_get_pretty (*errp) :NULL ); +return ret; }void user_creatable_add_qapi (ObjectOptions *options,Error **errp)
qom/trace-events +5 -0
@@ -11,3 +11,8 @@ object_property_del_child(void *obj, const char *type, void *child, const char * object_property_parse(void *obj, const char *type, const char *name, const char *value) "obj=%p type=%s prop=%s value=%s" object_class_dynamic_cast_assert(const char *type, const char *target, const char *file, int line, const char *func) "type=%s->%s (%s:%d:%s)" object_class_property_add(const char *type, const char *name, void *value) "type=%s name=%s value=%p" + +# object_interfaces.c +user_creatable_complete(void *obj, const char *type) "obj=%p type=%s" +user_creatable_prepare_delete(void *obj, const char *type) "obj=%p type=%s" +user_creatable_prepare_delete_result(void *obj, const char *type, const char *msg) "obj=%p type=%s msg=%s"
Signature · 1 line
2.54.0
[PULL 06/35] monitor: pass chardev ID into monitor constructor instead of object
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
Current the monitor_new_hmp/monitor_new_qmp constructors accept a Chardev object pointer. To facilitate the next commit which will introduce a QOM property for the character device ID, switch to accepting an chardev ID in the constructor.
Message-ID: <[email protected]>
--- include/monitor/monitor.h | 4 ++-- chardev/char.c | 3 ++- gdbstub/system.c | 2 +- monitor/hmp.c | 22 ++++++++++++++++------ monitor/monitor.c | 11 ++--------- monitor/qmp.c | 21 +++++++++++++++------ stubs/monitor-internal.c | 2 +- 7 files changed, 39 insertions(+), 26 deletions(-)
include/monitor/monitor.h +2 -2
@@ -29,9 +29,9 @@ bool monitor_cur_is_qmp(void);void monitor_init_globals (void );void monitor_init_globals_core (void );char *monitor_compat_id (void ); -void monitor_new_qmp (const char *id,Chardev *chr, +void monitor_new_qmp (const char *id,const char *chardev_id,bool pretty,Error **errp); -void monitor_new_hmp (const char *id,Chardev *chr, +void monitor_new_hmp (const char *id,const char *chardev_id,bool use_readline,Error **errp);int monitor_new (MonitorOptions *opts,bool allow_hmp,Error **errp);int monitor_new_opts (QemuOpts *opts,Error **errp);
chardev/char.c +2 -1
@@ -804,8 +804,9 @@ static Chardev *qemu_chr_new_from_name(const char *label, const char *filename, }if (qemu_opt_get_bool (opts,"mux" ,0 )) { +const char *chardev_id =qemu_opts_id (opts);assert (permit_mux_mon); -monitor_new_hmp (NULL , chr, true, &err); +monitor_new_hmp (NULL , chardev_id, true, &err);if (err) {error_report_err (err);object_unparent (OBJECT (chr));
gdbstub/system.c +1 -1
@@ -390,7 +390,7 @@ bool gdbserver_start(const char *device, Error **errp)/* Initialize a monitor terminal for gdb */ mon_chr =qemu_chardev_new (NULL ,TYPE_CHARDEV_GDB ,NULL ,NULL , &error_abort); -monitor_new_hmp (NULL , mon_chr, false, &error_abort); +monitor_new_hmp (NULL , mon_chr->label, false, &error_abort); }else {qemu_chr_fe_deinit (&gdbserver_system_state.chr, true); mon_chr = gdbserver_system_state.mon_chr;
monitor/hmp.c +16 -6
@@ -39,6 +39,7 @@#include "qemu/base-arch-defs.h" #include "qemu/target-info.h" #include "qemu/units.h" +#include "qapi/error.h" #include "exec/gdbstub.h" #include "system/block-backend.h" #include "trace.h"
@@ -1538,16 +1539,25 @@ static void monitor_readline_flush(void *opaque)monitor_flush (&mon->parent_obj); } -void monitor_new_hmp (const char *id,Chardev *chr, +void monitor_new_hmp (const char *id,const char *chardev_id,bool use_readline,Error **errp) {MonitorHMP *mon; g_autofree char *autoid = id ? NULL : monitor_compat_id(); -Object *obj =object_new_with_props (TYPE_MONITOR_HMP , -object_get_objects_root (), - id ? id : autoid, - errp, -NULL ); +Chardev *chr; +Object *obj; + + chr =qemu_chr_find (chardev_id); +if (chr ==NULL ) { +error_setg (errp,"chardev \"%s\" not found" , chardev_id); +return ; + } + + obj =object_new_with_props (TYPE_MONITOR_HMP , +object_get_objects_root (), + id ? id : autoid, + errp, +NULL );if (!obj) {return ; }
monitor/monitor.c +2 -9
@@ -741,13 +741,6 @@ char *monitor_compat_id(void)int monitor_new (MonitorOptions *opts,bool allow_hmp,Error **errp) {ERRP_GUARD (); -Chardev *chr; - - chr =qemu_chr_find (opts->chardev); -if (chr ==NULL ) { -error_setg (errp,"chardev \"%s\" not found" , opts->chardev); -return -1 ; - }if (!opts->has_mode) { opts->mode = allow_hmp ?MONITOR_MODE_READLINE :MONITOR_MODE_CONTROL ;
@@ -755,7 +748,7 @@ int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp)switch (opts->mode) {case MONITOR_MODE_CONTROL : -monitor_new_qmp (opts->id, chr, opts->pretty, errp); +monitor_new_qmp (opts->id, opts->chardev, opts->pretty, errp);break ;case MONITOR_MODE_READLINE :if (!allow_hmp) {
@@ -766,7 +759,7 @@ int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp)error_setg (errp,"'pretty' is not compatible with HMP monitors" );return -1 ; } -monitor_new_hmp (opts->id, chr, true, errp); +monitor_new_hmp (opts->id, opts->chardev, true, errp);break ;default :g_assert_not_reached ();
monitor/qmp.c +15 -6
@@ -527,16 +527,25 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)monitor_list_append (&mon->parent_obj); } -void monitor_new_qmp (const char *id,Chardev *chr, +void monitor_new_qmp (const char *id,const char *chardev_id,bool pretty,Error **errp) {MonitorQMP *mon; g_autofree char *autoid = id ? NULL : monitor_compat_id(); -Object *obj =object_new_with_props (TYPE_MONITOR_QMP , -object_get_objects_root (), - id ? id : autoid, - errp, -NULL ); +Chardev *chr; +Object *obj; + + chr =qemu_chr_find (chardev_id); +if (chr ==NULL ) { +error_setg (errp,"chardev \"%s\" not found" , chardev_id); +return ; + } + + obj =object_new_with_props (TYPE_MONITOR_QMP , +object_get_objects_root (), + id ? id : autoid, + errp, +NULL );if (!obj) {return ; }
stubs/monitor-internal.c +1 -1
@@ -8,7 +8,7 @@ int monitor_get_fd(Monitor *mon, const char *name, Error **errp)return -1 ; } -void monitor_new_hmp (const char *id,Chardev *chr, +void monitor_new_hmp (const char *id,const char *chardev_id,bool use_readline,Error **errp) { }
Signature · 1 line
2.54.0
[PULL 11/35] monitor: move monitor_data_(init|destroy) into QOM init/finalize
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
Start to take advantage of QOM, by using object init and finalize methods to replace monitor_data_init and monitor_data_destroy.
A standalone helper is provided to enable the I/O thread for QMP where appropriate for the chardev backend.
Message-ID: <[email protected]>
--- monitor/monitor-internal.h | 3 +-- monitor/hmp.c | 6 +++-- monitor/monitor.c | 46 +++++++++++++------------------------- monitor/qmp-cmds.c | 3 --- monitor/qmp.c | 34 +++++++++++++++------------- 5 files changed, 40 insertions(+), 52 deletions(-)
monitor/monitor-internal.h +1 -2
@@ -193,8 +193,7 @@ extern QemuMutex monitor_lock;extern MonitorList mon_list;void monitor_complete (Monitor *mon,Error **errp); -void monitor_data_init (Monitor *mon,bool is_qmp,bool use_io_thread); -void monitor_data_destroy (Monitor *mon); +void monitor_iothread_init (Monitor *mon);int monitor_can_read (void *opaque);void monitor_list_append (Monitor *mon);void monitor_fdsets_cleanup (void );
monitor/hmp.c +4 -2
@@ -48,6 +48,10 @@ OBJECT_DEFINE_TYPE(MonitorHMP, monitor_hmp, MONITOR_HMP, MONITOR);static void monitor_hmp_finalize (Object *obj) { +MonitorHMP *mon =MONITOR_HMP (obj); +if (mon->rs) { +readline_free (mon->rs); + } }static bool monitor_hmp_get_readline (Object *obj,Error **errp)
@@ -1590,8 +1594,6 @@ void monitor_new_hmp(const char *id, const char *chardev_id,return ; } -monitor_data_init (&mon->parent_obj, false, false); -if (mon->use_readline) { mon->rs = readline_init(monitor_readline_printf, monitor_readline_flush,
monitor/monitor.c +16 -30
88 lines not on this page · raw message
monitor/qmp-cmds.c +0 -3
@@ -168,8 +168,6 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,char *output =NULL ;MonitorHMP *hmp =MONITOR_HMP (object_new (TYPE_MONITOR_HMP )); -monitor_data_init (&hmp->parent_obj, false, false); -if (has_cpu_index) {int ret =monitor_set_cpu (&hmp->parent_obj, cpu_index);if (ret <0 ) {
@@ -186,7 +184,6 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index, } out: -monitor_data_destroy (&hmp->parent_obj);object_unref (hmp);return output; }
monitor/qmp.c +19 -15
@@ -73,8 +73,16 @@ QmpCommandList qmp_commands, qmp_cap_negotiation_commands;OBJECT_DEFINE_TYPE (MonitorQMP, monitor_qmp,MONITOR_QMP ,MONITOR ); +static void monitor_qmp_cleanup_req_queue_locked (MonitorQMP *mon); +static void monitor_qmp_finalize (Object *obj) { +MonitorQMP *mon =MONITOR_QMP (obj); + +json_message_parser_destroy (&mon->parser); +qemu_mutex_destroy (&mon->qmp_queue_lock); +monitor_qmp_cleanup_req_queue_locked (mon); +g_queue_free (mon->qmp_requests); }static bool monitor_qmp_get_pretty (Object *obj,Error **errp)
@@ -98,8 +106,15 @@ static void monitor_qmp_class_init(ObjectClass *cls, const void *data) monitor_qmp_set_pretty); } +static void handle_qmp_command (void *opaque,QObject *req,Error *err);static void monitor_qmp_init (Object *obj) { +MonitorQMP *mon =MONITOR_QMP (obj); + +qemu_mutex_init (&mon->qmp_queue_lock); + mon->qmp_requests =g_queue_new (); + +json_message_parser_init (&mon->parser, handle_qmp_command, mon,NULL ); }static bool qmp_oob_enabled (MonitorQMP *mon)
@@ -522,14 +537,6 @@ static void monitor_qmp_event(void *opaque, QEMUChrEvent event) } } -void monitor_data_destroy_qmp (MonitorQMP *mon) -{ -json_message_parser_destroy (&mon->parser); -qemu_mutex_destroy (&mon->qmp_queue_lock); -monitor_qmp_cleanup_req_queue_locked (mon); -g_queue_free (mon->qmp_requests); -} -static void monitor_qmp_setup_handlers_bh (void *opaque) {MonitorQMP *mon = opaque;
@@ -572,14 +579,11 @@ void monitor_new_qmp(const char *id, const char *chardev_id,qemu_chr_fe_set_echo (&mon->parent_obj.chr, true);/* Note: we run QMP monitor in I/O thread when @chr supports that */ -monitor_data_init (&mon->parent_obj, true, -qemu_chr_has_feature (mon->parent_obj.chr.chr, -QEMU_CHAR_FEATURE_GCONTEXT )); +if (qemu_chr_has_feature (mon->parent_obj.chr.chr, +QEMU_CHAR_FEATURE_GCONTEXT )) { +monitor_iothread_init (&mon->parent_obj); + } -qemu_mutex_init (&mon->qmp_queue_lock); - mon->qmp_requests =g_queue_new (); - -json_message_parser_init (&mon->parser, handle_qmp_command, mon,NULL );if (mon->parent_obj.use_io_thread) {/* * Make sure the old iowatch is gone. It's possible when
Signature · 1 line
2.54.0
[PULL 15/35] monitor: use class method for I/O thread request
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
Introducing a virtual "requires_iothread" method allows the code to automatically initialize the I/O thread during object completion.
Message-ID: <[email protected]>
--- monitor/monitor-internal.h | 14 +++++++++----- monitor/monitor.c | 22 ++++++++++++---------- monitor/qmp.c | 20 +++++++++++--------- 3 files changed, 32 insertions(+), 24 deletions(-)
monitor/monitor-internal.h +9 -5
@@ -121,6 +121,12 @@ struct MonitorClass {* the monitor to accept further client input */ void (*accept_input)(Monitor *mon); + +/* +* If non-NULL and returns true, then an I/O thread +* is required for processing the monitor +*/ +bool (*requires_iothread)(const Monitor *mon); };struct Monitor {
@@ -129,7 +135,6 @@ struct Monitor {CharFrontend chr;int suspend_cnt;/* Needs to be accessed atomically */ bool is_qmp; -bool use_io_thread;char *mon_cpu_path;QTAILQ_ENTRY (Monitor) entry;
@@ -159,9 +164,8 @@ struct MonitorHMP {bool use_readline;/* * State used only in the thread "owning" the monitor. -* If @use_io_thread, this is @mon_iothread. (This does not actually happen -* in the current state of the code.) -* Else, it's the main thread. +* This is currently always the main thread, since +* HMP does not allow use of the I/O thread at this time. * These members can be safely accessed without locks. */ ReadLineState *rs;
@@ -210,7 +214,7 @@ extern QemuMutex monitor_lock;extern MonitorList mon_list;void monitor_complete (Monitor *mon,Error **errp); -void monitor_iothread_init (Monitor *mon); +bool monitor_requires_iothread (const Monitor *mon);int monitor_can_read (void *opaque);void monitor_list_append (Monitor *mon);void monitor_fdsets_cleanup (void );
monitor/monitor.c +12 -10
@@ -162,6 +162,12 @@ bool monitor_cur_is_qmp(void)return cur_mon &&monitor_is_qmp (cur_mon); } +bool monitor_requires_iothread (const Monitor *mon) +{ +MonitorClass *cls =MONITOR_GET_CLASS (mon); +return cls->requires_iothread && cls->requires_iothread (mon); +} +/** * Is @mon is using readline? * Note: not all HMP monitors use readline, e.g., gdbserver has a
@@ -563,7 +569,7 @@ int monitor_suspend(Monitor *mon)qatomic_inc (&mon->suspend_cnt); -if (mon->use_io_thread) { +if (monitor_requires_iothread (mon)) {/* * Kick I/O thread to make sure this takes effect. It'll be * evaluated again in prepare() of the watch object.
@@ -596,7 +602,7 @@ void monitor_resume(Monitor *mon)if (qatomic_dec_fetch (&mon->suspend_cnt) ==0 ) {AioContext *ctx; -if (mon->use_io_thread) { +if (monitor_requires_iothread (mon)) { ctx =iothread_get_aio_context (mon_iothread); }else { ctx =qemu_get_aio_context ();
@@ -634,14 +640,6 @@ void monitor_list_append(Monitor *mon) } } -void monitor_iothread_init (Monitor *mon) -{ -if (!mon_iothread) { - mon_iothread =iothread_create ("mon_iothread" , &error_abort); - } - mon->use_io_thread = true; -} -void monitor_cleanup (void ) {/*
@@ -743,6 +741,10 @@ void monitor_complete(Monitor *mon, Error **errp)return ; } } + +if (monitor_requires_iothread (mon) && !mon_iothread) { + mon_iothread =iothread_create ("mon_iothread" , &error_abort); + } }int monitor_new (MonitorOptions *opts,bool allow_hmp,Error **errp)
monitor/qmp.c +11 -9
@@ -100,6 +100,7 @@ static void monitor_qmp_set_pretty(Object *obj, bool val, Error **errp) }static void monitor_qmp_emit_event (Monitor *mon,QAPIEvent event,QDict *qdict); +static bool monitor_qmp_requires_iothread (const Monitor *mon);static void monitor_qmp_class_init (ObjectClass *cls,const void *data) {
@@ -110,6 +111,7 @@ static void monitor_qmp_class_init(ObjectClass *cls, const void *data) monitor_qmp_set_pretty); moncls->emit_event = monitor_qmp_emit_event; + moncls->requires_iothread = monitor_qmp_requires_iothread; }static void handle_qmp_command (void *opaque,QObject *req,Error *err);
@@ -136,6 +138,11 @@ static void monitor_qmp_emit_event(Monitor *mon, QAPIEvent event, QDict *qdict)qmp_send_response (qmp, qdict); } +static bool monitor_qmp_requires_iothread (const Monitor *mon) +{ +return qemu_chr_has_feature (mon->chr.chr, +QEMU_CHAR_FEATURE_GCONTEXT ); +}static bool qmp_oob_enabled (MonitorQMP *mon) {
@@ -146,7 +153,8 @@ static void monitor_qmp_caps_reset(MonitorQMP *mon) {memset (mon->capab_offered,0 ,sizeof (mon->capab_offered));memset (mon->capab,0 ,sizeof (mon->capab)); - mon->capab_offered[QMP_CAPABILITY_OOB ] = mon->parent_obj.use_io_thread; + mon->capab_offered[QMP_CAPABILITY_OOB ] = +monitor_requires_iothread (MONITOR (mon)); }static void qmp_request_free (QMPRequest *req)
@@ -562,7 +570,7 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)MonitorQMP *mon = opaque;GMainContext *context; -assert (mon->parent_obj.use_io_thread); +assert (monitor_requires_iothread (MONITOR (mon))); context =iothread_get_g_main_context (mon_iothread);assert (context); qemu_chr_fe_set_handlers(&mon->parent_obj.chr, monitor_can_read,
@@ -598,13 +606,7 @@ void monitor_new_qmp(const char *id, const char *chardev_id,qemu_chr_fe_set_echo (&mon->parent_obj.chr, true); -/* Note: we run QMP monitor in I/O thread when @chr supports that */ -if (qemu_chr_has_feature (mon->parent_obj.chr.chr, -QEMU_CHAR_FEATURE_GCONTEXT )) { -monitor_iothread_init (&mon->parent_obj); - } - -if (mon->parent_obj.use_io_thread) { +if (monitor_requires_iothread (MONITOR (mon))) {/* * Make sure the old iowatch is gone. It's possible when * e.g. the chardev is in client mode, with wait=on.
Signature · 1 line
2.54.0
[PULL 25/35] monitor: reject attempts to delete the current monitor
In reply to Markus ArmbrusterFrom: Christian Brauner <[email protected]>
If an 'object_del' command for a QMP monitor arrives targetting the current monitor, reject this request. If the current monitor is deleted, it will be impossible to send any reply and the client won't be able to remove the corresponding chardev backend.
Note, it is not possible to rely on checking monitor_cur() because if 'object_del' is called via human-monitor-command, monitor_cur() will reflect the temporary HMP, not the QMP target that needs to be checked.
[DB: extracted monitor tracking from larger commit; added logic
to monitor_qmp_prepare_delete to reject request]
Message-ID: <[email protected]>
--- monitor/qmp.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
monitor/qmp.c +17 -0
@@ -72,6 +72,9 @@ typedef struct QMPRequest QMPRequest;QmpCommandList qmp_commands, qmp_cap_negotiation_commands; +/* Monitor being serviced by the dispatcher. Protected by BQL. */ +static MonitorQMP *qmp_dispatcher_current_mon; +OBJECT_DEFINE_TYPE (MonitorQMP, monitor_qmp,MONITOR_QMP ,MONITOR );static void monitor_qmp_cleanup_req_queue_locked (MonitorQMP *mon);
@@ -369,6 +372,7 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)*/ mon = req_obj->mon; + qmp_dispatcher_current_mon = mon;/* * We need to resume the monitor if handle_qmp_command()
@@ -429,6 +433,7 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data) }qmp_request_free (req_obj); + qmp_dispatcher_current_mon =NULL ; }qatomic_set (&qmp_dispatcher_co,NULL ); }
@@ -573,6 +578,11 @@ static void monitor_qmp_event(void *opaque, QEMUChrEvent event) } } +static bool monitor_qmp_dispatcher_is_servicing (MonitorQMP *mon) +{ +return qmp_dispatcher_current_mon == mon; +} +static void monitor_qmp_setup_handlers_bh (void *opaque) {MonitorQMP *mon = opaque;
@@ -645,6 +655,13 @@ static void monitor_qmp_complete(UserCreatable *uc, Error **errp)static bool monitor_qmp_prepare_delete (UserCreatable *uc,Error **errp) { +MonitorQMP *qmp =MONITOR_QMP (uc); + +if (monitor_qmp_dispatcher_is_servicing (qmp)) { +error_setg (errp,"Cannot delete the current QMP monitor" ); +return false; + } +error_setg (errp,"Deleting QMP monitors is not supported" );return false; }
Signature · 1 line
2.54.0
[PULL 07/35] monitor: add 'chardev' property to Monitor base class
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
This is associates both QMP and HMP monitors with a character device backend.
Message-ID: <[email protected]>
--- monitor/monitor-internal.h | 2 ++ monitor/hmp.c | 24 ++++++++++-------------- monitor/monitor.c | 36 ++++++++++++++++++++++++++++++++++++ monitor/qmp.c | 33 +++++++++++++++------------------ 4 files changed, 63 insertions(+), 32 deletions(-)
monitor/monitor-internal.h +2 -0
@@ -108,6 +108,7 @@ struct MonitorClass {struct Monitor {Object parent; +char *chardev_id;CharFrontend chr;int suspend_cnt;/* Needs to be accessed atomically */ bool is_qmp;
@@ -192,6 +193,7 @@ extern QmpCommandList qmp_commands, qmp_cap_negotiation_commands;extern QemuMutex monitor_lock;extern MonitorList mon_list; +void monitor_complete (Monitor *mon,Error **errp);void monitor_data_init (Monitor *mon,bool is_qmp,bool skip_flush,bool use_io_thread);void monitor_data_destroy (Monitor *mon);
monitor/hmp.c +10 -14
@@ -1542,28 +1542,24 @@ static void monitor_readline_flush(void *opaque)void monitor_new_hmp (const char *id,const char *chardev_id,bool use_readline,Error **errp) { +ERRP_GUARD ();MonitorHMP *mon; g_autofree char *autoid = id ? NULL : monitor_compat_id(); -Chardev *chr; -Object *obj; +Object *obj =object_new_with_props (TYPE_MONITOR_HMP , +object_get_objects_root (), + id ? id : autoid, + errp, +"chardev" , chardev_id, +NULL ); - chr =qemu_chr_find (chardev_id); -if (chr ==NULL ) { -error_setg (errp,"chardev \"%s\" not found" , chardev_id); -return ; - } - - obj =object_new_with_props (TYPE_MONITOR_HMP , -object_get_objects_root (), - id ? id : autoid, - errp, -NULL );if (!obj) {return ; } + mon =MONITOR_HMP (obj); -if (!qemu_chr_fe_init (&mon->parent_obj.chr, chr, errp)) { +monitor_complete (MONITOR (mon), errp); +if (*errp) {object_unparent (OBJECT (mon));return ; }
monitor/monitor.c +36 -0
@@ -79,10 +79,31 @@ OBJECT_DEFINE_ABSTRACT_TYPE(Monitor, monitor, MONITOR, OBJECT);static void monitor_finalize (Object *obj) { +Monitor *mon =MONITOR (obj); + +g_free (mon->chardev_id); +} + +static char *monitor_get_chardev_id (Object *obj,Error **errp) +{ +Monitor *mon =MONITOR (obj); + +return g_strdup (mon->chardev_id); +} + +static void monitor_set_chardev_id (Object *obj,const char *str,Error **errp) +{ +Monitor *mon =MONITOR (obj); + +g_free (mon->chardev_id); + mon->chardev_id =g_strdup (str); }static void monitor_class_init (ObjectClass *cls,const void *data) { +object_class_property_add_str (cls,"chardev" , + monitor_get_chardev_id, + monitor_set_chardev_id); }static void monitor_init (Object *obj)
@@ -738,6 +759,21 @@ char *monitor_compat_id(void)return g_strdup_printf ("compat_monitor%d" , monitor_device_index++); } +void monitor_complete (Monitor *mon,Error **errp) +{ +if (mon->chardev_id) { +Chardev *chr =qemu_chr_find (mon->chardev_id); +if (chr ==NULL ) { +error_setg (errp,"chardev \"%s\" not found" , mon->chardev_id); +return ; + } + +if (!qemu_chr_fe_init (&mon->chr, chr, errp)) { +return ; + } + } +} +int monitor_new (MonitorOptions *opts,bool allow_hmp,Error **errp) {ERRP_GUARD ();
monitor/qmp.c +15 -18
@@ -530,36 +530,33 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)void monitor_new_qmp (const char *id,const char *chardev_id,bool pretty,Error **errp) { +ERRP_GUARD ();MonitorQMP *mon; g_autofree char *autoid = id ? NULL : monitor_compat_id(); -Chardev *chr; -Object *obj; +Object *obj =object_new_with_props (TYPE_MONITOR_QMP , +object_get_objects_root (), + id ? id : autoid, + errp, +"chardev" , chardev_id, +NULL ); - chr =qemu_chr_find (chardev_id); -if (chr ==NULL ) { -error_setg (errp,"chardev \"%s\" not found" , chardev_id); -return ; - } - - obj =object_new_with_props (TYPE_MONITOR_QMP , -object_get_objects_root (), - id ? id : autoid, - errp, -NULL );if (!obj) {return ; } - mon =MONITOR_QMP (obj); -if (!qemu_chr_fe_init (&mon->parent_obj.chr, chr, errp)) { + mon =MONITOR_QMP (obj); +monitor_complete (MONITOR (mon), errp); +if (*errp) {object_unparent (OBJECT (mon));return ; } +qemu_chr_fe_set_echo (&mon->parent_obj.chr, true);/* Note: we run QMP monitor in I/O thread when @chr supports that */ monitor_data_init (&mon->parent_obj, true, false, -qemu_chr_has_feature (chr,QEMU_CHAR_FEATURE_GCONTEXT )); +qemu_chr_has_feature (mon->parent_obj.chr.chr, +QEMU_CHAR_FEATURE_GCONTEXT )); mon->pretty = pretty;
@@ -572,12 +569,12 @@ void monitor_new_qmp(const char *id, const char *chardev_id,* Make sure the old iowatch is gone. It's possible when * e.g. the chardev is in client mode, with wait=on. */ -remove_fd_in_watch (chr); +remove_fd_in_watch (mon->parent_obj.chr.chr);/* * Clean up listener IO sources early to prevent racy fd * handling between the main thread and the I/O thread. */ -remove_listener_fd_in_watch (chr); +remove_listener_fd_in_watch (mon->parent_obj.chr.chr);/* * We can't call qemu_chr_fe_set_handlers() directly here * since chardev might be running in the monitor I/O
Signature · 1 line
2.54.0
[PULL 34/35] qemu-options: document new monitor-hmp and monitor-qmp objects
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
Add new docs for the -object monitor-hmp and -object monitor-qmp options, updating -mon to state that it is legacy syntax sugar for the new -object args.
Message-ID: <[email protected]>
--- qemu-options.hx | 61 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 58 insertions(+), 3 deletions(-)
qemu-options.hx +58 -3
@@ -4955,6 +4955,11 @@ SRST port). The default device is ``vc`` in graphical mode and ``stdio`` in non graphical mode. Use ``-monitor none`` to disable the default monitor. + + The use of ``-monitor dev`` is syntactic sugar for creating a character + device from ``dev`` and pairing it with ``-object monitor-hmp``. + Both the character device and monitor object will be given an ID + ``compat_monitorNNN`` where ``NNN`` is a counter starting from 0. ERST DEF("qmp", HAS_ARG, QEMU_OPTION_qmp, \ "-qmp dev like -monitor but opens in 'control' mode\n",
@@ -4966,9 +4971,10 @@ SRST -qmp tcp:localhost:4444,server=on,wait=off - Not all options are configurable via this syntax; for maximum - flexibility use the ``-mon`` option and an accompanying ``-chardev``. - + The use of ``-qmp dev`` is syntactic sugar for creating a character + device from ``dev`` and pairing it with ``-object monitor-qmp``. + Both the character device and monitor object will be given an ID + ``compat_monitorNNN`` where ``NNN`` is a counter starting from 0. ERST DEF("qmp-pretty", HAS_ARG, QEMU_OPTION_qmp_pretty, \ "-qmp-pretty dev like -qmp but uses pretty JSON formatting\n",
@@ -4997,6 +5003,16 @@ SRST -mon chardev=mon1,mode=control,pretty=on enables the QMP monitor on localhost port 4444 with pretty-printing. + + The use of ``-mon mode=readline`` is syntactic sugar + for the new ``-object monitor-hmp`` option, each use of which + creates an object with the ID ``compat_monitorNNN`` where ``NNN`` is + a counter starting from 0. + + The use of ``-mon mode=control`` is syntactic sugar + for the new ``-object monitor-qmp`` option, each use of which + creates an object with the ID ``compat_monitorNNN`` where ``NNN`` is + a counter starting from 0. ERST DEF("debugcon", HAS_ARG, QEMU_OPTION_debugcon, \
@@ -5747,6 +5763,45 @@ SRST they are specified. Note that the 'id' property must be set. These objects are placed in the '/objects' path. + ``-object monitor-hmp,id=id,chardev=chardev_id,readline=on|off`` + Set up a monitor running the Human Monitor Protocol, + connected to the chardev ``chardev_id``. + + The ``id`` parameter is a unique ID that can be used + to dynamically delete the monitor at runtime. Note + that monitors created using the historical syntax + will be allocated IDs following the pattern ``compat_monmitorNNN``. + Mixing ``-object`` with ``-monitor`` and ``-mon`` syntax is + discouraged. + + The ``readline`` parameter, which defaults to ``on``, + controls whether the monitor provides line editing. + + ``-object monitor-qmp,id=id,chardev=chardev_id,pretty=on|off,close-action=none|delete`` + Set up a monitor running the QEMU Monitor Protocol, + connected to the chardev ``chardev_id``. + + The ``id`` parameter is a unique ID that can be used + to dynamically delete the monitor at runtime. Note + that monitors created using the historical syntax + will be allocated IDs following the pattern ``compat_monitorNNN``. + Mixing ``-object`` with ``-qmp``, ``-qmp-pretty`` and + ``-mon`` syntax is discouraged. + + The ``pretty`` parameter, which defaults to ``off``, + controls whether the monitor responses are pretty + printed as multi-line indented JSON, as opposed to + constrained to a single line without extraneous + whitespace. + + The ``close-action`` parameter, which defaults to ``none``, + controls what happens when the connection to the monitor + is terminated by the user. If set to ``delete``, then the + ``monitor-qmp`` object and its associated character + device are both immediately deleted. This can be useful + if an extra monitor was hotplugged for a specific task + and should be unplugged when completed. + ``-object memory-backend-file,id=id,size=size,mem-path=dir,share=on|off,discard-data=on|off,merge=on|off,dump=on|off,prealloc=on|off,host-nodes=host-nodes,policy=default|preferred|bind|interleave,align=align,offset=offset,readonly=on|off,rom=on|off|auto`` Creates a memory file backend object, which can be used to back the guest RAM with huge pages.
Signature · 1 line
2.54.0
[PULL 24/35] monitor: convert from oneshot BH to persistent BH
In reply to Markus ArmbrusterFrom: Christian Brauner <[email protected]>
Convert monitor_accept_input from a oneshot BH (aio_bh_schedule_oneshot) to a persistent BH (aio_bh_new + qemu_bh_schedule). Oneshot BHs cannot be cancelled, so monitor_resume() racing with destruction would schedule a callback against memory that monitor_qmp_destroy() is about to free. A persistent BH can be deleted during destruction, cancelling any pending schedule.
Message-ID: <[email protected]>
--- monitor/monitor-internal.h | 2 +- monitor/monitor.c | 25 ++++++++++++++----------- 2 files changed, 15 insertions(+), 12 deletions(-)
monitor/monitor-internal.h +1 -1
@@ -134,7 +134,7 @@ struct Monitor {char *chardev_id;CharFrontend chr;int suspend_cnt;/* Needs to be accessed atomically */ - +QEMUBH *accept_input_bh;/* persistent BH for monitor_accept_input */ char *mon_cpu_path;QTAILQ_ENTRY (Monitor) entry;
monitor/monitor.c +14 -11
@@ -83,6 +83,9 @@ static void monitor_finalize(Object *obj) {Monitor *mon =MONITOR (obj); +if (mon->accept_input_bh) { +qemu_bh_delete (mon->accept_input_bh); + }g_free (mon->chardev_id);g_free (mon->mon_cpu_path);qemu_chr_fe_deinit (&mon->chr, false);
@@ -569,15 +572,7 @@ static void monitor_accept_input(void *opaque)void monitor_resume (Monitor *mon) {if (qatomic_dec_fetch (&mon->suspend_cnt) ==0 ) { -AioContext *ctx; - -if (monitor_requires_iothread (mon)) { - ctx =iothread_get_aio_context (mon_iothread); - }else { - ctx =qemu_get_aio_context (); - } - -aio_bh_schedule_oneshot (ctx, monitor_accept_input, mon); +qemu_bh_schedule (mon->accept_input_bh); }trace_monitor_suspend (mon,-1 );
@@ -700,6 +695,7 @@ char *monitor_compat_id(void)static void monitor_complete (UserCreatable *uc,Error **errp) {Monitor *mon =MONITOR (uc); +AioContext *ctx;if (mon->chardev_id) {Chardev *chr =qemu_chr_find (mon->chardev_id);
@@ -713,9 +709,16 @@ static void monitor_complete(UserCreatable *uc, Error **errp) } } -if (monitor_requires_iothread (mon) && !mon_iothread) { - mon_iothread =iothread_create ("mon_iothread" , &error_abort); +if (monitor_requires_iothread (mon)) { +if (!mon_iothread) { + mon_iothread =iothread_create ("mon_iothread" , &error_abort); + } + + ctx =iothread_get_aio_context (mon_iothread); + }else { + ctx =qemu_get_aio_context (); } + mon->accept_input_bh =aio_bh_new (ctx, monitor_accept_input, mon); }int monitor_new (MonitorOptions *opts,bool allow_hmp,Error **errp)
Signature · 1 line
2.54.0
[PULL 28/35] tests/qtest: add tests for dynamic monitor add/remove
In reply to Markus ArmbrusterFrom: Christian Brauner <[email protected]>
Test the object-add/object-del QMP commands with the monitor-qmp object type.
- Basic lifecycle: chardev-add -> object-add -> object-del -> chardev-remove
- Error: object-add with nonexistent chardev
- Error: second monitor on same chardev (chardev already in use)
- Removal of CLI-created QMP monitor succeeds
- Error: object-remove on HMP monitor
- Re-add after remove: same id and chardev reusable after removal
[DB: modified to use object-add/object-del, removing redundant
scenarios already handled by object-add/del code]
Message-ID: <[email protected]>
--- tests/qtest/qmp-test.c | 174 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+)
tests/qtest/qmp-test.c +174 -0
@@ -337,6 +337,174 @@ static void test_qmp_missing_any_arg(void)qtest_quit (qts); } +static void test_qmp_monitor_add_remove (void ) +{ +QTestState *qts; +QDict *resp; + + qts =qtest_init (common_args); + +/* Create a null chardev for the dynamic monitor */ + resp =qtest_qmp (qts, +"{'execute': 'chardev-add'," +" 'arguments': {'id': 'monitor-chardev'," +" 'backend': {'type': 'null'," +" 'data': {}}}}" ); +g_assert (qdict_haskey (resp,"return" )); +qobject_unref (resp); + +/* Add a dynamic monitor */ + resp =qtest_qmp (qts, +"{'execute': 'object-add'," +" 'arguments': {'qom-type': 'monitor-qmp'," +" 'id': 'dyn-mon'," +" 'chardev': 'monitor-chardev'}}" ); +g_assert (qdict_haskey (resp,"return" )); +qobject_unref (resp); + +/* Remove the dynamic monitor */ + resp =qtest_qmp (qts, +"{'execute': 'object-del'," +" 'arguments': {'id': 'dyn-mon'}}" ); +g_assert (qdict_haskey (resp,"return" )); +qobject_unref (resp); + +/* Add again after remove -- same id and chardev should work */ + resp =qtest_qmp (qts, +"{'execute': 'object-add'," +" 'arguments': {'qom-type': 'monitor-qmp'," +" 'id': 'dyn-mon'," +" 'chardev': 'monitor-chardev'}}" ); +g_assert (qdict_haskey (resp,"return" )); +qobject_unref (resp); + +/* Clean up */ + resp =qtest_qmp (qts, +"{'execute': 'object-del'," +" 'arguments': {'id': 'dyn-mon'}}" ); +g_assert (qdict_haskey (resp,"return" )); +qobject_unref (resp); + + resp =qtest_qmp (qts, +"{'execute': 'chardev-remove'," +" 'arguments': {'id': 'monitor-chardev'}}" ); +g_assert (qdict_haskey (resp,"return" )); +qobject_unref (resp); + +qtest_quit (qts); +} + +static void test_qmp_monitor_error_paths (void ) +{ +QTestState *qts; +QDict *resp; + + qts =qtest_init (common_args); + +/* Error: chardev does not exist */ + resp =qtest_qmp (qts, +"{'execute': 'object-add'," +" 'arguments': {'qom-type': 'monitor-qmp'," +" 'id': 'bad-mon'," +" 'chardev': 'nonexistent'}}" ); +qmp_expect_error_and_unref (resp,"GenericError" ); + +qtest_quit (qts); +} + +static void test_qmp_monitor_chardev_in_use (void ) +{ +QTestState *qts; +QDict *resp; + + qts =qtest_init (common_args); + +/* Create a null chardev */ + resp =qtest_qmp (qts, +"{'execute': 'chardev-add'," +" 'arguments': {'id': 'shared-chr'," +" 'backend': {'type': 'null'," +" 'data': {}}}}" ); +g_assert (qdict_haskey (resp,"return" )); +qobject_unref (resp); + +/* Attach first monitor */ + resp =qtest_qmp (qts, +"{'execute': 'object-add'," +" 'arguments': {'qom-type': 'monitor-qmp'," +" 'id': 'mon-1'," +" 'chardev': 'shared-chr'}}" ); +g_assert (qdict_haskey (resp,"return" )); +qobject_unref (resp); + +/* Error: second monitor on the same chardev */ + resp =qtest_qmp (qts, +"{'execute': 'object-add'," +" 'arguments': {'qom-type': 'monitor-qmp'," +" 'id': 'mon-2'," +" 'chardev': 'shared-chr'}}" ); +qmp_expect_error_and_unref (resp,"GenericError" ); + +/* Clean up */ + resp =qtest_qmp (qts, +"{'execute': 'object-del'," +" 'arguments': {'id': 'mon-1'}}" ); +g_assert (qdict_haskey (resp,"return" )); +qobject_unref (resp); + + resp =qtest_qmp (qts, +"{'execute': 'chardev-remove'," +" 'arguments': {'id': 'shared-chr'}}" ); +g_assert (qdict_haskey (resp,"return" )); +qobject_unref (resp); + +qtest_quit (qts); +} + +static void test_qmp_monitor_remove_cli (void ) +{ +QTestState *qts; +QDict *resp; + +/* Launch with a named CLI monitor on a null chardev */ + qts =qtest_initf ("%s -chardev null,id=cli-chr" +" -object monitor-qmp,id=cli-mon,chardev=cli-chr" , + common_args); + +/* CLI-created QMP monitors can be removed */ + resp =qtest_qmp (qts, +"{'execute': 'object-del'," +" 'arguments': {'id': 'cli-mon'}}" ); +g_assert (qdict_haskey (resp,"return" )); +qobject_unref (resp); + + resp =qtest_qmp (qts, +"{'execute': 'chardev-remove'," +" 'arguments': {'id': 'cli-chr'}}" ); +g_assert (qdict_haskey (resp,"return" )); +qobject_unref (resp); + +qtest_quit (qts); +} + +static void test_qmp_monitor_remove_hmp (void ) +{ +QTestState *qts; +QDict *resp; + + qts =qtest_initf ("%s -chardev null,id=hmp-chr" +" -object monitor-hmp,id=hmp-mon,chardev=hmp-chr" , + common_args); + +/* Error: object_del must reject HMP monitors */ + resp =qtest_qmp (qts, +"{'execute': 'object-del'," +" 'arguments': {'id': 'hmp-mon'}}" ); +qmp_expect_error_and_unref (resp,"GenericError" ); + +qtest_quit (qts); +} +int main (int argc,char *argv[]) {g_test_init (&argc, &argv,NULL );
@@ -348,6 +516,12 @@ int main(int argc, char *argv[])#endif qtest_add_func ("qmp/preconfig" , test_qmp_preconfig);qtest_add_func ("qmp/missing-any-arg" , test_qmp_missing_any_arg); +qtest_add_func ("qmp/monitor-add-remove" , test_qmp_monitor_add_remove); +qtest_add_func ("qmp/monitor-error-paths" , test_qmp_monitor_error_paths); +qtest_add_func ("qmp/monitor-chardev-in-use" , + test_qmp_monitor_chardev_in_use); +qtest_add_func ("qmp/monitor-remove-cli" , test_qmp_monitor_remove_cli); +qtest_add_func ("qmp/monitor-remove-hmp" , test_qmp_monitor_remove_hmp);return g_test_run (); }
Signature · 1 line
2.54.0
[PULL 01/35] qom: replace 'can_be_deleted' with 'prepare_delete'
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
While most objects can perform all their cleanup in the finalizer method, there can be interactions with other resources / subsystems / threads which require that some cleanup be performed on an user creatable object before unparenting it and entering finalization.
The current 'can_be_deleted' method runs in the deletion path and is intended to be used to block deletion. While it could be used to perform cleanup tasks, its name suggests it should be free of side-effects.
Generalize this by renaming it to 'prepare_delete', explicitly allowing for cleanup to be provided. Existing users of 'can_be_deleted' are re-written, which provides them with more detailed/tailored error messages.
Message-ID: <[email protected]>
--- include/qom/object_interfaces.h | 26 +++++++++++++++++--------- include/system/event-loop-base.h | 2 +- backends/cryptodev.c | 12 +++++++++--- backends/hostmem.c | 7 +++++-- backends/iommufd.c | 11 ++++++++--- block/throttle-groups.c | 12 +++++++++--- event-loop-base.c | 8 ++++---- net/can/can_core.c | 6 ++++-- qom/object_interfaces.c | 13 +++++-------- util/main-loop.c | 6 ++++-- tests/qemu-iotests/245 | 4 ++-- 11 files changed, 68 insertions(+), 39 deletions(-)
include/qom/object_interfaces.h +17 -9
@@ -20,8 +20,10 @@ typedef struct UserCreatable UserCreatable;* UserCreatableClass: * @parent_class: the base class * @complete: callback to be called after @obj's properties are set. -* @can_be_deleted: callback to be called before an object is removed -* to check if @obj can be removed safely. +* @prepare_delete: to be called before an attempt to delete @obj +* to validate whether the object can be deleted and trigger any +* cleanup of any resources which have to be dealt with before the +* object is unparented and enters finalization. * * Interface is designed to work with -object/object-add/object_add * commands.
@@ -36,7 +38,9 @@ typedef struct UserCreatable UserCreatable;* For objects created without using -object/object-add/object_add, * @user_creatable_complete() wrapper should be called manually if * object's type implements USER_CREATABLE interface and needs -* complete() callback to be called. +* complete() callback to be called. Similarly @user_creatable_prepare_delete() +* should be called manually prior to an attempt to the delete the +* object. */ struct UserCreatableClass {/* <private> */
@@ -44,7 +48,7 @@ struct UserCreatableClass {/* <public> */ void (*complete)(UserCreatable *uc,Error **errp); -bool (*can_be_deleted)(UserCreatable *uc); +bool (*prepare_delete)(UserCreatable *uc,Error **errp); };/**
@@ -61,13 +65,17 @@ struct UserCreatableClass {bool user_creatable_complete (UserCreatable *uc,Error **errp);/** -* user_creatable_can_be_deleted: -* @uc: the object whose can_be_deleted() method is called if implemented +* user_creatable_prepare_delete: +* @uc: the user-creatable object whose prepare_delete() method is called +* @errp: if an error occurs, a pointer to an area to store the error * -* Wrapper to call can_be_deleted() method if one of types it's inherited -* from implements USER_CREATABLE interface. +* Wrapper to call prepare_delete() class method if defined, otherwise +* does nothing. +* +* Returns: %true on success or if prepare_delete() is not defined, +* %false on failure. */ -bool user_creatable_can_be_deleted (UserCreatable *uc); +bool user_creatable_prepare_delete (UserCreatable *uc,Error **errp);/** * user_creatable_add_qapi:
include/system/event-loop-base.h +1 -1
@@ -24,7 +24,7 @@ struct EventLoopBaseClass {void (*init)(EventLoopBase *base,Error **errp);void (*update_params)(EventLoopBase *base,Error **errp); -bool (*can_be_deleted)(EventLoopBase *base); +bool (*prepare_delete)(EventLoopBase *base,Error **errp); };struct EventLoopBase {
backends/cryptodev.c +9 -3
@@ -454,9 +454,15 @@ bool cryptodev_backend_is_ready(CryptoDevBackend *backend) }static bool -cryptodev_backend_can_be_deleted (UserCreatable *uc) +cryptodev_backend_prepare_delete (UserCreatable *uc,Error **errp) { -return !cryptodev_backend_is_used (CRYPTODEV_BACKEND (uc)); +if (cryptodev_backend_is_used (CRYPTODEV_BACKEND (uc))) { +error_setg (errp, +"Cannot delete cryptodev backend '%s' that is in use" , +object_get_canonical_path_component (OBJECT (uc))); +return false; + } +return true; }static void cryptodev_backend_instance_init (Object *obj)
@@ -613,7 +619,7 @@ cryptodev_backend_class_init(ObjectClass *oc, const void *data)UserCreatableClass *ucc =USER_CREATABLE_CLASS (oc); ucc->complete = cryptodev_backend_complete; - ucc->can_be_deleted = cryptodev_backend_can_be_deleted; + ucc->prepare_delete = cryptodev_backend_prepare_delete;QTAILQ_INIT (&crypto_clients); object_class_property_add(oc, "queues", "uint32",
backends/hostmem.c +5 -2
@@ -434,9 +434,12 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp) }static bool -host_memory_backend_can_be_deleted (UserCreatable *uc) +host_memory_backend_prepare_delete (UserCreatable *uc,Error **errp) {if (host_memory_backend_is_mapped (MEMORY_BACKEND (uc))) { +error_setg (errp, +"Cannot delete host memory backend '%s' which is mapped" , +object_get_canonical_path_component (OBJECT (uc)));return false; }else {return true;
@@ -508,7 +511,7 @@ host_memory_backend_class_init(ObjectClass *oc, const void *data)UserCreatableClass *ucc =USER_CREATABLE_CLASS (oc); ucc->complete = host_memory_backend_memory_complete; - ucc->can_be_deleted = host_memory_backend_can_be_deleted; + ucc->prepare_delete = host_memory_backend_prepare_delete; object_class_property_add_bool(oc, "merge", host_memory_backend_get_merge,
backends/iommufd.c +8 -3
@@ -63,11 +63,16 @@ static void iommufd_backend_set_fd(Object *obj, const char *str, Error **errp)trace_iommu_backend_set_fd (be->fd); } -static bool iommufd_backend_can_be_deleted (UserCreatable *uc) +static bool iommufd_backend_prepare_delete (UserCreatable *uc,Error **errp) {IOMMUFDBackend *be =IOMMUFD_BACKEND (uc); -return !be->users; +if (be->users) { +error_setg (errp,"Can not delete IOMMUFD backend '%s' with %d users" , +object_get_canonical_path_component (OBJECT (uc)), be->users); +return false; + } +return true; }static void iommufd_backend_complete (UserCreatable *uc,Error **errp)
@@ -92,7 +97,7 @@ static void iommufd_backend_class_init(ObjectClass *oc, const void *data) {UserCreatableClass *ucc =USER_CREATABLE_CLASS (oc); - ucc->can_be_deleted = iommufd_backend_can_be_deleted; + ucc->prepare_delete = iommufd_backend_prepare_delete; ucc->complete = iommufd_backend_complete;object_class_property_add_str (oc,"fd" ,NULL , iommufd_backend_set_fd);
block/throttle-groups.c +9 -3
@@ -960,9 +960,15 @@ static void throttle_group_get_limits(Object *obj, Visitor *v,visit_type_ThrottleLimits (v, name, &argp, errp); } -static bool throttle_group_can_be_deleted (UserCreatable *uc) +static bool throttle_group_prepare_delete (UserCreatable *uc,Error **errp) { -return OBJECT (uc)->ref ==1 ; +if (OBJECT (uc)->ref >1 ) { +error_setg (errp, +"Cannot delete throttle group '%s' with active references" , +object_get_canonical_path_component (OBJECT (uc))); +return false; + } +return true; } static void throttle_group_obj_class_init(ObjectClass *klass,
@@ -972,7 +978,7 @@ static void throttle_group_obj_class_init(ObjectClass *klass,UserCreatableClass *ucc =USER_CREATABLE_CLASS (klass); ucc->complete = throttle_group_obj_complete; - ucc->can_be_deleted = throttle_group_can_be_deleted; + ucc->prepare_delete = throttle_group_prepare_delete;/* individual properties */ for (i =0 ; i <sizeof (properties) /sizeof (ThrottleParamInfo); i++) {
event-loop-base.c +4 -4
@@ -85,13 +85,13 @@ static void event_loop_base_complete(UserCreatable *uc, Error **errp) } } -static bool event_loop_base_can_be_deleted (UserCreatable *uc) +static bool event_loop_base_prepare_delete (UserCreatable *uc,Error **errp) {EventLoopBaseClass *bc =EVENT_LOOP_BASE_GET_CLASS (uc);EventLoopBase *backend =EVENT_LOOP_BASE (uc); -if (bc->can_be_deleted) { -return bc->can_be_deleted (backend); +if (bc->prepare_delete) { +return bc->prepare_delete (backend, errp); }return true;
@@ -102,7 +102,7 @@ static void event_loop_base_class_init(ObjectClass *klass, {UserCreatableClass *ucc =USER_CREATABLE_CLASS (klass); ucc->complete = event_loop_base_complete; - ucc->can_be_deleted = event_loop_base_can_be_deleted; + ucc->prepare_delete = event_loop_base_prepare_delete; object_class_property_add(klass, "aio-max-batch", "int", event_loop_base_get_param,
net/can/can_core.c +4 -2
@@ -143,8 +143,10 @@ int can_bus_client_set_filters(CanBusClientState *client, } -static bool can_bus_can_be_deleted (UserCreatable *uc) +static bool can_bus_prepare_delete (UserCreatable *uc,Error **errp) { +error_setg (errp,"Deleting CAN bus device '%s' is not supported" , +object_get_canonical_path_component (OBJECT (uc)));return false; }
@@ -153,7 +155,7 @@ static void can_bus_class_init(ObjectClass *klass, {UserCreatableClass *uc_klass =USER_CREATABLE_CLASS (klass); - uc_klass->can_be_deleted = can_bus_can_be_deleted; + uc_klass->prepare_delete = can_bus_prepare_delete; }static const TypeInfo can_bus_info = {
qom/object_interfaces.c +5 -8
@@ -32,16 +32,14 @@ bool user_creatable_complete(UserCreatable *uc, Error **errp)return !*errp; } -bool user_creatable_can_be_deleted (UserCreatable *uc) +bool user_creatable_prepare_delete (UserCreatable *uc,Error **errp) { -UserCreatableClass *ucc =USER_CREATABLE_GET_CLASS (uc); -if (ucc->can_be_deleted) { -return ucc->can_be_deleted (uc); - }else { -return true; +if (ucc->prepare_delete) { +return ucc->prepare_delete (uc, errp); } +return true; }void user_creatable_add_qapi (ObjectOptions *options,Error **errp)
@@ -253,8 +251,7 @@ bool user_creatable_del(const char *id, Error **errp)return false; } -if (!user_creatable_can_be_deleted (USER_CREATABLE (obj))) { -error_setg (errp,"object '%s' is in use, can not be deleted" , id); +if (!user_creatable_prepare_delete (USER_CREATABLE (obj), errp)) {return false; }
util/main-loop.c +4 -2
@@ -218,8 +218,10 @@ static void main_loop_init(EventLoopBase *base, Error **errp) mloop = m; } -static bool main_loop_can_be_deleted (EventLoopBase *base) +static bool main_loop_prepare_delete (EventLoopBase *base,Error **errp) { +error_setg (errp,"Deleting main loop '%s' is not supported" , +object_get_canonical_path_component (OBJECT (base)));return false; }
@@ -229,7 +231,7 @@ static void main_loop_class_init(ObjectClass *oc, const void *class_data) bc->init = main_loop_init; bc->update_params = main_loop_update_params; - bc->can_be_deleted = main_loop_can_be_deleted; + bc->prepare_delete = main_loop_prepare_delete; }static const TypeInfo main_loop_info = {
tests/qemu-iotests/245 +2 -2
@@ -801,7 +801,7 @@ class TestBlockdevReopen(iotests.QMPTestCase): # Now group1 is in use, it cannot be deleted result = self.vm.qmp('object-del', id = 'group1') self.assert_qmp(result, 'error/class', 'GenericError') - self.assert_qmp(result, 'error/desc', "object 'group1' is in use, can not be deleted") + self.assert_qmp(result, 'error/desc', "Cannot delete throttle group 'group1' with active references") # Default options, this switches the group back to group0 self.reopen(opts)
@@ -809,7 +809,7 @@ class TestBlockdevReopen(iotests.QMPTestCase): # So now we cannot delete group0 result = self.vm.qmp('object-del', id = 'group0') self.assert_qmp(result, 'error/class', 'GenericError') - self.assert_qmp(result, 'error/desc', "object 'group0' is in use, can not be deleted") + self.assert_qmp(result, 'error/desc', "Cannot delete throttle group 'group0' with active references") # But group1 is free this time, and it can be deleted self.vm.cmd('object-del', id = 'group1')
Signature · 1 line
2.54.0
[PULL 21/35] monitor: drop unused monitor_is_qmp method
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
The previous patch dropped the only remaining use of monitor_is_qmp.
Message-ID: <[email protected]>
--- monitor/monitor-internal.h | 9 --------- monitor/monitor.c | 1 - 2 files changed, 10 deletions(-)
monitor/monitor-internal.h +0 -9
@@ -134,7 +134,6 @@ struct Monitor {char *chardev_id;CharFrontend chr;int suspend_cnt;/* Needs to be accessed atomically */ -bool is_qmp;char *mon_cpu_path;QTAILQ_ENTRY (Monitor) entry;
@@ -197,14 +196,6 @@ struct MonitorQMP {GQueue *qmp_requests; }; -/** -* Is @mon a QMP monitor? -*/ -static inline bool monitor_is_qmp (const Monitor *mon) -{ -return mon->is_qmp; -} - typedef QTAILQ_HEAD(MonitorList, Monitor) MonitorList;extern IOThread *mon_iothread;extern Coroutine *qmp_dispatcher_co;
monitor/monitor.c +0 -1
@@ -115,7 +115,6 @@ static void monitor_init(Object *obj)Monitor *mon =MONITOR (obj);qemu_mutex_init (&mon->mon_lock); - mon->is_qmp = !!object_dynamic_cast (obj,TYPE_MONITOR_QMP ); mon->outbuf =g_string_new (NULL ); }
Signature · 1 line
2.54.0
[PULL 05/35] monitor: minimal conversion of monitors to QOM
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
This introduces abstract QOM type "monitor", with concrete subtypes "monitor-hmp" and "monitor-qmp". This is the bare minimum conversion of just the type declarations and replacing g_new/g_free with object_new/object_unref.
Command line option "-monitor" now creates a "monitor-hmp" object "/objects/compat_monitorNNN" in addition to the character device "/chardevs/compat_monitorNNN". NNN counts up from zero.
Exception: "-monitor chardev:ID" creates a "monitor-hmp" object "/objects/ID", and does not create a character device.
"-qmp" and "-qmp-pretty" work the same, except they create a "monitor-qmp" object.
"-mon" now creates either a "monitor-hmp" or "monitor-qmp" object "/objects/ID" if the option argument provides an ID, else "/objects/compat_monitorNNN".
"-gdbstub" and "-serial mon:..." now create a "monitor-hmp" object "/objects/compat_monitorNNN".
Note that the object's name in "/objects/" matches the QemuOpts ID when it exists. The only cases where it doesn't exist are "-mon" without ID, "-gdbstub" and "-serial mon:".
A future patch will make "monitor-hmp" and "monitor-qmp" work with "-object" and "object-add".
Note: there is a slight change in the NNN values assigned. The old code would increment the counter for every monitor added (except for -mon, -serial mon:..., -gdbstub), regardless of whether it needed a "compat_monitorNNN" ID assignment. Now it is only incremented when an automatic ID assigned is needed (but even for -mon, -serial mon:..., -gdbstub).
Message-ID: <[email protected]>
--- include/monitor/monitor.h | 18 +++++++++++++++--- monitor/monitor-internal.h | 18 ++++++++++++++++-- chardev/char.c | 2 +- gdbstub/system.c | 2 +- monitor/hmp.c | 31 ++++++++++++++++++++++++++++--- monitor/monitor.c | 31 +++++++++++++++++++++++++++---- monitor/qmp-cmds.c | 15 ++++++++------- monitor/qmp.c | 31 ++++++++++++++++++++++++++++--- stubs/monitor-internal.c | 3 ++- system/vl.c | 7 +++---- 10 files changed, 129 insertions(+), 29 deletions(-)
include/monitor/monitor.h +15 -3
@@ -5,8 +5,17 @@#include "qapi/qapi-types-misc.h" #include "qemu/readline.h" #include "exec/hwaddr.h" +#include "qom/object.h" + +#define TYPE_MONITOR "monitor" +OBJECT_DECLARE_TYPE (Monitor, MonitorClass,MONITOR ); + +#define TYPE_MONITOR_HMP "monitor-hmp" +OBJECT_DECLARE_TYPE (MonitorHMP, MonitorHMPClass,MONITOR_HMP ); + +#define TYPE_MONITOR_QMP "monitor-qmp" +OBJECT_DECLARE_TYPE (MonitorQMP, MonitorQMPClass,MONITOR_QMP ); -typedef struct MonitorHMP MonitorHMP ;typedef struct MonitorOptions MonitorOptions ;#define QMP_REQ_QUEUE_LEN_MAX 8
@@ -19,8 +28,11 @@ bool monitor_cur_is_qmp(void);void monitor_init_globals (void );void monitor_init_globals_core (void ); -void monitor_new_qmp (Chardev *chr,bool pretty,Error **errp); -void monitor_new_hmp (Chardev *chr,bool use_readline,Error **errp); +char *monitor_compat_id (void ); +void monitor_new_qmp (const char *id,Chardev *chr, +bool pretty,Error **errp); +void monitor_new_hmp (const char *id,Chardev *chr, +bool use_readline,Error **errp);int monitor_new (MonitorOptions *opts,bool allow_hmp,Error **errp);int monitor_new_opts (QemuOpts *opts,Error **errp);void monitor_cleanup (void );
monitor/monitor-internal.h +16 -2
@@ -101,7 +101,13 @@ typedef struct HMPCommand {bool coroutine; } HMPCommand; + +struct MonitorClass { +ObjectClass parent_class; +}; +struct Monitor { +Object parent;CharFrontend chr;int suspend_cnt;/* Needs to be accessed atomically */ bool is_qmp;
@@ -127,6 +133,10 @@ struct Monitor {int reset_seen; }; +struct MonitorHMPClass { +MonitorClass parent_class; +}; +struct MonitorHMP {Monitor parent_obj;bool use_readline;
@@ -140,7 +150,11 @@ struct MonitorHMP {ReadLineState *rs; }; -typedef struct { +struct MonitorQMPClass { +MonitorClass parent_class; +}; + +struct MonitorQMP {Monitor parent_obj;JSONMessageParser parser;bool pretty;
@@ -160,7 +174,7 @@ typedef struct {QemuMutex qmp_queue_lock;/* Input queue that holds all the parsed QMP requests */ GQueue *qmp_requests; -} MonitorQMP; +};/** * Is @mon a QMP monitor?
chardev/char.c +1 -1
@@ -805,7 +805,7 @@ static Chardev *qemu_chr_new_from_name(const char *label, const char *filename,if (qemu_opt_get_bool (opts,"mux" ,0 )) {assert (permit_mux_mon); -monitor_new_hmp (chr, true, &err); +monitor_new_hmp (NULL , chr, true, &err);if (err) {error_report_err (err);object_unparent (OBJECT (chr));
gdbstub/system.c +1 -1
@@ -390,7 +390,7 @@ bool gdbserver_start(const char *device, Error **errp)/* Initialize a monitor terminal for gdb */ mon_chr =qemu_chardev_new (NULL ,TYPE_CHARDEV_GDB ,NULL ,NULL , &error_abort); -monitor_new_hmp (mon_chr, false, &error_abort); +monitor_new_hmp (NULL , mon_chr, false, &error_abort); }else {qemu_chr_fe_deinit (&gdbserver_system_state.chr, true); mon_chr = gdbserver_system_state.mon_chr;
monitor/hmp.c +28 -3
@@ -43,6 +43,20 @@#include "system/block-backend.h" #include "trace.h" +OBJECT_DEFINE_TYPE (MonitorHMP, monitor_hmp,MONITOR_HMP ,MONITOR ); + +static void monitor_hmp_finalize (Object *obj) +{ +} + +static void monitor_hmp_class_init (ObjectClass *cls,const void *data) +{ +} + +static void monitor_hmp_init (Object *obj) +{ +} +static void monitor_command_cb (void *opaque,const char *cmdline,void *readline_opaque) {
@@ -1524,12 +1538,23 @@ static void monitor_readline_flush(void *opaque)monitor_flush (&mon->parent_obj); } -void monitor_new_hmp (Chardev *chr,bool use_readline,Error **errp) +void monitor_new_hmp (const char *id,Chardev *chr, +bool use_readline,Error **errp) { -MonitorHMP *mon =g_new0 (MonitorHMP,1 ); +MonitorHMP *mon; + g_autofree char *autoid = id ? NULL : monitor_compat_id(); +Object *obj =object_new_with_props (TYPE_MONITOR_HMP , +object_get_objects_root (), + id ? id : autoid, + errp, +NULL ); +if (!obj) { +return ; + } + mon =MONITOR_HMP (obj);if (!qemu_chr_fe_init (&mon->parent_obj.chr, chr, errp)) { -g_free (mon); +object_unparent (OBJECT (mon));return ; }
monitor/monitor.c +27 -4
@@ -73,6 +73,22 @@ static GHashTable *coroutine_mon; /* Maps Coroutine* to Monitor* */MonitorList mon_list;static bool monitor_destroyed; +int monitor_device_index; + +OBJECT_DEFINE_ABSTRACT_TYPE (Monitor, monitor,MONITOR ,OBJECT ); + +static void monitor_finalize (Object *obj) +{ +} + +static void monitor_class_init (ObjectClass *cls,const void *data) +{ +} + +static void monitor_init (Object *obj) +{ +} +Monitor *monitor_cur (void ) {Monitor *mon;
@@ -598,7 +614,7 @@ void monitor_list_append(Monitor *mon)if (mon) {monitor_data_destroy (mon); -g_free (mon); +object_unparent (OBJECT (mon)); } }
@@ -680,7 +696,7 @@ void monitor_cleanup(void)monitor_flush (mon);monitor_data_destroy (mon);qemu_mutex_lock (&monitor_lock); -g_free (mon); +object_unparent (OBJECT (mon)); }qemu_mutex_unlock (&monitor_lock);
@@ -715,6 +731,13 @@ void monitor_init_globals(void)aio_co_schedule (iohandler_get_aio_context (), qmp_dispatcher_co); } +char *monitor_compat_id (void ) +{ +static int monitor_device_index; + +return g_strdup_printf ("compat_monitor%d" , monitor_device_index++); +} +int monitor_new (MonitorOptions *opts,bool allow_hmp,Error **errp) {ERRP_GUARD ();
@@ -732,7 +755,7 @@ int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp)switch (opts->mode) {case MONITOR_MODE_CONTROL : -monitor_new_qmp (chr, opts->pretty, errp); +monitor_new_qmp (opts->id, chr, opts->pretty, errp);break ;case MONITOR_MODE_READLINE :if (!allow_hmp) {
@@ -743,7 +766,7 @@ int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp)error_setg (errp,"'pretty' is not compatible with HMP monitors" );return -1 ; } -monitor_new_hmp (chr, true, errp); +monitor_new_hmp (opts->id, chr, true, errp);break ;default :g_assert_not_reached ();
monitor/qmp-cmds.c +8 -7
@@ -166,12 +166,12 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index, int64_t cpu_index, Error **errp) { char *output = NULL; -MonitorHMP hmp = {}; +MonitorHMP *hmp =MONITOR_HMP (object_new (TYPE_MONITOR_HMP )); -monitor_data_init (&hmp.parent_obj, false, true, false); +monitor_data_init (&hmp->parent_obj, false, true, false);if (has_cpu_index) { -int ret =monitor_set_cpu (&hmp.parent_obj, cpu_index); +int ret =monitor_set_cpu (&hmp->parent_obj, cpu_index);if (ret <0 ) {error_setg (errp,QERR_INVALID_PARAMETER_VALUE ,"cpu-index" ,"a CPU number" );
@@ -179,14 +179,15 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index, } } -handle_hmp_command (&hmp, command_line); +handle_hmp_command (hmp, command_line); - WITH_QEMU_LOCK_GUARD(&hmp.parent_obj.mon_lock) { - output =g_strdup (hmp.parent_obj.outbuf->str); + WITH_QEMU_LOCK_GUARD(&hmp->parent_obj.mon_lock) { + output =g_strdup (hmp->parent_obj.outbuf->str); } out: -monitor_data_destroy (&hmp.parent_obj); +monitor_data_destroy (&hmp->parent_obj); +object_unref (hmp);return output; }
monitor/qmp.c +28 -3
@@ -71,6 +71,20 @@ typedef struct QMPRequest QMPRequest;QmpCommandList qmp_commands, qmp_cap_negotiation_commands; +OBJECT_DEFINE_TYPE (MonitorQMP, monitor_qmp,MONITOR_QMP ,MONITOR ); + +static void monitor_qmp_finalize (Object *obj) +{ +} + +static void monitor_qmp_class_init (ObjectClass *cls,const void *data) +{ +} + +static void monitor_qmp_init (Object *obj) +{ +} +static bool qmp_oob_enabled (MonitorQMP *mon) {return mon->capab[QMP_CAPABILITY_OOB ];
@@ -513,12 +527,23 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)monitor_list_append (&mon->parent_obj); } -void monitor_new_qmp (Chardev *chr,bool pretty,Error **errp) +void monitor_new_qmp (const char *id,Chardev *chr, +bool pretty,Error **errp) { -MonitorQMP *mon =g_new0 (MonitorQMP,1 ); +MonitorQMP *mon; + g_autofree char *autoid = id ? NULL : monitor_compat_id(); +Object *obj =object_new_with_props (TYPE_MONITOR_QMP , +object_get_objects_root (), + id ? id : autoid, + errp, +NULL ); +if (!obj) { +return ; + } + mon =MONITOR_QMP (obj);if (!qemu_chr_fe_init (&mon->parent_obj.chr, chr, errp)) { -g_free (mon); +object_unparent (OBJECT (mon));return ; }qemu_chr_fe_set_echo (&mon->parent_obj.chr, true);
stubs/monitor-internal.c +2 -1
@@ -8,6 +8,7 @@ int monitor_get_fd(Monitor *mon, const char *name, Error **errp)return -1 ; } -void monitor_new_hmp (Chardev *chr,bool use_readline,Error **errp) +void monitor_new_hmp (const char *id,Chardev *chr, +bool use_readline,Error **errp) { }
system/vl.c +3 -4
@@ -1252,7 +1252,6 @@ static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp)static void monitor_parse (const char *str,const char *mode,bool pretty) { -static int monitor_device_index =0 ;QemuOpts *opts;const char *p;char label[32 ];
@@ -1260,8 +1259,9 @@ static void monitor_parse(const char *str, const char *mode, bool pretty)if (strstart (str,"chardev:" , &p)) {snprintf (label,sizeof (label),"%s" , p); }else { -snprintf (label,sizeof (label),"compat_monitor%d" , - monitor_device_index); + g_autofree char *id = monitor_compat_id(); +assert (strlen (id) <sizeof (label)); +memcpy (label, id,strlen (id) +1 ); opts =qemu_chr_parse_compat (label, str, true);if (!opts) {error_report ("parse error: %s" , str);
@@ -1277,7 +1277,6 @@ static void monitor_parse(const char *str, const char *mode, bool pretty) }else {assert (pretty == false); } - monitor_device_index++; } struct device_config {
Signature · 1 line
2.54.0
[PULL 12/35] monitor: use class methods for monitor_vprintf
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
This removes the need for using monitor_is_qmp() to check the subclass type, which is an anti-pattern.
Message-ID: <[email protected]>
--- monitor/monitor-internal.h | 7 +++++++ monitor/hmp.c | 13 +++++++++++++ monitor/monitor.c | 11 ++++------- 3 files changed, 24 insertions(+), 7 deletions(-)
monitor/monitor-internal.h +7 -0
@@ -104,6 +104,13 @@ typedef struct HMPCommand {struct MonitorClass {ObjectClass parent_class; + +/* +* If non-NULL, the monitor is able to print messages +* for attention of the client user +*/ + int (*vprintf)(Monitor *mon, const char *fmt, va_list ap) + G_GNUC_PRINTF(2, 0); };struct Monitor {
monitor/hmp.c +13 -0
@@ -68,11 +68,18 @@ static void monitor_hmp_set_readline(Object *obj, bool val, Error **errp) mon->use_readline = val; } +int monitor_hmp_vprintf(Monitor *mon, const char *fmt, va_list ap) + G_GNUC_PRINTF(2, 0); +static void monitor_hmp_class_init (ObjectClass *cls,const void *data) { +MonitorClass *moncls =MONITOR_CLASS (cls); +object_class_property_add_bool (cls,"readline" , monitor_hmp_get_readline, monitor_hmp_set_readline); + + moncls->vprintf = monitor_hmp_vprintf; }static void monitor_hmp_init (Object *obj)
@@ -87,6 +94,12 @@ static void monitor_hmp_init(Object *obj) hmp->use_readline = true; } +int monitor_hmp_vprintf (Monitor *mon,const char *fmt,va_list ap) +{ + g_autofree char *buf = g_strdup_vprintf(fmt, ap); +return monitor_puts (mon, buf); +} +static void monitor_command_cb (void *opaque,const char *cmdline,void *readline_opaque) {
monitor/monitor.c +4 -7
@@ -264,21 +264,18 @@ int monitor_puts(Monitor *mon, const char *str)int monitor_vprintf (Monitor *mon,const char *fmt,va_list ap) { -char *buf; -int n; +MonitorClass *moncls;if (!mon) {return -1 ; } -if (monitor_is_qmp (mon)) { + moncls =MONITOR_GET_CLASS (mon); +if (!moncls->vprintf) {return -1 ; } - buf =g_strdup_vprintf (fmt, ap); - n =monitor_puts (mon, buf); -g_free (buf); -return n; +return moncls->vprintf (mon, fmt, ap); }int monitor_printf (Monitor *mon,const char *fmt, ...)
Signature · 1 line
2.54.0
[PULL 09/35] monitor: add 'pretty' property to QMP Monitor class
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
This determines whether the QMP JSON responses are pretty printed with newlines and indentation, or compact with no extra whitespace.
Message-ID: <[email protected]>
--- monitor/qmp.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
monitor/qmp.c +18 -2
@@ -77,8 +77,25 @@ static void monitor_qmp_finalize(Object *obj) { } +static bool monitor_qmp_get_pretty (Object *obj,Error **errp) +{ +MonitorQMP *mon =MONITOR_QMP (obj); + +return mon->pretty; +} + +static void monitor_qmp_set_pretty (Object *obj,bool val,Error **errp) +{ +MonitorQMP *mon =MONITOR_QMP (obj); + + mon->pretty = val; +} +static void monitor_qmp_class_init (ObjectClass *cls,const void *data) { +object_class_property_add_bool (cls,"pretty" , + monitor_qmp_get_pretty, + monitor_qmp_set_pretty); }static void monitor_qmp_init (Object *obj)
@@ -538,6 +555,7 @@ void monitor_new_qmp(const char *id, const char *chardev_id, id ? id : autoid, errp,"chardev" , chardev_id, +"pretty" , pretty ?"yes" :"no" , NULL);if (!obj) {
@@ -558,8 +576,6 @@ void monitor_new_qmp(const char *id, const char *chardev_id,qemu_chr_has_feature (mon->parent_obj.chr.chr, QEMU_CHAR_FEATURE_GCONTEXT)); - mon->pretty = pretty; -qemu_mutex_init (&mon->qmp_queue_lock); mon->qmp_requests =g_queue_new ();
Signature · 1 line
2.54.0
[PULL 23/35] monitor: implement "user creatable" interface for adding monitors
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
Implement the user creatable QOM interface and define the monitor-qmp and monitor-hmp types in QAPI. This unlocks the ability to create them on the command line with -object or in HMP/QMP with object_add.
For example:
$QEMU -chardev stdio,id=monchr0 -object monitor-hmp,id=mon0,chrdev=monchr0
Initially the "prepare_delete" callback is hardcoded to return an error which means -object and object_add can be used, but object_del will fail. Support for deleting monitors will be introduced in subsequent commits.
Message-ID: <[email protected]>
--- qapi/qom.json | 43 +++++++++++++++++++++++ monitor/monitor-internal.h | 1 - monitor/hmp.c | 72 ++++++++++++++++++++++++-------------- monitor/monitor.c | 14 ++++++-- monitor/qmp.c | 45 ++++++++++++++++-------- stubs/monitor-internal.c | 1 + system/vl.c | 8 ++++- 7 files changed, 138 insertions(+), 46 deletions(-)
qapi/qom.json +43 -0
@@ -1187,6 +1187,45 @@ 'data': { '*cpu-affinity': ['uint16'], '*node-affinity': ['uint16'] } } +## +# @MonitorProperties: +# +# Properties for all monitors +# +# @chardev: ID of the character device providing the monitor transport +# +# Since: 11.1 +## +{ 'struct': 'MonitorProperties', + 'data': { 'chardev': 'str' }} + +## +# @MonitorHMPProperties: +# +# Properties for the HMP monitor +# +# @readline: whether to enable readline for line editing +# (default: true) +# +# Since: 11.1 +## +{ 'struct': 'MonitorHMPProperties', + 'base': 'MonitorProperties', + 'data': { '*readline': 'bool' } } + +## +# @MonitorQMPProperties: +# +# Properties for the QMP monitor +# +# @pretty: whether to pretty print JSON responses (default: false) +# +# Since: 11.1 +## +{ 'struct': 'MonitorQMPProperties', + 'base': 'MonitorProperties', + 'data': { '*pretty': 'bool' } } + ## # @ObjectType: #
@@ -1237,6 +1276,8 @@ 'memory-backend-ram', { 'name': 'memory-backend-shm', 'if': 'CONFIG_POSIX' }, + 'monitor-hmp', + 'monitor-qmp', 'pef-guest', { 'name': 'pr-manager-helper', 'if': 'CONFIG_LINUX' },
@@ -1315,6 +1356,8 @@ 'memory-backend-ram': 'MemoryBackendProperties', 'memory-backend-shm': { 'type': 'MemoryBackendShmProperties', 'if': 'CONFIG_POSIX' }, + 'monitor-hmp': 'MonitorHMPProperties', + 'monitor-qmp': 'MonitorQMPProperties', 'pr-manager-helper': { 'type': 'PrManagerHelperProperties', 'if': 'CONFIG_LINUX' }, 'qtest': 'QtestProperties',
monitor/monitor-internal.h +0 -1
@@ -204,7 +204,6 @@ extern QmpCommandList qmp_commands, qmp_cap_negotiation_commands;extern QemuMutex monitor_lock;extern MonitorList mon_list; -void monitor_complete (Monitor *mon,Error **errp);bool monitor_requires_iothread (const Monitor *mon);int monitor_can_read (void *opaque);void monitor_list_append (Monitor *mon);
monitor/hmp.c +45 -27
108 lines not on this page · raw message
monitor/monitor.c +12 -2
@@ -29,6 +29,7 @@#include "qapi/qapi-emit-events.h" #include "qapi/qapi-visit-control.h" #include "qobject/qdict.h" +#include "qom/object_interfaces.h" #include "qemu/error-report.h" #include "qemu/option.h" #include "system/qtest.h"
@@ -75,7 +76,8 @@ static bool monitor_destroyed;int monitor_device_index; -OBJECT_DEFINE_ABSTRACT_TYPE (Monitor, monitor,MONITOR ,OBJECT ); +OBJECT_DEFINE_TYPE_EXTENDED (Monitor, monitor,MONITOR ,OBJECT , true, + { TYPE_USER_CREATABLE }, {});static void monitor_finalize (Object *obj) {
@@ -103,11 +105,17 @@ static void monitor_set_chardev_id(Object *obj, const char *str, Error **errp) mon->chardev_id =g_strdup (str); } +static void monitor_complete (UserCreatable *uc,Error **errp); +static void monitor_class_init (ObjectClass *cls,const void *data) { +UserCreatableClass *ucc =USER_CREATABLE_CLASS (cls); +object_class_property_add_str (cls,"chardev" , monitor_get_chardev_id, monitor_set_chardev_id); + + ucc->complete = monitor_complete; }static void monitor_init (Object *obj)
@@ -689,8 +697,10 @@ char *monitor_compat_id(void)return g_strdup_printf ("compat_monitor%d" , monitor_device_index++); } -void monitor_complete (Monitor *mon,Error **errp) +static void monitor_complete (UserCreatable *uc,Error **errp) { +Monitor *mon =MONITOR (uc); +if (mon->chardev_id) {Chardev *chr =qemu_chr_find (mon->chardev_id);if (chr ==NULL ) {
monitor/qmp.c +30 -15
80 lines not on this page · raw message
stubs/monitor-internal.c +1 -0
@@ -11,4 +11,5 @@ int monitor_get_fd(Monitor *mon, const char *name, Error **errp)void monitor_new_hmp (const char *id,const char *chardev_id,bool use_readline,Error **errp) { +g_assert_not_reached (); }
system/vl.c +7 -1
@@ -1829,6 +1829,10 @@ static void object_option_add_visitor(Visitor *v) {ObjectOption *opt =g_new0 (ObjectOption,1 );visit_type_ObjectOptions (v,NULL , &opt->opts, &error_fatal); +if (opt->opts->qom_type ==OBJECT_TYPE_MONITOR_HMP || + opt->opts->qom_type ==OBJECT_TYPE_MONITOR_QMP ) { + default_monitor =0 ; + }QTAILQ_INSERT_TAIL (&object_opts, opt, next); }
@@ -1970,7 +1974,9 @@ static bool object_create_early(const char *type)/* Reason: property "chardev" */ if (g_str_equal (type,"rng-egd" ) || -g_str_equal (type,"qtest" )) { +g_str_equal (type,"qtest" ) || +g_str_equal (type,"monitor-hmp" ) || +g_str_equal (type,"monitor-qmp" )) {return false; }
Signature · 1 line
2.54.0
[PULL 19/35] monitor: use dynamic cast in QMP commands
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
Rather than asserting monitor_is_qmp(), use a QOM cast via MONITOR_QMP which performs an assert already.
Message-ID: <[email protected]>
--- monitor/qmp-cmds-control.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-)
monitor/qmp-cmds-control.c +2 -10
@@ -72,11 +72,7 @@ static bool qmp_caps_accept(MonitorQMP *mon, QMPCapabilityList *list,void qmp_qmp_capabilities (bool has_enable,QMPCapabilityList *enable,Error **errp) { -Monitor *cur_mon =monitor_cur (); -MonitorQMP *mon; - -assert (monitor_is_qmp (cur_mon)); - mon =container_of (cur_mon, MonitorQMP, parent_obj); +MonitorQMP *mon =MONITOR_QMP (monitor_cur ());if (mon->commands == &qmp_commands) { error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
@@ -122,11 +118,7 @@ static void query_commands_cb(const QmpCommand *cmd, void *opaque)CommandInfoList *qmp_query_commands (Error **errp) {CommandInfoList *list =NULL ; -Monitor *cur_mon =monitor_cur (); -MonitorQMP *mon; - -assert (monitor_is_qmp (cur_mon)); - mon =container_of (cur_mon, MonitorQMP, parent_obj); +MonitorQMP *mon =MONITOR_QMP (monitor_cur ());qmp_for_each_command (mon->commands, query_commands_cb, &list);
Signature · 1 line
2.54.0
[PULL 03/35] monitor: replace 'common' with 'parent_obj' in MonitorQMP
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
The field name 'parent_obj' is standard practice for QOM structs so align the QMP monitor.
Message-ID: <[email protected]>
--- monitor/monitor-internal.h | 2 +- monitor/monitor.c | 4 ++-- monitor/qmp-cmds-control.c | 4 ++-- monitor/qmp.c | 40 +++++++++++++++++++------------------- 4 files changed, 25 insertions(+), 25 deletions(-)
monitor/monitor-internal.h +1 -1
@@ -141,7 +141,7 @@ struct MonitorHMP { }; typedef struct { - Monitor common; + Monitor parent_obj; JSONMessageParser parser; bool pretty; /*
monitor/monitor.c +2 -2
@@ -308,7 +308,7 @@ static void monitor_qapi_event_emit(QAPIEvent event, QDict *qdict)continue ; } - qmp_mon =container_of (mon, MonitorQMP, common); + qmp_mon =container_of (mon, MonitorQMP, parent_obj); {QEMU_LOCK_GUARD (&mon->mon_lock);if (qmp_mon->commands == &qmp_cap_negotiation_commands) {
@@ -625,7 +625,7 @@ void monitor_data_destroy(Monitor *mon)g_free (mon->mon_cpu_path);qemu_chr_fe_deinit (&mon->chr, false);if (monitor_is_qmp (mon)) { -monitor_data_destroy_qmp (container_of (mon, MonitorQMP, common)); +monitor_data_destroy_qmp (container_of (mon, MonitorQMP, parent_obj)); }else {readline_free (container_of (mon, MonitorHMP, parent_obj)->rs); }
monitor/qmp-cmds-control.c +2 -2
@@ -76,7 +76,7 @@ void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable,MonitorQMP *mon;assert (monitor_is_qmp (cur_mon)); - mon =container_of (cur_mon, MonitorQMP, common); + mon =container_of (cur_mon, MonitorQMP, parent_obj);if (mon->commands == &qmp_commands) { error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
@@ -126,7 +126,7 @@ CommandInfoList *qmp_query_commands(Error **errp)MonitorQMP *mon;assert (monitor_is_qmp (cur_mon)); - mon =container_of (cur_mon, MonitorQMP, common); + mon =container_of (cur_mon, MonitorQMP, parent_obj);qmp_for_each_command (mon->commands, query_commands_cb, &list);
monitor/qmp.c +20 -20
@@ -80,7 +80,7 @@ static void monitor_qmp_caps_reset(MonitorQMP *mon) {memset (mon->capab_offered,0 ,sizeof (mon->capab_offered));memset (mon->capab,0 ,sizeof (mon->capab)); - mon->capab_offered[QMP_CAPABILITY_OOB ] = mon->common.use_io_thread; + mon->capab_offered[QMP_CAPABILITY_OOB ] = mon->parent_obj.use_io_thread; }static void qmp_request_free (QMPRequest *req)
@@ -124,7 +124,7 @@ static void monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon)* when we get here while the monitor is suspended. An * unfortunately timed CHR_EVENT_CLOSED can do the trick. */ -monitor_resume (&mon->common); +monitor_resume (&mon->parent_obj); } }
@@ -139,7 +139,7 @@ void qmp_send_response(MonitorQMP *mon, const QDict *rsp)trace_monitor_qmp_respond (mon, json->str);g_string_append_c (json,'\n' ); -monitor_puts (&mon->common, json->str); +monitor_puts (&mon->parent_obj, json->str);g_string_free (json, true); }
@@ -166,7 +166,7 @@ static void monitor_qmp_dispatch(MonitorQMP *mon, QObject *req)QDict *error; rsp =qmp_dispatch (mon->commands, req,qmp_oob_enabled (mon), - &mon->common); + &mon->parent_obj);if (mon->commands == &qmp_cap_negotiation_commands) { error =qdict_get_qdict (rsp,"error" );
@@ -207,7 +207,7 @@ static QMPRequest *monitor_qmp_requests_pop_any_with_lock(void)continue ; } - qmp_mon =container_of (mon, MonitorQMP, common); + qmp_mon =container_of (mon, MonitorQMP, parent_obj);qemu_mutex_lock (&qmp_mon->qmp_queue_lock); req_obj =g_queue_pop_head (qmp_mon->qmp_requests);if (req_obj) {
@@ -302,7 +302,7 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data) oob_enabled =qmp_oob_enabled (mon);if (oob_enabled && mon->qmp_requests->length ==QMP_REQ_QUEUE_LEN_MAX -1 ) { -monitor_resume (&mon->common); +monitor_resume (&mon->parent_obj); }/*
@@ -343,7 +343,7 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data) }if (!oob_enabled) { -monitor_resume (&mon->common); +monitor_resume (&mon->parent_obj); }qmp_request_free (req_obj);
@@ -408,7 +408,7 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err)*/ if (!qmp_oob_enabled (mon) || mon->qmp_requests->length ==QMP_REQ_QUEUE_LEN_MAX -1 ) { -monitor_suspend (&mon->common); +monitor_suspend (&mon->parent_obj); }/*
@@ -462,7 +462,7 @@ static void monitor_qmp_event(void *opaque, QEMUChrEvent event)switch (event) {case CHR_EVENT_OPENED : - WITH_QEMU_LOCK_GUARD(&mon->common.mon_lock) { + WITH_QEMU_LOCK_GUARD(&mon->parent_obj.mon_lock) { mon->commands = &qmp_cap_negotiation_commands;monitor_qmp_caps_reset (mon); }
@@ -504,27 +504,27 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)MonitorQMP *mon = opaque;GMainContext *context; -assert (mon->common.use_io_thread); +assert (mon->parent_obj.use_io_thread); context =iothread_get_g_main_context (mon_iothread);assert (context); -qemu_chr_fe_set_handlers (&mon->common.chr, monitor_can_read, +qemu_chr_fe_set_handlers (&mon->parent_obj.chr, monitor_can_read, monitor_qmp_read, monitor_qmp_event, -NULL , &mon->common, context, true); -monitor_list_append (&mon->common); +NULL , &mon->parent_obj, context, true); +monitor_list_append (&mon->parent_obj); }void monitor_init_qmp (Chardev *chr,bool pretty,Error **errp) {MonitorQMP *mon =g_new0 (MonitorQMP,1 ); -if (!qemu_chr_fe_init (&mon->common.chr, chr, errp)) { +if (!qemu_chr_fe_init (&mon->parent_obj.chr, chr, errp)) {g_free (mon);return ; } -qemu_chr_fe_set_echo (&mon->common.chr, true); +qemu_chr_fe_set_echo (&mon->parent_obj.chr, true);/* Note: we run QMP monitor in I/O thread when @chr supports that */ -monitor_data_init (&mon->common, true, false, +monitor_data_init (&mon->parent_obj, true, false,qemu_chr_has_feature (chr,QEMU_CHAR_FEATURE_GCONTEXT )); mon->pretty = pretty;
@@ -533,7 +533,7 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp) mon->qmp_requests =g_queue_new ();json_message_parser_init (&mon->parser, handle_qmp_command, mon,NULL ); -if (mon->common.use_io_thread) { +if (mon->parent_obj.use_io_thread) {/* * Make sure the old iowatch is gone. It's possible when * e.g. the chardev is in client mode, with wait=on.
@@ -553,9 +553,9 @@ void monitor_init_qmp(Chardev *chr, bool pretty, Error **errp) monitor_qmp_setup_handlers_bh, mon);/* The bottom half will add @mon to @mon_list */ }else { -qemu_chr_fe_set_handlers (&mon->common.chr, monitor_can_read, +qemu_chr_fe_set_handlers (&mon->parent_obj.chr, monitor_can_read, monitor_qmp_read, monitor_qmp_event, -NULL , &mon->common,NULL , true); -monitor_list_append (&mon->common); +NULL , &mon->parent_obj,NULL , true); +monitor_list_append (&mon->parent_obj); } }
Signature · 1 line
2.54.0
[PULL 17/35] util: use dynamic cast in error vreport
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
This eliminates a use of monitor_is_qmp() from the error reporting path.
Message-ID: <[email protected]>
--- util/error-report.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
util/error-report.c +6 -7
@@ -231,16 +231,15 @@ char *real_time_iso8601(void) G_GNUC_PRINTF(2, 0)static void vreport (report_type type,const char *fmt,va_list ap) { -Monitor *cur =monitor_cur (); -gchar *timestr; -/* * When current monitor is QMP, messages must go to stderr -* and have prefixes added +* and have prefixes added, so we cast to HMP, leaving 'cur' +* as NULL in QMP case */ -if (monitor_cur_is_qmp ()) { - cur =NULL ; - } +Monitor *cur =MONITOR ( +object_dynamic_cast (OBJECT (monitor_cur ()),TYPE_MONITOR_HMP )); +gchar *timestr; +if (!cur) {qemu_flockfile (stderr); }
Signature · 1 line
2.54.0
[PULL 20/35] monitor: use dynamic cast in monitor_is_hmp_non_interactive
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
Rather than checking !monitor_is_qmp(), use a dynamic cast to check for HMP.
Message-ID: <[email protected]>
--- monitor/monitor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
monitor/monitor.c +1 -1
@@ -170,7 +170,7 @@ static inline bool monitor_uses_readline(const MonitorHMP *mon)static inline bool monitor_is_hmp_non_interactive (const Monitor *mon) { -if (monitor_is_qmp (mon)) { +if (!object_dynamic_cast (OBJECT (mon),TYPE_MONITOR_HMP )) {return false; }
Signature · 1 line
2.54.0
[PULL 29/35] tests/functional: add e2e test for dynamic QMP monitor hotplug
In reply to Markus ArmbrusterFrom: Christian Brauner <[email protected]>
Add functional tests that exercise dynamic monitor hotplug with real socket connections:
- Hotplug cycle: chardev-add a unix socket, object-add, connect to the socket, receive the QMP greeting, negotiate capabilities, send query-version, disconnect, remove the monitor and chardev, then repeat the entire cycle a second time to verify cleanup and reuse.
- Self-removal: a dynamically-added monitor sends object-del targeting itself, verifying that the request is rejected
- Large response: send query-qmp-schema on a dynamic monitor to exercise the output buffer flush path with a large response payload.
- Events after negotiation: trigger STOP/RESUME events via the main monitor and verify they are delivered on the dynamic monitor.
This complements the qtest unit tests by verifying that a real QMP client can connect to a dynamically-added monitor and exchange messages.
[DB: modified to use object-add/object-del; adjust self-removal test
to validate rejection of request]
Message-ID: <[email protected]>
--- MAINTAINERS | 1 + tests/functional/generic/meson.build | 1 + .../generic/test_monitor_hotplug.py | 168 ++++++++++++++++++ 3 files changed, 170 insertions(+) create mode 100755 tests/functional/generic/test_monitor_hotplug.py
MAINTAINERS +1 -0
@@ -3672,6 +3672,7 @@ F: docs/interop/*qmp-* F: scripts/qmp/ F: tests/qtest/qmp-test.c F: tests/qtest/qmp-cmd-test.c +F: tests/functional/generic/test_monitor_hotplug.py T: git https://repo.or.cz/qemu/armbru.git qapi-next qtest
tests/functional/generic/meson.build +1 -0
@@ -4,6 +4,7 @@ tests_generic_system = [ 'empty_cpu_model', 'info_usernet', 'linters', + 'monitor_hotplug', 'version', 'vnc', ]
tests/functional/generic/test_monitor_hotplug.py new file +168 -0
168 lines not on this page · raw message
Signature · 1 line
2.54.0
[PULL 22/35] monitor: eliminate monitor_is_hmp_non_interactive method
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
The monitor_is_hmp_non_interactive method is used by monitor_suspend and monitor_resume, to make them a no-op if the HMP does not use readline.
There are only a handful of callers of suspend/resume and they can be made to skip the call when readline is not present.
Message-ID: <[email protected]>
--- include/monitor/monitor.h | 2 +- migration/migration-hmp-cmds.c | 5 ++++- monitor/hmp-cmds.c | 5 ++++- monitor/hmp.c | 11 ++++++++--- monitor/monitor.c | 30 +----------------------------- 5 files changed, 18 insertions(+), 35 deletions(-)
include/monitor/monitor.h +1 -1
@@ -37,7 +37,7 @@ int monitor_new(MonitorOptions *opts, bool allow_hmp, Error **errp);int monitor_new_opts (QemuOpts *opts,Error **errp);void monitor_cleanup (void ); -int monitor_suspend (Monitor *mon); +void monitor_suspend (Monitor *mon);void monitor_resume (Monitor *mon);int monitor_get_fd (Monitor *mon,const char *fdname,Error **errp);
migration/migration-hmp-cmds.c +4 -1
@@ -19,6 +19,7 @@#include "monitor/hmp.h" #include "monitor/hmp-completion.h" #include "monitor/monitor.h" +#include "monitor/monitor-internal.h" #include "qapi/error.h" #include "qapi/qapi-commands-migration.h" #include "qapi/qapi-visit-migration.h"
@@ -853,12 +854,14 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)if (!detach) {HMPMigrationStatus *status; +MonitorHMP *hmp =MONITOR_HMP (mon); -if (monitor_suspend (mon) <0 ) { +if (!hmp->use_readline) {monitor_printf (mon,"terminal does not allow synchronous " "migration, continuing detached\n" );return ; } +monitor_suspend (mon); status =g_malloc0 (sizeof (*status)); status->mon = mon;
monitor/hmp-cmds.c +4 -1
@@ -129,7 +129,10 @@ void hmp_info_version(Monitor *mon, const QDict *qdict)void hmp_quit (Monitor *mon,const QDict *qdict) { -monitor_suspend (mon); +MonitorHMP *hmp =MONITOR_HMP (mon); +if (hmp->use_readline) { +monitor_suspend (mon); + }qmp_quit (NULL ); }
monitor/hmp.c +8 -3
@@ -1528,13 +1528,16 @@ static void monitor_read(void *opaque, const uint8_t *buf, int size)static void monitor_event (void *opaque,QEMUChrEvent event) {Monitor *mon = opaque; +MonitorHMP *hmp =MONITOR_HMP (mon);switch (event) {case CHR_EVENT_MUX_IN :qemu_mutex_lock (&mon->mon_lock);if (mon->mux_out) { mon->mux_out =0 ; -monitor_resume (mon); +if (hmp->use_readline) { +monitor_resume (mon); + } }qemu_mutex_unlock (&mon->mon_lock);break ;
@@ -1547,7 +1550,9 @@ static void monitor_event(void *opaque, QEMUChrEvent event) }else {monitor_flush_locked (mon); } -monitor_suspend (mon); +if (hmp->use_readline) { +monitor_suspend (mon); + } mon->mux_out =1 ; }qemu_mutex_unlock (&mon->mon_lock);
@@ -1558,7 +1563,7 @@ static void monitor_event(void *opaque, QEMUChrEvent event) "information\n", QEMU_VERSION);qemu_mutex_lock (&mon->mon_lock); mon->reset_seen =1 ; -if (!mon->mux_out) { +if (!mon->mux_out && hmp->use_readline) {/* Suspend-resume forces the prompt to be printed. */ monitor_suspend (mon);monitor_resume (mon);
monitor/monitor.c +1 -29
@@ -157,25 +157,6 @@ bool monitor_requires_iothread(const Monitor *mon)return cls->requires_iothread && cls->requires_iothread (mon); } -/** -* Is @mon is using readline? -* Note: not all HMP monitors use readline, e.g., gdbserver has a -* non-interactive HMP monitor, so readline is not used there. -*/ -static inline bool monitor_uses_readline (const MonitorHMP *mon) -{ -return mon->use_readline; -} - -static inline bool monitor_is_hmp_non_interactive (const Monitor *mon) -{ -if (!object_dynamic_cast (OBJECT (mon),TYPE_MONITOR_HMP )) { -return false; - } - -return !monitor_uses_readline (container_of (mon, MonitorHMP, parent_obj)); -} -static gboolean monitor_unblocked (void *do_not_use,GIOCondition cond,void *opaque) {
@@ -550,12 +531,8 @@ static gboolean qapi_event_throttle_equal(const void *a, const void *b)return TRUE; } -int monitor_suspend (Monitor *mon) +void monitor_suspend (Monitor *mon) { -if (monitor_is_hmp_non_interactive (mon)) { -return -ENOTTY ; - } -qatomic_inc (&mon->suspend_cnt);if (monitor_requires_iothread (mon)) {
@@ -567,7 +544,6 @@ int monitor_suspend(Monitor *mon) }trace_monitor_suspend (mon,1 ); -return 0 ; }static void monitor_accept_input (void *opaque)
@@ -584,10 +560,6 @@ static void monitor_accept_input(void *opaque)void monitor_resume (Monitor *mon) { -if (monitor_is_hmp_non_interactive (mon)) { -return ; - } -if (qatomic_dec_fetch (&mon->suspend_cnt) ==0 ) {AioContext *ctx;
Signature · 1 line
2.54.0
[PULL 27/35] monitor: implement support for deleting QMP objects
In reply to Markus ArmbrusterFrom: Christian Brauner <[email protected]>
The removal sequence is:
1. Remove from mon_list under monitor_lock. This must happen before disconnecting chardev handlers to prevent event broadcast from calling monitor_flush_locked() after the gcontext reset, which would create an out_watch on the wrong GMainContext (see monitor_cancel_out_watch()).
2. Cancel any pending out_watch while gcontext still points to the correct context.
3. Disconnect chardev handlers, passing context=NULL and close the connection.
4. Drain pending requests from any in-flight monitor_qmp_read().
5. Destroy the monitor object
[DB: extracted from a larger commit and refactored to apply
to the new monitor class structure. Remove 'self delete' feature which requires complex special-case code]
Message-ID: <[email protected]>
--- monitor/monitor-internal.h | 2 ++ monitor/monitor.c | 22 ++++++++++++++++ monitor/qmp.c | 54 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 76 insertions(+), 2 deletions(-)
monitor/monitor-internal.h +2 -0
@@ -178,6 +178,7 @@ struct MonitorQMP {Monitor parent_obj;JSONMessageParser parser;bool pretty; +bool setup_pending;/* iothread BH has not yet set up chardev handlers */ /* * When a client connects, we're in capabilities negotiation mode. * @commands is &qmp_cap_negotiation_commands then. When command
@@ -206,6 +207,7 @@ extern MonitorList mon_list;bool monitor_requires_iothread (const Monitor *mon);int monitor_can_read (void *opaque); +void monitor_cancel_out_watch (Monitor *mon);void monitor_list_append (Monitor *mon);void monitor_fdsets_cleanup (void );int monitor_set_cpu (Monitor *mon,int cpu_index);
monitor/monitor.c +22 -0
@@ -179,6 +179,28 @@ static gboolean monitor_unblocked(void *do_not_use, GIOCondition cond,return G_SOURCE_REMOVE ; } +/* Cancel a pending out_watch GSource. Caller must hold mon_lock. */ +void monitor_cancel_out_watch (Monitor *mon) +{ +if (mon->out_watch) { +GMainContext *ctx =NULL ; +GSource *src; + +if (monitor_requires_iothread (mon)) { + ctx =iothread_get_g_main_context (mon_iothread); + } + src =g_main_context_find_source_by_id (ctx, mon->out_watch); +if (!src && ctx) { +/* Handler disconnect may have reset gcontext to NULL. */ + src =g_main_context_find_source_by_id (NULL , mon->out_watch); + } +if (src) { +g_source_destroy (src); + } + mon->out_watch =0 ; + } +} +/* Caller must hold mon->mon_lock */ void monitor_flush_locked (Monitor *mon) {
monitor/qmp.c +52 -2
86 lines not on this page · raw message
Signature · 1 line
2.54.0
[PULL 04/35] monitor: rename monitor_init* to monitor_new*
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
The current "monitor_init" functions will clash with the methods of the same name that are required by QOM. To ease the transition to QOM, rename them out of the way.
Message-ID: <[email protected]>
--- include/monitor/monitor.h | 8 ++++---- chardev/char.c | 2 +- gdbstub/system.c | 2 +- monitor/hmp.c | 2 +- monitor/monitor.c | 10 +++++----- monitor/qmp.c | 2 +- storage-daemon/qemu-storage-daemon.c | 2 +- stubs/monitor-internal.c | 2 +- system/vl.c | 2 +- 9 files changed, 16 insertions(+), 16 deletions(-)
include/monitor/monitor.h +4 -4
@@ -19,10 +19,10 @@ bool monitor_cur_is_qmp(void);void monitor_init_globals (void );void monitor_init_globals_core (void ); -void monitor_init_qmp (Chardev *chr,bool pretty,Error **errp); -void monitor_init_hmp (Chardev *chr,bool use_readline,Error **errp); -int monitor_init (MonitorOptions *opts,bool allow_hmp,Error **errp); -int monitor_init_opts (QemuOpts *opts,Error **errp); +void monitor_new_qmp (Chardev *chr,bool pretty,Error **errp); +void monitor_new_hmp (Chardev *chr,bool use_readline,Error **errp); +int monitor_new (MonitorOptions *opts,bool allow_hmp,Error **errp); +int monitor_new_opts (QemuOpts *opts,Error **errp);void monitor_cleanup (void );int monitor_suspend (Monitor *mon);
chardev/char.c +1 -1
@@ -805,7 +805,7 @@ static Chardev *qemu_chr_new_from_name(const char *label, const char *filename,if (qemu_opt_get_bool (opts,"mux" ,0 )) {assert (permit_mux_mon); -monitor_init_hmp (chr, true, &err); +monitor_new_hmp (chr, true, &err);if (err) {error_report_err (err);object_unparent (OBJECT (chr));
gdbstub/system.c +1 -1
@@ -390,7 +390,7 @@ bool gdbserver_start(const char *device, Error **errp)/* Initialize a monitor terminal for gdb */ mon_chr =qemu_chardev_new (NULL ,TYPE_CHARDEV_GDB ,NULL ,NULL , &error_abort); -monitor_init_hmp (mon_chr, false, &error_abort); +monitor_new_hmp (mon_chr, false, &error_abort); }else {qemu_chr_fe_deinit (&gdbserver_system_state.chr, true); mon_chr = gdbserver_system_state.mon_chr;
monitor/hmp.c +1 -1
@@ -1524,7 +1524,7 @@ static void monitor_readline_flush(void *opaque)monitor_flush (&mon->parent_obj); } -void monitor_init_hmp (Chardev *chr,bool use_readline,Error **errp) +void monitor_new_hmp (Chardev *chr,bool use_readline,Error **errp) {MonitorHMP *mon =g_new0 (MonitorHMP,1 );
monitor/monitor.c +5 -5
@@ -715,7 +715,7 @@ void monitor_init_globals(void)aio_co_schedule (iohandler_get_aio_context (), qmp_dispatcher_co); } -int monitor_init (MonitorOptions *opts,bool allow_hmp,Error **errp) +int monitor_new (MonitorOptions *opts,bool allow_hmp,Error **errp) {ERRP_GUARD ();Chardev *chr;
@@ -732,7 +732,7 @@ int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp)switch (opts->mode) {case MONITOR_MODE_CONTROL : -monitor_init_qmp (chr, opts->pretty, errp); +monitor_new_qmp (chr, opts->pretty, errp);break ;case MONITOR_MODE_READLINE :if (!allow_hmp) {
@@ -743,7 +743,7 @@ int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp)error_setg (errp,"'pretty' is not compatible with HMP monitors" );return -1 ; } -monitor_init_hmp (chr, true, errp); +monitor_new_hmp (chr, true, errp);break ;default :g_assert_not_reached ();
@@ -752,7 +752,7 @@ int monitor_init(MonitorOptions *opts, bool allow_hmp, Error **errp)return *errp ?-1 :0 ; } -int monitor_init_opts (QemuOpts *opts,Error **errp) +int monitor_new_opts (QemuOpts *opts,Error **errp) {Visitor *v;MonitorOptions *options;
@@ -765,7 +765,7 @@ int monitor_init_opts(QemuOpts *opts, Error **errp)return -1 ; } - ret =monitor_init (options, true, errp); + ret =monitor_new (options, true, errp);qapi_free_MonitorOptions (options);return ret; }
monitor/qmp.c +1 -1
@@ -513,7 +513,7 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)monitor_list_append (&mon->parent_obj); } -void monitor_init_qmp (Chardev *chr,bool pretty,Error **errp) +void monitor_new_qmp (Chardev *chr,bool pretty,Error **errp) {MonitorQMP *mon =g_new0 (MonitorQMP,1 );
storage-daemon/qemu-storage-daemon.c +1 -1
@@ -330,7 +330,7 @@ static void process_options(int argc, char *argv[], bool pre_init_pass)visit_free (v);/* TODO Catch duplicate monitor IDs */ -monitor_init (monitor, false, &error_fatal); +monitor_new (monitor, false, &error_fatal);qapi_free_MonitorOptions (monitor);break ; }
stubs/monitor-internal.c +1 -1
@@ -8,6 +8,6 @@ int monitor_get_fd(Monitor *mon, const char *name, Error **errp)return -1 ; } -void monitor_init_hmp (Chardev *chr,bool use_readline,Error **errp) +void monitor_new_hmp (Chardev *chr,bool use_readline,Error **errp) { }
system/vl.c +1 -1
@@ -1247,7 +1247,7 @@ static int fsdev_init_func(void *opaque, QemuOpts *opts, Error **errp)static int mon_init_func (void *opaque,QemuOpts *opts,Error **errp) { -return monitor_init_opts (opts, errp); +return monitor_new_opts (opts, errp); }static void monitor_parse (const char *str,const char *mode,bool pretty)
Signature · 1 line
2.54.0
[PULL 33/35] tests: switch from -mon to -object monitor-qmp
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
Use the new preferred low level option for configuring the QMP service in libqtest and the python Machine class used by tests. This will avoid triggering deprecation warnings after the subsequent commit.
Message-ID: <[email protected]>
--- tests/qtest/libqtest.c | 2 +- python/qemu/machine/machine.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
tests/qtest/libqtest.c +1 -1
@@ -460,7 +460,7 @@ gchar *qtest_qemu_args(const char *extra_args)"-qtest unix:%s " "-qtest-log %s " "-chardev socket,path=%s,id=char0 " -"-mon chardev=char0,mode=control " +"-object monitor-qmp,id=qmp0,chardev=char0 " "-display none " "-audio none " "%s "
python/qemu/machine/machine.py +2 -2
@@ -304,8 +304,8 @@ def _harness_args(self) -> List[str]: ) else: moncdev = f"socket,id=mon,path={self._monitor_address}" - args.extend(['-chardev', moncdev, '-mon', - 'chardev=mon,mode=control']) + args.extend(['-chardev', moncdev, '-object', + 'monitor-qmp,id=qmp,chardev=mon']) return args def _console_args(self, interactive: bool = False) -> List[str]:
Signature · 1 line
2.54.0
[PULL 35/35] docs: mark '-mon' as deprecated in favour of -object
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
The high level -qmp and -monitor options can remain as convenience wrappers, but the low level -mon is completed obsoleted by the new -object support with 'monitor-qmp' and 'monitor-hmp' types.
Message-ID: <[email protected]>
--- docs/about/deprecated.rst | 10 ++++++++++ docs/devel/writing-monitor-commands.rst | 4 ++-- docs/system/arm/xenpvh.rst | 4 ++-- docs/system/i386/xen.rst | 3 ++- docs/system/i386/xenpvh.rst | 4 ++-- system/vl.c | 2 ++ qemu-options.hx | 15 +++++++-------- 7 files changed, 27 insertions(+), 15 deletions(-)
docs/about/deprecated.rst +10 -0
@@ -61,6 +61,16 @@ The ``debug-threads`` option of the ``-name`` argument is now ignored. Thread naming is unconditionally enabled for all platforms where it is supported. +``-mon`` option (since 11.1) +'''''''''''''''''''''''''''' + +The ``-mon`` option was the generic mechanism for creating monitor +objects if the convenience ``-qmp`` or ``-monitor`` options were not +flexible enough. The monitor objects have been converted to QOM, so +``-mon mode=readline`` is replaced by ``-object monitor-hmp`` and +``-mon mode=control`` is replaced by ``-object monitor-qmp``. The +short convenience options are not deprecated, only ``-mon``. + QEMU Machine Protocol (QMP) commands ------------------------------------
docs/devel/writing-monitor-commands.rst +2 -2
@@ -52,8 +52,8 @@ shown here. First, QEMU should be started like this:: # qemu-system-TARGET [...] \ - -chardev socket,id=qmp,port=4444,host=localhost,server=on \ - -mon chardev=qmp,mode=control,pretty=on + -chardev socket,id=chrqmp0,port=4444,host=localhost,server=on \ + -object monitor-qmp,chardev=chrqmp0,pretty=on,id=qmp0 Then, in a different terminal::
docs/system/arm/xenpvh.rst +2 -2
@@ -27,9 +27,9 @@ Sample QEMU xenpvh commands for running and connecting with Xen: qemu-system-aarch64 -xen-domid 1 \ -chardev socket,id=libxl-cmd,path=qmp-libxl-1,server=on,wait=off \ - -mon chardev=libxl-cmd,mode=control \ + -object monitor-qmp,id=qmp0,chardev=libxl-cmd \ -chardev socket,id=libxenstat-cmd,path=qmp-libxenstat-1,server=on,wait=off \ - -mon chardev=libxenstat-cmd,mode=control \ + -object monitor-qmp,id=qmp1,chardev=libxenstat-cmd \ -xen-attach -name guest0 -vnc none -display none -nographic \ -machine xenpvh -m 1301 \ -chardev socket,id=chrtpm,path=tmp/vtpm2/swtpm-sock \
docs/system/i386/xen.rst +2 -1
@@ -79,7 +79,8 @@ of type ``xen-console`` to connect to it. For the Xen console equivalent of the handy ``-serial mon:stdio`` option, for example: .. parsed-literal:: - -chardev stdio,mux=on,id=char0,signal=off -mon char0 \\ + -chardev stdio,mux=on,id=char0,signal=off \\ + -object monitor-hmp,chardev=char0,id=hmp0 \\ -device xen-console,chardev=char0 The Xen network device is ``xen-net-device``, which becomes the default NIC
docs/system/i386/xenpvh.rst +2 -2
@@ -33,9 +33,9 @@ case you need to construct one manually: qemu-system-i386 -xen-domid 3 -no-shutdown \ -chardev socket,id=libxl-cmd,path=/var/run/xen/qmp-libxl-3,server=on,wait=off \ - -mon chardev=libxl-cmd,mode=control \ + -object monitor-qmp,id=qmp0,chardev=libxl-cmd \ -chardev socket,id=libxenstat-cmd,path=/var/run/xen/qmp-libxenstat-3,server=on,wait=off \ - -mon chardev=libxenstat-cmd,mode=control \ + -object monitor-qmp,id=qmp1,chardev=libxenstat-cmd \ -nodefaults \ -no-user-config \ -xen-attach -name g0 \
system/vl.c +2 -0
@@ -3239,6 +3239,8 @@ void qemu_init(int argc, char **argv) default_monitor =0 ;break ;case QEMU_OPTION_mon: +warn_report_once ("'-mon' is deprecated, use '-object' with " +"'monitor-hmp' or 'monitor-qmp' types instead" );if (!qemu_opts_parse_noisily (qemu_find_opts ("mon" ), optarg, true)) {exit (1 );
qemu-options.hx +7 -8
@@ -4154,7 +4154,7 @@ The general form of a character device option is: :: -chardev stdio,mux=on,id=char0 \ - -mon chardev=char0,mode=readline \ + -object monitor-hmp,id=hmp0,chardev=char0 \ -serial chardev:char0 \ -serial chardev:char0
@@ -4166,7 +4166,7 @@ The general form of a character device option is: :: -chardev stdio,mux=on,id=char0 \ - -mon chardev=char0,mode=readline \ + -object monitor-hmp,id=hmp0,chardev=char0 \ -parallel chardev:char0 \ -chardev tcp,...,mux=on,id=char1 \ -serial chardev:char1 \
@@ -5004,12 +5004,12 @@ SRST enables the QMP monitor on localhost port 4444 with pretty-printing. - The use of ``-mon mode=readline`` is syntactic sugar + The use of ``-mon mode=readline`` is deprecated syntactic sugar for the new ``-object monitor-hmp`` option, each use of which creates an object with the ID ``compat_monitorNNN`` where ``NNN`` is a counter starting from 0. - The use of ``-mon mode=control`` is syntactic sugar + The use of ``-mon mode=control`` is deprecated syntactic sugar for the new ``-object monitor-qmp`` option, each use of which creates an object with the ID ``compat_monitorNNN`` where ``NNN`` is a counter starting from 0.
@@ -5771,8 +5771,7 @@ SRST to dynamically delete the monitor at runtime. Note that monitors created using the historical syntax will be allocated IDs following the pattern ``compat_monmitorNNN``. - Mixing ``-object`` with ``-monitor`` and ``-mon`` syntax is - discouraged. + Mixing ``-object`` with ``-monitor`` syntax is discouraged. The ``readline`` parameter, which defaults to ``on``, controls whether the monitor provides line editing.
@@ -5785,8 +5784,8 @@ SRST to dynamically delete the monitor at runtime. Note that monitors created using the historical syntax will be allocated IDs following the pattern ``compat_monitorNNN``. - Mixing ``-object`` with ``-qmp``, ``-qmp-pretty`` and - ``-mon`` syntax is discouraged. + Mixing ``-object`` with ``-qmp`` and ``-qmp-pretty`` + syntax is discouraged. The ``pretty`` parameter, which defaults to ``off``, controls whether the monitor responses are pretty
Signature · 1 line
2.54.0
Re: [PULL 35/35] docs: mark '-mon' as deprecated in favour of -object
Quoted 9 lines · Markus Armbruster
From: Daniel P. Berrangé <[email protected]>
The high level
-qmpand-monitoroptions can remain as convenience wrappers, but the low level-monis completed obsoleted by the new-objectsupport with 'monitor-qmp' and 'monitor-hmp' types.
system/vl.c void qemu_init(int argc, char **argv) · line 3243
Quoted 5 lines
system/vl.c +2
-0@@ -3239,6 +3239,8 @@ void qemu_init(int argc, char **argv) default_monitor =0 ;break ;case QEMU_OPTION_mon: +warn_report_once ("'-mon' is deprecated, use '-object' with " +"'monitor-hmp' or 'monitor-qmp' types instead" );
Can we be more precise in deprecation messages of this kind, please? This warning is not enough for the user to be able to figure out how to change their existing command line.
I had to look into deprecated.rst, check the git history to see which commit added the deprecation, then find out how this commit was adjusting the examples in the tree.
Notably, "-object monitor-hmp" appears to require an "id=" despite nothing actually caring what that ID is, and the warning message doesn't say how to determine whether your existing -mon option should be changed to monitor-hmp or monitor-qmp. The deprecated.rst docs do say that part, but don't mention the ID bit.
thanks
-- PMM
Re: [PULL 35/35] docs: mark '-mon' as deprecated in favour of -object
Quoted 20 lines · Peter Maydell
Quoted 9 lines · Markus Armbruster
From: Daniel P. Berrangé <[email protected]>
The high level
-qmpand-monitoroptions can remain as convenience wrappers, but the low level-monis completed obsoleted by the new-objectsupport with 'monitor-qmp' and 'monitor-hmp' types.system/vl.c void qemu_init(int argc, char **argv) · line 3243
Quoted 5 lines
system/vl.c +2
-0@@ -3239,6 +3239,8 @@ void qemu_init(int argc, char **argv) default_monitor =0 ;break ;case QEMU_OPTION_mon: +warn_report_once ("'-mon' is deprecated, use '-object' with " +"'monitor-hmp' or 'monitor-qmp' types instead" );Can we be more precise in deprecation messages of this kind, please? This warning is not enough for the user to be able to figure out how to change their existing command line.
Quoted 6 lines
I had to look into
deprecated.rst, check the git history to see which commit added the deprecation, then find out how this commit was adjusting the examples in the tree.Notably, "-object monitor-hmp" appears to require an "id=" despite nothing actually caring what that ID is,
Yes, -object requires @id regardless of object type. It calls object_new_with_props_from_qdict() via user_creatable_add_qapi(). Together, they create the object at /objects/ID.
Quoted 4 lines
and the warning message
doesn't say how to determine whether your existing -mon option should be changed to monitor-hmp or monitor-qmp. Thedeprecated.rstdocs do say that part, but don't mention the ID bit.
Fair points.
Is this just "do better in the future", or are you also asking for a patch to improve this warning?
Re: [PULL 35/35] docs: mark '-mon' as deprecated in favour of -object
Quoted 9 lines · Markus Armbruster
Quoted 8 lines · Peter Maydell
Can we be more precise in deprecation messages of this kind, please? This warning is not enough for the user to be able to figure out how to change their existing command line.
I had to look into
deprecated.rst, check the git history to see which commit added the deprecation, then find out how this commit was adjusting the examples in the tree.
Quoted 6 lines
Quoted 2 lines
Notably, "-object monitor-hmp" appears to require an "id=" despite nothing actually caring what that ID is,
Yes, -object requires @id regardless of object type. It calls
object_new_with_props_from_qdict()viauser_creatable_add_qapi(). Together, they create the object at /objects/ID.
Yes, but usually you need the ID because it's going to plug into some other option somewhere else; other options like -drive can take an id=, but don't insist on it if you don't need it, and some options like chardev are only useful if you connect them to something else by using the ID elsewhere in the command line, so whether they require id= or not never comes up. So the idea of an option requiring an ID even though it is never referenced anywhere else may well be new to the user.
Mostly I mention it because otherwise I think the typical user is going to do what I did, i.e. replace "-mon" with "-object monitor-hmp" and then immediately get a different error from QEMU. The warning being more precise could save them a step.
Quoted 9 lines
Quoted 4 lines
and the warning message
doesn't say how to determine whether your existing -mon option should be changed to monitor-hmp or monitor-qmp. Thedeprecated.rstdocs do say that part, but don't mention the ID bit.Fair points.
Is this just "do better in the future", or are you also asking for a patch to improve this warning?
Both, preferably :-)
There's an argument that the deprecation warning could look at the option the user actually provided and emit the exact right new option to use instead. But I don't think that's something we've ever done before for deprecations.
-- PMM
Re: [PULL 35/35] docs: mark '-mon' as deprecated in favour of -object
Quoted 17 lines · Peter Maydell
Quoted 9 lines · Markus Armbruster
Quoted 8 lines · Peter Maydell
Can we be more precise in deprecation messages of this kind, please? This warning is not enough for the user to be able to figure out how to change their existing command line.
I had to look into
deprecated.rst, check the git history to see which commit added the deprecation, then find out how this commit was adjusting the examples in the tree.Quoted 6 lines
Quoted 2 lines
Notably, "-object monitor-hmp" appears to require an "id=" despite nothing actually caring what that ID is,
Yes, -object requires @id regardless of object type. It calls
object_new_with_props_from_qdict()viauser_creatable_add_qapi(). Together, they create the object at /objects/ID.
Quoted 8 lines
Yes, but usually you need the ID because it's going to plug into some other option somewhere else; other options like -drive can take an id=, but don't insist on it if you don't need it, and some options like chardev are only useful if you connect them to something else by using the ID elsewhere in the command line, so whether they require id= or not never comes up. So the idea of an option requiring an ID even though it is never referenced anywhere else may well be new to the user.
FYI, the ID for the monitor can be used with object-del QMP/HMP commands.
Quoted 4 lines
Mostly I mention it because otherwise I think the typical user is going to do what I did, i.e. replace "-mon" with "-object monitor-hmp" and then immediately get a different error from QEMU. The warning being more precise could save them a step.
I was thinking that the user would do 'man qemu' where we have docs for "-object", for each object type including "monitor-hmp/qmp", which are shown taking an ID parameter.
Quoted 11 lines
Quoted 10 lines
Quoted 5 lines
and the warning message
doesn't say how to determine whether your existing -mon option should be changed to monitor-hmp or monitor-qmp. Thedeprecated.rstdocs do say that part, but don't mention the ID bit.Fair points.
Is this just "do better in the future", or are you also asking for a patch to improve this warning?
Quoted 6 lines
Both, preferably :-)
There's an argument that the deprecation warning could look at the option the user actually provided and emit the exact right new option to use instead. But I don't think that's something we've ever done before for deprecations.
Essentially we have three distinct places with info - the deprecation message at runtime, the deprecated.rst docs, and the qemu.1 manual page. It is a trade off between how much effort we want to put into each area.
For humans I anticipated they'd be mostly using -monitor / -qmp convenience options, with -mon mostly used by machines which are already familiar with -object usage already.
With regards,
Daniel
Signature · 3 lines
|: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
Re: [PULL 35/35] docs: mark '-mon' as deprecated in favour of -object
Quoted 9 lines · Daniel P. Berrangé
Quoted 4 lines · Peter Maydell
Mostly I mention it because otherwise I think the typical user is going to do what I did, i.e. replace "-mon" with "-object monitor-hmp" and then immediately get a different error from QEMU. The warning being more precise could save them a step.
I was thinking that the user would do 'man qemu' where we have docs for "-object", for each object type including "monitor-hmp/qmp", which are shown taking an ID parameter.
But they only have to look at the docs because the warning message we just printed doesn't give them enough information.
FWIW, I did try the docs: putting "monitor-hmp" into the online HTML search produces as the top hit the "Deprecated features" subsection, which also doesn't give enough information (and doesn't hyperlink to anywhere else that gives more information[*]), and then a lot of other irrelevant hits because the search seems to be word based so it shows you everything with either "monitor" or "HMP".
At this point I gave up and started looking around in the git history.
[*] partly this is because hyperlinks into the qemu-options.hx docs are awkward. We have syntax so you can say "SRST(something)" to get a linkable label at the top of a section, but nothing more precisely targeted than that. (I think that ultimately we should consider splitting out "build manpages" and "build HTML docs" into completely separate sphinx doc source trees and sphinx-doc runs (with build time fiddling for where they share content), because sphinx really really doesn't want "different docs formats are different content" and it shows up in various places including this "it's hard to define a label in content that's shared between manpages and HTML" one.)
Quoted 9 lines
Quoted 4 lines
There's an argument that the deprecation warning could look at the option the user actually provided and emit the exact right new option to use instead. But I don't think that's something we've ever done before for deprecations.
Essentially we have three distinct places with info - the deprecation message at runtime, the
deprecated.rstdocs, and the qemu.1 manual page. It is a trade off between how much effort we want to put into each area.
One of these is the one that by definition every user who needs to update their command lines will see, though.
Quoted 3 lines
For humans I anticipated they'd be mostly using -monitor / -qmp convenience options, with -mon mostly used by machines which are already familiar with -object usage already.
Mmm. I'm not quite sure why my scripts use -mon + -chardev rather than '-monitor tcp:...'.
thanks
-- PMM
[PULL 32/35] monitor: add support for auto-deleting monitors upon close
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
The default monitor is usually a long lived object that will exist for the entire lifetime of the VM. A monitor can only service a single client at a time though, and so it might be desirable to hotplug additional monitors at runtime for specific tasks. If doing that, however, there is a need to remove the monitor when it is no longer needed.
A use case for hotplugging a monitor can involve a user wishing to spawn an ad hoc script that uses a temporary monitor. The script can ask the management application to hotplug a monitor and pass back a pre-opened FD using SCM_RIGHTS. In this case the lifetime of the script is not tied to the management application and thus it is desirable to have automatic cleanup when the script exits.
Allowing a client to run "object-del" against its own monitor adds complex edge cases, as it would be desirable to send the QMP response despite the monitor sending it being deleted. Doing "object-del" alone will also result in orphaning a character device backend instance, as there is no opportunity to run the companion "chardev-del" command.
A simpler way to ensure cleanup is to add the concept of auto-deleting monitor objects. Specifically when the "CHR_EVENT_CLOSED" event is emitted, the equivalent of "object-del" + "chardev-del" can be run internally. Since the transient client has already droppped its monitor connection, there is no synchronization to be concerned about with sending QMP replies. There is still some internal synchronization needed, however, between the character device event callback and the bottom-half that runs the delete. There is a chance that an incoming client connection may arise before the bottom-half runs, which has to be checked. Once the monitor object is deleted, the event callback is unregistered from the character device, eliminating any further races before the character device is fully deleted.
This is implemented via a new "close-action=none|delete" property on the 'monitor-qmp' object. This concept could be extended with further actions in future, for example:
* close-action=shutdown - graceful guest shutdown
* close-action=terminate - immediate guest poweroff
* close-action=stop - pause guest CPUs while the monitor is not connected to any client
This is left as an exercise for future interested contributors.
Message-ID: <[email protected]>
--- qapi/qom.json | 22 ++++- monitor/monitor-internal.h | 3 + monitor/qmp.c | 80 ++++++++++++++++++ .../generic/test_monitor_hotplug.py | 83 ++++++++++++++++++- 4 files changed, 183 insertions(+), 5 deletions(-)
qapi/qom.json +21 -1
@@ -1213,6 +1213,22 @@ 'base': 'MonitorProperties', 'data': { '*readline': 'bool' } } + +## +# @MonitorQMPCloseAction: +# +# Action to take when the character device backend is closed. +# +# @none: take no action +# +# @delete: delete both the 'monitor-qmp' object and its associated +# character device backend object +# +# Since: 11.1 +## +{ 'enum': 'MonitorQMPCloseAction', + 'data': ['none', 'delete'] } + ## # @MonitorQMPProperties: #
@@ -1220,11 +1236,15 @@ # # @pretty: whether to pretty print JSON responses (default: false) # +# @close-action: action to take when the character device backend is +# closed (default: none) +# # Since: 11.1 ## { 'struct': 'MonitorQMPProperties', 'base': 'MonitorProperties', - 'data': { '*pretty': 'bool' } } + 'data': { '*pretty': 'bool', + '*close-action': 'MonitorQMPCloseAction' } } ## # @ObjectType:
monitor/monitor-internal.h +3 -0
@@ -28,6 +28,7 @@#include "chardev/char-fe.h" #include "monitor/monitor.h" #include "qapi/qapi-types-control.h" +#include "qapi/qapi-types-qom.h" #include "qapi/qmp-registry.h" #include "qobject/json-parser.h" #include "qemu/readline.h"
@@ -178,7 +179,9 @@ struct MonitorQMP {Monitor parent_obj;JSONMessageParser parser;bool pretty; +MonitorQMPCloseAction close_action;bool setup_pending;/* iothread BH has not yet set up chardev handlers */ +bool delete_pending;/* close_action has started 'delete' process */ /* * When a client connects, we're in capabilities negotiation mode. * @commands is &qmp_cap_negotiation_commands then. When command
monitor/qmp.c +80 -0
115 lines not on this page · raw message
tests/functional/generic/test_monitor_hotplug.py +79 -4
101 lines not on this page · raw message
Signature · 1 line
2.54.0
[PULL 02/35] monitor: replace 'common' with 'parent_obj' in MonitorHMP
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
The field name 'parent_obj' is standard practice for QOM structs so align the HMP monitor.
Message-ID: <[email protected]>
--- monitor/monitor-internal.h | 2 +- monitor/hmp-cmds.c | 2 +- monitor/hmp.c | 43 ++++++++++++++++++++------------------ monitor/monitor.c | 6 +++--- monitor/qmp-cmds.c | 10 ++++----- ui/ui-hmp-cmds.c | 2 +- 6 files changed, 34 insertions(+), 31 deletions(-)
monitor/monitor-internal.h +1 -1
@@ -128,7 +128,7 @@ struct Monitor { };struct MonitorHMP { -Monitor common; +Monitor parent_obj;bool use_readline;/* * State used only in the thread "owning" the monitor.
monitor/hmp-cmds.c +1 -1
@@ -287,7 +287,7 @@ void hmp_info_sync_profile(Monitor *mon, const QDict *qdict)void hmp_info_history (Monitor *mon,const QDict *qdict) { -MonitorHMP *hmp_mon =container_of (mon, MonitorHMP, common); +MonitorHMP *hmp_mon =container_of (mon, MonitorHMP, parent_obj);int i;const char *str;
monitor/hmp.c +23 -20
130 lines not on this page · raw message
monitor/monitor.c +3 -3
@@ -132,7 +132,7 @@ static inline bool monitor_is_hmp_non_interactive(const Monitor *mon)return false; } -return !monitor_uses_readline (container_of (mon, MonitorHMP, common)); +return !monitor_uses_readline (container_of (mon, MonitorHMP, parent_obj)); } static gboolean monitor_unblocked(void *do_not_use, GIOCondition cond,
@@ -542,7 +542,7 @@ static void monitor_accept_input(void *opaque)qemu_mutex_lock (&mon->mon_lock);if (!monitor_is_qmp (mon) && mon->reset_seen) { -MonitorHMP *hmp_mon =container_of (mon, MonitorHMP, common); +MonitorHMP *hmp_mon =container_of (mon, MonitorHMP, parent_obj);assert (hmp_mon->rs);readline_restart (hmp_mon->rs);qemu_mutex_unlock (&mon->mon_lock);
@@ -627,7 +627,7 @@ void monitor_data_destroy(Monitor *mon)if (monitor_is_qmp (mon)) {monitor_data_destroy_qmp (container_of (mon, MonitorQMP, common)); }else { -readline_free (container_of (mon, MonitorHMP, common)->rs); +readline_free (container_of (mon, MonitorHMP, parent_obj)->rs); }g_string_free (mon->outbuf, true);qemu_mutex_destroy (&mon->mon_lock);
monitor/qmp-cmds.c +5 -5
@@ -168,10 +168,10 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,char *output =NULL ;MonitorHMP hmp = {}; -monitor_data_init (&hmp.common, false, true, false); +monitor_data_init (&hmp.parent_obj, false, true, false);if (has_cpu_index) { -int ret =monitor_set_cpu (&hmp.common, cpu_index); +int ret =monitor_set_cpu (&hmp.parent_obj, cpu_index);if (ret <0 ) {error_setg (errp,QERR_INVALID_PARAMETER_VALUE ,"cpu-index" ,"a CPU number" );
@@ -181,12 +181,12 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,handle_hmp_command (&hmp, command_line); - WITH_QEMU_LOCK_GUARD(&hmp.common.mon_lock) { - output =g_strdup (hmp.common.outbuf->str); + WITH_QEMU_LOCK_GUARD(&hmp.parent_obj.mon_lock) { + output =g_strdup (hmp.parent_obj.outbuf->str); } out: -monitor_data_destroy (&hmp.common); +monitor_data_destroy (&hmp.parent_obj);return output; }
ui/ui-hmp-cmds.c +1 -1
@@ -343,7 +343,7 @@ void hmp_change_vnc(Monitor *mon, const char *device, const char *target,return ; }if (!arg) { -MonitorHMP *hmp_mon =container_of (mon, MonitorHMP, common); +MonitorHMP *hmp_mon =container_of (mon, MonitorHMP, parent_obj);monitor_read_password (hmp_mon, hmp_change_read_arg,NULL ); }else {qmp_change_vnc_password (arg, errp);
Signature · 1 line
2.54.0
[PULL 10/35] monitor: remove 'skip_flush' field
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
The 'skip_flush' field is set on the dummy throwaway HMP monitor object created by QMP's 'human-monitor-command', as an indication not to try to write data to the chardev. Instead the QMP command impl will grab the data straight out of the in-memory buffer.
The flag is redundant, however, as the monitor code could instead simply check the 'fe_is_open' field on the CharFrontend, which will be false in the same scenarios that 'skip_flush' is true.
Message-ID: <[email protected]>
--- monitor/monitor-internal.h | 4 +--- monitor/hmp.c | 2 +- monitor/monitor.c | 11 +++++++---- monitor/qmp-cmds.c | 2 +- monitor/qmp.c | 2 +- 5 files changed, 11 insertions(+), 10 deletions(-)
monitor/monitor-internal.h +1 -3
@@ -112,7 +112,6 @@ struct Monitor {CharFrontend chr;int suspend_cnt;/* Needs to be accessed atomically */ bool is_qmp; -bool skip_flush;bool use_io_thread;char *mon_cpu_path;
@@ -194,8 +193,7 @@ extern QemuMutex monitor_lock;extern MonitorList mon_list;void monitor_complete (Monitor *mon,Error **errp); -void monitor_data_init (Monitor *mon,bool is_qmp,bool skip_flush, -bool use_io_thread); +void monitor_data_init (Monitor *mon,bool is_qmp,bool use_io_thread);void monitor_data_destroy (Monitor *mon);int monitor_can_read (void *opaque);void monitor_list_append (Monitor *mon);
monitor/hmp.c +1 -1
@@ -1590,7 +1590,7 @@ void monitor_new_hmp(const char *id, const char *chardev_id,return ; } -monitor_data_init (&mon->parent_obj, false, false, false); +monitor_data_init (&mon->parent_obj, false, false);if (mon->use_readline) { mon->rs = readline_init(monitor_readline_printf,
monitor/monitor.c +7 -4
@@ -190,7 +190,12 @@ void monitor_flush_locked(Monitor *mon)size_t len;const char *buf; -if (mon->skip_flush) { +/* +* When used by QMP human-monitor-command, no chardev +* will be connected, as we want to just collect the +* output in the buffer +*/ +if (!mon->chr.fe_is_open) {return ; }
@@ -644,8 +649,7 @@ static void monitor_iothread_init(void) mon_iothread =iothread_create ("mon_iothread" , &error_abort); } -void monitor_data_init (Monitor *mon,bool is_qmp,bool skip_flush, -bool use_io_thread) +void monitor_data_init (Monitor *mon,bool is_qmp,bool use_io_thread) {if (use_io_thread && !mon_iothread) {monitor_iothread_init ();
@@ -653,7 +657,6 @@ void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,qemu_mutex_init (&mon->mon_lock); mon->is_qmp = is_qmp; mon->outbuf =g_string_new (NULL ); - mon->skip_flush = skip_flush; mon->use_io_thread = use_io_thread; }
monitor/qmp-cmds.c +1 -1
@@ -168,7 +168,7 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,char *output =NULL ;MonitorHMP *hmp =MONITOR_HMP (object_new (TYPE_MONITOR_HMP )); -monitor_data_init (&hmp->parent_obj, false, true, false); +monitor_data_init (&hmp->parent_obj, false, false);if (has_cpu_index) {int ret =monitor_set_cpu (&hmp->parent_obj, cpu_index);
monitor/qmp.c +1 -1
@@ -572,7 +572,7 @@ void monitor_new_qmp(const char *id, const char *chardev_id,qemu_chr_fe_set_echo (&mon->parent_obj.chr, true);/* Note: we run QMP monitor in I/O thread when @chr supports that */ -monitor_data_init (&mon->parent_obj, true, false, +monitor_data_init (&mon->parent_obj, true,qemu_chr_has_feature (mon->parent_obj.chr.chr,QEMU_CHAR_FEATURE_GCONTEXT ));
Signature · 1 line
2.54.0
[PULL 16/35] monitor: use dynamic cast in monitor_qmp_requests_pop_any_with_lock
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
This eliminates a use of monitor_is_qmp() from the QMP coroutine dispatch path.
Message-ID: <[email protected]>
--- monitor/qmp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
monitor/qmp.c +3 -2
@@ -277,11 +277,12 @@ static QMPRequest *monitor_qmp_requests_pop_any_with_lock(void)MonitorQMP *qmp_mon; QTAILQ_FOREACH(mon, &mon_list, entry) { -if (!monitor_is_qmp (mon)) { + qmp_mon =MONITOR_QMP ( +object_dynamic_cast (OBJECT (mon),TYPE_MONITOR_QMP )); +if (!qmp_mon) {continue ; } - qmp_mon =container_of (mon, MonitorQMP, parent_obj);qemu_mutex_lock (&qmp_mon->qmp_queue_lock); req_obj =g_queue_pop_head (qmp_mon->qmp_requests);if (req_obj) {
Signature · 1 line
2.54.0
[PULL 14/35] monitor: use class methods for monitor_accept_input
In reply to Markus ArmbrusterFrom: Daniel P. Berrangé <[email protected]>
This removes the need for using monitor_is_qmp() to check the subclass type, which is an anti-pattern.
Message-ID: <[email protected]>
--- monitor/monitor-internal.h | 5 +++++ monitor/hmp.c | 16 ++++++++++++++++ monitor/monitor.c | 12 +++--------- 3 files changed, 24 insertions(+), 9 deletions(-)
monitor/monitor-internal.h +5 -0
@@ -116,6 +116,11 @@ struct MonitorClass {* notifications back to the client */ void (*emit_event)(Monitor *mon,QAPIEvent event,QDict *qdict); +/* +* If non-NULL, perform any actions needed to prepare +* the monitor to accept further client input +*/ +void (*accept_input)(Monitor *mon); };struct Monitor {
monitor/hmp.c +16 -0
@@ -70,6 +70,7 @@ static void monitor_hmp_set_readline(Object *obj, bool val, Error **errp) int monitor_hmp_vprintf(Monitor *mon, const char *fmt, va_list ap) G_GNUC_PRINTF(2, 0); +static void monitor_hmp_accept_input (Monitor *mon);static void monitor_hmp_class_init (ObjectClass *cls,const void *data) {
@@ -80,6 +81,7 @@ static void monitor_hmp_class_init(ObjectClass *cls, const void *data) monitor_hmp_set_readline); moncls->vprintf = monitor_hmp_vprintf; + moncls->accept_input = monitor_hmp_accept_input; }static void monitor_hmp_init (Object *obj)
@@ -100,6 +102,20 @@ int monitor_hmp_vprintf(Monitor *mon, const char *fmt, va_list ap)return monitor_puts (mon, buf); } +static void monitor_hmp_accept_input (Monitor *mon) +{ +qemu_mutex_lock (&mon->mon_lock); +if (mon->reset_seen) { +MonitorHMP *hmp =MONITOR_HMP (mon); +assert (hmp->rs); +readline_restart (hmp->rs); +qemu_mutex_unlock (&mon->mon_lock); +readline_show_prompt (hmp->rs); + }else { +qemu_mutex_unlock (&mon->mon_lock); + } +} +static void monitor_command_cb (void *opaque,const char *cmdline,void *readline_opaque) {
monitor/monitor.c +3 -9
@@ -578,16 +578,10 @@ int monitor_suspend(Monitor *mon)static void monitor_accept_input (void *opaque) {Monitor *mon = opaque; +MonitorClass *cls =MONITOR_GET_CLASS (mon); -qemu_mutex_lock (&mon->mon_lock); -if (!monitor_is_qmp (mon) && mon->reset_seen) { -MonitorHMP *hmp_mon =container_of (mon, MonitorHMP, parent_obj); -assert (hmp_mon->rs); -readline_restart (hmp_mon->rs); -qemu_mutex_unlock (&mon->mon_lock); -readline_show_prompt (hmp_mon->rs); - }else { -qemu_mutex_unlock (&mon->mon_lock); +if (cls->accept_input) { + cls->accept_input (mon); }qemu_chr_fe_accept_input (&mon->chr);
Signature · 1 line
2.54.0
Stefan Hajnoczi
AppliedApplied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/11.1 for any user-visible changes.