[libvirt] [PATCH] Account for defined networks when generating bridge names

[Cole Robinson <crobinso redhat com>]

This is the second part of the fix for rhbz 479622. If we are generating
a bridge name for a virtual network, don't collide with any bridge name
in a defined network. This patch also generates a bridge name at network
define time, if none was passed in the xml.

The downside to all this is that it won't fix things for existing
victims of the bug: if they have 2 networks with the same bridge device
in the xml, we can't intelligently remedy the situation. This patch just
helps prevent future users getting into that predicament.

Thanks,
Cole

diff --git a/src/bridge.c b/src/bridge.c
index fc11429..4446a95 100644
--- a/src/bridge.c
+++ b/src/bridge.c
@@ -49,7 +49,7 @@
 #include "util.h"
 #include "logging.h"
 
-#define MAX_BRIDGE_ID 256
+#define MAX_TAP_ID 256
 
 #define JIFFIES_TO_MS(j) (((j)*1000)/HZ)
 #define MS_TO_JIFFIES(ms) (((ms)*HZ)/1000)
@@ -127,32 +127,13 @@ brShutdown(brControl *ctl)
 #ifdef SIOCBRADDBR
 int
 brAddBridge(brControl *ctl,
-            char **name)
+            char *name)
 {
     if (!ctl || !ctl->fd || !name)
         return EINVAL;
 
-    if (*name) {
-        if (ioctl(ctl->fd, SIOCBRADDBR, *name) == 0)
-            return 0;
-    } else {
-        int id = 0;
-        do {
-            char try[50];
-
-            snprintf(try, sizeof(try), "virbr%d", id);
-
-            if (ioctl(ctl->fd, SIOCBRADDBR, try) == 0) {
-                if (!(*name = strdup(try))) {
-                    ioctl(ctl->fd, SIOCBRDELBR, name);
-                    return ENOMEM;
-                }
-                return 0;
-            }
-
-            id++;
-        } while (id < MAX_BRIDGE_ID);
-    }
+    if (ioctl(ctl->fd, SIOCBRADDBR, name) == 0)
+        return 0;
 
     return errno;
 }
@@ -546,7 +527,7 @@ brAddTap(brControl *ctl,
         }
 
         id++;
-    } while (subst && id <= MAX_BRIDGE_ID);
+    } while (subst && id <= MAX_TAP_ID);
 
  error:
     close(fd);
diff --git a/src/bridge.h b/src/bridge.h
index 2491123..59c92dc 100644
--- a/src/bridge.h
+++ b/src/bridge.h
@@ -47,7 +47,7 @@ int     brInit                  (brControl **ctl);
 void    brShutdown              (brControl *ctl);
 
 int     brAddBridge             (brControl *ctl,
-                                 char **name);
+                                 char *name);
 int     brDeleteBridge          (brControl *ctl,
                                  const char *name);
 int     brHasBridge             (brControl *ctl,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index fd4ea61..65cbbf0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -191,6 +191,7 @@ virFree;
 
 
 # network_conf.h
+virNetworkAllocateBridge;
 virNetworkAssignDef;
 virNetworkBridgeInUse;
 virNetworkConfigFile;
diff --git a/src/network_conf.c b/src/network_conf.c
index 0d0545f..50daf1d 100644
--- a/src/network_conf.c
+++ b/src/network_conf.c
@@ -43,6 +43,7 @@
 #include "buf.h"
 #include "c-ctype.h"
 
+#define MAX_BRIDGE_ID 256
 #define VIR_FROM_THIS VIR_FROM_NETWORK
 
 VIR_ENUM_DECL(virNetworkForward)
@@ -860,6 +861,34 @@ char *virNetworkConfigFile(virConnectPtr conn,
     return ret;
 }
 
+char *virNetworkAllocateBridge(virConnectPtr conn,
+                               const virNetworkObjListPtr nets)
+{
+
+    int id = 0;
+    char *newname;
+
+    do {
+        char try[50];
+
+        snprintf(try, sizeof(try), "virbr%d", id);
+
+        if (!virNetworkBridgeInUse(nets, try)) {
+            if (!(newname = strdup(try))) {
+                virReportOOMError(conn);
+                return NULL;
+            }
+            return newname;
+        }
+
+        id++;
+    } while (id < MAX_BRIDGE_ID);
+
+    virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                          _("Bridge generation exceeded max id %d"),
+                          MAX_BRIDGE_ID);
+    return NULL;
+}
 
 void virNetworkObjLock(virNetworkObjPtr obj)
 {
diff --git a/src/network_conf.h b/src/network_conf.h
index 5ed549e..b3f0b13 100644
--- a/src/network_conf.h
+++ b/src/network_conf.h
@@ -166,6 +166,8 @@ char *virNetworkConfigFile(virConnectPtr conn,
                            const char *dir,
                            const char *name);
 
+char *virNetworkAllocateBridge(virConnectPtr conn,
+                               const virNetworkObjListPtr nets);
 
 void virNetworkObjLock(virNetworkObjPtr obj);
 void virNetworkObjUnlock(virNetworkObjPtr obj);
diff --git a/src/network_driver.c b/src/network_driver.c
index d750565..d83f902 100644
--- a/src/network_driver.c
+++ b/src/network_driver.c
@@ -812,7 +812,12 @@ static int networkStartNetworkDaemon(virConnectPtr conn,
         return -1;
     }
 
-    if ((err = brAddBridge(driver->brctl, &network->def->bridge))) {
+    if (!network->def->bridge &&
+        !(network->def->bridge = virNetworkAllocateBridge(conn,
+                                                          &driver->networks)))
+        return -1;
+
+    if ((err = brAddBridge(driver->brctl, network->def->bridge))) {
         virReportSystemError(conn, err,
                              _("cannot create bridge '%s'"),
                              network->def->bridge);
@@ -1147,11 +1152,18 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) {
     if (!(def = virNetworkDefParseString(conn, xml)))
         goto cleanup;
 
-    if (def->bridge &&
-        virNetworkBridgeInUse(&driver->networks, def->bridge)) {
-        networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-                           _("bridge name '%s' already in use."), def->bridge);
-        goto cleanup;
+    if (def->bridge) {
+        if (virNetworkBridgeInUse(&driver->networks, def->bridge)) {
+            networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                               _("bridge name '%s' already in use."),
+                               def->bridge);
+            goto cleanup;
+        }
+    } else {
+        /* Allocate a bridge name */
+        if (!(def->bridge = virNetworkAllocateBridge(conn,
+                                                     &driver->networks)))
+            goto cleanup;
     }
 
     if (!(network = virNetworkAssignDef(conn,