[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 04/11] util: Add util to find PCI device address by its vendor and product IDs



On 19/06/13 23:03, John Ferlan wrote:
On 06/07/2013 01:03 PM, Osier Yang wrote:
As the first user of virTraverseDirectory, it falls through to the 2
depth from "/sys/devices", and returns the address of the PCI device
of which both "vendor" and "device" have the specified value.  See the
test for an example.
Two patches ago we the commit message mentions 'udev' backend and 'hal'
backend styles - is this covering those cases?  Are there "other" styles
that need to be considered?

The util is to translate the PCI device name represented by HAL backend,
e.g. pci_$vendor_$product into pci address. I should mention it here.


I suppose as long as the layout never
changes things will work...  assuming this is the only layout that needs
to be considered.



---
  src/libvirt_private.syms                           |   1 +
  src/util/virutil.c                                 | 150 +++++++++++++++++++++
  src/util/virutil.h                                 |   5 +
  tests/sysfs/devices/pci0000:00/0000:00:1f.1/device |   1 +
  tests/sysfs/devices/pci0000:00/0000:00:1f.1/vendor |   1 +
  tests/sysfs/devices/pci0000:00/0000:00:1f.2/device |   1 +
  tests/sysfs/devices/pci0000:00/0000:00:1f.2/vendor |   1 +
  tests/sysfs/devices/pci0000:00/0000:00:1f.4/device |   1 +
  tests/sysfs/devices/pci0000:00/0000:00:1f.4/vendor |   1 +
  tests/utiltest.c                                   |  41 +++++-
  10 files changed, 201 insertions(+), 2 deletions(-)
  create mode 100644 tests/sysfs/devices/pci0000:00/0000:00:1f.1/device
  create mode 100644 tests/sysfs/devices/pci0000:00/0000:00:1f.1/vendor
  create mode 100644 tests/sysfs/devices/pci0000:00/0000:00:1f.2/device
  create mode 100644 tests/sysfs/devices/pci0000:00/0000:00:1f.2/vendor
  create mode 100644 tests/sysfs/devices/pci0000:00/0000:00:1f.4/device
  create mode 100644 tests/sysfs/devices/pci0000:00/0000:00:1f.4/vendor

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fe182e8..f6ae42d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1932,6 +1932,7 @@ virDoubleToStr;
  virEnumFromString;
  virEnumToString;
  virFindFCHostCapableVport;
+virFindPCIDeviceByVPD;
  virFormatIntDecimal;
  virGetDeviceID;
  virGetDeviceUnprivSGIO;
diff --git a/src/util/virutil.c b/src/util/virutil.c
index c1938f9..8309568 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1996,6 +1996,147 @@ cleanup:
      VIR_FREE(vports);
      return ret;
  }
