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

[John Ferlan]

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.


John