pci: Move some pci sriov helper code out of node device driver to util/pci

This patch moves some of the sriov related pci code from node_device driver
to src/util/pci.[ch]. Some functions had to go thru name and argument list
change to accommodate the move.

Signed-off-by: Roopa Prabhu <roprabhu@cisco.com>
Signed-off-by: Christian Benvenuti <benve@cisco.com>
Signed-off-by: David Wang <dwang2@cisco.com>

src/Makefile.am

@@ -954,7 +954,10 @@ libvirt_driver_nodedev_la_SOURCES = $(NODE_DEVICE_DRIVER_SOURCES)
 libvirt_driver_nodedev_la_CFLAGS = \
 		-I@top_srcdir@/src/conf $(AM_CFLAGS)
 libvirt_driver_nodedev_la_LDFLAGS = $(AM_LDFLAGS)
-libvirt_driver_nodedev_la_LIBADD =
+libvirt_driver_nodedev_la_LIBADD =       \
+		libvirt_util.la          \
+		../gnulib/lib/libgnu.la
+
 if HAVE_HAL
 libvirt_driver_nodedev_la_SOURCES += $(NODE_DEVICE_DRIVER_HAL_SOURCES)
 libvirt_driver_nodedev_la_CFLAGS += $(HAL_CFLAGS)

src/conf/node_device_conf.c

@@ -36,6 +36,7 @@
 #include "util.h"
 #include "buf.h"
 #include "uuid.h"
+#include "pci.h"
 
 #define VIR_FROM_THIS VIR_FROM_NODEDEV
 

src/conf/node_device_conf.h

@@ -82,13 +82,6 @@ enum virNodeDevPCICapFlags {
     VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION		= (1 << 1),
 };
 
-struct pci_config_address {
-    unsigned int domain;
-    unsigned int bus;
-    unsigned int slot;
-    unsigned int function;
-};
-
 typedef struct _virNodeDevCapsDef virNodeDevCapsDef;
 typedef virNodeDevCapsDef *virNodeDevCapsDefPtr;
 struct _virNodeDevCapsDef {

src/node_device/node_device_driver.h

@@ -37,10 +37,6 @@
 # define LINUX_SYSFS_VPORT_CREATE_POSTFIX "/vport_create"
 # define LINUX_SYSFS_VPORT_DELETE_POSTFIX "/vport_delete"
 
-# define SRIOV_FOUND 0
-# define SRIOV_NOT_FOUND 1
-# define SRIOV_ERROR -1
-
 # define LINUX_NEW_DEVICE_WAIT_TIME 60
 
 # ifdef HAVE_HAL
@@ -63,14 +59,6 @@ int check_fc_host_linux(union _virNodeDevCapData *d);
 #  define check_vport_capable(d) check_vport_capable_linux(d)
 int check_vport_capable_linux(union _virNodeDevCapData *d);
 
-#  define get_physical_function(s,d) get_physical_function_linux(s,d)
-int get_physical_function_linux(const char *sysfs_path,
-                                union _virNodeDevCapData *d);
-
-#  define get_virtual_functions(s,d) get_virtual_functions_linux(s,d)
-int get_virtual_functions_linux(const char *sysfs_path,
-                                union _virNodeDevCapData *d);
-
 #  define read_wwn(host, file, wwn) read_wwn_linux(host, file, wwn)
 int read_wwn_linux(int host, const char *file, char **wwn);
 
@@ -78,8 +66,6 @@ int read_wwn_linux(int host, const char *file, char **wwn);
 
 #  define check_fc_host(d)                      (-1)
 #  define check_vport_capable(d)                (-1)
-#  define get_physical_function(sysfs_path, d)
-#  define get_virtual_functions(sysfs_path, d)
 #  define read_wwn(host, file, wwn)
 
 # endif /* __linux__ */

src/node_device/node_device_hal.c

@@ -35,6 +35,7 @@
 #include "datatypes.h"
 #include "memory.h"
 #include "uuid.h"
+#include "pci.h"
 #include "logging.h"
 #include "node_device_driver.h"
 
@@ -146,8 +147,13 @@ static int gather_pci_cap(LibHalContext *ctx, const char *udi,
             (void)virStrToLong_ui(p+1, &p, 16, &d->pci_dev.function);
         }
 
-        get_physical_function(sysfs_path, d);
+        if (!pciGetPhysicalFunction(sysfs_path, &d->pci_dev.physical_function))
-        get_virtual_functions(sysfs_path, d);
+            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)
+            d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
 
         VIR_FREE(sysfs_path);
     }

src/node_device/node_device_linux_sysfs.c

@@ -205,195 +205,4 @@ out:
     return retval;
 }
 
