> +/**
> + * @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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
