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

[Ján Tomko]

On Wed, Apr 29, 2015 at 12:37:38PM -0400, John Ferlan wrote:
> 
> 
> 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.

That is the generic description shared with all the pools. Maybe the
docs need some clarification?

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

Even though it is possible to configure two servers with the same IQN,
I don't see why we should support starting pools with both of them at
at the same time.

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

So we have bug A - starting two pools from the same host with the same IQN

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

and bug B - starting two pools from different hosts with the same IQN

Since we can't reliably tell it the hosts are different or the same,
and I doubt the usefulness of two different hosts using the same IQN,
why not just reduce this to one bug and reject duplicate IQNs regardless
of the host?

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

The iscsiadm output only contains IP addresses for me.

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

I don't think doing that much work just to work around misconfigured
systems is worth it.

Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150430/f0d71a0c/attachment-0001.sig>