+
+struct virFindPCIDeviceByVPDData {
+     const char *filename;
+     const char *value;
+};
+
+static int
+virFindPCIDeviceByVPDCallback(const char *fpath,
+                              void *opaque)
+{
+    struct virFindPCIDeviceByVPDData *data = opaque;
+    char *p = NULL;
+    char *buf = NULL;
+    int ret = -1;
+
+    p = strrchr(fpath, '/');
+    p++;
+
+    if (STRNEQ(p, data->filename))
+        return -1;
+
+    if (virFileReadAll(fpath, 1024, &buf) < 0)
+        return -1;
+
+    if ((p = strchr(buf, '\n')))
+        *p = '\0';
+
+    if (STRNEQ(buf, data->value))
+        goto cleanup;
+
+    ret = 0;
+cleanup:
+    VIR_FREE(buf);
+    return ret;
+}
+
+# define SYSFS_DEVICES_PATH "/sys/devices"
+
+/**
+ * virFindPCIDeviceByVPD:
+ * @sysfs_prefix: The directory path where starts to traverse, defaults
+ *                to SYSFS_DEVICES_PATH.
+ * @vendor: vendor ID in string
+ * @product: product ID in string
+ *
+ * Traverse specified directory tree (@sysfs_prefix) to find out the PCI
+ * device address (e.g. "0000\:00\:1f.2") by @vendor and @product.
+ *
+ * Return the PCI device address as string on success, or NULL on
+ * failure.
+ */
+char *
+virFindPCIDeviceByVPD(const char *sysfs_prefix,
+                      const char *vendor,
+                      const char *product)
+{
+    const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_DEVICES_PATH;
+    char **vendor_paths = NULL;
+    int nvendor_paths = -1;
+    char **product_paths = NULL;
+    int nproduct_paths = -1;
+    unsigned int flags = 0;
+    char *ret = NULL;
+    bool found = false;
+    char *p1 = NULL;
+    char *p2 = NULL;
+    int i, j;
+
+    flags |= (VIR_TRAVERSE_DIRECTORY_IGNORE_HIDDEN_FILES |
+              VIR_TRAVERSE_DIRECTORY_FALL_THROUGH);
+
+    struct virFindPCIDeviceByVPDData vendor_data = {
+        .filename = "vendor",
+        .value = vendor,
+    };
+
+    struct virFindPCIDeviceByVPDData product_data = {
+        .filename = "device",
+        .value = product,
+    };
+
+    if ((nvendor_paths = virTraverseDirectory(prefix, 2, flags,
+                                              virFindPCIDeviceByVPDCallback,
+                                              NULL, &vendor_data,
+                                              &vendor_paths)) < 0 ||
+        (nproduct_paths = virTraverseDirectory(prefix, 2, flags,
+                                               virFindPCIDeviceByVPDCallback,
+                                               NULL, &product_data,
+                                               &product_paths)) < 0)
+        goto cleanup;
+
+    if (!nvendor_paths || !nproduct_paths) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unable to find PCI device with vendor '%s' "
+                         "product '%s' in '%s'"), vendor, product, prefix);
+        goto cleanup;
+    }
+
+    for (i = 0; i < nvendor_paths; i++) {
+        p1 = strrchr(vendor_paths[i], '/');
+
+        for (j = 0; j < nproduct_paths; j++) {
+            p2 = strrchr(product_paths[j], '/');
+
+            if ((p1 - vendor_paths[i]) != (p2 - product_paths[j]))
+                continue;
+
+            if (STREQLEN(vendor_paths[i], product_paths[j],
+                         p1 - vendor_paths[i])) {
+                found = true;
+                break;
+            }
+        }
+
+        if (found)
+            break;
+    }
+
+    if (found) {
+        p1 = strrchr(vendor_paths[i], '/');
+        *p1 = '\0';
+        p2 = strrchr(vendor_paths[i], '/');
+
+        if (VIR_STRDUP(ret, p2 + 1) < 0)
+            goto cleanup;
+    }
+
+cleanup:
+    if (nvendor_paths > 0) {
+        for (i = 0; i < nvendor_paths; i++)
+            VIR_FREE(vendor_paths[i]);
+        VIR_FREE(vendor_paths);
+    }
+
+    if (nproduct_paths > 0) {
+        for (i = 0; i < nproduct_paths; i++)
+            VIR_FREE(product_paths[i]);
+        VIR_FREE(product_paths);
+    }
+    return ret;
+}
  #else
  int
  virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED,
@@ -2048,6 +2189,15 @@ virFindFCHostCapableVport(const char *sysfs_prefix ATTRIBUTE_UNUSED)
      virReportSystemError(ENOSYS, "%s", _("Not supported on this platform"));
      return NULL;
  }
+
+int
+virFindPCIDeviceByVPD(const char *sysfs_prefix,
+                      const char *vendor,
+                      const char *product)
+{
+    virReportSystemError(ENOSYS, "%s", _("Not supported on this platform"));
+    return NULL;
+}
  #endif /* __linux__ */
/**
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 6c46f23..99d3ea2 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -218,4 +218,9 @@ int virTraverseDirectory(const char *dirpath,
                           char ***filepaths);
      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3)
+char *virFindPCIDeviceByVPD(const char *sysfs_prefix,
+                            const char *vendor,
+                            const char *product)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
In the code if sysfs_prefix is NULL, then you use a constant path, so I
think you have to change these to 2 & 3 respectively

yes.


+
  #endif /* __VIR_UTIL_H__ */
