Re: [Xen-devel] [PATCH 1/2] libxl: Wait until QEMU removed the device before tearing it down

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Boris Ostrovsky writes ("[PATCH 1/2] libxl: Wait until QEMU removed the device 
before tearing it down"):
> When a device is hot-unplugged libxl sends QEMU a "device-del" message
> (via QMP). This call returns after QEMU has initiated device removal by
> sending an interrupt to the guest. At some point later QEMU is expected
> to clean up after the device (such as unbind/unmap MSIs), which will
> occur when the guest signals that the device has been ejected.

I haven't repro'd this bug, but after looking at your patch, and
staring at the code, I think you may have misdiagnosed the problem.

I found this:

    if (type == LIBXL_DOMAIN_TYPE_HVM) {
       [stuff]
    } else if (type != LIBXL_DOMAIN_TYPE_PV)
        abort();
    {

This is, of course, very bizarre.  For a moment I put it down to
coding style, but the subsequent block is the PV pci unplug path.  It
would appear therefore that HVM PCI unplug executes first the HVM
code, and then immediately afterwards the PV code.

This strangeness was introduced in abfb006f
 "tools/libxl: explicitly grant access to needed I/O-memory ranges"
which seems to contain an entirely wrong hunk.

Can you try the patch below and see if it helps your problem ?

AFAICT your other patch probably isn't needed in the light of all
this.  If it is, then I have misunderstood (and perhaps the commit
message could be more explicit about the bug that this is allegedly
fixing - calling a patch `Simplify ....' makes it sound like the kind
of cleanup we don't want to take during the freeze).

Thanks
Ian.

Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 9f40100..bec25a9 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1255,9 +1255,9 @@ static int do_pci_remove(libxl__gc *gc, uint32_t domid,
             rc = ERROR_FAIL;
             goto out_fail;
         }
-    } else if (type != LIBXL_DOMAIN_TYPE_PV)
-        abort();
-    {
+    } else {
+        assert(type == LIBXL_DOMAIN_TYPE_PV);
+
         char *sysfs_path = libxl__sprintf(gc, 
SYSFS_PCI_DEV"/"PCI_BDF"/resource", pcidev->domain,
                                          pcidev->bus, pcidev->dev, 
pcidev->func);
         FILE *f = fopen(sysfs_path, "r");

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel