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

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

[Peter Maydell]

The documentation for device_class_set_parent_realize() suggests that
the parent realize function pointer is "usually in the instance
structure".  This is definitely wrong, because the function pointer
is a class thing, and should live in a class struct, not an instance
struct.  Update the documentation to correct this, instead
recommending that it should be placed in the class struct for the
child class.

At time of writing, of the 75 or so uses of this function, about 50
put the parent_realize pointer in the child class struct, and about
25 in the parent class struct.  We recommend the former both because
it is the more common choice, and also because it makes more logical
sense: it's the child that knows (a) that it exists at all and that
(b) it wants to have a realize method that will mean it needs to
chain back to the parent, and we shouldn't need to edit the parent's
class struct because we want to add a child class that chains to its
realize method.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 include/hw/core/qdev.h | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/include/hw/core/qdev.h b/include/hw/core/qdev.h
index 8e91e2fcb08..1330e0153d4 100644
--- a/include/hw/core/qdev.h
+++ b/include/hw/core/qdev.h
@@ -953,9 +953,18 @@ void device_class_set_props_n(DeviceClass *dc, const Property *props, size_t n);
  *
  * This is intended to be used when the new realize function will
  * eventually call its parent realization function during creation.
- * This requires storing the function call somewhere (usually in the
- * instance structure) so you can eventually call
- * dc->parent_realize(dev, errp)
+ * @parent_realize should be where you want to save the pointer to
+ * the parent class realize function; the recommended location is a
+ * field named "parent_realize" in your class's class struct.
+ * (Some existing code puts the field in the parent class's class
+ * struct; this will work but is not the preferred thing for new code.)
+ *
+ * In your realize function you should chain to your parent's
+ * realize function using something like::
+ *
+ *    MyThingClass *mc = MY_THING_GET_CLASS(dev);
+ *    (do your child class realize actions here)
+ *    mc->parent_realize(dev, errp);
  */
 void device_class_set_parent_realize(DeviceClass *dc,
                                      DeviceRealize dev_realize,
-- 
2.43.0

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

[Daniel P. Berrangé]

On Thu, Oct 15, 2026 at 05:33:52PM +0100, Peter Maydell wrote:
> The documentation for device_class_set_parent_realize() suggests that
> the parent realize function pointer is "usually in the instance
> structure".  This is definitely wrong, because the function pointer
> is a class thing, and should live in a class struct, not an instance
> struct.  Update the documentation to correct this, instead
> recommending that it should be placed in the class struct for the
> child class.
> 
> At time of writing, of the 75 or so uses of this function, about 50
> put the parent_realize pointer in the child class struct, and about
> 25 in the parent class struct.  We recommend the former both because
> it is the more common choice, and also because it makes more logical
> sense: it's the child that knows (a) that it exists at all and that
> (b) it wants to have a realize method that will mean it needs to
> chain back to the parent, and we shouldn't need to edit the parent's
> class struct because we want to add a child class that chains to its
> realize method.

I'm confused why we need to store a copy of the parent's
realize method at all, when we can just reference it from
the parent class struct ?

Taking one example

  static void pl050_kbd_realize(DeviceState *dev, Error **errp)
  {
      PL050DeviceClass *pdc = PL050_GET_CLASS(dev);
      PL050KbdState *s = PL050_KBD_DEVICE(dev);
      PL050State *ps = PL050(dev);

      if (!sysbus_realize(SYS_BUS_DEVICE(&s->kbd), errp)) {
          return;
      }

      ps->ps2dev = PS2_DEVICE(&s->kbd);
      pdc->parent_realize(dev, errp);
  }

why wouldn't we rewrite that as

  static void pl050_kbd_realize(DeviceState *dev, Error **errp)
  {
      PL050DeviceClass *pdc = PL050_GET_CLASS(dev);
      DeviceClass *parent = DEVICE_CLASS(object_class_get_parent(OBJECT_CLASS(pdc)));
      PL050KbdState *s = PL050_KBD_DEVICE(dev);
      PL050State *ps = PL050(dev);

      if (!sysbus_realize(SYS_BUS_DEVICE(&s->kbd), errp)) {
          return;
      }

      ps->ps2dev = PS2_DEVICE(&s->kbd);
      parent->realize(dev, errp);
  }

am I missing a reason we need to store the parent's
realize method pointer ?


> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  include/hw/core/qdev.h | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/core/qdev.h b/include/hw/core/qdev.h
> index 8e91e2fcb08..1330e0153d4 100644
> --- a/include/hw/core/qdev.h
> +++ b/include/hw/core/qdev.h
> @@ -953,9 +953,18 @@ void device_class_set_props_n(DeviceClass *dc, const Property *props, size_t n);
>   *
>   * This is intended to be used when the new realize function will
>   * eventually call its parent realization function during creation.
> - * This requires storing the function call somewhere (usually in the
> - * instance structure) so you can eventually call
> - * dc->parent_realize(dev, errp)
> + * @parent_realize should be where you want to save the pointer to
> + * the parent class realize function; the recommended location is a
> + * field named "parent_realize" in your class's class struct.
> + * (Some existing code puts the field in the parent class's class
> + * struct; this will work but is not the preferred thing for new code.)
> + *
> + * In your realize function you should chain to your parent's
> + * realize function using something like::
> + *
> + *    MyThingClass *mc = MY_THING_GET_CLASS(dev);
> + *    (do your child class realize actions here)
> + *    mc->parent_realize(dev, errp);
>   */
>  void device_class_set_parent_realize(DeviceClass *dc,
>                                       DeviceRealize dev_realize,
> -- 
> 2.43.0
> 

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

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

[Peter Maydell]

On Thu, 15 Oct 2026 at 17:44, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Oct 15, 2026 at 05:33:52PM +0100, Peter Maydell wrote:
> > The documentation for device_class_set_parent_realize() suggests that
> > the parent realize function pointer is "usually in the instance
> > structure".  This is definitely wrong, because the function pointer
> > is a class thing, and should live in a class struct, not an instance
> > struct.  Update the documentation to correct this, instead
> > recommending that it should be placed in the class struct for the
> > child class.
> >
> > At time of writing, of the 75 or so uses of this function, about 50
> > put the parent_realize pointer in the child class struct, and about
> > 25 in the parent class struct.  We recommend the former both because
> > it is the more common choice, and also because it makes more logical
> > sense: it's the child that knows (a) that it exists at all and that
> > (b) it wants to have a realize method that will mean it needs to
> > chain back to the parent, and we shouldn't need to edit the parent's
> > class struct because we want to add a child class that chains to its
> > realize method.
>
> I'm confused why we need to store a copy of the parent's
> realize method at all, when we can just reference it from
> the parent class struct ?
>
> Taking one example
>
>   static void pl050_kbd_realize(DeviceState *dev, Error **errp)
>   {
>       PL050DeviceClass *pdc = PL050_GET_CLASS(dev);
>       PL050KbdState *s = PL050_KBD_DEVICE(dev);
>       PL050State *ps = PL050(dev);
>
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->kbd), errp)) {
>           return;
>       }
>
>       ps->ps2dev = PS2_DEVICE(&s->kbd);
>       pdc->parent_realize(dev, errp);
>   }
>
> why wouldn't we rewrite that as
>
>   static void pl050_kbd_realize(DeviceState *dev, Error **errp)
>   {
>       PL050DeviceClass *pdc = PL050_GET_CLASS(dev);
>       DeviceClass *parent = DEVICE_CLASS(object_class_get_parent(OBJECT_CLASS(pdc)));
>       PL050KbdState *s = PL050_KBD_DEVICE(dev);
>       PL050State *ps = PL050(dev);
>
>       if (!sysbus_realize(SYS_BUS_DEVICE(&s->kbd), errp)) {
>           return;
>       }
>
>       ps->ps2dev = PS2_DEVICE(&s->kbd);
>       parent->realize(dev, errp);
>   }
>
> am I missing a reason we need to store the parent's
> realize method pointer ?