diff --git a/tests/sysfs/devices/pci0000:00/0000:00:1f.1/device b/tests/sysfs/devices/pci0000:00/0000:00:1f.1/device
new file mode 100644
index 0000000..9f26c70
--- /dev/null
+++ b/tests/sysfs/devices/pci0000:00/0000:00:1f.1/device
@@ -0,0 +1 @@
+0x1e04
diff --git a/tests/sysfs/devices/pci0000:00/0000:00:1f.1/vendor b/tests/sysfs/devices/pci0000:00/0000:00:1f.1/vendor
new file mode 100644
index 0000000..aee5132
--- /dev/null
+++ b/tests/sysfs/devices/pci0000:00/0000:00:1f.1/vendor
@@ -0,0 +1 @@
+0x8087
diff --git a/tests/sysfs/devices/pci0000:00/0000:00:1f.2/device b/tests/sysfs/devices/pci0000:00/0000:00:1f.2/device
new file mode 100644
index 0000000..a0681c4
--- /dev/null
+++ b/tests/sysfs/devices/pci0000:00/0000:00:1f.2/device
@@ -0,0 +1 @@
+0x1e03
diff --git a/tests/sysfs/devices/pci0000:00/0000:00:1f.2/vendor b/tests/sysfs/devices/pci0000:00/0000:00:1f.2/vendor
new file mode 100644
index 0000000..ce6dc4d
--- /dev/null
+++ b/tests/sysfs/devices/pci0000:00/0000:00:1f.2/vendor
@@ -0,0 +1 @@
+0x8086
diff --git a/tests/sysfs/devices/pci0000:00/0000:00:1f.4/device b/tests/sysfs/devices/pci0000:00/0000:00:1f.4/device
new file mode 100644
index 0000000..3c7202c
--- /dev/null
+++ b/tests/sysfs/devices/pci0000:00/0000:00:1f.4/device
@@ -0,0 +1 @@
+0x1e08
diff --git a/tests/sysfs/devices/pci0000:00/0000:00:1f.4/vendor b/tests/sysfs/devices/pci0000:00/0000:00:1f.4/vendor
new file mode 100644
index 0000000..ce6dc4d
--- /dev/null
+++ b/tests/sysfs/devices/pci0000:00/0000:00:1f.4/vendor
@@ -0,0 +1 @@
+0x8086
diff --git a/tests/utiltest.c b/tests/utiltest.c
index 9d18652..8d3dbfa 100644
--- a/tests/utiltest.c
+++ b/tests/utiltest.c
@@ -9,7 +9,12 @@
  #include "viralloc.h"
  #include "testutils.h"
  #include "virutil.h"
+#include "virstring.h"
+#include "virfile.h"
+static char *sysfs_devices_prefix;
+
Since there's a VIR_FREE() on this below:

static char *sysfs_devices_prefix = NULL;

+#define TEST_SYSFS_DEVICES_PREFIX sysfs_devices_prefix
static void
  testQuietError(void *userData ATTRIBUTE_UNUSED,
@@ -150,14 +155,43 @@ testParseVersionString(const void *data ATTRIBUTE_UNUSED)
      return 0;
  }
-
-
+static int
+testFindPCIDeviceByVPD(const void *data ATTRIBUTE_UNUSED)
+{
+    char *addr = NULL;
+    const char *expected_addr = "0000:00:1f.2";
+    const char *vendor = "0x8086";
+    const char *device = "0x1e03";
+    int ret = -1;
+
+    if (!(addr = virFindPCIDeviceByVPD(TEST_SYSFS_DEVICES_PREFIX,
+                                       vendor, device)))
+        return -1;
+
+    if (STRNEQ(addr, expected_addr))
+        goto cleanup;
Since we're reusing this:

        VIR_FREE(addr);


Seems to be ACK-able with the nits I've noted fixed.

John
+
+    if ((addr = virFindPCIDeviceByVPD(TEST_SYSFS_DEVICES_PREFIX,
+                                      "0x7076", "0x2434")))
+        goto cleanup;
+
+    ret = 0;
+cleanup:
+    VIR_FREE(addr);
+    return ret;
+}
static int
  mymain(void)
  {
      int result = 0;
+ if (virAsprintf(&sysfs_devices_prefix, "%s/%s", abs_srcdir,
+                    "sysfs/devices/") < 0) {
+        result = -1;
+        goto cleanup;
+    }
+
      virSetErrorFunc(NULL, testQuietError);
#define DO_TEST(_name) \
@@ -171,7 +205,10 @@ mymain(void)
      DO_TEST(IndexToDiskName);
      DO_TEST(DiskNameToIndex);
      DO_TEST(ParseVersionString);
+    DO_TEST(FindPCIDeviceByVPD);
+cleanup:
+    VIR_FREE(sysfs_devices_prefix);
      return result == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
  }


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]