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

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

 



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@xxxxxxxxxx>
> ---
>  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
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]