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