Re: [libvirt] [PATCH 7/7] storage: Check for duplicate host for incoming pool def
On 04/20/2015 11:11 AM, Ján Tomko wrote:
On Sun, Apr 19, 2015 at 08:49:12PM -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1171984
>
> Use the new virIsSameHostnameInfo API to determine whether the proposed
> storage pool definition matches the existing storage pool definition
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/conf/storage_conf.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 4852dfb..c1bc242 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -2415,7 +2415,16 @@ virStoragePoolSourceMatchSingleHost(virStoragePoolSourcePtr
poolsrc,
> if (poolsrc->hosts[0].port != defsrc->hosts[0].port)
> return false;
>
This function is called when parsing the configuration, which should not
depend on host state.
For example, if libvirt is started really early at boot time and the
hostnames cannot be resolved by the DNS yet, they will pass the check
but they will disappear on libvirtd restart.
Hmm... the downside of unreliable dependencies.
The hostname->ip pairings are not stable either, so if we do this, I
think it should be done on pool startup, not config parsing.
Right, but by the time we get to pool startup we'd be at the same point
as referenced above - if done early enough at boot time, then it's not
going to fail, but perhaps better than nothing.
I suppose at least moving to startup allows for better error paths since
currently errors can be overwritten or ignored.
> - 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.
Or just disallow starting two pools with the same targetname, since they
are supposed to be unique?
'targetname' as in ?
A 'target.path' per pool would need to be unique, but using the same
target.path into two different networked pools is something that should
work. And the pool target path (/dev/disk/by-path) is used by multiple
iSCSI pools on the same host, so it cannot be used as something unique
per host.
John