On 06/07/2013 01:03 PM, Osier Yang wrote:
> By traversing sysfs directory like "/sys/bus/pci/devices/0000:00:1f:2/"
> to find out the scsi host whose "unique_id" has the specified value.
> And returns the host number.
>
> Address like "0000:00:1f:2" will be retrieved from the "parent" of
> scsi_host adapter. E.g.
>
> <adapter type='scsi_host' parent='pci_0000_00_1f_2' unique_id='2'/>
Like my comment in 4/11 this code certainly assumes a specific directory
path.
> ---
> src/libvirt_private.syms | 1 +
> src/util/virutil.c | 128 +++++++++++++++++++++
> src/util/virutil.h | 6 +
> .../ata1/host0/scsi_host/host0/unique_id | 1 +
> .../ata2/host1/scsi_host/host1/unique_id | 1 +
> tests/utiltest.c | 65 +++++++++--
> 6 files changed, 192 insertions(+), 10 deletions(-)
> create mode 100644 tests/sysfs/bus/pci/devices/0000:00:1f.2/ata1/host0/scsi_host/host0/unique_id
> create mode 100644 tests/sysfs/bus/pci/devices/0000:00:1f.2/ata2/host1/scsi_host/host1/unique_id
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index f6ae42d..ce39cc6 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1954,6 +1954,7 @@ virIsCapableVport;
> virIsDevMapperDevice;
> virManageVport;
> virParseNumber;
> +virParseStableScsiHostAddress;
> virParseVersionString;
> virPipeReadUntilEOF;
> virReadFCHost;
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index 8309568..a80574f 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -2137,6 +2137,125 @@ cleanup:
> }
> return ret;
> }
> +
> +struct virParseStableScsiHostAddressData {
> + const char *filename;
> + unsigned int unique_id;
> +};
> +
> +static int
> +virParseStableScsiHostAddressCallback(const char *fpath,
> + void *opaque)
> +{
> + struct virParseStableScsiHostAddressData *data = opaque;
> + unsigned int unique_id;
> + char *buf = NULL;
> + char *p;
> + 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 (virStrToLong_ui(buf, NULL, 10, &unique_id) < 0)
> + goto cleanup;
> +
> + if (unique_id != data->unique_id)
> + goto cleanup;
> +
> + ret = 0;
> +cleanup:
> + VIR_FREE(buf);
> + return ret;
> +}
> +
> +# define SYSFS_BUS_PCI_DEVICES "/sys/bus/pci/devices"
> +
> +/**
> + * virParseStableScsiHostAddress:
> + * @sysfs_prefix: The directory path where starts to traverse, defaults
> + * to SYSFS_BUS_PCI_DEVICES.
> + * @addr: The parent's PCI address
> + * @unique_id: The value of sysfs "unique_id" of the scsi host.
> + *
> + * Returns the host name (e.g. host10) of the scsi host whose parent
> + * has address @addr, and "unique_id" has value @unique_id, on success.
> + * Or NULL on failure.
> + */
> +char *
> +virParseStableScsiHostAddress(const char *sysfs_prefix,
> + const char *address,
> + unsigned int unique_id)
> +{
> + const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_BUS_PCI_DEVICES;
> + unsigned int flags = 0;
> + char **paths = NULL;
> + int npaths = 0;
> + char *dir = NULL;
> + char *p1 = NULL;
> + char *p2 = NULL;
> + char *ret = NULL;
> + int i;
> +
> + struct virParseStableScsiHostAddressData data = {
> + .filename = "unique_id",
> + .unique_id = unique_id,
> + };
> +
> + if (virAsprintf(&dir, "%s/%s", prefix, address) < 0) {
> + virReportOOMError();
> + return NULL;
> + }
> +
> + flags |= (VIR_TRAVERSE_DIRECTORY_IGNORE_HIDDEN_FILES |
> + VIR_TRAVERSE_DIRECTORY_FALL_THROUGH);
> +
> + if ((npaths = virTraverseDirectory(dir, 4, flags,
> + virParseStableScsiHostAddressCallback,
> + NULL, &data, &paths)) < 0) {
> + goto cleanup;
> + } else if (npaths == 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unable to find scsi host with PCI address "
> + "'%s' unique_id '%u'"), address, unique_id);
> + goto cleanup;
> + } else if (npaths != 1) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unexpected multiple paths returned for PCI address "
> + "'%s' unique_id '%u'"), address, unique_id);
Is this the right error? Will it be confusing with errors from
virTraverseDirectory()? I don't think it's multiple paths being returned.
> + goto cleanup;
> + }
> +
> + if (!(p1 = strstr(paths[0], "scsi_host"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("unexpected returned path '%s' for PCI address "
> + "'%s' unique_id '%u'"), paths[0], address, unique_id);
> + goto cleanup;
> + }
> +
> + p1 += strlen("scsi_host");
> + p2 = strrchr(p1, '/');
> +
> + if (VIR_STRNDUP(ret, p1 + 1, p2 - p1 -1) < 0)
> + goto cleanup;
> +
> +cleanup:
> + VIR_FREE(dir);
> + if (npaths > 0) {
> + for (i = 0; i < npaths; i++)
> + VIR_FREE(paths[i]);
> + VIR_FREE(paths);
> + }
> + return ret;
> +}
> #else
> int
> virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED,
> @@ -2198,6 +2317,15 @@ virFindPCIDeviceByVPD(const char *sysfs_prefix,
> virReportSystemError(ENOSYS, "%s", _("Not supported on this platform"));
> return NULL;
> }
> +
> +char *
> +virParseStableScsiHostAddress(const char *sysfs_prefix,
> + const char *address,
> + unsigned int unique_id)
> +{
> + 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 99d3ea2..2747cf1 100644
> --- a/src/util/virutil.h
> +++ b/src/util/virutil.h
> @@ -223,4 +223,10 @@ char *virFindPCIDeviceByVPD(const char *sysfs_prefix,
> const char *product)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>
> +char *virParseStableScsiHostAddress(const char *sysfs_prefix,
> + const char *address,
> + unsigned int unique_id)
> +
> + ATTRIBUTE_NONNULL(2);
> +
> #endif /* __VIR_UTIL_H__ */
> diff --git a/tests/sysfs/bus/pci/devices/0000:00:1f.2/ata1/host0/scsi_host/host0/unique_id b/tests/sysfs/bus/pci/devices/0000:00:1f.2/ata1/host0/scsi_host/host0/unique_id
> new file mode 100644
> index 0000000..d00491f
> --- /dev/null
> +++ b/tests/sysfs/bus/pci/devices/0000:00:1f.2/ata1/host0/scsi_host/host0/unique_id
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/sysfs/bus/pci/devices/0000:00:1f.2/ata2/host1/scsi_host/host1/unique_id b/tests/sysfs/bus/pci/devices/0000:00:1f.2/ata2/host1/scsi_host/host1/unique_id
> new file mode 100644
> index 0000000..0cfbf08
> --- /dev/null
> +++ b/tests/sysfs/bus/pci/devices/0000:00:1f.2/ata2/host1/scsi_host/host1/unique_id
> @@ -0,0 +1 @@
> +2
> diff --git a/tests/utiltest.c b/tests/utiltest.c
> index 8d3dbfa..41fdd7e 100644
> --- a/tests/utiltest.c
> +++ b/tests/utiltest.c
> @@ -11,10 +11,12 @@
> #include "virutil.h"
> #include "virstring.h"
> #include "virfile.h"
> +#include "virerror.h"
>
> -static char *sysfs_devices_prefix;
> +static char *test_sysfs;
Perhaps should have been part of 4/11...
>
> -#define TEST_SYSFS_DEVICES_PREFIX sysfs_devices_prefix
> +#define VIR_FROM_THIS VIR_FROM_NONE
> +#define TEST_SYSFS test_sysfs
>
> static void
> testQuietError(void *userData ATTRIBUTE_UNUSED,
> @@ -158,36 +160,78 @@ testParseVersionString(const void *data ATTRIBUTE_UNUSED)
> 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";
> + char *dir = NULL;
> + char *addr = NULL;
> int ret = -1;
>
> - if (!(addr = virFindPCIDeviceByVPD(TEST_SYSFS_DEVICES_PREFIX,
> - vendor, device)))
> + if (virAsprintf(&dir, "%s/%s", TEST_SYSFS, "devices") < 0) {
> + virReportOOMError();
> return -1;
> + }
> +
> + if (!(addr = virFindPCIDeviceByVPD(dir, vendor, device)))
> + goto cleanup;
>
> if (STRNEQ(addr, expected_addr))
> goto cleanup;
>
> - if ((addr = virFindPCIDeviceByVPD(TEST_SYSFS_DEVICES_PREFIX,
> - "0x7076", "0x2434")))
> + if ((addr = virFindPCIDeviceByVPD(dir, "0x7076", "0x2434")))
> goto cleanup;
>
> ret = 0;
> cleanup:
> + VIR_FREE(dir);
> VIR_FREE(addr);
> return ret;
> }
It seems this part of the change should have been squashed into 4/11 as
it's not related to this particular set of changes.... That includes the
test_sysfs modification...
Seems to be ACKable with nits resolved.
John
>
> static int
> +testParseStableScsiHostAddress(const void *data ATTRIBUTE_UNUSED)
> +{
> + const char *addr = "0000:00:1f.2";
> + unsigned int host0_unique_id = 1;
> + unsigned int host1_unique_id = 2;
> + char *rc0 = NULL;
> + char *rc1 = NULL;
> + char *dir = NULL;
> + int ret = -1;
> +
> + if (virAsprintf(&dir, "%s/%s", TEST_SYSFS, "bus/pci/devices") < 0) {
> + virReportOOMError();
> + return -1;
> + }
> +
> + if (!(rc0 = virParseStableScsiHostAddress(dir, addr,
> + host0_unique_id)))
> + goto cleanup;
> +
> + if (STRNEQ(rc0, "host0"))
> + goto cleanup;
> +
> + if (!(rc1 = virParseStableScsiHostAddress(dir, addr,
> + host1_unique_id)))
> + goto cleanup;
> +
> + if (STRNEQ(rc1, "host1"))
> + goto cleanup;
> + ret = 0;
> +cleanup:
> + VIR_FREE(dir);
> + VIR_FREE(rc0);
> + VIR_FREE(rc1);
> + return ret;
> +}
> +
> +static int
> mymain(void)
> {
> int result = 0;
>
> - if (virAsprintf(&sysfs_devices_prefix, "%s/%s", abs_srcdir,
> - "sysfs/devices/") < 0) {
> + if (virAsprintf(&test_sysfs, "%s/%s", abs_srcdir, "sysfs/") < 0) {
> + virReportOOMError();
> result = -1;
> goto cleanup;
> }
> @@ -206,9 +250,10 @@ mymain(void)
> DO_TEST(DiskNameToIndex);
> DO_TEST(ParseVersionString);
> DO_TEST(FindPCIDeviceByVPD);
> + DO_TEST(ParseStableScsiHostAddress);
>
> cleanup:
> - VIR_FREE(sysfs_devices_prefix);
> + VIR_FREE(test_sysfs);
> return result == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
> }
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list