[PATCH for-11.2 v3 00/15] qdev: Clarify and enforce the device realization lifecycle

[PATCH for-11.2 v3 00/15] qdev: Clarify and enforce the device realization lifecycle

[Akihiko Odaki]

qdev currently represents a device's realization state with a single
boolean. This cannot distinguish a device that has never been realized
from one whose realization has failed or that has been unrealized, nor
can it represent realization in progress. Consequently, the same device
can enter DeviceClass::realize() reentrantly or more than once.
Supporting that complicates device implementations, and the behavior is
untested and likely broken.

This series first clarifies the division of work between
TypeInfo.instance_init, pre-realize property setting, and
DeviceClass::realize, including the implications of device introspection
and property-dependent child creation.

The remaining direct reads of DeviceState::realized are then converted
to qdev_is_realized(). Keeping users behind this accessor allows the
underlying lifecycle representation to change without exposing it; the
accessor is also made const so callers with a const DeviceState can use
it.

Before changing that representation, the series fixes an existing
qdev invariant. qdev_realize() can attach an unparented device to a
bus before giving it a QOM parent. A failure between those operations
leaves a bus child that object_unparent() cannot remove, causing
bus_unparent() to loop indefinitely. qdev_realize() now establishes
the QOM parent before attaching the device to its bus and rolls the
parent back on failure.

Finally, DeviceState::realized and the QOM realized property are
replaced with four explicit phases: initialized, realizing, realized,
and retired.
Entering the realizing phase before invoking callbacks prevents
reentrant realization. A failure after realization starts, or the start
of unrealization, moves the device to the terminal retired phase, so a
device's realize method is invoked at most once.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
Changes in v3:
- Ensured devices are QOM-parented before attaching them to a bus,
  avoiding unparentable bus children when realization fails.
- Clarified property-dependent child creation.
- Removed documentation references to the QOM realized property.
- Replaced DeviceState::realized and the QOM realized property with
  device phases. The realizing phase prevents reentrant realization, and
  the terminal retired phase prevents another attempt after realization
  fails or unrealization begins.
- Link to v2: https://lore.kernel.org/qemu-devel/20260629-qdev-v2-1-8cd1d9be0d8d@rsg.ci.i.u-tokyo.ac.jp

Changes in v2:
- Clarified the ordering of #TypeInfo.instance_init, pre-realize
  property setting, and realization.
- Clarified that #TypeInfo.instance_init is for per-instance properties,
  not class properties.
- Link to v1: https://lore.kernel.org/qemu-devel/20250908-qdev-v1-1-df236f7ce5bd@rsg.ci.i.u-tokyo.ac.jp

---
Akihiko Odaki (15):
      qdev: Clarify instantiation and realization
      qdev: Make qdev_is_realized() take a const DeviceState *
      hw/hyperv/balloon: Use qdev_is_realized()
      hw/intc/apic: Use qdev_is_realized()
      hw/mem/memory-device: Use qdev_is_realized()
      hw/mem/pc-dimm: Use qdev_is_realized()
      hw/nvram: Use qdev_is_realized()
      hw/ppc/pnv_xscom: Use qdev_is_realized()
      hw/vfio: Use qdev_is_realized()
      hw/virtio/virtio-mem: Use qdev_is_realized()
      hw/virtio/virtio-qmp: Use qdev_is_realized()
      target/i386/cpu: Use qdev_is_realized()
      target/s390x: Use qdev_is_realized()
      hw/qdev: Parent device before setting parent bus
      hw/qdev: Prevent devices from being realized more than once

 qapi/common.json           |  19 ++++++
 include/hw/core/qdev.h     |  62 ++++++++++---------
 hw/core/qdev-clock.c       |   4 +-
 hw/core/qdev-properties.c  |   4 +-
 hw/core/qdev.c             | 147 ++++++++++++++++++++++++++++-----------------
 hw/hyperv/hv-balloon.c     |   2 +-
 hw/intc/apic_common.c      |   2 +-
 hw/mem/memory-device.c     |   4 +-
 hw/mem/pc-dimm.c           |   2 +-
 hw/nvram/xlnx-bbram.c      |   2 +-
 hw/nvram/xlnx-efuse.c      |   2 +-
 hw/ppc/pnv_xscom.c         |   2 +-
 hw/scsi/scsi-bus.c         |   4 +-
 hw/vfio/container-legacy.c |   4 +-
 hw/vfio/device.c           |   4 +-
 hw/vfio/iommufd.c          |   2 +-
 hw/virtio/virtio-mem.c     |   4 +-
 hw/virtio/virtio-qmp.c     |   4 +-
 qom/qom-qmp-cmds.c         |   2 +-
 system/qdev-monitor.c      |   5 +-
 target/i386/cpu.c          |   2 +-
 target/s390x/cpu_models.c  |   4 +-
 tests/unit/test-qdev.c     | 125 +++++++++++++++++++++++++++++++++++++-
 23 files changed, 301 insertions(+), 111 deletions(-)
---
base-commit: 499039798cdad7d86b787fec0eaf1da4151c0f05
change-id: 20250906-qdev-9e5cb7c06ffa

Best regards,
--  
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

[PATCH for-11.2 v3 01/15] qdev: Clarify instantiation and realization

[Akihiko Odaki]

The distinction of instantiation and realization was vague in the old
documentation so this change clarifies it.

The old documentation said:
> The former may not fail (and must not abort or exit, since it is
> called during device introspection already), and the latter may return
> error information to the caller and must be re-entrant.
> Trivial field initializations should go into #TypeInfo.instance_init.
> Operations depending on @props static properties should go into
> @realize.

The first problem with the old documentation is that it is unclear what
"trivial field initializations" means and why triviality makes
initialization appropriate for #TypeInfo.instance_init. Another problem
is that the documentation is not comprehensive enough; for example, it
mentions @props static properties, but it does not say anything about
the other properties.

The keys to distinguish instantiation and realization are property
setting and device introspection. The fact that property setting happens
after #TypeInfo.instance_init and before realization implies that
operations depending on properties should go into @realize.

The fact that instantiation happens during device introspection but
realization does not implies:
- Instance properties should be added in #TypeInfo.instance_init.
- Instantiation must not have any side effect not contained in the
  instance.
- Any operations without special requirements should go into @realize so
  that they can be skipped during device introspection.

Note these two facts to guide appropriate instantiation and realization.

We also omit mention of the realized property because it is a QOM
interface detail, not part of the device API.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 include/hw/core/qdev.h | 52 ++++++++++++++++++++++++++++----------------------
 1 file changed, 29 insertions(+), 23 deletions(-)

diff --git a/include/hw/core/qdev.h b/include/hw/core/qdev.h
index e14762234115..aba7072131c4 100644
--- a/include/hw/core/qdev.h
+++ b/include/hw/core/qdev.h
@@ -22,27 +22,35 @@
  * Realization
  * -----------
  *
- * Devices are constructed in two stages:
- *
- * 1) object instantiation via object_initialize() and
- * 2) device realization via the #DeviceState.realized property
- *
- * The former may not fail (and must not abort or exit, since it is called
- * during device introspection already), and the latter may return error
- * information to the caller and must be re-entrant.
- * Trivial field initializations should go into #TypeInfo.instance_init.
- * Operations depending on @props static properties should go into @realize.
+ * Devices are constructed in the following order:
+ *
+ * 1) #TypeInfo.instance_init
+ * 2) pre-realize property value setting
+ * 3) device realization
+ *
+ * #TypeInfo.instance_init may not fail. #DeviceClass.realize can
+ * fail, returning error information to the caller. A device realize
+ * method should handle being called again after it has failed once.
+ * #TypeInfo.instance_init should add instance properties but must not
+ * have any side effect not contained in the instance, since it happens
+ * during device introspection already. Any operations without special
+ * requirements should go @realize so that they can be skipped during
+ * device introspection.
  * After successful realization, setting static properties will fail.
  *
- * As an interim step, the #DeviceState.realized property can also be
- * set with qdev_realize(). In the future, devices will propagate this
- * state change to their children and along busses they expose. The
- * point in time will be deferred to machine creation, so that values
- * set in @realize will not be introspectable beforehand. Therefore
- * devices must not create children during @realize; they should
- * initialize them via object_initialize() in their own
- * #TypeInfo.instance_init and forward the realization events
- * appropriately.
+ * In the future, devices will propagate this state change to their
+ * children and along busses they expose. The point in time will be
+ * deferred to machine creation, so that values set in @realize will not
+ * be introspectable beforehand. Therefore devices should not create
+ * children during @realize; they should initialize them (e.g. by
+ * calling object_initialize_child()) in their own
+ * #TypeInfo.instance_init method, and then realize them (e.g. by
+ * calling qdev_realize()) in their own #DeviceClass.realize method.
+ *
+ * Occasionally a device may need to decide whether or not to create
+ * a child object based on the value of a property. In this case it
+ * will need to both create and realize the child in its realize method,
+ * because the property value is not known until that point.
  *
  * Any type may override the @realize and/or @unrealize callbacks but needs
  * to call the parent type's implementation if keeping their functionality
@@ -101,10 +109,8 @@ typedef int (*DeviceSyncConfig)(DeviceState *dev, Error **errp);
 /**
  * struct DeviceClass - The base class for all devices.
  * @props: Properties accessing state fields.
- * @realize: Callback function invoked when the #DeviceState:realized
- * property is changed to %true.
- * @unrealize: Callback function invoked when the #DeviceState:realized
- * property is changed to %false.
+ * @realize: Callback function to realize the device.
+ * @unrealize: Callback function to unrealize the device.
  * @sync_config: Callback function invoked when QMP command device-sync-config
  * is called. Should synchronize device configuration from host to guest part
  * and notify the guest about the change.

-- 
2.55.0

[PATCH for-11.2 v3 02/15] qdev: Make qdev_is_realized() take a const DeviceState *

[Akihiko Odaki]

qdev_is_realized() only reads DeviceState. Make its parameter const so a
later caller with a const DeviceState * can use it.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 include/hw/core/qdev.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/core/qdev.h b/include/hw/core/qdev.h
index aba7072131c4..3f47ea72c009 100644
--- a/include/hw/core/qdev.h
+++ b/include/hw/core/qdev.h
@@ -443,7 +443,7 @@ DeviceState *qdev_try_new(const char *name);
  * Context: May be called outside big qemu lock.
  * Return: true if the device has been fully constructed, false otherwise.
  */
-static inline bool qdev_is_realized(DeviceState *dev)
+static inline bool qdev_is_realized(const DeviceState *dev)
 {
     return qatomic_load_acquire(&dev->realized);
 }

-- 
2.55.0

[PATCH for-11.2 v3 03/15] hw/hyperv/balloon: Use qdev_is_realized()

[Akihiko Odaki]

DeviceState fields should be accessed through qdev helpers rather than
directly. Use qdev_is_realized() instead of reading
DeviceState::realized directly.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/hyperv/hv-balloon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/hyperv/hv-balloon.c b/hw/hyperv/hv-balloon.c
index 9dd759f11e8a..b8664a246b47 100644
--- a/hw/hyperv/hv-balloon.c
+++ b/hw/hyperv/hv-balloon.c
@@ -197,7 +197,7 @@ static int build_dimm_list(Object *obj, void *opaque)
 
     if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
         DeviceState *dev = DEVICE(obj);
-        if (dev->realized) { /* only realized DIMMs matter */
+        if (qdev_is_realized(dev)) { /* only realized DIMMs matter */
             *list = g_slist_prepend(*list, dev);
         }
     }

-- 
2.55.0

[PATCH for-11.2 v3 04/15] hw/intc/apic: Use qdev_is_realized()

[Akihiko Odaki]

