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é <[email protected]>
> > 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 :|
