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

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?

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

> + > #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; > } > >