On Sun, Nov 30, 2008 at 11:55:51PM +0000, Daniel P. Berrange wrote:
This patch makes the network driver thread safe, following the pattern of the earlier patches
@@ -95,6 +107,7 @@ networkAutostartConfigs(struct network_d unsigned int i;
Hum, you're not locking the driver itself during the loop ? If a concurrent call were to remove one of the network object in the meantime, that would lead to potentially random memory accesses no ?
for (i = 0 ; i < driver->networks.count ; i++) { + virNetworkObjLock(driver->networks.objs[i]); if (driver->networks.objs[i]->autostart && !virNetworkIsActive(driver->networks.objs[i]) && networkStartNetworkDaemon(NULL, driver, driver->networks.objs[i]) < 0) { @@ -103,6 +116,7 @@ networkAutostartConfigs(struct network_d driver->networks.objs[i]->def->name, err ? err->message : NULL); } + virNetworkObjUnlock(driver->networks.objs[i]); } }
[...]
cleanup: + if (network) + virNetworkObjUnlock(network);
like for domain, I would suggest the unlocking accessor to accept NULL and drop all the if constructs If there is a good reason networkAutostartConfigs() doesn't need to lock the top level driver lock, okay, otherwise +1 after fix, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/