5:05 a.m.
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(a)redhat.com>
---
src/conf/network_conf.c | 46 +++++++++++++++++++++++++++++++++++++---------
1 file changed, 37 insertions(+), 9 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 8cf9ffd..7af303e 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -157,16 +157,19 @@ virNetworkObjPtr
virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
const unsigned char *uuid)
{
+ virNetworkObjPtr ret = NULL;
size_t i;
for (i = 0; i < nets->count; i++) {
virObjectLock(nets->objs[i]);
- if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN))
- return nets->objs[i];
+ if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) {
+ ret = nets->objs[i];
+ break;
+ }
virObjectUnlock(nets->objs[i]);
}
- return NULL;
+ return ret;
}
virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
@@ -184,16 +187,19 @@ virNetworkObjPtr
virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
const char *name)
{
+ virNetworkObjPtr ret = NULL;
size_t i;
for (i = 0; i < nets->count; i++) {
virObjectLock(nets->objs[i]);
- if (STREQ(nets->objs[i]->def->name, name))
- return nets->objs[i];
+ if (STREQ(nets->objs[i]->def->name, name)) {
+ ret = nets->objs[i];
+ break;
+ }
virObjectUnlock(nets->objs[i]);
}
- return NULL;
+ return ret;
}
virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
const char *name)
@@ -467,13 +473,17 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
{
virNetworkObjPtr network;
- 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);
if (VIR_APPEND_ELEMENT_COPY(nets->objs, nets->count, network) < 0)
@@ -481,9 +491,11 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
network->def = def;
network->persistent = !live;
+ virObjectUnlock(nets);
return network;
error:
+ virObjectUnlock(nets);
virObjectUnlock(network);
virObjectUnref(network);
return NULL;
@@ -654,6 +666,7 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
size_t i;
virObjectUnlock(net);
+ virObjectLock(nets);
for (i = 0; i < nets->count; i++) {
virObjectLock(nets->objs[i]);
if (nets->objs[i] == net) {
@@ -664,6 +677,7 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
}
virObjectUnlock(nets->objs[i]);
}
+ virObjectUnlock(nets);
}
/* return ips[index], or NULL if there aren't enough ips */
@@ -3158,6 +3172,7 @@ int virNetworkBridgeInUse(virNetworkObjListPtr nets,
size_t i;
unsigned int ret = 0;
+ virObjectLock(nets);
for (i = 0; i < nets->count; i++) {
virObjectLock(nets->objs[i]);
if (nets->objs[i]->def->bridge &&
@@ -3166,6 +3181,7 @@ int virNetworkBridgeInUse(virNetworkObjListPtr nets,
ret = 1;
virObjectUnlock(nets->objs[i]);
}
+ virObjectUnlock(nets);
return ret;
}
@@ -4325,6 +4341,7 @@ virNetworkObjListExport(virConnectPtr conn,
if (nets && VIR_ALLOC_N(tmp_nets, netobjs->count + 1) < 0)
goto cleanup;
+ virObjectLock(netobjs);
for (i = 0; i < netobjs->count; i++) {
virNetworkObjPtr netobj = netobjs->objs[i];
virObjectLock(netobj);
@@ -4335,6 +4352,7 @@ virNetworkObjListExport(virConnectPtr conn,
netobj->def->name,
netobj->def->uuid))) {
virObjectUnlock(netobj);
+ virObjectUnlock(netobjs);
goto cleanup;
}
tmp_nets[nnets] = net;
@@ -4343,6 +4361,7 @@ virNetworkObjListExport(virConnectPtr conn,
}
virObjectUnlock(netobj);
}
+ virObjectUnlock(netobjs);
if (tmp_nets) {
/* trim the array to the final size */
@@ -4370,7 +4389,9 @@ virNetworkObjListExport(virConnectPtr conn,
* @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.
*/
@@ -4382,10 +4403,12 @@ virNetworkObjListForEach(virNetworkObjListPtr nets,
int ret = 0;
size_t i = 0;
+ virObjectLock(nets);
for (i = 0; i < nets->count; i++) {
if (callback(nets->objs[i], opaque) < 0)
ret = -1;
}
+ virObjectUnlock(nets);
return ret;
}
@@ -4401,6 +4424,7 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
int got = 0;
size_t i;
+ virObjectLock(nets);
for (i = 0; i < nets->count && got < nnames; i++) {
virNetworkObjPtr obj = nets->objs[i];
virObjectLock(obj);
@@ -4413,12 +4437,14 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
(!active && !virNetworkObjIsActive(obj))) {
if (VIR_STRDUP(names[got], obj->def->name) < 0) {
virObjectUnlock(obj);
+ virObjectUnlock(nets);
goto error;
}
got++;
}
virObjectUnlock(obj);
}
+ virObjectUnlock(nets);
return got;
@@ -4437,6 +4463,7 @@ virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets,
int count = 0;
size_t i;
+ virObjectLock(nets);
for (i = 0; i < nets->count; i++) {
virNetworkObjPtr obj = nets->objs[i];
virObjectLock(obj);
@@ -4450,6 +4477,7 @@ virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets,
count++;
virObjectUnlock(obj);
}
+ virObjectUnlock(nets);
return count;
}
--
2.0.5