The QMP device_add monitor command converts the QDict arguments to
QemuOpts and then back again to QDict. This process only supports scalar
types. Device properties like virtio-blk-pci's iothread-vq-mapping (an
array of objects) are silently dropped by qemu_opts_from_qdict() during
the QemuOpts conversion even though QAPI is capable of validating them.
As a result, hotplugging virtio-blk-pci devices with the
iothread-vq-mapping property does not work as expected (the property is
ignored).
Get rid of the QemuOpts conversion in qmp_device_add() and call
qdev_device_add_from_qdict() with from_json=true. Using the QMP
command's QDict arguments directly allows non-scalar properties.
The HMP is also adjusted since qmp_device_add()'s now expects properly
typed JSON arguments and cannot be used from HMP anymore. Move the code
that was previously in qmp_device_add() (with QemuOpts conversion and
from_json=false) into hmp_device_add() so that its behavior is
unchanged.
This patch changes the behavior of QMP device_add but not HMP
device_add. QMP clients that sent incorrectly typed device_add QMP
commands no longer work. This is a breaking change but clients should be
using the correct types already. See the netdev_add QAPIfication in
commit db2a380c8457 for similar reasoning and object-add in commit
9151e59a8b6e. Unlike those commits, we continue to rely on 'gen': false
for the time being.
Move the drain_call_rcu() invocation into qdev_device_add_from_qdict()
so all callers benefit from it automatically. This avoids code
duplication.
Markus helped me figure this out and even provided a draft patch. The
code ended up very close to what he suggested.
Suggested-by: Markus Armbruster <arm...@redhat.com>
Cc: Daniel P. Berrangé <berra...@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
---
system/qdev-monitor.c | 56 ++++++++++++++++++++++---------------------
1 file changed, 29 insertions(+), 27 deletions(-)
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 6af6ef7d66..8a756b1a91 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -725,6 +725,17 @@ err_del_dev:
if (dev) {
object_unparent(OBJECT(dev));
object_unref(OBJECT(dev));
+
+ /*
+ * Drain all pending RCU callbacks. This is done because
+ * some bus related operations can delay a device removal
+ * (in this case this can happen if device is added and then
+ * removed due to a configuration error)
+ * to a RCU callback, but user might expect that this interface
+ * will finish its job completely once qmp command returns result
+ * to the user
+ */
+ drain_call_rcu();
}
return NULL;
}
@@ -849,34 +860,10 @@ void hmp_info_qdm(Monitor *mon, const QDict *qdict)
void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
{
- QemuOpts *opts;
DeviceState *dev;
- opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, errp);
- if (!opts) {
- return;
- }
- if (!monitor_cur_is_qmp() && qdev_device_help(opts)) {
- qemu_opts_del(opts);
- return;
- }
- dev = qdev_device_add(opts, errp);
- if (!dev) {
- /*
- * Drain all pending RCU callbacks. This is done because
- * some bus related operations can delay a device removal
- * (in this case this can happen if device is added and then
- * removed due to a configuration error)
- * to a RCU callback, but user might expect that this interface
- * will finish its job completely once qmp command returns result
- * to the user
- */
- drain_call_rcu();
-
- qemu_opts_del(opts);
- return;
- }
- object_unref(OBJECT(dev));
+ dev = qdev_device_add_from_qdict(qdict, true, errp);
+ object_unref(dev);
}
static DeviceState *find_device_state(const char *id, Error **errp)
@@ -967,8 +954,23 @@ void qmp_device_del(const char *id, Error **errp)
void hmp_device_add(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
+ QemuOpts *opts;
+ DeviceState *dev;
- qmp_device_add((QDict *)qdict, NULL, &err);
+ opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, &err);
+ if (!opts) {
+ goto out;
+ }
+ if (qdev_device_help(opts)) {
+ qemu_opts_del(opts);
+ return;
+ }
+ dev = qdev_device_add(opts, &err);
+ if (!dev) {
+ qemu_opts_del(opts);
+ }
+ object_unref(dev);
+out:
hmp_handle_error(mon, err);
}
--
2.45.2
