[libvirt] [PATCH v2 1/3] virutil: Introduce virGetSCSIHostNumber

[Michal Privoznik]

On 06.10.2014 23:49, John Ferlan wrote:
> Create/use virGetSCSIHostNumber to replace the static getHostNumber
>
> Removed the "if (result &&" since result is now required to be non NULL
> on input.
>
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>   src/libvirt_private.syms           |  1 +
>   src/storage/storage_backend_scsi.c | 40 +++------------------------
>   src/util/virutil.c                 | 56 ++++++++++++++++++++++++++++++++++++++
>   src/util/virutil.h                 |  4 +++
>   4 files changed, 65 insertions(+), 36 deletions(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index d6265ac..189e07c 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2151,6 +2151,7 @@ virGetGroupList;
>   virGetGroupName;
>   virGetHostname;
>   virGetListenFDs;
> +virGetSCSIHostNumber;
>   virGetSelfLastChanged;
>   virGetUnprivSGIOSysfsPath;
>   virGetUserCacheDirectory;
> diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
> index 16ed328..d9f3ff2 100644
> --- a/src/storage/storage_backend_scsi.c
> +++ b/src/storage/storage_backend_scsi.c
> @@ -511,38 +511,6 @@ virStorageBackendSCSITriggerRescan(uint32_t host)
>       return retval;
>   }
>
> -static int
> -getHostNumber(const char *adapter_name,
> -              unsigned int *result)
> -{
> -    char *host = (char *)adapter_name;
> -
> -    /* Specifying adapter like 'host5' is still supported for
> -     * back-compat reason.
> -     */
> -    if (STRPREFIX(host, "scsi_host")) {
> -        host += strlen("scsi_host");
> -    } else if (STRPREFIX(host, "fc_host")) {
> -        host += strlen("fc_host");
> -    } else if (STRPREFIX(host, "host")) {
> -        host += strlen("host");
> -    } else {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("Invalid adapter name '%s' for SCSI pool"),
> -                       adapter_name);
> -        return -1;
> -    }
> -
> -    if (result && virStrToLong_ui(host, NULL, 10, result) == -1) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                       _("Invalid adapter name '%s' for SCSI pool"),
> -                       adapter_name);
> -        return -1;
> -    }
> -
> -    return 0;
> -}
> -
>   static char *
>   getAdapterName(virStoragePoolSourceAdapter adapter)
>   {
> @@ -610,7 +578,7 @@ createVport(virStoragePoolSourceAdapter adapter)
>            return -1;
>       }
>
> -    if (getHostNumber(adapter.data.fchost.parent, &parent_host) < 0)
> +    if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0)
>           return -1;
>
>       if (virManageVport(parent_host, adapter.data.fchost.wwpn,
> @@ -643,7 +611,7 @@ deleteVport(virStoragePoolSourceAdapter adapter)
>                                          adapter.data.fchost.wwpn)))
>           return -1;
>
> -    if (getHostNumber(adapter.data.fchost.parent, &parent_host) < 0)
> +    if (virGetSCSIHostNumber(adapter.data.fchost.parent, &parent_host) < 0)
>           goto cleanup;
>
>       if (virManageVport(parent_host, adapter.data.fchost.wwpn,
> @@ -683,7 +651,7 @@ virStorageBackendSCSICheckPool(virConnectPtr conn ATTRIBUTE_UNUSED,
>           }
>       }
>
> -    if (getHostNumber(name, &host) < 0)
> +    if (virGetSCSIHostNumber(name, &host) < 0)
>           goto cleanup;
>
>       if (virAsprintf(&path, "%s/host%d",
> @@ -712,7 +680,7 @@ virStorageBackendSCSIRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED,
>       if (!(name = getAdapterName(pool->def->source.adapter)))
>           return -1;
>
> -    if (getHostNumber(name, &host) < 0)
> +    if (virGetSCSIHostNumber(name, &host) < 0)
>           goto out;
>
>       VIR_DEBUG("Scanning host%u", host);
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index 5197969..1c23d7c 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -1838,6 +1838,54 @@ virFindSCSIHostByPCI(const char *sysfs_prefix,
>       return ret;
>   }
>
> +/* virGetSCSIHostNumber:
> + * @adapter_name: Name of the host adapter
> + * @result: Return the entry value as unsigned int
> + *
> + * Convert the various forms of scsi_host names into the numeric
> + * host# value that can be used in order to scan sysfs looking for
> + * the specific host.
> + *
> + * Names can be either "scsi_host#" or just "host#", where
> + * "host#" is the back-compat format, but both equate to
> + * the same source adapter.  First check if both pool and def
> + * are using same format (easier) - if so, then compare
> + *
> + * Returns 0 on success, and @result has the host number.
> + * Otherwise returns -1.
> + */
> +int
> +virGetSCSIHostNumber(const char *adapter_name,
> +                     unsigned int *result)
> +{
> +    char *host = (char *)adapter_name;

I think this isn't needed. I mean, not only typecast, but the @host 
variable itself.

> +
> +    /* Specifying adapter like 'host5' is still supported for
> +     * back-compat reason.
> +     */
> +    if (STRPREFIX(host, "scsi_host")) {
> +        host += strlen("scsi_host");
> +    } else if (STRPREFIX(host, "fc_host")) {
> +        host += strlen("fc_host");
> +    } else if (STRPREFIX(host, "host")) {
> +        host += strlen("host");
> +    } else {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Invalid adapter name '%s' for SCSI pool"),
> +                       adapter_name);
> +        return -1;
> +    }
> +
> +    if (virStrToLong_ui(host, NULL, 10, result) == -1) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Invalid adapter name '%s' for SCSI pool"),
> +                       adapter_name);
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
>   /* virReadFCHost:
>    * @sysfs_prefix: "fc_host" sysfs path, defaults to SYSFS_FC_HOST_PATH
>    * @host: Host number, E.g. 5 of "fc_host/host5"
> @@ -2209,6 +2257,14 @@ virFindSCSIHostByPCI(const char *sysfs_prefix ATTRIBUTE_UNUSED,
>   }
>
>   int
> +virGetSCSIHostNumber(const char *adapter_name ATTRIBUTE_UNUSED,
> +                     unsigned int *result ATTRIBUTE_UNUSED)
> +{
> +    virReportSystemError(ENOSYS, "%s", _("Not supported on this platform"));
> +    return NULL;
> +}
> +
> +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 54f1148..442c998 100644
> --- a/src/util/virutil.h
> +++ b/src/util/virutil.h
> @@ -173,6 +173,10 @@ char *
>   virFindSCSIHostByPCI(const char *sysfs_prefix,
>                        const char *parentaddr,
>                        unsigned int unique_id);
> +int
> +virGetSCSIHostNumber(const char *adapter_name,
> +                     unsigned int *result)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>   int virReadFCHost(const char *sysfs_prefix,
>                     int host,
>                     const char *entry,
>

Michal