diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h index 5522e05464..23829f32f9 100644 --- a/monitor/monitor-internal.h +++ b/monitor/monitor-internal.h @@ -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 diff --git a/monitor/qmp.c b/monitor/qmp.c index 2bec0f3c96..223e0643c2 100644 --- a/monitor/qmp.c +++ b/monitor/qmp.c @@ -28,6 +28,7 @@ #include "monitor-internal.h" #include "qapi/error.h" #include "qapi/qapi-commands-control.h" +#include "qapi/qapi-commands-char.h" #include "qobject/qdict.h" #include "qobject/qjson.h" #include "qobject/qlist.h" @@ -103,6 +104,20 @@ static void monitor_qmp_set_pretty(Object *obj, bool val, Error **errp) mon->pretty = val; } +static int monitor_qmp_get_close_action(Object *obj, Error **errp) +{ + MonitorQMP *mon = MONITOR_QMP(obj); + + return mon->close_action; +} + +static void monitor_qmp_set_close_action(Object *obj, int val, Error **errp) +{ + MonitorQMP *mon = MONITOR_QMP(obj); + + mon->close_action = val; +} + 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); @@ -117,6 +132,11 @@ 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); + object_class_property_add_enum(cls, "close-action", + "MonitorQMPCloseAction", + &MonitorQMPCloseAction_lookup, + monitor_qmp_get_close_action, + monitor_qmp_set_close_action); moncls->emit_event = monitor_qmp_emit_event; moncls->requires_iothread = monitor_qmp_requires_iothread; @@ -550,11 +570,49 @@ static QDict *qmp_greeting(MonitorQMP *mon) ver, cap_list); } +static void monitor_qmp_self_delete_bh(void *opaque) +{ + MonitorQMP *mon = opaque; + const char *mon_id = object_get_canonical_path_component( + OBJECT(mon)); + g_autofree char *chardev_id = g_strdup(mon->parent_obj.chardev_id); + Error *local_error = NULL; + + if (!mon_id) { + /* + * Another monitor raced & ran 'object-del' on 'mon' + * before this BH got scheduled, so we have a ref on + * mon from monitor_qmp_event but it is already + * unparented. + */ + object_unref(mon); + return; + } + + user_creatable_del(mon_id, &local_error); + /* Pairs with ref from monitor_qmp_event */ + object_unref(mon); + if (local_error != NULL) { + error_report_err(local_error); + } else { + qmp_chardev_remove(chardev_id, NULL); + } +} + static void monitor_qmp_event(void *opaque, QEMUChrEvent event) { QDict *data; MonitorQMP *mon = opaque; + /* + * Protect against race if a client drops & quickly + * reconnects - we'll have the delete BH scheduled + * so must not honour a new open request + */ + if (mon->delete_pending) { + return; + } + switch (event) { case CHR_EVENT_OPENED: WITH_QEMU_LOCK_GUARD(&mon->parent_obj.mon_lock) { @@ -577,6 +635,28 @@ static void monitor_qmp_event(void *opaque, QEMUChrEvent event) json_message_parser_init(&mon->parser, handle_qmp_command, mon, NULL); monitor_fdsets_cleanup(); + switch (mon->close_action) { + case MONITOR_QMP_CLOSE_ACTION_NONE: + break; + case MONITOR_QMP_CLOSE_ACTION_DELETE: + mon->delete_pending = true; + /* + * Do NOT run in the AIO context associated with the + * monitor. We need to run in the default AIO context + * which is the same context in which 'qmp_object_del' + * will execute + * + * Hold an extra ref in case a separate monitor races + * with the BH by processing an explicit 'object-del'. + * Will be released by monitor_qmp_self_delete_bh + */ + object_ref(mon); + aio_bh_schedule_oneshot(qemu_get_aio_context(), + monitor_qmp_self_delete_bh, mon); + break; + default: + g_assert_not_reached(); + } break; case CHR_EVENT_BREAK: case CHR_EVENT_MUX_IN: diff --git a/qapi/qom.json b/qapi/qom.json index 33a827b9fa..d96cc5aa21 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -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: diff --git a/tests/functional/generic/test_monitor_hotplug.py b/tests/functional/generic/test_monitor_hotplug.py index 55af8a238f..86092e781c 100755 --- a/tests/functional/generic/test_monitor_hotplug.py +++ b/tests/functional/generic/test_monitor_hotplug.py @@ -25,7 +25,7 @@ class MonitorHotplug(QemuSystemTest): sock_dir = self.socket_dir() self._sock_path = os.path.join(sock_dir.name, 'hotplug.sock') - def _add_monitor(self): + def _add_monitor(self, autodelete=False): """Create a chardev + monitor and return the socket path.""" sock = self._sock_path self.vm.cmd('chardev-add', id='hotplug-chr', backend={ @@ -39,9 +39,15 @@ class MonitorHotplug(QemuSystemTest): 'wait': False, } }) - self.vm.cmd('object-add', id='hotplug-mon', - qom_type='monitor-qmp', - chardev='hotplug-chr') + if autodelete: + self.vm.cmd('object-add', id='hotplug-mon', + qom_type='monitor-qmp', + chardev='hotplug-chr', + close_action='delete') + else: + self.vm.cmd('object-add', id='hotplug-mon', + qom_type='monitor-qmp', + chardev='hotplug-chr') return sock def _remove_monitor(self): @@ -118,6 +124,75 @@ class MonitorHotplug(QemuSystemTest): # Clean up the chardev self.vm.cmd('chardev-remove', id='hotplug-chr') + def test_auto_delete(self): + """ + A dynamically-added monitor configured with 'close-action=delete' + should see itself deleted when the client is closed. + """ + self.set_machine('none') + self.vm.add_args('-nodefaults') + self.vm.launch() + + sock = self._add_monitor(autodelete=True) + + cdevs = [c["label"] for c in self.vm.cmd('query-chardev')] + objs = [o["name"] for o in self.vm.cmd('qom-list', path='/objects')] + assert ('hotplug-chr' in cdevs) + assert ('hotplug-mon' in objs) + + qmp = QEMUMonitorProtocol(sock) + greeting = qmp.connect(negotiate=True) + self.assertIn('QMP', greeting) + + cdevs = [c["label"] for c in self.vm.cmd('query-chardev')] + objs = [o["name"] for o in self.vm.cmd('qom-list', path='/objects')] + assert ('hotplug-chr' in cdevs) + assert ('hotplug-mon' in objs) + + qmp.close() + + # Wait upto 10 seconds max for chardev to auto-delete, which + # is hopefully enough for reliability under high load + for i in range(int(10 / 0.2)): + cdevs = [c["label"] for c in self.vm.cmd('query-chardev')] + if 'hotplug-chr' not in cdevs: + break + # Wait a little more then try again + time.sleep(0.2) + + cdevs = [c["label"] for c in self.vm.cmd('query-chardev')] + objs = [o["name"] for o in self.vm.cmd('qom-list', path='/objects')] + assert ('hotplug-chr' not in cdevs) + assert ('hotplug-mon' not in objs) + + def test_reconnect(self): + """ + A dynamically-added monitor configured without 'close-action' + should allow reconnects after the client is closed. + """ + self.set_machine('none') + self.vm.add_args('-nodefaults') + self.vm.launch() + + sock = self._add_monitor() + + qmp = QEMUMonitorProtocol(sock) + qmp.connect(negotiate=True) + + resp = qmp.cmd_obj({'execute': 'query-chardev'}) + self.assertIn('return', resp) + + qmp.close() + + qmp = QEMUMonitorProtocol(sock) + qmp.connect(negotiate=True) + + resp = qmp.cmd_obj({'execute': 'query-chardev'}) + self.assertIn('return', resp) + + qmp.close() + self._remove_monitor() + def test_large_response(self): """ Send a command with a large response (query-qmp-schema) on a