Re: [Qemu-devel] [PATCH 01/15] qdev: do not reset a device until the parent has been initialized

[Michael S. Tsirkin]

On Mon, Dec 17, 2012 at 05:24:36PM +0100, Paolo Bonzini wrote:
> When a device creates a bus and more devices as part of its init callback, the
> child device could be reset while the parent is still only partly initialized.
> In this case, the right thing to do is to delay resetting the child.  Do not
> do it at all in qdev_init, instead use qdev_reset_all to reset already-created
> devices when the state goes from CREATED to INITIALIZED.
> 
> This happens when hotplugging a usb-storage device.  Without this patch,
> initialization of a hotplugged usb-storage device would run in pre-order.
> Initialization of a coldplugged usb-storage device would run according to
> qdev_reset_all semantics (pre-order right now, post-order later in the
> series).

Is this a problem or just not pretty?

>  This patch makes things consistent.
> 
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
>  hw/qdev.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/qdev.c b/hw/qdev.c
> index 599382c..2fdf4ac 100644
> --- a/hw/qdev.c
> +++ b/hw/qdev.c
> @@ -176,8 +176,9 @@ int qdev_init(DeviceState *dev)
>                                         dev->alias_required_for_version);
>      }
>      dev->state = DEV_STATE_INITIALIZED;
> -    if (dev->hotplugged) {
> -        device_reset(dev);
> +    if (dev->hotplugged &&
> +        dev->parent_bus->parent->state == DEV_STATE_INITIALIZED) {
> +        qdev_reset_all(dev);

Let's add a comment explaining the why of this test, and when
will reset happen if it does not trigger here.

>      }
>      return 0;
>  }
> -- 
> 1.8.0.2
>