nodedev: Fix the improper logic when enumerating SRIOV VF

virPCIGetVirtualFunctions returns 0 even if there is no "virtfn"
entry under the device sysfs path.

And virPCIGetVirtualFunctions returns -1 when it fails to get
the PCI config space of one VF, however, with keeping the
the VFs already detected.

That's why udevProcessPCI and gather_pci_cap use logic like:

if (!virPCIGetVirtualFunctions(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.

However, 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
    (given that the successfully detected VFs are kept , it makes
    sense to not give up on the failure of one VF too) with a warning,
    so virPCIGetVirtualFunctions will not return -1 except out of memory.

  * Free the allocated *virtual_functions when out of memory

And thus the logic can be changed to:

    /* Out of memory */
    int ret = virPCIGetVirtualFunctions(syspath,
                                        &data->pci_dev.virtual_functions,
                                        &data->pci_dev.num_virtual_functions);

    if (ret < 0 )
        goto out;
    if (data->pci_dev.num_virtual_functions > 0)
        data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
This commit is contained in:
Osier Yang 2013-03-25 21:10:51 +08:00
parent 96d3086a4f
commit 9a3ff01d7f
3 changed files with 44 additions and 26 deletions

View File

@ -152,12 +152,16 @@ static int gather_pci_cap(LibHalContext *ctx, const char *udi,
&d->pci_dev.physical_function))
d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
if (!virPCIGetVirtualFunctions(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;
int ret = virPCIGetVirtualFunctions(sysfs_path,
&d->pci_dev.virtual_functions,
&d->pci_dev.num_virtual_functions);
if (ret < 0) {
VIR_FREE(sysfs_path);
return -1;
}
if (d->pci_dev.num_virtual_functions > 0)
d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
VIR_FREE(sysfs_path);
}

View File

@ -416,6 +416,7 @@ static int udevProcessPCI(struct udev_device *device,
union _virNodeDevCapData *data = &def->caps->data;
int ret = -1;
char *p;
int rc;
syspath = udev_device_get_syspath(device);
@ -484,9 +485,13 @@ static int udevProcessPCI(struct udev_device *device,
if (!virPCIGetPhysicalFunction(syspath, &data->pci_dev.physical_function))
data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
if (!virPCIGetVirtualFunctions(syspath, &data->pci_dev.virtual_functions,
&data->pci_dev.num_virtual_functions) ||
data->pci_dev.num_virtual_functions > 0)
rc = virPCIGetVirtualFunctions(syspath,
&data->pci_dev.virtual_functions,
&data->pci_dev.num_virtual_functions);
/* Out of memory */
if (rc < 0)
goto out;
else if (!rc && (data->pci_dev.num_virtual_functions > 0))
data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
ret = 0;

View File

@ -1985,6 +1985,7 @@ virPCIGetVirtualFunctions(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;
@ -2006,45 +2007,53 @@ virPCIGetVirtualFunctions(const char *sysfs_path,
*num_virtual_functions = 0;
while ((entry = readdir(dir))) {
if (STRPREFIX(entry->d_name, "virtfn")) {
virPCIDeviceAddress *config_addr = NULL;
if (virBuildPath(&device_link, sysfs_path, entry->d_name) == -1) {
virReportOOMError();
goto out;
goto error;
}
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 (virPCIGetDeviceAddressFromSysfsLink(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;
} else {
(*num_virtual_functions)++;
continue;
}
if (VIR_ALLOC_N(*virtual_functions,
*num_virtual_functions + 1) < 0) {
virReportOOMError();
VIR_FREE(config_addr);
goto error;
}
(*virtual_functions)[*num_virtual_functions] = config_addr;
(*num_virtual_functions)++;
VIR_FREE(device_link);
}
}
ret = 0;
out:
cleanup:
VIR_FREE(device_link);
if (dir)
closedir(dir);
return ret;
error:
if (*virtual_functions) {
for (i = 0; i < *num_virtual_functions; i++)
VIR_FREE((*virtual_functions)[i]);
VIR_FREE(*virtual_functions);
}
goto cleanup;
}
/*