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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 10/28/2014 07:36 PM, Michal Privoznik wrote:
> 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@xxxxxxxxxx>
>> ---
>>   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.
> 
You'll see it's just a cut-n-paste/copy of the former getHostNumber
function above.

I see your point though - it's not a 'const char const *', so adjusting
the adapter_name pointer is perfectly reasonable and what I went with.


John
>> +
>> +    /* 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
> 

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]