Re: [libvirt] [PATCH v2 19/24] bridge_driver: Drop networkDriverLock() from almost everywhere
On Thu, Mar 05, 2015 at 12:05:20 +0100, Michal Privoznik wrote:
Now that we have fine grained locks, there's no need to lock the
whole driver. We can rely on self-locking APIs.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/network/bridge_driver.c | 49 +++++++--------------------------------------
1 file changed, 7 insertions(+), 42 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index d3f3f4a..529ba2b 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -129,9 +129,7 @@ networkObjFromNetwork(virNetworkPtr net)
virNetworkObjPtr network;
char uuidstr[VIR_UUID_STRING_BUFLEN];
- networkDriverLock();
network = virNetworkObjFindByUUID(driver->networks, net->uuid);
- networkDriverUnlock();
if (!network) {
virUUIDFormat(net->uuid, uuidstr);
@@ -264,6 +262,11 @@ networkRemoveInactive(virNetworkObjPtr net)
int ret = -1;
+ virObjectRef(net);
Since you've already added proper reference counting in the previous
patch and the callers of this function will always have the reference
you don't need to ref it here ...
+ virObjectUnlock(net);
+ networkDriverLock();
+ virObjectLock(net);
... also since there are only two functions that call tis piece of code
you can avoid removing the network lock from them and avoid doing this
locking dance.
+
/* remove the (possibly) existing dnsmasq and radvd files */
if (!(dctx = dnsmasqContextNew(def->name,
driver->dnsmasqStateDir))) {
...
@@ -700,11 +705,9 @@ networkStateAutoStart(void)
if (!driver)
return;
- networkDriverLock();
virNetworkObjListForEach(driver->networks,
networkAutostartConfig,
In the callback above the networkStartNetwork() function is called that
accesses the network driver via the reference in the global variable,
but the accessed member is driver->stateDir which is immutable when
looking at the code.
Please annotate (in a separate patch) that the following fields are
immutable:
driver->networkConfigDir
driver->networkAutostartDir
driver->stateDir
driver->pidDir
driver->dnsmasqStateDir
driver->radvdStateDir
and that the network list is self locking so that people after me won't
need to find it out again.
ACK
Peter
Attachments:
- signature.asc (application/pgp-signature — 819 bytes)