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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list