[libvirt] [PATCH 7/7] storage: Guess the parent if it's not specified for vHBA

[Osier Yang]

On 06/04/13 04:01, John Ferlan wrote:
> On 03/25/2013 12:43 PM, Osier Yang wrote:
>> This finds the parent for vHBA by iterating over all the HBA
>> which supports vport_ops capability on the host, and return
>> the first one which is online, not saturated (vports in use
>> is less than max_vports).
>> ---
>>   src/libvirt_private.syms           |  1 +
>>   src/storage/storage_backend_scsi.c | 10 ++--
>>   src/util/virutil.c                 | 93 ++++++++++++++++++++++++++++++++++++++
>>   src/util/virutil.h                 |  2 +
>>   4 files changed, 102 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>> index 3df7632..932a448 100644
>> --- a/src/libvirt_private.syms
>> +++ b/src/libvirt_private.syms
>> @@ -1850,6 +1850,7 @@ virFileStripSuffix;
>>   virFileUnlock;
>>   virFileWaitForDevices;
>>   virFileWriteStr;
>> +virFindFCHostCapableVport;
>>   virFindFileInPath;
>>   virFormatIntDecimal;
>>   virGetDeviceID;
>> diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
>> index 0bb4e70..b1ff127 100644
>> --- a/src/storage/storage_backend_scsi.c
>> +++ b/src/storage/storage_backend_scsi.c
>> @@ -658,10 +658,12 @@ createVport(virStoragePoolSourceAdapter adapter)
>>                                 adapter.data.fchost.wwpn))
>>           return 0;
>>   
>> -    if (!adapter.data.fchost.parent) {
>> -        virReportError(VIR_ERR_XML_ERROR, "%s",
>> -                       _("'parent' for vHBA must be specified"));
>> -        return -1;
>> +    if (!adapter.data.fchost.parent &&
>> +        !(adapter.data.fchost.parent = virFindFCHostCapableVport(NULL))) {
>> +         virReportError(VIR_ERR_XML_ERROR, "%s",
>> +                       _("'parent' for vHBA not specified, and "
>> +                         "cannot find one on this host"));
>> +         return -1;
>>       }
>>   
>>       if (getHostNumber(adapter.data.fchost.parent, &parent_host) < 0)
>> diff --git a/src/util/virutil.c b/src/util/virutil.c
>> index a8b9962..55d807e 100644
>> --- a/src/util/virutil.c
>> +++ b/src/util/virutil.c
>> @@ -3670,6 +3670,92 @@ cleanup:
>>       VIR_FREE(wwpn_buf);
>>       return ret;
>>   }
>> +
>> +# define PORT_STATE_ONLINE "Online"
>> +
>> +/* virFindFCHostCapableVport:
>> + *
>> + * Iterate over the sysfs and find out the first online HBA which
>> + * supports vport, and not saturated,.
>> + */
>> +char *
>> +virFindFCHostCapableVport(const char *sysfs_prefix)
>> +{
>> +    const char *prefix = sysfs_prefix ? sysfs_prefix : SYSFS_FC_HOST_PATH;
>> +    DIR *dir = NULL;
>> +    struct dirent *entry = NULL;
>> +    char *max_vports = NULL;
>> +    char *vports = NULL;
>> +    char *state = NULL;
>> +    char *ret = NULL;
>> +
>> +    if (!(dir = opendir(prefix))) {
>> +        virReportSystemError(errno,
>> +                             _("Failed to opendir path '%s'"),
>> +                             prefix);
>> +        return NULL;
>> +    }
>> +
>> +    while ((entry = readdir(dir))) {
>> +        unsigned int host;
>> +        char *p = NULL;
>> +
>> +        if (entry->d_name[0] == '.')
>> +            continue;
>> +
>> +        p = entry->d_name + strlen("host");
> Assumes all entries are prefixed by "host", right? Thus should anything
> that is not be filtered like the ".", "..", and hidden files are above?

Why not to filter them?

>
>> +        if (virStrToLong_ui(p, NULL, 10, &host) == -1) {
>> +            VIR_DEBUG("Failed to parse host number from '%s'",
>> +                      entry->d_name);
>> +            continue;
>> +        }
>> +
>> +        if (!virIsCapableVport(NULL, host))
>> +            continue;
>> +
>> +        if (virReadFCHost(NULL, host, "port_state", &state) < 0) {
>> +             VIR_DEBUG("Failed to read port_state for host%d", host);
>> +             continue;
>> +        }
>> +
>> +        /* Skip the not online FC host */
>> +        if (!STREQ(state, PORT_STATE_ONLINE)) {
> s/!STREQ/STRNEQ/ ??

Ah, right, corrected.

>
>> +            VIR_FREE(state);
>> +            continue;
>> +        }
>> +        VIR_FREE(state);
>> +
>> +        if (virReadFCHost(NULL, host, "max_npiv_vports", &max_vports) < 0) {
>> +             VIR_DEBUG("Failed to read max_npiv_vports for host%d", host);
>> +             continue;
>> +        }
>> +
>> +        if (virReadFCHost(NULL, host, "npiv_vports_inuse", &vports) < 0) {
>> +             VIR_DEBUG("Failed to read npiv_vports_inuse for host%d", host);
>> +             VIR_FREE(max_vports);
>> +             continue;
>> +        }
>> +
>> +        /* Compare from the strings directly, instead of converting
>> +         * the strings to integers first
>> +         */
>> +        if ((strlen(max_vports) >= strlen(vports)) ||
>> +            ((strlen(max_vports) == strlen(vports)) &&
>> +             strcmp(max_vports, vports) > 0)) {
>> +            ret = strdup(entry->d_name);
>> +            goto cleanup;
>> +        }
>> +
>> +        VIR_FREE(max_vports);
>> +        VIR_FREE(vports);
>> +    }
>> +
>> +cleanup:
>> +    closedir(dir);
>> +    VIR_FREE(max_vports);
>> +    VIR_FREE(vports);
>> +    return ret;
>> +}
>>   #else
>>   int
>>   virReadFCHost(const char *sysfs_prefix ATTRIBUTE_UNUSED,
>> @@ -3715,4 +3801,11 @@ virGetFCHostNameByWWN(const char *sysfs_prefix ATTRIBUTE_UNUSED,
>>       return NULL;
>>   }
>>   
>> +char *
>> +virFindFCHostCapableVport(const char *sysfs_prefix ATTRIBUTE_UNUSED)
>> +{
>> +    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 d134034..cde4218 100644
>> --- a/src/util/virutil.h
>> +++ b/src/util/virutil.h
>> @@ -322,4 +322,6 @@ char *virGetFCHostNameByWWN(const char *sysfs_prefix,
>>                               const char *wwpn)
>>       ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
>>   
>> +char *virFindFCHostCapableVport(const char *sysfs_prefix);
>> +
>>   #endif /* __VIR_UTIL_H__ */
>>
> ACK - NOTE that I still haven't seen anywhere "fc_host" would be
> necessary from patch 5/7.
>
> John
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list