-
-static int logStrToLong_ui(char const *s,
-                           char **end_ptr,
-                           int base,
-                           unsigned int *result)
-{
-    int ret = 0;
-
-    ret = virStrToLong_ui(s, end_ptr, base, result);
-    if (ret != 0) {
-        VIR_ERROR(_("Failed to convert '%s' to unsigned int"), s);
-    } else {
-        VIR_DEBUG("Converted '%s' to unsigned int %u", s, *result);
-    }
-
-    return ret;
-}
-
-
-static int parse_pci_config_address(char *address, struct pci_config_address *bdf)
-{
-    char *p = NULL;
-    int ret = -1;
-
-    if ((address == NULL) || (logStrToLong_ui(address, &p, 16,
-                                              &bdf->domain) == -1)) {
-        goto out;
-    }
-
-    if ((p == NULL) || (logStrToLong_ui(p+1, &p, 16,
-                                        &bdf->bus) == -1)) {
-        goto out;
-    }
-
-    if ((p == NULL) || (logStrToLong_ui(p+1, &p, 16,
-                                        &bdf->slot) == -1)) {
-        goto out;
-    }
-
-    if ((p == NULL) || (logStrToLong_ui(p+1, &p, 16,
-                                        &bdf->function) == -1)) {
-        goto out;
-    }
-
-    ret = 0;
-
-out:
-    return ret;
-}
-
-
-
-
-static int get_sriov_function(const char *device_link,
-                              struct pci_config_address **bdf)
-{
-    char *config_address = NULL;
-    char *device_path = NULL;
-    char errbuf[64];
-    int ret = SRIOV_ERROR;
-
-    VIR_DEBUG("Attempting to resolve device path from device link '%s'",
-              device_link);
-
-    if (!virFileExists(device_link)) {
-
-        VIR_DEBUG("SR IOV function link '%s' does not exist", device_link);
-        /* Not an SR IOV device, not an error, either. */
-        ret = SRIOV_NOT_FOUND;
-
-        goto out;
-
-    }
-
-    device_path = canonicalize_file_name (device_link);
-    if (device_path == NULL) {
-        memset(errbuf, '\0', sizeof(errbuf));
-        VIR_ERROR(_("Failed to resolve device link '%s': '%s'"), device_link,
-                  virStrerror(errno, errbuf, sizeof(errbuf)));
-        goto out;
-    }
-
-    VIR_DEBUG("SR IOV device path is '%s'", device_path);
-    config_address = basename(device_path);
-    if (VIR_ALLOC(*bdf) != 0) {
-        VIR_ERROR(_("Failed to allocate memory for PCI device name"));
-        goto out;
-    }
-
-    if (parse_pci_config_address(config_address, *bdf) != 0) {
-        VIR_ERROR(_("Failed to parse PCI config address '%s'"), config_address);
-        goto out;
-    }
-
-    VIR_DEBUG("SR IOV function %.4x:%.2x:%.2x.%.1x",
-              (*bdf)->domain,
-              (*bdf)->bus,
-              (*bdf)->slot,
-              (*bdf)->function);
-
-    ret = SRIOV_FOUND;
-
-out:
-    VIR_FREE(device_path);
-    return ret;
-}
-
-
-int get_physical_function_linux(const char *sysfs_path,
-                                union _virNodeDevCapData *d ATTRIBUTE_UNUSED)
-{
-    int ret = -1;
-    char *device_link = NULL;
-
-    VIR_DEBUG("Attempting to get SR IOV physical function for device "
-              "with sysfs path '%s'", sysfs_path);
-
-    if (virBuildPath(&device_link, sysfs_path, "physfn") == -1) {
-        virReportOOMError();
-    } else {
-        ret = get_sriov_function(device_link, &d->pci_dev.physical_function);
-        if (ret == SRIOV_FOUND) {
-            d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION;
-        }
-    }
-
-    VIR_FREE(device_link);
-    return ret;
-}
-
-
-int get_virtual_functions_linux(const char *sysfs_path,
-                                union _virNodeDevCapData *d)
-{
-    int ret = -1;
-    DIR *dir = NULL;
-    struct dirent *entry = NULL;
-    char *device_link = NULL;
-
-    VIR_DEBUG("Attempting to get SR IOV virtual functions for device"
-              "with sysfs path '%s'", sysfs_path);
-
-    dir = opendir(sysfs_path);
-    if (dir == NULL) {
-        goto out;
-    }
-
-    while ((entry = readdir(dir))) {
-        if (STRPREFIX(entry->d_name, "virtfn")) {
-            /* This local is just to avoid lines of code much > 80 col. */
-            unsigned int *num_funcs = &d->pci_dev.num_virtual_functions;
-
-            if (virBuildPath(&device_link, sysfs_path, entry->d_name) == -1) {
-                virReportOOMError();
-                goto out;
-            }
-
-            VIR_DEBUG("Number of virtual functions: %d", *num_funcs);
-            if (VIR_REALLOC_N(d->pci_dev.virtual_functions,
-                              (*num_funcs) + 1) != 0) {
-                virReportOOMError();
-                goto out;
-            }
-
-            if (get_sriov_function(device_link,
-                                   &d->pci_dev.virtual_functions[*num_funcs])
-                                   != SRIOV_FOUND) {
-
-                /* We should not get back SRIOV_NOT_FOUND in this
-                 * case, so if we do, it's an error. */
-                VIR_ERROR(_("Failed to get SR IOV function from device link '%s'"),
-                          device_link);
-                goto out;
-            } else {
-                (*num_funcs)++;
-                d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
-            }
-
-            VIR_FREE(device_link);
-        }
-    }
-
-    ret = 0;
-
-out:
-    if (dir)
-        closedir(dir);
-    VIR_FREE(device_link);
-    return ret;
-}
-
 #endif /* __linux__ */

