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@redhat.com>
---
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;
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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 01/25/2018 10:23 AM, Erik Skultety wrote: > 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@redhat.com> > --- > 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(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
> -----Original Message-----
> From: libvir-list-bounces@redhat.com [mailto:libvir-list-
> bounces@redhat.com] On Behalf Of Erik Skultety
> Sent: Thursday, January 25, 2018 5:24 PM
> To: libvir-list@redhat.com
> Cc: Erik Skultety <eskultet@redhat.com>
> Subject: [libvirt] [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@redhat.com>
> ---
> 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@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.