On Wed, Jun 17, 2026 at 02:49:57PM +0200, Paolo Bonzini wrote: > On 6/16/26 17:55, Daniel P. Berrangé wrote: > > Prepare for the move to dynamically allocated memory regions by > > introducing memory_region_new and memory_region_new_io functions > > which call through to object_new instead of object_initialize. > > > > TBD: add "new" variants for all the other memory_region_init > > variants. > > > > Signed-off-by: Daniel P. Berrangé <[email protected]> > > Zoltan already proposed this, but I really think it's a bad idea. > > MemoryRegionOps callback will certainly access fields in the parent. Having > a separate object gives you no benefit because you still have the same > use-after-free if the MemoryRegion outlives the device. Likewise for buses.
I see the opposite rationale. We needlessly have two ways of allocating QOM objects. The embedding does not magically keep the child object alive, and it introduces confusion and unexpected behaviour as ref counts don't work. > For buses, the fact that the parent and child live at the same time is > explicitly tracked with mutual refcount and unparent. If we get mutual ref counting correct, then there is not benefit to having 2 different ways of allocating QOM objects. We should eliminate the embedding as pointless extra complexity and focus on getting ref counting done correctly. > If it's not, we have > the hack that Akihiko mentioned > (https://lists.gnu.org/archive/html/qemu-devel/2026-06/msg03459.html): That hack is too gross. > > However, this is insufficient to avoid calling object_ref() for all > > embedded objects. For example, consider an embedded Device that has a > > MemoryRegion. When referencing a MemoryRegion for guest memory access, > > QEMU automatically references the owning Device to keep the MemoryRegion > > alive. However, that reference is ineffective if the Device itself is > > embedded, because object_ref() does not keep the containing storage > > alive. > > If the device itself is embedded, then all the invariants must hold > recursively: > > - the device must be kept alive by mutual references between the device > itself and its parent; > > - unparent for the device should wait for all guest memory accesses to be > either completed or cancelled. > > So, the problem is that "QEMU automatically references the owning Device to > keep the MemoryRegion alive" is a hack that should die. I added it, mea > culpa. > > The model is that if it makes sense to have B outlive A, you use new. If it > doesn't, you use init. For MemoryRegions or anything that has callbacks, it > makes no sense to outlive the implementor of the callbacks. There are > exceptions if the number of objects is dynamic but they're very rare. > > I agree that there's extra complexity added by embedding; but I disagree > that removing embedding will fix any bugs, and because it will hide a > relationship between objects, I believe the complexity is not accidental. The relationship between objects does not need to be expressed with embedding one inside another struct. This should be represented at the class level with properties for the child relationships, which would give us introspection via QMP too. The embedding isn't adding any real value IMHO. 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 :|
