On Wed, 14 Oct 2026 at 14:58, Mark Cave-Ayland <[email protected]> wrote: > > 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 > >> <[email protected]> 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 <[email protected]> > >> > >> 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.
Yes, the *instance* part is definitely wrong. But there's still "parent class struct" vs "child class struct". This patch puts it in the parent class struct, and as I say above I kind of prefer child class struct and think we do that the most often. -- PMM
