Re: [PATCH 05/22] i8259.c: move parent_realize from PICClass to I8259CommonClass

[Mark Cave-Ayland <mark.caveayland@xxxxxxxxxxx>]

On 14/10/2026 12:00, Mark Cave-Ayland wrote:

> On 13/10/2026 16:54, Peter Maydell wrote:
>
> > On Wed, 16 Sept 2026 at 11:49, Mark Cave-Ayland
> > <mark.caveayland@xxxxxxxxxxx> wrote:
> > > Both the emulated and KVM i8259 implementations store a reference to the
> > > parent_realize function in their respective classes. Start by moving
> > > parent_realize from PICClass to I8259CommonClass and removing 
> > > PICClass since
> > > it is no longer required.
> > >
> > > Signed-off-by: Mark Cave-Ayland <mark.caveayland@xxxxxxxxxxx>
> > This is one of those things where I think we're not 100% consistent
> > about whether we think the parent_realize field should go in the
> > child class struct or the parent class struct. The name "parent_realize"
> > implies it ought to be in the child struct, though, and I think we
> > do that more often than we put it in the parent class. This also
> > makes some kind of separation-of-concerns sense too: 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.
> >
> > The docs for device_class_set_parent_realize() claim the parent
> > realize ptr is stored "usually in the instance structure" which
> > I think is just completely wrong.
> >
> > I could be persuaded otherwise. As usual with QOM/qdev, I'm
> > mostly interested in (a) consistency and (b) documenting what the
> > "right thing" is. Realize, reset and unrealize are the three
> > methods where QOM's lack of convenient support for "child
> > implementation of method will need to chain to parent impl"
> > is most awkward.
> Yeah it's a tricky one. This patch was really just refactoring what was 
> already there, but I'm open to moving it into the instance if you think 
> that is better. I agree with your point about the child instance being 
> the one that knows it needs to chain back to the parent.
> I'll have an experiment and if it looks good, I'll move it to the 
> instance in v2.
I did a search over the codebase and checked all references to 
DeviceRealize, and they are all stored in QOM classes and make use of 
device_class_set_parent_realize(). So for now I think we can say it is 
the documentation for device_class_set_parent_realize() that is wrong, 
and I will leave it this way in v2 of the series.

ATB,

Mark.