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. Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- monitor/hmp.c | 6 +++-- monitor/monitor-internal.h | 3 +-- monitor/monitor.c | 46 +++++++++++++------------------------- monitor/qmp-cmds.c | 3 --- monitor/qmp.c | 34 +++++++++++++++------------- 5 files changed, 40 insertions(+), 52 deletions(-) diff --git a/monitor/hmp.c b/monitor/hmp.c index 516aa25d4c..2bb2333da5 100644 --- a/monitor/hmp.c +++ b/monitor/hmp.c @@ -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 *chardev_id, bool use_readline, Error **errp) return; } - monitor_data_init(&mon->parent_obj, false, false); - if (mon->use_readline) { mon->rs = readline_init(monitor_readline_printf, monitor_readline_flush, diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h index d34f9be139..6a4c3b0f37 100644 --- a/monitor/monitor-internal.h +++ b/monitor/monitor-internal.h @@ -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); diff --git a/monitor/monitor.c b/monitor/monitor.c index e3e5623a4b..76df379a2c 100644 --- a/monitor/monitor.c +++ b/monitor/monitor.c @@ -80,6 +80,10 @@ static void monitor_finalize(Object *obj) Monitor *mon = MONITOR(obj); g_free(mon->chardev_id); + g_free(mon->mon_cpu_path); + qemu_chr_fe_deinit(&mon->chr, false); + g_string_free(mon->outbuf, true); + qemu_mutex_destroy(&mon->mon_lock); } static char *monitor_get_chardev_id(Object *obj, Error **errp) @@ -106,6 +110,11 @@ static void monitor_class_init(ObjectClass *cls, const void *data) 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); } Monitor *monitor_cur(void) @@ -637,38 +646,16 @@ void monitor_list_append(Monitor *mon) qemu_mutex_unlock(&monitor_lock); if (mon) { - monitor_data_destroy(mon); object_unparent(OBJECT(mon)); } } -static void monitor_iothread_init(void) -{ - mon_iothread = iothread_create("mon_iothread", &error_abort); -} - -void monitor_data_init(Monitor *mon, bool is_qmp, bool use_io_thread) +void monitor_iothread_init(Monitor *mon) { - if (use_io_thread && !mon_iothread) { - monitor_iothread_init(); + if (!mon_iothread) { + mon_iothread = iothread_create("mon_iothread", &error_abort); } - qemu_mutex_init(&mon->mon_lock); - mon->is_qmp = is_qmp; - mon->outbuf = g_string_new(NULL); - mon->use_io_thread = use_io_thread; -} - -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, parent_obj)); - } else { - readline_free(container_of(mon, MonitorHMP, parent_obj)->rs); - } - g_string_free(mon->outbuf, true); - qemu_mutex_destroy(&mon->mon_lock); + mon->use_io_thread = true; } void monitor_cleanup(void) @@ -686,7 +673,7 @@ void monitor_cleanup(void) * Letting the iothread continue while shutting down the dispatcher * means that new requests may still be coming in. This is okay, * we'll just leave them in the queue without sending a response - * and monitor_data_destroy() will free them. + * and object finalization will free them. */ WITH_QEMU_LOCK_GUARD(&monitor_lock) { qmp_dispatcher_co_shutdown = true; @@ -700,8 +687,8 @@ void monitor_cleanup(void) /* * We need to explicitly stop the I/O thread (but not destroy it), * clean up the monitor resources, then destroy the I/O thread since - * we need to unregister from chardev below in - * monitor_data_destroy(), and chardev is not thread-safe yet + * we need to unregister from chardev below in object + * finalization, and chardev is not thread-safe yet */ if (mon_iothread) { iothread_stop(mon_iothread); @@ -716,7 +703,6 @@ void monitor_cleanup(void) /* Permit QAPI event emission from character frontend release */ qemu_mutex_unlock(&monitor_lock); monitor_flush(mon); - monitor_data_destroy(mon); qemu_mutex_lock(&monitor_lock); object_unparent(OBJECT(mon)); } diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c index 0d9adad288..6cb0b587fb 100644 --- a/monitor/qmp-cmds.c +++ b/monitor/qmp-cmds.c @@ -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; } diff --git a/monitor/qmp.c b/monitor/qmp.c index e3318b57db..694d303215 100644 --- a/monitor/qmp.c +++ b/monitor/qmp.c @@ -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 *chardev_id, bool pretty, Error **errp) 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)); - - qemu_mutex_init(&mon->qmp_queue_lock); - mon->qmp_requests = g_queue_new(); + if (qemu_chr_has_feature(mon->parent_obj.chr.chr, + QEMU_CHAR_FEATURE_GCONTEXT)) { + monitor_iothread_init(&mon->parent_obj); + } - 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 -- 2.54.0