src/node_device/node_device_udev.c

@@ -37,6 +37,7 @@
 #include "uuid.h"
 #include "util.h"
 #include "buf.h"
+#include "pci.h"
 
 #define VIR_FROM_THIS VIR_FROM_NODEDEV
 
@@ -480,8 +481,13 @@ static int udevProcessPCI(struct udev_device *device,
         goto out;
     }
 
-    get_physical_function(syspath, data);
+    if (!pciGetPhysicalFunction(syspath, &data->pci_dev.physical_function))
-    get_virtual_functions(syspath, data);
+        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)
+        data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION;
 
     ret = 0;
 

src/util/pci.c

@@ -32,6 +32,7 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#include <stdlib.h>
 
 #include "logging.h"
 #include "memory.h"
@@ -48,6 +49,10 @@
 #define PCI_ID_LEN 10   /* "XXXX XXXX" */
 #define PCI_ADDR_LEN 13 /* "XXXX:XX:XX.X" */
 
+#define SRIOV_FOUND 0
+#define SRIOV_NOT_FOUND 1
+#define SRIOV_ERROR -1
+
 struct _pciDevice {
     unsigned      domain;
     unsigned      bus;
@@ -1679,3 +1684,228 @@ int pciDeviceIsAssignable(pciDevice *dev,
 
     return 1;
 }
+
+static int
+logStrToLong_ui(char const *s,
+                char **end_ptr,
+                int base,
+                unsigned int *result)
+{
+    int ret = 0;
+
+    ret = virStrToLong_ui(s, end_ptr, base, result);
+    if (ret != 0) {
+        VIR_ERROR(_("Failed to convert '%s' to unsigned int"), s);
+    } else {
+        VIR_DEBUG("Converted '%s' to unsigned int %u", s, *result);
+    }
+
+    return ret;
+}
+
+static int
+pciParsePciConfigAddress(char *address,
+                         struct pci_config_address *bdf)
+{
+    char *p = NULL;
+    int ret = -1;
+
+    if ((address == NULL) || (logStrToLong_ui(address, &p, 16,
+                                              &bdf->domain) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (logStrToLong_ui(p+1, &p, 16,
+                                        &bdf->bus) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (logStrToLong_ui(p+1, &p, 16,
+                                        &bdf->slot) == -1)) {
+        goto out;
+    }
+
+    if ((p == NULL) || (logStrToLong_ui(p+1, &p, 16,
+                                        &bdf->function) == -1)) {
+        goto out;
+    }
+
+    ret = 0;
+
+out:
+    return ret;
+}
+
+static int
+pciGetPciConfigAddressFromSysfsDeviceLink(const char *device_link,
+                                          struct pci_config_address **bdf)
+{
+    char *config_address = NULL;
+    char *device_path = NULL;
+    char errbuf[64];
+    int ret = -1;
+
+    VIR_DEBUG("Attempting to resolve device path from device link '%s'",
+              device_link);
+
+    if (!virFileExists(device_link)) {
+        VIR_DEBUG("sysfs_path '%s' does not exist", device_link);
+        return ret;
+    }
+
+    device_path = canonicalize_file_name (device_link);
+    if (device_path == NULL) {
+        memset(errbuf, '\0', sizeof(errbuf));
+        pciReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Failed to resolve device link '%s': '%s'"),
+                       device_link, virStrerror(errno, errbuf,
+                       sizeof(errbuf)));
+        return ret;
+    }
+
+    config_address = basename(device_path);
+    if (VIR_ALLOC(*bdf) != 0) {
+        virReportOOMError();
+        goto out;
+    }
+
+    if (pciParsePciConfigAddress(config_address, *bdf) != 0) {
+        pciReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Failed to parse PCI config address '%s'"),
+                       config_address);
+        VIR_FREE(*bdf);
+        goto out;
+    }
+
+    VIR_DEBUG("pci_config_address %.4x:%.2x:%.2x.%.1x",
+              (*bdf)->domain,
+              (*bdf)->bus,
+              (*bdf)->slot,
+              (*bdf)->function);
+
+    ret = 0;
+
+out:
+    VIR_FREE(device_path);
+
+    return ret;
+}
+
+#ifdef __linux__
+/*
+ * Returns Physical function given a virtual function
+ */
+int
+pciGetPhysicalFunction(const char *vf_sysfs_path,
+                       struct pci_config_address **physical_function)
+{
+    int ret = -1;
+    char *device_link = NULL;
+
+    VIR_DEBUG("Attempting to get SR IOV physical function for device "
+              "with sysfs path '%s'", vf_sysfs_path);
+
+    if (virBuildPath(&device_link, vf_sysfs_path, "physfn") == -1) {
+        virReportOOMError();
+        return ret;
+    } else {
+        ret = pciGetPciConfigAddressFromSysfsDeviceLink(device_link,
+                                                        physical_function);
+    }
+
+    VIR_FREE(device_link);
+
+    return ret;
+}
+
+/*
+ * Returns virtual functions of a physical function
+ */
+int
+pciGetVirtualFunctions(const char *sysfs_path,
+                       struct pci_config_address ***virtual_functions,
+                       unsigned int *num_virtual_functions)
+{
+    int ret = -1;
+    DIR *dir = NULL;
+    struct dirent *entry = NULL;
+    char *device_link = NULL;
+    char errbuf[64];
+
+    VIR_DEBUG("Attempting to get SR IOV virtual functions for device"
+              "with sysfs path '%s'", sysfs_path);
+
+    dir = opendir(sysfs_path);
+    if (dir == NULL) {
+        memset(errbuf, '\0', sizeof(errbuf));
+        pciReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Failed to open dir '%s': '%s'"),
+                       sysfs_path, virStrerror(errno, errbuf,
+                       sizeof(errbuf)));
+        return ret;
+    }
+
+    *virtual_functions = NULL;
+    *num_virtual_functions = 0;
+    while ((entry = readdir(dir))) {
+        if (STRPREFIX(entry->d_name, "virtfn")) {
+
+            if (virBuildPath(&device_link, sysfs_path, entry->d_name) == -1) {
+                virReportOOMError();
+                goto out;
+            }
+
+            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])) !=
+                SRIOV_FOUND) {
+                /* We should not get back SRIOV_NOT_FOUND in this
+                 * case, so if we do, it's an error. */
+                pciReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("Failed to get SR IOV function from device "
+                               "link '%s'"), device_link);
+                VIR_FREE(device_link);
+                goto out;
+            } else {
+                (*num_virtual_functions)++;
+            }
+            VIR_FREE(device_link);
+        }
+    }
+
+    ret = 0;
+
+out:
+    if (dir)
+        closedir(dir);
+
+    return ret;
+}
+#else
+int
+pciGetPhysicalFunction(const char *vf_sysfs_path,
+                       struct pci_config_address **physical_function)
+{
+    pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciGetPhysicalFunction is not "
+                   "supported on non-linux platforms"));
+    return -1;
+}
+
+int
+pciGetVirtualFunctions(const char *sysfs_path,
+                       struct pci_config_address ***virtual_functions,
+                       unsigned int *num_virtual_functions)
+{
+    pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciGetVirtualFunctions is not "
+                   "supported on non-linux platforms"));
+    return -1;
+}
+#endif /* __linux__ */

src/util/pci.h

@@ -27,6 +27,13 @@
 typedef struct _pciDevice pciDevice;
 typedef struct _pciDeviceList pciDeviceList;
 
+struct pci_config_address {
+    unsigned int domain;
+    unsigned int bus;
+    unsigned int slot;
+    unsigned int function;
+};
+
 pciDevice *pciGetDevice      (unsigned       domain,
                               unsigned       bus,
                               unsigned       slot,
@@ -74,4 +81,11 @@ int pciDeviceIsAssignable(pciDevice *dev,
                           int strict_acs_check);
 int pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher);
 
+int pciGetPhysicalFunction(const char *sysfs_path,
+                           struct pci_config_address **phys_fn);
+
+int pciGetVirtualFunctions(const char *sysfs_path,
+                           struct pci_config_address ***virtual_functions,
+                           unsigned int *num_virtual_functions);
+
 #endif /* __VIR_PCI_H__ */