I stumbled over this while working on a feature branch. Instead of
throwing the whole branch at you as a reproducer, I give you a mock up.
This is fdctrl_connect_drives():
dev = qdev_create(&fdctrl->bus.bus, "floppy");
qdev_prop_set_uint32(dev, "unit", i);
qdev_prop_set_enum(dev, "drive-type", fdctrl->qdev_for_drives[i].type);
blk_ref(blk);
blk_detach_dev(blk, fdc_dev);
fdctrl->qdev_for_drives[i].blk = NULL;
qdev_prop_set_drive(dev, "drive", blk, &local_err);
blk_unref(blk);
if (local_err) {
error_propagate(errp, local_err);
return;
}
object_property_set_bool(OBJECT(dev), true, "realized", &local_err);
if (local_err) {
error_propagate(errp, local_err);
return;
}
What if qdev_prop_set_drive() fails? I don't have a reproducer ready (I
do on my feature branch), so let's mock it, and also instrument the
place where things go wrong. Patch appended. To try it, run
qemu-system-x86_64 without arguments.
Turns out the failure bubbles up into device_set_realized() for the
isa-fdc, where the cleanup code calls object_unparent(). This unparents
children, and ends up in bus_unparent() for the isa-fdc's floppy-bus:
#4 0x0000555555abdb7f in bus_unparent (obj=0x55555675a9f0)
at /work/armbru/qemu/hw/core/bus.c:148
#5 0x0000555555d2aea6 in object_finalize_child_property
(obj=0x55555675a800, name=0x555557281230 "floppy-bus.0",
opaque=0x55555675a9f0) at /work/armbru/qemu/qom/object.c:1672
#6 0x0000555555d2872b in object_property_del_child
(obj=0x55555675a800, child=0x55555675a9f0, errp=0x0)
at /work/armbru/qemu/qom/object.c:628
#7 0x0000555555d2880b in object_unparent (obj=0x55555675a9f0)
at /work/armbru/qemu/qom/object.c:647
#8 0x0000555555ab9e10 in device_unparent (obj=0x55555675a800)
at /work/armbru/qemu/hw/core/qdev.c:1101
This loop there
while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
DeviceState *dev = kid->child;
object_unparent(OBJECT(dev));
}
makes no progreess because OBJECT(dev)->parent is still null, and
therefore object_unparent() does nothing.
Possible culprit: qdev_try_create() calls qdev_set_parent_bus(), which
adds the device to the bus, but leaves ->parent null. If this isn't
wrong outright, it's at least a dangerous state.
Work-around: call qdev_set_id(dev, NULL) right after qdev_create().
This sets ->parent.
>From 2554db096866138a85482d683e57a38166bb425b Mon Sep 17 00:00:00 2001
From: Markus Armbruster <arm...@redhat.com>
Date: Mon, 4 May 2020 15:58:10 +0200
Subject: [PATCH] qdev: Hack to reproduce infinite loop in bus_unparent()
---
hw/block/fdc.c | 4 ++++
hw/core/bus.c | 5 +++++
2 files changed, 9 insertions(+)
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 9628cc171e..f57558eea4 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2523,7 +2523,11 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl,
DeviceState *fdc_dev,
blk_ref(blk);
blk_detach_dev(blk, fdc_dev);
fdctrl->qdev_for_drives[i].blk = NULL;
+#if 0
qdev_prop_set_drive(dev, "drive", blk, &local_err);
+#else
+ error_setg(&local_err, "hack");
+#endif
blk_unref(blk);
if (local_err) {
diff --git a/hw/core/bus.c b/hw/core/bus.c
index 3dc0a825f0..3620a7be54 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -135,12 +135,17 @@ static void bus_unparent(Object *obj)
BusState *bus = BUS(obj);
BusChild *kid;
+ printf("### %s bus=%p %s\n",
+ __func__, obj, object_get_typename(obj));
/* Only the main system bus has no parent, and that bus is never freed */
assert(bus->parent);
while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
DeviceState *dev = kid->child;
+ printf("### %s kid=%p %s\n",
+ __func__, OBJECT(dev), object_get_typename(OBJECT(dev)));
object_unparent(OBJECT(dev));
+ assert(kid != QTAILQ_FIRST(&bus->children));
}
QLIST_REMOVE(bus, sibling);
bus->parent->num_child_bus--;
--
2.21.1
