Re: [Qemu-devel] [PATCH v2 1/2] xen-bus: Fix backend state transition on device reset

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Anthony PERARD <anthony.perard@citrix.com>
> Sent: 23 August 2019 11:16
> To: qemu-devel@nongnu.org
> Cc: Anthony Perard <anthony.perard@citrix.com>; qemu-stable@nongnu.org; Stefano Stabellini
> <sstabellini@kernel.org>; Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org
> Subject: [PATCH v2 1/2] xen-bus: Fix backend state transition on device reset
> 
> When a frontend wants to reset its state and the backend one, it
> starts with setting "Closing", then waits for the backend (QEMU) to do
> the same.
> 
> But when QEMU is setting "Closing" to its state, it triggers an event
> (xenstore watch) that re-execute xen_device_backend_changed() and set
> the backend state to "Closed". QEMU should wait for the frontend to
> set "Closed" before doing the same.
> 
> Before setting "Closed" to the backend_state, we are also going to
> check if there is a frontend. If that the case, when the backend state
> is set to "Closing" the frontend should react and sets its state to
> "Closing" then "Closed". The backend should wait for that to happen.
> 
> Fixes: b6af8926fb858c4f1426e5acb2cfc1f0580ec98a
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

> ---
> Cc: qemu-stable@nongnu.org
> ---
> 
> Notes:
>     v2:
>     - use a helper
>     - Add InitWait and Initialised to the list of active state
> 
>  hw/xen/xen-bus.c | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
> index e40500242d..62c127b926 100644
> --- a/hw/xen/xen-bus.c
> +++ b/hw/xen/xen-bus.c
> @@ -516,6 +516,23 @@ static void xen_device_backend_set_online(XenDevice *xendev, bool online)
>      xen_device_backend_printf(xendev, "online", "%u", online);
>  }
> 
> +/*
> + * Tell from the state whether the frontend is likely alive,
> + * i.e. it will react to a change of state of the backend.
> + */
> +static bool xen_device_state_is_active(enum xenbus_state state)
> +{
> +    switch (state) {
> +    case XenbusStateInitWait:
> +    case XenbusStateInitialised:
> +    case XenbusStateConnected:
> +    case XenbusStateClosing:
> +        return true;
> +    default:
> +        return false;
> +    }
> +}
> +
>  static void xen_device_backend_changed(void *opaque)
>  {
>      XenDevice *xendev = opaque;
> @@ -539,11 +556,11 @@ static void xen_device_backend_changed(void *opaque)
> 
>      /*
>       * If the toolstack (or unplug request callback) has set the backend
> -     * state to Closing, but there is no active frontend (i.e. the
> -     * state is not Connected) then set the backend state to Closed.
> +     * state to Closing, but there is no active frontend then set the
> +     * backend state to Closed.
>       */
>      if (xendev->backend_state == XenbusStateClosing &&
> -        xendev->frontend_state != XenbusStateConnected) {
> +        !xen_device_state_is_active(state)) {
>          xen_device_backend_set_state(xendev, XenbusStateClosed);
>      }
> 
> --
> Anthony PERARD