virNetworkObjFindBy*: Return an reference to found object

This patch turns both virNetworkObjFindByUUID() and
virNetworkObjFindByName() to return an referenced object so that
even if caller unlocks it, it's for sure that object won't
disappear meanwhile. Especially if the object (in general) is
locked and unlocked during the caller run.
Moreover, this commit is nicely small, since the object unrefing
can be done in virNetworkObjEndAPI().

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 87ee3494c7d92e0f701ea671ba00077395a603a3..d7c5dec48e1f3f8459c5ea68d214799df7415b6e 100644 (file)
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -136,6 +136,7 @@ virNetworkObjEndAPI(virNetworkObjPtr *net)
         return;
 
     virObjectUnlock(*net);
+    virObjectUnref(*net);
     *net = NULL;
 }
 
@@ -157,6 +158,15 @@ virNetworkObjListPtr virNetworkObjListNew(void)
     return nets;
 }
 
+/**
+ * virNetworkObjFindByUUIDLocked:
+ * @nets: list of network objects
+ * @uuid: network uuid to find
+ *
+ * This functions requires @nets to be locked already!
+ *
+ * Returns: not locked, but ref'd network object.
+ */
 virNetworkObjPtr
 virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
                               const unsigned char *uuid)
@@ -168,10 +178,20 @@ virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
 
     ret = virHashLookup(nets->objs, uuidstr);
     if (ret)
-        virObjectLock(ret);
+        virObjectRef(ret);
     return ret;
 }
 
+/**
+ * virNetworkObjFindByUUID:
+ * @nets: list of network objects
+ * @uuid: network uuid to find
+ *
+ * This functions locks @nets and find network object which
+ * corresponds to @uuid.
+ *
+ * Returns: locked and ref'd network object.
+ */
 virNetworkObjPtr
 virNetworkObjFindByUUID(virNetworkObjListPtr nets,
                         const unsigned char *uuid)
@@ -181,6 +201,8 @@ virNetworkObjFindByUUID(virNetworkObjListPtr nets,
     virObjectLock(nets);
     ret = virNetworkObjFindByUUIDLocked(nets, uuid);
     virObjectUnlock(nets);
+    if (ret)
+        virObjectLock(ret);
     return ret;
 }
 
@@ -199,6 +221,15 @@ virNetworkObjSearchName(const void *payload,
     return want;
 }
 
+/*
+ * virNetworkObjFindByNameLocked:
+ * @nets: list of network objects
+ * @name: network name to find
+ *
+ * This functions requires @nets to be locked already!
+ *
+ * Returns: not locked, but ref'd network object.
+ */
 virNetworkObjPtr
 virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
                               const char *name)
@@ -207,10 +238,20 @@ virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
 
     ret = virHashSearch(nets->objs, virNetworkObjSearchName, name);
     if (ret)
-        virObjectLock(ret);
+        virObjectRef(ret);
     return ret;
 }
 
+/**
+ * virNetworkObjFindByName:
+ * @nets: list of network objects
+ * @name: network name to find
+ *
+ * This functions locks @nets and find network object which
+ * corresponds to @name.
+ *
+ * Returns: locked and ref'd network object.
+ */
 virNetworkObjPtr
 virNetworkObjFindByName(virNetworkObjListPtr nets,
                         const char *name)
@@ -220,6 +261,8 @@ virNetworkObjFindByName(virNetworkObjListPtr nets,
     virObjectLock(nets);
     ret = virNetworkObjFindByNameLocked(nets, name);
     virObjectUnlock(nets);
+    if (ret)
+        virObjectLock(ret);
     return ret;
 }
 
@@ -484,6 +527,7 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
     virObjectLock(nets);
     if ((network = virNetworkObjFindByNameLocked(nets, def->name))) {
         virObjectUnlock(nets);
+        virObjectLock(network);
         virNetworkObjAssignDef(network, def, live);
         return network;
     }
