Introduce a struct for holding the list of VFs returned by virPCIGetVirtualFunctions so that we can employ automatic memory clearing and also allow querying more information at once. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/conf/node_device_conf.c | 16 +++++--- src/libvirt_private.syms | 1 + src/util/virnetdev.c | 10 +++-- src/util/virpci.c | 79 ++++++++++++++++++------------------- src/util/virpci.h | 18 +++++++-- 5 files changed, 72 insertions(+), 52 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index d12d97f077..d363ddc22e 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2623,6 +2623,7 @@ static int virNodeDeviceGetPCISRIOVCaps(const char *sysfsPath, virNodeDevCapPCIDev *pci_dev) { + g_autoptr(virPCIVirtualFunctionList) vfs = g_new0(virPCIVirtualFunctionList, 1); size_t i; int ret; @@ -2644,11 +2645,16 @@ virNodeDeviceGetPCISRIOVCaps(const char *sysfsPath, if (pci_dev->physical_function) pci_dev->flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; - ret = virPCIGetVirtualFunctions(sysfsPath, &pci_dev->virtual_functions, - &pci_dev->num_virtual_functions, - &pci_dev->max_virtual_functions); - if (ret < 0) - return ret; + if (virPCIGetVirtualFunctions(sysfsPath, &vfs) < 0) + return -1; + + pci_dev->virtual_functions = g_new0(virPCIDeviceAddress *, vfs->nfunctions); + + for (i = 0; i < vfs->nfunctions; i++) + pci_dev->virtual_functions[i] = g_steal_pointer(&vfs->functions[i].addr); + + pci_dev->num_virtual_functions = vfs->nfunctions; + pci_dev->max_virtual_functions = vfs->maxfunctions; if (pci_dev->num_virtual_functions > 0 || pci_dev->max_virtual_functions > 0) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e4168a06b9..20e74dd5d5 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3016,6 +3016,7 @@ virPCIHeaderTypeToString; virPCIIsVirtualFunction; virPCIStubDriverTypeFromString; virPCIStubDriverTypeToString; +virPCIVirtualFunctionListFree; virZPCIDeviceAddressIsIncomplete; virZPCIDeviceAddressIsPresent; diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index d578becb20..ea5aba7116 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1231,7 +1231,7 @@ virNetDevGetVirtualFunctions(const char *pfname, size_t i; g_autofree char *pf_sysfs_device_link = NULL; g_autofree char *pfPhysPortID = NULL; - unsigned int max_vfs; + g_autoptr(virPCIVirtualFunctionList) vfs = NULL; *virt_fns = NULL; *n_vfname = 0; @@ -1242,14 +1242,18 @@ virNetDevGetVirtualFunctions(const char *pfname, if (virNetDevSysfsFile(&pf_sysfs_device_link, pfname, "device") < 0) goto cleanup; - if (virPCIGetVirtualFunctions(pf_sysfs_device_link, virt_fns, n_vfname, &max_vfs) < 0) + if (virPCIGetVirtualFunctions(pf_sysfs_device_link, &vfs) < 0) goto cleanup; - *vfname = g_new0(char *, *n_vfname); + *vfname = g_new0(char *, vfs->nfunctions); + *virt_fns = g_new0(virPCIDeviceAddress *, vfs->nfunctions); + *n_vfname = vfs->nfunctions; for (i = 0; i < *n_vfname; i++) { g_autofree char *pci_sysfs_device_link = NULL; + virt_fns[i] = g_steal_pointer(&vfs->functions[i].addr); + if (virPCIDeviceAddressGetSysfsFile((*virt_fns)[i], &pci_sysfs_device_link) < 0) { virReportSystemError(ENOSYS, "%s", diff --git a/src/util/virpci.c b/src/util/virpci.c index 67c3896dd5..2afbf40af3 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2247,6 +2247,22 @@ virZPCIDeviceAddressIsPresent(const virZPCIDeviceAddress *addr) } +void +virPCIVirtualFunctionListFree(virPCIVirtualFunctionList *list) +{ + size_t i; + + if (!list) + return; + + for (i = 0; i < list->nfunctions; i++) { + g_free(list->functions[i].addr); + } + + g_free(list); +} + + #ifdef __linux__ virPCIDeviceAddress * @@ -2321,62 +2337,54 @@ virPCIGetPhysicalFunction(const char *vf_sysfs_path, */ int virPCIGetVirtualFunctions(const char *sysfs_path, - virPCIDeviceAddress ***virtual_functions, - size_t *num_virtual_functions, - unsigned int *max_virtual_functions) + virPCIVirtualFunctionList **vfs) { - size_t i; g_autofree char *totalvfs_file = NULL; g_autofree char *totalvfs_str = NULL; - g_autofree virPCIDeviceAddress *config_addr = NULL; + g_autoptr(virPCIVirtualFunctionList) list = g_new0(virPCIVirtualFunctionList, 1); - *virtual_functions = NULL; - *num_virtual_functions = 0; - *max_virtual_functions = 0; + *vfs = NULL; totalvfs_file = g_strdup_printf("%s/sriov_totalvfs", sysfs_path); if (virFileExists(totalvfs_file)) { char *end = NULL; /* so that terminating \n doesn't create error */ + unsigned long long maxfunctions = 0; if (virFileReadAll(totalvfs_file, 16, &totalvfs_str) < 0) - goto error; - if (virStrToLong_ui(totalvfs_str, &end, 10, max_virtual_functions) < 0) { + return -1; + if (virStrToLong_ull(totalvfs_str, &end, 10, &maxfunctions) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unrecognized value in %s: %s"), totalvfs_file, totalvfs_str); - goto error; + return -1; } + list->maxfunctions = maxfunctions; } do { g_autofree char *device_link = NULL; + struct virPCIVirtualFunction fnc = { NULL }; + /* look for virtfn%d links until one isn't found */ - device_link = g_strdup_printf("%s/virtfn%zu", sysfs_path, - *num_virtual_functions); + device_link = g_strdup_printf("%s/virtfn%zu", sysfs_path, list->nfunctions); if (!virFileExists(device_link)) break; - if (!(config_addr = virPCIGetDeviceAddressFromSysfsLink(device_link))) { + if (!(fnc.addr = virPCIGetDeviceAddressFromSysfsLink(device_link))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to get SRIOV function from device link '%s'"), device_link); - goto error; + return -1; } - VIR_APPEND_ELEMENT(*virtual_functions, *num_virtual_functions, config_addr); + VIR_APPEND_ELEMENT(list->functions, list->nfunctions, fnc); } while (1); - VIR_DEBUG("Found %zu virtual functions for %s", - *num_virtual_functions, sysfs_path); - return 0; + VIR_DEBUG("Found %zu virtual functions for %s", list->nfunctions, sysfs_path); - error: - for (i = 0; i < *num_virtual_functions; i++) - VIR_FREE((*virtual_functions)[i]); - VIR_FREE(*virtual_functions); - *num_virtual_functions = 0; - return -1; + *vfs = g_steal_pointer(&list); + return 0; } @@ -2403,24 +2411,21 @@ virPCIGetVirtualFunctionIndex(const char *pf_sysfs_device_link, { int ret = -1; size_t i; - size_t num_virt_fns = 0; - unsigned int max_virt_fns = 0; g_autofree virPCIDeviceAddress *vf_bdf = NULL; - virPCIDeviceAddress **virt_fns = NULL; + g_autoptr(virPCIVirtualFunctionList) virt_fns = NULL; if (!(vf_bdf = virPCIGetDeviceAddressFromSysfsLink(vf_sysfs_device_link))) return ret; - if (virPCIGetVirtualFunctions(pf_sysfs_device_link, &virt_fns, - &num_virt_fns, &max_virt_fns) < 0) { + if (virPCIGetVirtualFunctions(pf_sysfs_device_link, &virt_fns) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Error getting physical function's '%s' " "virtual_functions"), pf_sysfs_device_link); goto out; } - for (i = 0; i < num_virt_fns; i++) { - if (virPCIDeviceAddressEqual(vf_bdf, virt_fns[i])) { + for (i = 0; i < virt_fns->nfunctions; i++) { + if (virPCIDeviceAddressEqual(vf_bdf, virt_fns->functions[i].addr)) { *vf_index = i; ret = 0; break; @@ -2428,12 +2433,6 @@ virPCIGetVirtualFunctionIndex(const char *pf_sysfs_device_link, } out: - - /* free virtual functions */ - for (i = 0; i < num_virt_fns; i++) - VIR_FREE(virt_fns[i]); - - VIR_FREE(virt_fns); return ret; } @@ -2641,9 +2640,7 @@ virPCIGetPhysicalFunction(const char *vf_sysfs_path G_GNUC_UNUSED, int virPCIGetVirtualFunctions(const char *sysfs_path G_GNUC_UNUSED, - virPCIDeviceAddress ***virtual_functions G_GNUC_UNUSED, - size_t *num_virtual_functions G_GNUC_UNUSED, - unsigned int *max_virtual_functions G_GNUC_UNUSED) + virPCIVirtualFunctionList **vfs G_GNUC_UNUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported)); return -1; diff --git a/src/util/virpci.h b/src/util/virpci.h index 1fa6c58d50..ce6a056c9c 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -212,10 +212,22 @@ virPCIGetDeviceAddressFromSysfsLink(const char *device_link); int virPCIGetPhysicalFunction(const char *vf_sysfs_path, virPCIDeviceAddress **pf); +struct virPCIVirtualFunction { + virPCIDeviceAddress *addr; +}; + +struct _virPCIVirtualFunctionList { + struct virPCIVirtualFunction *functions; + size_t nfunctions; + size_t maxfunctions; +}; +typedef struct _virPCIVirtualFunctionList virPCIVirtualFunctionList; + +void virPCIVirtualFunctionListFree(virPCIVirtualFunctionList *list); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virPCIVirtualFunctionList, virPCIVirtualFunctionListFree); + int virPCIGetVirtualFunctions(const char *sysfs_path, - virPCIDeviceAddress ***virtual_functions, - size_t *num_virtual_functions, - unsigned int *max_virtual_functions); + virPCIVirtualFunctionList **vfs); int virPCIIsVirtualFunction(const char *vf_sysfs_device_link); -- 2.31.1

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/util/virpci.c | 4 ++-- src/util/virpci.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/virpci.c b/src/util/virpci.c index 7059b29ec3..7dbca5f3ff 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2461,7 +2461,7 @@ virPCIDeviceAddressGetSysfsFile(virPCIDeviceAddress *addr, int virPCIGetNetName(const char *device_link_sysfs_path, size_t idx, - char *physPortID, + const char *physPortID, char **netname) { g_autofree char *pcidev_sysfs_net_path = NULL; @@ -2672,7 +2672,7 @@ virPCIDeviceAddressGetSysfsFile(virPCIDeviceAddress *dev G_GNUC_UNUSED, int virPCIGetNetName(const char *device_link_sysfs_path G_GNUC_UNUSED, size_t idx G_GNUC_UNUSED, - char *physPortID G_GNUC_UNUSED, + const char *physPortID G_GNUC_UNUSED, char **netname G_GNUC_UNUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported)); diff --git a/src/util/virpci.h b/src/util/virpci.h index ce6a056c9c..60f93fa87d 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -240,7 +240,7 @@ int virPCIDeviceAddressGetSysfsFile(virPCIDeviceAddress *addr, int virPCIGetNetName(const char *device_link_sysfs_path, size_t idx, - char *physPortID, + const char *physPortID, char **netname); bool virPCIDeviceAddressIsValid(virPCIDeviceAddress *addr, -- 2.31.1

Remove the conversion from virPCIVirtualFunctionList which encapsulates the list of virtual functions to two disjunct arrays. This greatly simplifies the fetching of the parameters as well as cleanup in the caller. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/network/bridge_driver.c | 22 ++++++---------------- src/util/virnetdev.c | 26 ++++---------------------- src/util/virnetdev.h | 7 ++----- 3 files changed, 12 insertions(+), 43 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 4ffcecdf9f..2555119892 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2598,29 +2598,25 @@ networkShutdownNetworkBridge(virNetworkObj *obj G_GNUC_UNUSED) static int networkCreateInterfacePool(virNetworkDef *netdef) { - size_t numVirtFns = 0; - char **vfNames = NULL; - virPCIDeviceAddress **virtFns; - + g_autoptr(virPCIVirtualFunctionList) vfs = NULL; int ret = -1; size_t i; if (netdef->forward.npfs == 0 || netdef->forward.nifs > 0) return 0; - if ((virNetDevGetVirtualFunctions(netdef->forward.pfs->dev, &vfNames, - &virtFns, &numVirtFns)) < 0) { + if (virNetDevGetVirtualFunctions(netdef->forward.pfs->dev, &vfs) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not get Virtual functions on %s"), netdef->forward.pfs->dev); goto cleanup; } - netdef->forward.ifs = g_new0(virNetworkForwardIfDef, numVirtFns); + netdef->forward.ifs = g_new0(virNetworkForwardIfDef, vfs->nfunctions); - for (i = 0; i < numVirtFns; i++) { - virPCIDeviceAddress *thisVirtFn = virtFns[i]; - const char *thisName = vfNames[i]; + for (i = 0; i < vfs->nfunctions; i++) { + virPCIDeviceAddress *thisVirtFn = vfs->functions[i].addr; + const char *thisName = vfs->functions[i].ifname; virNetworkForwardIfDef *thisIf = &netdef->forward.ifs[netdef->forward.nifs]; @@ -2689,12 +2685,6 @@ networkCreateInterfacePool(virNetworkDef *netdef) if (netdef->forward.nifs == 0) g_clear_pointer(&netdef->forward.ifs, g_free); - for (i = 0; i < numVirtFns; i++) { - g_free(vfNames[i]); - g_free(virtFns[i]); - } - g_free(vfNames); - g_free(virtFns); return ret; } diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 4db3b255f6..5b4c585716 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1213,24 +1213,17 @@ virNetDevGetPhysPortName(const char *ifname, /** * virNetDevGetVirtualFunctions: - * * @pfname : name of the physical function interface name - * @vfname: array that will hold the interface names of the virtual_functions - * @n_vfname: pointer to the number of virtual functions + * @vfs: Filled with struct describing the virtual functions of @pfname * * Returns 0 on success and -1 on failure */ - int virNetDevGetVirtualFunctions(const char *pfname, - char ***vfname, - virPCIDeviceAddress ***virt_fns, - size_t *n_vfname) + virPCIVirtualFunctionList **vfs) { - size_t i; g_autofree char *pf_sysfs_device_link = NULL; g_autofree char *pfPhysPortID = NULL; - g_autoptr(virPCIVirtualFunctionList) vfs = NULL; if (virNetDevGetPhysPortID(pfname, &pfPhysPortID) < 0) return -1; @@ -1238,18 +1231,9 @@ virNetDevGetVirtualFunctions(const char *pfname, if (virNetDevSysfsFile(&pf_sysfs_device_link, pfname, "device") < 0) return -1; - if (virPCIGetVirtualFunctionsFull(pf_sysfs_device_link, &vfs, pfPhysPortID) < 0) + if (virPCIGetVirtualFunctionsFull(pf_sysfs_device_link, vfs, pfPhysPortID) < 0) return -1; - *vfname = g_new0(char *, vfs->nfunctions); - *virt_fns = g_new0(virPCIDeviceAddress *, vfs->nfunctions); - *n_vfname = vfs->nfunctions; - - for (i = 0; i < *n_vfname; i++) { - virt_fns[i] = g_steal_pointer(&vfs->functions[i].addr); - vfname[i] = g_steal_pointer(&vfs->functions[i].ifname); - } - return 0; } @@ -1448,9 +1432,7 @@ virNetDevGetPhysPortName(const char *ifname G_GNUC_UNUSED, int virNetDevGetVirtualFunctions(const char *pfname G_GNUC_UNUSED, - char ***vfname G_GNUC_UNUSED, - virPCIDeviceAddress ***virt_fns G_GNUC_UNUSED, - size_t *n_vfname G_GNUC_UNUSED) + virPCIVirtualFunctionList **vfs G_GNUC_UNUSED) { virReportSystemError(ENOSYS, "%s", _("Unable to get virtual functions on this platform")); diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index de786c9789..790fae5f7a 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -252,11 +252,8 @@ int virNetDevGetPhysPortName(const char *ifname, G_GNUC_WARN_UNUSED_RESULT; int virNetDevGetVirtualFunctions(const char *pfname, - char ***vfname, - virPCIDeviceAddress ***virt_fns, - size_t *n_vfname) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) - ATTRIBUTE_NONNULL(4) G_GNUC_WARN_UNUSED_RESULT; + virPCIVirtualFunctionList **vfs) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; int virNetDevSaveNetConfig(const char *linkdev, int vf, const char *stateDir, -- 2.31.1