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@xxxxxxxxxx>
> ---
> 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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list