@@ -500,13 +544,13 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
 
     network->def = def;
     network->persistent = !live;
+    virObjectRef(network);
     virObjectUnlock(nets);
     return network;
 
  error:
     virObjectUnlock(nets);
-    virObjectUnlock(network);
-    virObjectUnref(network);
+    virNetworkObjEndAPI(&network);
     return NULL;
 
 }
@@ -680,7 +724,6 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
     virObjectLock(nets);
     virObjectLock(net);
     virHashRemoveEntry(nets->objs, uuidstr);
-    virObjectUnlock(net);
     virObjectUnlock(nets);
     virObjectUnref(net);
 }

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 50b97daf7745fe539c0829828724f64e103763f6..ed05ace6ddd0e8b4aa0124b00c7d6e6ad3f84b6b 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2939,7 +2939,6 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn, const char *xml)
     if (networkStartNetwork(network) < 0) {
         virNetworkRemoveInactive(driver->networks,
                                  network);
-        network = NULL;
         goto cleanup;
     }
 
@@ -2988,7 +2987,6 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn, const char *xml)
     if (virNetworkSaveConfig(driver->networkConfigDir, def) < 0) {
         if (!virNetworkObjIsActive(network)) {
             virNetworkRemoveInactive(driver->networks, network);
-            network = NULL;
             goto cleanup;
         }
         /* if network was active already, just undo new persistent
@@ -3052,11 +3050,8 @@ networkUndefine(virNetworkPtr net)
 
     VIR_INFO("Undefining network '%s'", network->def->name);
     if (!active) {
-        if (networkRemoveInactive(network) < 0) {
-            network = NULL;
+        if (networkRemoveInactive(network) < 0)
             goto cleanup;
-        }
-        network = NULL;
     } else {
 
         /* if the network still exists, it was active, and we need to make
@@ -3313,13 +3308,10 @@ static int networkDestroy(virNetworkPtr net)
                                         VIR_NETWORK_EVENT_STOPPED,
                                         0);
 
-    if (!network->persistent) {
-        if (networkRemoveInactive(network) < 0) {
-            network = NULL;
-            ret = -1;
-            goto cleanup;
-        }
-        network = NULL;
+    if (!network->persistent &&
+        networkRemoveInactive(network) < 0) {
+        ret = -1;
+        goto cleanup;
     }
 
  cleanup:

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 72f40ed81d1c99b7fce1dceabb83d8c0bd54c242..90af80a8c4c8cd0a512979bfc4bd831e172486a3 100644 (file)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -787,7 +787,7 @@ testOpenDefault(virConnectPtr conn)
         goto error;
     }
     netobj->active = 1;
-    virObjectUnlock(netobj);
+    virNetworkObjEndAPI(&netobj);
 
     if (!(interfacedef = virInterfaceDefParseString(defaultInterfaceXML)))
         goto error;
@@ -1155,7 +1155,7 @@ testParseNetworks(testConnPtr privconn,
         }
 
         obj->active = 1;
-        virObjectUnlock(obj);
+        virNetworkObjEndAPI(&obj);
     }
 
     ret = 0;
@@ -3733,7 +3733,6 @@ static int testNetworkUndefine(virNetworkPtr network)
                                         0);
 
     virNetworkRemoveInactive(privconn->networks, privnet);
-    privnet = NULL;
     ret = 0;
 
  cleanup:
@@ -3847,10 +3846,9 @@ static int testNetworkDestroy(virNetworkPtr network)
     event = virNetworkEventLifecycleNew(privnet->def->name, privnet->def->uuid,
                                         VIR_NETWORK_EVENT_STOPPED,
                                         0);
-    if (!privnet->persistent) {
+    if (!privnet->persistent)
         virNetworkRemoveInactive(privconn->networks, privnet);
-        privnet = NULL;
-    }
+
     ret = 0;
 
  cleanup: