Re: [libvirt] [PATCH v12 01/11] change used_by: specify both driver and domain

Hello ChunYan, Good to see your patchset into smaller pieces. This patch looks almost OK for me, but there are problems you'll need to look into. On Mon, 2014-02-17 at 14:32 +0800, Chunyan Liu wrote: > Add driver info to used_by, to avoid conflict among different drivers if there > are more than one drivers existing and using the hostdev. > > Signed-off-by: Chunyan Liu <cyliu(a)suse.com&gt; > --- > src/lxc/lxc_hostdev.c | 11 +++++++---- > src/qemu/qemu_conf.h | 2 ++ > src/qemu/qemu_driver.c | 8 ++++---- > src/qemu/qemu_hostdev.c | 41 +++++++++++++++++++++++++---------------- > src/util/virpci.c | 30 +++++++++++++++++++++++------- > src/util/virpci.h | 9 ++++++--- > src/util/virscsi.c | 30 ++++++++++++++++++++++-------- > src/util/virscsi.h | 7 +++++-- > src/util/virusb.c | 29 ++++++++++++++++++++++------- > src/util/virusb.h | 8 ++++++-- > tests/virscsitest.c | 6 +++--- > 11 files changed, 125 insertions(+), 56 deletions(-) > > diff --git a/src/lxc/lxc_hostdev.c b/src/lxc/lxc_hostdev.c > index 3b371fc..77ce965 100644 > --- a/src/lxc/lxc_hostdev.c > +++ b/src/lxc/lxc_hostdev.c > @@ -60,7 +60,7 @@ virLXCUpdateActiveUsbHostdevs(virLXCDriverPtr driver, > continue; > } > > - virUSBDeviceSetUsedBy(usb, def->name); > + virUSBDeviceSetUsedBy(usb, "QEMU", def->name); You meant LXC_DRIVER_NAME rather than "QEMU" here, right? > virObjectLock(driver->activeUsbHostdevs); > if (virUSBDeviceListAdd(driver->activeUsbHostdevs, usb) < 0) { > @@ -90,7 +90,9 @@ virLXCPrepareHostdevUSBDevices(virLXCDriverPtr driver, > for (i = 0; i < count; i++) { > virUSBDevicePtr usb = virUSBDeviceListGet(list, i); > if ((tmp = virUSBDeviceListFind(driver->activeUsbHostdevs, usb))) { > - const char *other_name = virUSBDeviceGetUsedBy(tmp); > + const char *other_name = NULL; > + const char *other_drvname = NULL; > + virUSBDeviceGetUsedBy(tmp, &other_drvname, &other_name); > > if (other_name) > virReportError(VIR_ERR_OPERATION_INVALID, > @@ -103,7 +105,7 @@ virLXCPrepareHostdevUSBDevices(virLXCDriverPtr driver, > goto error; > } > > - virUSBDeviceSetUsedBy(usb, name); > + virUSBDeviceSetUsedBy(usb, "QEMU", name); Same here, I guess that should be LXC_DRIVER_NAME, no? > VIR_DEBUG("Adding %03d.%03d dom=%s to activeUsbHostdevs", > virUSBDeviceGetBus(usb), virUSBDeviceGetDevno(usb), name); > /* > @@ -352,6 +354,7 @@ virLXCDomainReAttachHostUsbDevices(virLXCDriverPtr driver, > virDomainHostdevDefPtr hostdev = hostdevs[i]; > virUSBDevicePtr usb, tmp; > const char *used_by = NULL; > + const char *used_by_drvname = NULL; > > if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) > continue; > @@ -389,7 +392,7 @@ virLXCDomainReAttachHostUsbDevices(virLXCDriverPtr driver, > continue; > } > > - used_by = virUSBDeviceGetUsedBy(tmp); > + virUSBDeviceGetUsedBy(tmp, &used_by_drvname, &used_by); Where is used_by_drvname used? If that really isn't used, then replacing it by NULL would be fine. > if (STREQ_NULLABLE(used_by, name)) { > VIR_DEBUG("Removing %03d.%03d dom=%s from activeUsbHostdevs", > hostdev->source.subsys.u.usb.bus, > diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h > index 1f44a76..158cc1a 100644 > --- a/src/qemu/qemu_conf.h > +++ b/src/qemu/qemu_conf.h > @@ -53,6 +53,8 @@ > # error "Port me" > # endif > > +# define QEMU_DRIVER_NAME "QEMU" > + > typedef struct _virQEMUDriver virQEMUDriver; > typedef virQEMUDriver *virQEMUDriverPtr; > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 59e018d..1fe8992 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -97,8 +97,6 @@ > > #define VIR_FROM_THIS VIR_FROM_QEMU > > -#define QEMU_DRIVER_NAME "QEMU" > - > #define QEMU_NB_MEM_PARAM 3 > > #define QEMU_NB_BLOCK_IO_TUNE_PARAM 6 > @@ -11325,7 +11323,9 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev) > virObjectLock(driver->inactivePciHostdevs); > other = virPCIDeviceListFind(driver->activePciHostdevs, pci); > if (other) { > - const char *other_name = virPCIDeviceGetUsedBy(other); > + const char *other_name = NULL; > + const char *other_drvname = NULL; > + virPCIDeviceGetUsedBy(other, &other_drvname, &other_name); Is other_drvname used any here? Seems not from the patch, so using

> if (other_name) > virReportError(VIR_ERR_OPERATION_INVALID, > @@ -16684,7 +16684,7 @@ static virDriver qemuDriver = { > > > static virStateDriver qemuStateDriver = { > - .name = "QEMU", > + .name = QEMU_DRIVER_NAME, > .stateInitialize = qemuStateInitialize, > .stateAutoStart = qemuStateAutoStart, > .stateCleanup = qemuStateCleanup, > diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c > index 1b16386..01c24f9 100644 > --- a/src/qemu/qemu_hostdev.c > +++ b/src/qemu/qemu_hostdev.c > @@ -177,7 +177,7 @@ qemuUpdateActivePciHostdevs(virQEMUDriverPtr driver, > goto cleanup; > > } > - virPCIDeviceSetUsedBy(dev, def->name); > + virPCIDeviceSetUsedBy(dev, QEMU_DRIVER_NAME, def->name); > > /* Setup the original states for the PCI device */ > virPCIDeviceSetUnbindFromStub(dev, hostdev->origstates.states.pci.unbind_from_stub); > @@ -230,7 +230,7 @@ qemuUpdateActiveUsbHostdevs(virQEMUDriverPtr driver, > continue; > } > > - virUSBDeviceSetUsedBy(usb, def->name); > + virUSBDeviceSetUsedBy(usb, QEMU_DRIVER_NAME, def->name); > > if (virUSBDeviceListAdd(driver->activeUsbHostdevs, usb) < 0) { > virUSBDeviceFree(usb); > @@ -274,13 +274,13 @@ qemuUpdateActiveScsiHostdevs(virQEMUDriverPtr driver, > goto cleanup; > > if ((tmp = virSCSIDeviceListFind(driver->activeScsiHostdevs, scsi))) { > - if (virSCSIDeviceSetUsedBy(tmp, def->name) < 0) { > + if (virSCSIDeviceSetUsedBy(tmp, QEMU_DRIVER_NAME, def->name) < 0) { > virSCSIDeviceFree(scsi); > goto cleanup; > } > virSCSIDeviceFree(scsi); > } else { > - if (virSCSIDeviceSetUsedBy(scsi, def->name) < 0 || > + if (virSCSIDeviceSetUsedBy(scsi, QEMU_DRIVER_NAME, def->name) < 0 || > virSCSIDeviceListAdd(driver->activeScsiHostdevs, scsi) < 0) { > virSCSIDeviceFree(scsi); > goto cleanup; > @@ -693,7 +693,9 @@ qemuPrepareHostdevPCIDevices(virQEMUDriverPtr driver, > * the dev is in list driver->activePciHostdevs. > */ > if ((other = virPCIDeviceListFind(driver->activePciHostdevs, dev))) { > - const char *other_name = virPCIDeviceGetUsedBy(other); > + const char *other_name = NULL; > + const char *other_drvname = NULL; > + virPCIDeviceGetUsedBy(other, &other_drvname, &other_name); Again, other_drvname seems to be unused. > if (other_name) > virReportError(VIR_ERR_OPERATION_INVALID, > @@ -766,7 +768,7 @@ qemuPrepareHostdevPCIDevices(virQEMUDriverPtr driver, > activeDev = virPCIDeviceListFind(driver->activePciHostdevs, dev); > > if (activeDev) > - virPCIDeviceSetUsedBy(activeDev, name); > + virPCIDeviceSetUsedBy(activeDev, QEMU_DRIVER_NAME, name); > } > > /* Loop 8: Now set the original states for hostdev def */ > @@ -857,7 +859,9 @@ qemuPrepareHostdevUSBDevices(virQEMUDriverPtr driver, > for (i = 0; i < count; i++) { > virUSBDevicePtr usb = virUSBDeviceListGet(list, i); > if ((tmp = virUSBDeviceListFind(driver->activeUsbHostdevs, usb))) { > - const char *other_name = virUSBDeviceGetUsedBy(tmp); > + const char *other_name = NULL; > + const char *other_drvname = NULL; > + virUSBDeviceGetUsedBy(tmp, &other_drvname, &other_name); Here too. > if (other_name) > virReportError(VIR_ERR_OPERATION_INVALID, > @@ -870,7 +874,7 @@ qemuPrepareHostdevUSBDevices(virQEMUDriverPtr driver, > goto error; > } > > - virUSBDeviceSetUsedBy(usb, name); > + virUSBDeviceSetUsedBy(usb, QEMU_DRIVER_NAME, name); > VIR_DEBUG("Adding %03d.%03d dom=%s to activeUsbHostdevs", > virUSBDeviceGetBus(usb), virUSBDeviceGetDevno(usb), name); > /* > @@ -1140,10 +1144,10 @@ qemuPrepareHostdevSCSIDevices(virQEMUDriverPtr driver, > goto error; > } > > - if (virSCSIDeviceSetUsedBy(tmp, name) < 0) > + if (virSCSIDeviceSetUsedBy(tmp, QEMU_DRIVER_NAME, name) < 0) > goto error; > } else { > - if (virSCSIDeviceSetUsedBy(scsi, name) < 0) > + if (virSCSIDeviceSetUsedBy(scsi, QEMU_DRIVER_NAME, name) < 0) > goto error; > > VIR_DEBUG("Adding %s to activeScsiHostdevs", virSCSIDeviceGetName(scsi)); > @@ -1275,10 +1279,14 @@ qemuDomainReAttachHostdevDevices(virQEMUDriverPtr driver, > * been used by this domain. > */ > activeDev = virPCIDeviceListFind(driver->activePciHostdevs, dev); > - if (activeDev && > - STRNEQ_NULLABLE(name, virPCIDeviceGetUsedBy(activeDev))) { > - virPCIDeviceListDel(pcidevs, dev); > - continue; > + if (activeDev) { > + const char *tmp_name = NULL; > + const char *tmp_drvname = NULL; > + virPCIDeviceGetUsedBy(activeDev, &tmp_drvname, &tmp_name); Is tmp_drvname actually used?

> + if (STRNEQ_NULLABLE(name, tmp_name)) { > + virPCIDeviceListDel(pcidevs, dev); > + continue; > + } > } > > virPCIDeviceListDel(driver->activePciHostdevs, dev); > @@ -1333,6 +1341,7 @@ qemuDomainReAttachHostUsbDevices(virQEMUDriverPtr driver, > virDomainHostdevDefPtr hostdev = hostdevs[i]; > virUSBDevicePtr usb, tmp; > const char *used_by = NULL; > + const char *used_by_drvname = NULL; > > if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) > continue; > @@ -1370,7 +1379,7 @@ qemuDomainReAttachHostUsbDevices(virQEMUDriverPtr driver, > continue; > } > > - used_by = virUSBDeviceGetUsedBy(tmp); > + virUSBDeviceGetUsedBy(tmp, &used_by_drvname, &used_by); Same here, used_by_drvname isn't used. > if (STREQ_NULLABLE(used_by, name)) { > VIR_DEBUG("Removing %03d.%03d dom=%s from activeUsbHostdevs", > hostdev->source.subsys.u.usb.bus, > @@ -1445,7 +1454,7 @@ qemuDomainReAttachHostScsiDevices(virQEMUDriverPtr driver, > hostdev->source.subsys.u.scsi.unit, > name); > > - virSCSIDeviceListDel(driver->activeScsiHostdevs, tmp, name); > + virSCSIDeviceListDel(driver->activeScsiHostdevs, tmp, QEMU_DRIVER_NAME, name); > virSCSIDeviceFree(scsi); > } > virObjectUnlock(driver->activeScsiHostdevs); > diff --git a/src/util/virpci.c b/src/util/virpci.c > index 00d1064..4ea2bcf 100644 > --- a/src/util/virpci.c > +++ b/src/util/virpci.c > @@ -59,7 +59,10 @@ struct _virPCIDevice { > char name[PCI_ADDR_LEN]; /* domain:bus:slot.function */ > char id[PCI_ID_LEN]; /* product vendor */ > char *path; > - const char *used_by; /* The domain which uses the device */ > + > + /* The driver:domain which uses the device */ > + char *used_by_drvname; > + char *used_by_domname; Why changing from const char* to char*?

> unsigned int pcie_cap_pos; > unsigned int pci_pm_cap_pos; > @@ -1635,6 +1638,8 @@ virPCIDeviceFree(virPCIDevicePtr dev) > VIR_DEBUG("%s %s: freeing", dev->id, dev->name); > VIR_FREE(dev->path); > VIR_FREE(dev->stubDriver); > + VIR_FREE(dev->used_by_drvname); > + VIR_FREE(dev->used_by_domname); > VIR_FREE(dev); > } > > @@ -1704,16 +1709,27 @@ virPCIDeviceSetReprobe(virPCIDevicePtr dev, bool reprobe) > dev->reprobe = reprobe; > } > > -void > -virPCIDeviceSetUsedBy(virPCIDevicePtr dev, const char *name) > +int > +virPCIDeviceSetUsedBy(virPCIDevicePtr dev, > + const char *drv_name, > + const char *dom_name) > { > - dev->used_by = name; > + if (VIR_STRDUP(dev->used_by_drvname, drv_name) < 0) > + return -1; > + if (VIR_STRDUP(dev->used_by_domname, dom_name) < 0) > + return -1; > + You will leak memory here if there was already a value set, You need to free dev->used_by_drvname and dev->used_by_domname before VIR_STRDUP'ing to them. However, it would be much more convenient to keep const char*. > + return 0; > } > > -const char * > -virPCIDeviceGetUsedBy(virPCIDevicePtr dev) > +void > +virPCIDeviceGetUsedBy(virPCIDevicePtr dev, > + const char **drv_name, > + const char **dom_name) > { > - return dev->used_by; > + *drv_name = dev->used_by_drvname; > + *dom_name = dev->used_by_domname; > } > > void virPCIDeviceReattachInit(virPCIDevicePtr pci) > diff --git a/src/util/virpci.h b/src/util/virpci.h > index ac6dae1..20ffe54 100644 > --- a/src/util/virpci.h > +++ b/src/util/virpci.h > @@ -66,9 +66,12 @@ int virPCIDeviceSetStubDriver(virPCIDevicePtr dev, > const char *driver) > ATTRIBUTE_NONNULL(2); > const char *virPCIDeviceGetStubDriver(virPCIDevicePtr dev); > -void virPCIDeviceSetUsedBy(virPCIDevice *dev, > - const char *used_by); > -const char *virPCIDeviceGetUsedBy(virPCIDevice *dev); > +int virPCIDeviceSetUsedBy(virPCIDevice *dev, > + const char *drv_name, > + const char *dom_name); > +void virPCIDeviceGetUsedBy(virPCIDevice *dev, > + const char **drv_name, > + const char **dom_name); > unsigned int virPCIDeviceGetUnbindFromStub(virPCIDevicePtr dev); > void virPCIDeviceSetUnbindFromStub(virPCIDevice *dev, > bool unbind); > diff --git a/src/util/virscsi.c b/src/util/virscsi.c > index acc3815..5e28328 100644 > --- a/src/util/virscsi.c > +++ b/src/util/virscsi.c > @@ -47,6 +47,11 @@ > > /* For virReportOOMError() and virReportSystemError() */ > #define VIR_FROM_THIS VIR_FROM_NONE > +struct _virUsedByInfo { > + char *drvname; /* which driver */ > + char *domname; /* which domain */ > +}; > +typedef struct _virUsedByInfo *virUsedByInfoPtr; > > struct _virSCSIDevice { > unsigned int adapter; > @@ -57,7 +62,7 @@ struct _virSCSIDevice { > char *name; /* adapter:bus:target:unit */ > char *id; /* model:vendor */ > char *sg_path; /* e.g. /dev/sg2 */ > - char **used_by; /* name of the domains using this dev */ > + virUsedByInfoPtr *used_by; /* driver:domain(s) using this dev */ > size_t n_used_by; /* how many domains are using this dev */ > > bool readonly; > @@ -274,19 +279,26 @@ virSCSIDeviceFree(virSCSIDevicePtr dev) > VIR_FREE(dev->id); > VIR_FREE(dev->name); > VIR_FREE(dev->sg_path); > - for (i = 0; i < dev->n_used_by; i++) > + for (i = 0; i < dev->n_used_by; i++) { > + VIR_FREE(dev->used_by[i]->drvname); > + VIR_FREE(dev->used_by[i]->domname); > VIR_FREE(dev->used_by[i]); > + } > VIR_FREE(dev->used_by); > VIR_FREE(dev); > } > > int > virSCSIDeviceSetUsedBy(virSCSIDevicePtr dev, > - const char *name) > + const char *drvname, > + const char *domname) > { > - char *copy = NULL; > - > - if (VIR_STRDUP(copy, name) < 0) > + virUsedByInfoPtr copy; > + if (VIR_ALLOC(copy) < 0) > + return -1; > + if (VIR_STRDUP(copy->drvname, drvname) < 0) > + return -1; > + if (VIR_STRDUP(copy->domname, domname) < 0) > return -1; > > return VIR_APPEND_ELEMENT(dev->used_by, dev->n_used_by, copy); > @@ -427,13 +439,15 @@ virSCSIDeviceListSteal(virSCSIDeviceListPtr list, > void > virSCSIDeviceListDel(virSCSIDeviceListPtr list, > virSCSIDevicePtr dev, > - const char *name) > + const char *drvname, > + const char *domname) > { > virSCSIDevicePtr tmp = NULL; > size_t i; > > for (i = 0; i < dev->n_used_by; i++) { > - if (STREQ_NULLABLE(dev->used_by[i], name)) { > + if (STREQ_NULLABLE(dev->used_by[i]->drvname, drvname) && > + STREQ_NULLABLE(dev->used_by[i]->domname, domname)) { > if (dev->n_used_by > 1) { > VIR_DELETE_ELEMENT(dev->used_by, i, dev->n_used_by); I'ld say that doesn't free everything. You'll need to VIR_FREE the dev->drvname and dev->domname before. > } else { > diff --git a/src/util/virscsi.h b/src/util/virscsi.h > index 1b685eb..c67837f 100644 > --- a/src/util/virscsi.h > +++ b/src/util/virscsi.h > @@ -53,7 +53,9 @@ virSCSIDevicePtr virSCSIDeviceNew(const char *sysfs_prefix, > bool shareable); > > void virSCSIDeviceFree(virSCSIDevicePtr dev); > -int virSCSIDeviceSetUsedBy(virSCSIDevicePtr dev, const char *name); > +int virSCSIDeviceSetUsedBy(virSCSIDevicePtr dev, > + const char *drvname, > + const char *domname); > bool virSCSIDeviceIsAvailable(virSCSIDevicePtr dev); > const char *virSCSIDeviceGetName(virSCSIDevicePtr dev); > unsigned int virSCSIDeviceGetAdapter(virSCSIDevicePtr dev); > @@ -87,7 +89,8 @@ virSCSIDevicePtr virSCSIDeviceListSteal(virSCSIDeviceListPtr list, > virSCSIDevicePtr dev); > void virSCSIDeviceListDel(virSCSIDeviceListPtr list, > virSCSIDevicePtr dev, > - const char *name); > + const char *drvname, > + const char *domname); > virSCSIDevicePtr virSCSIDeviceListFind(virSCSIDeviceListPtr list, > virSCSIDevicePtr dev); > > diff --git a/src/util/virusb.c b/src/util/virusb.c > index bb5980d..4e22973 100644 > --- a/src/util/virusb.c > +++ b/src/util/virusb.c > @@ -55,7 +55,10 @@ struct _virUSBDevice { > char name[USB_ADDR_LEN]; /* domain:bus:slot.function */ > char id[USB_ID_LEN]; /* product vendor */ > char *path; > - const char *used_by; /* name of the domain using this dev */ > + > + /* driver:domain using this dev */ > + char *used_by_drvname; > + char *used_by_domname; > }; Again, why changing from const char* to char*? It will mostly lead to memory handling troubles. > struct _virUSBDeviceList { > @@ -375,19 +378,31 @@ virUSBDeviceFree(virUSBDevicePtr dev) > return; > VIR_DEBUG("%s %s: freeing", dev->id, dev->name); > VIR_FREE(dev->path); > + VIR_FREE(dev->used_by_drvname); > + VIR_FREE(dev->used_by_domname); > VIR_FREE(dev); > } > > - > -void virUSBDeviceSetUsedBy(virUSBDevicePtr dev, > - const char *name) > +int > +virUSBDeviceSetUsedBy(virUSBDevicePtr dev, > + const char *drv_name, > + const char *dom_name) > { > - dev->used_by = name; > + if (VIR_STRDUP(dev->used_by_drvname, drv_name) < 0) > + return -1; > + if (VIR_STRDUP(dev->used_by_domname, dom_name) < 0) > + return -1; What if there was already values set? There would be a memory leak: either use const char* or VIR_FREE before the VIR_STRDUP. > + > + return 0; > } > > -const char * virUSBDeviceGetUsedBy(virUSBDevicePtr dev) > +void > +virUSBDeviceGetUsedBy(virUSBDevicePtr dev, > + const char **drv_name, > + const char **dom_name) > { > - return dev->used_by; > + *drv_name = dev->used_by_drvname; > + *dom_name = dev->used_by_domname; > } > > const char *virUSBDeviceGetName(virUSBDevicePtr dev) > diff --git a/src/util/virusb.h b/src/util/virusb.h > index e0a7c4c..f98ea21 100644 > --- a/src/util/virusb.h > +++ b/src/util/virusb.h > @@ -60,8 +60,12 @@ int virUSBDeviceFind(unsigned int vendor, > virUSBDevicePtr *usb); > > void virUSBDeviceFree(virUSBDevicePtr dev); > -void virUSBDeviceSetUsedBy(virUSBDevicePtr dev, const char *name); > -const char *virUSBDeviceGetUsedBy(virUSBDevicePtr dev); > +int virUSBDeviceSetUsedBy(virUSBDevicePtr dev, > + const char *drv_name, > + const char *dom_name); > +void virUSBDeviceGetUsedBy(virUSBDevicePtr dev, > + const char **drv_name, > + const char **dom_name); > const char *virUSBDeviceGetName(virUSBDevicePtr dev); > > unsigned int virUSBDeviceGetBus(virUSBDevicePtr dev); > diff --git a/tests/virscsitest.c b/tests/virscsitest.c > index d4b3e4a..586c41b 100644 > --- a/tests/virscsitest.c > +++ b/tests/virscsitest.c > @@ -91,13 +91,13 @@ test2(const void *data ATTRIBUTE_UNUSED) > if (!virSCSIDeviceIsAvailable(dev)) > goto cleanup; > > - if (virSCSIDeviceSetUsedBy(dev, "fc18") < 0) > + if (virSCSIDeviceSetUsedBy(dev, "QEMU", "fc18") < 0) > goto cleanup; > > if (virSCSIDeviceIsAvailable(dev)) > goto cleanup; > > - if (virSCSIDeviceSetUsedBy(dev, "fc20") < 0) > + if (virSCSIDeviceSetUsedBy(dev, "QEMU", "fc20") < 0) > goto cleanup; > > if (virSCSIDeviceIsAvailable(dev)) > @@ -117,7 +117,7 @@ test2(const void *data ATTRIBUTE_UNUSED) > if (!virSCSIDeviceListFind(list, dev)) > goto cleanup; > > - virSCSIDeviceListDel(list, dev, "fc20"); > + virSCSIDeviceListDel(list, dev, "QEMU", "fc20"); > > if (!virSCSIDeviceListFind(list, dev)) > goto cleanup; May be some tests with the "LXC" value couldn't harm. -- Cedric