On 2026/09/03 19:24, Philippe Mathieu-Daudé wrote:
Hi Akihiko,
Are we still waiting for feedback from Igor?
Yes, we are waiting, but mostly as a precaution. I am comfortable
landing this series now.
Regards,
Akihiko Odaki
On 22/7/26 14:05, Philippe Mathieu-Daudé wrote:
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-
[email protected]/
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 <[email protected]>
---
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;
}
