[RFC PATCH v2 134/137] qom: Delete /machine/unattached and error on unparented realize

[Alexander Graf <graf@amazon.com>]

With every board-created device, memory region, IRQ, and singleton
now given an explicit QOM parent, the /machine/unattached container
serves no legitimate purpose.  Remove it and turn the
device_set_realized() fallback into a hard error, so that any
future orphan is caught at realize time rather than silently
adopted into an anonymous bucket.

  hw/core/qdev.c   error out on !obj->parent instead of adopting
  hw/core/qdev-user.c   drop the fake unattached container
  system/vl.c      drop "unattached" from machine container list
  hw/i386/pc.c     search /machine recursively for the FDC instead
                   of the (unattached, peripheral, peripheral-anon)
                   container triple
  tests/unit/      parent test devices explicitly under /machine

After this commit, `info qom-tree` no longer shows
/machine/unattached on any target/machine combination, and
`git grep '"unattached"'` returns zero hits.

AI-used-for: code (refactoring)
Signed-off-by: Alexander Graf <graf@amazon.com>
---
 hw/core/qdev-user.c                 |  6 +-----
 hw/core/qdev.c                      | 20 ++++----------------
 hw/i386/pc.c                        | 11 +----------
 include/hw/core/qdev.h              |  2 +-
 system/vl.c                         |  1 -
 tests/unit/test-qdev-global-props.c | 17 +++++++----------
 tests/unit/test-qdev.c              | 12 ++++--------
 7 files changed, 18 insertions(+), 51 deletions(-)

diff --git a/hw/core/qdev-user.c b/hw/core/qdev-user.c
index 051f8fbd02..592a8369a3 100644
--- a/hw/core/qdev-user.c
+++ b/hw/core/qdev-user.c
@@ -11,9 +11,5 @@
 
 void qdev_create_fake_machine(void)
 {
-    Object *fake_machine_obj;
-
-    fake_machine_obj = object_property_add_new_container(object_get_root(),
-                                                         "machine");
-    object_property_add_new_container(fake_machine_obj, "unattached");
+    object_property_add_new_container(object_get_root(), "machine");
 }
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 9a68fb426e..df3ba02839 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -486,8 +486,6 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
     BusState *bus;
     NamedClockList *ncl;
     Error *local_err = NULL;
-    bool unattached_parent = false;
-    static int unattached_count;
 
     if (dev->hotplugged && !dc->hotpluggable) {
         error_setg(errp, "Device '%s' does not support hotplugging",
@@ -501,12 +499,10 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
         }
 
         if (!obj->parent) {
-            gchar *name = g_strdup_printf("device[%d]", unattached_count++);
-
-            object_property_add_child(machine_get_container("unattached"),
-                                      name, obj);
-            unattached_parent = true;
-            g_free(name);
+            error_setg(errp, "device type '%s' has no QOM parent; "
+                       "use qdev_new() with a parent",
+                       object_get_typename(obj));
+            return;
         }
 
         hotplug_ctrl = qdev_get_hotplug_handler(dev);
@@ -634,14 +630,6 @@ post_realize_fail:
 
 fail:
     error_propagate(errp, local_err);
-    if (unattached_parent) {
-        /*
-         * Beware, this doesn't just revert
-         * object_property_add_child(), it also runs bus_remove()!
-         */
-        object_unparent(OBJECT(dev));
-        unattached_count--;
-    }
 }
 
 static bool device_get_hotpluggable(Object *obj, Error **errp)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 398371ff6f..bab8ee453f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -368,24 +368,15 @@ static int check_fdc(Object *obj, void *opaque)
     return 0;
 }
 
