[PATCH v2 12/12] qdev-monitor: drain RCU callbacks in qdev_device_add_from_qdict

["Marc-André Lureau" <marcandre.lureau@redhat.com>]

qmp_device_add() and hmp_device_add() drain pending RCU callbacks after
a failed device_add, since some bus teardown (e.g. bus_remove_child())
is deferred to call_rcu(). -device on the command line reaches the same
qdev_device_add_from_qdict() with errp pointing at error_fatal, whose
ERRP_GUARD() exits before control returns to the caller, so the caller's
drain_call_rcu() never runs.

Move the drain into qdev_device_add_from_qdict()'s own err_del_dev path
and drop the now-redundant calls in qmp_device_add() and
hmp_device_add(). Gate it behind a new drain_rcu parameter, because
virtio-net failover can reach qdev_device_add_from_qdict() from an MMIO
write dispatched under address_space_write()'s RCU read lock.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/monitor/qdev.h |  3 ++-
 hw/audio/intel-hda.c   |  2 +-
 hw/net/virtio-net.c    |  2 +-
 system/qdev-monitor.c  | 42 ++++++++++++++++--------------------------
 4 files changed, 20 insertions(+), 29 deletions(-)

diff --git a/include/monitor/qdev.h b/include/monitor/qdev.h
index f85f25738d58..72d2fa4654d3 100644
--- a/include/monitor/qdev.h
+++ b/include/monitor/qdev.h
@@ -12,7 +12,8 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp);
 int qdev_device_help(QemuOpts *opts);
 DeviceState *qdev_device_add(QemuOpts *opts, Error **errp);
 DeviceState *qdev_device_add_from_qdict(const QDict *opts,
-                                        bool from_json, Error **errp);
+                                        bool from_json, bool drain_rcu,
+                                        Error **errp);
 BusState *qdev_find_default_bus(DeviceClass *dc, Error **errp);
 
 /**
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 3d361a4976c6..128c5ee612ce 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -1313,7 +1313,7 @@ static void intel_hda_and_codec_init(const char *audiodev)
     BusState *hdabus;
 
     qdict_put_str(props, "driver", "intel-hda");
-    intel_hda = qdev_device_add_from_qdict(props, false, &error_fatal);
+    intel_hda = qdev_device_add_from_qdict(props, false, true, &error_fatal);
     hdabus = QLIST_FIRST(&intel_hda->child_bus);
 
     codec = qdev_new("hda-duplex");
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index f0e3beb29032..0c5da8cb71af 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -916,7 +916,7 @@ static void failover_add_primary(VirtIONet *n, Error **errp)
 
     dev = qdev_device_add_from_qdict(n->primary_opts,
                                      n->primary_opts_from_json,
-                                     &err);
+                                     false, &err);
     if (err) {
         qobject_unref(n->primary_opts);
         n->primary_opts = NULL;
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 00fed791cce1..77508bb1c3f1 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -650,7 +650,8 @@ BusState *qdev_find_default_bus(DeviceClass *dc, Error **errp)
 }
 
 DeviceState *qdev_device_add_from_qdict(const QDict *opts,
-                                        bool from_json, Error **errp)
+                                        bool from_json, bool drain_rcu,
+                                        Error **errp)
 {
     ERRP_GUARD();
     DeviceClass *dc;
@@ -746,6 +747,18 @@ err_del_dev:
     object_unparent(OBJECT(dev));
     object_unref(OBJECT(dev));
 
+    /*
+     * Some bus teardown (e.g. bus_remove_child()) is deferred via
+     * call_rcu(). When safe, drain those callbacks so the failed
+     * device is fully torn down before we return. Callers that are
+     * already inside an RCU read-side critical section (e.g. MMIO
+     * dispatch during virtio-net failover) must pass drain_rcu=false
+     * to avoid deadlocking on the grace period.
+     */
+    if (drain_rcu) {
+        drain_call_rcu();
+    }
+
     return NULL;
 }
 
@@ -755,7 +768,7 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
     QDict *qdict = qemu_opts_to_qdict(opts, NULL);
     DeviceState *ret;
 
-    ret = qdev_device_add_from_qdict(qdict, false, errp);
+    ret = qdev_device_add_from_qdict(qdict, false, true, errp);
     if (ret) {
         qemu_opts_del(opts);
     }
@@ -869,19 +882,7 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
 {
     DeviceState *dev;
 
-    dev = qdev_device_add_from_qdict(qdict, true, 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();
-    }
+    dev = qdev_device_add_from_qdict(qdict, true, true, errp);
     object_unref(OBJECT(dev));
 }
 
@@ -1017,17 +1018,6 @@ void hmp_device_add(Monitor *mon, const QDict *qdict)
     }
     dev = qdev_device_add(opts, &err);
     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);
     }
     object_unref(dev);

-- 
2.55.0