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 <[email protected]>
> ---
>  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

--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to