Because the parent class's struct doesn't have a pointer to
its realize method. pl050_class_init() does this to set
up its realize method:

    DeviceClass *dc = DEVICE_CLASS(oc);

    dc->realize = pl050_realize;

and that's the only place where pl050_realize is pointed to.

To put the child class's realize method into place we need
to overwrite the DeviceClass::realize field, which means we
need somewhere to put the old value.

We could alternatively solve this by saying:
 (a) every class struct must have a "realize" field,
     and the class init sets both DeviceClass::realize and
     MyClass::realize (perhaps restrict this to only "every
     class that has or might need to have subclasses")
 (b) to chain to the superclass realize, get the MyParentClass
     class pointer and call MyParentClass::realize

The support (or lack thereof) in QOM for methods like realize,
unrealize and reset which should pretty much always chain to
their parent class implementation is pretty terrible, so if
we can come up with something more ergonomic I'd be happy to
throw out the hacks we have today.

thanks
-- PMM

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

[Daniel P. Berrangé]

On Thu, Oct 15, 2026 at 07:16:58PM +0100, Peter Maydell wrote:
> On Thu, 15 Oct 2026 at 17:44, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Thu, Oct 15, 2026 at 05:33:52PM +0100, Peter Maydell wrote:
> > > The documentation for device_class_set_parent_realize() suggests that
> > > the parent realize function pointer is "usually in the instance
> > > structure".  This is definitely wrong, because the function pointer
> > > is a class thing, and should live in a class struct, not an instance
> > > struct.  Update the documentation to correct this, instead
> > > recommending that it should be placed in the class struct for the
> > > child class.
> > >
> > > At time of writing, of the 75 or so uses of this function, about 50
> > > put the parent_realize pointer in the child class struct, and about
> > > 25 in the parent class struct.  We recommend the former both because
> > > it is the more common choice, and also because it makes more logical
> > > sense: it's the child that knows (a) that it exists at all and that
> > > (b) it wants to have a realize method that will mean it needs to
> > > chain back to the parent, and we shouldn't need to edit the parent's
> > > class struct because we want to add a child class that chains to its
> > > realize method.
> >
> > I'm confused why we need to store a copy of the parent's
> > realize method at all, when we can just reference it from
> > the parent class struct ?
> >
> > Taking one example
> >
> >   static void pl050_kbd_realize(DeviceState *dev, Error **errp)
> >   {
> >       PL050DeviceClass *pdc = PL050_GET_CLASS(dev);
> >       PL050KbdState *s = PL050_KBD_DEVICE(dev);
> >       PL050State *ps = PL050(dev);
> >
> >       if (!sysbus_realize(SYS_BUS_DEVICE(&s->kbd), errp)) {
> >           return;
> >       }
> >
> >       ps->ps2dev = PS2_DEVICE(&s->kbd);
> >       pdc->parent_realize(dev, errp);
> >   }
> >
> > why wouldn't we rewrite that as
> >
> >   static void pl050_kbd_realize(DeviceState *dev, Error **errp)
> >   {
> >       PL050DeviceClass *pdc = PL050_GET_CLASS(dev);
> >       DeviceClass *parent = DEVICE_CLASS(object_class_get_parent(OBJECT_CLASS(pdc)));
> >       PL050KbdState *s = PL050_KBD_DEVICE(dev);
> >       PL050State *ps = PL050(dev);
> >
> >       if (!sysbus_realize(SYS_BUS_DEVICE(&s->kbd), errp)) {
> >           return;
> >       }
> >
> >       ps->ps2dev = PS2_DEVICE(&s->kbd);
> >       parent->realize(dev, errp);
> >   }
> >
> > am I missing a reason we need to store the parent's
> > realize method pointer ?
> 
> Because the parent class's struct doesn't have a pointer to
> its realize method. pl050_class_init() does this to set
> up its realize method:
> 
>     DeviceClass *dc = DEVICE_CLASS(oc);
> 
>     dc->realize = pl050_realize;
> 
> and that's the only place where pl050_realize is pointed to.
> 
> To put the child class's realize method into place we need
> to overwrite the DeviceClass::realize field, which means we
> need somewhere to put the old value.

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>

> We could alternatively solve this by saying:
>  (a) every class struct must have a "realize" field,
>      and the class init sets both DeviceClass::realize and
>      MyClass::realize (perhaps restrict this to only "every
>      class that has or might need to have subclasses")
>  (b) to chain to the superclass realize, get the MyParentClass
>      class pointer and call MyParentClass::realize
> 
> The support (or lack thereof) in QOM for methods like realize,
> unrealize and reset which should pretty much always chain to
> their parent class implementation is pretty terrible, so if
> we can come up with something more ergonomic I'd be happy to
> throw out the hacks we have today.

AFAICT, we can already do everything we need with the parent class
to chain up parent virtual methods without having  to keep extra
pointers around.

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

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

[Daniel P. Berrangé]

On Fri, Oct 16, 2026 at 10:15:01AM +0100, Daniel P. Berrangé wrote:
> On Thu, Oct 15, 2026 at 07:16:58PM +0100, Peter Maydell wrote:
> > On Thu, 15 Oct 2026 at 17:44, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > >
> > > On Thu, Oct 15, 2026 at 05:33:52PM +0100, Peter Maydell wrote:
> > > > The documentation for device_class_set_parent_realize() suggests that
> > > > the parent realize function pointer is "usually in the instance
> > > > structure".  This is definitely wrong, because the function pointer
> > > > is a class thing, and should live in a class struct, not an instance
> > > > struct.  Update the documentation to correct this, instead
> > > > recommending that it should be placed in the class struct for the
> > > > child class.
> > > >
> > > > At time of writing, of the 75 or so uses of this function, about 50
> > > > put the parent_realize pointer in the child class struct, and about
> > > > 25 in the parent class struct.  We recommend the former both because
> > > > it is the more common choice, and also because it makes more logical
> > > > sense: it's the child that knows (a) that it exists at all and that
> > > > (b) it wants to have a realize method that will mean it needs to
> > > > chain back to the parent, and we shouldn't need to edit the parent's
> > > > class struct because we want to add a child class that chains to its
> > > > realize method.
> > >
> > > I'm confused why we need to store a copy of the parent's
> > > realize method at all, when we can just reference it from
> > > the parent class struct ?
> > >
> > > Taking one example
> > >
> > >   static void pl050_kbd_realize(DeviceState *dev, Error **errp)
> > >   {
> > >       PL050DeviceClass *pdc = PL050_GET_CLASS(dev);
> > >       PL050KbdState *s = PL050_KBD_DEVICE(dev);
> > >       PL050State *ps = PL050(dev);
> > >
> > >       if (!sysbus_realize(SYS_BUS_DEVICE(&s->kbd), errp)) {
> > >           return;
> > >       }
> > >
> > >       ps->ps2dev = PS2_DEVICE(&s->kbd);
> > >       pdc->parent_realize(dev, errp);
> > >   }
> > >
> > > why wouldn't we rewrite that as
> > >
> > >   static void pl050_kbd_realize(DeviceState *dev, Error **errp)
> > >   {
> > >       PL050DeviceClass *pdc = PL050_GET_CLASS(dev);
> > >       DeviceClass *parent = DEVICE_CLASS(object_class_get_parent(OBJECT_CLASS(pdc)));
> > >       PL050KbdState *s = PL050_KBD_DEVICE(dev);
> > >       PL050State *ps = PL050(dev);
> > >
> > >       if (!sysbus_realize(SYS_BUS_DEVICE(&s->kbd), errp)) {
> > >           return;
> > >       }
> > >
> > >       ps->ps2dev = PS2_DEVICE(&s->kbd);
> > >       parent->realize(dev, errp);
> > >   }
> > >
> > > am I missing a reason we need to store the parent's
> > > realize method pointer ?
> > 
> > Because the parent class's struct doesn't have a pointer to
> > its realize method. pl050_class_init() does this to set
> > up its realize method:
> > 
> >     DeviceClass *dc = DEVICE_CLASS(oc);
> > 
> >     dc->realize = pl050_realize;
> > 
> > and that's the only place where pl050_realize is pointed to.
> > 
> > To put the child class's realize method into place we need
> > to overwrite the DeviceClass::realize field, which means we
> > need somewhere to put the old value.
> 
> 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>
> 
> > We could alternatively solve this by saying:
> >  (a) every class struct must have a "realize" field,
> >      and the class init sets both DeviceClass::realize and
> >      MyClass::realize (perhaps restrict this to only "every
> >      class that has or might need to have subclasses")
> >  (b) to chain to the superclass realize, get the MyParentClass
> >      class pointer and call MyParentClass::realize
> > 
> > The support (or lack thereof) in QOM for methods like realize,
> > unrealize and reset which should pretty much always chain to
> > their parent class implementation is pretty terrible, so if
> > we can come up with something more ergonomic I'd be happy to
> > throw out the hacks we have today.
> 
> AFAICT, we can already do everything we need with the parent class
> to chain up parent virtual methods without having  to keep extra
> pointers around.

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)
+
 /**
  * qdev_realize_and_unref: Realize @dev and drop a reference
  * @dev: device to realize


which would be used thus:


diff --git a/hw/input/pl050.c b/hw/input/pl050.c
index 48df2734e26..abe939171f2 100644
--- a/hw/input/pl050.c
+++ b/hw/input/pl050.c
@@ -168,7 +168,7 @@ static void pl050_kbd_realize(DeviceState *dev, Error **errp)
     }
 
     ps->ps2dev = PS2_DEVICE(&s->kbd);
-    pdc->parent_realize(dev, errp);
+    qdev_realize_chain_parent(dev, pdc, errp);
 }
 
 static void pl050_kbd_init(Object *obj)
@@ -191,7 +191,8 @@ static void pl050_mouse_realize(DeviceState *dev, Error **errp)
     }
 
     ps->ps2dev = PS2_DEVICE(&s->mouse);
-    pdc->parent_realize(dev, errp);
+
+    qdev_realize_chain_parent(dev, pdc, errp);
 }
 
 static void pl050_mouse_init(Object *obj)
@@ -206,10 +207,8 @@ static void pl050_mouse_init(Object *obj)
 static void pl050_kbd_class_init(ObjectClass *oc, const void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
-    PL050DeviceClass *pdc = PL050_CLASS(oc);
 
-    device_class_set_parent_realize(dc, pl050_kbd_realize,
-                                    &pdc->parent_realize);
+    dc->realize = pl050_kbd_realize;
 }
 
 static const TypeInfo pl050_kbd_info = {
@@ -223,10 +222,8 @@ static const TypeInfo pl050_kbd_info = {
 static void pl050_mouse_class_init(ObjectClass *oc, const void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
-    PL050DeviceClass *pdc = PL050_CLASS(oc);
 
-    device_class_set_parent_realize(dc, pl050_mouse_realize,
-                                    &pdc->parent_realize);
+    dc->realize = pl050_mouse_realize;
 }
 
 static const TypeInfo pl050_mouse_info = {
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;
 };
 
 #define TYPE_PL050 "pl050"



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

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

[Peter Maydell]

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?

> +
>  /**
>   * qdev_realize_and_unref: Realize @dev and drop a reference
>   * @dev: device to realize
>
>
> which would be used thus:
>
>
> diff --git a/hw/input/pl050.c b/hw/input/pl050.c
> index 48df2734e26..abe939171f2 100644
> --- a/hw/input/pl050.c
> +++ b/hw/input/pl050.c
> @@ -168,7 +168,7 @@ static void pl050_kbd_realize(DeviceState *dev, Error **errp)
>      }
>
>      ps->ps2dev = PS2_DEVICE(&s->kbd);
> -    pdc->parent_realize(dev, errp);
> +    qdev_realize_chain_parent(dev, pdc, errp);
>  }
>
>  static void pl050_kbd_init(Object *obj)
> @@ -191,7 +191,8 @@ static void pl050_mouse_realize(DeviceState *dev, Error **errp)
>      }
>
>      ps->ps2dev = PS2_DEVICE(&s->mouse);
> -    pdc->parent_realize(dev, errp);
> +
> +    qdev_realize_chain_parent(dev, pdc, errp);
>  }
>
>  static void pl050_mouse_init(Object *obj)
> @@ -206,10 +207,8 @@ static void pl050_mouse_init(Object *obj)
>  static void pl050_kbd_class_init(ObjectClass *oc, const void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
> -    PL050DeviceClass *pdc = PL050_CLASS(oc);
>
> -    device_class_set_parent_realize(dc, pl050_kbd_realize,
> -                                    &pdc->parent_realize);
> +    dc->realize = pl050_kbd_realize;
>  }
>
>  static const TypeInfo pl050_kbd_info = {
> @@ -223,10 +222,8 @@ static const TypeInfo pl050_kbd_info = {
>  static void pl050_mouse_class_init(ObjectClass *oc, const void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(oc);
> -    PL050DeviceClass *pdc = PL050_CLASS(oc);
>
> -    device_class_set_parent_realize(dc, pl050_mouse_realize,
> -                                    &pdc->parent_realize);
> +    dc->realize = pl050_mouse_realize;
>  }
>
>  static const TypeInfo pl050_mouse_info = {
> 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.

thanks
-- PMM

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

[Daniel P. Berrangé]

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

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

[Peter Maydell]

On Fri, 16 Oct 2026 at 11:53, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> 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:

> > > +#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'.

Isn't that always what you want, though? In a class hierarchy
Object->Device->A->B->C, C's realize method always wants to
chain to B's realize method, and more generally "invoke the
superclass method" from C wants B's implementation, not A's
(assuming B actually implemented it).

> 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);

I still don't understand why we need to get the parent's
object class by name here, rather than saying "give me
the parent class of this object's class".

thanks
-- PMM

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

[Daniel P. Berrangé]

On Fri, Oct 16, 2026 at 03:09:55PM +0100, Peter Maydell wrote:
> On Fri, 16 Oct 2026 at 11:53, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > 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:
> 
> > > > +#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'.
> 
> Isn't that always what you want, though? In a class hierarchy
> Object->Device->A->B->C, C's realize method always wants to
> chain to B's realize method, and more generally "invoke the
> superclass method" from C wants B's implementation, not A's
> (assuming B actually implemented it).

Yes, C wants to invoke B's realize method, but then B also
wants to invoke A's realize method.

> > 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);
> 
> I still don't understand why we need to get the parent's
> object class by name here, rather than saying "give me
> the parent class of this object's class".

No matter which realize() method we are executing in, the "dev"
object parameter will always be an instance of the leaf class "C"
and thus the result of object_class_get_parent(objct_get_class(dev))
will always be B. That is only correct if executgnig "C"s realize
method, so breaks if we need to chain up realize many times.

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

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

[Peter Maydell]

On Fri, 16 Oct 2026 at 15:26, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Fri, Oct 16, 2026 at 03:09:55PM +0100, Peter Maydell wrote:
> > On Fri, 16 Oct 2026 at 11:53, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > > 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);
> >
> > I still don't understand why we need to get the parent's
> > object class by name here, rather than saying "give me
> > the parent class of this object's class".
>
> No matter which realize() method we are executing in, the "dev"
> object parameter will always be an instance of the leaf class "C"
> and thus the result of object_class_get_parent(objct_get_class(dev))
> will always be B. That is only correct if executgnig "C"s realize
> method, so breaks if we need to chain up realize many times.

Ah, thanks -- I finally get it.

-- PMM

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

[Mark Cave-Ayland]

On 16/10/2026 11:14, 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)

Even better! So then we could just do:

   qdev_realize_chain_parent(dev, errp);

in the child class? I can see this would make adapting existing users to 
make use of the new macro really easy.
> 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?

I'm certainly leaning towards a simple-is-better approach from the ideas 
being suggested here.

>> +
>>   /**
>>    * qdev_realize_and_unref: Realize @dev and drop a reference
>>    * @dev: device to realize
>>
>>
>> which would be used thus:
>>
>>
>> diff --git a/hw/input/pl050.c b/hw/input/pl050.c
>> index 48df2734e26..abe939171f2 100644
>> --- a/hw/input/pl050.c
>> +++ b/hw/input/pl050.c
>> @@ -168,7 +168,7 @@ static void pl050_kbd_realize(DeviceState *dev, Error **errp)
>>       }
>>
>>       ps->ps2dev = PS2_DEVICE(&s->kbd);
>> -    pdc->parent_realize(dev, errp);
>> +    qdev_realize_chain_parent(dev, pdc, errp);
>>   }
>>
>>   static void pl050_kbd_init(Object *obj)
>> @@ -191,7 +191,8 @@ static void pl050_mouse_realize(DeviceState *dev, Error **errp)
>>       }
>>
>>       ps->ps2dev = PS2_DEVICE(&s->mouse);
>> -    pdc->parent_realize(dev, errp);
>> +
>> +    qdev_realize_chain_parent(dev, pdc, errp);
>>   }
>>
>>   static void pl050_mouse_init(Object *obj)
>> @@ -206,10 +207,8 @@ static void pl050_mouse_init(Object *obj)
>>   static void pl050_kbd_class_init(ObjectClass *oc, const void *data)
>>   {
>>       DeviceClass *dc = DEVICE_CLASS(oc);
>> -    PL050DeviceClass *pdc = PL050_CLASS(oc);
>>
>> -    device_class_set_parent_realize(dc, pl050_kbd_realize,
>> -                                    &pdc->parent_realize);
>> +    dc->realize = pl050_kbd_realize;
>>   }
>>
>>   static const TypeInfo pl050_kbd_info = {
>> @@ -223,10 +222,8 @@ static const TypeInfo pl050_kbd_info = {
>>   static void pl050_mouse_class_init(ObjectClass *oc, const void *data)
>>   {
>>       DeviceClass *dc = DEVICE_CLASS(oc);
>> -    PL050DeviceClass *pdc = PL050_CLASS(oc);
>>
>> -    device_class_set_parent_realize(dc, pl050_mouse_realize,
>> -                                    &pdc->parent_realize);
>> +    dc->realize = pl050_mouse_realize;
>>   }
>>
>>   static const TypeInfo pl050_mouse_info = {
>> 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.

ATB,

Mark.

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

[Mark Cave-Ayland]

On 16/10/2026 10:30, Daniel P. Berrangé wrote:

> 
> On Fri, Oct 16, 2026 at 10:15:01AM +0100, Daniel P. Berrangé wrote:
>> On Thu, Oct 15, 2026 at 07:16:58PM +0100, Peter Maydell wrote:
>>> On Thu, 15 Oct 2026 at 17:44, Daniel P. Berrangé <berrange@redhat.com> wrote:
>>>>
>>>> On Thu, Oct 15, 2026 at 05:33:52PM +0100, Peter Maydell wrote:
>>>>> The documentation for device_class_set_parent_realize() suggests that
>>>>> the parent realize function pointer is "usually in the instance
>>>>> structure".  This is definitely wrong, because the function pointer
>>>>> is a class thing, and should live in a class struct, not an instance
>>>>> struct.  Update the documentation to correct this, instead
>>>>> recommending that it should be placed in the class struct for the
>>>>> child class.
>>>>>
>>>>> At time of writing, of the 75 or so uses of this function, about 50
>>>>> put the parent_realize pointer in the child class struct, and about
>>>>> 25 in the parent class struct.  We recommend the former both because
>>>>> it is the more common choice, and also because it makes more logical
>>>>> sense: it's the child that knows (a) that it exists at all and that
>>>>> (b) it wants to have a realize method that will mean it needs to
>>>>> chain back to the parent, and we shouldn't need to edit the parent's
>>>>> class struct because we want to add a child class that chains to its
>>>>> realize method.
>>>>
>>>> I'm confused why we need to store a copy of the parent's
>>>> realize method at all, when we can just reference it from
>>>> the parent class struct ?
>>>>
>>>> Taking one example
>>>>
>>>>    static void pl050_kbd_realize(DeviceState *dev, Error **errp)
>>>>    {
>>>>        PL050DeviceClass *pdc = PL050_GET_CLASS(dev);
>>>>        PL050KbdState *s = PL050_KBD_DEVICE(dev);
>>>>        PL050State *ps = PL050(dev);
>>>>
>>>>        if (!sysbus_realize(SYS_BUS_DEVICE(&s->kbd), errp)) {
>>>>            return;
>>>>        }
>>>>
>>>>        ps->ps2dev = PS2_DEVICE(&s->kbd);
>>>>        pdc->parent_realize(dev, errp);
>>>>    }
>>>>
>>>> why wouldn't we rewrite that as
>>>>
>>>>    static void pl050_kbd_realize(DeviceState *dev, Error **errp)
>>>>    {
>>>>        PL050DeviceClass *pdc = PL050_GET_CLASS(dev);
>>>>        DeviceClass *parent = DEVICE_CLASS(object_class_get_parent(OBJECT_CLASS(pdc)));
>>>>        PL050KbdState *s = PL050_KBD_DEVICE(dev);
>>>>        PL050State *ps = PL050(dev);
>>>>
>>>>        if (!sysbus_realize(SYS_BUS_DEVICE(&s->kbd), errp)) {
>>>>            return;
>>>>        }
>>>>
>>>>        ps->ps2dev = PS2_DEVICE(&s->kbd);
>>>>        parent->realize(dev, errp);
>>>>    }
>>>>
>>>> am I missing a reason we need to store the parent's
>>>> realize method pointer ?
>>>
>>> Because the parent class's struct doesn't have a pointer to
>>> its realize method. pl050_class_init() does this to set
>>> up its realize method:
>>>
>>>      DeviceClass *dc = DEVICE_CLASS(oc);
>>>
>>>      dc->realize = pl050_realize;
>>>
>>> and that's the only place where pl050_realize is pointed to.
>>>
>>> To put the child class's realize method into place we need
>>> to overwrite the DeviceClass::realize field, which means we
>>> need somewhere to put the old value.
>>
>> 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>
>>
>>> We could alternatively solve this by saying:
>>>   (a) every class struct must have a "realize" field,
>>>       and the class init sets both DeviceClass::realize and
>>>       MyClass::realize (perhaps restrict this to only "every
>>>       class that has or might need to have subclasses")
>>>   (b) to chain to the superclass realize, get the MyParentClass
>>>       class pointer and call MyParentClass::realize
>>>
>>> The support (or lack thereof) in QOM for methods like realize,
>>> unrealize and reset which should pretty much always chain to
>>> their parent class implementation is pretty terrible, so if
>>> we can come up with something more ergonomic I'd be happy to
>>> throw out the hacks we have today.
>>
>> AFAICT, we can already do everything we need with the parent class
>> to chain up parent virtual methods without having  to keep extra
>> pointers around.
> 
> 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)
> +
>   /**
>    * qdev_realize_and_unref: Realize @dev and drop a reference
>    * @dev: device to realize
> 
> 
> which would be used thus:
> 
> 
> diff --git a/hw/input/pl050.c b/hw/input/pl050.c
> index 48df2734e26..abe939171f2 100644
> --- a/hw/input/pl050.c
> +++ b/hw/input/pl050.c
> @@ -168,7 +168,7 @@ static void pl050_kbd_realize(DeviceState *dev, Error **errp)
>       }
>   
>       ps->ps2dev = PS2_DEVICE(&s->kbd);
> -    pdc->parent_realize(dev, errp);
> +    qdev_realize_chain_parent(dev, pdc, errp);
>   }
>   
>   static void pl050_kbd_init(Object *obj)
> @@ -191,7 +191,8 @@ static void pl050_mouse_realize(DeviceState *dev, Error **errp)
>       }
>   
>       ps->ps2dev = PS2_DEVICE(&s->mouse);
> -    pdc->parent_realize(dev, errp);
> +
> +    qdev_realize_chain_parent(dev, pdc, errp);
>   }
>   
>   static void pl050_mouse_init(Object *obj)
> @@ -206,10 +207,8 @@ static void pl050_mouse_init(Object *obj)
>   static void pl050_kbd_class_init(ObjectClass *oc, const void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(oc);
> -    PL050DeviceClass *pdc = PL050_CLASS(oc);
>   
> -    device_class_set_parent_realize(dc, pl050_kbd_realize,
> -                                    &pdc->parent_realize);
> +    dc->realize = pl050_kbd_realize;
>   }
>   
>   static const TypeInfo pl050_kbd_info = {
> @@ -223,10 +222,8 @@ static const TypeInfo pl050_kbd_info = {
>   static void pl050_mouse_class_init(ObjectClass *oc, const void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(oc);
> -    PL050DeviceClass *pdc = PL050_CLASS(oc);
>   
> -    device_class_set_parent_realize(dc, pl050_mouse_realize,
> -                                    &pdc->parent_realize);
> +    dc->realize = pl050_mouse_realize;
>   }
>   
>   static const TypeInfo pl050_mouse_info = {
> 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;
>   };
>   
>   #define TYPE_PL050 "pl050"
Oh that's nice! I think we could come up with something better regarding 
the name (maybe just qdev_parent_realize()?), but this approach makes 
things considerably easier :)


ATB,

Mark.

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

[Mark Cave-Ayland]

On 15/10/2026 17:33, Peter Maydell wrote:

> The documentation for device_class_set_parent_realize() suggests that
> the parent realize function pointer is "usually in the instance
> structure".  This is definitely wrong, because the function pointer
> is a class thing, and should live in a class struct, not an instance
> struct.  Update the documentation to correct this, instead
> recommending that it should be placed in the class struct for the
> child class.
> 
> At time of writing, of the 75 or so uses of this function, about 50
> put the parent_realize pointer in the child class struct, and about
> 25 in the parent class struct.  We recommend the former both because
> it is the more common choice, and also because it makes more logical
> sense: it's the child that knows (a) that it exists at all and that
> (b) it wants to have a realize method that will mean it needs to
> chain back to the parent, and we shouldn't need to edit the parent's
> class struct because we want to add a child class that chains to its
> realize method.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   include/hw/core/qdev.h | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/core/qdev.h b/include/hw/core/qdev.h
> index 8e91e2fcb08..1330e0153d4 100644
> --- a/include/hw/core/qdev.h
> +++ b/include/hw/core/qdev.h
> @@ -953,9 +953,18 @@ void device_class_set_props_n(DeviceClass *dc, const Property *props, size_t n);
>    *
>    * This is intended to be used when the new realize function will
>    * eventually call its parent realization function during creation.
> - * This requires storing the function call somewhere (usually in the
> - * instance structure) so you can eventually call
> - * dc->parent_realize(dev, errp)
> + * @parent_realize should be where you want to save the pointer to
> + * the parent class realize function; the recommended location is a

Do we need something stronger than "recommended location" if we want to 
encourage people to always do it this way in future?

> + * field named "parent_realize" in your class's class struct.
> + * (Some existing code puts the field in the parent class's class
> + * struct; this will work but is not the preferred thing for new code.)
> + *
> + * In your realize function you should chain to your parent's
> + * realize function using something like::
> + *
> + *    MyThingClass *mc = MY_THING_GET_CLASS(dev);
> + *    (do your child class realize actions here)
> + *    mc->parent_realize(dev, errp);
>    */
>   void device_class_set_parent_realize(DeviceClass *dc,
>                                        DeviceRealize dev_realize,

Otherwise:

Reviewed-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>


ATB,

Mark.