Hi On Thu, Apr 30, 2026 at 7:32 PM Peter Maydell <[email protected]> wrote: > > On Thu, 30 Apr 2026 at 16:18, Philippe Mathieu-Daudé <[email protected]> > wrote: > > > > On 30/4/26 17:12, Peter Maydell wrote: > > > On Mon, 27 Apr 2026 at 20:45, Marc-André Lureau > > > <[email protected]> wrote: > > >> > > >> +static void virt_instance_finalize(Object *obj) > > >> +{ > > >> + VirtMachineState *vms = VIRT_MACHINE(obj); > > >> + > > >> + for (int i = 0; i < ARRAY_SIZE(vms->flash); i++) { > > >> + if (vms->flash[i] && !qdev_is_realized(DEVICE(vms->flash[i]))) { > > >> + object_unref(OBJECT(vms->flash[i])); > > >> + } > > > > > > We currently use qdev_is_realized() almost nowhere in > > > the codebase. Isn't there some way to do this such that we > > > don't have to care about this because the object is > > > parented and gets auto-dereffed ? Having each object > > > have to care about the possibility of "inited but > > > not realized" devices in finalize seems like it's going > > > to result in a lot of bugs... > > > > Why are we creating pflash devices if we aren't realizing them > > (thus not using them at all)? Destroying them when unneeded > > would avoid that manual cleanup, right? > > I wondered that, but what happens is that we init the pflash > devices in the machine's instance_init by calling virt_flash_create(), > and then later we do the "configure, realize, map" part in > virt_flash_map() in the MachineClass::init method. So if > you init the Machine object but never call its init method > before finalizing it, the flash devices are still in the > inited-but-not-realized state. (This is similar to how with > a normal qdev object, if you init it but don't realize it > it will init but not realize all its child objects.) > (We need to create the flash devices earlier than we do > the other devices on the board to make the machine properties > that virt_flash_create() sets up work.) > > Since virt_flash_create1() calls object_property_add_child() > to make the flash device a child of the machine, I think > we could make that be the only reference, by having > virt_flash_create1() do: > > Object *obj = object_new_with_props(TYPE_PFLASH_CFI01, OBJECT(vms), > name, &error_fatal, NULL); > DeviceState *dev = DEVICE(obj); > > and then using sysbus_realize() rather than sysbus_realize_and_unref() > in virt_flash_map1(). Not tested, though...
Yes, in general, if a device creates & keeps a reference to an instance, it should be a strong reference (we don't have support for weak reference the way gobject has for ex). It should then unref it unconditonally on finalize. Is it ok if it is addressed in a follow-up series after this one lands? I am afraid it would delay this series or make it longer, and a shift its focus thanks
