hw/core/irq.c | 35 +++++++++++++ hw/isa/piix.c | 65 ++++++++++++++--------- include/hw/core/irq.h | 75 +++++++++++++++++++++++++-- include/hw/southbridge/piix.h | 12 ++--- include/qemu/osdep.h | 19 +++++++ include/qom/object.h | 98 ++++++++++++++++++++++++++++++----- include/system/memory.h | 76 +++++++++++++++++++++++---- meson.build | 1 + meson_options.txt | 2 + qom/object.c | 36 ++++++++++++- scripts/meson-buildoptions.sh | 3 ++ system/memory.c | 85 +++++++++++++++++++++++++----- 12 files changed, 433 insertions(+), 74 deletions(-)
QOM has two rather unusual / surprising features historicall
* The ability to embed a QOM instance's memory inside another
struct
* The ability to register properties against the instnce
instead of struct
While they both look convenient on the surface, they also
have significant undesirable side effects (see the commit
message for each patch for details).
The premise of this series is that their convenience does
not outweigh their downsides, and we would be better off
long term by eliminating their usage, rather than trying
to add more hacks on top to mitigate their downsides.
This comes out of two separate conversations this week
* Migration series where Peter proposed changes that
make use of instance properties. When I commented,
Peter rightly pointed out that our docs do not
discourage use of instance properties:
https://lists.gnu.org/archive/html/qemu-devel/2026-06/msg02368.html
* QOM series where Akihiko proposed some funky hacks
to reference counting to better track object lifecycle
when embedding structs. We had a short discussion about
discouraging QOM embedding, so this is a real world
proposal:
https://lists.gnu.org/archive/html/qemu-devel/2026-06/msg03459.html
This is not likely to be a quick task, so this series
starts small
* Adds a "QEMU_DEPRECATIONS" annotation for internal APIs
* Deprecate the QOM instance embedding
* Deprecate the QOM instance properties
* Deprecate the memory region embedding APIs
* Deprecate the IRQ embedding APIs
* Convert PIIX to eliminate embedding as a
demonstration.
The QEMU_DEPRECATIONS idea is an effect to improve our our
historic practice where we introduce a new preferred API
and never really tell anyone the old APIs is bad to use.
If --enable-deprecations is given to configure, every use
of a deprecated API emits a compiler warning. This is not
enabled by default in this series, since we have -Werror
by default.
Note that the proposal to stop embedding memory regions has
been made by Zoltan earlier this year and was rejected by Paolo:
https://lists.nongnu.org/archive/html/qemu-devel/2026-01/msg05435.html
https://lists.nongnu.org/archive/html/qemu-devel/2026-05/msg06665.html
I don't really agree with the analysis there. IMHO, the
concept of embedding objects is to horrendous to allow to
live any longer. Yes, that's a big job, but long term it
is worth it.
Daniel P. Berrangé (7):
meson: add --enable-deprecations configure flag
qom: deprecated embedding object structs within other objects
qom: deprecate use of instance properties
system: add memory_region_new / memory_region_new_io
system: add qemu_irq_new / qemu_irq_new_child / qemu_irq_new_array
hw/isa: convert PIIX embedded QOM objects to heap allocated
qom: improve error message for invalid ID values
hw/core/irq.c | 35 +++++++++++++
hw/isa/piix.c | 65 ++++++++++++++---------
include/hw/core/irq.h | 75 +++++++++++++++++++++++++--
include/hw/southbridge/piix.h | 12 ++---
include/qemu/osdep.h | 19 +++++++
include/qom/object.h | 98 ++++++++++++++++++++++++++++++-----
include/system/memory.h | 76 +++++++++++++++++++++++----
meson.build | 1 +
meson_options.txt | 2 +
qom/object.c | 36 ++++++++++++-
scripts/meson-buildoptions.sh | 3 ++
system/memory.c | 85 +++++++++++++++++++++++++-----
12 files changed, 433 insertions(+), 74 deletions(-)
--
2.54.0
Daniel P. Berrangé <berrange@redhat.com> writes: > QOM has two rather unusual / surprising features historicall > > * The ability to embed a QOM instance's memory inside another > struct > * The ability to register properties against the instnce > instead of struct I figure you mean "against the instance instead of the class". [...]
On Wed, Jun 17, 2026 at 12:59:21PM +0200, Markus Armbruster wrote: > Daniel P. Berrangé <berrange@redhat.com> writes: > > > QOM has two rather unusual / surprising features historicall > > > > * The ability to embed a QOM instance's memory inside another > > struct > > * The ability to register properties against the instnce > > instead of struct > > I figure you mean "against the instance instead of the class". Yes indeed With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
On 16/06/2026 16:55, Daniel P. Berrangé wrote: > QOM has two rather unusual / surprising features historicall > > * The ability to embed a QOM instance's memory inside another > struct > * The ability to register properties against the instnce > instead of struct > > While they both look convenient on the surface, they also > have significant undesirable side effects (see the commit > message for each patch for details). > > The premise of this series is that their convenience does > not outweigh their downsides, and we would be better off > long term by eliminating their usage, rather than trying > to add more hacks on top to mitigate their downsides. > > This comes out of two separate conversations this week > > * Migration series where Peter proposed changes that > make use of instance properties. When I commented, > Peter rightly pointed out that our docs do not > discourage use of instance properties: > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2026-2D06_msg02368.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=v-N9FJZ1ugUjERquAp1KfuZ3B20G9mdbHpEEFP5zkfs&e= > > * QOM series where Akihiko proposed some funky hacks > to reference counting to better track object lifecycle > when embedding structs. We had a short discussion about > discouraging QOM embedding, so this is a real world > proposal: > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2026-2D06_msg03459.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=mHTDgFeywt1U_2Q9uTnAyG6zp-VjguJ4DZIJAX59vaY&e= > > This is not likely to be a quick task, so this series > starts small > > * Adds a "QEMU_DEPRECATIONS" annotation for internal APIs > * Deprecate the QOM instance embedding > * Deprecate the QOM instance properties > * Deprecate the memory region embedding APIs > * Deprecate the IRQ embedding APIs > * Convert PIIX to eliminate embedding as a > demonstration. > > The QEMU_DEPRECATIONS idea is an effect to improve our our > historic practice where we introduce a new preferred API > and never really tell anyone the old APIs is bad to use. > > If --enable-deprecations is given to configure, every use > of a deprecated API emits a compiler warning. This is not > enabled by default in this series, since we have -Werror > by default. I'm not sure if we're ready to start flagging things via the compiler, however I'm in broad agreement that this is direction we need to travel. Thanks for beating me to it, and starting the conversation :) > Note that the proposal to stop embedding memory regions has > been made by Zoltan earlier this year and was rejected by Paolo: > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.nongnu.org_archive_html_qemu-2Ddevel_2026-2D01_msg05435.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=oAY3a1R4rnhYdl7-qnoTVjVdGOX1Qj-GvGlOX77oZrk&e= > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.nongnu.org_archive_html_qemu-2Ddevel_2026-2D05_msg06665.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=WmpukrnrmuhhTz7hLgAA-9FStZ3fbzH3P1aT_U9f6GY&e= > > I don't really agree with the analysis there. IMHO, the > concept of embedding objects is to horrendous to allow to > live any longer. Yes, that's a big job, but long term it > is worth it. FWIW my main objection to the series was that it added yet another way of doing something to fix a particular issue in legacy code. I have a suspicion that if we can make everything in QOM properly refcounted then this approach could work globally: in particular I can see it would fix issues where the flatviews hold references to memory regions outside of the device lifecycle until the RCU thread tidies up. However as you can see, there would be quite a bit of work to get there. But at least if we can all agree on the way forward, we can start to get the relevant changes in place. > Daniel P. Berrangé (7): > meson: add --enable-deprecations configure flag > qom: deprecated embedding object structs within other objects > qom: deprecate use of instance properties > system: add memory_region_new / memory_region_new_io > system: add qemu_irq_new / qemu_irq_new_child / qemu_irq_new_array > hw/isa: convert PIIX embedded QOM objects to heap allocated > qom: improve error message for invalid ID values > > hw/core/irq.c | 35 +++++++++++++ > hw/isa/piix.c | 65 ++++++++++++++--------- > include/hw/core/irq.h | 75 +++++++++++++++++++++++++-- > include/hw/southbridge/piix.h | 12 ++--- > include/qemu/osdep.h | 19 +++++++ > include/qom/object.h | 98 ++++++++++++++++++++++++++++++----- > include/system/memory.h | 76 +++++++++++++++++++++++---- > meson.build | 1 + > meson_options.txt | 2 + > qom/object.c | 36 ++++++++++++- > scripts/meson-buildoptions.sh | 3 ++ > system/memory.c | 85 +++++++++++++++++++++++++----- > 12 files changed, 433 insertions(+), 74 deletions(-) ATB, Mark.
On Wed, 17 Jun 2026, Mark Cave-Ayland wrote: > On 16/06/2026 16:55, Daniel P. Berrangé wrote: > >> QOM has two rather unusual / surprising features historicall >> >> * The ability to embed a QOM instance's memory inside another >> struct >> * The ability to register properties against the instnce >> instead of struct >> >> While they both look convenient on the surface, they also >> have significant undesirable side effects (see the commit >> message for each patch for details). >> >> The premise of this series is that their convenience does >> not outweigh their downsides, and we would be better off >> long term by eliminating their usage, rather than trying >> to add more hacks on top to mitigate their downsides. >> >> This comes out of two separate conversations this week >> >> * Migration series where Peter proposed changes that >> make use of instance properties. When I commented, >> Peter rightly pointed out that our docs do not >> discourage use of instance properties: >> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2026-2D06_msg02368.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=v-N9FJZ1ugUjERquAp1KfuZ3B20G9mdbHpEEFP5zkfs&e= >> >> * QOM series where Akihiko proposed some funky hacks >> to reference counting to better track object lifecycle >> when embedding structs. We had a short discussion about >> discouraging QOM embedding, so this is a real world >> proposal: >> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.gnu.org_archive_html_qemu-2Ddevel_2026-2D06_msg03459.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=mHTDgFeywt1U_2Q9uTnAyG6zp-VjguJ4DZIJAX59vaY&e= >> >> This is not likely to be a quick task, so this series >> starts small >> >> * Adds a "QEMU_DEPRECATIONS" annotation for internal APIs >> * Deprecate the QOM instance embedding >> * Deprecate the QOM instance properties >> * Deprecate the memory region embedding APIs >> * Deprecate the IRQ embedding APIs >> * Convert PIIX to eliminate embedding as a >> demonstration. >> >> The QEMU_DEPRECATIONS idea is an effect to improve our our >> historic practice where we introduce a new preferred API >> and never really tell anyone the old APIs is bad to use. >> >> If --enable-deprecations is given to configure, every use >> of a deprecated API emits a compiler warning. This is not >> enabled by default in this series, since we have -Werror >> by default. > > I'm not sure if we're ready to start flagging things via the compiler, > however I'm in broad agreement that this is direction we need to travel. > Thanks for beating me to it, and starting the conversation :) > >> Note that the proposal to stop embedding memory regions has >> been made by Zoltan earlier this year and was rejected by Paolo: >> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.nongnu.org_archive_html_qemu-2Ddevel_2026-2D01_msg05435.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=oAY3a1R4rnhYdl7-qnoTVjVdGOX1Qj-GvGlOX77oZrk&e= >> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.nongnu.org_archive_html_qemu-2Ddevel_2026-2D05_msg06665.html&d=DwIDaQ&c=s883GpUCOChKOHiocYtGcg&r=c23RpsaH4D2MKyD3EPJTDa0BAxz6tV8aUJqVSoytEiY&m=VzbegkrZ5TGWl9jpxLUfD3gMp4ZZjyUrMiBfOzSjpcaOU-hz3SuUIaGUiIM4BQdV&s=WmpukrnrmuhhTz7hLgAA-9FStZ3fbzH3P1aT_U9f6GY&e= >> >> I don't really agree with the analysis there. IMHO, the >> concept of embedding objects is to horrendous to allow to >> live any longer. Yes, that's a big job, but long term it >> is worth it. > > FWIW my main objection to the series was that it added yet another way of > doing something to fix a particular issue in legacy code. I have a suspicion > that if we can make everything in QOM properly refcounted then this approach > could work globally: in particular I can see it would fix issues where the > flatviews hold references to memory regions outside of the device lifecycle > until the RCU thread tidies up. > > However as you can see, there would be quite a bit of work to get there. But > at least if we can all agree on the way forward, we can start to get the > relevant changes in place. We can't convert the whole code base at once and it's unrealistic to expect a single contributor like me to do that to accept my patch. What I proposed is a start that can be used now for the cases I have and can serve as a basis for future conversions. I did not propose to convert everything either because I think having both ref counted and externally managed memory regions can be useful in different cases and I don't see having two ways for this confusing considering we have object_new and object_initialize for same reasons. But if we deprecate object_initialize then the memory_region_init functions are not needed either. Originally I also thought about splitting alloc and init similar to how macOS Foundation does it (i.e. [[obj alloc] init]) so the allocation of the memory region could be done by either object_new or embedded but that does not work due to object_new setting free function and object_initialize clearing it so you can only use either one or the other. Therefore since memory_region_init calls object_initialize we need a memory_region_new variant that uses object_new instead or we end up having to convert the whole code base again to decouple object initialization from memory_region_init so I did not go that way. Regards, BALATON Zoltan
On Tue, 16 Jun 2026 at 16:56, Daniel P. Berrangé <berrange@redhat.com> wrote: > > QOM has two rather unusual / surprising features historicall > > * The ability to embed a QOM instance's memory inside another > struct > * The ability to register properties against the instnce > instead of struct > > While they both look convenient on the surface, they also > have significant undesirable side effects (see the commit > message for each patch for details). > > The premise of this series is that their convenience does > not outweigh their downsides, and we would be better off > long term by eliminating their usage, rather than trying > to add more hacks on top to mitigate their downsides. The thing I would like to see before we mark object_initialize_child and friends as deprecated is clear documentation of "this is how we would like you to write 'container/SoC' style devices, here is an device written to the approved style you can look at". Currently we have in the codebase a pretty wide range of different ways to write devices: - really ancient, not QOM/qdev at all - qdev style (lots of Device* pointers) - embedded-struct style and I'm not sure if this would be adding a fourth style, or rolling back to qdev style. (Borrowing a paragraph I wrote last time this came up:) I'm not opposed to the idea of making a design decision that this struct-embedding is no longer what we want to do, and defining that something else is our new best practice for how to write devices. But I think we would need to start by reaching a consensus that that *is* what we want to do, and documenting that "best practice" somewhere in docs/devel/. Then we can all be on the same page about the design patterns we want and it will be clearer to reviewers whether new code and new APIs and conversions of old code fit into those patterns or not. I think we're getting closer on the "consensus" part but the "document the new best practice" part is important I think. thanks -- PMM
On 16/06/2026 17:12, Peter Maydell wrote: > On Tue, 16 Jun 2026 at 16:56, Daniel P. Berrangé <berrange@redhat.com> wrote: >> >> QOM has two rather unusual / surprising features historicall >> >> * The ability to embed a QOM instance's memory inside another >> struct >> * The ability to register properties against the instnce >> instead of struct >> >> While they both look convenient on the surface, they also >> have significant undesirable side effects (see the commit >> message for each patch for details). >> >> The premise of this series is that their convenience does >> not outweigh their downsides, and we would be better off >> long term by eliminating their usage, rather than trying >> to add more hacks on top to mitigate their downsides. > > The thing I would like to see before we mark object_initialize_child > and friends as deprecated is clear documentation of "this is how > we would like you to write 'container/SoC' style devices, here is > an device written to the approved style you can look at". > > Currently we have in the codebase a pretty wide range of > different ways to write devices: > - really ancient, not QOM/qdev at all > - qdev style (lots of Device* pointers) > - embedded-struct style > and I'm not sure if this would be adding a fourth style, or > rolling back to qdev style. > > (Borrowing a paragraph I wrote last time this came up:) > > I'm not opposed to the idea of making a design decision that this > struct-embedding is no longer what we want to do, and defining > that something else is our new best practice for how to write devices. > But I think we would need to start by reaching a consensus that that > *is* what we want to do, and documenting that "best practice" somewhere > in docs/devel/. Then we can all be on the same page about the design > patterns we want and it will be clearer to reviewers whether new > code and new APIs and conversions of old code fit into those > patterns or not. > > I think we're getting closer on the "consensus" part but > the "document the new best practice" part is important I think. Agreed. The only issue I can see here is that often documentation isn't good enough: as an example, we've standardised on using "parent_obj" as the field name for several years now, but we still get patches using different names because developers struggle to find the documentation, and reviewers aren't often aware of changes in how we model devices etc. Based on this experience, I think the only way we can realistically do this is to teach checkpatch.pl about it so that using functions such as object_initialize_child(), object_property_add() etc. will cause CI failure. I'd love to be able to teach it about "parent_obj" and not to embed objects that aren't pointers, but I don't know if that's possible? ATB, Mark.
On Wed, Jun 17, 2026 at 11:39:57AM +0100, Mark Cave-Ayland wrote:
> On 16/06/2026 17:12, Peter Maydell wrote:
>
> > On Tue, 16 Jun 2026 at 16:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > QOM has two rather unusual / surprising features historicall
> > >
> > > * The ability to embed a QOM instance's memory inside another
> > > struct
> > > * The ability to register properties against the instnce
> > > instead of struct
> > >
> > > While they both look convenient on the surface, they also
> > > have significant undesirable side effects (see the commit
> > > message for each patch for details).
> > >
> > > The premise of this series is that their convenience does
> > > not outweigh their downsides, and we would be better off
> > > long term by eliminating their usage, rather than trying
> > > to add more hacks on top to mitigate their downsides.
> >
> > The thing I would like to see before we mark object_initialize_child
> > and friends as deprecated is clear documentation of "this is how
> > we would like you to write 'container/SoC' style devices, here is
> > an device written to the approved style you can look at".
> >
> > Currently we have in the codebase a pretty wide range of
> > different ways to write devices:
> > - really ancient, not QOM/qdev at all
> > - qdev style (lots of Device* pointers)
> > - embedded-struct style
> > and I'm not sure if this would be adding a fourth style, or
> > rolling back to qdev style.
> >
> > (Borrowing a paragraph I wrote last time this came up:)
> >
> > I'm not opposed to the idea of making a design decision that this
> > struct-embedding is no longer what we want to do, and defining
> > that something else is our new best practice for how to write devices.
> > But I think we would need to start by reaching a consensus that that
> > *is* what we want to do, and documenting that "best practice" somewhere
> > in docs/devel/. Then we can all be on the same page about the design
> > patterns we want and it will be clearer to reviewers whether new
> > code and new APIs and conversions of old code fit into those
> > patterns or not.
> >
> > I think we're getting closer on the "consensus" part but
> > the "document the new best practice" part is important I think.
>
> Agreed. The only issue I can see here is that often documentation isn't good
> enough: as an example, we've standardised on using "parent_obj" as the field
> name for several years now, but we still get patches using different names
> because developers struggle to find the documentation, and reviewers aren't
> often aware of changes in how we model devices etc.
>
> Based on this experience, I think the only way we can realistically do this
> is to teach checkpatch.pl about it so that using functions such as
> object_initialize_child(), object_property_add() etc. will cause CI failure.
>
> I'd love to be able to teach it about "parent_obj" and not to embed objects
> that aren't pointers, but I don't know if that's possible?
For embedding we can teach checkpatch to whine about any use of
object_initialize() in new code easily enough which catches most
cases.
For 'parent_obj' I thought about a macro assert:
$ git diff
diff --git a/include/qom/object.h b/include/qom/object.h
index e9ce15d595..00d9424e3d 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -283,6 +283,7 @@ struct Object
module_obj_name##_class_init(ObjectClass *oc, const void *data); \
static void \
module_obj_name##_init(Object *obj); \
+ G_STATIC_ASSERT(offsetof(ModuleObjName, parent_obj) == 0); \
\
static const TypeInfo module_obj_name##_info = { \
.parent = TYPE_##PARENT_MODULE_OBJ_NAME, \
Unfortunately only a small number use OBJECT_DEFINE_TYPE macros,
most use DEFINE_TYPES which does not have access to the struct
names :-( Still that does show a few violators of the current
rule.
I also tried the same assert in OBJECT_DECLARE_TYPE which would
catch all usage, but that falls over when the header pre-declares
the struct name, and the source file has its actual definition.
We don't want to force the struct definition into the header
so I'm out of options for build-time checks there :-(
With regards,
Daniel
--
|: https://berrange.com ~~ https://hachyderm.io/@berrange :|
|: https://libvirt.org ~~ https://entangle-photo.org :|
|: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
On Tue, Jun 16, 2026 at 05:12:27PM +0100, Peter Maydell wrote: > On Tue, 16 Jun 2026 at 16:56, Daniel P. Berrangé <berrange@redhat.com> wrote: > > > > QOM has two rather unusual / surprising features historicall > > > > * The ability to embed a QOM instance's memory inside another > > struct > > * The ability to register properties against the instnce > > instead of struct > > > > While they both look convenient on the surface, they also > > have significant undesirable side effects (see the commit > > message for each patch for details). > > > > The premise of this series is that their convenience does > > not outweigh their downsides, and we would be better off > > long term by eliminating their usage, rather than trying > > to add more hacks on top to mitigate their downsides. > > The thing I would like to see before we mark object_initialize_child > and friends as deprecated is clear documentation of "this is how > we would like you to write 'container/SoC' style devices, here is > an device written to the approved style you can look at". Do we have any documentation currently that touches on this area that we can start from / adapt to best practices ? I can do adaptation, but I'm not an expert on Device code, so probably not best placed to write a new doc fro mscratch. > Currently we have in the codebase a pretty wide range of > different ways to write devices: > - really ancient, not QOM/qdev at all > - qdev style (lots of Device* pointers) > - embedded-struct style > and I'm not sure if this would be adding a fourth style, or > rolling back to qdev style. Per my commit message in the PIIX patch, I'm not convinced we need to store any device pointers in the instance structs at all in many (possibly most) cases. The instance struct fields are mostly only referenced during creation time and then never again, with the QOM 'child' property holding the permanent reference. > I'm not opposed to the idea of making a design decision that this > struct-embedding is no longer what we want to do, and defining > that something else is our new best practice for how to write devices. > But I think we would need to start by reaching a consensus that that > *is* what we want to do, and documenting that "best practice" somewhere > in docs/devel/. Then we can all be on the same page about the design > patterns we want and it will be clearer to reviewers whether new > code and new APIs and conversions of old code fit into those > patterns or not. > > I think we're getting closer on the "consensus" part but > the "document the new best practice" part is important I think. Yep, best practice docs must be a key part of the story. With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
On 16/06/2026 17:40, Daniel P. Berrangé wrote: > On Tue, Jun 16, 2026 at 05:12:27PM +0100, Peter Maydell wrote: >> On Tue, 16 Jun 2026 at 16:56, Daniel P. Berrangé <berrange@redhat.com> wrote: >>> >>> QOM has two rather unusual / surprising features historicall >>> >>> * The ability to embed a QOM instance's memory inside another >>> struct >>> * The ability to register properties against the instnce >>> instead of struct >>> >>> While they both look convenient on the surface, they also >>> have significant undesirable side effects (see the commit >>> message for each patch for details). >>> >>> The premise of this series is that their convenience does >>> not outweigh their downsides, and we would be better off >>> long term by eliminating their usage, rather than trying >>> to add more hacks on top to mitigate their downsides. >> >> The thing I would like to see before we mark object_initialize_child >> and friends as deprecated is clear documentation of "this is how >> we would like you to write 'container/SoC' style devices, here is >> an device written to the approved style you can look at". > > Do we have any documentation currently that touches on > this area that we can start from / adapt to best > practices ? I can do adaptation, but I'm not an expert > on Device code, so probably not best placed to write a > new doc fro mscratch. > >> Currently we have in the codebase a pretty wide range of >> different ways to write devices: >> - really ancient, not QOM/qdev at all >> - qdev style (lots of Device* pointers) >> - embedded-struct style >> and I'm not sure if this would be adding a fourth style, or >> rolling back to qdev style. > > Per my commit message in the PIIX patch, I'm not convinced > we need to store any device pointers in the instance > structs at all in many (possibly most) cases. The instance > struct fields are mostly only referenced during creation > time and then never again, with the QOM 'child' property > holding the permanent reference. I think it depends: we do already have hot paths where people use a direct pointer reference instead of an object property lookup, but these cases are generally in the minority. And there's no reason why for those cases the pointer couldn't be cached within the device state during device realize or similar, along with a suitable comment of course. The other case in which I've found device pointers in the device state useful is during low-level debugging sessions i.e. where state is corrupted enough that calling object functions in gdb may fail. >> I'm not opposed to the idea of making a design decision that this >> struct-embedding is no longer what we want to do, and defining >> that something else is our new best practice for how to write devices. >> But I think we would need to start by reaching a consensus that that >> *is* what we want to do, and documenting that "best practice" somewhere >> in docs/devel/. Then we can all be on the same page about the design >> patterns we want and it will be clearer to reviewers whether new >> code and new APIs and conversions of old code fit into those >> patterns or not. >> >> I think we're getting closer on the "consensus" part but >> the "document the new best practice" part is important I think. > > Yep, best practice docs must be a key part of the story. ATB, Mark.
© 2016 - 2026 Red Hat, Inc.