On 2026/07/24 20:44, Markus Armbruster wrote:
Akihiko Odaki <[email protected]> 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/[email protected]/
Regards,
Akihiko Odaki
