[libvirt] [PATCH 3/3] Network duplicate UUID/name checking

[Cole Robinson]

On 05/27/2010 12:00 PM, Daniel P. Berrange wrote:
> The network driver is not doing correct checking for
> duplicate UUID/name values. This introduces a new method
> virNetworkObjIsDuplicate, based on the previously
> written virDomainObjIsDuplicate.
> 
> * src/conf/network_conf.c, src/conf/network_conf.c,
>   src/libvirt_private.syms: Add virNetworkObjIsDuplicate,
> * src/network/bridge_driver.c: Call virNetworkObjIsDuplicate
>   for checking uniqueness of uuid/names
> ---
>  src/conf/network_conf.c     |   65 +++++++++++++++++++++++++++++++++++++++++++
>  src/conf/network_conf.h     |    4 ++
>  src/libvirt_private.syms    |    1 +
>  src/network/bridge_driver.c |    7 ++++
>  4 files changed, 77 insertions(+), 0 deletions(-)
> 
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index 06537d1..347fc0b 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -940,6 +940,71 @@ error:
>      return ret;
>  }
>  
> +
> +/*
> + * virNetworkObjIsDuplicate:
> + * @doms : virNetworkObjListPtr to search
> + * @def  : virNetworkDefPtr definition of network to lookup
> + * @check_active: If true, ensure that network is not active
> + *
> + * Returns: -1 on error
> + *          0 if network is new
> + *          1 if network is a duplicate
> + */
> +int
> +virNetworkObjIsDuplicate(virNetworkObjListPtr doms,
> +                         virNetworkDefPtr def,
> +                         unsigned int check_active)
> +{
> +    int ret = -1;
> +    int dupVM = 0;
> +    virNetworkObjPtr vm = NULL;
> +
> +    /* See if a VM with matching UUID already exists */
> +    vm = virNetworkFindByUUID(doms, def->uuid);
> +    if (vm) {
> +        /* UUID matches, but if names don't match, refuse it */
> +        if (STRNEQ(vm->def->name, def->name)) {
> +            char uuidstr[VIR_UUID_STRING_BUFLEN];
> +            virUUIDFormat(vm->def->uuid, uuidstr);
> +            virNetworkReportError(VIR_ERR_OPERATION_FAILED,
> +                                  _("network '%s' is already defined with uuid %s"),
> +                                  vm->def->name, uuidstr);
> +            goto cleanup;
> +        }
> +
> +        if (check_active) {
> +            /* UUID & name match, but if VM is already active, refuse it */
> +            if (virNetworkObjIsActive(vm)) {
> +                virNetworkReportError(VIR_ERR_OPERATION_INVALID,
> +                                      _("network is already active as '%s'"),
> +                                      vm->def->name);
> +                goto cleanup;
> +            }
> +        }
> +
> +        dupVM = 1;
> +    } else {
> +        /* UUID does not match, but if a name matches, refuse it */
> +        vm = virNetworkFindByName(doms, def->name);
> +        if (vm) {
> +            char uuidstr[VIR_UUID_STRING_BUFLEN];
> +            virUUIDFormat(vm->def->uuid, uuidstr);
> +            virNetworkReportError(VIR_ERR_OPERATION_FAILED,
> +                                  _("network '%s' already exists with uuid %s"),
> +                                  def->name, uuidstr);
> +            goto cleanup;
> +        }
> +    }
> +
> +    ret = dupVM;
> +cleanup:
> +    if (vm)
> +        virNetworkObjUnlock(vm);
> +    return ret;
> +}
> +
> +
>  void virNetworkObjLock(virNetworkObjPtr obj)
>  {
>      virMutexLock(&obj->lock);
> diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
> index 127a23a..c4956b6 100644
> --- a/src/conf/network_conf.h
> +++ b/src/conf/network_conf.h
> @@ -169,6 +169,10 @@ int virNetworkSetBridgeName(const virNetworkObjListPtr nets,
>                              virNetworkDefPtr def,
>                              int check_collision);
>  
> +int virNetworkObjIsDuplicate(virNetworkObjListPtr doms,
> +                             virNetworkDefPtr def,
> +                             unsigned int check_active);
> +
>  void virNetworkObjLock(virNetworkObjPtr obj);
>  void virNetworkObjUnlock(virNetworkObjPtr obj);
>  
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index e7cd6dd..414e937 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -469,6 +469,7 @@ virNetworkSaveConfig;
>  virNetworkSetBridgeName;
>  virNetworkObjLock;
>  virNetworkObjUnlock;
> +virNetworkObjIsDuplicate;
>  
>  
>  # nodeinfo.h
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 5d7ef19..01be425 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -1265,6 +1265,9 @@ static virNetworkPtr networkCreate(virConnectPtr conn, const char *xml) {
>      if (!(def = virNetworkDefParseString(xml)))
>          goto cleanup;
>  
> +    if (virNetworkObjIsDuplicate(&driver->networks, def, 1) < 0)
> +        goto cleanup;
> +
>      if (virNetworkSetBridgeName(&driver->networks, def, 1))
>          goto cleanup;
>  
> @@ -1295,12 +1298,16 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) {
>      virNetworkDefPtr def;
>      virNetworkObjPtr network = NULL;
>      virNetworkPtr ret = NULL;
> +    int dupNet;
> 

Why not just check the error code directly like in the other use?

Aside from that, ACK

- Cole

>      networkDriverLock(driver);
>  
>      if (!(def = virNetworkDefParseString(xml)))
>          goto cleanup;
>  
> +    if ((dupNet = virNetworkObjIsDuplicate(&driver->networks, def, 0)) < 0)
> +        goto cleanup;
> +
>      if (virNetworkSetBridgeName(&driver->networks, def, 1))
>          goto cleanup;
>