| Message ID | 20190607135430.22149-4-kwolf@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | monitor: Split monitor.c in core/HMP/QMP/misc | expand |
* Kevin Wolf (kwolf@redhat.com) wrote: > Currently, struct Monitor mixes state that is only relevant for HMP, > state that is only relevant for QMP, and some actually shared state. > In particular, a MonitorQMP field is present in the state of any > monitor, even if it's not a QMP monitor and therefore doesn't use the > state. > > As a first step towards a clean separation between QMP and HMP, let > MonitorQMP extend Monitor and create a MonitorQMP object only when the > monitor is actually a QMP monitor. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > monitor.c | 214 ++++++++++++++++++++++++++++++------------------------ > 1 file changed, 118 insertions(+), 96 deletions(-) > > diff --git a/monitor.c b/monitor.c > index bb23cc0450..d18cf18393 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -166,26 +166,6 @@ struct MonFdset { > QLIST_ENTRY(MonFdset) next; > }; > > -typedef struct { > - JSONMessageParser parser; > - /* > - * When a client connects, we're in capabilities negotiation mode. > - * @commands is &qmp_cap_negotiation_commands then. When command > - * qmp_capabilities succeeds, we go into command mode, and > - * @command becomes &qmp_commands. > - */ > - QmpCommandList *commands; > - bool capab_offered[QMP_CAPABILITY__MAX]; /* capabilities offered */ > - bool capab[QMP_CAPABILITY__MAX]; /* offered and accepted */ > - /* > - * Protects qmp request/response queue. > - * Take monitor_lock first when you need both. > - */ > - QemuMutex qmp_queue_lock; > - /* Input queue that holds all the parsed QMP requests */ > - GQueue *qmp_requests; > -} MonitorQMP; > - > /* > * To prevent flooding clients, events can be throttled. The > * throttling is calculated globally, rather than per-Monitor > @@ -218,7 +198,6 @@ struct Monitor { > */ > ReadLineState *rs; > > - MonitorQMP qmp; > gchar *mon_cpu_path; > mon_cmd_t *cmd_table; > QTAILQ_ENTRY(Monitor) entry; > @@ -239,6 +218,27 @@ struct Monitor { > int mux_out; > }; > > +typedef struct { > + Monitor common; > + JSONMessageParser parser; > + /* > + * When a client connects, we're in capabilities negotiation mode. > + * @commands is &qmp_cap_negotiation_commands then. When command > + * qmp_capabilities succeeds, we go into command mode, and > + * @command becomes &qmp_commands. > + */ > + QmpCommandList *commands; > + bool capab_offered[QMP_CAPABILITY__MAX]; /* capabilities offered */ > + bool capab[QMP_CAPABILITY__MAX]; /* offered and accepted */ > + /* > + * Protects qmp request/response queue. > + * Take monitor_lock first when you need both. > + */ > + QemuMutex qmp_queue_lock; > + /* Input queue that holds all the parsed QMP requests */ > + GQueue *qmp_requests; > +} MonitorQMP; > + > /* Shared monitor I/O thread */ > IOThread *mon_iothread; > > @@ -247,7 +247,7 @@ QEMUBH *qmp_dispatcher_bh; > > struct QMPRequest { > /* Owner of the request */ > - Monitor *mon; > + MonitorQMP *mon; > /* > * Request object to be handled or Error to be reported > * (exactly one of them is non-null) > @@ -355,18 +355,18 @@ static void qmp_request_free(QMPRequest *req) > } > > /* Caller must hold mon->qmp.qmp_queue_lock */ > -static void monitor_qmp_cleanup_req_queue_locked(Monitor *mon) > +static void monitor_qmp_cleanup_req_queue_locked(MonitorQMP *mon) > { > - while (!g_queue_is_empty(mon->qmp.qmp_requests)) { > - qmp_request_free(g_queue_pop_head(mon->qmp.qmp_requests)); > + while (!g_queue_is_empty(mon->qmp_requests)) { > + qmp_request_free(g_queue_pop_head(mon->qmp_requests)); > } > } > > -static void monitor_qmp_cleanup_queues(Monitor *mon) > +static void monitor_qmp_cleanup_queues(MonitorQMP *mon) > { > - qemu_mutex_lock(&mon->qmp.qmp_queue_lock); > + qemu_mutex_lock(&mon->qmp_queue_lock); > monitor_qmp_cleanup_req_queue_locked(mon); > - qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); > + qemu_mutex_unlock(&mon->qmp_queue_lock); > } > > > @@ -478,17 +478,17 @@ int monitor_printf(Monitor *mon, const char *fmt, ...) > return ret; > } > > -static void qmp_send_response(Monitor *mon, const QDict *rsp) > +static void qmp_send_response(MonitorQMP *mon, const QDict *rsp) > { > const QObject *data = QOBJECT(rsp); > QString *json; > > - json = mon->flags & MONITOR_USE_PRETTY ? qobject_to_json_pretty(data) : > - qobject_to_json(data); > + json = mon->common.flags & MONITOR_USE_PRETTY ? > + qobject_to_json_pretty(data) : qobject_to_json(data); > assert(json != NULL); > > qstring_append_chr(json, '\n'); > - monitor_puts(mon, qstring_get_str(json)); > + monitor_puts(&mon->common, qstring_get_str(json)); > > qobject_unref(json); > } > @@ -511,12 +511,17 @@ static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = { > 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) > - && mon->qmp.commands != &qmp_cap_negotiation_commands) { > - qmp_send_response(mon, qdict); > + if (!monitor_is_qmp(mon)) { > + continue; > + } > + > + qmp_mon = container_of(mon, MonitorQMP, common); > + if (qmp_mon->commands != &qmp_cap_negotiation_commands) { > + qmp_send_response(qmp_mon, qdict); > } > } > } > @@ -710,29 +715,33 @@ static void monitor_data_init(Monitor *mon, int flags, bool skip_flush, > } > memset(mon, 0, sizeof(Monitor)); > qemu_mutex_init(&mon->mon_lock); > - qemu_mutex_init(&mon->qmp.qmp_queue_lock); > mon->outbuf = qstring_new(); > /* Use *mon_cmds by default. */ > mon->cmd_table = mon_cmds; > mon->skip_flush = skip_flush; > mon->use_io_thread = use_io_thread; > - mon->qmp.qmp_requests = g_queue_new(); > mon->flags = flags; > } > > +static 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_data_destroy(Monitor *mon) > { > g_free(mon->mon_cpu_path); > qemu_chr_fe_deinit(&mon->chr, false); > if (monitor_is_qmp(mon)) { > - json_message_parser_destroy(&mon->qmp.parser); > + MonitorQMP *qmp_mon = container_of(mon, MonitorQMP, common); > + monitor_data_destroy_qmp(qmp_mon); > } > readline_free(mon->rs); > qobject_unref(mon->outbuf); > qemu_mutex_destroy(&mon->mon_lock); > - qemu_mutex_destroy(&mon->qmp.qmp_queue_lock); > - monitor_qmp_cleanup_req_queue_locked(mon); > - g_queue_free(mon->qmp.qmp_requests); > } > > char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index, > @@ -1085,8 +1094,9 @@ static void query_commands_cb(QmpCommand *cmd, void *opaque) > CommandInfoList *qmp_query_commands(Error **errp) > { > CommandInfoList *list = NULL; > + MonitorQMP *mon = container_of(cur_mon, MonitorQMP, common); > > - qmp_for_each_command(cur_mon->qmp.commands, query_commands_cb, &list); > + qmp_for_each_command(mon->commands, query_commands_cb, &list); > > return list; > } > @@ -1153,16 +1163,16 @@ static void monitor_init_qmp_commands(void) > qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG); > } > > -static bool qmp_oob_enabled(Monitor *mon) > +static bool qmp_oob_enabled(MonitorQMP *mon) > { > - return mon->qmp.capab[QMP_CAPABILITY_OOB]; > + return mon->capab[QMP_CAPABILITY_OOB]; > } > > -static void monitor_qmp_caps_reset(Monitor *mon) > +static void monitor_qmp_caps_reset(MonitorQMP *mon) > { > - memset(mon->qmp.capab_offered, 0, sizeof(mon->qmp.capab_offered)); > - memset(mon->qmp.capab, 0, sizeof(mon->qmp.capab)); > - mon->qmp.capab_offered[QMP_CAPABILITY_OOB] = mon->use_io_thread; > + 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; > } > > /* > @@ -1170,7 +1180,7 @@ static void monitor_qmp_caps_reset(Monitor *mon) > * On success, set mon->qmp.capab[], and return true. > * On error, set @errp, and return false. > */ > -static bool qmp_caps_accept(Monitor *mon, QMPCapabilityList *list, > +static bool qmp_caps_accept(MonitorQMP *mon, QMPCapabilityList *list, > Error **errp) > { > GString *unavailable = NULL; > @@ -1179,7 +1189,7 @@ static bool qmp_caps_accept(Monitor *mon, QMPCapabilityList *list, > memset(capab, 0, sizeof(capab)); > > for (; list; list = list->next) { > - if (!mon->qmp.capab_offered[list->value]) { > + if (!mon->capab_offered[list->value]) { > if (!unavailable) { > unavailable = g_string_new(QMPCapability_str(list->value)); > } else { > @@ -1196,25 +1206,27 @@ static bool qmp_caps_accept(Monitor *mon, QMPCapabilityList *list, > return false; > } > > - memcpy(mon->qmp.capab, capab, sizeof(capab)); > + memcpy(mon->capab, capab, sizeof(capab)); > return true; > } > > void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable, > Error **errp) > { > - if (cur_mon->qmp.commands == &qmp_commands) { > + MonitorQMP *mon = container_of(cur_mon, MonitorQMP, common); > + > + if (mon->commands == &qmp_commands) { > error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND, > "Capabilities negotiation is already complete, command " > "ignored"); > return; > } > > - if (!qmp_caps_accept(cur_mon, enable, errp)) { > + if (!qmp_caps_accept(mon, enable, errp)) { > return; > } > > - cur_mon->qmp.commands = &qmp_commands; > + mon->commands = &qmp_commands; > } > > /* Set the current CPU defined by the user. Callers must hold BQL. */ > @@ -4121,27 +4133,27 @@ static int monitor_can_read(void *opaque) > * Null @rsp can only happen for commands with QCO_NO_SUCCESS_RESP. > * Nothing is emitted then. > */ > -static void monitor_qmp_respond(Monitor *mon, QDict *rsp) > +static void monitor_qmp_respond(MonitorQMP *mon, QDict *rsp) > { > if (rsp) { > qmp_send_response(mon, rsp); > } > } > > -static void monitor_qmp_dispatch(Monitor *mon, QObject *req) > +static void monitor_qmp_dispatch(MonitorQMP *mon, QObject *req) > { > Monitor *old_mon; > QDict *rsp; > QDict *error; > > old_mon = cur_mon; > - cur_mon = mon; > + cur_mon = &mon->common; > > - rsp = qmp_dispatch(mon->qmp.commands, req, qmp_oob_enabled(mon)); > + rsp = qmp_dispatch(mon->commands, req, qmp_oob_enabled(mon)); > > cur_mon = old_mon; > > - if (mon->qmp.commands == &qmp_cap_negotiation_commands) { > + if (mon->commands == &qmp_cap_negotiation_commands) { > error = qdict_get_qdict(rsp, "error"); > if (error > && !g_strcmp0(qdict_get_try_str(error, "class"), > @@ -4166,24 +4178,30 @@ static void monitor_qmp_dispatch(Monitor *mon, QObject *req) > * monitor to the end of mon_list queue. > * > * Note: if the function returned with non-NULL, then the caller will > - * be with mon->qmp.qmp_queue_lock held, and the caller is responsible > + * be with qmp_mon->qmp_queue_lock held, and the caller is responsible > * to release it. > */ > static QMPRequest *monitor_qmp_requests_pop_any_with_lock(void) > { > QMPRequest *req_obj = NULL; > Monitor *mon; > + MonitorQMP *qmp_mon; > > qemu_mutex_lock(&monitor_lock); > > QTAILQ_FOREACH(mon, &mon_list, entry) { > - qemu_mutex_lock(&mon->qmp.qmp_queue_lock); > - req_obj = g_queue_pop_head(mon->qmp.qmp_requests); > + if (!monitor_is_qmp(mon)) { > + continue; > + } > + > + qmp_mon = container_of(mon, MonitorQMP, common); > + qemu_mutex_lock(&qmp_mon->qmp_queue_lock); > + req_obj = g_queue_pop_head(qmp_mon->qmp_requests); > if (req_obj) { > /* With the lock of corresponding queue held */ > break; > } > - qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); > + qemu_mutex_unlock(&qmp_mon->qmp_queue_lock); > } > > if (req_obj) { > @@ -4205,7 +4223,7 @@ static void monitor_qmp_bh_dispatcher(void *data) > QMPRequest *req_obj = monitor_qmp_requests_pop_any_with_lock(); > QDict *rsp; > bool need_resume; > - Monitor *mon; > + MonitorQMP *mon; > > if (!req_obj) { > return; > @@ -4214,8 +4232,8 @@ static void monitor_qmp_bh_dispatcher(void *data) > mon = req_obj->mon; > /* qmp_oob_enabled() might change after "qmp_capabilities" */ > need_resume = !qmp_oob_enabled(mon) || > - mon->qmp.qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1; > - qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); > + mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1; > + qemu_mutex_unlock(&mon->qmp_queue_lock); > if (req_obj->req) { > QDict *qdict = qobject_to(QDict, req_obj->req); > QObject *id = qdict ? qdict_get(qdict, "id") : NULL; > @@ -4231,7 +4249,7 @@ static void monitor_qmp_bh_dispatcher(void *data) > > if (need_resume) { > /* Pairs with the monitor_suspend() in handle_qmp_command() */ > - monitor_resume(mon); > + monitor_resume(&mon->common); > } > qmp_request_free(req_obj); > > @@ -4241,7 +4259,7 @@ static void monitor_qmp_bh_dispatcher(void *data) > > static void handle_qmp_command(void *opaque, QObject *req, Error *err) > { > - Monitor *mon = opaque; > + MonitorQMP *mon = opaque; > QObject *id = NULL; > QDict *qdict; > QMPRequest *req_obj; > @@ -4273,7 +4291,7 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) > req_obj->err = err; > > /* Protect qmp_requests and fetching its length. */ > - qemu_mutex_lock(&mon->qmp.qmp_queue_lock); > + qemu_mutex_lock(&mon->qmp_queue_lock); > > /* > * Suspend the monitor when we can't queue more requests after > @@ -4282,8 +4300,8 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) > * command, for backward compatibility. > */ > if (!qmp_oob_enabled(mon) || > - mon->qmp.qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) { > - monitor_suspend(mon); > + mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) { > + monitor_suspend(&mon->common); > } > > /* > @@ -4291,9 +4309,9 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) > * handled in time order. Ownership for req_obj, req, > * etc. will be delivered to the handler side. > */ > - assert(mon->qmp.qmp_requests->length < QMP_REQ_QUEUE_LEN_MAX); > - g_queue_push_tail(mon->qmp.qmp_requests, req_obj); > - qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); > + assert(mon->qmp_requests->length < QMP_REQ_QUEUE_LEN_MAX); > + g_queue_push_tail(mon->qmp_requests, req_obj); > + qemu_mutex_unlock(&mon->qmp_queue_lock); > > /* Kick the dispatcher routine */ > qemu_bh_schedule(qmp_dispatcher_bh); > @@ -4301,9 +4319,9 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) > > static void monitor_qmp_read(void *opaque, const uint8_t *buf, int size) > { > - Monitor *mon = opaque; > + MonitorQMP *mon = opaque; > > - json_message_parser_feed(&mon->qmp.parser, (const char *) buf, size); > + json_message_parser_feed(&mon->parser, (const char *) buf, size); > } > > static void monitor_read(void *opaque, const uint8_t *buf, int size) > @@ -4389,7 +4407,7 @@ void monitor_resume(Monitor *mon) > trace_monitor_suspend(mon, -1); > } > > -static QDict *qmp_greeting(Monitor *mon) > +static QDict *qmp_greeting(MonitorQMP *mon) > { > QList *cap_list = qlist_new(); > QObject *ver = NULL; > @@ -4398,7 +4416,7 @@ static QDict *qmp_greeting(Monitor *mon) > qmp_marshal_query_version(NULL, &ver, NULL); > > for (cap = 0; cap < QMP_CAPABILITY__MAX; cap++) { > - if (mon->qmp.capab_offered[cap]) { > + if (mon->capab_offered[cap]) { > qlist_append_str(cap_list, QMPCapability_str(cap)); > } > } > @@ -4411,11 +4429,11 @@ static QDict *qmp_greeting(Monitor *mon) > static void monitor_qmp_event(void *opaque, int event) > { > QDict *data; > - Monitor *mon = opaque; > + MonitorQMP *mon = opaque; > > switch (event) { > case CHR_EVENT_OPENED: > - mon->qmp.commands = &qmp_cap_negotiation_commands; > + mon->commands = &qmp_cap_negotiation_commands; > monitor_qmp_caps_reset(mon); > data = qmp_greeting(mon); > qmp_send_response(mon, data); > @@ -4430,8 +4448,8 @@ static void monitor_qmp_event(void *opaque, int event) > * is closed. > */ > monitor_qmp_cleanup_queues(mon); > - json_message_parser_destroy(&mon->qmp.parser); > - json_message_parser_init(&mon->qmp.parser, handle_qmp_command, > + json_message_parser_destroy(&mon->parser); > + json_message_parser_init(&mon->parser, handle_qmp_command, > mon, NULL); > mon_refcount--; > monitor_fdsets_cleanup(); > @@ -4593,30 +4611,34 @@ static void monitor_list_append(Monitor *mon) > > static void monitor_qmp_setup_handlers_bh(void *opaque) > { > - Monitor *mon = opaque; > + MonitorQMP *mon = opaque; > GMainContext *context; > > - assert(mon->use_io_thread); > + assert(mon->common.use_io_thread); > context = iothread_get_g_main_context(mon_iothread); > assert(context); > - qemu_chr_fe_set_handlers(&mon->chr, monitor_can_read, monitor_qmp_read, > - monitor_qmp_event, NULL, mon, context, true); > - monitor_list_append(mon); > + qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read, > + monitor_qmp_read, monitor_qmp_event, > + NULL, &mon->common, context, true); > + monitor_list_append(&mon->common); > } > > static void monitor_init_qmp(Chardev *chr, int flags) > { > - Monitor *mon = g_malloc(sizeof(*mon)); > + MonitorQMP *mon = g_malloc0(sizeof(*mon)); > > /* Note: we run QMP monitor in I/O thread when @chr supports that */ > - monitor_data_init(mon, flags, false, > + monitor_data_init(&mon->common, flags, false, > qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT)); > > - qemu_chr_fe_init(&mon->chr, chr, &error_abort); > - qemu_chr_fe_set_echo(&mon->chr, true); > + qemu_mutex_init(&mon->qmp_queue_lock); > + mon->qmp_requests = g_queue_new(); > > - json_message_parser_init(&mon->qmp.parser, handle_qmp_command, mon, NULL); > - if (mon->use_io_thread) { > + qemu_chr_fe_init(&mon->common.chr, chr, &error_abort); > + qemu_chr_fe_set_echo(&mon->common.chr, true); > + > + json_message_parser_init(&mon->parser, handle_qmp_command, mon, NULL); > + if (mon->common.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. > @@ -4631,10 +4653,10 @@ static void monitor_init_qmp(Chardev *chr, int flags) > monitor_qmp_setup_handlers_bh, mon); > /* The bottom half will add @mon to @mon_list */ > } else { > - qemu_chr_fe_set_handlers(&mon->chr, monitor_can_read, > + qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read, > monitor_qmp_read, monitor_qmp_event, > - NULL, mon, NULL, true); > - monitor_list_append(mon); > + NULL, &mon->common, NULL, true); > + monitor_list_append(&mon->common); > } > } > > -- > 2.20.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/monitor.c b/monitor.c index bb23cc0450..d18cf18393 100644 --- a/monitor.c +++ b/monitor.c @@ -166,26 +166,6 @@ struct MonFdset { QLIST_ENTRY(MonFdset) next; }; -typedef struct { - JSONMessageParser parser; - /* - * When a client connects, we're in capabilities negotiation mode. - * @commands is &qmp_cap_negotiation_commands then. When command - * qmp_capabilities succeeds, we go into command mode, and - * @command becomes &qmp_commands. - */ - QmpCommandList *commands; - bool capab_offered[QMP_CAPABILITY__MAX]; /* capabilities offered */ - bool capab[QMP_CAPABILITY__MAX]; /* offered and accepted */ - /* - * Protects qmp request/response queue. - * Take monitor_lock first when you need both. - */ - QemuMutex qmp_queue_lock; - /* Input queue that holds all the parsed QMP requests */ - GQueue *qmp_requests; -} MonitorQMP; - /* * To prevent flooding clients, events can be throttled. The * throttling is calculated globally, rather than per-Monitor @@ -218,7 +198,6 @@ struct Monitor { */ ReadLineState *rs; - MonitorQMP qmp; gchar *mon_cpu_path; mon_cmd_t *cmd_table; QTAILQ_ENTRY(Monitor) entry; @@ -239,6 +218,27 @@ struct Monitor { int mux_out; }; +typedef struct { + Monitor common; + JSONMessageParser parser; + /* + * When a client connects, we're in capabilities negotiation mode. + * @commands is &qmp_cap_negotiation_commands then. When command + * qmp_capabilities succeeds, we go into command mode, and + * @command becomes &qmp_commands. + */ + QmpCommandList *commands; + bool capab_offered[QMP_CAPABILITY__MAX]; /* capabilities offered */ + bool capab[QMP_CAPABILITY__MAX]; /* offered and accepted */ + /* + * Protects qmp request/response queue. + * Take monitor_lock first when you need both. + */ + QemuMutex qmp_queue_lock; + /* Input queue that holds all the parsed QMP requests */ + GQueue *qmp_requests; +} MonitorQMP; + /* Shared monitor I/O thread */ IOThread *mon_iothread; @@ -247,7 +247,7 @@ QEMUBH *qmp_dispatcher_bh; struct QMPRequest { /* Owner of the request */ - Monitor *mon; + MonitorQMP *mon; /* * Request object to be handled or Error to be reported * (exactly one of them is non-null) @@ -355,18 +355,18 @@ static void qmp_request_free(QMPRequest *req) } /* Caller must hold mon->qmp.qmp_queue_lock */ -static void monitor_qmp_cleanup_req_queue_locked(Monitor *mon) +static void monitor_qmp_cleanup_req_queue_locked(MonitorQMP *mon) { - while (!g_queue_is_empty(mon->qmp.qmp_requests)) { - qmp_request_free(g_queue_pop_head(mon->qmp.qmp_requests)); + while (!g_queue_is_empty(mon->qmp_requests)) { + qmp_request_free(g_queue_pop_head(mon->qmp_requests)); } } -static void monitor_qmp_cleanup_queues(Monitor *mon) +static void monitor_qmp_cleanup_queues(MonitorQMP *mon) { - qemu_mutex_lock(&mon->qmp.qmp_queue_lock); + qemu_mutex_lock(&mon->qmp_queue_lock); monitor_qmp_cleanup_req_queue_locked(mon); - qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); + qemu_mutex_unlock(&mon->qmp_queue_lock); } @@ -478,17 +478,17 @@ int monitor_printf(Monitor *mon, const char *fmt, ...) return ret; } -static void qmp_send_response(Monitor *mon, const QDict *rsp) +static void qmp_send_response(MonitorQMP *mon, const QDict *rsp) { const QObject *data = QOBJECT(rsp); QString *json; - json = mon->flags & MONITOR_USE_PRETTY ? qobject_to_json_pretty(data) : - qobject_to_json(data); + json = mon->common.flags & MONITOR_USE_PRETTY ? + qobject_to_json_pretty(data) : qobject_to_json(data); assert(json != NULL); qstring_append_chr(json, '\n'); - monitor_puts(mon, qstring_get_str(json)); + monitor_puts(&mon->common, qstring_get_str(json)); qobject_unref(json); } @@ -511,12 +511,17 @@ static MonitorQAPIEventConf monitor_qapi_event_conf[QAPI_EVENT__MAX] = { 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) - && mon->qmp.commands != &qmp_cap_negotiation_commands) { - qmp_send_response(mon, qdict); + if (!monitor_is_qmp(mon)) { + continue; + } + + qmp_mon = container_of(mon, MonitorQMP, common); + if (qmp_mon->commands != &qmp_cap_negotiation_commands) { + qmp_send_response(qmp_mon, qdict); } } } @@ -710,29 +715,33 @@ static void monitor_data_init(Monitor *mon, int flags, bool skip_flush, } memset(mon, 0, sizeof(Monitor)); qemu_mutex_init(&mon->mon_lock); - qemu_mutex_init(&mon->qmp.qmp_queue_lock); mon->outbuf = qstring_new(); /* Use *mon_cmds by default. */ mon->cmd_table = mon_cmds; mon->skip_flush = skip_flush; mon->use_io_thread = use_io_thread; - mon->qmp.qmp_requests = g_queue_new(); mon->flags = flags; } +static 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_data_destroy(Monitor *mon) { g_free(mon->mon_cpu_path); qemu_chr_fe_deinit(&mon->chr, false); if (monitor_is_qmp(mon)) { - json_message_parser_destroy(&mon->qmp.parser); + MonitorQMP *qmp_mon = container_of(mon, MonitorQMP, common); + monitor_data_destroy_qmp(qmp_mon); } readline_free(mon->rs); qobject_unref(mon->outbuf); qemu_mutex_destroy(&mon->mon_lock); - qemu_mutex_destroy(&mon->qmp.qmp_queue_lock); - monitor_qmp_cleanup_req_queue_locked(mon); - g_queue_free(mon->qmp.qmp_requests); } char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index, @@ -1085,8 +1094,9 @@ static void query_commands_cb(QmpCommand *cmd, void *opaque) CommandInfoList *qmp_query_commands(Error **errp) { CommandInfoList *list = NULL; + MonitorQMP *mon = container_of(cur_mon, MonitorQMP, common); - qmp_for_each_command(cur_mon->qmp.commands, query_commands_cb, &list); + qmp_for_each_command(mon->commands, query_commands_cb, &list); return list; } @@ -1153,16 +1163,16 @@ static void monitor_init_qmp_commands(void) qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG); } -static bool qmp_oob_enabled(Monitor *mon) +static bool qmp_oob_enabled(MonitorQMP *mon) { - return mon->qmp.capab[QMP_CAPABILITY_OOB]; + return mon->capab[QMP_CAPABILITY_OOB]; } -static void monitor_qmp_caps_reset(Monitor *mon) +static void monitor_qmp_caps_reset(MonitorQMP *mon) { - memset(mon->qmp.capab_offered, 0, sizeof(mon->qmp.capab_offered)); - memset(mon->qmp.capab, 0, sizeof(mon->qmp.capab)); - mon->qmp.capab_offered[QMP_CAPABILITY_OOB] = mon->use_io_thread; + 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; } /* @@ -1170,7 +1180,7 @@ static void monitor_qmp_caps_reset(Monitor *mon) * On success, set mon->qmp.capab[], and return true. * On error, set @errp, and return false. */ -static bool qmp_caps_accept(Monitor *mon, QMPCapabilityList *list, +static bool qmp_caps_accept(MonitorQMP *mon, QMPCapabilityList *list, Error **errp) { GString *unavailable = NULL; @@ -1179,7 +1189,7 @@ static bool qmp_caps_accept(Monitor *mon, QMPCapabilityList *list, memset(capab, 0, sizeof(capab)); for (; list; list = list->next) { - if (!mon->qmp.capab_offered[list->value]) { + if (!mon->capab_offered[list->value]) { if (!unavailable) { unavailable = g_string_new(QMPCapability_str(list->value)); } else { @@ -1196,25 +1206,27 @@ static bool qmp_caps_accept(Monitor *mon, QMPCapabilityList *list, return false; } - memcpy(mon->qmp.capab, capab, sizeof(capab)); + memcpy(mon->capab, capab, sizeof(capab)); return true; } void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable, Error **errp) { - if (cur_mon->qmp.commands == &qmp_commands) { + MonitorQMP *mon = container_of(cur_mon, MonitorQMP, common); + + if (mon->commands == &qmp_commands) { error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND, "Capabilities negotiation is already complete, command " "ignored"); return; } - if (!qmp_caps_accept(cur_mon, enable, errp)) { + if (!qmp_caps_accept(mon, enable, errp)) { return; } - cur_mon->qmp.commands = &qmp_commands; + mon->commands = &qmp_commands; } /* Set the current CPU defined by the user. Callers must hold BQL. */ @@ -4121,27 +4133,27 @@ static int monitor_can_read(void *opaque) * Null @rsp can only happen for commands with QCO_NO_SUCCESS_RESP. * Nothing is emitted then. */ -static void monitor_qmp_respond(Monitor *mon, QDict *rsp) +static void monitor_qmp_respond(MonitorQMP *mon, QDict *rsp) { if (rsp) { qmp_send_response(mon, rsp); } } -static void monitor_qmp_dispatch(Monitor *mon, QObject *req) +static void monitor_qmp_dispatch(MonitorQMP *mon, QObject *req) { Monitor *old_mon; QDict *rsp; QDict *error; old_mon = cur_mon; - cur_mon = mon; + cur_mon = &mon->common; - rsp = qmp_dispatch(mon->qmp.commands, req, qmp_oob_enabled(mon)); + rsp = qmp_dispatch(mon->commands, req, qmp_oob_enabled(mon)); cur_mon = old_mon; - if (mon->qmp.commands == &qmp_cap_negotiation_commands) { + if (mon->commands == &qmp_cap_negotiation_commands) { error = qdict_get_qdict(rsp, "error"); if (error && !g_strcmp0(qdict_get_try_str(error, "class"), @@ -4166,24 +4178,30 @@ static void monitor_qmp_dispatch(Monitor *mon, QObject *req) * monitor to the end of mon_list queue. * * Note: if the function returned with non-NULL, then the caller will - * be with mon->qmp.qmp_queue_lock held, and the caller is responsible + * be with qmp_mon->qmp_queue_lock held, and the caller is responsible * to release it. */ static QMPRequest *monitor_qmp_requests_pop_any_with_lock(void) { QMPRequest *req_obj = NULL; Monitor *mon; + MonitorQMP *qmp_mon; qemu_mutex_lock(&monitor_lock); QTAILQ_FOREACH(mon, &mon_list, entry) { - qemu_mutex_lock(&mon->qmp.qmp_queue_lock); - req_obj = g_queue_pop_head(mon->qmp.qmp_requests); + if (!monitor_is_qmp(mon)) { + continue; + } + + qmp_mon = container_of(mon, MonitorQMP, common); + qemu_mutex_lock(&qmp_mon->qmp_queue_lock); + req_obj = g_queue_pop_head(qmp_mon->qmp_requests); if (req_obj) { /* With the lock of corresponding queue held */ break; } - qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); + qemu_mutex_unlock(&qmp_mon->qmp_queue_lock); } if (req_obj) { @@ -4205,7 +4223,7 @@ static void monitor_qmp_bh_dispatcher(void *data) QMPRequest *req_obj = monitor_qmp_requests_pop_any_with_lock(); QDict *rsp; bool need_resume; - Monitor *mon; + MonitorQMP *mon; if (!req_obj) { return; @@ -4214,8 +4232,8 @@ static void monitor_qmp_bh_dispatcher(void *data) mon = req_obj->mon; /* qmp_oob_enabled() might change after "qmp_capabilities" */ need_resume = !qmp_oob_enabled(mon) || - mon->qmp.qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1; - qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); + mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1; + qemu_mutex_unlock(&mon->qmp_queue_lock); if (req_obj->req) { QDict *qdict = qobject_to(QDict, req_obj->req); QObject *id = qdict ? qdict_get(qdict, "id") : NULL; @@ -4231,7 +4249,7 @@ static void monitor_qmp_bh_dispatcher(void *data) if (need_resume) { /* Pairs with the monitor_suspend() in handle_qmp_command() */ - monitor_resume(mon); + monitor_resume(&mon->common); } qmp_request_free(req_obj); @@ -4241,7 +4259,7 @@ static void monitor_qmp_bh_dispatcher(void *data) static void handle_qmp_command(void *opaque, QObject *req, Error *err) { - Monitor *mon = opaque; + MonitorQMP *mon = opaque; QObject *id = NULL; QDict *qdict; QMPRequest *req_obj; @@ -4273,7 +4291,7 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) req_obj->err = err; /* Protect qmp_requests and fetching its length. */ - qemu_mutex_lock(&mon->qmp.qmp_queue_lock); + qemu_mutex_lock(&mon->qmp_queue_lock); /* * Suspend the monitor when we can't queue more requests after @@ -4282,8 +4300,8 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) * command, for backward compatibility. */ if (!qmp_oob_enabled(mon) || - mon->qmp.qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) { - monitor_suspend(mon); + mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1) { + monitor_suspend(&mon->common); } /* @@ -4291,9 +4309,9 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) * handled in time order. Ownership for req_obj, req, * etc. will be delivered to the handler side. */ - assert(mon->qmp.qmp_requests->length < QMP_REQ_QUEUE_LEN_MAX); - g_queue_push_tail(mon->qmp.qmp_requests, req_obj); - qemu_mutex_unlock(&mon->qmp.qmp_queue_lock); + assert(mon->qmp_requests->length < QMP_REQ_QUEUE_LEN_MAX); + g_queue_push_tail(mon->qmp_requests, req_obj); + qemu_mutex_unlock(&mon->qmp_queue_lock); /* Kick the dispatcher routine */ qemu_bh_schedule(qmp_dispatcher_bh); @@ -4301,9 +4319,9 @@ static void handle_qmp_command(void *opaque, QObject *req, Error *err) static void monitor_qmp_read(void *opaque, const uint8_t *buf, int size) { - Monitor *mon = opaque; + MonitorQMP *mon = opaque; - json_message_parser_feed(&mon->qmp.parser, (const char *) buf, size); + json_message_parser_feed(&mon->parser, (const char *) buf, size); } static void monitor_read(void *opaque, const uint8_t *buf, int size) @@ -4389,7 +4407,7 @@ void monitor_resume(Monitor *mon) trace_monitor_suspend(mon, -1); } -static QDict *qmp_greeting(Monitor *mon) +static QDict *qmp_greeting(MonitorQMP *mon) { QList *cap_list = qlist_new(); QObject *ver = NULL; @@ -4398,7 +4416,7 @@ static QDict *qmp_greeting(Monitor *mon) qmp_marshal_query_version(NULL, &ver, NULL); for (cap = 0; cap < QMP_CAPABILITY__MAX; cap++) { - if (mon->qmp.capab_offered[cap]) { + if (mon->capab_offered[cap]) { qlist_append_str(cap_list, QMPCapability_str(cap)); } } @@ -4411,11 +4429,11 @@ static QDict *qmp_greeting(Monitor *mon) static void monitor_qmp_event(void *opaque, int event) { QDict *data; - Monitor *mon = opaque; + MonitorQMP *mon = opaque; switch (event) { case CHR_EVENT_OPENED: - mon->qmp.commands = &qmp_cap_negotiation_commands; + mon->commands = &qmp_cap_negotiation_commands; monitor_qmp_caps_reset(mon); data = qmp_greeting(mon); qmp_send_response(mon, data); @@ -4430,8 +4448,8 @@ static void monitor_qmp_event(void *opaque, int event) * is closed. */ monitor_qmp_cleanup_queues(mon); - json_message_parser_destroy(&mon->qmp.parser); - json_message_parser_init(&mon->qmp.parser, handle_qmp_command, + json_message_parser_destroy(&mon->parser); + json_message_parser_init(&mon->parser, handle_qmp_command, mon, NULL); mon_refcount--; monitor_fdsets_cleanup(); @@ -4593,30 +4611,34 @@ static void monitor_list_append(Monitor *mon) static void monitor_qmp_setup_handlers_bh(void *opaque) { - Monitor *mon = opaque; + MonitorQMP *mon = opaque; GMainContext *context; - assert(mon->use_io_thread); + assert(mon->common.use_io_thread); context = iothread_get_g_main_context(mon_iothread); assert(context); - qemu_chr_fe_set_handlers(&mon->chr, monitor_can_read, monitor_qmp_read, - monitor_qmp_event, NULL, mon, context, true); - monitor_list_append(mon); + qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read, + monitor_qmp_read, monitor_qmp_event, + NULL, &mon->common, context, true); + monitor_list_append(&mon->common); } static void monitor_init_qmp(Chardev *chr, int flags) { - Monitor *mon = g_malloc(sizeof(*mon)); + MonitorQMP *mon = g_malloc0(sizeof(*mon)); /* Note: we run QMP monitor in I/O thread when @chr supports that */ - monitor_data_init(mon, flags, false, + monitor_data_init(&mon->common, flags, false, qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT)); - qemu_chr_fe_init(&mon->chr, chr, &error_abort); - qemu_chr_fe_set_echo(&mon->chr, true); + qemu_mutex_init(&mon->qmp_queue_lock); + mon->qmp_requests = g_queue_new(); - json_message_parser_init(&mon->qmp.parser, handle_qmp_command, mon, NULL); - if (mon->use_io_thread) { + qemu_chr_fe_init(&mon->common.chr, chr, &error_abort); + qemu_chr_fe_set_echo(&mon->common.chr, true); + + json_message_parser_init(&mon->parser, handle_qmp_command, mon, NULL); + if (mon->common.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. @@ -4631,10 +4653,10 @@ static void monitor_init_qmp(Chardev *chr, int flags) monitor_qmp_setup_handlers_bh, mon); /* The bottom half will add @mon to @mon_list */ } else { - qemu_chr_fe_set_handlers(&mon->chr, monitor_can_read, + qemu_chr_fe_set_handlers(&mon->common.chr, monitor_can_read, monitor_qmp_read, monitor_qmp_event, - NULL, mon, NULL, true); - monitor_list_append(mon); + NULL, &mon->common, NULL, true); + monitor_list_append(&mon->common); } }
Currently, struct Monitor mixes state that is only relevant for HMP, state that is only relevant for QMP, and some actually shared state. In particular, a MonitorQMP field is present in the state of any monitor, even if it's not a QMP monitor and therefore doesn't use the state. As a first step towards a clean separation between QMP and HMP, let MonitorQMP extend Monitor and create a MonitorQMP object only when the monitor is actually a QMP monitor. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- monitor.c | 214 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 118 insertions(+), 96 deletions(-)