On 16/10/2026 11:14, Peter Maydell wrote:
On Fri, 16 Oct 2026 at 10:31, Daniel P. Berrangé <[email protected]> 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.
