Daniel P. Berrange wrote:
On Mon, Feb 16, 2009 at 06:40:59PM -0500, Cole Robinson wrote:
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))) {
This will cause a thread deadlock once you add the locking I describe for virNetworkBridgeInUse() in the previous patch. This is because the current virNetworkObjPtr 'network' here will be locked, then the function you're calling with then try to lock it again.
A deep called function like networkStartNetworkDaemon() shouldn't be iterating over all network objects, so this is the wrong place to try and fix this problem.
I'm guessing you're trying to fix up existing defined networks without a bridge here, so IMHO, this is better done at daemon startup, where we load all the configs off disk. This will avoid the locking trouble
Do it in 'networkStartup',m just after the virNetworkLoadAllConfigs call, but before autostart is done.
Okay, I rolled these changes and the BridgeInUse changes into one patch (along with Jim's suggestions). I added a helper function SetBridgeName which basically does: if user passed bridge name: verify it doesn't collide else: generate bridge name We call this in Define and CreateXML. When loading configs from a driver restart, we only attempt to generate a bridge: if checking for collisions failed, the network wouldn't even be defined, which would only cause more pain for users. Patch below. Thanks, Cole