[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2 3/3] conf: Fix vHBA checkParent logic for pool creation



> diff --git a/src/storage/storage_backend_scsi.c b/src/storage/storage_backend_scsi.c
> index 365ab77..d225e4f 100644
> --- a/src/storage/storage_backend_scsi.c
> +++ b/src/storage/storage_backend_scsi.c
> @@ -220,6 +220,7 @@ checkParent(virConnectPtr conn,
>              const char *name,
>              const char *parent_name)
>  {
> +    unsigned int host_num;
>      char *scsi_host_name = NULL;
>      char *vhba_parent = NULL;
>      bool retval = false;
> @@ -230,20 +231,52 @@ checkParent(virConnectPtr conn,
>      if (!conn)
>          return true;
>
> -    if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0)
> +    if (virSCSIHostGetNumber(parent_name, &host_num) < 0) {
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       _("parent '%s' is not properly formatted"), name);
>          goto cleanup;
> +    }
>
> -    if (!(vhba_parent = virNodeDeviceGetParentName(conn, scsi_host_name)))
> +    if (!virVHBAPathExists(NULL, host_num)) {
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       ("parent '%s' is not a vHBA/HBA"), parent_name);

    Under what circumstances can parent be a ^^^^vHBA??

>          goto cleanup;
> +    }
>
> -    if (STRNEQ(parent_name, vhba_parent)) {
> -        virReportError(VIR_ERR_XML_ERROR,
> -                       _("Parent attribute '%s' does not match parent '%s' "
> -                         "determined for the '%s' wwnn/wwpn lookup."),
> -                       parent_name, vhba_parent, name);
> +    if (virAsprintf(&scsi_host_name, "scsi_%s", name) < 0)
> +        goto cleanup;
> +
> +    if (virSCSIHostGetNumber(scsi_host_name, &host_num) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("host name '%s' is not properly formatted"), name);
>          goto cleanup;
>      }
>
> +    /* If scsi_host_name is vport capable, then it's an HBA, thus compare
> +     * only against the parent_name; otherwise, as long as the scsi_host_name
> +     * path exists, then the scsi_host_name is a vHBA in which case we need
> +     * to compare against it's parent. */
> +    if (virVHBAIsVportCapable(NULL, host_num)) {

I truly find it pointless to try to do any kind of parent validation on HBA
device. Can a HBA device actually have a scsi_host parent? If not, then we
should only validate whether the device given by its wwnn and wwpn exist (we
already know that since we're here) is vport capable and then if the 'parent'
attribute has been defined, then return an error about invalid configuration,
rather than trying to compare parent names in that matter.

Erik


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]