Re: [PATCH 7/7] qdev-monitor: drain RCU callbacks in qdev_device_add_from_qdict

[Akihiko Odaki]

On 2026/07/25 23:00, Marc-André Lureau wrote:
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
so it always runs, and drop the now-redundant calls in qmp_device_add()
and hmp_device_add().

It is reachable from virtio-net failover during guest feature 
negotiation. MMIO dispatch holds an RCU read lock at 
address_space_write(). If deferred primary creation fails, 
drain_call_rcu() waits for a grace period while its own thread remains a 
reader, deadlocking the VM.

Regards,
Akihiko Odaki


Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
---
  system/qdev-monitor.c | 33 ++++++++++-----------------------
  1 file changed, 10 insertions(+), 23 deletions(-)

diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 00fed791cce1..884d407d60d4 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -746,6 +746,16 @@ err_del_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 a caller might expect
+     * the failed device to be fully torn down once this function
+     * returns.
+     */
+    drain_call_rcu();
+
      return NULL;
  }
  
@@ -870,18 +880,6 @@ 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();
-    }
      object_unref(OBJECT(dev));
  }
  
@@ -1017,17 +1015,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);