Re: [PATCH for-4.5] reset PCI devices on force removal even when QEMU returns error

[Ian Campbell <Ian.Campbell@citrix.com>]

On Fri, 2014-11-28 at 16:53 +0000, Stefano Stabellini wrote:

CCing Boris because he was fixing a similar sounding issue in the same
area. Not sure if this is related to those patches or not.

> On do_pci_remove when QEMU returns error, we just bail out early without
> resetting the device. On domain shutdown we are racing with QEMU exiting
> and most often QEMU closes the QMP connection before executing the
> requested command.
> 
> In these cases if force=1, it makes sense to go ahead with rest of the
> PCI device removal, that includes resetting the device and calling
> xc_deassign_device. Otherwise we risk not resetting the device properly
> on domain shutdown.

ISTR seeing a conversation along the lines of there being a better
solution which was more 4.6 material, is that right?

> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

But, I'd prefer to see a version which logs when the qemu side has
failed but it is continuing. Probably just adding right after the
existing if:
        if (rc)
            LOG("Something appropriate");
would do the trick.

Also this needs RM input from Konrad.

> 
> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index 316643c..0ac0b93 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -1243,7 +1245,7 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
>              rc = ERROR_INVAL;
>              goto out_fail;
>          }
> -        if (rc) {
> +        if (rc && !force) {
>              rc = ERROR_FAIL;
>              goto out_fail;
>          }