On Fri, Feb 27, 2009 at 10:57:35AM -0500, Cole Robinson wrote:
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.
ACK, this looks safe now. 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 :|