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

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?

> + 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/ ??

> + 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(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list