[libvirt] [PATCH] network driver: Fix regression defining colliding networks at daemon startup.

[Daniel P. Berrange]

On Thu, May 28, 2009 at 01:16:25PM -0400, Cole Robinson wrote:
> If two virtual networks have the same hardcoded bridge device (which
> prevents them from being active simultaneously) we still want to define
> them at daemon startup, so the user has a fighting chance of correcting
> the XML error.
> 
> Add an extra flag to SetBridge to avoid reporting an error if there is
> a bridge collision, and use this when loading network configs at startup.
> 
> This regressed via commit 6c2c73fc.
> 
> Signed-off-by: Cole Robinson <crobinso at redhat.com>

ACK

> 
> diff --git a/src/network_conf.c b/src/network_conf.c
> index b4da3fb..1e0cbb8 100644
> --- a/src/network_conf.c
> +++ b/src/network_conf.c
> @@ -724,7 +724,6 @@ virNetworkObjPtr virNetworkLoadConfig(virConnectPtr conn,
>      virNetworkDefPtr def = NULL;
>      virNetworkObjPtr net;
>      int autostart;
> -    char *tmp;
>  
>      if ((configFile = virNetworkConfigFile(conn, configDir, name)) == NULL)
>          goto error;
> @@ -745,13 +744,10 @@ virNetworkObjPtr virNetworkLoadConfig(virConnectPtr conn,
>          goto error;
>      }
>  
> -    /* Generate a bridge if none is found, but don't check for collisions
> +    /* Generate a bridge if none is specified, but don't check for collisions
>       * if a bridge is hardcoded, so the network is at least defined
>       */
> -    if ((tmp = virNetworkAllocateBridge(conn, nets, def->bridge)) != NULL) {
> -        VIR_FREE(def->bridge);
> -        def->bridge = tmp;
> -    } else
> +    if (virNetworkSetBridgeName(conn, nets, def, 0))
>          goto error;
>  
>      if (!(net = virNetworkAssignDef(conn, nets, def)))
> @@ -913,12 +909,17 @@ char *virNetworkAllocateBridge(virConnectPtr conn,
>  
>  int virNetworkSetBridgeName(virConnectPtr conn,
>                              const virNetworkObjListPtr nets,
> -                            virNetworkDefPtr def) {
> +                            virNetworkDefPtr def,
> +                            int check_collision) {
>  
>      int ret = -1;
>  
>      if (def->bridge && !strstr(def->bridge, "%d")) {
> -        if (virNetworkBridgeInUse(nets, def->bridge, def->name)) {
> +        /* We may want to skip collision detection in this case (ex. when
> +         * loading configs at daemon startup, so the network is at least
> +         * defined. */
> +        if (check_collision &&
> +            virNetworkBridgeInUse(nets, def->bridge, def->name)) {
>              networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
>                                 _("bridge name '%s' already in use."),
>                                 def->bridge);
> diff --git a/src/network_conf.h b/src/network_conf.h
> index 365d469..1d51c83 100644
> --- a/src/network_conf.h
> +++ b/src/network_conf.h
> @@ -179,7 +179,8 @@ char *virNetworkAllocateBridge(virConnectPtr conn,
>  
>  int virNetworkSetBridgeName(virConnectPtr conn,
>                              const virNetworkObjListPtr nets,
> -                            virNetworkDefPtr def);
> +                            virNetworkDefPtr def,
> +                            int check_collision);
>  
>  void virNetworkObjLock(virNetworkObjPtr obj);
>  void virNetworkObjUnlock(virNetworkObjPtr obj);
> diff --git a/src/network_driver.c b/src/network_driver.c
> index 3518e01..10d5fd3 100644
> --- a/src/network_driver.c
> +++ b/src/network_driver.c
> @@ -1096,7 +1096,7 @@ static virNetworkPtr networkCreate(virConnectPtr conn, const char *xml) {
>      if (!(def = virNetworkDefParseString(conn, xml)))
>          goto cleanup;
>  
> -    if (virNetworkSetBridgeName(conn, &driver->networks, def))
> +    if (virNetworkSetBridgeName(conn, &driver->networks, def, 1))
>          goto cleanup;
>  
>      if (!(network = virNetworkAssignDef(conn,
> @@ -1133,7 +1133,7 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) {
>      if (!(def = virNetworkDefParseString(conn, xml)))
>          goto cleanup;
>  
> -    if (virNetworkSetBridgeName(conn, &driver->networks, def))
> +    if (virNetworkSetBridgeName(conn, &driver->networks, def, 1))
>          goto cleanup;
>  
>      if (!(network = virNetworkAssignDef(conn,



Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|