Re: [PATCH 2/2] libxl: do not automatically force detach of block devices

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Sep 03, 2020 at 11:05:37AM +0100, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@xxxxxxxxxx>
> 
> The manpage for 'xl' documents that guest co-operation is required for a (non-
> forced) block-detach operation and that it may consequently fail. Currently,
> however, the implementation of generic device removal means that a time-out
> of a block-detach is being automatically re-tried with the force flag set
> rather than failing. This patch stops such behaviour.

Won't this break cleanup on domain shutdown if the guest doesn't close
the devices itself?

I think we need some special-casing on shutdown that keeps the current
behavior on that case.

> 
> Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>
> ---
> Cc: Ian Jackson <iwj@xxxxxxxxxxxxxx>
> Cc: Wei Liu <wl@xxxxxxx>
> Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> ---
>  tools/libxl/libxl_device.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index 0381c5d509..d17ca78848 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -1092,7 +1092,8 @@ static void device_backend_callback(libxl__egc *egc, 
> libxl__ev_devstate *ds,
>  
>      if (rc == ERROR_TIMEDOUT &&
>          aodev->action == LIBXL__DEVICE_ACTION_REMOVE &&
> -        !aodev->force) {
> +        !aodev->force &&
> +        aodev->dev->kind != LIBXL__DEVICE_KIND_VBD) {

Doing this differentiation for block only seems weird, we should treat
all devices equally.

Thanks, Roger.