[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Sun, Apr 19, 2015 at 20:49:12 -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 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;
>  
> -    return STREQ(poolsrc->hosts[0].name, defsrc->hosts[0].name);
> +    if (STRNEQ(poolsrc->hosts[0].name, defsrc->hosts[0].name)) {

This check is almost worthless. In the normal use case the user won't
try to define the pool with same name twice. On many other cases the
pool def might use different hostnames or so. Using the resolve helper
always might save this condition ....

> +        /* Matching just a name isn't reliable as someone could provide
> +         * the name for one pool and the IP Address for another pool, so
> +         * for a "true" check we must compare the resolved name, but that's
> +         * expensive, so only do this check if using different names
> +         */

... and you wouldn't need to explain why you've done it this way.

> +        return virIsSameHostnameInfo(poolsrc->hosts[0].name,
> +                                     defsrc->hosts[0].name);
> +    }
> +    return true;
>  }

Peter

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]