Re: [PATCH v1 5/7] xen-bus: Set offline if backend's state is XenbusStateClosed

[Paul Durrant <xadimgnik@xxxxxxxxx>]

On 10/11/2023 15:42, Volodymyr Babchuk wrote:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
>
> Both state (XenbusStateClosed) and online (0) are expected by
> toolstack/xl devd to completely destroy the device. But "offline"
> is never being set by the backend resulting in timeout during
> domain destruction, garbage in Xestore and still running Qemu
> instance.
>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
> ---
>   hw/xen/xen-bus.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
> index 75474d4b43..6e7ec3af64 100644
> --- a/hw/xen/xen-bus.c
> +++ b/hw/xen/xen-bus.c
> @@ -519,6 +519,10 @@ static void xen_device_backend_changed(void *opaque, const 
> char *path)
>           xen_device_backend_set_state(xendev, XenbusStateClosed);
>       }
>   
> +    if (xen_device_backend_get_state(xendev) == XenbusStateClosed) {
> +        xen_device_backend_set_online(xendev, false);
> +    }
> +
>       /*
>        * If a backend is still 'online' then we should leave it alone but,
>        * if a backend is not 'online', then the device is a candidate
I don't understand what you're trying to do here. Just a few lines up 
from this hunk there is:
 506    if (xen_device_backend_scanf(xendev, "online", "%u", &online) 
!= 1) {
 507        online = 0;
 508    }
 509
 510    xen_device_backend_set_online(xendev, !!online);

Why is this not sufficient? What happens if the frontend decides to stop 
and start (e.g. for a driver update)? I'm guessing the backend will be 
destroyed... which is not very friendly.
  Paul