@samitouri / QOSamiQemu / commits / c38575a126

hw/qdev: Parent device before setting parent bus

Commit 9940b2cfbc05 ("qdev: New qdev_new(), qdev_realize(), etc.") says "device state 'no QOM parent, but plugged into bus' is dangerous". In such a case, unrealizing the bus will hang in bus_unparent(): while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) { DeviceState *dev = kid->child; object_unparent(OBJECT(dev)); } object_unparent() does nothing when its argument has no QOM parent, and the loop spins forever. However, that commit did not completely eliminate such a situation. When the device is not parented, device_set_realized() lets /machine/unattached parent it, but it happens after setting parent bus. Therefore, any failure between the two operations can leave the device in a dangerous state. qdev_realize() at least asserts that the device is not already realized and prevents one realization failure pattern, but it is not comprehensive. Besides, it will trip with a command line like the following: qemu-system-x86_64 -M none -nodefaults -nographic \ -device ipmi-bmc-sim,realized=on Eliminate the dangerous state by ensuring that the device is parented before calling qdev_set_parent_bus(). Also, stop asserting that the device is not already realized in qdev_realize(); it is broken and no longer serves any purpose. Fixes: 9940b2cfbc05 ("qdev: New qdev_new(), qdev_realize(), etc.") Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> Message-ID: <20260721-qdev-v3-14-d2e226fa002e@rsg.ci.i.u-tokyo.ac.jp>

Akihiko Odaki committed Jul 21, 2026 at 17:17 UTC c38575a1265cb219230e75ede8e1750b6a269709
2 files changed +42 -22
hw/core/qdev.c
+29 -22
@@ -264,17 +264,43 @@ static void device_reset_child_foreach(Object *obj, ResettableChildCallback cb,
264
265 bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp)
266 {
267 - assert(!dev->realized && !dev->parent_bus);
267 + static int unattached_count;
268 + bool unattached_parent = false;
269 +
270 + assert(!dev->parent_bus);
271 +
272 + if (!OBJECT(dev)->parent) {
273 + gchar *name = g_strdup_printf("device[%d]", unattached_count++);
274 +
275 + object_property_add_child(machine_get_container("unattached"),
276 + name, OBJECT(dev));
277 + unattached_parent = true;
278 + g_free(name);
279 + }
280
281 if (bus) {
282 if (!qdev_set_parent_bus(dev, bus, errp)) {
271 - return false;
283 + goto fail;
284 }
285 } else {
286 assert(!DEVICE_GET_CLASS(dev)->bus_type);
287 }
288
277 - return object_property_set_bool(OBJECT(dev), "realized", true, errp);
289 + if (object_property_set_bool(OBJECT(dev), "realized", true, errp)) {
290 + return true;
291 + }
292 +
293 +fail:
294 + if (unattached_parent) {
295 + /*
296 + * Beware, this doesn't just revert
297 + * object_property_add_child(), it also runs bus_remove()!
298 + */
299 + object_unparent(OBJECT(dev));
300 + unattached_count--;
301 + }
302 +
303 + return false;
304 }
305
306 bool qdev_realize_and_unref(DeviceState *dev, BusState *bus, Error **errp)
@@ -479,8 +505,6 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
505 BusState *bus;
506 NamedClockList *ncl;
507 Error *local_err = NULL;
482 - bool unattached_parent = false;
483 - static int unattached_count;
508
509 if (dev->hotplugged && !dc->hotpluggable) {
510 error_setg(errp, "Device '%s' does not support hotplugging",
@@ -493,15 +517,6 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
517 goto fail;
518 }
519
496 - if (!obj->parent) {
497 - gchar *name = g_strdup_printf("device[%d]", unattached_count++);
498 -
499 - object_property_add_child(machine_get_container("unattached"),
500 - name, obj);
501 - unattached_parent = true;
502 - g_free(name);
503 - }
504 -
520 hotplug_ctrl = qdev_get_hotplug_handler(dev);
521 if (hotplug_ctrl) {
522 hotplug_handler_pre_plug(hotplug_ctrl, dev, &local_err);
@@ -627,14 +642,6 @@ post_realize_fail:
642
643 fail:
644 error_propagate(errp, local_err);
630 - if (unattached_parent) {
631 - /*
632 - * Beware, this doesn't just revert
633 - * object_property_add_child(), it also runs bus_remove()!
634 - */
635 - object_unparent(OBJECT(dev));
636 - unattached_count--;
637 - }
645 }
646
647 static bool device_get_hotpluggable(Object *obj, Error **errp)
tests/unit/test-qdev.c
+13
@@ -78,6 +78,16 @@ static void test_qdev_free_properties(void)
78 object_unref(mt);
79 }
80
81 +static void test_qdev_double_realization(void)
82 +{
83 + MyDev *mt = STATIC_TYPE(object_new(TYPE_MY_DEV));
84 +
85 + qdev_realize(DEVICE(mt), NULL, &error_fatal);
86 + qdev_realize(DEVICE(mt), NULL, &error_fatal);
87 + object_unparent(OBJECT(mt));
88 + object_unref(OBJECT(mt));
89 +}
90 +
91
92 int main(int argc, char **argv)
93 {
@@ -90,6 +100,9 @@ int main(int argc, char **argv)
100 g_test_add_func("/qdev/free-properties",
101 test_qdev_free_properties);
102
103 + g_test_add_func("/qdev/double-realization",
104 + test_qdev_double_realization);
105 +
106 g_test_run();
107
108 return 0;