Re: [libvirt] [PATCH 7/7] storage: Check for duplicate host for incoming pool def
...
>>> - return STREQ(poolsrc->hosts[0].name,
defsrc->hosts[0].name);
>>> + if (STRNEQ(poolsrc->hosts[0].name, defsrc->hosts[0].name)) {
>>> + /* Matching just a name isn't reliable as someone could provide
>>> + * the name for one pool and the IP Address for another pool, so
>>
>> Resolving them is IMHO just as unreliable.
>>
>> Re: the original bug - is it possible to check that we have connected to
>> a session with a different hostname than what we requested?
>
> What does the connected session hostname have to do with the original
> bug? The bug requests checking that an iSCSI pool doesn't use a "<host
> name='<some IP Address>'..." for one pool:
>
> <host name='10.66.6.12' port='3260'/>
>
> and a resolved name for another pool on the same host:
>
> <host name='test1' port='3260'/>
>
> Where :
>
> # cat /etc/hosts
> 10.66.6.12 test1
>
> The bug pointed out iSCSI in particular, but since other pools use the
> <source... <host name='%s'.../>... /> XML formatting it seemed
logical
> to have them all use the same checks.
>
The bug requests that this should be forbidden when defining the pool
(which is NOTABUG), because we then allow starting both of them, but
stopping one of them also stops the other one, while libvirt still
thinks it's up (this is the real bug).
Sure the part about causing an error at define time I see your point,
but extrapolating a bit at startup time it's still an issue to have two
pools pointing to the same source.
Just closing it NOTABUG because the wording isn't precise doesn't mean
that part of the bug still is valid and needs some sort of resolution.
Without letting two pools use the same network source host and device
target path, we'd never be faced with the shutdown one conundrum. We
disallow certain duplication at definition time, but network pools have
an extra problem of source host resolution that's not resolved (yet).
I can take my 'localhost' netfs pool, change a couple of parameters
(name to prefix with "dup-", targetpath to prefix with "dup-", and
host
name to be '127.0.0.1') and can start two pools up looking at the same
source.
The shutdown path is just "assuming" that the startup path disallowed
duplicate pools pointing at the same source paths, but I believe
allowing a pool to be started that's looking at the same target is still
a bug.
We definitely shouldn't resolve the hostnames when we parse the XML, the
XML parser/formatter should not depend on the host state.
Resolving the hostname on pool startup would at least remove the
roulette from XML parser, but still won't be foolproof -
multiple IP addresses can point to the same host.
More importantly, it won't solve the underlying issue:
We don't care what host we connect to.
As long as virStorageBackendISCSISession finds the <source><device path>
in the output of iscsiadm --mode session, the pool is marked as started.
While it might make sense to have the pool accessible via different
networks (or different address families), our iscsi backend is not ready
for that.
I don't know how to allow that, (without guessing what iscsiadm resolved
the hostname to), but marking the pool as started, if the session was
created by another pool is wrong.
Right - the iscsi backend has made extra assumptions based on the theory
that the source host checks ensured that the source host name wasn't
duplicated.
I know the NETFS pool doesn't suffer from the same destroy issue, I'm
not so sure about Gluster and Sheepdog as I don't have a configuration
of them handy.
>>
>> 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. If we did have two different hosts,
then they could conceivably have the same target name/path. That could
be a way to configure some sort of hot standby or primary/secondary
where the device paths are the same, but the hosts change. For iSCSI
it'll create different iqn, portal results.
John