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@xxxxxxxxxx> --- 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