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