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

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 12/02/2014 08:43 AM, Ian Campbell wrote:
> 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.

Not really. I was trying to fix something that another Stefano's patch 
from yesterday is trying to address:

http://lists.xenproject.org/archives/html/xen-devel/2014-12/msg00160.html

although I thought that removing the call to xc_domain_irq_permission() 
would be the solution.

(which reminds me --- and maybe I shouldn't overload this thread --- 
that this patch from IanJ needs to get in as well:

http://lists.xenproject.org/archives/html/xen-devel/2014-11/msg01342.html

)

Thanks.
-boris


>
>> 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;
>>           }
>