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&gt; >> --- >> 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 ?