Re: [PATCH] include/hw/core/qdev.h: Fix where device_class_set_parent_realize docs suggest putting parent_realize

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Oct 16, 2026 at 11:14:16AM +0100, Peter Maydell wrote:
> On Fri, 16 Oct 2026 at 10:31, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Fri, Oct 16, 2026 at 10:15:01AM +0100, Daniel P. Berrangé wrote:
> > > We're overwriting the DeviceClass::realize field in the class
> > > struct instance for "pl050_keyboard". We still have the completely
> > > independent class struct instance for "pl050" (which is what
> > > object_class_get_parent() returns) that contains the 'realize'
> > > field value set by the parent class.
> > >
> > > eg if I put a break on pl050_kbd_realize():
> > >
> > >   (gdb) print pdc->parent_class->parent_class->parent_class->type->name
> > >   $5 = 0x555557de40a0 "pl050_keyboard"
> > >   (gdb) print pdc->parent_class->parent_class->realize
> > >   $6 = (DeviceRealize) 0x55555585db67 <pl050_kbd_realize>
> > >
> > > and separately
> > >
> > >   (gdb) print ((DeviceClass*)(object_class_get_parent(pdc)))->parent_class->type->name
> > >   $7 = 0x555557de3ee0 "pl050"
> > >   (gdb) print ((DeviceClass*)(object_class_get_parent(pdc)))->realize
> > >   $8 = (DeviceRealize) 0x55555585daf4 <pl050_realize>
> 
> Ah. I didn't realize you could get at the parent's class
> struct like this.
> 
> > We can make it a little less ugly in the callers with a macro in
> > qdev.h thus:
> >
> > diff --git a/include/hw/core/qdev.h b/include/hw/core/qdev.h
> > index 8e91e2fcb08..18e85850c31 100644
> > --- a/include/hw/core/qdev.h
> > +++ b/include/hw/core/qdev.h
> > @@ -461,6 +461,9 @@ static inline bool qdev_is_realized(const DeviceState *dev)
> >   */
> >  bool qdev_realize(DeviceState *dev, BusState *bus, Error **errp);
> >
> > +#define qdev_realize_chain_parent(dev, dc, errp) \
> > +    DEVICE_CLASS(object_class_get_parent(OBJECT_CLASS(dc)))->realize(dev, errp)
> 
> I'm tempted to suggest that we drop the 'dc' argument here and
> have the macro get the class from the dev:
>     DEVICE_CLASS(object_class_get_parent(object_get_class(OBJECT(dev))))->realize(dev,
> errp)
> 
> because I think almost all realize methods wanting to chain back up
> won't have the class pointer to hand.
>
> We could maybe have an extra layer of abstraction here such that
> you write something like:
> 
>    parent_invoke(DEVICE_CLASS, dev)->realize(dev, errp);
> 
> Or is that trying to be too clever?

This won't work if we have multiple levels in the class hierarchy
that need chaining up, as object_class_get_parent(object_get_class(dev))
will always just get the first level parent class of 'dev'.

Somewhere/how we need to identify the current class level we're
calling from, so we know what parent to ask for.

Another option is to do what GObject does and provide a simple
helper for getting the parent class, from OBJECT_DEFINE_TYPE*
macros:

    ...
    static inline ObjectClass * \
    module_obj_name##_parent_class() { \
        return object_class_by_name(TYPE_##PARENT_MODULE_OBJ_NAME); \
    } \
    ...

which would be used

    DeviceClass *dc = DEVICE_CLASS(pl050_kbd_parent_class());

    dc->realize(dev, errp);

but many (most) of qdev impls don't use the relatively new
OBJECT_DEFINE_TYPE macros. They would have to instead explicitly
reference the parent type name:

    DeviceClass *dc = DEVICE_CLASS(object_class_by_name(TYPE_PL050));
    dc->realize(dev, errp);

which isn't too terrible.

> > diff --git a/include/hw/input/pl050.h b/include/hw/input/pl050.h
> > index 2cdfa74f73c..7169afc151f 100644
> > --- a/include/hw/input/pl050.h
> > +++ b/include/hw/input/pl050.h
> > @@ -17,8 +17,6 @@
> >
> >  struct PL050DeviceClass {
> >      SysBusDeviceClass parent_class;
> > -
> > -    DeviceRealize parent_realize;
> >  };
> 
> Yes, this is clearly vastly better than we have at the moment.
> device_class_set_parent_unrealize() can do similarly.
> 
> Does something similar work when QOM interfaces are involved?
> We have resettable_class_set_parent_phases() which deals with
> updating the methods of ResettableClass, which is an interface
> declared by TYPE_DEVICE.

Yes, each QOM interface has a ObjectClass associated with it,
so you can chase the parent class links upto ResettableClass.

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 :|