[libvirt] [PATCH v12 01/11] change used_by: specify both driver and domain
Chunyan Liu
cyliu at suse.com
Thu Feb 20 07:20:17 UTC 2014
2014-02-17 16:31 GMT+08:00 Cedric Bosdonnat <cbosdonnat at suse.com>:
> 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 at suse.com>
> > ---
> > 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
NULLwould surely be preferable.
Not used in this patch since tmp_drvname check is always 'success' within
the qemu
code. It will be used when switching to using common library. I'll change
it.
> > 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*?
>
used_by_domname is better to be char *, we can never be sure when someone
uses
this this API with a string that they free sooner than we expect. Much
safer to
strdup the parameter. And used_by_drvname is just a parallel.
Will check and correct the memory leak.
.
>
> > 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140220/d6c8ca72/attachment-0001.htm>
More information about the libvir-list
mailing list