DeviceState fields should be accessed through qdev helpers rather than
directly. Use qdev_is_realized() instead of reading
DeviceState::realized directly.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/intc/apic_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 49c03a5bceef..0f0f37d45734 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -424,7 +424,7 @@ static void apic_common_set_id(Object *obj, Visitor *v, const char *name,
     DeviceState *dev = DEVICE(obj);
     uint32_t value;
 
-    if (dev->realized) {
+    if (qdev_is_realized(dev)) {
         qdev_prop_set_after_realize(dev, name, errp);
         return;
     }

-- 
2.55.0

[PATCH for-11.2 v3 05/15] hw/mem/memory-device: Use qdev_is_realized()

[Akihiko Odaki]

DeviceState fields should be accessed through qdev helpers rather than
directly. Use qdev_is_realized() instead of reading
DeviceState::realized directly.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/mem/memory-device.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
index 9cf82b84cf44..24bebfb6b31e 100644
--- a/hw/mem/memory-device.c
+++ b/hw/mem/memory-device.c
@@ -59,7 +59,7 @@ static int memory_device_build_list(Object *obj, void *opaque)
 
     if (object_dynamic_cast(obj, TYPE_MEMORY_DEVICE)) {
         DeviceState *dev = DEVICE(obj);
-        if (dev->realized) { /* only realized memory devices matter */
+        if (qdev_is_realized(dev)) {
             *list = g_slist_insert_sorted(*list, dev, memory_device_addr_sort);
         }
     }
@@ -326,7 +326,7 @@ static int memory_device_plugged_size(Object *obj, void *opaque)
         const MemoryDeviceState *md = MEMORY_DEVICE(obj);
         const MemoryDeviceClass *mdc = MEMORY_DEVICE_GET_CLASS(obj);
 
-        if (dev->realized && !memory_device_is_empty(md)) {
+        if (qdev_is_realized(dev) && !memory_device_is_empty(md)) {
             *size += mdc->get_plugged_size(md, &error_abort);
         }
     }

-- 
2.55.0

[PATCH for-11.2 v3 06/15] hw/mem/pc-dimm: Use qdev_is_realized()

[Akihiko Odaki]

DeviceState fields should be accessed through qdev helpers rather than
directly. Use qdev_is_realized() instead of reading
DeviceState::realized directly.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/mem/pc-dimm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 3efe47f499a3..68862926ee2d 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -103,7 +103,7 @@ static int pc_dimm_slot2bitmap(Object *obj, void *opaque)
 
     if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
         DeviceState *dev = DEVICE(obj);
-        if (dev->realized) { /* count only realized DIMMs */
+        if (qdev_is_realized(dev)) { /* count only realized DIMMs */
             PCDIMMDevice *d = PC_DIMM(obj);
             set_bit(d->slot, bitmap);
         }

-- 
2.55.0

[PATCH for-11.2 v3 07/15] hw/nvram: Use qdev_is_realized()

[Akihiko Odaki]

DeviceState fields should be accessed through qdev helpers rather than
directly. Use qdev_is_realized() instead of reading
DeviceState::realized directly.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/nvram/xlnx-bbram.c | 2 +-
 hw/nvram/xlnx-efuse.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/nvram/xlnx-bbram.c b/hw/nvram/xlnx-bbram.c
index edfb592a5ec9..e336874bdef4 100644
--- a/hw/nvram/xlnx-bbram.c
+++ b/hw/nvram/xlnx-bbram.c
@@ -468,7 +468,7 @@ static void bbram_prop_set_drive(Object *obj, Visitor *v, const char *name,
     qdev_prop_drive.set(obj, v, name, opaque, errp);
 
     /* Fill initial data if backend is attached after realized */
-    if (dev->realized) {
+    if (qdev_is_realized(dev)) {
         bbram_bdrv_read(XLNX_BBRAM(obj), errp);
     }
 }
diff --git a/hw/nvram/xlnx-efuse.c b/hw/nvram/xlnx-efuse.c
index 1a9650ba128d..03c9fbc0d076 100644
--- a/hw/nvram/xlnx-efuse.c
+++ b/hw/nvram/xlnx-efuse.c
@@ -233,7 +233,7 @@ static void efuse_prop_set_drive(Object *obj, Visitor *v, const char *name,
     qdev_prop_drive.set(obj, v, name, opaque, errp);
 
     /* Fill initial data if backend is attached after realized */
-    if (dev->realized) {
+    if (qdev_is_realized(dev)) {
         efuse_bdrv_read(XLNX_EFUSE(obj), errp);
     }
 }

-- 
2.55.0

[PATCH for-11.2 v3 08/15] hw/ppc/pnv_xscom: Use qdev_is_realized()

[Akihiko Odaki]

DeviceState fields should be accessed through qdev helpers rather than
directly. Use qdev_is_realized() instead of reading
DeviceState::realized directly.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/ppc/pnv_xscom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
index dc1ffc6c014f..7af722584e29 100644
--- a/hw/ppc/pnv_xscom.c
+++ b/hw/ppc/pnv_xscom.c
@@ -253,7 +253,7 @@ static int xscom_dt_child(Object *child, void *opaque)
         /*
          * Only "realized" devices should be configured in the DT
          */
-        if (xc->dt_xscom && DEVICE(child)->realized) {
+        if (xc->dt_xscom && qdev_is_realized(DEVICE(child))) {
             _FDT((xc->dt_xscom(xd, args->fdt, args->xscom_offset)));
         }
     }

-- 
2.55.0

[PATCH for-11.2 v3 09/15] hw/vfio: Use qdev_is_realized()

[Akihiko Odaki]

DeviceState fields should be accessed through qdev helpers rather than
directly. Use qdev_is_realized() instead of reading
DeviceState::realized directly.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/vfio/container-legacy.c | 4 ++--
 hw/vfio/device.c           | 4 ++--
 hw/vfio/iommufd.c          | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/vfio/container-legacy.c b/hw/vfio/container-legacy.c
index d301b27aa652..7ac7b3721458 100644
--- a/hw/vfio/container-legacy.c
+++ b/hw/vfio/container-legacy.c
@@ -1075,7 +1075,7 @@ static int vfio_legacy_pci_hot_reset(VFIODevice *vbasedev, bool single)
 
         /* Prep dependent devices for reset and clear our marker. */
         QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
-            if (!vbasedev_iter->dev->realized ||
+            if (!qdev_is_realized(vbasedev_iter->dev) ||
                 !vfio_pci_from_vfio_device(vbasedev_iter)) {
                 continue;
             }
@@ -1160,7 +1160,7 @@ out:
         }
 
         QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
-            if (!vbasedev_iter->dev->realized ||
+            if (!qdev_is_realized(vbasedev_iter->dev) ||
                 !vfio_pci_from_vfio_device(vbasedev_iter)) {
                 continue;
             }
diff --git a/hw/vfio/device.c b/hw/vfio/device.c
index 1a7f8088aad9..4f119596336f 100644
--- a/hw/vfio/device.c
+++ b/hw/vfio/device.c
@@ -59,13 +59,13 @@ void vfio_device_reset_handler(void *opaque)
 
     trace_vfio_device_reset_handler();
     QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) {
-        if (vbasedev->dev->realized) {
+        if (qdev_is_realized(vbasedev->dev)) {
             vbasedev->ops->vfio_compute_needs_reset(vbasedev);
         }
     }
 
     QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) {
-        if (vbasedev->dev->realized && vbasedev->needs_reset) {
+        if (qdev_is_realized(vbasedev->dev) && vbasedev->needs_reset) {
             vbasedev->ops->vfio_hot_reset_multi(vbasedev);
         }
     }
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 6ff668d2597e..242644aa0010 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -829,7 +829,7 @@ iommufd_cdev_dep_get_realized_vpdev(struct vfio_pci_dependent_device *dep_dev,
 
     vbasedev_tmp = iommufd_cdev_pci_find_by_devid(dep_dev->devid);
     if (!vfio_pci_from_vfio_device(vbasedev_tmp) ||
-        !vbasedev_tmp->dev->realized) {
+        !qdev_is_realized(vbasedev_tmp->dev)) {
         return NULL;
     }
 

-- 
2.55.0

[PATCH for-11.2 v3 10/15] hw/virtio/virtio-mem: Use qdev_is_realized()

[Akihiko Odaki]

DeviceState fields should be accessed through qdev helpers rather than
directly. Use qdev_is_realized() instead of reading
DeviceState::realized directly.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/virtio/virtio-mem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 35e03ed75998..7130ed852d9c 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -1453,7 +1453,7 @@ static void virtio_mem_set_requested_size(Object *obj, Visitor *v,
      * The block size and memory backend are not fixed until the device was
      * realized. realize() will verify these properties then.
      */
-    if (DEVICE(obj)->realized) {
+    if (qdev_is_realized(DEVICE(obj))) {
         if (!QEMU_IS_ALIGNED(value, vmem->block_size)) {
             error_setg(errp, "'%s' has to be multiples of '%s' (0x%" PRIx64
                        ")", name, VIRTIO_MEM_BLOCK_SIZE_PROP,
@@ -1507,7 +1507,7 @@ static void virtio_mem_set_block_size(Object *obj, Visitor *v, const char *name,
     VirtIOMEM *vmem = VIRTIO_MEM(obj);
     uint64_t value;
 
-    if (DEVICE(obj)->realized) {
+    if (qdev_is_realized(DEVICE(obj))) {
         error_setg(errp, "'%s' cannot be changed", name);
         return;
     }

-- 
2.55.0

[PATCH for-11.2 v3 11/15] hw/virtio/virtio-qmp: Use qdev_is_realized()

[Akihiko Odaki]

DeviceState fields should be accessed through qdev helpers rather than
directly. Use qdev_is_realized() instead of reading
DeviceState::realized directly.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/virtio/virtio-qmp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-qmp.c b/hw/virtio/virtio-qmp.c
index 57cc03828c2b..5aaeebdd96af 100644
--- a/hw/virtio/virtio-qmp.c
+++ b/hw/virtio/virtio-qmp.c
@@ -659,7 +659,7 @@ static int query_dev_child(Object *child, void *opaque)
 {
     VirtioInfoList **vdevs = opaque;
     Object *dev = object_dynamic_cast(child, TYPE_VIRTIO_DEVICE);
-    if (dev != NULL && DEVICE(dev)->realized) {
+    if (dev != NULL && qdev_is_realized(DEVICE(dev))) {
         VirtIODevice *vdev = VIRTIO_DEVICE(dev);
         VirtioInfo *info = g_new(VirtioInfo, 1);
 
@@ -688,7 +688,7 @@ VirtIODevice *qmp_find_virtio_device(const char *path)
     /* Verify the canonical path is a realized virtio device */
     Object *dev = object_dynamic_cast(object_resolve_path(path, NULL),
                                       TYPE_VIRTIO_DEVICE);
-    if (!dev || !DEVICE(dev)->realized) {
+    if (!dev || !qdev_is_realized(DEVICE(dev))) {
         return NULL;
     }
     return VIRTIO_DEVICE(dev);

-- 
2.55.0

[PATCH for-11.2 v3 12/15] target/i386/cpu: Use qdev_is_realized()

[Akihiko Odaki]

DeviceState fields should be accessed through qdev helpers rather than
directly. Use qdev_is_realized() instead of reading
DeviceState::realized directly.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 target/i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 5805d33ab92d..7a2c3cf358d3 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -10387,7 +10387,7 @@ static void x86_cpu_set_bit_prop(Object *obj, Visitor *v, const char *name,
     BitProperty *fp = opaque;
     bool value;
 
-    if (dev->realized) {
+    if (qdev_is_realized(dev)) {
         qdev_prop_set_after_realize(dev, name, errp);
         return;
     }

-- 
2.55.0

[PATCH for-11.2 v3 13/15] target/s390x: Use qdev_is_realized()

[Akihiko Odaki]

DeviceState fields should be accessed through qdev helpers rather than
directly. Use qdev_is_realized() instead of reading
DeviceState::realized directly.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 target/s390x/cpu_models.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 2dd9aac8078c..f292af7ad01c 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -714,7 +714,7 @@ static void set_feature(Object *obj, Visitor *v, const char *name,
     S390CPU *cpu = S390_CPU(obj);
     bool value;
 
-    if (dev->realized) {
+    if (qdev_is_realized(dev)) {
         error_setg(errp, "Attempt to set property '%s' on '%s' after "
                    "it was realized", name, object_get_typename(obj));
         return;
@@ -770,7 +770,7 @@ static void set_feature_group(Object *obj, Visitor *v, const char *name,
     S390CPU *cpu = S390_CPU(obj);
     bool value;
 
-    if (dev->realized) {
+    if (qdev_is_realized(dev)) {
         error_setg(errp, "Attempt to set property '%s' on '%s' after "
                    "it was realized", name, object_get_typename(obj));
         return;

-- 
2.55.0

[PATCH for-11.2 v3 14/15] hw/qdev: Parent device before setting parent bus

[Akihiko Odaki]

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>
---
 hw/core/qdev.c         | 51 ++++++++++++++++++++++++++++----------------------
 tests/unit/test-qdev.c | 13 +++++++++++++
 2 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index e2aab3d1fc61..0b0f2f47fa78 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -264,17 +264,43 @@ static void device_reset_child_foreach(Object *obj, ResettableChildCallback cb,
 
 bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp)
 {
-    assert(!dev->realized && !dev->parent_bus);
+    static int unattached_count;
+    bool unattached_parent = false;
+
+    assert(!dev->parent_bus);
+
+    if (!OBJECT(dev)->parent) {
+        gchar *name = g_strdup_printf("device[%d]", unattached_count++);
+
+        object_property_add_child(machine_get_container("unattached"),
+                                  name, OBJECT(dev));
+        unattached_parent = true;
+        g_free(name);
+    }
 
     if (bus) {
         if (!qdev_set_parent_bus(dev, bus, errp)) {
-            return false;
+            goto fail;
         }
     } else {
         assert(!DEVICE_GET_CLASS(dev)->bus_type);
     }
 
-    return object_property_set_bool(OBJECT(dev), "realized", true, errp);
+    if (object_property_set_bool(OBJECT(dev), "realized", true, errp)) {
+        return true;
+    }
+
+fail:
+    if (unattached_parent) {
+        /*
+         * Beware, this doesn't just revert
+         * object_property_add_child(), it also runs bus_remove()!
+         */
+        object_unparent(OBJECT(dev));
+        unattached_count--;
+    }
+
+    return false;
 }
 
 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)
     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",
@@ -493,15 +517,6 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
             goto fail;
         }
 
-        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);
-        }
-
         hotplug_ctrl = qdev_get_hotplug_handler(dev);
         if (hotplug_ctrl) {
             hotplug_handler_pre_plug(hotplug_ctrl, dev, &local_err);
@@ -627,14 +642,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/tests/unit/test-qdev.c b/tests/unit/test-qdev.c
index 20eae38e03f4..77c3eee71713 100644
--- a/tests/unit/test-qdev.c
+++ b/tests/unit/test-qdev.c
@@ -78,6 +78,16 @@ static void test_qdev_free_properties(void)
     object_unref(mt);
 }
 
+static void test_qdev_double_realization(void)
+{
+    MyDev *mt = STATIC_TYPE(object_new(TYPE_MY_DEV));
+
+    qdev_realize(DEVICE(mt), NULL, &error_fatal);
+    qdev_realize(DEVICE(mt), NULL, &error_fatal);
+    object_unparent(OBJECT(mt));
+    object_unref(OBJECT(mt));
+}
+
 
 int main(int argc, char **argv)
 {
@@ -90,6 +100,9 @@ int main(int argc, char **argv)
     g_test_add_func("/qdev/free-properties",
                     test_qdev_free_properties);
 
+    g_test_add_func("/qdev/double-realization",
+                    test_qdev_double_realization);
+
     g_test_run();
 
     return 0;

-- 
2.55.0

[PATCH for-11.2 v3 15/15] hw/qdev: Prevent devices from being realized more than once

[Akihiko Odaki]

qdev currently permits reentrant realization of the same device. It also
permits another realization attempt after a device has been unrealized
or a previous attempt has failed. Either path can invoke
DeviceClass::realize() more than once. Supporting repeated realization
adds complexity to device implementations. It is untested and likely
broken.

Replace the bool DeviceState::realized field with the enum-valued
DeviceState::phase field. The enum has four values:

- initialized
- realizing
- realized
- retired

Realization can start only in the initialized phase. It moves the device
to the realizing phase before invoking callbacks, preventing another
realization attempt. Successful realization moves it to the realized
phase; failure after realization has started moves it to the retired
phase. Unrealization also moves a realized device to the retired phase.

The QOM realized property is an internal lifecycle property, not for
end users. Replace it with the enum-valued phase property.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 qapi/common.json          |  19 ++++++++
 include/hw/core/qdev.h    |  12 ++---
 hw/core/qdev-clock.c      |   4 +-
 hw/core/qdev-properties.c |   4 +-
 hw/core/qdev.c            |  98 ++++++++++++++++++++++++++--------------
 hw/scsi/scsi-bus.c        |   4 +-
 qom/qom-qmp-cmds.c        |   2 +-
 system/qdev-monitor.c     |   5 ++-
 tests/unit/test-qdev.c    | 112 +++++++++++++++++++++++++++++++++++++++++++++-
 9 files changed, 212 insertions(+), 48 deletions(-)

diff --git a/qapi/common.json b/qapi/common.json
index af7e3d618a7c..88a308cbd172 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -7,6 +7,25 @@
 # *****************
 ##
 
+##
+# @DevicePhase:
+#
+# An enumeration of the device phases
+#
+# @initialized: the initial phase
+#
+# @realizing: the phase during realization
+#
+# @realized: the phase after realization
+#
+# @retired: the terminal phase entered when unrealization begins or
+#           realization fails after starting
+#
+# Since: 11.1
+##
+{ 'enum': 'DevicePhase',
+  'data': [ 'initialized', 'realizing', 'realized', 'retired' ] }
+
 ##
 # @IoOperationType:
 #
diff --git a/include/hw/core/qdev.h b/include/hw/core/qdev.h
index 3f47ea72c009..c9012155a6ec 100644
--- a/include/hw/core/qdev.h
+++ b/include/hw/core/qdev.h
@@ -1,6 +1,7 @@
 #ifndef QDEV_CORE_H
 #define QDEV_CORE_H
 
+#include "qapi/qapi-types-common.h"
 #include "qemu/atomic.h"
 #include "qemu/queue.h"
 #include "qemu/bitmap.h"
@@ -29,8 +30,9 @@
  * 3) device realization
  *
  * #TypeInfo.instance_init may not fail. #DeviceClass.realize can
- * fail, returning error information to the caller. A device realize
- * method should handle being called again after it has failed once.
+ * fail, returning error information to the caller. A device's realize
+ * method is called at most once, even if realization fails or the
+ * device is later unrealized.
  * #TypeInfo.instance_init should add instance properties but must not
  * have any side effect not contained in the instance, since it happens
  * during device introspection already. Any operations without special
@@ -239,9 +241,9 @@ struct DeviceState {
      */
     char *canonical_path;
     /**
-     * @realized: has device been realized?
+     * @phase: the current phase
      */
-    bool realized;
+    DevicePhase phase;
     /**
      * @pending_deleted_event: track pending deletion events during unplug
      */
@@ -445,7 +447,7 @@ DeviceState *qdev_try_new(const char *name);
  */
 static inline bool qdev_is_realized(const DeviceState *dev)
 {
-    return qatomic_load_acquire(&dev->realized);
+    return qatomic_load_acquire(&dev->phase) == DEVICE_PHASE_REALIZED;
 }
 
 /**
diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
index 861f78f94c64..efb33a3a27e7 100644
--- a/hw/core/qdev-clock.c
+++ b/hw/core/qdev-clock.c
@@ -30,7 +30,7 @@ static NamedClockList *qdev_init_clocklist(DeviceState *dev, const char *name,
      * Clock must be added before realize() so that we can compute the
      * clock's canonical path during device_realize().
      */
-    assert(!dev->realized);
+    assert(dev->phase != DEVICE_PHASE_REALIZED);
 
     /*
      * The ncl structure is freed by qdev_finalize_clocklist() which will
@@ -186,6 +186,6 @@ Clock *qdev_alias_clock(DeviceState *dev, const char *name,
 
 void qdev_connect_clock_in(DeviceState *dev, const char *name, Clock *source)
 {
-    assert(!dev->realized);
+    assert(dev->phase != DEVICE_PHASE_REALIZED);
     clock_set_source(qdev_get_clock_in(dev, name), source);
 }
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 34d7b26a7303..66cff4b9338c 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -33,7 +33,7 @@ static bool qdev_prop_allow_set(Object *obj, const char *name,
 {
     DeviceState *dev = DEVICE(obj);
 
-    if (dev->realized && !info->realized_set_allowed) {
+    if (dev->phase == DEVICE_PHASE_REALIZED && !info->realized_set_allowed) {
         qdev_prop_set_after_realize(dev, name, errp);
         return false;
     }
@@ -46,7 +46,7 @@ void qdev_prop_allow_set_link_before_realize(const Object *obj,
 {
     DeviceState *dev = DEVICE(obj);
 
-    if (dev->realized) {
+    if (dev->phase == DEVICE_PHASE_REALIZED) {
         error_setg(errp, "Attempt to set link property '%s' on device '%s' "
                    "(type '%s') after it was realized",
                    name, dev->id, object_get_typename(obj));
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 0b0f2f47fa78..994af2bec4fc 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -134,7 +134,7 @@ bool qdev_set_parent_bus(DeviceState *dev, BusState *bus, Error **errp)
     dev->parent_bus = bus;
     object_ref(OBJECT(bus));
     bus_add_child(bus, dev);
-    if (dev->realized) {
+    if (dev->phase == DEVICE_PHASE_REALIZED) {
         resettable_change_parent(OBJECT(dev), OBJECT(bus),
                                  OBJECT(old_parent_bus));
     }
@@ -230,7 +230,7 @@ bool qdev_should_hide_device(const QDict *opts, bool from_json, Error **errp)
 void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id,
                                  int required_for_version)
 {
-    assert(!dev->realized);
+    assert(dev->phase != DEVICE_PHASE_REALIZED);
     dev->instance_id_alias = alias_id;
     dev->alias_required_for_version = required_for_version;
 }
@@ -286,7 +286,7 @@ bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp)
         assert(!DEVICE_GET_CLASS(dev)->bus_type);
     }
 
-    if (object_property_set_bool(OBJECT(dev), "realized", true, errp)) {
+    if (object_property_set_str(OBJECT(dev), "phase", "realized", errp)) {
         return true;
     }
 
@@ -314,7 +314,7 @@ bool qdev_realize_and_unref(DeviceState *dev, BusState *bus, Error **errp)
 
 void qdev_unrealize(DeviceState *dev)
 {
-    object_property_set_bool(OBJECT(dev), "realized", false, &error_abort);
+    object_property_set_str(OBJECT(dev), "phase", "retired", &error_abort);
 }
 
 static int qdev_assert_realized_properly_cb(Object *obj, void *opaque)
@@ -324,7 +324,7 @@ static int qdev_assert_realized_properly_cb(Object *obj, void *opaque)
 
     if (dev) {
         dc = DEVICE_GET_CLASS(dev);
-        assert(dev->realized);
+        assert(dev->phase == DEVICE_PHASE_REALIZED);
         assert(dev->parent_bus || !dc->bus_type);
     }
     return 0;
@@ -477,10 +477,10 @@ bool qdev_unplug_blocked(DeviceState *dev, Error **errp)
     return false;
 }
 
-static bool device_get_realized(Object *obj, Error **errp)
+static int device_get_phase(Object *obj, Error **errp)
 {
     DeviceState *dev = DEVICE(obj);
-    return dev->realized;
+    return dev->phase;
 }
 
 static bool check_only_migratable(Object *obj, Error **errp)
@@ -497,14 +497,29 @@ static bool check_only_migratable(Object *obj, Error **errp)
     return true;
 }
 
-static void device_set_realized(Object *obj, bool value, Error **errp)
+static void device_set_phase(Object *obj, int value, Error **errp)
 {
+    ERRP_GUARD();
     DeviceState *dev = DEVICE(obj);
     DeviceClass *dc = DEVICE_GET_CLASS(dev);
+    DevicePhase old_value = dev->phase;
     HotplugHandler *hotplug_ctrl;
     BusState *bus;
     NamedClockList *ncl;
-    Error *local_err = NULL;
+
+    if (value == old_value) {
+        return;
+    }
+
+    if (old_value == DEVICE_PHASE_REALIZING) {
+        error_setg(errp, "The device is currently realizing");
+        return;
+    }
+
+    if (old_value == DEVICE_PHASE_RETIRED) {
+        error_setg(errp, "A device cannot transition from retired to another phase");
+        return;
+    }
 
     if (dev->hotplugged && !dc->hotpluggable) {
         error_setg(errp, "Device '%s' does not support hotplugging",
@@ -512,22 +527,33 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
         return;
     }
 
-    if (value && !dev->realized) {
+    switch (value) {
+    case DEVICE_PHASE_INITIALIZED:
+        error_setg(errp, "A device cannot transition from another phase to initialized");
+        return;
+
+    case DEVICE_PHASE_REALIZING:
+        error_setg(errp, "The realizing phase cannot be set via property");
+        return;
+
+    case DEVICE_PHASE_REALIZED:
+       qatomic_set(&dev->phase, DEVICE_PHASE_REALIZING);
+
         if (!check_only_migratable(obj, errp)) {
             goto fail;
         }
 
         hotplug_ctrl = qdev_get_hotplug_handler(dev);
         if (hotplug_ctrl) {
-            hotplug_handler_pre_plug(hotplug_ctrl, dev, &local_err);
-            if (local_err != NULL) {
+            hotplug_handler_pre_plug(hotplug_ctrl, dev, errp);
+            if (*errp) {
                 goto fail;
             }
         }
 
         if (dc->realize) {
-            dc->realize(dev, &local_err);
-            if (local_err != NULL) {
+            dc->realize(dev, errp);
+            if (*errp) {
                 goto fail;
             }
         }
@@ -554,7 +580,7 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
                                                qdev_get_vmsd(dev), dev,
                                                dev->instance_id_alias,
                                                dev->alias_required_for_version,
-                                               &local_err) < 0) {
+                                               errp) < 0) {
                 goto post_realize_fail;
             }
         }
@@ -583,25 +609,30 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
         dev->pending_deleted_event = false;
 
         if (hotplug_ctrl) {
-            hotplug_handler_plug(hotplug_ctrl, dev, &local_err);
-            if (local_err != NULL) {
+            hotplug_handler_plug(hotplug_ctrl, dev, errp);
+            if (*errp) {
                 goto child_realize_fail;
             }
        }
 
-       qatomic_store_release(&dev->realized, value);
-
-    } else if (!value && dev->realized) {
+       qatomic_store_release(&dev->phase, value);
+       return;
 
+    case DEVICE_PHASE_RETIRED:
         /*
          * Change the value so that any concurrent users are aware
-         * that the device is going to be unrealized
+         * that the device is going to be retired
          *
-         * TODO: change .realized property to enum that states
-         * each phase of the device realization/unrealization
+         * TODO: change .phase property to state
+         * each sub-phase of the device realization/unrealization
          */
 
-        qatomic_set(&dev->realized, value);
+        qatomic_set(&dev->phase, value);
+
+        if (old_value == DEVICE_PHASE_INITIALIZED) {
+            return;
+        }
+
         /*
          * Ensure that concurrent users see this update prior to
          * any other changes done by unrealize.
@@ -619,10 +650,11 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
         }
         dev->pending_deleted_event = true;
         DEVICE_LISTENER_CALL(unrealize, Reverse, dev);
-    }
+        return;
 
-    assert(local_err == NULL);
-    return;
+    default:
+        g_assert_not_reached();
+    }
 
 child_realize_fail:
     QLIST_FOREACH(bus, &dev->child_bus, sibling) {
@@ -641,7 +673,7 @@ post_realize_fail:
     }
 
 fail:
-    error_propagate(errp, local_err);
+    qatomic_store_release(&dev->phase, DEVICE_PHASE_RETIRED);
 }
 
 static bool device_get_hotpluggable(Object *obj, Error **errp)
@@ -670,7 +702,6 @@ static void device_initfn(Object *obj)
     }
 
     dev->instance_id_alias = -1;
-    dev->realized = false;
     dev->allow_unplug_during_migration = false;
 
     QLIST_INIT(&dev->gpios);
@@ -736,7 +767,7 @@ static void device_unparent(Object *obj)
     DeviceState *dev = DEVICE(obj);
     BusState *bus;
 
-    if (dev->realized) {
+    if (dev->phase == DEVICE_PHASE_REALIZED) {
         qdev_unrealize(dev);
     }
     while (dev->num_child_bus) {
@@ -768,7 +799,7 @@ static void device_class_init(ObjectClass *class, const void *data)
 
     /* by default all devices were considered as hotpluggable,
      * so with intent to check it in generic qdev_unplug() /
-     * device_set_realized() functions make every device
+     * device_set_phase() functions make every device
      * hotpluggable. Devices that shouldn't be hotpluggable,
      * should override it in their class_init()
      */
@@ -786,8 +817,9 @@ static void device_class_init(ObjectClass *class, const void *data)
      */
     dc->legacy_reset = NULL;
 
-    object_class_property_add_bool(class, "realized",
-                                   device_get_realized, device_set_realized);
+    object_class_property_add_enum(class, "phase", "DevicePhase",
+                                   &DevicePhase_lookup,
+                                   device_get_phase, device_set_phase);
     object_class_property_add_bool(class, "hotpluggable",
                                    device_get_hotpluggable, NULL);
     object_class_property_add_bool(class, "hotplugged",
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index dccb2f25b2af..3a83a8457527 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -57,8 +57,8 @@ static SCSIDevice *do_scsi_device_find(SCSIBus *bus,
     /*
      * This function might run on the IO thread and we might race against
      * main thread hot-plugging the device.
-     * We assume that as soon as .realized is set to true we can let
-     * the user access the device.
+     * We assume that as soon as the device is realized we can let
+     * the user access it.
      */
 
     if (retval && !include_unrealized && !qdev_is_realized(&retval->qdev)) {
diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c
index 48b38d2b7f73..5acbeac024e4 100644
--- a/qom/qom-qmp-cmds.c
+++ b/qom/qom-qmp-cmds.c
@@ -211,7 +211,7 @@ ObjectPropertyInfoList *qmp_device_list_properties(const char *typename,
 
         /* Skip Object and DeviceState properties */
         if (strcmp(prop->name, "type") == 0 ||
-            strcmp(prop->name, "realized") == 0 ||
+            strcmp(prop->name, "phase") == 0 ||
             strcmp(prop->name, "hotpluggable") == 0 ||
             strcmp(prop->name, "hotplugged") == 0 ||
             strcmp(prop->name, "parent_bus") == 0) {
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 00fed791cce1..8641f5f72dff 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -596,7 +596,7 @@ const char *qdev_set_id(DeviceState *dev, char *id, Error **errp)
 {
     ObjectProperty *prop;
 
-    assert(!dev->id && !dev->realized);
+    assert(!dev->id && dev->phase != DEVICE_PHASE_REALIZED);
 
     /*
      * object_property_[try_]add_child() below will assert the device
@@ -1078,7 +1078,8 @@ static int qdev_add_hotpluggable_device(Object *obj, void *opaque)
         return 0;
     }
 
-    if (dev->realized && object_property_get_bool(obj, "hotpluggable", NULL)) {
+    if (dev->phase == DEVICE_PHASE_REALIZED &&
+        object_property_get_bool(obj, "hotpluggable", NULL)) {
         *list = g_slist_append(*list, dev);
     }
 
diff --git a/tests/unit/test-qdev.c b/tests/unit/test-qdev.c
index 77c3eee71713..14935f4d3d9e 100644
--- a/tests/unit/test-qdev.c
+++ b/tests/unit/test-qdev.c
@@ -10,6 +10,8 @@ typedef struct MyDev MyDev;
 DECLARE_INSTANCE_CHECKER(MyDev, STATIC_TYPE,
                          TYPE_MY_DEV)
 
+#define TYPE_REENTRANT_REALIZATION "reentrant-realization"
+
 struct MyDev {
     DeviceState parent_obj;
 
@@ -17,6 +19,9 @@ struct MyDev {
     char *prop_string;
     uint32_t *prop_array_u32;
     uint32_t prop_array_u32_nb;
+    uint16_t realization_count;
+    uint16_t unrealization_count;
+    Error *realization_err;
 };
 
 static const Property my_dev_props[] = {
@@ -26,11 +31,25 @@ static const Property my_dev_props[] = {
                      qdev_prop_uint32, uint32_t),
 };
 
+static void my_dev_realize(DeviceState *dev, Error **errp)
+{
+    MyDev *mt = STATIC_TYPE(dev);
+    mt->realization_count++;
+    error_propagate(errp, g_steal_pointer(&mt->realization_err));
+}
+
+static void my_dev_unrealize(DeviceState *dev)
+{
+    MyDev *mt = STATIC_TYPE(dev);
+    mt->unrealization_count++;
+}
+
 static void my_dev_class_init(ObjectClass *oc, const void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
 
-    dc->realize = NULL;
+    dc->realize = my_dev_realize;
+    dc->unrealize = my_dev_unrealize;
     device_class_set_props(dc, my_dev_props);
 }
 
@@ -41,6 +60,25 @@ static const TypeInfo my_dev_type_info = {
     .class_init = my_dev_class_init,
 };
 
+static void reentrant_realization_realize(DeviceState *dev, Error **errp)
+{
+    g_assert_false(qdev_realize(dev, NULL, NULL));
+}
+
+static void reentrant_realization_class_init(ObjectClass *oc, const void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    dc->realize = reentrant_realization_realize;
+}
+
+static const TypeInfo reentrant_realization_type_info = {
+    .name = TYPE_REENTRANT_REALIZATION,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(DeviceState),
+    .class_init = reentrant_realization_class_init,
+};
+
 /*
  * Initialize a fake machine, being prepared for future tests.
  *
@@ -82,8 +120,64 @@ static void test_qdev_double_realization(void)
 {
     MyDev *mt = STATIC_TYPE(object_new(TYPE_MY_DEV));
 
+    g_assert_cmpint(mt->realization_count, ==, 0);
     qdev_realize(DEVICE(mt), NULL, &error_fatal);
+    g_assert_cmpint(mt->realization_count, ==, 1);
+    qdev_realize(DEVICE(mt), NULL, &error_fatal);
+    g_assert_cmpint(mt->realization_count, ==, 1);
+    object_unparent(OBJECT(mt));
+    object_unref(OBJECT(mt));
+}
+
+static void test_qdev_realize_after_unrealization(void)
+{
+    Object *mt = object_new(TYPE_MY_DEV);
+
+    qdev_unrealize(DEVICE(mt));
+    g_assert_false(qdev_realize(DEVICE(mt), NULL, NULL));
+    object_unparent(mt);
+    object_unref(mt);
+}
+
+static void test_qdev_reentrant_realization(void)
+{
+    Object *obj = object_new(TYPE_REENTRANT_REALIZATION);
+
+    qdev_realize(DEVICE(obj), NULL, &error_fatal);
+    object_unparent(OBJECT(obj));
+    object_unref(obj);
+}
+
+static void test_qdev_retry_realization(void)
+{
+    MyDev *mt = STATIC_TYPE(object_new(TYPE_MY_DEV));
+
+    error_setg(&mt->realization_err, "error");
+    g_assert_false(qdev_realize(DEVICE(mt), NULL, NULL));
+    g_assert_false(qdev_realize(DEVICE(mt), NULL, NULL));
+    object_unparent(OBJECT(mt));
+    object_unref(OBJECT(mt));
+}
+
+static void test_qdev_unrealize_after_realization(void)
+{
+    MyDev *mt = STATIC_TYPE(object_new(TYPE_MY_DEV));
+
     qdev_realize(DEVICE(mt), NULL, &error_fatal);
+    g_assert_cmpint(mt->unrealization_count, ==, 0);
+    qdev_unrealize(DEVICE(mt));
+    g_assert_cmpint(mt->unrealization_count, ==, 1);
+    object_unparent(OBJECT(mt));
+    object_unref(OBJECT(mt));
+}
+
+static void test_qdev_unrealize_without_realization(void)
+{
+    MyDev *mt = STATIC_TYPE(object_new(TYPE_MY_DEV));
+
+    g_assert_cmpint(mt->unrealization_count, ==, 0);
+    qdev_unrealize(DEVICE(mt));
+    g_assert_cmpint(mt->unrealization_count, ==, 0);
     object_unparent(OBJECT(mt));
     object_unref(OBJECT(mt));
 }
@@ -95,6 +189,7 @@ int main(int argc, char **argv)
 
     module_call_init(MODULE_INIT_QOM);
     type_register_static(&my_dev_type_info);
+    type_register_static(&reentrant_realization_type_info);
     test_init_machine();
 
     g_test_add_func("/qdev/free-properties",
@@ -103,6 +198,21 @@ int main(int argc, char **argv)
     g_test_add_func("/qdev/double-realization",
                     test_qdev_double_realization);
 
+    g_test_add_func("/qdev/realize-after-unrealization",
+                    test_qdev_realize_after_unrealization);
+
+    g_test_add_func("/qdev/reentrant-realization",
+                    test_qdev_reentrant_realization);
+
+    g_test_add_func("/qdev/retry-realization",
+                    test_qdev_retry_realization);
+
+    g_test_add_func("/qdev/unrealize-after-realization",
+                    test_qdev_unrealize_after_realization);
+
+    g_test_add_func("/qdev/unrealize-without-realization",
+                    test_qdev_unrealize_without_realization);
+
     g_test_run();
 
     return 0;

-- 
2.55.0

Re: [PATCH for-11.2 v3 01/15] qdev: Clarify instantiation and realization

[BALATON Zoltan]

On Tue, 21 Jul 2026, Akihiko Odaki wrote:
> The distinction of instantiation and realization was vague in the old
> documentation so this change clarifies it.
>
> The old documentation said:
>> The former may not fail (and must not abort or exit, since it is
>> called during device introspection already), and the latter may return
>> error information to the caller and must be re-entrant.
>> Trivial field initializations should go into #TypeInfo.instance_init.
>> Operations depending on @props static properties should go into
>> @realize.
>
> The first problem with the old documentation is that it is unclear what
> "trivial field initializations" means and why triviality makes
> initialization appropriate for #TypeInfo.instance_init. Another problem
> is that the documentation is not comprehensive enough; for example, it
> mentions @props static properties, but it does not say anything about
> the other properties.
>
> The keys to distinguish instantiation and realization are property
> setting and device introspection. The fact that property setting happens
> after #TypeInfo.instance_init and before realization implies that
> operations depending on properties should go into @realize.
>
> The fact that instantiation happens during device introspection but
> realization does not implies:
> - Instance properties should be added in #TypeInfo.instance_init.
> - Instantiation must not have any side effect not contained in the
>  instance.
> - Any operations without special requirements should go into @realize so
>  that they can be skipped during device introspection.
>
> Note these two facts to guide appropriate instantiation and realization.
>
> We also omit mention of the realized property because it is a QOM
> interface detail, not part of the device API.
>
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
> include/hw/core/qdev.h | 52 ++++++++++++++++++++++++++++----------------------
> 1 file changed, 29 insertions(+), 23 deletions(-)
>
> diff --git a/include/hw/core/qdev.h b/include/hw/core/qdev.h
> index e14762234115..aba7072131c4 100644
> --- a/include/hw/core/qdev.h
> +++ b/include/hw/core/qdev.h
> @@ -22,27 +22,35 @@
>  * Realization
>  * -----------
>  *
> - * Devices are constructed in two stages:
> - *
> - * 1) object instantiation via object_initialize() and
> - * 2) device realization via the #DeviceState.realized property
> - *
> - * The former may not fail (and must not abort or exit, since it is called
> - * during device introspection already), and the latter may return error
> - * information to the caller and must be re-entrant.
> - * Trivial field initializations should go into #TypeInfo.instance_init.
> - * Operations depending on @props static properties should go into @realize.
> + * Devices are constructed in the following order:
> + *
> + * 1) #TypeInfo.instance_init
> + * 2) pre-realize property value setting
> + * 3) device realization
> + *
> + * #TypeInfo.instance_init may not fail. #DeviceClass.realize can
> + * fail, returning error information to the caller. A device realize
> + * method should handle being called again after it has failed once.

Doesn't the last patch in this series contradict this sentence? Maybe it 
should be removed and say instead that after failed realize the object can 
only be destroyed, no other methods can be called.

> + * #TypeInfo.instance_init should add instance properties but must not
> + * have any side effect not contained in the instance, since it happens
> + * during device introspection already. Any operations without special
> + * requirements should go @realize so that they can be skipped during

You need something between go and @realize such as to or into?

> + * device introspection.
>  * After successful realization, setting static properties will fail.
>  *
> - * As an interim step, the #DeviceState.realized property can also be
> - * set with qdev_realize(). In the future, devices will propagate this
> - * state change to their children and along busses they expose. The
> - * point in time will be deferred to machine creation, so that values
> - * set in @realize will not be introspectable beforehand. Therefore
> - * devices must not create children during @realize; they should
> - * initialize them via object_initialize() in their own
> - * #TypeInfo.instance_init and forward the realization events
> - * appropriately.
> + * In the future, devices will propagate this state change to their
> + * children and along busses they expose. The point in time will be
> + * deferred to machine creation, so that values set in @realize will not
> + * be introspectable beforehand. Therefore devices should not create
> + * children during @realize; they should initialize them (e.g. by
> + * calling object_initialize_child()) in their own
> + * #TypeInfo.instance_init method, and then realize them (e.g. by
> + * calling qdev_realize()) in their own #DeviceClass.realize method.

This complicates devices...

> + * Occasionally a device may need to decide whether or not to create
> + * a child object based on the value of a property. In this case it
> + * will need to both create and realize the child in its realize method,
> + * because the property value is not known until that point.

...and not always possible. So can we relax this so that only children 
that add properties that need to be introspectable and settable has to be 
created in init and otherwise it can be deferred to realize? That way most 
simple devices don't need an init method at all. I'd really say by default 
do everyting in realize unless it needs to be available before realize 
such as properties that change what realize does and avoid splitting 
creating objects between init and realize when not absolutely needed.

Regards,
BALATON Zoltan

>  *
>  * Any type may override the @realize and/or @unrealize callbacks but needs
>  * to call the parent type's implementation if keeping their functionality
> @@ -101,10 +109,8 @@ typedef int (*DeviceSyncConfig)(DeviceState *dev, Error **errp);
> /**
>  * struct DeviceClass - The base class for all devices.
>  * @props: Properties accessing state fields.
> - * @realize: Callback function invoked when the #DeviceState:realized
> - * property is changed to %true.
> - * @unrealize: Callback function invoked when the #DeviceState:realized
> - * property is changed to %false.
> + * @realize: Callback function to realize the device.
> + * @unrealize: Callback function to unrealize the device.
>  * @sync_config: Callback function invoked when QMP command device-sync-config
>  * is called. Should synchronize device configuration from host to guest part
>  * and notify the guest about the change.
>
>

Re: [PATCH for-11.2 v3 01/15] qdev: Clarify instantiation and realization

[Akihiko Odaki]

On 2026/07/21 20:14, BALATON Zoltan wrote:
> On Tue, 21 Jul 2026, Akihiko Odaki wrote:
>> The distinction of instantiation and realization was vague in the old
>> documentation so this change clarifies it.
>>
>> The old documentation said:
>>> The former may not fail (and must not abort or exit, since it is
>>> called during device introspection already), and the latter may return
>>> error information to the caller and must be re-entrant.
>>> Trivial field initializations should go into #TypeInfo.instance_init.
>>> Operations depending on @props static properties should go into
>>> @realize.
>>
>> The first problem with the old documentation is that it is unclear what
>> "trivial field initializations" means and why triviality makes
>> initialization appropriate for #TypeInfo.instance_init. Another problem
>> is that the documentation is not comprehensive enough; for example, it
>> mentions @props static properties, but it does not say anything about
>> the other properties.
>>
>> The keys to distinguish instantiation and realization are property
>> setting and device introspection. The fact that property setting happens
>> after #TypeInfo.instance_init and before realization implies that
>> operations depending on properties should go into @realize.
>>
>> The fact that instantiation happens during device introspection but
>> realization does not implies:
>> - Instance properties should be added in #TypeInfo.instance_init.
>> - Instantiation must not have any side effect not contained in the
>>  instance.
>> - Any operations without special requirements should go into @realize so
>>  that they can be skipped during device introspection.
>>
>> Note these two facts to guide appropriate instantiation and realization.
>>
>> We also omit mention of the realized property because it is a QOM
>> interface detail, not part of the device API.
>>
>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> ---
>> include/hw/core/qdev.h | 52 +++++++++++++++++++++++++++ 
>> +----------------------
>> 1 file changed, 29 insertions(+), 23 deletions(-)
>>
>> diff --git a/include/hw/core/qdev.h b/include/hw/core/qdev.h
>> index e14762234115..aba7072131c4 100644
>> --- a/include/hw/core/qdev.h
>> +++ b/include/hw/core/qdev.h
>> @@ -22,27 +22,35 @@
>>  * Realization
>>  * -----------
>>  *
>> - * Devices are constructed in two stages:
>> - *
>> - * 1) object instantiation via object_initialize() and
>> - * 2) device realization via the #DeviceState.realized property
>> - *
>> - * The former may not fail (and must not abort or exit, since it is 
>> called
>> - * during device introspection already), and the latter may return error
>> - * information to the caller and must be re-entrant.
>> - * Trivial field initializations should go into #TypeInfo.instance_init.
>> - * Operations depending on @props static properties should go into 
>> @realize.
>> + * Devices are constructed in the following order:
>> + *
>> + * 1) #TypeInfo.instance_init
>> + * 2) pre-realize property value setting
>> + * 3) device realization
>> + *
>> + * #TypeInfo.instance_init may not fail. #DeviceClass.realize can
>> + * fail, returning error information to the caller. A device realize
>> + * method should handle being called again after it has failed once.
> 
> Doesn't the last patch in this series contradict this sentence? Maybe it 
> should be removed and say instead that after failed realize the object 
> can only be destroyed, no other methods can be called.

The last patch updates it.

> 
>> + * #TypeInfo.instance_init should add instance properties but must not
>> + * have any side effect not contained in the instance, since it happens
>> + * during device introspection already. Any operations without special
>> + * requirements should go @realize so that they can be skipped during
> 
> You need something between go and @realize such as to or into?

This is another existing quirk, but we can fix it now.

> 
>> + * device introspection.
>>  * After successful realization, setting static properties will fail.
>>  *
>> - * As an interim step, the #DeviceState.realized property can also be
>> - * set with qdev_realize(). In the future, devices will propagate this
>> - * state change to their children and along busses they expose. The
>> - * point in time will be deferred to machine creation, so that values
>> - * set in @realize will not be introspectable beforehand. Therefore
>> - * devices must not create children during @realize; they should
>> - * initialize them via object_initialize() in their own
>> - * #TypeInfo.instance_init and forward the realization events
>> - * appropriately.
>> + * In the future, devices will propagate this state change to their
>> + * children and along busses they expose. The point in time will be
>> + * deferred to machine creation, so that values set in @realize will not
>> + * be introspectable beforehand. Therefore devices should not create
>> + * children during @realize; they should initialize them (e.g. by
>> + * calling object_initialize_child()) in their own
>> + * #TypeInfo.instance_init method, and then realize them (e.g. by
>> + * calling qdev_realize()) in their own #DeviceClass.realize method.
> 
> This complicates devices...
> 
>> + * Occasionally a device may need to decide whether or not to create
>> + * a child object based on the value of a property. In this case it
>> + * will need to both create and realize the child in its realize method,
>> + * because the property value is not known until that point.
> 
> ...and not always possible. So can we relax this so that only children 
> that add properties that need to be introspectable and settable has to 
> be created in init and otherwise it can be deferred to realize? That way 
> most simple devices don't need an init method at all. I'd really say by 
> default do everyting in realize unless it needs to be available before 
> realize such as properties that change what realize does and avoid 
> splitting creating objects between init and realize when not absolutely 
> needed.

I remember that there was a similar discussion regarding the timing to 
call memory_region_init(), and I don't have a strong opinion here, so 
I'll relax this requirement unless someone suggests otherwise.

Regards,
Akihiko Odaki

Re: [PATCH for-11.2 v3 02/15] qdev: Make qdev_is_realized() take a const DeviceState *

[Philippe Mathieu-Daudé]

On 21/7/26 10:16, Akihiko Odaki wrote:
> qdev_is_realized() only reads DeviceState. Make its parameter const so a
> later caller with a const DeviceState * can use it.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   include/hw/core/qdev.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH for-11.2 v3 04/15] hw/intc/apic: Use qdev_is_realized()

[Philippe Mathieu-Daudé]

On 21/7/26 10:16, Akihiko Odaki wrote:
> DeviceState fields should be accessed through qdev helpers rather than
> directly. Use qdev_is_realized() instead of reading
> DeviceState::realized directly.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   hw/intc/apic_common.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH for-11.2 v3 11/15] hw/virtio/virtio-qmp: Use qdev_is_realized()

[Philippe Mathieu-Daudé]

On 21/7/26 10:17, Akihiko Odaki wrote:
> DeviceState fields should be accessed through qdev helpers rather than
> directly. Use qdev_is_realized() instead of reading
> DeviceState::realized directly.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   hw/virtio/virtio-qmp.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH for-11.2 v3 12/15] target/i386/cpu: Use qdev_is_realized()

[Philippe Mathieu-Daudé]

On 21/7/26 10:17, Akihiko Odaki wrote:
> DeviceState fields should be accessed through qdev helpers rather than
> directly. Use qdev_is_realized() instead of reading
> DeviceState::realized directly.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   target/i386/cpu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH for-11.2 v3 13/15] target/s390x: Use qdev_is_realized()

[Philippe Mathieu-Daudé]

On 21/7/26 10:17, Akihiko Odaki wrote:
> DeviceState fields should be accessed through qdev helpers rather than
> directly. Use qdev_is_realized() instead of reading
> DeviceState::realized directly.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   target/s390x/cpu_models.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH for-11.2 v3 06/15] hw/mem/pc-dimm: Use qdev_is_realized()

[Philippe Mathieu-Daudé]

On 21/7/26 10:16, Akihiko Odaki wrote:
> DeviceState fields should be accessed through qdev helpers rather than
> directly. Use qdev_is_realized() instead of reading
> DeviceState::realized directly.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   hw/mem/pc-dimm.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH for-11.2 v3 07/15] hw/nvram: Use qdev_is_realized()

[Philippe Mathieu-Daudé]

On 21/7/26 10:16, Akihiko Odaki wrote:
> DeviceState fields should be accessed through qdev helpers rather than
> directly. Use qdev_is_realized() instead of reading
> DeviceState::realized directly.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   hw/nvram/xlnx-bbram.c | 2 +-
>   hw/nvram/xlnx-efuse.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH for-11.2 v3 08/15] hw/ppc/pnv_xscom: Use qdev_is_realized()

[Philippe Mathieu-Daudé]

On 21/7/26 10:16, Akihiko Odaki wrote:
> DeviceState fields should be accessed through qdev helpers rather than
> directly. Use qdev_is_realized() instead of reading
> DeviceState::realized directly.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   hw/ppc/pnv_xscom.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH for-11.2 v3 09/15] hw/vfio: Use qdev_is_realized()

[Philippe Mathieu-Daudé]

On 21/7/26 10:16, Akihiko Odaki wrote:
> DeviceState fields should be accessed through qdev helpers rather than
> directly. Use qdev_is_realized() instead of reading
> DeviceState::realized directly.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   hw/vfio/container-legacy.c | 4 ++--
>   hw/vfio/device.c           | 4 ++--
>   hw/vfio/iommufd.c          | 2 +-
>   3 files changed, 5 insertions(+), 5 deletions(-)


> diff --git a/hw/vfio/device.c b/hw/vfio/device.c
> index 1a7f8088aad9..4f119596336f 100644
> --- a/hw/vfio/device.c
> +++ b/hw/vfio/device.c
> @@ -59,13 +59,13 @@ void vfio_device_reset_handler(void *opaque)
>   
>       trace_vfio_device_reset_handler();
>       QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) {
> -        if (vbasedev->dev->realized) {
> +        if (qdev_is_realized(vbasedev->dev)) {
>               vbasedev->ops->vfio_compute_needs_reset(vbasedev);
>           }
>       }
>   
>       QLIST_FOREACH(vbasedev, &vfio_device_list, global_next) {
> -        if (vbasedev->dev->realized && vbasedev->needs_reset) {
> +        if (qdev_is_realized(vbasedev->dev) && vbasedev->needs_reset) {
>               vbasedev->ops->vfio_hot_reset_multi(vbasedev);
>           }
>       }

Interesting.

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH for-11.2 v3 10/15] hw/virtio/virtio-mem: Use qdev_is_realized()

[Philippe Mathieu-Daudé]

On 21/7/26 10:17, Akihiko Odaki wrote:
> DeviceState fields should be accessed through qdev helpers rather than
> directly. Use qdev_is_realized() instead of reading
> DeviceState::realized directly.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   hw/virtio/virtio-mem.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH for-11.2 v3 05/15] hw/mem/memory-device: Use qdev_is_realized()

[Philippe Mathieu-Daudé]

On 21/7/26 10:16, Akihiko Odaki wrote:
> DeviceState fields should be accessed through qdev helpers rather than
> directly. Use qdev_is_realized() instead of reading
> DeviceState::realized directly.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   hw/mem/memory-device.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH for-11.2 v3 03/15] hw/hyperv/balloon: Use qdev_is_realized()

[Philippe Mathieu-Daudé]

On 21/7/26 10:16, Akihiko Odaki wrote:
> DeviceState fields should be accessed through qdev helpers rather than
> directly. Use qdev_is_realized() instead of reading
> DeviceState::realized directly.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   hw/hyperv/hv-balloon.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH for-11.2 v3 15/15] hw/qdev: Prevent devices from being realized more than once

[Philippe Mathieu-Daudé]

Hi Akihiko,

On 21/7/26 10:17, Akihiko Odaki wrote:
> qdev currently permits reentrant realization of the same device. It also
> permits another realization attempt after a device has been unrealized
> or a previous attempt has failed. Either path can invoke
> DeviceClass::realize() more than once. Supporting repeated realization
> adds complexity to device implementations. It is untested and likely
> broken.
> 
> Replace the bool DeviceState::realized field with the enum-valued
> DeviceState::phase field. The enum has four values:
> 
> - initialized
> - realizing
> - realized
> - retired

Excellent.

I have been working on something similar.

I'd start the first patch only including:

DEVICE_PHASE_UNREALIZED (false)
DEVICE_PHASE_REALIZED (true)

Then gradually rename DEVICE_PHASE_REALIZED -> DEVICE_PHASE_CREATED
and add the DEVICE_PHASE_REALIZING and DEVICE_PHASE_RETIRED phases,
so we can discuss them during the review process.

> Realization can start only in the initialized phase. It moves the device
> to the realizing phase before invoking callbacks, preventing another
> realization attempt. Successful realization moves it to the realized
> phase; failure after realization has started moves it to the retired
> phase. Unrealization also moves a realized device to the retired phase.

So what is the difference between 'initialized' and 'retired'?

> The QOM realized property is an internal lifecycle property, not for
> end users. Replace it with the enum-valued phase property.
> 
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
>   qapi/common.json          |  19 ++++++++
>   include/hw/core/qdev.h    |  12 ++---
>   hw/core/qdev-clock.c      |   4 +-
>   hw/core/qdev-properties.c |   4 +-
>   hw/core/qdev.c            |  98 ++++++++++++++++++++++++++--------------
>   hw/scsi/scsi-bus.c        |   4 +-
>   qom/qom-qmp-cmds.c        |   2 +-
>   system/qdev-monitor.c     |   5 ++-
>   tests/unit/test-qdev.c    | 112 +++++++++++++++++++++++++++++++++++++++++++++-
>   9 files changed, 212 insertions(+), 48 deletions(-)
> 
> diff --git a/qapi/common.json b/qapi/common.json
> index af7e3d618a7c..88a308cbd172 100644
> --- a/qapi/common.json
> +++ b/qapi/common.json
> @@ -7,6 +7,25 @@
>   # *****************
>   ##
>   
> +##
> +# @DevicePhase:
> +#
> +# An enumeration of the device phases
> +#
> +# @initialized: the initial phase
> +#
> +# @realizing: the phase during realization
> +#
> +# @realized: the phase after realization
> +#
> +# @retired: the terminal phase entered when unrealization begins or
> +#           realization fails after starting
> +#
> +# Since: 11.1
> +##
> +{ 'enum': 'DevicePhase',
> +  'data': [ 'initialized', 'realizing', 'realized', 'retired' ] }
> +


> @@ -477,10 +477,10 @@ bool qdev_unplug_blocked(DeviceState *dev, Error **errp)
>       return false;
>   }
>   
> -static bool device_get_realized(Object *obj, Error **errp)
> +static int device_get_phase(Object *obj, Error **errp)

DevicePhase

>   {
>       DeviceState *dev = DEVICE(obj);
> -    return dev->realized;
> +    return dev->phase;
>   }


> @@ -670,7 +702,6 @@ static void device_initfn(Object *obj)
>       }
>   
>       dev->instance_id_alias = -1;
> -    dev->realized = false;

Can we keep the initialization explicit? This helps when navigating
the code base.

>       dev->allow_unplug_during_migration = false;
>   
>       QLIST_INIT(&dev->gpios);

Re: [PATCH for-11.2 v3 14/15] hw/qdev: Parent device before setting parent bus

[Philippe Mathieu-Daudé]

On 21/7/26 10:17, Akihiko Odaki wrote:
> 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>
> ---
>   hw/core/qdev.c         | 51 ++++++++++++++++++++++++++++----------------------
>   tests/unit/test-qdev.c | 13 +++++++++++++
>   2 files changed, 42 insertions(+), 22 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

Re: [PATCH for-11.2 v3 15/15] hw/qdev: Prevent devices from being realized more than once

[Akihiko Odaki]

On 2026/07/22 5:11, Philippe Mathieu-Daudé wrote:
> Hi Akihiko,
> 
> On 21/7/26 10:17, Akihiko Odaki wrote:
>> qdev currently permits reentrant realization of the same device. It also
>> permits another realization attempt after a device has been unrealized
>> or a previous attempt has failed. Either path can invoke
>> DeviceClass::realize() more than once. Supporting repeated realization
>> adds complexity to device implementations. It is untested and likely
>> broken.
>>
>> Replace the bool DeviceState::realized field with the enum-valued
>> DeviceState::phase field. The enum has four values:
>>
>> - initialized
>> - realizing
>> - realized
>> - retired
> 
> Excellent.
> 
> I have been working on something similar.
> 
> I'd start the first patch only including:
> 
> DEVICE_PHASE_UNREALIZED (false)
> DEVICE_PHASE_REALIZED (true)
> 
> Then gradually rename DEVICE_PHASE_REALIZED -> DEVICE_PHASE_CREATED
> and add the DEVICE_PHASE_REALIZING and DEVICE_PHASE_RETIRED phases,
> so we can discuss them during the review process.

A gradual conversion makes sense. I kept the "realized" phase as-is 
because it maps exactly to the current external behavior. This patch 
splits the internal "unrealized" state into three distinct phases, but 
the external concept of being "realized" remains unchanged. This allows 
us to avoid a tree-wide refactoring, which is also why 
qdev_is_realized() is preserved.

> 
>> Realization can start only in the initialized phase. It moves the device
>> to the realizing phase before invoking callbacks, preventing another
>> realization attempt. Successful realization moves it to the realized
>> phase; failure after realization has started moves it to the retired
>> phase. Unrealization also moves a realized device to the retired phase.
> 
> So what is the difference between 'initialized' and 'retired'?

The first statement in this paragraph differentiates 'initialized' from 
everything else: realization can start only in the initialized phase. A 
'retired' device cannot be realized. This property avoids re-entrancy.

> 
>> The QOM realized property is an internal lifecycle property, not for
>> end users. Replace it with the enum-valued phase property.
>>
>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> ---
>>   qapi/common.json          |  19 ++++++++
>>   include/hw/core/qdev.h    |  12 ++---
>>   hw/core/qdev-clock.c      |   4 +-
>>   hw/core/qdev-properties.c |   4 +-
>>   hw/core/qdev.c            |  98 +++++++++++++++++++++++++ 
>> +--------------
>>   hw/scsi/scsi-bus.c        |   4 +-
>>   qom/qom-qmp-cmds.c        |   2 +-
>>   system/qdev-monitor.c     |   5 ++-
>>   tests/unit/test-qdev.c    | 112 ++++++++++++++++++++++++++++++++++++ 
>> +++++++++-
>>   9 files changed, 212 insertions(+), 48 deletions(-)
>>
>> diff --git a/qapi/common.json b/qapi/common.json
>> index af7e3d618a7c..88a308cbd172 100644
>> --- a/qapi/common.json
>> +++ b/qapi/common.json
>> @@ -7,6 +7,25 @@
>>   # *****************
>>   ##
>> +##
>> +# @DevicePhase:
>> +#
>> +# An enumeration of the device phases
>> +#
>> +# @initialized: the initial phase
>> +#
>> +# @realizing: the phase during realization
>> +#
>> +# @realized: the phase after realization
>> +#
>> +# @retired: the terminal phase entered when unrealization begins or
>> +#           realization fails after starting
>> +#
>> +# Since: 11.1
>> +##
>> +{ 'enum': 'DevicePhase',
>> +  'data': [ 'initialized', 'realizing', 'realized', 'retired' ] }
>> +
> 
> 
>> @@ -477,10 +477,10 @@ bool qdev_unplug_blocked(DeviceState *dev, Error 
>> **errp)
>>       return false;
>>   }
>> -static bool device_get_realized(Object *obj, Error **errp)
>> +static int device_get_phase(Object *obj, Error **errp)
> 
> DevicePhase

device_get_phase() must return int to match the getter type required by
object_class_property_add_enum():

     int (*get)(Object *, Error **)

Using DevicePhase there would not match the callback type.

> 
>>   {
>>       DeviceState *dev = DEVICE(obj);
>> -    return dev->realized;
>> +    return dev->phase;
>>   }
> 
> 
>> @@ -670,7 +702,6 @@ static void device_initfn(Object *obj)
>>       }
>>       dev->instance_id_alias = -1;
>> -    dev->realized = false;
> 
> Can we keep the initialization explicit? This helps when navigating
> the code base.

Yes, I will make that change in the next version.

Regards,
Akihiko Odaki

> 
>>       dev->allow_unplug_during_migration = false;
>>       QLIST_INIT(&dev->gpios);

Re: [PATCH for-11.2 v3 15/15] hw/qdev: Prevent devices from being realized more than once

[Philippe Mathieu-Daudé]

On 22/7/26 07:12, Akihiko Odaki wrote:
> On 2026/07/22 5:11, Philippe Mathieu-Daudé wrote:
>> Hi Akihiko,
>>
>> On 21/7/26 10:17, Akihiko Odaki wrote:
>>> qdev currently permits reentrant realization of the same device. It also
>>> permits another realization attempt after a device has been unrealized
>>> or a previous attempt has failed. Either path can invoke
>>> DeviceClass::realize() more than once. Supporting repeated realization
>>> adds complexity to device implementations. It is untested and likely
>>> broken.
>>>
>>> Replace the bool DeviceState::realized field with the enum-valued
>>> DeviceState::phase field. The enum has four values:
>>>
>>> - initialized
>>> - realizing
>>> - realized
>>> - retired
>>
>> Excellent.
>>
>> I have been working on something similar.
>>
>> I'd start the first patch only including:
>>
>> DEVICE_PHASE_UNREALIZED (false)
>> DEVICE_PHASE_REALIZED (true)
>>
>> Then gradually rename DEVICE_PHASE_REALIZED -> DEVICE_PHASE_CREATED
>> and add the DEVICE_PHASE_REALIZING and DEVICE_PHASE_RETIRED phases,
>> so we can discuss them during the review process.
> 
> A gradual conversion makes sense. I kept the "realized" phase as-is 
> because it maps exactly to the current external behavior. This patch 
> splits the internal "unrealized" state into three distinct phases, but 
> the external concept of being "realized" remains unchanged. This allows 
> us to avoid a tree-wide refactoring, which is also why 
> qdev_is_realized() is preserved.
> 
>>
>>> Realization can start only in the initialized phase. It moves the device
>>> to the realizing phase before invoking callbacks, preventing another
>>> realization attempt. Successful realization moves it to the realized
>>> phase; failure after realization has started moves it to the retired
>>> phase. Unrealization also moves a realized device to the retired phase.
>>
>> So what is the difference between 'initialized' and 'retired'?
> 
> The first statement in this paragraph differentiates 'initialized' from 
> everything else: realization can start only in the initialized phase. A 
> 'retired' device cannot be realized. This property avoids re-entrancy.

But we do use unrealize -> realize again, in hotplug path.

So we need to be able to move from 'retired' to 'realizing'
again, thus my wonder what is the difference between 'realizing'
and 'initialized'.

I.e. this test should pass:

static void test_qdev_realize_hotplug(void)
{
     Object *mt = object_new(TYPE_MY_DEV);

     /* plug */
     g_assert_false(qdev_realize(DEVICE(mt), NULL, NULL));

     /* unplug */
     qdev_unrealize(DEVICE(mt));

     /* re-plug */
     g_assert_false(qdev_realize(DEVICE(mt), NULL, NULL));
     qdev_unrealize(DEVICE(mt));
     object_unparent(mt);
     object_unref(mt);
}

Maybe your 'retired' could be renamed as transient 'unrealizing',
similar to 'realizing' phase, then we could transition to the
'unrealized' initial phase?

> 
>>
>>> The QOM realized property is an internal lifecycle property, not for
>>> end users. Replace it with the enum-valued phase property.
>>>
>>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>> ---
>>>   qapi/common.json          |  19 ++++++++
>>>   include/hw/core/qdev.h    |  12 ++---
>>>   hw/core/qdev-clock.c      |   4 +-
>>>   hw/core/qdev-properties.c |   4 +-
>>>   hw/core/qdev.c            |  98 +++++++++++++++++++++++++ 
>>> +--------------
>>>   hw/scsi/scsi-bus.c        |   4 +-
>>>   qom/qom-qmp-cmds.c        |   2 +-
>>>   system/qdev-monitor.c     |   5 ++-
>>>   tests/unit/test-qdev.c    | 112 +++++++++++++++++++++++++++++++++++ 
>>> + +++++++++-
>>>   9 files changed, 212 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/qapi/common.json b/qapi/common.json
>>> index af7e3d618a7c..88a308cbd172 100644
>>> --- a/qapi/common.json
>>> +++ b/qapi/common.json
>>> @@ -7,6 +7,25 @@
>>>   # *****************
>>>   ##
>>> +##
>>> +# @DevicePhase:
>>> +#
>>> +# An enumeration of the device phases
>>> +#
>>> +# @initialized: the initial phase
>>> +#
>>> +# @realizing: the phase during realization
>>> +#
>>> +# @realized: the phase after realization
>>> +#
>>> +# @retired: the terminal phase entered when unrealization begins or
>>> +#           realization fails after starting
>>> +#
>>> +# Since: 11.1
>>> +##
>>> +{ 'enum': 'DevicePhase',
>>> +  'data': [ 'initialized', 'realizing', 'realized', 'retired' ] }
>>> +
>>
>>
>>> @@ -477,10 +477,10 @@ bool qdev_unplug_blocked(DeviceState *dev, 
>>> Error **errp)
>>>       return false;
>>>   }
>>> -static bool device_get_realized(Object *obj, Error **errp)
>>> +static int device_get_phase(Object *obj, Error **errp)
>>
>> DevicePhase
> 
> device_get_phase() must return int to match the getter type required by
> object_class_property_add_enum():
> 
>      int (*get)(Object *, Error **)
> 
> Using DevicePhase there would not match the callback type.

Ah right.

> 
>>
>>>   {
>>>       DeviceState *dev = DEVICE(obj);
>>> -    return dev->realized;
>>> +    return dev->phase;
>>>   }

Re: [PATCH for-11.2 v3 15/15] hw/qdev: Prevent devices from being realized more than once

[Akihiko Odaki]

On 2026/07/22 18:57, Philippe Mathieu-Daudé wrote:
> On 22/7/26 07:12, Akihiko Odaki wrote:
>> On 2026/07/22 5:11, Philippe Mathieu-Daudé wrote:
>>> Hi Akihiko,
>>>
>>> On 21/7/26 10:17, Akihiko Odaki wrote:
>>>> qdev currently permits reentrant realization of the same device. It 
>>>> also
>>>> permits another realization attempt after a device has been unrealized
>>>> or a previous attempt has failed. Either path can invoke
>>>> DeviceClass::realize() more than once. Supporting repeated realization
>>>> adds complexity to device implementations. It is untested and likely
>>>> broken.
>>>>
>>>> Replace the bool DeviceState::realized field with the enum-valued
>>>> DeviceState::phase field. The enum has four values:
>>>>
>>>> - initialized
>>>> - realizing
>>>> - realized
>>>> - retired
>>>
>>> Excellent.
>>>
>>> I have been working on something similar.
>>>
>>> I'd start the first patch only including:
>>>
>>> DEVICE_PHASE_UNREALIZED (false)
>>> DEVICE_PHASE_REALIZED (true)
>>>
>>> Then gradually rename DEVICE_PHASE_REALIZED -> DEVICE_PHASE_CREATED
>>> and add the DEVICE_PHASE_REALIZING and DEVICE_PHASE_RETIRED phases,
>>> so we can discuss them during the review process.
>>
>> A gradual conversion makes sense. I kept the "realized" phase as-is 
>> because it maps exactly to the current external behavior. This patch 
>> splits the internal "unrealized" state into three distinct phases, but 
>> the external concept of being "realized" remains unchanged. This 
>> allows us to avoid a tree-wide refactoring, which is also why 
>> qdev_is_realized() is preserved.
>>
>>>
>>>> Realization can start only in the initialized phase. It moves the 
>>>> device
>>>> to the realizing phase before invoking callbacks, preventing another
>>>> realization attempt. Successful realization moves it to the realized
>>>> phase; failure after realization has started moves it to the retired
>>>> phase. Unrealization also moves a realized device to the retired phase.
>>>
>>> So what is the difference between 'initialized' and 'retired'?
>>
>> The first statement in this paragraph differentiates 'initialized' 
>> from everything else: realization can start only in the initialized 
>> phase. A 'retired' device cannot be realized. This property avoids re- 
>> entrancy.
> 
> But we do use unrealize -> realize again, in hotplug path.
> 
> So we need to be able to move from 'retired' to 'realizing'
> again, thus my wonder what is the difference between 'realizing'
> and 'initialized'.
> 
> I.e. this test should pass:
> 
> static void test_qdev_realize_hotplug(void)
> {
>      Object *mt = object_new(TYPE_MY_DEV);
> 
>      /* plug */
>      g_assert_false(qdev_realize(DEVICE(mt), NULL, NULL));
> 
>      /* unplug */
>      qdev_unrealize(DEVICE(mt));
> 
>      /* re-plug */
>      g_assert_false(qdev_realize(DEVICE(mt), NULL, NULL));
>      qdev_unrealize(DEVICE(mt));
>      object_unparent(mt);
>      object_unref(mt);
> }
> 
> Maybe your 'retired' could be renamed as transient 'unrealizing',
> similar to 'realizing' phase, then we could transition to the
> 'unrealized' initial phase?

I could not find an in-tree hotplug path that unrealizes and then
realizes the same DeviceState.

In the normal device_del path, the unplug handler unrealizes the device, 
and completed unplug then unparents it. A later device_add calls 
qdev_new(), so it realizes a new DeviceState. Reusing an ID or slot does 
not reuse the object.

The virtio-net failover path does retain and replug the same object, but 
it deliberately keeps the device realized. The partial-unplug path skips 
the unplug handler, and replug invokes the pre_plug and plug callbacks 
directly instead of qdev_realize().

More generally, I believe same-instance unrealize -> realize is unsafe. 
Realize callbacks may create QOM children whose lifetime is tied to the 
DeviceState rather than its realized state [1]. For example, 
memory_region_init() initializes an embedded QOM object and adds it as a 
child of the device. Unrealizing the device does not generally finalize 
that MemoryRegion, so a second realization may try to initialize the 
same object again.

So introducing an unrealizing -> unrealized transition would require 
every realize/unrealize pair to restore the state of a fresh instance. 
That contract is not tested, and is the complexity this series is meant 
to remove.

[1] 
https://lore.kernel.org/qemu-devel/64bc4a38-f1d2-45ff-8f4c-c941d6a41e18@rsg.ci.i.u-tokyo.ac.jp/

Regards,
Akihiko Odaki

> 
>>
>>>
>>>> The QOM realized property is an internal lifecycle property, not for
>>>> end users. Replace it with the enum-valued phase property.
>>>>
>>>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>>> ---
>>>>   qapi/common.json          |  19 ++++++++
>>>>   include/hw/core/qdev.h    |  12 ++---
>>>>   hw/core/qdev-clock.c      |   4 +-
>>>>   hw/core/qdev-properties.c |   4 +-
>>>>   hw/core/qdev.c            |  98 +++++++++++++++++++++++++ 
>>>> +--------------
>>>>   hw/scsi/scsi-bus.c        |   4 +-
>>>>   qom/qom-qmp-cmds.c        |   2 +-
>>>>   system/qdev-monitor.c     |   5 ++-
>>>>   tests/unit/test-qdev.c    | 112 ++++++++++++++++++++++++++++++++++ 
>>>> + + +++++++++-
>>>>   9 files changed, 212 insertions(+), 48 deletions(-)
>>>>
>>>> diff --git a/qapi/common.json b/qapi/common.json
>>>> index af7e3d618a7c..88a308cbd172 100644
>>>> --- a/qapi/common.json
>>>> +++ b/qapi/common.json
>>>> @@ -7,6 +7,25 @@
>>>>   # *****************
>>>>   ##
>>>> +##
>>>> +# @DevicePhase:
>>>> +#
>>>> +# An enumeration of the device phases
>>>> +#
>>>> +# @initialized: the initial phase
>>>> +#
>>>> +# @realizing: the phase during realization
>>>> +#
>>>> +# @realized: the phase after realization
>>>> +#
>>>> +# @retired: the terminal phase entered when unrealization begins or
>>>> +#           realization fails after starting
>>>> +#
>>>> +# Since: 11.1
>>>> +##
>>>> +{ 'enum': 'DevicePhase',
>>>> +  'data': [ 'initialized', 'realizing', 'realized', 'retired' ] }
>>>> +
>>>
>>>
>>>> @@ -477,10 +477,10 @@ bool qdev_unplug_blocked(DeviceState *dev, 
>>>> Error **errp)
>>>>       return false;
>>>>   }
>>>> -static bool device_get_realized(Object *obj, Error **errp)
>>>> +static int device_get_phase(Object *obj, Error **errp)
>>>
>>> DevicePhase
>>
>> device_get_phase() must return int to match the getter type required by
>> object_class_property_add_enum():
>>
>>      int (*get)(Object *, Error **)
>>
>> Using DevicePhase there would not match the callback type.
> 
> Ah right.
> 
>>
>>>
>>>>   {
>>>>       DeviceState *dev = DEVICE(obj);
>>>> -    return dev->realized;
>>>> +    return dev->phase;
>>>>   }

Re: [PATCH for-11.2 v3 15/15] hw/qdev: Prevent devices from being realized more than once

[Philippe Mathieu-Daudé]

On 22/7/26 13:08, Akihiko Odaki wrote:
> On 2026/07/22 18:57, Philippe Mathieu-Daudé wrote:
>> On 22/7/26 07:12, Akihiko Odaki wrote:
>>> On 2026/07/22 5:11, Philippe Mathieu-Daudé wrote:
>>>> Hi Akihiko,
>>>>
>>>> On 21/7/26 10:17, Akihiko Odaki wrote:
>>>>> qdev currently permits reentrant realization of the same device. It 
>>>>> also
>>>>> permits another realization attempt after a device has been unrealized
>>>>> or a previous attempt has failed. Either path can invoke
>>>>> DeviceClass::realize() more than once. Supporting repeated realization
>>>>> adds complexity to device implementations. It is untested and likely
>>>>> broken.
>>>>>
>>>>> Replace the bool DeviceState::realized field with the enum-valued
>>>>> DeviceState::phase field. The enum has four values:
>>>>>
>>>>> - initialized
>>>>> - realizing
>>>>> - realized
>>>>> - retired
>>>>
>>>> Excellent.
>>>>
>>>> I have been working on something similar.
>>>>
>>>> I'd start the first patch only including:
>>>>
>>>> DEVICE_PHASE_UNREALIZED (false)
>>>> DEVICE_PHASE_REALIZED (true)
>>>>
>>>> Then gradually rename DEVICE_PHASE_REALIZED -> DEVICE_PHASE_CREATED
>>>> and add the DEVICE_PHASE_REALIZING and DEVICE_PHASE_RETIRED phases,
>>>> so we can discuss them during the review process.
>>>
>>> A gradual conversion makes sense. I kept the "realized" phase as-is 
>>> because it maps exactly to the current external behavior. This patch 
>>> splits the internal "unrealized" state into three distinct phases, 
>>> but the external concept of being "realized" remains unchanged. This 
>>> allows us to avoid a tree-wide refactoring, which is also why 
>>> qdev_is_realized() is preserved.
>>>
>>>>
>>>>> Realization can start only in the initialized phase. It moves the 
>>>>> device
>>>>> to the realizing phase before invoking callbacks, preventing another
>>>>> realization attempt. Successful realization moves it to the realized
>>>>> phase; failure after realization has started moves it to the retired
>>>>> phase. Unrealization also moves a realized device to the retired 
>>>>> phase.
>>>>
>>>> So what is the difference between 'initialized' and 'retired'?
>>>
>>> The first statement in this paragraph differentiates 'initialized' 
>>> from everything else: realization can start only in the initialized 
>>> phase. A 'retired' device cannot be realized. This property avoids 
>>> re- entrancy.
>>
>> But we do use unrealize -> realize again, in hotplug path.
>>
>> So we need to be able to move from 'retired' to 'realizing'
>> again, thus my wonder what is the difference between 'realizing'
>> and 'initialized'.
>>
>> I.e. this test should pass:
>>
>> static void test_qdev_realize_hotplug(void)
>> {
>>      Object *mt = object_new(TYPE_MY_DEV);
>>
>>      /* plug */
>>      g_assert_false(qdev_realize(DEVICE(mt), NULL, NULL));
>>
>>      /* unplug */
>>      qdev_unrealize(DEVICE(mt));
>>
>>      /* re-plug */
>>      g_assert_false(qdev_realize(DEVICE(mt), NULL, NULL));
>>      qdev_unrealize(DEVICE(mt));
>>      object_unparent(mt);
>>      object_unref(mt);
>> }
>>
>> Maybe your 'retired' could be renamed as transient 'unrealizing',
>> similar to 'realizing' phase, then we could transition to the
>> 'unrealized' initial phase?
> 
> I could not find an in-tree hotplug path that unrealizes and then
> realizes the same DeviceState.
> 
> In the normal device_del path, the unplug handler unrealizes the device, 
> and completed unplug then unparents it. A later device_add calls 
> qdev_new(), so it realizes a new DeviceState. Reusing an ID or slot does 
> not reuse the object.

Igor, could you provide your advices here?

> The virtio-net failover path does retain and replug the same object, but 
> it deliberately keeps the device realized. The partial-unplug path skips 
> the unplug handler, and replug invokes the pre_plug and plug callbacks 
> directly instead of qdev_realize().
> 
> More generally, I believe same-instance unrealize -> realize is unsafe. 
> Realize callbacks may create QOM children whose lifetime is tied to the 
> DeviceState rather than its realized state [1]. For example, 
> memory_region_init() initializes an embedded QOM object and adds it as a 
> child of the device. Unrealizing the device does not generally finalize 
> that MemoryRegion, so a second realization may try to initialize the 
> same object again.

I totally concur here.

> 
> So introducing an unrealizing -> unrealized transition would require 
> every realize/unrealize pair to restore the state of a fresh instance. 
> That contract is not tested, and is the complexity this series is meant 
> to remove.
> 
> [1] https://lore.kernel.org/qemu-devel/64bc4a38-f1d2-45ff-8f4c- 
> c941d6a41e18@rsg.ci.i.u-tokyo.ac.jp/
> 
> Regards,
> Akihiko Odaki
> 
>>
>>>
>>>>
>>>>> The QOM realized property is an internal lifecycle property, not for
>>>>> end users. Replace it with the enum-valued phase property.
>>>>>
>>>>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>>>> ---
>>>>>   qapi/common.json          |  19 ++++++++
>>>>>   include/hw/core/qdev.h    |  12 ++---
>>>>>   hw/core/qdev-clock.c      |   4 +-
>>>>>   hw/core/qdev-properties.c |   4 +-
>>>>>   hw/core/qdev.c            |  98 +++++++++++++++++++++++++ 
>>>>> +--------------
>>>>>   hw/scsi/scsi-bus.c        |   4 +-
>>>>>   qom/qom-qmp-cmds.c        |   2 +-
>>>>>   system/qdev-monitor.c     |   5 ++-
>>>>>   tests/unit/test-qdev.c    | 112 +++++++++++++++++++++++++++++++++ 
>>>>> + + + +++++++++-
>>>>>   9 files changed, 212 insertions(+), 48 deletions(-)
>>>>>
>>>>> diff --git a/qapi/common.json b/qapi/common.json
>>>>> index af7e3d618a7c..88a308cbd172 100644
>>>>> --- a/qapi/common.json
>>>>> +++ b/qapi/common.json
>>>>> @@ -7,6 +7,25 @@
>>>>>   # *****************
>>>>>   ##
>>>>> +##
>>>>> +# @DevicePhase:
>>>>> +#
>>>>> +# An enumeration of the device phases
>>>>> +#
>>>>> +# @initialized: the initial phase
>>>>> +#
>>>>> +# @realizing: the phase during realization
>>>>> +#
>>>>> +# @realized: the phase after realization
>>>>> +#
>>>>> +# @retired: the terminal phase entered when unrealization begins or
>>>>> +#           realization fails after starting
>>>>> +#
>>>>> +# Since: 11.1
>>>>> +##
>>>>> +{ 'enum': 'DevicePhase',
>>>>> +  'data': [ 'initialized', 'realizing', 'realized', 'retired' ] }
>>>>> +
>>>>
>>>>
>>>>> @@ -477,10 +477,10 @@ bool qdev_unplug_blocked(DeviceState *dev, 
>>>>> Error **errp)
>>>>>       return false;
>>>>>   }
>>>>> -static bool device_get_realized(Object *obj, Error **errp)
>>>>> +static int device_get_phase(Object *obj, Error **errp)
>>>>
>>>> DevicePhase
>>>
>>> device_get_phase() must return int to match the getter type required by
>>> object_class_property_add_enum():
>>>
>>>      int (*get)(Object *, Error **)
>>>
>>> Using DevicePhase there would not match the callback type.
>>
>> Ah right.
>>
>>>
>>>>
>>>>>   {
>>>>>       DeviceState *dev = DEVICE(obj);
>>>>> -    return dev->realized;
>>>>> +    return dev->phase;
>>>>>   }
> 
> 

Re: [PATCH for-11.2 v3 00/15] qdev: Clarify and enforce the device realization lifecycle

[Markus Armbruster]

Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> writes:

> qdev currently represents a device's realization state with a single
> boolean. This cannot distinguish a device that has never been realized
> from one whose realization has failed or that has been unrealized, nor
> can it represent realization in progress. Consequently, the same device
> can enter DeviceClass::realize() reentrantly or more than once.

.realized is initially false.  It is only ever modified in
device_set_realized(), which is the setter of QOM property "realized" of
"device" and its subtypes.  device_set_realized() is only called when
the property is set.  It does nothing when the new value is the same as
the old value.

Code changes property "realized" only in qdev_realize() and
qdev_unrealize().

Implementations of .realize may call qdev_realize() for their
components.  Having this loop back would be a bug.

If your claim "can enter reentrantly" is correct, we have bugs to fix.
I believe it is incorrect.

There are two kinds of devices, onboard and user-created.

User-created devices go through qdev_device_add_from_qdict().  If
qdev_realize() fails, the device is immediately destroyed.

Onboard devices get created and realized by board code.  It commonly
treats qdev_realize() failure as fatal error.  Trying again instead
would be a bug.

If your claim "can enter more than once" is correct, we have bugs to
fix.  I believe it is incorrect.

Exception, sort of: users can manipulate properties with QMP command
qom-set.  This is generally unsupported and a Very Bad Idea[*].  There
might be supported exceptions (I don't know), but "realized" is
definitely not among them.  Unsurprisingly, it's a fast path to grief:

    $ qemu-system-x86_64 -S -display none -monitor stdio -nodefaults
    QEMU 11.0.90 monitor - type 'help' for more information
    (qemu) qom-set /machine/i440fx realized false
    (qemu) qom-set /machine/i440fx realized true
    qemu-system-x86_64: ../system/memory.c:2585: memory_region_add_subregion_common: Assertion `!subregion->container' failed.

I figure your series rejects the second qom-set.  I'm pretty sure the
first qom-set already wounds the VM fatally[**].  Same as for a
multitude of other properties that aren't prepared to be qom-set at
arbitrary times.

Mind, I'm not objecting to adding additional guards against .realize()
getting called more than exactly once.  I'm only challenging your cover
letter, and probably your commit messages (which I haven't read).

[...]


[*] One could argue that having command qom-set is a Very Bad Idea.

[**] It just doesn't crash a stopped VM right away for me.

Re: [PATCH for-11.2 v3 00/15] qdev: Clarify and enforce the device realization lifecycle

[Akihiko Odaki]

On 2026/07/24 20:44, Markus Armbruster wrote:
> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> writes:
> 
>> qdev currently represents a device's realization state with a single
>> boolean. This cannot distinguish a device that has never been realized
>> from one whose realization has failed or that has been unrealized, nor
>> can it represent realization in progress. Consequently, the same device
>> can enter DeviceClass::realize() reentrantly or more than once.
> 
> .realized is initially false.  It is only ever modified in
> device_set_realized(), which is the setter of QOM property "realized" of
> "device" and its subtypes.  device_set_realized() is only called when
> the property is set.  It does nothing when the new value is the same as
> the old value.
> 
> Code changes property "realized" only in qdev_realize() and
> qdev_unrealize().
> 
> Implementations of .realize may call qdev_realize() for their
> components.  Having this loop back would be a bug.
> 
> If your claim "can enter reentrantly" is correct, we have bugs to fix.
> I believe it is incorrect.
> 
> There are two kinds of devices, onboard and user-created.
> 
> User-created devices go through qdev_device_add_from_qdict().  If
> qdev_realize() fails, the device is immediately destroyed.
> 
> Onboard devices get created and realized by board code.  It commonly
> treats qdev_realize() failure as fatal error.  Trying again instead
> would be a bug.
> 
> If your claim "can enter more than once" is correct, we have bugs to
> fix.  I believe it is incorrect.
> 
> Exception, sort of: users can manipulate properties with QMP command
> qom-set.  This is generally unsupported and a Very Bad Idea[*].  There
> might be supported exceptions (I don't know), but "realized" is
> definitely not among them.  Unsurprisingly, it's a fast path to grief:
> 
>      $ qemu-system-x86_64 -S -display none -monitor stdio -nodefaults
>      QEMU 11.0.90 monitor - type 'help' for more information
>      (qemu) qom-set /machine/i440fx realized false
>      (qemu) qom-set /machine/i440fx realized true
>      qemu-system-x86_64: ../system/memory.c:2585: memory_region_add_subregion_common: Assertion `!subregion->container' failed.
> 
> I figure your series rejects the second qom-set.  I'm pretty sure the
> first qom-set already wounds the VM fatally[**].  Same as for a
> multitude of other properties that aren't prepared to be qom-set at
> arbitrary times.
> 
> Mind, I'm not objecting to adding additional guards against .realize()
> getting called more than exactly once.  I'm only challenging your cover
> letter, and probably your commit messages (which I haven't read).
> 
> [...]
> 
> 
> [*] One could argue that having command qom-set is a Very Bad Idea.
> 
> [**] It just doesn't crash a stopped VM right away for me.
> 

You are right. The change assumes that the same device enters realize() 
twice in no supported scenario; otherwise it will break a real use case, 
so Philippe has requested confirmation from Igor [1].

The primary point the cover letter meant to convey is that the current 
QOM property itself does not actively prevent the "realized" state from 
being incorrectly toggled, as your qom-set example clearly demonstrates. 
This change is meant to align the setter's explicit checks with our 
actual assumption.

Since the cover letter omitted this explicit assumption, it 
inadvertently sounded like I was challenging it. Commit messages have 
the same issue. I will fix the wording in both the cover letter and the 
commit messages for the next version.

[1] 
https://lore.kernel.org/qemu-devel/a79f2ad8-d1ad-4536-85d1-0a01ad2f9513@oss.qualcomm.com/

Regards,
Akihiko Odaki

Re: [PATCH for-11.2 v3 00/15] qdev: Clarify and enforce the device realization lifecycle

[Philippe Mathieu-Daudé]

On 21/7/26 10:16, Akihiko Odaki wrote:

> ---
> Akihiko Odaki (15):
>        qdev: Clarify instantiation and realization
>        qdev: Make qdev_is_realized() take a const DeviceState *
>        hw/hyperv/balloon: Use qdev_is_realized()
>        hw/intc/apic: Use qdev_is_realized()
>        hw/mem/memory-device: Use qdev_is_realized()
>        hw/mem/pc-dimm: Use qdev_is_realized()
>        hw/nvram: Use qdev_is_realized()
>        hw/ppc/pnv_xscom: Use qdev_is_realized()
>        hw/vfio: Use qdev_is_realized()
>        hw/virtio/virtio-mem: Use qdev_is_realized()
>        hw/virtio/virtio-qmp: Use qdev_is_realized()
>        target/i386/cpu: Use qdev_is_realized()
>        target/s390x: Use qdev_is_realized()
>        hw/qdev: Parent device before setting parent bus

First 14 non-controvesial patches queued. Waiting for Igor or
Michael to comment on the last one.

>        hw/qdev: Prevent devices from being realized more than once

Re: [PATCH for-11.2 v3 00/15] qdev: Clarify and enforce the device realization lifecycle

[Philippe Mathieu-Daudé]

On 15/8/26 15:59, Philippe Mathieu-Daudé wrote:
> On 21/7/26 10:16, Akihiko Odaki wrote:
> 
>> ---
>> Akihiko Odaki (15):
>>        qdev: Clarify instantiation and realization
>>        qdev: Make qdev_is_realized() take a const DeviceState *
>>        hw/hyperv/balloon: Use qdev_is_realized()
>>        hw/intc/apic: Use qdev_is_realized()
>>        hw/mem/memory-device: Use qdev_is_realized()
>>        hw/mem/pc-dimm: Use qdev_is_realized()
>>        hw/nvram: Use qdev_is_realized()
>>        hw/ppc/pnv_xscom: Use qdev_is_realized()
>>        hw/vfio: Use qdev_is_realized()
>>        hw/virtio/virtio-mem: Use qdev_is_realized()
>>        hw/virtio/virtio-qmp: Use qdev_is_realized()
>>        target/i386/cpu: Use qdev_is_realized()
>>        target/s390x: Use qdev_is_realized()
>>        hw/qdev: Parent device before setting parent bus
> 
> First 14 non-controvesial patches queued.

I'm dropping the 1st patch which isn't reviewed.

> Waiting for Igor or
> Michael to comment on the last one.
> 
>>        hw/qdev: Prevent devices from being realized more than once
>