From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH for-4.5] reset PCI devices on force removal even when QEMU returns error Date: Tue, 02 Dec 2014 10:09:26 -0500 Message-ID: <547DD626.3070600@oracle.com> References: <1417527800.24320.29.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1417527800.24320.29.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Stefano Stabellini Cc: "Wei Liu (Intern)" , xen-devel@lists.xensource.com, Ian Jackson , linux@eikelenboom.it List-Id: xen-devel@lists.xenproject.org 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 > Acked-by: Ian Campbell > > 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; >> } >