[libvirt] [PATCH 7/7] storage: Guess the parent if it's not specified for vHBA
Osier Yang
jyang at redhat.com
Mon Apr 8 10:36:44 UTC 2013
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
More information about the libvir-list
mailing list