[PATCH 0/6] util: Refactor fetching of virtual function list
Note that this applies on top of the 'VIR_APPEND_ELEMENT' refactor I've sent earlier. Pipeline: https://gitlab.com/pipo.sk/libvirt/-/pipelines/348524810 Peter Krempa (6): virNetDevGetVirtualFunctions: Remove 'max_vfs' argument virPCIGetVirtualFunctions: Simplify cleanup of returned data virPCIGetVirtualFunctionIndex: Refactor cleanup virPCIGetNetName: Make 'physPortID' argument const virPCIGetVirtualFunctions: Fetch also network interface name if needed virNetDevGetVirtualFunctions: Directly return virPCIVirtualFunctionList src/conf/node_device_conf.c | 16 +++-- src/libvirt_private.syms | 2 + src/network/bridge_driver.c | 23 ++----- src/util/virnetdev.c | 65 +++--------------- src/util/virnetdev.h | 8 +-- src/util/virpci.c | 127 +++++++++++++++++++++--------------- src/util/virpci.h | 24 +++++-- 7 files changed, 122 insertions(+), 143 deletions(-) -- 2.31.1
The only caller doesn't use it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/network/bridge_driver.c | 3 +-- src/util/virnetdev.c | 11 ++++------- src/util/virnetdev.h | 5 ++--- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c9ee4b1a4e..4ffcecdf9f 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2599,7 +2599,6 @@ static int networkCreateInterfacePool(virNetworkDef *netdef) { size_t numVirtFns = 0; - unsigned int maxVirtFns = 0; char **vfNames = NULL; virPCIDeviceAddress **virtFns; @@ -2610,7 +2609,7 @@ networkCreateInterfacePool(virNetworkDef *netdef) return 0; if ((virNetDevGetVirtualFunctions(netdef->forward.pfs->dev, &vfNames, - &virtFns, &numVirtFns, &maxVirtFns)) < 0) { + &virtFns, &numVirtFns)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not get Virtual functions on %s"), netdef->forward.pfs->dev); diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index fe531a3260..d578becb20 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1225,17 +1225,16 @@ int virNetDevGetVirtualFunctions(const char *pfname, char ***vfname, virPCIDeviceAddress ***virt_fns, - size_t *n_vfname, - unsigned int *max_vfs) + size_t *n_vfname) { int ret = -1; size_t i; g_autofree char *pf_sysfs_device_link = NULL; g_autofree char *pfPhysPortID = NULL; + unsigned int max_vfs; *virt_fns = NULL; *n_vfname = 0; - *max_vfs = 0; if (virNetDevGetPhysPortID(pfname, &pfPhysPortID) < 0) goto cleanup; @@ -1243,8 +1242,7 @@ 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, virt_fns, n_vfname, &max_vfs) < 0) goto cleanup; *vfname = g_new0(char *, *n_vfname); @@ -1480,8 +1478,7 @@ 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, - unsigned int *max_vfs G_GNUC_UNUSED) + size_t *n_vfname 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 b694f4ac35..de786c9789 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -254,10 +254,9 @@ int virNetDevGetPhysPortName(const char *ifname, int virNetDevGetVirtualFunctions(const char *pfname, char ***vfname, virPCIDeviceAddress ***virt_fns, - size_t *n_vfname, - unsigned int *max_vfs) + size_t *n_vfname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) - ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) G_GNUC_WARN_UNUSED_RESULT; + ATTRIBUTE_NONNULL(4) G_GNUC_WARN_UNUSED_RESULT; int virNetDevSaveNetConfig(const char *linkdev, int vf, const char *stateDir, -- 2.31.1
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@redhat.com> --- 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
The 'ret' variable and 'out' label can be removed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virpci.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/util/virpci.c b/src/util/virpci.c index 2afbf40af3..7059b29ec3 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2409,31 +2409,28 @@ virPCIGetVirtualFunctionIndex(const char *pf_sysfs_device_link, const char *vf_sysfs_device_link, int *vf_index) { - int ret = -1; size_t i; g_autofree virPCIDeviceAddress *vf_bdf = NULL; g_autoptr(virPCIVirtualFunctionList) virt_fns = NULL; if (!(vf_bdf = virPCIGetDeviceAddressFromSysfsLink(vf_sysfs_device_link))) - return ret; + return -1; 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; + return -1; } for (i = 0; i < virt_fns->nfunctions; i++) { if (virPCIDeviceAddressEqual(vf_bdf, virt_fns->functions[i].addr)) { *vf_index = i; - ret = 0; - break; + return 0; } } - out: - return ret; + return -1; } /* -- 2.31.1
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- 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
'virNetDevGetVirtualFunctions' calls 'virPCIGetVirtualFunctions' and then re-iterates the returned list to fetch the interface names for the returned virtual functions. If we move the fetching of the interface name into virPCIGetVirtualFunctions we can simplify the code and remove a bunch of impossible error states. To accomplish this the function is renamed to 'virPCIGetVirtualFunctionsFull' while keeping a wrapper with original name and if the physical port ID is passed the interface name is fetched too without the need to re-convert the address into a sysfs link. For now 'virNetDevGetVirtualFunctions' still converts the returned data into two lists. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virnetdev.c | 44 ++++++---------------------------------- src/util/virpci.c | 39 ++++++++++++++++++++++++++++------- src/util/virpci.h | 4 ++++ 4 files changed, 43 insertions(+), 45 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 20e74dd5d5..fd02b27c51 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3011,6 +3011,7 @@ virPCIGetPhysicalFunction; virPCIGetVirtualFunctionIndex; virPCIGetVirtualFunctionInfo; virPCIGetVirtualFunctions; +virPCIGetVirtualFunctionsFull; virPCIHeaderTypeFromString; virPCIHeaderTypeToString; virPCIIsVirtualFunction; diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index ea5aba7116..4db3b255f6 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1227,62 +1227,30 @@ virNetDevGetVirtualFunctions(const char *pfname, virPCIDeviceAddress ***virt_fns, size_t *n_vfname) { - int ret = -1; size_t i; g_autofree char *pf_sysfs_device_link = NULL; g_autofree char *pfPhysPortID = NULL; g_autoptr(virPCIVirtualFunctionList) vfs = NULL; - *virt_fns = NULL; - *n_vfname = 0; - if (virNetDevGetPhysPortID(pfname, &pfPhysPortID) < 0) - goto cleanup; + return -1; if (virNetDevSysfsFile(&pf_sysfs_device_link, pfname, "device") < 0) - goto cleanup; + return -1; - if (virPCIGetVirtualFunctions(pf_sysfs_device_link, &vfs) < 0) - goto cleanup; + 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++) { - 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", - _("Failed to get PCI SYSFS file")); - goto cleanup; - } - - if (virPCIGetNetName(pci_sysfs_device_link, 0, - pfPhysPortID, &((*vfname)[i])) < 0) { - goto cleanup; - } - - if (!(*vfname)[i]) - VIR_INFO("VF does not have an interface name"); + vfname[i] = g_steal_pointer(&vfs->functions[i].ifname); } - ret = 0; - - cleanup: - if (ret < 0) { - virStringListFreeCount(*vfname, *n_vfname); - - for (i = 0; i < *n_vfname; i++) - VIR_FREE((*virt_fns)[i]); - VIR_FREE(*virt_fns); - *vfname = NULL; - *n_vfname = 0; - } - return ret; + return 0; } /** diff --git a/src/util/virpci.c b/src/util/virpci.c index 7dbca5f3ff..915a4903ca 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -2257,12 +2257,21 @@ virPCIVirtualFunctionListFree(virPCIVirtualFunctionList *list) for (i = 0; i < list->nfunctions; i++) { g_free(list->functions[i].addr); + g_free(list->functions[i].ifname); } g_free(list); } +int +virPCIGetVirtualFunctions(const char *sysfs_path, + virPCIVirtualFunctionList **vfs) +{ + return virPCIGetVirtualFunctionsFull(sysfs_path, vfs, NULL); +} + + #ifdef __linux__ virPCIDeviceAddress * @@ -2332,12 +2341,20 @@ virPCIGetPhysicalFunction(const char *vf_sysfs_path, } -/* - * Returns virtual functions of a physical function +/** + * virPCIGetVirtualFunctionsFull: + * @sysfs_path: path to physical function sysfs entry + * @vfs: filled with the virtual function data + * @pfPhysPortID: Optional physical port id. If provided the network interface + * name of the VFs is queried too. + * + * + * Returns virtual functions of a physical function. */ int -virPCIGetVirtualFunctions(const char *sysfs_path, - virPCIVirtualFunctionList **vfs) +virPCIGetVirtualFunctionsFull(const char *sysfs_path, + virPCIVirtualFunctionList **vfs, + const char *pfPhysPortID) { g_autofree char *totalvfs_file = NULL; g_autofree char *totalvfs_str = NULL; @@ -2363,7 +2380,7 @@ virPCIGetVirtualFunctions(const char *sysfs_path, do { g_autofree char *device_link = NULL; - struct virPCIVirtualFunction fnc = { NULL }; + struct virPCIVirtualFunction fnc = { NULL, NULL }; /* look for virtfn%d links until one isn't found */ device_link = g_strdup_printf("%s/virtfn%zu", sysfs_path, list->nfunctions); @@ -2378,6 +2395,13 @@ virPCIGetVirtualFunctions(const char *sysfs_path, return -1; } + if (pfPhysPortID) { + if (virPCIGetNetName(device_link, 0, pfPhysPortID, &fnc.ifname) < 0) { + g_free(fnc.addr); + return -1; + } + } + VIR_APPEND_ELEMENT(list->functions, list->nfunctions, fnc); } while (1); @@ -2636,8 +2660,9 @@ virPCIGetPhysicalFunction(const char *vf_sysfs_path G_GNUC_UNUSED, } int -virPCIGetVirtualFunctions(const char *sysfs_path G_GNUC_UNUSED, - virPCIVirtualFunctionList **vfs G_GNUC_UNUSED) +virPCIGetVirtualFunctionsFull(const char *sysfs_path G_GNUC_UNUSED, + virPCIVirtualFunctionList **vfs G_GNUC_UNUSED, + const char *pfPhysPortID 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 60f93fa87d..9a3db6c6d8 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -214,6 +214,7 @@ int virPCIGetPhysicalFunction(const char *vf_sysfs_path, struct virPCIVirtualFunction { virPCIDeviceAddress *addr; + char *ifname; }; struct _virPCIVirtualFunctionList { @@ -226,6 +227,9 @@ typedef struct _virPCIVirtualFunctionList virPCIVirtualFunctionList; void virPCIVirtualFunctionListFree(virPCIVirtualFunctionList *list); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virPCIVirtualFunctionList, virPCIVirtualFunctionListFree); +int virPCIGetVirtualFunctionsFull(const char *sysfs_path, + virPCIVirtualFunctionList **vfs, + const char *pfPhysPortID); int virPCIGetVirtualFunctions(const char *sysfs_path, virPCIVirtualFunctionList **vfs); -- 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@redhat.com> --- 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
On a %A in %Y, Peter Krempa wrote:
Note that this applies on top of the 'VIR_APPEND_ELEMENT' refactor I've sent earlier.
Pipeline: https://gitlab.com/pipo.sk/libvirt/-/pipelines/348524810
Peter Krempa (6): virNetDevGetVirtualFunctions: Remove 'max_vfs' argument virPCIGetVirtualFunctions: Simplify cleanup of returned data virPCIGetVirtualFunctionIndex: Refactor cleanup virPCIGetNetName: Make 'physPortID' argument const virPCIGetVirtualFunctions: Fetch also network interface name if needed virNetDevGetVirtualFunctions: Directly return virPCIVirtualFunctionList
src/conf/node_device_conf.c | 16 +++-- src/libvirt_private.syms | 2 + src/network/bridge_driver.c | 23 ++----- src/util/virnetdev.c | 65 +++--------------- src/util/virnetdev.h | 8 +-- src/util/virpci.c | 127 +++++++++++++++++++++--------------- src/util/virpci.h | 24 +++++-- 7 files changed, 122 insertions(+), 143 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Jano Tomko -
Peter Krempa