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

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(a)redhat.com&gt; > --- > 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