On Fri, Apr 17, 2009 at 10:45:19AM +0100, Daniel P. Berrange wrote:
>>> The problem with this approach is that the bridge name potentially
>>> ends up being different every time the virtual network is started.
>>> The bridge name needs to be stable,
>> Why? I can't remember ever having to refer to it, and if I did, I can
>> get that information at runtime by parsing the output from "virsh
>> net-dumpxml <name of the network>". What am I not thinking of? :)
> The non-QEMU drivers I'm afraid.
Ah, I see. I've not spent much time with those. Longer term, would it
perhaps make sense to let them specify the name of the network they want
to use like we do for the QEMU driver?
> > Index: libvirt-0.6.1/src/network_conf.c
> > ===================================================================
> > --- libvirt-0.6.1.orig/src/network_conf.c 2009-04-16 20:49:28.851655724 +0200
> > +++ libvirt-0.6.1/src/network_conf.c 2009-04-17 09:42:33.075084537 +0200
> > @@ -747,7 +747,7 @@
> > /* Generate a bridge if none is found, but don't check for collisions
> > * if a bridge is hardcoded, so the network is at least defined
> > */
> > - if (!def->bridge && !(def->bridge = virNetworkAllocateBridge(conn, nets)))
> > + if (!(def->bridge = virNetworkAllocateBridge(conn, nets, def->bridge)))
> > goto error;
> I think this would leak memory - the def-bridge string being passed in
> is overwritten upon return, if its a non-null string.
Good catch.
Updated patch:
Index: libvirt-0.6.1/src/network_conf.c
===================================================================
--- libvirt-0.6.1.orig/src/network_conf.c 2009-04-16 20:49:28.851655724 +0200
+++ libvirt-0.6.1/src/network_conf.c 2009-04-17 13:11:54.494770980 +0200
@@ -724,6 +724,7 @@
virNetworkDefPtr def = NULL;
virNetworkObjPtr net;
int autostart;
+ char *tmp;
if ((configFile = virNetworkConfigFile(conn, configDir, name)) == NULL)
goto error;
@@ -747,7 +748,10 @@
/* Generate a bridge if none is found, but don't check for collisions
* if a bridge is hardcoded, so the network is at least defined
*/
- if (!def->bridge && !(def->bridge = virNetworkAllocateBridge(conn, nets)))
+ if (tmp = virNetworkAllocateBridge(conn, nets, def->bridge)) {
+ VIR_FREE(def->bridge);
+ def->bridge = tmp;
+ } else
goto error;
if (!(net = virNetworkAssignDef(conn, nets, def)))
@@ -875,16 +879,20 @@
}
char *virNetworkAllocateBridge(virConnectPtr conn,
- const virNetworkObjListPtr nets)
+ const virNetworkObjListPtr nets,
+ const char *template)
{
int id = 0;
char *newname;
+ if (!template)
+ template = "virbr%d";
+
do {
char try[50];
- snprintf(try, sizeof(try), "virbr%d", id);
+ snprintf(try, sizeof(try), template, id);
if (!virNetworkBridgeInUse(nets, try, NULL)) {
if (!(newname = strdup(try))) {
@@ -909,7 +917,7 @@
int ret = -1;
- if (def->bridge) {
+ if (def->bridge && !strstr(def->bridge, "%d")) {
if (virNetworkBridgeInUse(nets, def->bridge, def->name)) {
networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("bridge name '%s' already in use."),
@@ -918,7 +926,7 @@
}
} else {
/* Allocate a bridge name */
- if (!(def->bridge = virNetworkAllocateBridge(conn, nets)))
+ if (!(def->bridge = virNetworkAllocateBridge(conn, nets, def->bridge)))
goto error;
}
Index: libvirt-0.6.1/src/network_conf.h
===================================================================
--- libvirt-0.6.1.orig/src/network_conf.h 2009-04-16 20:49:28.899655820 +0200
+++ libvirt-0.6.1/src/network_conf.h 2009-04-17 09:42:33.075084537 +0200
@@ -174,7 +174,8 @@
const char *skipname);
char *virNetworkAllocateBridge(virConnectPtr conn,
- const virNetworkObjListPtr nets);
+ const virNetworkObjListPtr nets,
+ const char *template);
int virNetworkSetBridgeName(virConnectPtr conn,
const virNetworkObjListPtr nets,
--
Soren Hansen |
Lead Virtualisation Engineer | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list