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

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

> > 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. >