[libvirt] [REPOST 5/8] virutil: Introduce virReadSCSIUniqueId

[Michal Privoznik]

On 08.07.2014 13:54, John Ferlan wrote:
> Introduce a new function to read the current scsi_host entry and return
> the value found in the 'unique_id' file.
>
> Add a 'scsihosttest' test (similar to the fchosttest, but incorporating some
> of the concepts of the mocked pci test library) in order to read the
> unique_id file like would be found in the /sys/class/scsi_host tree.
>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>   src/libvirt_private.syms |   1 +
>   src/util/virutil.c       |  53 ++++++++++
>   src/util/virutil.h       |   4 +
>   tests/Makefile.am        |   7 ++
>   tests/scsihosttest.c     | 254 +++++++++++++++++++++++++++++++++++++++++++++++
>   5 files changed, 319 insertions(+)
>   create mode 100644 tests/scsihosttest.c
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 6d7bf41..bf365ac 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2119,6 +2119,7 @@ virParseOwnershipIds;
>   virParseVersionString;
>   virPipeReadUntilEOF;
>   virReadFCHost;
> +virReadSCSIUniqueId;
>   virScaleInteger;
>   virSetBlocking;
>   virSetCloseExec;
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index 95d1ff9..c73ce06 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -1681,6 +1681,50 @@ virGetDeviceUnprivSGIO(const char *path,
>   # define SYSFS_FC_HOST_PATH "/sys/class/fc_host/"
>   # define SYSFS_SCSI_HOST_PATH "/sys/class/scsi_host/"
>
> +/* virReadSCSIUniqueId:
> + * @sysfs_prefix: "scsi_host" sysfs path, defaults to SYSFS_SCSI_HOST_PATH
> + * @host: Host number, E.g. 5 of "scsi_host/host5"
> + * @result: Return the entry value as an unsigned int
> + *
> + * Read the value of the "scsi_host" unique_id file.
> + *
> + * Returns 0 on success, and @result is filled with the unique_id value
> + * Otherwise returns -1
> + */
> +int
> +virReadSCSIUniqueId(const char *sysfs_prefix,
> +                    int host,
> +                    int *result)
> +{
> +    char *sysfs_path = NULL;
> +    char *p = NULL;
> +    int ret = -1;
> +    char *buf = NULL;
> +    int unique_id;
> +
> +    if (virAsprintf(&sysfs_path, "%s/host%d/unique_id",
> +                    sysfs_prefix ? sysfs_prefix : SYSFS_SCSI_HOST_PATH,
> +                    host) < 0)
> +        goto cleanup;

This prints a message on error.

> +
> +    if (virFileReadAll(sysfs_path, 1024, &buf) < 0)
> +        goto cleanup;

And so does this.

> +
> +    if ((p = strchr(buf, '\n')))
> +        *p = '\0';
> +
> +    if (virStrToLong_i(buf, NULL, 10, &unique_id) < 0)
> +        goto cleanup;

This, however does not. If the unique_id file didn't contain a number, 
the caller gets -1 returned but have no clue why. I think:

virReportError(VIR_ERR_INTERNAL_ERROR,
                _(unable to parse unique_id: %s"), buf);

will do. (yes, we are misusing the VIR_ERR_INTERNAL_ERROR code soo much).

> +
> +    *result = unique_id;
> +    ret = 0;
> +
> + cleanup:
> +    VIR_FREE(sysfs_path);
> +    VIR_FREE(buf);
> +    return ret;
> +}
> +
>   /* virReadFCHost:
>    * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH
>    * @host: Host number, E.g. 5 of "fc_host/host5"
> @@ -2034,6 +2078,15 @@ virFindFCHostCapableVport(const char *sysfs_prefix)
>   }
>   #else
>   int
> +virReadSCSIUniqueId(const char *sysfs_prefix ATTRIBUTE_UNUSED,
> +                    int host ATTRIBUTE_UNUSED,
> +                    unsigned int *result ATTRIBUTE_UNUSED)
> +{
> +    virReportSystemError(ENOSYS, "%s", _("Not supported on this platform"));
> +    return -1;
> +}
> +
> +int
>   virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED,
>                 int host ATTRIBUTE_UNUSED,
>                 const char *entry ATTRIBUTE_UNUSED,
> diff --git a/src/util/virutil.h b/src/util/virutil.h
> index 2bb74e2..1407dfd 100644
> --- a/src/util/virutil.h
> +++ b/src/util/virutil.h
> @@ -164,6 +164,10 @@ int virGetDeviceUnprivSGIO(const char *path,
>                              int *unpriv_sgio);
>   char *virGetUnprivSGIOSysfsPath(const char *path,
>                                   const char *sysfs_dir);
> +int virReadSCSIUniqueId(const char *sysfs_prefix,
> +                        int host,
> +                        int *result)
> +    ATTRIBUTE_NONNULL(3);
>   int virReadFCHost(const char *sysfs_prefix,
>                     int host,
>                     const char *entry,
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index bc1040a..ecb2f34 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -83,6 +83,7 @@ EXTRA_DIST =		\
>   	domainsnapshotxml2xmlin \
>   	domainsnapshotxml2xmlout \
>   	fchostdata \
> +	scsihostdata \
>   	interfaceschemadata \
>   	lxcconf2xmldata \
>   	lxcxml2xmldata \
> @@ -188,6 +189,7 @@ endif WITH_REMOTE
>
>   if WITH_LINUX
>   test_programs += fchosttest
> +test_programs += scsihosttest
>   endif WITH_LINUX
>
>   if WITH_LIBVIRTD
> @@ -1146,8 +1148,13 @@ fchosttest_SOURCES = \
>          fchosttest.c testutils.h testutils.c
>   fchosttest_LDADD = $(LDADDS)
>
> +scsihosttest_SOURCES = \
> +       scsihosttest.c testutils.h testutils.c
> +scsihosttest_LDADD = $(LDADDS)
> +
>   else ! WITH_LINUX
>   EXTRA_DIST += fchosttest.c
> +EXTRA_DIST += scsihosttest.c
>   endif  ! WITH_LINUX
>
>   if WITH_LINUX
> diff --git a/tests/scsihosttest.c b/tests/scsihosttest.c
> new file mode 100644
> index 0000000..990fe80
> --- /dev/null
> +++ b/tests/scsihosttest.c

Nice, new test.

Michal