[libvirt] [PATCH 00/11] Allow creation of vHBA by parent_wwnn/wwpn or fabric_name

[John Ferlan]

On 12/12/2016 07:50 PM, Jim Fehlig wrote:
> On 12/03/2016 07:09 AM, John Ferlan wrote:
>>
>> ping?
> 
> Sorry, I seem to have lost your initial cover letter so responding here...
> 
>>
>> On 11/18/2016 09:26 AM, John Ferlan wrote:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1349696
>>>
>>> Lots of details in the bz, but essentially the problem is that providing
>>> a "parent" scsi_hostX value has drawbacks on reboots because what was
>>> scsi_hostX could turn into scsi_hostY on subsequent reboots.
>>>
>>> So add the ability to use the parent wwnn/wwpn or fabric_wwn as 'search'
>>> criteria in order to create either non persistent vHBA's via nodedev or
>>> persistent vHBA's via storage pools.
> 
> I've not looked at all the patches in detail, but I have been giving
> them a fair bit of testing and haven't noticed any problems thus far.
> 

Thanks - that's good to know...

> Once question I do have is related to volume detection in vport-based
> pools. Currently when there are multiple paths to a LUN only the active
> ones are displayed in the pool. Inactive paths are ignored since
> saferead() fails in
> src/storage/storage_backend.c:virStorageBackendDetectBlockVolFormatFD().
> Without the inactive paths, there would be no path to the LUN if it was
> trespassed. I made the below hack to
> virStorageBackendUpdateVolTargetInfo() to show the inactive paths too,
> and was able to pass those to the guest allowing it to handle
> multipathing. Is there a reason the inactive paths are ignored?
> 

I suppose for NPIV/vHBA LUN's it perhaps makes sense to have inactive
paths. Been a while since I went down that path...

Seems like virStorageBackendSCSINewLun should be able to receive a -2 in
order to ignore VIR_STORAGE_VOL_READ_NOERROR. There's a sequence of
changes I made starting with commit id '22346003d' which allow/support a
-2 return.  Those lead to a linked bz in the commit message describing a
problem with multipath device with 'active' and 'ghost' devices.  I'd
have to dig more on that to jiggle the memory.

Perhaps it's a path that needs a bit more code to more properly
determine details of the LUN returning -2... IOW: Something that would
dig on the failed device to see if it was mpath related.

John

> Regards,
> Jim
> 
> Index: libvirt-2.5.0/src/storage/storage_backend.c
> ===================================================================
> --- libvirt-2.5.0.orig/src/storage/storage_backend.c
> +++ libvirt-2.5.0/src/storage/storage_backend.c
> @@ -1941,8 +1941,11 @@ virStorageBackendUpdateVolTargetInfo(vir
> 
>      if (withBlockVolFormat) {
>          if ((ret = virStorageBackendDetectBlockVolFormatFD(target, fd,
> -                                                           readflags))
> < 0)
> +                                                           readflags))
> < 0) {
> +            if (ret == -2)
> +                ret = 0;
>              goto cleanup;
> +        }
>      }
> 
>   cleanup:
>