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;
> }
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list