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

[John Ferlan]

On 04/21/2015 04:31 AM, Peter Krempa wrote:
> On Mon, Apr 20, 2015 at 12:23:25 -0400, John Ferlan wrote:
>>
>>
>> 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 at 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.
> 
> Not only unreliable dependencies. The issue might also happen with
> factors external to the host. For example by adding a new DNS name for
> the "duplicate" host.
> 
>>
>>>
>>> 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.
> 
> Well, that's actually not true. If the second (duplicate) pool is
> starting at one point, the address either can be resolved and deemed
> duplicate, or the resolution would fail anyways and the pool would not
> be started.
> 
> When compared with the approach when defining the pool (or reading back
> XML files):
> 1) if the hostname is not duplicate or not resolvable at the point when
> you define the pool but changes to be duplicate later you wouldn't catch
> the issue
> 2) if the pool source becomes duplicate in the life of the host it would
> vanish after libvirt restart
> 
> 
Working through refactoring and I've reached this point... Started
looking into having the 'startPool' backends make the duplicate pool
checks, but that won't work because by that time the new definition
would have been added to the driver->pools list. Also we don't have the
full driver->pools list passed, just the new definition.

So since the hangup seems to be on the define processing, what if I add
a fourth parameter 'check_active' to virStoragePoolSourceFindDuplicate
(like virStoragePoolObjIsDuplicate), which only does the resolution
checking if we're about to become active?  That would mean another round
of checks in storagePoolCreate

Optionally, a new API such as virStoragePoolSourceNetworkCheck (or
whatever name someone prefers) could perform the checks for the
CreateXML and Create paths

John