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

[John Ferlan]

On 07/19/2017 08:42 AM, Erik Skultety wrote:
> On Tue, Jul 18, 2017 at 11:10:40AM -0400, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1458708
>>
>> When originally designed in commit id '42a021c1', providing the
>> 'parent' attribute to the <adapter type='fc_host' wwnn='vHBA_wwnn'
>> wwpn='vHBA_wwpn'/> was checked to make sure that the "parent" of
>> the wwnn/wwpn matched that of the provided parent "just in case"
>> someone created the node device first, then defined the storage
>> pool using that node device afterwards. The result is to not
>> perform the vport_create call when the scsi_host represented by
>> the wwnn/wwpn already exists since it would fail.
> 
> Okay, so we can both agree that these scenarios are rather unlikely. Why can't
> we just ignore the 'parent' attribute completely once we find out that a device
> with corresponding wwnn and wwpn already exist? Delete wouldn't be a problem in
> that case either, we do have a functional logic querying the parent
> automatically if none had been provided at the time of creation of the pool.
> 
> Erik
> 

Nothing quickly sprang to mind until I read the storage pool section on
@parent and remembered that it's useful to avoid multiple pools using
the same source as the duplicate pool checks done as part of
virStoragePoolObjSourceMatchTypeISCSI

John

>>
>> Eventually someone came up with the brilliant idea to provide
>> wwnn/wwpn of an HBA instead of a vHBA, e.g. <adapter type='fc_host'
>> wwnn='HBA_wwnn' wwpn='HBA_wwpn'/>. This is the same as creating a
>> storage pool backed to the HBA that just happens to also be vport
>> (vHBA) capable. The logic to bypass the vport_create call was the
>> same as the vHBA code since the wwn's already exist. Once that was
>> determined to work, adding in the 'parent' attribute caused a problem
>> for the DeleteVport code, which was fixed by commit id '2c8e30ee7e'.
>>
>> The next test tried was providing a valid pair of wwns that would
>> find the scsi_host HBA, but providing the wrong name for the 'parent'
>> attribute. This caused a different failure because at DeleteVport
>> time if a parent is provided it was assumed valid especially since
>> the CreateVport code would check that by calling virVHBAPathExists.
>>
>> So alter the checkParent code to first ensure that the provided
>> parent_name is a valid HBA/vHBA, then check if the found scsi_host
>> is an HBA or a vHBA and make the appropriate check against the
>> parent_name similar to the check made in virNodeDeviceDeleteVport.
>>