> -----Original Message----- > From: libvir-list-bounces@xxxxxxxxxx [mailto:libvir-list- > bounces@xxxxxxxxxx] On Behalf Of Erik Skultety > Sent: Thursday, January 25, 2018 5:24 PM > To: libvir-list@xxxxxxxxxx > Cc: Erik Skultety <eskultet@xxxxxxxxxx> > Subject: [PATCH 07/15] nodedev: Introduce > virNodeDeviceCapsListExport > > Whether asking for a number of capabilities supported by a device or > listing them, it's handled essentially by a copy-paste code, so extract > the common stuff into this new helper which also updates all capabilities > just before touching them. > > Signed-off-by: Erik Skultety <eskultet@xxxxxxxxxx> > --- > src/conf/node_device_conf.c | 73 > +++++++++++++++++++++++++++++++++++ > src/conf/node_device_conf.h | 5 +++ > src/libvirt_private.syms | 1 + > src/node_device/node_device_driver.c | 75 +++++++++---------------------- > ----- > 4 files changed, 97 insertions(+), 57 deletions(-) > > diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c > index 217673a56..9467bb415 100644 > --- a/src/conf/node_device_conf.c > +++ b/src/conf/node_device_conf.c > @@ -2487,6 +2487,79 @@ virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def) } > > > +/** > + * virNodeDeviceCapsListExport: > + * @def: node device definition > + * @list: pointer to an array to store all supported capabilities by a > +device > + * > + * Takes the definition, scans through all the capabilities that the > +device > + * supports (including the nested caps) and populates a newly allocated > +list > + * with them. Caller is responsible for freeing the list. > + * If NULL is passed to @list, only the number of caps will be returned. > + * > + * Returns the number of capabilities the device supports, -1 on error. > + */ > +int > +virNodeDeviceCapsListExport(virNodeDeviceDefPtr def, > + virNodeDevCapType **list) { > + virNodeDevCapsDefPtr caps = NULL; > + virNodeDevCapType *tmp = NULL; > + bool want_list = !!list; > + int ncaps = 0; > + int ret = -1; > + > +#define MAYBE_ADD_CAP(cap) \ > + do { \ > + if (want_list) \ > + tmp[ncaps] = cap; \ > + } while (0) > + > + if (want_list && VIR_ALLOC_N(tmp, VIR_NODE_DEV_CAP_LAST - 1) < 0) > + goto cleanup; > + > + for (caps = def->caps; caps; caps = caps->next) { > + unsigned int flags; > + > + MAYBE_ADD_CAP(caps->data.type); > + ncaps++; > + > + /* check nested caps for a given type as well */ > + if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) { > + flags = caps->data.scsi_host.flags; > + > + if (flags & VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { > + MAYBE_ADD_CAP(VIR_NODE_DEV_CAP_FC_HOST); > + ncaps++; > + } > + > + if (flags & VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) { > + MAYBE_ADD_CAP(VIR_NODE_DEV_CAP_VPORTS); > + ncaps++; > + } > + } > + > + if (caps->data.type == VIR_NODE_DEV_CAP_PCI_DEV) { > + flags = caps->data.pci_dev.flags; > + > + if (flags & VIR_NODE_DEV_CAP_FLAG_PCI_MDEV) { > + MAYBE_ADD_CAP(VIR_NODE_DEV_CAP_MDEV_TYPES); > + ncaps++; > + } > + } > + } > + > +#undef MAYBE_ADD_CAP > + > + if (want_list) > + VIR_STEAL_PTR(*list, tmp); > + ret = ncaps; > + cleanup: > + VIR_FREE(tmp); > + return ret; > +} > + > + > #ifdef __linux__ > > int > diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h > index 7e32f5c05..53cdfdb01 100644 > --- a/src/conf/node_device_conf.h > +++ b/src/conf/node_device_conf.h > @@ -403,4 +403,9 @@ virNodeDeviceGetPCIDynamicCaps(const char *sysfsPath, > > int > virNodeDeviceUpdateCaps(virNodeDeviceDefPtr def); > + > +int > +virNodeDeviceCapsListExport(virNodeDeviceDefPtr def, > + virNodeDevCapType **list); > + > #endif /* __VIR_NODE_DEVICE_CONF_H__ */ diff --git > a/src/libvirt_private.syms b/src/libvirt_private.syms index > 6098cf121..1698e6227 100644 > --- a/src/libvirt_private.syms > +++ b/src/libvirt_private.syms > @@ -699,6 +699,7 @@ virNodeDevCapMdevTypeFree; virNodeDevCapsDefFree; > virNodeDevCapTypeFromString; virNodeDevCapTypeToString; > +virNodeDeviceCapsListExport; > virNodeDeviceCreateVport; > virNodeDeviceDefFormat; > virNodeDeviceDefFree; > diff --git a/src/node_device/node_device_driver.c > b/src/node_device/node_device_driver.c > index 48f45474c..8fb08742b 100644 > --- a/src/node_device/node_device_driver.c > +++ b/src/node_device/node_device_driver.c > @@ -306,8 +306,6 @@ nodeDeviceNumOfCaps(virNodeDevicePtr device) { > virNodeDeviceObjPtr obj; > virNodeDeviceDefPtr def; > - virNodeDevCapsDefPtr caps; > - int ncaps = 0; > int ret = -1; > > if (!(obj = nodeDeviceObjFindByName(device->name))) > @@ -317,27 +315,7 @@ nodeDeviceNumOfCaps(virNodeDevicePtr device) > if (virNodeDeviceNumOfCapsEnsureACL(device->conn, def) < 0) > goto cleanup; > > - for (caps = def->caps; caps; caps = caps->next) { > - ++ncaps; > - > - if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) { > - if (caps->data.scsi_host.flags & > - VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) > - ncaps++; > - > - if (caps->data.scsi_host.flags & > - VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) > - ncaps++; > - } > - if (caps->data.type == VIR_NODE_DEV_CAP_PCI_DEV) { > - if (caps->data.pci_dev.flags & > - VIR_NODE_DEV_CAP_FLAG_PCI_MDEV) > - ncaps++; > - } > - > - } > - > - ret = ncaps; > + ret = virNodeDeviceCapsListExport(def, NULL); > > cleanup: > virNodeDeviceObjEndAPI(&obj); > @@ -353,9 +331,10 @@ nodeDeviceListCaps(virNodeDevicePtr device, { > virNodeDeviceObjPtr obj; > virNodeDeviceDefPtr def; > - virNodeDevCapsDefPtr caps; > + virNodeDevCapType *list = NULL; > int ncaps = 0; > int ret = -1; > + size_t i; variable i should be initialized. Or the act is uncertain when cleanup. Except this, this series of patches work well for me. > > if (!(obj = nodeDeviceObjFindByName(device->name))) > return -1; > @@ -364,46 +343,28 @@ nodeDeviceListCaps(virNodeDevicePtr device, > if (virNodeDeviceListCapsEnsureACL(device->conn, def) < 0) > goto cleanup; > > - for (caps = def->caps; caps && ncaps < maxnames; caps = caps->next) { > - if (VIR_STRDUP(names[ncaps++], virNodeDevCapTypeToString(caps- > >data.type)) < 0) > + if ((ncaps = virNodeDeviceCapsListExport(def, &list)) < 0) > + goto cleanup; > + > + if (ncaps > maxnames) > + ncaps = maxnames; > + > + for (i = 0; i < ncaps; i++) { > + if (VIR_STRDUP(names[i], virNodeDevCapTypeToString(list[i])) < > + 0) > goto cleanup; > - > - if (caps->data.type == VIR_NODE_DEV_CAP_SCSI_HOST) { > - if (ncaps < maxnames && > - caps->data.scsi_host.flags & > - VIR_NODE_DEV_CAP_FLAG_HBA_FC_HOST) { > - if (VIR_STRDUP(names[ncaps++], > - > virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_FC_HOST)) < 0) > - goto cleanup; > - } > - > - if (ncaps < maxnames && > - caps->data.scsi_host.flags & > - VIR_NODE_DEV_CAP_FLAG_HBA_VPORT_OPS) { > - if (VIR_STRDUP(names[ncaps++], > - > virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_VPORTS)) < 0) > - goto cleanup; > - } > - } > - if (caps->data.type == VIR_NODE_DEV_CAP_PCI_DEV) { > - if (ncaps < maxnames && > - caps->data.pci_dev.flags & > - VIR_NODE_DEV_CAP_FLAG_PCI_MDEV) { > - if (VIR_STRDUP(names[ncaps++], > - > virNodeDevCapTypeToString(VIR_NODE_DEV_CAP_MDEV_TYPES)) < 0) > - goto cleanup; > - } > - } > } > + > ret = ncaps; > > cleanup: > virNodeDeviceObjEndAPI(&obj); > - if (ret == -1) { > - --ncaps; > - while (--ncaps >= 0) > - VIR_FREE(names[ncaps]); > + if (ret < 0) { > + size_t j; > + for (j = 0; j < i; j++) > + VIR_FREE(names[j]); > } > + > + VIR_FREE(list); > return ret; > } > > -- > 2.13.6 > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list