Re: [PATCH for-11.2 v3 14/15] hw/qdev: Parent device before setting parent bus

On 21/7/26 10:17, Akihiko Odaki wrote:
> Commit 9940b2cfbc05 ("qdev: New qdev_new(), qdev_realize(), etc.") says
> "device state 'no QOM parent, but plugged into bus' is dangerous". In
> such a case, unrealizing the bus will hang in bus_unparent():
> 
>      while ((kid = QTAILQ_FIRST(&bus->children)) != NULL) {
>          DeviceState *dev = kid->child;
>          object_unparent(OBJECT(dev));
>      }
> 
> object_unparent() does nothing when its argument has no QOM parent,
> and the loop spins forever.
> 
> However, that commit did not completely eliminate such a situation.
> When the device is not parented, device_set_realized() lets
> /machine/unattached parent it, but it happens after setting parent bus.
> Therefore, any failure between the two operations can leave the device
> in a dangerous state.
> 
> qdev_realize() at least asserts that the device is not already realized
> and prevents one realization failure pattern, but it is not
> comprehensive. Besides, it will trip with a command line like the
> following:
> 
>      qemu-system-x86_64 -M none -nodefaults -nographic \
>          -device ipmi-bmc-sim,realized=on
> 
> Eliminate the dangerous state by ensuring that the device is parented
> before calling qdev_set_parent_bus(). Also, stop asserting that the
> device is not already realized in qdev_realize(); it is broken and
> no longer serves any purpose.
> 
> Fixes: 9940b2cfbc05 ("qdev: New qdev_new(), qdev_realize(), etc.")
> Signed-off-by: Akihiko Odaki <[email protected]>
> ---
>   hw/core/qdev.c         | 51 ++++++++++++++++++++++++++++----------------------
>   tests/unit/test-qdev.c | 13 +++++++++++++
>   2 files changed, 42 insertions(+), 22 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <[email protected]>