pciGetVirtualFunctions returns 0 even if there is no "virtfn" entry under the device sysfs path. And pciGetVirtualFunctions returns -1 when it tries to get the PCI config space of the VF, however, with keeping the the VFs already detected. That's why udevProcessPCI and gather_pci_cap use logic like: if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions) || data->pci_dev.num_virtual_functions > 0) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; to tag the PCI device with "virtual_function" cap. This results in a VF will aslo get "virtual_function" cap. This patch fixes it by: * Ignoring the VF which has failure of getting PCI config space of the device (given that the succesfully detected VFs are kept , it makes sense to not give up on the failure of one VF too) with a warning, so pciGetVirtualFunctions will always return 0 except out of memory. * Free the allocated *virtual_functions when out of memory And thus the logic can be changed to: /* Out of memory */ if (pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions) < 0) goto out; else if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, &data->pci_dev.num_virtual_functions) && (data->pci_dev.num_virtual_functions > 0)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; This also fixes a memory leak in the old codes (it realloc the *virtual_functions first, and doesn't shrink it when it fails on getting the PCI config space). --- src/node_device/node_device_hal.c | 13 +++++++++-- src/node_device/node_device_udev.c | 11 +++++++-- src/util/pci.c | 37 +++++++++++++++++++++++------------ 3 files changed, 42 insertions(+), 19 deletions(-) diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index ff73db0..69e6ebf 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -151,10 +151,17 @@ static int gather_pci_cap(LibHalContext *ctx, const char *udi, if (!pciGetPhysicalFunction(sysfs_path, &d->pci_dev.physical_function)) d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; - if (!pciGetVirtualFunctions(sysfs_path, &d->pci_dev.virtual_functions, - &d->pci_dev.num_virtual_functions) || - d->pci_dev.num_virtual_functions > 0) + if (!pciGetVirtualFunctions(sysfs_path, + &d->pci_dev.virtual_functions, + &d->pci_dev.num_virtual_functions) < 0) { + VIR_FREE(sysfs_path); + return -1; + } else if (!pciGetVirtualFunctions(sysfs_path, + &d->pci_dev.virtual_functions, + &d->pci_dev.num_virtual_functions) && + (d->pci_dev.num_virtual_functions > 0)) d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; + } VIR_FREE(sysfs_path); } diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index acd78f2..72b8850 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -484,9 +484,14 @@ static int udevProcessPCI(struct udev_device *device, if (!pciGetPhysicalFunction(syspath, &data->pci_dev.physical_function)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; - if (!pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, - &data->pci_dev.num_virtual_functions) || - data->pci_dev.num_virtual_functions > 0) + /* Out of memory */ + if (pciGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions, + &data->pci_dev.num_virtual_functions) < 0) + goto out; + else if (!pciGetVirtualFunctions(syspath, + &data->pci_dev.virtual_functions, + &data->pci_dev.num_virtual_functions) && + (data->pci_dev.num_virtual_functions > 0)) data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; ret = 0; diff --git a/src/util/pci.c b/src/util/pci.c index d1ad121..3b4d96a 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -1930,6 +1930,7 @@ pciGetVirtualFunctions(const char *sysfs_path, unsigned int *num_virtual_functions) { int ret = -1; + int i; DIR *dir = NULL; struct dirent *entry = NULL; char *device_link = NULL; @@ -1952,6 +1953,7 @@ pciGetVirtualFunctions(const char *sysfs_path, *num_virtual_functions = 0; while ((entry = readdir(dir))) { if (STRPREFIX(entry->d_name, "virtfn")) { + struct pci_config_address *config_addr = NULL; if (virBuildPath(&device_link, sysfs_path, entry->d_name) == -1) { virReportOOMError(); @@ -1960,24 +1962,23 @@ pciGetVirtualFunctions(const char *sysfs_path, VIR_DEBUG("Number of virtual functions: %d", *num_virtual_functions); - if (VIR_REALLOC_N(*virtual_functions, - (*num_virtual_functions) + 1) != 0) { - virReportOOMError(); - VIR_FREE(device_link); - goto out; - } if (pciGetPciConfigAddressFromSysfsDeviceLink(device_link, - &((*virtual_functions)[*num_virtual_functions])) != + &config_addr) != SRIOV_FOUND) { - /* We should not get back SRIOV_NOT_FOUND in this - * case, so if we do, it's an error. */ - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to get SR IOV function from device " - "link '%s'"), device_link); + VIR_WARN("Failed to get SRIOV function from device " + "link '%s'", device_link); VIR_FREE(device_link); - goto out; + continue; } else { + if (VIR_ALLOC_N(*virtual_functions, + *num_virtual_functions + 1) < 0) { + virReportOOMError(); + VIR_FREE(config_addr); + goto out; + } + + (*virtual_functions)[*num_virtual_functions] = config_addr; (*num_virtual_functions)++; } VIR_FREE(device_link); @@ -1985,8 +1986,18 @@ pciGetVirtualFunctions(const char *sysfs_path, } ret = 0; + goto cleanup; out: + if (*virtual_functions) { + for (i = 0; i < *num_virtual_functions; i++) + VIR_FREE((*virtual_functions)[i]); + VIR_FREE(*virtual_functions); + } + +cleanup: + if (device_link) + VIR_FREE(device_link); if (dir) closedir(dir); -- 1.7.7.6