On 2013年01月15日 17:56, Osier Yang wrote:
pciGetVirtualFunctions returns 0 even if there is no "virtfn"
entry under the device sysfs path.
And pciGetVirtualFunctions 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 (!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.
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 pciGetVirtualFunctions 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 = pciGetVirtualFunctions(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;
---
src/node_device/node_device_hal.c | 12 +++++++--
src/node_device/node_device_udev.c | 11 ++++++--
src/util/virpci.c | 46 +++++++++++++++++++++---------------
3 files changed, 44 insertions(+), 25 deletions(-)
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c
index e64d6ac..6da5873 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -151,9 +151,15 @@ 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)
+ int ret = pciGetVirtualFunctions(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);
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 62f6320..a90217d 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -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 (!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)
+ rc = pciGetVirtualFunctions(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;
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 0fb9923..ee4b3a8 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1968,6 +1968,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;
@@ -1989,45 +1990,52 @@ 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();
- 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 (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;
- } 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;
}
/*
Eh, changes on *_udev.c was lost when committing. With the following
diff squashed in:
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index a90217d..d8637a2 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -491,7 +491,8 @@ static int udevProcessPCI(struct udev_device *device,
/* Out of memory */
if (rc < 0)
goto out;
- else if (!rc && (data->pci_dev.num_virtual_functions > 0))
+
+ if (data->pci_dev.num_virtual_functions > 0)
data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
ret = 0;
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list