[PATCH v3 09/15] virNetworkObjListPtr: Make APIs self-locking

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

Every API that touches internal structure of the object must lock
the object first. Not every API that has the object as an
argument needs to do that though. Some APIs just pass the object
to lower layers which, however, must lock the object then. Look
at the code, you'll get my meaning soon.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
 src/conf/network_conf.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index be5cf0e..98342f9 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -488,13 +488,17 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
     virNetworkObjPtr network;
     char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-    if ((network = virNetworkObjFindByName(nets, def->name))) {
+    virObjectLock(nets);
+    if ((network = virNetworkObjFindByNameLocked(nets, def->name))) {
+        virObjectUnlock(nets);
         virNetworkObjAssignDef(network, def, live);
         return network;
     }
 
-    if (!(network = virNetworkObjNew()))
+    if (!(network = virNetworkObjNew())) {
+        virObjectUnlock(nets);
         return NULL;
+    }
     virObjectLock(network);
 
     virUUIDFormat(def->uuid, uuidstr);
@@ -503,9 +507,11 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
 
     network->def = def;
     network->persistent = !live;
+    virObjectUnlock(nets);
     return network;
 
  error:
+    virObjectUnlock(nets);
     virObjectUnlock(network);
     virObjectUnref(network);
     return NULL;
@@ -676,8 +682,14 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
     char uuidstr[VIR_UUID_STRING_BUFLEN];
 
     virUUIDFormat(net->def->uuid, uuidstr);
+    virObjectRef(net);
     virObjectUnlock(net);
+    virObjectLock(nets);
+    virObjectLock(net);
     virHashRemoveEntry(nets->objs, uuidstr);
+    virObjectUnlock(net);
+    virObjectUnlock(nets);
+    virObjectUnref(net);
 }
 
 /* return ips[index], or NULL if there aren't enough ips */
@@ -3195,7 +3207,9 @@ int virNetworkBridgeInUse(virNetworkObjListPtr nets,
     virNetworkObjPtr obj;
     struct virNetworkBridgeInUseHelperData data = {bridge, skipname};
 
+    virObjectLock(nets);
     obj = virHashSearch(nets->objs, virNetworkBridgeInUseHelper, &data);
+    virObjectUnlock(nets);
 
     return obj != NULL;
 }
@@ -4395,6 +4409,7 @@ virNetworkObjListExport(virConnectPtr conn,
     int ret = -1;
     struct virNetworkObjListData data = { conn, NULL, filter, flags, 0, false};
 
+    virObjectLock(netobjs);
     if (nets && VIR_ALLOC_N(data.nets, virHashSize(netobjs->objs) + 1) < 0)
         goto cleanup;
 
@@ -4412,6 +4427,7 @@ virNetworkObjListExport(virConnectPtr conn,
 
     ret = data.nnets;
  cleanup:
+    virObjectUnlock(netobjs);
     while (data.nets && data.nnets)
         virObjectUnref(data.nets[--data.nnets]);
 
@@ -4443,7 +4459,9 @@ virNetworkObjListForEachHelper(void *payload,
  * @opaque: pointer to pass to the @callback
  *
  * Function iterates over the list of network objects and calls
- * passed callback over each one of them.
+ * passed callback over each one of them. You should avoid
+ * calling those virNetworkObjList APIs, which lock the list
+ * again in favor of their virNetworkObj*Locked variants.
  *
  * Returns: 0 on success, -1 otherwise.
  */
@@ -4453,7 +4471,9 @@ virNetworkObjListForEach(virNetworkObjListPtr nets,
                          void *opaque)
 {
     struct virNetworkObjListForEachHelperData data = {callback, opaque, 0};
+    virObjectLock(nets);
     virHashForEach(nets->objs, virNetworkObjListForEachHelper, &data);
+    virObjectUnlock(nets);
     return data.ret;
 }
 
@@ -4515,7 +4535,9 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
     struct virNetworkObjListGetHelperData data = {
         conn, filter, names, nnames, active, 0, false};
 
+    virObjectLock(nets);
     virHashForEach(nets->objs, virNetworkObjListGetHelper, &data);
+    virObjectUnlock(nets);
 
     if (data.error)
         goto cleanup;
@@ -4538,7 +4560,9 @@ virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets,
     struct virNetworkObjListGetHelperData data = {
         conn, filter, NULL, -1, active, 0, false};
 
+    virObjectLock(nets);
     virHashForEach(nets->objs, virNetworkObjListGetHelper, &data);
+    virObjectUnlock(nets);
 
     return data.got;
 }
@@ -4576,5 +4600,7 @@ virNetworkObjListPrune(virNetworkObjListPtr nets,
 {
     struct virNetworkObjListPruneHelperData data = {flags};
 
+    virObjectLock(nets);
     virHashRemoveSet(nets->objs, virNetworkObjListPruneHelper, &data);
+    virObjectUnlock(nets);
 }
-- 
2.0.5

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list