-static const char * const fdc_container_path[] = {
-    "unattached", "peripheral", "peripheral-anon"
-};
-
 /*
  * Locate the FDC at IO address 0x3f0, in order to configure the CMOS registers
  * and ACPI objects.
  */
 static ISADevice *pc_find_fdc0(void)
 {
-    int i;
-    Object *container;
     CheckFdcState state = { 0 };
 
-    for (i = 0; i < ARRAY_SIZE(fdc_container_path); i++) {
-        container = machine_get_container(fdc_container_path[i]);
-        object_child_foreach(container, check_fdc, &state);
-    }
+    object_child_foreach_recursive(qdev_get_machine(), check_fdc, &state);
 
     if (state.multiple) {
         warn_report("multiple floppy disk controllers with "
diff --git a/include/hw/core/qdev.h b/include/hw/core/qdev.h
index 9220e9e3b1..eded1b715d 100644
--- a/include/hw/core/qdev.h
+++ b/include/hw/core/qdev.h
@@ -462,7 +462,7 @@ static inline bool qdev_is_realized(DeviceState *dev)
  * initialization.
  * @dev must not be plugged into a bus already.
  * If @bus, plug @dev into @bus.  This takes a reference to @dev.
- * If @dev has no QOM parent, make one up, taking another reference.
+ * @dev must already have a QOM parent (see qdev_new()).
  *
  * If you created @dev via object_new() directly (holding a floating
  * reference), you probably want to use qdev_realize_and_unref() instead.
diff --git a/system/vl.c b/system/vl.c
index b4434839df..e51399d389 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -2192,7 +2192,6 @@ static void parse_memory_options(void)
 static void qemu_create_machine_containers(Object *machine)
 {
     static const char *const containers[] = {
-        "unattached",
         "peripheral",
         "peripheral-anon",
     };
diff --git a/tests/unit/test-qdev-global-props.c b/tests/unit/test-qdev-global-props.c
index 8ea362cbb9..a941774b9d 100644
--- a/tests/unit/test-qdev-global-props.c
+++ b/tests/unit/test-qdev-global-props.c
@@ -78,25 +78,22 @@ static const TypeInfo subclass_type = {
  * inherit the global states of the parent process) will try to create qdev
  * and realize the device.
  *
- * Realization of such anonymous qdev (with no parent object) requires both
- * the machine object and its "unattached" container to be at least present.
+ * Realization requires that the device already has a QOM parent.
  */
 static void test_init_machine(void)
 {
     /* This is a fake machine - it doesn't need to be a machine object */
-    Object *machine = object_property_add_new_container(
+    object_property_add_new_container(
         object_get_root(), "machine");
 
-    /* This container must exist for anonymous qdevs to realize() */
-    object_property_add_new_container(machine, "unattached");
-}
+    }
 
 /* Test simple static property setting to default value */
 static void test_static_prop_subprocess(void)
 {
     MyType *mt;
 
-    mt = STATIC_TYPE(object_new(TYPE_STATIC_PROPS));
+    mt = STATIC_TYPE(object_new_child(qdev_get_machine(), "dev[*]", TYPE_STATIC_PROPS));
     qdev_realize(DEVICE(mt), NULL, &error_fatal);
 
     g_assert_cmpuint(mt->prop1, ==, PROP_DEFAULT);
@@ -131,7 +128,7 @@ static void test_static_globalprop_subprocess(void)
 
     register_global_properties(props);
 
-    mt = STATIC_TYPE(object_new(TYPE_STATIC_PROPS));
+    mt = STATIC_TYPE(object_new_child(qdev_get_machine(), "dev[*]", TYPE_STATIC_PROPS));
     qdev_realize(DEVICE(mt), NULL, &error_fatal);
 
     g_assert_cmpuint(mt->prop1, ==, 200);
@@ -249,7 +246,7 @@ static void test_dynamic_globalprop_subprocess(void)
 
     register_global_properties(props);
 
-    mt = DYNAMIC_TYPE(object_new(TYPE_DYNAMIC_PROPS));
+    mt = DYNAMIC_TYPE(object_new_child(qdev_get_machine(), "dev[*]", TYPE_DYNAMIC_PROPS));
     qdev_realize(DEVICE(mt), NULL, &error_fatal);
 
     g_assert_cmpuint(mt->prop1, ==, 101);
@@ -295,7 +292,7 @@ static void test_subclass_global_props(void)
 
     register_global_properties(props);
 
-    mt = STATIC_TYPE(object_new(TYPE_SUBCLASS));
+    mt = STATIC_TYPE(object_new_child(qdev_get_machine(), "dev[*]", TYPE_SUBCLASS));
     qdev_realize(DEVICE(mt), NULL, &error_fatal);
 
     g_assert_cmpuint(mt->prop1, ==, 102);
diff --git a/tests/unit/test-qdev.c b/tests/unit/test-qdev.c
index 20eae38e03..eafffb7175 100644
--- a/tests/unit/test-qdev.c
+++ b/tests/unit/test-qdev.c
@@ -44,24 +44,21 @@ static const TypeInfo my_dev_type_info = {
 /*
  * Initialize a fake machine, being prepared for future tests.
  *
- * Realization of anonymous qdev (with no parent object) requires both
- * the machine object and its "unattached" container to be at least present.
+ * Realization requires that the device already has a QOM parent.
  */
 static void test_init_machine(void)
 {
     /* This is a fake machine - it doesn't need to be a machine object */
-    Object *machine = object_property_add_new_container(
+    object_property_add_new_container(
         object_get_root(), "machine");
 
-    /* This container must exist for anonymous qdevs to realize() */
-    object_property_add_new_container(machine, "unattached");
-}
+    }
 
 static void test_qdev_free_properties(void)
 {
     MyDev *mt;
 
-    mt = STATIC_TYPE(object_new(TYPE_MY_DEV));
+    mt = STATIC_TYPE(object_new_child(qdev_get_machine(), "dev[*]", TYPE_MY_DEV));
     object_set_props(OBJECT(mt), &error_fatal,
                      "string", "something",
                      "array-u32", "12,13",
@@ -75,7 +72,6 @@ static void test_qdev_free_properties(void)
     g_assert_cmpuint(mt->prop_array_u32[1], ==, 13);
 
     object_unparent(OBJECT(mt));
-    object_unref(mt);
 }
 
 
-- 
2.47.1