Re: [libvirt] [PATCH v2] storage: Disallow usage of the HBA for a fc_host backing

> +/** > + * @name: Name from a wwnn/wwpn lookup > + * > + * Validate that the @name fetched from the wwnn/wwpn is a vHBA and not > + * an HBA as that should be a configuration error. It's only possible to > + * use an existing wwnn/wwpn of a vHBA because that's what someone would > + * have created using the node device create functionality. Using the > + * HBA "just because" it has a wwnn/wwpn and the characteristics of a > + * vHBA is just not valid > + * > + * Returns the @scsi_host_name to be used or NULL on errror > + */ > +static char * > +checkName(const char *name) > +{ > + char *scsi_host_name = NULL; > + unsigned int host_num; > + > + if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0) > + return NULL; > + > + if (virSCSIHostGetNumber(scsi_host_name, &host_num) < 0) { > + virReportError(VIR_ERR_INTERNAL_ERROR, > + _("host name '%s' is not properly formatted"), > + name); > + goto error; > + } > + > + /* If scsi_host_name is vport capable, then it's an HBA. This is > + * a configuration error as the wwnn/wwpn should only be for a vHBA */ > + if (virVHBAIsVportCapable(NULL, host_num)) { > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("the wwnn/wwpn for '%s' are assigned to an HBA"), > + scsi_host_name); > + goto error; > + } > + > + return scsi_host_name; > + > + error: > + VIR_FREE(scsi_host_name); > + return NULL; > +} > + ... > @@ -275,6 +316,7 @@ createVport(virConnectPtr conn, > virStorageAdapterFCHostPtr fchost) > { > char *name = NULL; > + char *scsi_host_name = NULL; > virStoragePoolFCRefreshInfoPtr cbdata = NULL; > virThread thread; > int ret = -1; > @@ -288,6 +330,9 @@ createVport(virConnectPtr conn, > * this pool and we don't have to create the vHBA > */ > if ((name = virVHBAGetHostByWWN(NULL, fchost->wwnn, fchost->wwpn))) { > + if (!(scsi_host_name = checkName(name))) > + goto cleanup; Ok, so I learn from my mistakes, do you plan on building upon this in the foreseeable future with a follow-up series you haven't sent yet, but have on a local branch? Because if not, I don't really see a point in returning a string which only gets free'd on the function exit - checkName should probably become something like "isVHBA" and return boolean. And even if you do have a follow-up on a local branch, I still think that converting the return value should be part of that series, solely because until then, @scsi_host_name wouldn't have any meaning. Erik