Re: [libvirt] [PATCH 7/7] storage: Check for duplicate host for incoming pool def

On 04/23/2015 07:50 AM, Ján Tomko wrote: > On Wed, Apr 22, 2015 at 01:17:20PM -0400, John Ferlan wrote: ... Having the dialog in your other series caused me to remember there was still a question here. >>> >>>>> >>>>> Or just disallow starting two pools with the same targetname, since they >>>>> are supposed to be unique? >>>>> >>>> >>>> 'targetname' as in ? >>> >>> The <source><device path='xxxx'> attribute of the iscsi pool, which we >>> feed to iscsiadm via --targetname. >>> >> >> Oh - OK - that check is done, but the results overridden if the >> <source...> duplicate check fails. > > I don't see the check being done anywhere. > virStoragePoolSourceFindDuplicate for VIR_STORAGE_POOL_ISCSI calls virStoragePoolSourceFindDuplicateDevices which does the dual pool search through source.devices[i].path (or the IQN for the device). If a duplicate is found, then it would check if the source hostname's match. The formatstorage page describes "Will be used in combination with a directory or device element." with regards to unique identification.

Since port is optional, even that check is sketchy since one could have the same name, but not provide the port number in the incoming definition and thus have a duplicate. I suppose allowing two different 'iscsid' servers to advertise the same "name" isn't necessarily an issue. It could allow for someone to set up some sort of hot standby or backup (or who knows what) on separate servers without needing to manage the IQN mapping between the two.

In the long run, the path in a vol-list is a combination of /dev/disk-by-path (or <target... <path>...>), the host name/addr, and the IQN such as: /dev/disk/by-path/ip-192.168.122.1:3260-iscsi-iqn.2013-12.com.example:iscsi-chap-netpool-lun-1 and this links to the block device on the host (eg, ../../dev/sdb). If there was a second pool started using the same "host" as the first pool, but just by a different name, it too would use the same block device, so now there would be two pools using the same device. Since using the same device isn't allowed for other pools, the iSCSI pool should also block usage.

And yes, a completely separate host using the same IQN would also use the same block device (but that's a different bug IMO).

Perhaps the ultimate root cause is as you pointed out along the way that virStorageBackendISCSISession just looks for (and assumes) the 'targetname' is unique for the host (assumes that the hostname checks already rooted out differences). But that shows the second bug with this code is that we could have the two hosts with the same IQN on each, but our search will only ever find the "first" one (the regex in virISCSIExtractSession doesn't compare hostnames).

So while even my .last of this and my v2 series doesn't resolve the second issue, I believe they do at least inhibit the same host by a different name issue. If the v2 of the series was accepted, then the "next step" would be to add/extract that hostname:port from the iscsiadm command and determine whether it "matched" the expected one so that we could find that theoretical second/different host using the same IQN as the first.