11:45 a.m.
Well, this is interesting. I've turned the virNetworkObjList into
using a virHash. And even though the performance in my testing
(using the very same test program as in previous versions of the
patch set) increased. Without these patches, the test program
took 56s to finish. With these patches (that is with virHash
table) it takes 46s, which is better. But previously, with
network objects stored in flat array I was able to get somewhere
around 23s! I wonder what may be the cause.
NB, nearly all patches in the series have been ACKed already, but
since I'm switching the virNetworkObjList internals, I'm posting
them again.
Michal Privoznik (15):
virNetworkObjListPtr: Turn list into a hash table
network_conf: Make virNetworkObj actually virObject
network_conf: Introduce virNetworkObjEndAPI
bridge_driver: Use virNetworkObjEndAPI
test_driver: Use virNetworkObjEndAPI
parallels_network: Use virNetworkObjEndAPI
virNetworkObjList: Derive from virObjectLockableClass
network_conf: Introduce locked versions of lookup functions
virNetworkObjListPtr: Make APIs self-locking
virNetworkObjFindBy*: Return an reference to found object
struct _virNetworkDriverState: Annotate items
bridge_driver: Drop networkDriverLock() from everywhere
test_driver: Drop testDriverLock() from almost everywhere
parallels_network: Drop parallelsDriverLock() from everywhere.
bridge_driver: Use more of networkObjFromNetwork
cfg.mk | 2 -
src/conf/network_conf.c | 545 +++++++++++++++++++++++------------
src/conf/network_conf.h | 13 +-
src/libvirt_private.syms | 7 +-
src/network/bridge_driver.c | 197 +++----------
src/network/bridge_driver_platform.h | 5 +-
src/parallels/parallels_network.c | 62 +---
src/test/test_driver.c | 108 ++-----
tests/networkxml2conftest.c | 4 +-
tests/objectlocking.ml | 4 -
10 files changed, 441 insertions(+), 506 deletions(-)
--
2.0.5
11:45 a.m.
New subject: [libvirt] [PATCH v3 01/15] virNetworkObjListPtr: Turn list into a hash table
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/network_conf.c | 382 ++++++++++++++++++++++++++++++------------------
1 file changed, 237 insertions(+), 145 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 9c1d578..ae29c47 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -54,8 +54,7 @@
struct _virNetworkObjList {
virObject parent;
- size_t count;
- virNetworkObjPtr *objs;
+ virHashTablePtr objs;
};
VIR_ENUM_IMPL(virNetworkForward,
@@ -96,6 +95,13 @@ static int virNetworkObjOnceInit(void)
VIR_ONCE_GLOBAL_INIT(virNetworkObj)
+static void
+virNetworkObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED)
+{
+ virNetworkObjPtr obj = payload;
+ virNetworkObjFree(obj);
+}
+
virNetworkObjListPtr virNetworkObjListNew(void)
{
virNetworkObjListPtr nets;
@@ -106,37 +112,52 @@ virNetworkObjListPtr virNetworkObjListNew(void)
if (!(nets = virObjectNew(virNetworkObjListClass)))
return NULL;
+ if (!(nets->objs = virHashCreate(50, virNetworkObjListDataFree))) {
+ virObjectUnref(nets);
+ return NULL;
+ }
+
return nets;
}
virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
const unsigned char *uuid)
{
- size_t i;
+ virNetworkObjPtr ret = NULL;
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
- for (i = 0; i < nets->count; i++) {
- virNetworkObjLock(nets->objs[i]);
- if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN))
- return nets->objs[i];
- virNetworkObjUnlock(nets->objs[i]);
- }
+ virUUIDFormat(uuid, uuidstr);
- return NULL;
+ ret = virHashLookup(nets->objs, uuidstr);
+ if (ret)
+ virNetworkObjLock(ret);
+ return ret;
+}
+
+static int
+virNetworkObjSearchName(const void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ const void *data)
+{
+ virNetworkObjPtr net = (virNetworkObjPtr) payload;
+ int want = 0;
+
+ virNetworkObjLock(net);
+ if (STREQ(net->def->name, (const char *)data))
+ want = 1;
+ virNetworkObjUnlock(net);
+ return want;
}
virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
const char *name)
{
- size_t i;
+ virNetworkObjPtr ret = NULL;
- for (i = 0; i < nets->count; i++) {
- virNetworkObjLock(nets->objs[i]);
- if (STREQ(nets->objs[i]->def->name, name))
- return nets->objs[i];
- virNetworkObjUnlock(nets->objs[i]);
- }
-
- return NULL;
+ ret = virHashSearch(nets->objs, virNetworkObjSearchName, name);
+ if (ret)
+ virNetworkObjLock(ret);
+ return ret;
}
bool
@@ -315,12 +336,8 @@ static void
virNetworkObjListDispose(void *obj)
{
virNetworkObjListPtr nets = obj;
- size_t i;
- for (i = 0; i < nets->count; i++)
- virNetworkObjFree(nets->objs[i]);
-
- VIR_FREE(nets->objs);
+ virHashFree(nets->objs);
}
/*
@@ -403,6 +420,7 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
bool live)
{
virNetworkObjPtr network;
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
if ((network = virNetworkObjFindByName(nets, def->name))) {
virNetworkObjAssignDef(network, def, live);
@@ -419,8 +437,11 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
}
virNetworkObjLock(network);
- if (VIR_APPEND_ELEMENT_COPY(nets->objs, nets->count, network) < 0 ||
- !(network->class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE)))
+ if (!(network->class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE)))
+ goto error;
+
+ virUUIDFormat(def->uuid, uuidstr);
+ if (virHashAddEntry(nets->objs, uuidstr, network) < 0)
goto error;
/* The first three class IDs are already taken */
@@ -600,20 +621,11 @@ virNetworkConfigChangeSetup(virNetworkObjPtr network, unsigned int
flags)
void virNetworkRemoveInactive(virNetworkObjListPtr nets,
virNetworkObjPtr net)
{
- size_t i;
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ virUUIDFormat(net->def->uuid, uuidstr);
virNetworkObjUnlock(net);
- for (i = 0; i < nets->count; i++) {
- virNetworkObjLock(nets->objs[i]);
- if (nets->objs[i] == net) {
- virNetworkObjUnlock(nets->objs[i]);
- virNetworkObjFree(nets->objs[i]);
-
- VIR_DELETE_ELEMENT(nets->objs, i, nets->count);
- break;
- }
- virNetworkObjUnlock(nets->objs[i]);
- }
+ virHashRemoveEntry(nets->objs, uuidstr);
}
/* return ips[index], or NULL if there aren't enough ips */
@@ -3102,23 +3114,39 @@ char *virNetworkConfigFile(const char *dir,
return ret;
}
+struct virNetworkBridgeInUseHelperData {
+ const char *bridge;
+ const char *skipname;
+};
+
+static int
+virNetworkBridgeInUseHelper(const void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ const void *opaque)
+{
+ int ret = 0;
+ virNetworkObjPtr net = (virNetworkObjPtr) payload;
+ const struct virNetworkBridgeInUseHelperData *data = opaque;
+
+ virNetworkObjLock(net);
+ if (net->def->bridge &&
+ STREQ(net->def->bridge, data->bridge) &&
+ !(data->skipname && STREQ(net->def->name, data->skipname)))
+ ret = 1;
+ virNetworkObjUnlock(net);
+ return ret;
+}
+
int virNetworkBridgeInUse(virNetworkObjListPtr nets,
const char *bridge,
const char *skipname)
{
- size_t i;
- unsigned int ret = 0;
+ virNetworkObjPtr obj;
+ struct virNetworkBridgeInUseHelperData data = {bridge, skipname};
- for (i = 0; i < nets->count; i++) {
- virNetworkObjLock(nets->objs[i]);
- if (nets->objs[i]->def->bridge &&
- STREQ(nets->objs[i]->def->bridge, bridge) &&
- !(skipname && STREQ(nets->objs[i]->def->name, skipname)))
- ret = 1;
- virNetworkObjUnlock(nets->objs[i]);
- }
+ obj = virHashSearch(nets->objs, virNetworkBridgeInUseHelper, &data);
- return ret;
+ return obj != NULL;
}
char *virNetworkAllocateBridge(virNetworkObjListPtr nets,
@@ -4271,6 +4299,52 @@ virNetworkMatch(virNetworkObjPtr netobj,
}
#undef MATCH
+struct virNetworkObjListData {
+ virConnectPtr conn;
+ virNetworkPtr *nets;
+ virNetworkObjListFilter filter;
+ unsigned int flags;
+ int nnets;
+ bool error;
+};
+
+static void
+virNetworkObjListPopulate(void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ void *opaque)
+{
+ struct virNetworkObjListData *data = opaque;
+ virNetworkObjPtr obj = payload;
+ virNetworkPtr net = NULL;
+
+ if (data->error)
+ return;
+
+ virNetworkObjLock(obj);
+
+ if (data->filter &&
+ !data->filter(data->conn, obj->def))
+ goto cleanup;
+
+ if (!virNetworkMatch(obj, data->flags))
+ goto cleanup;
+
+ if (!data->nets) {
+ data->nnets++;
+ goto cleanup;
+ }
+
+ if (!(net = virGetNetwork(data->conn, obj->def->name,
obj->def->uuid))) {
+ data->error = true;
+ goto cleanup;
+ }
+
+ data->nets[data->nnets++] = net;
+
+ cleanup:
+ virNetworkObjUnlock(obj);
+}
+
int
virNetworkObjListExport(virConnectPtr conn,
virNetworkObjListPtr netobjs,
@@ -4278,53 +4352,50 @@ virNetworkObjListExport(virConnectPtr conn,
virNetworkObjListFilter filter,
unsigned int flags)
{
- virNetworkPtr *tmp_nets = NULL;
- virNetworkPtr net = NULL;
- int nnets = 0;
int ret = -1;
- size_t i;
+ struct virNetworkObjListData data = { conn, NULL, filter, flags, 0, false};
- if (nets && VIR_ALLOC_N(tmp_nets, netobjs->count + 1) < 0)
+ if (nets && VIR_ALLOC_N(data.nets, virHashSize(netobjs->objs) + 1) <
0)
goto cleanup;
- for (i = 0; i < netobjs->count; i++) {
- virNetworkObjPtr netobj = netobjs->objs[i];
- virNetworkObjLock(netobj);
- if ((!filter || filter(conn, netobj->def)) &&
- virNetworkMatch(netobj, flags)) {
- if (nets) {
- if (!(net = virGetNetwork(conn,
- netobj->def->name,
- netobj->def->uuid))) {
- virNetworkObjUnlock(netobj);
- goto cleanup;
- }
- tmp_nets[nnets] = net;
- }
- nnets++;
- }
- virNetworkObjUnlock(netobj);
- }
+ virHashForEach(netobjs->objs, virNetworkObjListPopulate, &data);
- if (tmp_nets) {
+ if (data.error)
+ goto cleanup;
+
+ if (data.nets) {
/* trim the array to the final size */
- ignore_value(VIR_REALLOC_N(tmp_nets, nnets + 1));
- *nets = tmp_nets;
- tmp_nets = NULL;
+ ignore_value(VIR_REALLOC_N(data.nets, data.nnets + 1));
+ *nets = data.nets;
+ data.nets = NULL;
}
- ret = nnets;
-
+ ret = data.nnets;
cleanup:
- if (tmp_nets) {
- for (i = 0; i < nnets; i++)
- virObjectUnref(tmp_nets[i]);
- }
+ while (data.nets && data.nnets)
+ virObjectUnref(data.nets[--data.nnets]);
- VIR_FREE(tmp_nets);
+ VIR_FREE(data.nets);
return ret;
}
+struct virNetworkObjListForEachHelperData {
+ virNetworkObjListIterator callback;
+ void *opaque;
+ int ret;
+};
+
+static void
+virNetworkObjListForEachHelper(void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ void *opaque)
+{
+ struct virNetworkObjListForEachHelperData *data = opaque;
+
+ if (data->callback(payload, data->opaque) < 0)
+ data->ret = -1;
+}
+
/**
* virNetworkObjListForEach:
* @nets: a list of network objects
@@ -4341,15 +4412,54 @@ virNetworkObjListForEach(virNetworkObjListPtr nets,
virNetworkObjListIterator callback,
void *opaque)
{
- int ret = 0;
- size_t i = 0;
+ struct virNetworkObjListForEachHelperData data = {callback, opaque, 0};
+ virHashForEach(nets->objs, virNetworkObjListForEachHelper, &data);
+ return data.ret;
+}
- for (i = 0; i < nets->count; i++) {
- if (callback(nets->objs[i], opaque) < 0)
- ret = -1;
+struct virNetworkObjListGetHelperData {
+ virConnectPtr conn;
+ virNetworkObjListFilter filter;
+ char **names;
+ int nnames;
+ bool active;
+ int got;
+ bool error;
+};
+
+static void
+virNetworkObjListGetHelper(void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ void *opaque)
+{
+ struct virNetworkObjListGetHelperData *data = opaque;
+ virNetworkObjPtr obj = payload;
+
+ if (data->error)
+ return;
+
+ if (data->nnames >= 0 &&
+ data->got == data->nnames)
+ return;
+
+ virNetworkObjLock(obj);
+
+ if (data->filter &&
+ !data->filter(data->conn, obj->def))
+ goto cleanup;
+
+ if ((data->active && virNetworkObjIsActive(obj)) ||
+ (!data->active && !virNetworkObjIsActive(obj))) {
+ if (data->names &&
+ VIR_STRDUP(data->names[data->got], obj->def->name) < 0) {
+ data->error = true;
+ goto cleanup;
+ }
+ data->got++;
}
- return ret;
+ cleanup:
+ virNetworkObjUnlock(obj);
}
int
@@ -4360,34 +4470,23 @@ virNetworkObjListGetNames(virNetworkObjListPtr nets,
virNetworkObjListFilter filter,
virConnectPtr conn)
{
- int got = 0;
- size_t i;
+ int ret = -1;
- for (i = 0; i < nets->count && got < nnames; i++) {
- virNetworkObjPtr obj = nets->objs[i];
- virNetworkObjLock(obj);
- if (filter && !filter(conn, obj->def)) {
- virNetworkObjUnlock(obj);
- continue;
- }
+ struct virNetworkObjListGetHelperData data = {
+ conn, filter, names, nnames, active, 0, false};
- if ((active && virNetworkObjIsActive(obj)) ||
- (!active && !virNetworkObjIsActive(obj))) {
- if (VIR_STRDUP(names[got], obj->def->name) < 0) {
- virNetworkObjUnlock(obj);
- goto error;
- }
- got++;
- }
- virNetworkObjUnlock(obj);
+ virHashForEach(nets->objs, virNetworkObjListGetHelper, &data);
+
+ if (data.error)
+ goto cleanup;
+
+ ret = data.got;
+ cleanup:
+ if (ret < 0) {
+ while (data.got)
+ VIR_FREE(data.names[--data.got]);
}
-
- return got;
-
- error:
- for (i = 0; i < got; i++)
- VIR_FREE(names[i]);
- return -1;
+ return ret;
}
int
@@ -4396,24 +4495,31 @@ virNetworkObjListNumOfNetworks(virNetworkObjListPtr nets,
virNetworkObjListFilter filter,
virConnectPtr conn)
{
- int count = 0;
- size_t i;
+ struct virNetworkObjListGetHelperData data = {
+ conn, filter, NULL, -1, active, 0, false};
- for (i = 0; i < nets->count; i++) {
- virNetworkObjPtr obj = nets->objs[i];
- virNetworkObjLock(obj);
- if (filter && !filter(conn, obj->def)) {
- virNetworkObjUnlock(obj);
- continue;
- }
+ virHashForEach(nets->objs, virNetworkObjListGetHelper, &data);
- if ((active && virNetworkObjIsActive(obj)) ||
- (!active && !virNetworkObjIsActive(obj)))
- count++;
- virNetworkObjUnlock(obj);
- }
+ return data.got;
+}
- return count;
+struct virNetworkObjListPruneHelperData {
+ unsigned int flags;
+};
+
+static int
+virNetworkObjListPruneHelper(const void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ const void *opaque)
+{
+ const struct virNetworkObjListPruneHelperData *data = opaque;
+ virNetworkObjPtr obj = (virNetworkObjPtr) payload;
+ int want = 0;
+
+ virNetworkObjLock(obj);
+ want = virNetworkMatch(obj, data->flags);
+ virNetworkObjUnlock(obj);
+ return want;
}
/**
@@ -4428,21 +4534,7 @@ void
virNetworkObjListPrune(virNetworkObjListPtr nets,
unsigned int flags)
{
- size_t i = 0;
+ struct virNetworkObjListPruneHelperData data = {flags};
- while (i < nets->count) {
- virNetworkObjPtr obj = nets->objs[i];
-
- virNetworkObjLock(obj);
-
- if (virNetworkMatch(obj, flags)) {
- virNetworkObjUnlock(obj);
- virNetworkObjFree(obj);
-
- VIR_DELETE_ELEMENT(nets->objs, i, nets->count);
- } else {
- virNetworkObjUnlock(obj);
- i++;
- }
- }
+ virHashRemoveSet(nets->objs, virNetworkObjListPruneHelper, &data);
}
--
2.0.5
8:17 a.m.
New subject: [libvirt] [PATCH v3 01/15] virNetworkObjListPtr: Turn list into a hash table
On Tue, Mar 10, 2015 at 17:45:07 +0100, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/network_conf.c | 382 ++++++++++++++++++++++++++++++------------------
1 file changed, 237 insertions(+), 145 deletions(-)
...
int virNetworkBridgeInUse(virNetworkObjListPtr nets,
const char *bridge,
const char *skipname)
{
- size_t i;
- unsigned int ret = 0;
+ virNetworkObjPtr obj;
+ struct virNetworkBridgeInUseHelperData data = {bridge, skipname};
- for (i = 0; i < nets->count; i++) {
- virNetworkObjLock(nets->objs[i]);
- if (nets->objs[i]->def->bridge &&
- STREQ(nets->objs[i]->def->bridge, bridge) &&
- !(skipname && STREQ(nets->objs[i]->def->name, skipname)))
- ret = 1;
- virNetworkObjUnlock(nets->objs[i]);
- }
+ obj = virHashSearch(nets->objs, virNetworkBridgeInUseHelper, &data);
- return ret;
+ return obj != NULL;
}
char *virNetworkAllocateBridge(virNetworkObjListPtr nets,
@@ -4271,6 +4299,52 @@ virNetworkMatch(virNetworkObjPtr netobj,
}
#undef MATCH
+struct virNetworkObjListData {
+ virConnectPtr conn;
+ virNetworkPtr *nets;
+ virNetworkObjListFilter filter;
+ unsigned int flags;
+ int nnets;
+ bool error;
+};
+
+static void
+virNetworkObjListPopulate(void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ void *opaque)
+{
+ struct virNetworkObjListData *data = opaque;
+ virNetworkObjPtr obj = payload;
+ virNetworkPtr net = NULL;
Similarly to the domain objects this will become problematic once you
drop the driver lock. I'm planing to fix the domain version soon, so
I'll need to update this one too as we don't have any prior art.
+
+ if (data->error)
+ return;
+
+ virNetworkObjLock(obj);
+
+ if (data->filter &&
+ !data->filter(data->conn, obj->def))
+ goto cleanup;
+
+ if (!virNetworkMatch(obj, data->flags))
+ goto cleanup;
+
+ if (!data->nets) {
+ data->nnets++;
+ goto cleanup;
+ }
+
+ if (!(net = virGetNetwork(data->conn, obj->def->name,
obj->def->uuid))) {
+ data->error = true;
+ goto cleanup;
+ }
+
+ data->nets[data->nnets++] = net;
+
+ cleanup:
+ virNetworkObjUnlock(obj);
+}
+
...
+static int
+virNetworkObjListPruneHelper(const void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ const void *opaque)
+{
+ const struct virNetworkObjListPruneHelperData *data = opaque;
+ virNetworkObjPtr obj = (virNetworkObjPtr) payload;
+ int want = 0;
+
+ virNetworkObjLock(obj);
+ want = virNetworkMatch(obj, data->flags);
+ virNetworkObjUnlock(obj);
+ return want;
}
/**
@@ -4428,21 +4534,7 @@ void
virNetworkObjListPrune(virNetworkObjListPtr nets,
unsigned int flags)
{
- size_t i = 0;
+ struct virNetworkObjListPruneHelperData data = {flags};
- while (i < nets->count) {
- virNetworkObjPtr obj = nets->objs[i];
-
- virNetworkObjLock(obj);
-
- if (virNetworkMatch(obj, flags)) {
- virNetworkObjUnlock(obj);
- virNetworkObjFree(obj);
-
- VIR_DELETE_ELEMENT(nets->objs, i, nets->count);
- } else {
- virNetworkObjUnlock(obj);
- i++;
- }
- }
+ virHashRemoveSet(nets->objs, virNetworkObjListPruneHelper, &data);
}
Now that I've persusaded you to add a function like this you converted
the code to the hash table that supports deletion. Oh well :)
ACK
Peter
11:45 a.m.
New subject: [libvirt] [PATCH v3 02/15] network_conf: Make virNetworkObj actually virObject
So far it's just a structure which happens to have 'Obj' in its
name, but otherwise it not related to virObject at all. No
reference counting, not virObjectLock(), nothing.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
cfg.mk | 2 -
src/conf/network_conf.c | 110 ++++++++++++++++++++------------------
src/conf/network_conf.h | 8 ++-
src/libvirt_private.syms | 4 +-
src/network/bridge_driver.c | 56 +++++++++----------
src/parallels/parallels_network.c | 16 +++---
src/test/test_driver.c | 32 +++++------
tests/networkxml2conftest.c | 4 +-
tests/objectlocking.ml | 2 -
9 files changed, 116 insertions(+), 118 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 07a794a..6885f9e 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -160,7 +160,6 @@ useless_free_options = \
--name=virNWFilterRuleDefFree \
--name=virNWFilterRuleInstFree \
--name=virNetworkDefFree \
- --name=virNetworkObjFree \
--name=virNodeDeviceDefFree \
--name=virNodeDeviceObjFree \
--name=virObjectUnref \
@@ -249,7 +248,6 @@ useless_free_options = \
# y virNetworkDefFree
# n virNetworkFree (returns int)
# n virNetworkFreeName (returns int)
-# y virNetworkObjFree
# n virNodeDevCapsDefFree FIXME
# y virNodeDeviceDefFree
# n virNodeDeviceFree (returns int)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index ae29c47..609990c 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -79,11 +79,19 @@ VIR_ENUM_IMPL(virNetworkForwardDriverName,
VIR_ENUM_IMPL(virNetworkTaint, VIR_NETWORK_TAINT_LAST,
"hook-script");
+static virClassPtr virNetworkObjClass;
static virClassPtr virNetworkObjListClass;
+static void virNetworkObjDispose(void *obj);
static void virNetworkObjListDispose(void *obj);
static int virNetworkObjOnceInit(void)
{
+ if (!(virNetworkObjClass = virClassNew(virClassForObjectLockable(),
+ "virNetworkObj",
+ sizeof(virNetworkObj),
+ virNetworkObjDispose)))
+ return -1;
+
if (!(virNetworkObjListClass = virClassNew(virClassForObject(),
"virNetworkObjList",
sizeof(virNetworkObjList),
@@ -99,7 +107,33 @@ static void
virNetworkObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED)
{
virNetworkObjPtr obj = payload;
- virNetworkObjFree(obj);
+ virObjectUnref(obj);
+}
+
+virNetworkObjPtr
+virNetworkObjNew(void)
+{
+ virNetworkObjPtr net;
+
+ if (virNetworkObjInitialize() < 0)
+ return NULL;
+
+ if (!(net = virObjectLockableNew(virNetworkObjClass)))
+ return NULL;
+
+ if (!(net->class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE)))
+ goto error;
+
+ /* The first three class IDs are already taken */
+ ignore_value(virBitmapSetBit(net->class_id, 0));
+ ignore_value(virBitmapSetBit(net->class_id, 1));
+ ignore_value(virBitmapSetBit(net->class_id, 2));
+
+ return net;
+
+ error:
+ virObjectUnref(net);
+ return NULL;
}
virNetworkObjListPtr virNetworkObjListNew(void)
@@ -130,7 +164,7 @@ virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
ret = virHashLookup(nets->objs, uuidstr);
if (ret)
- virNetworkObjLock(ret);
+ virObjectLock(ret);
return ret;
}
@@ -142,10 +176,10 @@ virNetworkObjSearchName(const void *payload,
virNetworkObjPtr net = (virNetworkObjPtr) payload;
int want = 0;
- virNetworkObjLock(net);
+ virObjectLock(net);
if (STREQ(net->def->name, (const char *)data))
want = 1;
- virNetworkObjUnlock(net);
+ virObjectUnlock(net);
return want;
}
@@ -156,7 +190,7 @@ virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
ret = virHashSearch(nets->objs, virNetworkObjSearchName, name);
if (ret)
- virNetworkObjLock(ret);
+ virObjectLock(ret);
return ret;
}
@@ -318,18 +352,14 @@ virNetworkDefFree(virNetworkDefPtr def)
VIR_FREE(def);
}
-void virNetworkObjFree(virNetworkObjPtr net)
+static void
+virNetworkObjDispose(void *obj)
{
- if (!net)
- return;
+ virNetworkObjPtr net = obj;
virNetworkDefFree(net->def);
virNetworkDefFree(net->newDef);
virBitmapFree(net->class_id);
-
- virMutexDestroy(&net->lock);
-
- VIR_FREE(net);
}
static void
@@ -427,35 +457,21 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
return network;
}
- if (VIR_ALLOC(network) < 0)
+ if (!(network = virNetworkObjNew()))
return NULL;
- if (virMutexInit(&network->lock) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("cannot initialize mutex"));
- VIR_FREE(network);
- return NULL;
- }
- virNetworkObjLock(network);
-
- if (!(network->class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE)))
- goto error;
+ virObjectLock(network);
virUUIDFormat(def->uuid, uuidstr);
if (virHashAddEntry(nets->objs, uuidstr, network) < 0)
goto error;
- /* The first three class IDs are already taken */
- ignore_value(virBitmapSetBit(network->class_id, 0));
- ignore_value(virBitmapSetBit(network->class_id, 1));
- ignore_value(virBitmapSetBit(network->class_id, 2));
-
network->def = def;
network->persistent = !live;
return network;
error:
- virNetworkObjUnlock(network);
- virNetworkObjFree(network);
+ virObjectUnlock(network);
+ virObjectUnref(network);
return NULL;
}
@@ -624,7 +640,7 @@ void virNetworkRemoveInactive(virNetworkObjListPtr nets,
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(net->def->uuid, uuidstr);
- virNetworkObjUnlock(net);
+ virObjectUnlock(net);
virHashRemoveEntry(nets->objs, uuidstr);
}
@@ -3026,7 +3042,7 @@ virNetworkLoadAllState(virNetworkObjListPtr nets,
continue;
if ((net = virNetworkLoadState(nets, stateDir, entry->d_name)))
- virNetworkObjUnlock(net);
+ virObjectUnlock(net);
}
closedir(dir);
@@ -3067,7 +3083,7 @@ int virNetworkLoadAllConfigs(virNetworkObjListPtr nets,
autostartDir,
entry->d_name);
if (net)
- virNetworkObjUnlock(net);
+ virObjectUnlock(net);
}
closedir(dir);
@@ -3128,12 +3144,12 @@ virNetworkBridgeInUseHelper(const void *payload,
virNetworkObjPtr net = (virNetworkObjPtr) payload;
const struct virNetworkBridgeInUseHelperData *data = opaque;
- virNetworkObjLock(net);
+ virObjectLock(net);
if (net->def->bridge &&
STREQ(net->def->bridge, data->bridge) &&
!(data->skipname && STREQ(net->def->name, data->skipname)))
ret = 1;
- virNetworkObjUnlock(net);
+ virObjectUnlock(net);
return ret;
}
@@ -4251,21 +4267,11 @@ virNetworkObjIsDuplicate(virNetworkObjListPtr nets,
cleanup:
if (net)
- virNetworkObjUnlock(net);
+ virObjectUnlock(net);
return ret;
}
-void virNetworkObjLock(virNetworkObjPtr obj)
-{
- virMutexLock(&obj->lock);
-}
-
-void virNetworkObjUnlock(virNetworkObjPtr obj)
-{
- virMutexUnlock(&obj->lock);
-}
-
#define MATCH(FLAG) (flags & (FLAG))
static bool
virNetworkMatch(virNetworkObjPtr netobj,
@@ -4320,7 +4326,7 @@ virNetworkObjListPopulate(void *payload,
if (data->error)
return;
- virNetworkObjLock(obj);
+ virObjectLock(obj);
if (data->filter &&
!data->filter(data->conn, obj->def))
@@ -4342,7 +4348,7 @@ virNetworkObjListPopulate(void *payload,
data->nets[data->nnets++] = net;
cleanup:
- virNetworkObjUnlock(obj);
+ virObjectUnlock(obj);
}
int
@@ -4442,7 +4448,7 @@ virNetworkObjListGetHelper(void *payload,
data->got == data->nnames)
return;
- virNetworkObjLock(obj);
+ virObjectLock(obj);
if (data->filter &&
!data->filter(data->conn, obj->def))
@@ -4459,7 +4465,7 @@ virNetworkObjListGetHelper(void *payload,
}
cleanup:
- virNetworkObjUnlock(obj);
+ virObjectUnlock(obj);
}
int
@@ -4516,9 +4522,9 @@ virNetworkObjListPruneHelper(const void *payload,
virNetworkObjPtr obj = (virNetworkObjPtr) payload;
int want = 0;
- virNetworkObjLock(obj);
+ virObjectLock(obj);
want = virNetworkMatch(obj, data->flags);
- virNetworkObjUnlock(obj);
+ virObjectUnlock(obj);
return want;
}
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 3575659..1423676 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -258,7 +258,7 @@ struct _virNetworkDef {
typedef struct _virNetworkObj virNetworkObj;
typedef virNetworkObj *virNetworkObjPtr;
struct _virNetworkObj {
- virMutex lock;
+ virObjectLockable parent;
pid_t dnsmasqPid;
pid_t radvdPid;
@@ -275,6 +275,8 @@ struct _virNetworkObj {
unsigned int taint;
};
+virNetworkObjPtr virNetworkObjNew(void);
+
typedef struct _virNetworkObjList virNetworkObjList;
typedef virNetworkObjList *virNetworkObjListPtr;
@@ -305,7 +307,6 @@ bool virNetworkObjTaint(virNetworkObjPtr obj,
virNetworkTaintFlags taint);
void virNetworkDefFree(virNetworkDefPtr def);
-void virNetworkObjFree(virNetworkObjPtr net);
typedef bool (*virNetworkObjListFilter)(virConnectPtr conn,
virNetworkDefPtr def);
@@ -412,9 +413,6 @@ int virNetworkObjIsDuplicate(virNetworkObjListPtr nets,
virNetworkDefPtr def,
bool check_active);
-void virNetworkObjLock(virNetworkObjPtr obj);
-void virNetworkObjUnlock(virNetworkObjPtr obj);
-
VIR_ENUM_DECL(virNetworkForward)
# define VIR_CONNECT_LIST_NETWORKS_FILTERS_ACTIVE \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e0d5459..5e78cf7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -565,7 +565,6 @@ virNetworkLoadAllState;
virNetworkObjAssignDef;
virNetworkObjFindByName;
virNetworkObjFindByUUID;
-virNetworkObjFree;
virNetworkObjGetPersistentDef;
virNetworkObjIsDuplicate;
virNetworkObjListExport;
@@ -574,11 +573,10 @@ virNetworkObjListGetNames;
virNetworkObjListNew;
virNetworkObjListNumOfNetworks;
virNetworkObjListPrune;
-virNetworkObjLock;
+virNetworkObjNew;
virNetworkObjReplacePersistentDef;
virNetworkObjSetDefTransient;
virNetworkObjTaint;
-virNetworkObjUnlock;
virNetworkObjUnsetDefTransient;
virNetworkObjUpdate;
virNetworkRemoveInactive;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 5752acb..70467f6 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -348,9 +348,9 @@ networkUpdateState(virNetworkObjPtr obj,
{
int ret = -1;
- virNetworkObjLock(obj);
+ virObjectLock(obj);
if (!virNetworkObjIsActive(obj)) {
- virNetworkObjUnlock(obj);
+ virObjectUnlock(obj);
return 0;
}
@@ -403,7 +403,7 @@ networkUpdateState(virNetworkObjPtr obj,
ret = 0;
cleanup:
- virNetworkObjUnlock(obj);
+ virObjectUnlock(obj);
return ret;
}
@@ -413,7 +413,7 @@ networkAutostartConfig(virNetworkObjPtr net,
{
int ret = -1;
- virNetworkObjLock(net);
+ virObjectLock(net);
if (net->autostart &&
!virNetworkObjIsActive(net) &&
networkStartNetwork(net) < 0)
@@ -421,7 +421,7 @@ networkAutostartConfig(virNetworkObjPtr net,
ret = 0;
cleanup:
- virNetworkObjUnlock(net);
+ virObjectUnlock(net);
return ret;
}
@@ -1745,7 +1745,7 @@ networkRefreshDaemonsHelper(virNetworkObjPtr net,
void *opaque ATTRIBUTE_UNUSED)
{
- virNetworkObjLock(net);
+ virObjectLock(net);
if (virNetworkObjIsActive(net) &&
((net->def->forward.type == VIR_NETWORK_FORWARD_NONE) ||
(net->def->forward.type == VIR_NETWORK_FORWARD_NAT) ||
@@ -1759,7 +1759,7 @@ networkRefreshDaemonsHelper(virNetworkObjPtr net,
networkRefreshDhcpDaemon(net);
networkRefreshRadvd(net);
}
- virNetworkObjUnlock(net);
+ virObjectUnlock(net);
return 0;
}
@@ -1780,7 +1780,7 @@ networkReloadFirewallRulesHelper(virNetworkObjPtr net,
void *opaque ATTRIBUTE_UNUSED)
{
- virNetworkObjLock(net);
+ virObjectLock(net);
if (virNetworkObjIsActive(net) &&
((net->def->forward.type == VIR_NETWORK_FORWARD_NONE) ||
(net->def->forward.type == VIR_NETWORK_FORWARD_NAT) ||
@@ -1793,7 +1793,7 @@ networkReloadFirewallRulesHelper(virNetworkObjPtr net,
/* failed to add but already logged */
}
}
- virNetworkObjUnlock(net);
+ virObjectUnlock(net);
return 0;
}
@@ -2497,7 +2497,7 @@ static virNetworkPtr networkLookupByUUID(virConnectPtr conn,
cleanup:
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
return ret;
}
@@ -2523,7 +2523,7 @@ static virNetworkPtr networkLookupByName(virConnectPtr conn,
cleanup:
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
return ret;
}
@@ -2671,7 +2671,7 @@ static int networkIsActive(virNetworkPtr net)
cleanup:
if (obj)
- virNetworkObjUnlock(obj);
+ virObjectUnlock(obj);
return ret;
}
@@ -2690,7 +2690,7 @@ static int networkIsPersistent(virNetworkPtr net)
cleanup:
if (obj)
- virNetworkObjUnlock(obj);
+ virObjectUnlock(obj);
return ret;
}
@@ -2960,7 +2960,7 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn, const char
*xml)
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
networkDriverUnlock();
return ret;
}
@@ -3017,7 +3017,7 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn, const char
*xml)
if (freeDef)
virNetworkDefFree(def);
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
networkDriverUnlock();
return ret;
}
@@ -3078,7 +3078,7 @@ networkUndefine(virNetworkPtr net)
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
networkDriverUnlock();
return ret;
}
@@ -3250,7 +3250,7 @@ networkUpdate(virNetworkPtr net,
ret = 0;
cleanup:
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
networkDriverUnlock();
return ret;
}
@@ -3285,7 +3285,7 @@ static int networkCreate(virNetworkPtr net)
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
networkDriverUnlock();
return ret;
}
@@ -3336,7 +3336,7 @@ static int networkDestroy(virNetworkPtr net)
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
networkDriverUnlock();
return ret;
}
@@ -3365,7 +3365,7 @@ static char *networkGetXMLDesc(virNetworkPtr net,
cleanup:
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
return ret;
}
@@ -3390,7 +3390,7 @@ static char *networkGetBridgeName(virNetworkPtr net) {
cleanup:
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
return bridge;
}
@@ -3411,7 +3411,7 @@ static int networkGetAutostart(virNetworkPtr net,
cleanup:
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
return ret;
}
@@ -3479,7 +3479,7 @@ static int networkSetAutostart(virNetworkPtr net,
VIR_FREE(configFile);
VIR_FREE(autostartLink);
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
networkDriverUnlock();
return ret;
}
@@ -3652,7 +3652,7 @@ networkGetDHCPLeases(virNetworkPtr network,
virJSONValueFree(leases_array);
if (obj)
- virNetworkObjUnlock(obj);
+ virObjectUnlock(obj);
return rv;
@@ -4125,7 +4125,7 @@ networkAllocateActualDevice(virDomainDefPtr dom,
cleanup:
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
return ret;
error:
@@ -4328,7 +4328,7 @@ networkNotifyActualDevice(virDomainDefPtr dom,
ret = 0;
cleanup:
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
return ret;
error:
@@ -4478,7 +4478,7 @@ networkReleaseActualDevice(virDomainDefPtr dom,
ret = 0;
cleanup:
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
virDomainActualNetDefFree(iface->data.network.actual);
iface->data.network.actual = NULL;
@@ -4582,7 +4582,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr)
ret = 0;
cleanup:
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
return ret;
}
diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c
index 62ed268..86038bf 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -230,7 +230,7 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj)
goto cleanup;
net->active = 1;
net->autostart = 1;
- virNetworkObjUnlock(net);
+ virObjectUnlock(net);
return net;
cleanup:
@@ -265,7 +265,7 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
}
net->active = 1;
net->autostart = 1;
- virNetworkObjUnlock(net);
+ virObjectUnlock(net);
return net;
@@ -445,7 +445,7 @@ static virNetworkPtr parallelsNetworkLookupByUUID(virConnectPtr conn,
cleanup:
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
return ret;
}
@@ -469,7 +469,7 @@ static virNetworkPtr parallelsNetworkLookupByName(virConnectPtr conn,
cleanup:
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
return ret;
}
@@ -496,7 +496,7 @@ static char *parallelsNetworkGetXMLDesc(virNetworkPtr net,
cleanup:
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
return ret;
}
@@ -517,7 +517,7 @@ static int parallelsNetworkIsActive(virNetworkPtr net)
cleanup:
if (obj)
- virNetworkObjUnlock(obj);
+ virObjectUnlock(obj);
return ret;
}
@@ -538,7 +538,7 @@ static int parallelsNetworkIsPersistent(virNetworkPtr net)
cleanup:
if (obj)
- virNetworkObjUnlock(obj);
+ virObjectUnlock(obj);
return ret;
}
@@ -563,7 +563,7 @@ static int parallelsNetworkGetAutostart(virNetworkPtr net,
cleanup:
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
return ret;
}
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 693b930..20c77de 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -787,7 +787,7 @@ testOpenDefault(virConnectPtr conn)
goto error;
}
netobj->active = 1;
- virNetworkObjUnlock(netobj);
+ virObjectUnlock(netobj);
if (!(interfacedef = virInterfaceDefParseString(defaultInterfaceXML)))
goto error;
@@ -1155,7 +1155,7 @@ testParseNetworks(testConnPtr privconn,
}
obj->active = 1;
- virNetworkObjUnlock(obj);
+ virObjectUnlock(obj);
}
ret = 0;
@@ -3508,7 +3508,7 @@ static virNetworkPtr testNetworkLookupByUUID(virConnectPtr conn,
cleanup:
if (net)
- virNetworkObjUnlock(net);
+ virObjectUnlock(net);
return ret;
}
@@ -3532,7 +3532,7 @@ static virNetworkPtr testNetworkLookupByName(virConnectPtr conn,
cleanup:
if (net)
- virNetworkObjUnlock(net);
+ virObjectUnlock(net);
return ret;
}
@@ -3621,7 +3621,7 @@ static int testNetworkIsActive(virNetworkPtr net)
cleanup:
if (obj)
- virNetworkObjUnlock(obj);
+ virObjectUnlock(obj);
return ret;
}
@@ -3642,7 +3642,7 @@ static int testNetworkIsPersistent(virNetworkPtr net)
cleanup:
if (obj)
- virNetworkObjUnlock(obj);
+ virObjectUnlock(obj);
return ret;
}
@@ -3675,7 +3675,7 @@ static virNetworkPtr testNetworkCreateXML(virConnectPtr conn, const
char *xml)
if (event)
testObjectEventQueue(privconn, event);
if (net)
- virNetworkObjUnlock(net);
+ virObjectUnlock(net);
testDriverUnlock(privconn);
return ret;
}
@@ -3708,7 +3708,7 @@ virNetworkPtr testNetworkDefineXML(virConnectPtr conn, const char
*xml)
if (event)
testObjectEventQueue(privconn, event);
if (net)
- virNetworkObjUnlock(net);
+ virObjectUnlock(net);
testDriverUnlock(privconn);
return ret;
}
@@ -3746,7 +3746,7 @@ static int testNetworkUndefine(virNetworkPtr network)
if (event)
testObjectEventQueue(privconn, event);
if (privnet)
- virNetworkObjUnlock(privnet);
+ virObjectUnlock(privnet);
testDriverUnlock(privconn);
return ret;
}
@@ -3796,7 +3796,7 @@ testNetworkUpdate(virNetworkPtr net,
ret = 0;
cleanup:
if (network)
- virNetworkObjUnlock(network);
+ virObjectUnlock(network);
testDriverUnlock(privconn);
return ret;
}
@@ -3833,7 +3833,7 @@ static int testNetworkCreate(virNetworkPtr network)
if (event)
testObjectEventQueue(privconn, event);
if (privnet)
- virNetworkObjUnlock(privnet);
+ virObjectUnlock(privnet);
return ret;
}
@@ -3866,7 +3866,7 @@ static int testNetworkDestroy(virNetworkPtr network)
if (event)
testObjectEventQueue(privconn, event);
if (privnet)
- virNetworkObjUnlock(privnet);
+ virObjectUnlock(privnet);
testDriverUnlock(privconn);
return ret;
}
@@ -3893,7 +3893,7 @@ static char *testNetworkGetXMLDesc(virNetworkPtr network,
cleanup:
if (privnet)
- virNetworkObjUnlock(privnet);
+ virObjectUnlock(privnet);
return ret;
}
@@ -3922,7 +3922,7 @@ static char *testNetworkGetBridgeName(virNetworkPtr network) {
cleanup:
if (privnet)
- virNetworkObjUnlock(privnet);
+ virObjectUnlock(privnet);
return bridge;
}
@@ -3947,7 +3947,7 @@ static int testNetworkGetAutostart(virNetworkPtr network,
cleanup:
if (privnet)
- virNetworkObjUnlock(privnet);
+ virObjectUnlock(privnet);
return ret;
}
@@ -3972,7 +3972,7 @@ static int testNetworkSetAutostart(virNetworkPtr network,
cleanup:
if (privnet)
- virNetworkObjUnlock(privnet);
+ virObjectUnlock(privnet);
return ret;
}
diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c
index 280db30..6df161f 100644
--- a/tests/networkxml2conftest.c
+++ b/tests/networkxml2conftest.c
@@ -40,7 +40,7 @@ testCompareXMLToConfFiles(const char *inxml, const char *outconf,
dnsmasqCapsPtr
if (!(dev = virNetworkDefParseString(inXmlData)))
goto fail;
- if (VIR_ALLOC(obj) < 0)
+ if (!(obj = virNetworkObjNew()))
goto fail;
obj->def = dev;
@@ -66,7 +66,7 @@ testCompareXMLToConfFiles(const char *inxml, const char *outconf,
dnsmasqCapsPtr
VIR_FREE(actual);
VIR_FREE(pidfile);
virCommandFree(cmd);
- virNetworkObjFree(obj);
+ virObjectUnref(obj);
dnsmasqContextFree(dctx);
return ret;
}
diff --git a/tests/objectlocking.ml b/tests/objectlocking.ml
index 7826148..fd83d6d 100644
--- a/tests/objectlocking.ml
+++ b/tests/objectlocking.ml
@@ -84,7 +84,6 @@ let lockedObjMethods = [
*)
let objectLockMethods = [
"virDomainObjLock";
- "virNetworkObjLock";
"virStoragePoolObjLock";
"virNodeDevObjLock"
]
@@ -99,7 +98,6 @@ let objectLockMethods = [
*)
let objectUnlockMethods = [
"virDomainObjUnlock";
- "virNetworkObjUnlock";
"virStoragePoolObjUnlock";
"virNodeDevObjUnlock"
]
--
2.0.5
3:51 a.m.
New subject: [libvirt] [PATCH v3 02/15] network_conf: Make virNetworkObj actually virObject
On Tue, Mar 10, 2015 at 17:45:08 +0100, Michal Privoznik wrote:
So far it's just a structure which happens to have 'Obj'
in its
name, but otherwise it not related to virObject at all. No
reference counting, not virObjectLock(), nothing.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
cfg.mk | 2 -
src/conf/network_conf.c | 110 ++++++++++++++++++++------------------
src/conf/network_conf.h | 8 ++-
src/libvirt_private.syms | 4 +-
src/network/bridge_driver.c | 56 +++++++++----------
src/parallels/parallels_network.c | 16 +++---
src/test/test_driver.c | 32 +++++------
tests/networkxml2conftest.c | 4 +-
tests/objectlocking.ml | 2 -
9 files changed, 116 insertions(+), 118 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 07a794a..6885f9e 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -160,7 +160,6 @@ useless_free_options = \
--name=virNWFilterRuleDefFree \
--name=virNWFilterRuleInstFree \
--name=virNetworkDefFree \
- --name=virNetworkObjFree \
--name=virNodeDeviceDefFree \
--name=virNodeDeviceObjFree \
--name=virObjectUnref \
@@ -249,7 +248,6 @@ useless_free_options = \
# y virNetworkDefFree
# n virNetworkFree (returns int)
# n virNetworkFreeName (returns int)
-# y virNetworkObjFree
# n virNodeDevCapsDefFree FIXME
# y virNodeDeviceDefFree
# n virNodeDeviceFree (returns int)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index ae29c47..609990c 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -79,11 +79,19 @@ VIR_ENUM_IMPL(virNetworkForwardDriverName,
VIR_ENUM_IMPL(virNetworkTaint, VIR_NETWORK_TAINT_LAST,
"hook-script");
+static virClassPtr virNetworkObjClass;
static virClassPtr virNetworkObjListClass;
+static void virNetworkObjDispose(void *obj);
static void virNetworkObjListDispose(void *obj);
static int virNetworkObjOnceInit(void)
{
+ if (!(virNetworkObjClass = virClassNew(virClassForObjectLockable(),
+ "virNetworkObj",
+ sizeof(virNetworkObj),
+ virNetworkObjDispose)))
+ return -1;
+
if (!(virNetworkObjListClass = virClassNew(virClassForObject(),
"virNetworkObjList",
sizeof(virNetworkObjList),
@@ -99,7 +107,33 @@ static void
virNetworkObjListDataFree(void *payload, const void *name ATTRIBUTE_UNUSED)
This function can now be replaced with virObjectFreeHashData.
{
virNetworkObjPtr obj = payload;
- virNetworkObjFree(obj);
+ virObjectUnref(obj);
+}
Since I've already reviewed this previously and the conversion to
virHash is ok here.
ACK if you use the existing free helper.
Peter
11:45 a.m.
New subject: [libvirt] [PATCH v3 03/15] network_conf: Introduce virNetworkObjEndAPI
This is practically copy of qemuDomObjEndAPI. The reason why is
it so widely available is to avoid code duplication, since the
function is going to be called from our bridge driver, test
driver and parallels driver too.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/network_conf.c | 20 ++++++++++++++------
src/conf/network_conf.h | 1 +
src/libvirt_private.syms | 1 +
3 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 609990c..f677e3c 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -136,6 +136,16 @@ virNetworkObjNew(void)
return NULL;
}
+void
+virNetworkObjEndAPI(virNetworkObjPtr *net)
+{
+ if (!*net)
+ return;
+
+ virObjectUnlock(*net);
+ *net = NULL;
+}
+
virNetworkObjListPtr virNetworkObjListNew(void)
{
virNetworkObjListPtr nets;
@@ -3041,8 +3051,8 @@ virNetworkLoadAllState(virNetworkObjListPtr nets,
if (!virFileStripSuffix(entry->d_name, ".xml"))
continue;
- if ((net = virNetworkLoadState(nets, stateDir, entry->d_name)))
- virObjectUnlock(net);
+ net = virNetworkLoadState(nets, stateDir, entry->d_name);
+ virNetworkObjEndAPI(&net);
}
closedir(dir);
@@ -3082,8 +3092,7 @@ int virNetworkLoadAllConfigs(virNetworkObjListPtr nets,
configDir,
autostartDir,
entry->d_name);
- if (net)
- virObjectUnlock(net);
+ virNetworkObjEndAPI(&net);
}
closedir(dir);
@@ -4266,8 +4275,7 @@ virNetworkObjIsDuplicate(virNetworkObjListPtr nets,
}
cleanup:
- if (net)
- virObjectUnlock(net);
+ virNetworkObjEndAPI(&net);
return ret;
}
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 1423676..e0ed714 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -276,6 +276,7 @@ struct _virNetworkObj {
};
virNetworkObjPtr virNetworkObjNew(void);
+void virNetworkObjEndAPI(virNetworkObjPtr *net);
typedef struct _virNetworkObjList virNetworkObjList;
typedef virNetworkObjList *virNetworkObjListPtr;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5e78cf7..5fbe094 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -563,6 +563,7 @@ virNetworkIpDefPrefix;
virNetworkLoadAllConfigs;
virNetworkLoadAllState;
virNetworkObjAssignDef;
+virNetworkObjEndAPI;
virNetworkObjFindByName;
virNetworkObjFindByUUID;
virNetworkObjGetPersistentDef;
--
2.0.5
3:52 a.m.
New subject: [libvirt] [PATCH v3 03/15] network_conf: Introduce virNetworkObjEndAPI
On Tue, Mar 10, 2015 at 17:45:09 +0100, Michal Privoznik wrote:
This is practically copy of qemuDomObjEndAPI. The reason why is
it so widely available is to avoid code duplication, since the
function is going to be called from our bridge driver, test
driver and parallels driver too.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/network_conf.c | 20 ++++++++++++++------
src/conf/network_conf.h | 1 +
src/libvirt_private.syms | 1 +
3 files changed, 16 insertions(+), 6 deletions(-)
ACK from previous version stands, this had no changes.
Peter
11:45 a.m.
New subject: [libvirt] [PATCH v3 04/15] bridge_driver: Use virNetworkObjEndAPI
So far, this is pure code replacement. But once we introduce
reference counting to virNetworkObj this will be more handy as
there'll be only one function to change: virNetworkObjEndAPI().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/network/bridge_driver.c | 57 +++++++++++++++------------------------------
1 file changed, 19 insertions(+), 38 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 70467f6..97fda3f 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2496,8 +2496,7 @@ static virNetworkPtr networkLookupByUUID(virConnectPtr conn,
ret = virGetNetwork(conn, network->def->name, network->def->uuid);
cleanup:
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
return ret;
}
@@ -2522,8 +2521,7 @@ static virNetworkPtr networkLookupByName(virConnectPtr conn,
ret = virGetNetwork(conn, network->def->name, network->def->uuid);
cleanup:
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
return ret;
}
@@ -2670,8 +2668,7 @@ static int networkIsActive(virNetworkPtr net)
ret = virNetworkObjIsActive(obj);
cleanup:
- if (obj)
- virObjectUnlock(obj);
+ virNetworkObjEndAPI(&obj);
return ret;
}
@@ -2689,8 +2686,7 @@ static int networkIsPersistent(virNetworkPtr net)
ret = obj->persistent;
cleanup:
- if (obj)
- virObjectUnlock(obj);
+ virNetworkObjEndAPI(&obj);
return ret;
}
@@ -2959,8 +2955,7 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn, const char
*xml)
virNetworkDefFree(def);
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
networkDriverUnlock();
return ret;
}
@@ -3016,8 +3011,7 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn, const char
*xml)
virObjectEventStateQueue(driver->networkEventState, event);
if (freeDef)
virNetworkDefFree(def);
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
networkDriverUnlock();
return ret;
}
@@ -3077,8 +3071,7 @@ networkUndefine(virNetworkPtr net)
cleanup:
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
networkDriverUnlock();
return ret;
}
@@ -3249,8 +3242,7 @@ networkUpdate(virNetworkPtr net,
}
ret = 0;
cleanup:
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
networkDriverUnlock();
return ret;
}
@@ -3284,8 +3276,7 @@ static int networkCreate(virNetworkPtr net)
cleanup:
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
networkDriverUnlock();
return ret;
}
@@ -3335,8 +3326,7 @@ static int networkDestroy(virNetworkPtr net)
cleanup:
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
networkDriverUnlock();
return ret;
}
@@ -3364,8 +3354,7 @@ static char *networkGetXMLDesc(virNetworkPtr net,
ret = virNetworkDefFormat(def, flags);
cleanup:
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
return ret;
}
@@ -3389,8 +3378,7 @@ static char *networkGetBridgeName(virNetworkPtr net) {
ignore_value(VIR_STRDUP(bridge, network->def->bridge));
cleanup:
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
return bridge;
}
@@ -3410,8 +3398,7 @@ static int networkGetAutostart(virNetworkPtr net,
ret = 0;
cleanup:
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
return ret;
}
@@ -3478,8 +3465,7 @@ static int networkSetAutostart(virNetworkPtr net,
cleanup:
VIR_FREE(configFile);
VIR_FREE(autostartLink);
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
networkDriverUnlock();
return ret;
}
@@ -3651,8 +3637,7 @@ networkGetDHCPLeases(virNetworkPtr network,
VIR_FREE(custom_lease_file);
virJSONValueFree(leases_array);
- if (obj)
- virObjectUnlock(obj);
+ virNetworkObjEndAPI(&obj);
return rv;
@@ -4124,8 +4109,7 @@ networkAllocateActualDevice(virDomainDefPtr dom,
ret = 0;
cleanup:
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
return ret;
error:
@@ -4327,8 +4311,7 @@ networkNotifyActualDevice(virDomainDefPtr dom,
ret = 0;
cleanup:
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
return ret;
error:
@@ -4477,8 +4460,7 @@ networkReleaseActualDevice(virDomainDefPtr dom,
}
ret = 0;
cleanup:
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
if (iface->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
virDomainActualNetDefFree(iface->data.network.actual);
iface->data.network.actual = NULL;
@@ -4581,8 +4563,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr)
ret = 0;
cleanup:
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
return ret;
}
--
2.0.5
4 a.m.
New subject: [libvirt] [PATCH v3 04/15] bridge_driver: Use virNetworkObjEndAPI
On Tue, Mar 10, 2015 at 17:45:10 +0100, Michal Privoznik wrote:
So far, this is pure code replacement. But once we introduce
reference counting to virNetworkObj this will be more handy as
there'll be only one function to change: virNetworkObjEndAPI().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/network/bridge_driver.c | 57 +++++++++++++++------------------------------
1 file changed, 19 insertions(+), 38 deletions(-)
ACK, I don't see any change.
Peter
11:45 a.m.
New subject: [libvirt] [PATCH v3 05/15] test_driver: Use virNetworkObjEndAPI
So far, this is pure code replacement. But once we introduce
reference counting to virNetworkObj this will be more handy as
there'll be only one function to change: virNetworkObjEndAPI().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/test/test_driver.c | 42 ++++++++++++++----------------------------
1 file changed, 14 insertions(+), 28 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 20c77de..72f40ed 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3507,8 +3507,7 @@ static virNetworkPtr testNetworkLookupByUUID(virConnectPtr conn,
ret = virGetNetwork(conn, net->def->name, net->def->uuid);
cleanup:
- if (net)
- virObjectUnlock(net);
+ virNetworkObjEndAPI(&net);
return ret;
}
@@ -3531,8 +3530,7 @@ static virNetworkPtr testNetworkLookupByName(virConnectPtr conn,
ret = virGetNetwork(conn, net->def->name, net->def->uuid);
cleanup:
- if (net)
- virObjectUnlock(net);
+ virNetworkObjEndAPI(&net);
return ret;
}
@@ -3620,8 +3618,7 @@ static int testNetworkIsActive(virNetworkPtr net)
ret = virNetworkObjIsActive(obj);
cleanup:
- if (obj)
- virObjectUnlock(obj);
+ virNetworkObjEndAPI(&obj);
return ret;
}
@@ -3641,8 +3638,7 @@ static int testNetworkIsPersistent(virNetworkPtr net)
ret = obj->persistent;
cleanup:
- if (obj)
- virObjectUnlock(obj);
+ virNetworkObjEndAPI(&obj);
return ret;
}
@@ -3674,8 +3670,7 @@ static virNetworkPtr testNetworkCreateXML(virConnectPtr conn, const
char *xml)
virNetworkDefFree(def);
if (event)
testObjectEventQueue(privconn, event);
- if (net)
- virObjectUnlock(net);
+ virNetworkObjEndAPI(&net);
testDriverUnlock(privconn);
return ret;
}
@@ -3707,8 +3702,7 @@ virNetworkPtr testNetworkDefineXML(virConnectPtr conn, const char
*xml)
virNetworkDefFree(def);
if (event)
testObjectEventQueue(privconn, event);
- if (net)
- virObjectUnlock(net);
+ virNetworkObjEndAPI(&net);
testDriverUnlock(privconn);
return ret;
}
@@ -3745,8 +3739,7 @@ static int testNetworkUndefine(virNetworkPtr network)
cleanup:
if (event)
testObjectEventQueue(privconn, event);
- if (privnet)
- virObjectUnlock(privnet);
+ virNetworkObjEndAPI(&privnet);
testDriverUnlock(privconn);
return ret;
}
@@ -3795,8 +3788,7 @@ testNetworkUpdate(virNetworkPtr net,
ret = 0;
cleanup:
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
testDriverUnlock(privconn);
return ret;
}
@@ -3832,8 +3824,7 @@ static int testNetworkCreate(virNetworkPtr network)
cleanup:
if (event)
testObjectEventQueue(privconn, event);
- if (privnet)
- virObjectUnlock(privnet);
+ virNetworkObjEndAPI(&privnet);
return ret;
}
@@ -3865,8 +3856,7 @@ static int testNetworkDestroy(virNetworkPtr network)
cleanup:
if (event)
testObjectEventQueue(privconn, event);
- if (privnet)
- virObjectUnlock(privnet);
+ virNetworkObjEndAPI(&privnet);
testDriverUnlock(privconn);
return ret;
}
@@ -3892,8 +3882,7 @@ static char *testNetworkGetXMLDesc(virNetworkPtr network,
ret = virNetworkDefFormat(privnet->def, flags);
cleanup:
- if (privnet)
- virObjectUnlock(privnet);
+ virNetworkObjEndAPI(&privnet);
return ret;
}
@@ -3921,8 +3910,7 @@ static char *testNetworkGetBridgeName(virNetworkPtr network) {
ignore_value(VIR_STRDUP(bridge, privnet->def->bridge));
cleanup:
- if (privnet)
- virObjectUnlock(privnet);
+ virNetworkObjEndAPI(&privnet);
return bridge;
}
@@ -3946,8 +3934,7 @@ static int testNetworkGetAutostart(virNetworkPtr network,
ret = 0;
cleanup:
- if (privnet)
- virObjectUnlock(privnet);
+ virNetworkObjEndAPI(&privnet);
return ret;
}
@@ -3971,8 +3958,7 @@ static int testNetworkSetAutostart(virNetworkPtr network,
ret = 0;
cleanup:
- if (privnet)
- virObjectUnlock(privnet);
+ virNetworkObjEndAPI(&privnet);
return ret;
}
--
2.0.5
4:01 a.m.
New subject: [libvirt] [PATCH v3 05/15] test_driver: Use virNetworkObjEndAPI
On Tue, Mar 10, 2015 at 17:45:11 +0100, Michal Privoznik wrote:
So far, this is pure code replacement. But once we introduce
reference counting to virNetworkObj this will be more handy as
there'll be only one function to change: virNetworkObjEndAPI().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/test/test_driver.c | 42 ++++++++++++++----------------------------
1 file changed, 14 insertions(+), 28 deletions(-)
ACK, I don't see any change.
Peter
11:45 a.m.
New subject: [libvirt] [PATCH v3 06/15] parallels_network: Use virNetworkObjEndAPI
So far, this is pure code replacement. But once we introduce
reference counting to virNetworkObj this will be more handy as
there'll be only one function to change: virNetworkObjEndAPI().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/parallels/parallels_network.c | 29 ++++++++++++-----------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c
index 86038bf..1bcd2d3 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -230,7 +230,6 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj)
goto cleanup;
net->active = 1;
net->autostart = 1;
- virObjectUnlock(net);
return net;
cleanup:
@@ -241,7 +240,7 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj)
static virNetworkObjPtr
parallelsAddRoutedNetwork(parallelsConnPtr privconn)
{
- virNetworkObjPtr net;
+ virNetworkObjPtr net = NULL;
virNetworkDefPtr def;
if (VIR_ALLOC(def) < 0)
@@ -265,7 +264,6 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
}
net->active = 1;
net->autostart = 1;
- virObjectUnlock(net);
return net;
@@ -277,7 +275,7 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
static int parallelsLoadNetworks(parallelsConnPtr privconn)
{
virJSONValuePtr jobj, jobj2;
- virNetworkObjPtr net;
+ virNetworkObjPtr net = NULL;
int ret = -1;
int count;
size_t i;
@@ -305,16 +303,19 @@ static int parallelsLoadNetworks(parallelsConnPtr privconn)
net = parallelsLoadNetwork(privconn, jobj2);
if (!net)
goto cleanup;
+ else
+ virNetworkObjEndAPI(&net);
}
- if (!parallelsAddRoutedNetwork(privconn))
+ if (!(net = parallelsAddRoutedNetwork(privconn)))
goto cleanup;
ret = 0;
cleanup:
virJSONValueFree(jobj);
+ virNetworkObjEndAPI(&net);
return ret;
}
@@ -444,8 +445,7 @@ static virNetworkPtr parallelsNetworkLookupByUUID(virConnectPtr conn,
ret = virGetNetwork(conn, network->def->name, network->def->uuid);
cleanup:
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
return ret;
}
@@ -468,8 +468,7 @@ static virNetworkPtr parallelsNetworkLookupByName(virConnectPtr conn,
ret = virGetNetwork(conn, network->def->name, network->def->uuid);
cleanup:
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
return ret;
}
@@ -495,8 +494,7 @@ static char *parallelsNetworkGetXMLDesc(virNetworkPtr net,
ret = virNetworkDefFormat(network->def, flags);
cleanup:
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
return ret;
}
@@ -516,8 +514,7 @@ static int parallelsNetworkIsActive(virNetworkPtr net)
ret = virNetworkObjIsActive(obj);
cleanup:
- if (obj)
- virObjectUnlock(obj);
+ virNetworkObjEndAPI(&obj);
return ret;
}
@@ -537,8 +534,7 @@ static int parallelsNetworkIsPersistent(virNetworkPtr net)
ret = obj->persistent;
cleanup:
- if (obj)
- virObjectUnlock(obj);
+ virNetworkObjEndAPI(&obj);
return ret;
}
@@ -562,8 +558,7 @@ static int parallelsNetworkGetAutostart(virNetworkPtr net,
ret = 0;
cleanup:
- if (network)
- virObjectUnlock(network);
+ virNetworkObjEndAPI(&network);
return ret;
}
--
2.0.5
4:02 a.m.
New subject: [libvirt] [PATCH v3 06/15] parallels_network: Use virNetworkObjEndAPI
On Tue, Mar 10, 2015 at 17:45:12 +0100, Michal Privoznik wrote:
So far, this is pure code replacement. But once we introduce
reference counting to virNetworkObj this will be more handy as
there'll be only one function to change: virNetworkObjEndAPI().
Not entirely true ...
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/parallels/parallels_network.c | 29 ++++++++++++-----------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c
index 86038bf..1bcd2d3 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -230,7 +230,6 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr
jobj)
goto cleanup;
net->active = 1;
net->autostart = 1;
- virObjectUnlock(net);
... this ...
return net;
cleanup:
@@ -241,7 +240,7 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr
jobj)
static virNetworkObjPtr
parallelsAddRoutedNetwork(parallelsConnPtr privconn)
{
- virNetworkObjPtr net;
+ virNetworkObjPtr net = NULL;
virNetworkDefPtr def;
if (VIR_ALLOC(def) < 0)
@@ -265,7 +264,6 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
}
net->active = 1;
net->autostart = 1;
- virObjectUnlock(net);
return net;
@@ -277,7 +275,7 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
static int parallelsLoadNetworks(parallelsConnPtr privconn)
{
virJSONValuePtr jobj, jobj2;
- virNetworkObjPtr net;
+ virNetworkObjPtr net = NULL;
int ret = -1;
int count;
size_t i;
@@ -305,16 +303,19 @@ static int parallelsLoadNetworks(parallelsConnPtr privconn)
net = parallelsLoadNetwork(privconn, jobj2);
if (!net)
goto cleanup;
+ else
+ virNetworkObjEndAPI(&net);
... is a somewhat semantic change.
}
- if (!parallelsAddRoutedNetwork(privconn))
+ if (!(net = parallelsAddRoutedNetwork(privconn)))
goto cleanup;
ret = 0;
cleanup:
virJSONValueFree(jobj);
+ virNetworkObjEndAPI(&net);
return ret;
}
Anyways. ACK.
Peter
11:45 a.m.
New subject: [libvirt] [PATCH v3 07/15] virNetworkObjList: Derive from virObjectLockableClass
Later we can turn APIs to lock the object if needed instead of
relying on caller to mutually exclude itself (probably done by
locking a big lock anyway).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/network_conf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index f677e3c..399c372 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -52,7 +52,7 @@
#define CLASS_ID_BITMAP_SIZE (1<<16)
struct _virNetworkObjList {
- virObject parent;
+ virObjectLockable parent;
virHashTablePtr objs;
};
@@ -92,7 +92,7 @@ static int virNetworkObjOnceInit(void)
virNetworkObjDispose)))
return -1;
- if (!(virNetworkObjListClass = virClassNew(virClassForObject(),
+ if (!(virNetworkObjListClass = virClassNew(virClassForObjectLockable(),
"virNetworkObjList",
sizeof(virNetworkObjList),
virNetworkObjListDispose)))
@@ -153,7 +153,7 @@ virNetworkObjListPtr virNetworkObjListNew(void)
if (virNetworkObjInitialize() < 0)
return NULL;
- if (!(nets = virObjectNew(virNetworkObjListClass)))
+ if (!(nets = virObjectLockableNew(virNetworkObjListClass)))
return NULL;
if (!(nets->objs = virHashCreate(50, virNetworkObjListDataFree))) {
--
2.0.5
4:04 a.m.
New subject: [libvirt] [PATCH v3 07/15] virNetworkObjList: Derive from virObjectLockableClass
On Tue, Mar 10, 2015 at 17:45:13 +0100, Michal Privoznik wrote:
Later we can turn APIs to lock the object if needed instead of
relying on caller to mutually exclude itself (probably done by
locking a big lock anyway).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/network_conf.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
ACK
11:45 a.m.
New subject: [libvirt] [PATCH v3 08/15] network_conf: Introduce locked versions of lookup functions
This is going to be needed later, when some functions already
have the virNetworkObjList object already locked and need to
lookup a object to work on. As an example of such function is
virNetworkAssignDef(). The other use case might be in
virNetworkObjListForEach() callback.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/network_conf.c | 34 ++++++++++++++++++++++++++++++----
src/conf/network_conf.h | 4 ++++
src/libvirt_private.syms | 2 ++
3 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 399c372..be5cf0e 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -164,8 +164,9 @@ virNetworkObjListPtr virNetworkObjListNew(void)
return nets;
}
-virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
- const unsigned char *uuid)
+virNetworkObjPtr
+virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
+ const unsigned char *uuid)
{
virNetworkObjPtr ret = NULL;
char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -178,6 +179,18 @@ virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
return ret;
}
+virNetworkObjPtr
+virNetworkObjFindByUUID(virNetworkObjListPtr nets,
+ const unsigned char *uuid)
+{
+ virNetworkObjPtr ret;
+
+ virObjectLock(nets);
+ ret = virNetworkObjFindByUUIDLocked(nets, uuid);
+ virObjectUnlock(nets);
+ return ret;
+}
+
static int
virNetworkObjSearchName(const void *payload,
const void *name ATTRIBUTE_UNUSED,
@@ -193,8 +206,9 @@ virNetworkObjSearchName(const void *payload,
return want;
}
-virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
- const char *name)
+virNetworkObjPtr
+virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
+ const char *name)
{
virNetworkObjPtr ret = NULL;
@@ -204,6 +218,18 @@ virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
return ret;
}
+virNetworkObjPtr
+virNetworkObjFindByName(virNetworkObjListPtr nets,
+ const char *name)
+{
+ virNetworkObjPtr ret;
+
+ virObjectLock(nets);
+ ret = virNetworkObjFindByNameLocked(nets, name);
+ virObjectUnlock(nets);
+ return ret;
+}
+
bool
virNetworkObjTaint(virNetworkObjPtr obj,
virNetworkTaintFlags taint)
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index e0ed714..3e926f7 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -300,8 +300,12 @@ virNetworkObjIsActive(const virNetworkObj *net)
virNetworkObjListPtr virNetworkObjListNew(void);
+virNetworkObjPtr virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
+ const unsigned char *uuid);
virNetworkObjPtr virNetworkObjFindByUUID(virNetworkObjListPtr nets,
const unsigned char *uuid);
+virNetworkObjPtr virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
+ const char *name);
virNetworkObjPtr virNetworkObjFindByName(virNetworkObjListPtr nets,
const char *name);
bool virNetworkObjTaint(virNetworkObjPtr obj,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5fbe094..64808ce 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -565,7 +565,9 @@ virNetworkLoadAllState;
virNetworkObjAssignDef;
virNetworkObjEndAPI;
virNetworkObjFindByName;
+virNetworkObjFindByNameLocked;
virNetworkObjFindByUUID;
+virNetworkObjFindByUUIDLocked;
virNetworkObjGetPersistentDef;
virNetworkObjIsDuplicate;
virNetworkObjListExport;
--
2.0.5
4:05 a.m.
New subject: [libvirt] [PATCH v3 08/15] network_conf: Introduce locked versions of lookup functions
On Tue, Mar 10, 2015 at 17:45:14 +0100, Michal Privoznik wrote:
This is going to be needed later, when some functions already
have the virNetworkObjList object already locked and need to
lookup a object to work on. As an example of such function is
virNetworkAssignDef(). The other use case might be in
virNetworkObjListForEach() callback.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/network_conf.c | 34 ++++++++++++++++++++++++++++++----
src/conf/network_conf.h | 4 ++++
src/libvirt_private.syms | 2 ++
3 files changed, 36 insertions(+), 4 deletions(-)
ACK,
Peter
11:45 a.m.
New subject: [libvirt] [PATCH v3 09/15] virNetworkObjListPtr: Make APIs self-locking
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 | 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
8:28 a.m.
New subject: [libvirt] [PATCH v3 09/15] virNetworkObjListPtr: Make APIs self-locking
On Tue, Mar 10, 2015 at 17:45:15 +0100, Michal Privoznik wrote:
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 | 32 +++++++++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)
ACK,
Peter
11:45 a.m.
New subject: [libvirt] [PATCH v3 10/15] 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(a)redhat.com>
---
src/conf/network_conf.c | 15 ++++++++++-----
src/network/bridge_driver.c | 18 +++++-------------
src/test/test_driver.c | 10 ++++------
3 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 98342f9..a2aa4dc 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -143,6 +143,7 @@ virNetworkObjEndAPI(virNetworkObjPtr *net)
return;
virObjectUnlock(*net);
+ virObjectUnref(*net);
*net = NULL;
}
@@ -175,7 +176,7 @@ virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
ret = virHashLookup(nets->objs, uuidstr);
if (ret)
- virObjectLock(ret);
+ virObjectRef(ret);
return ret;
}
@@ -188,6 +189,8 @@ virNetworkObjFindByUUID(virNetworkObjListPtr nets,
virObjectLock(nets);
ret = virNetworkObjFindByUUIDLocked(nets, uuid);
virObjectUnlock(nets);
+ if (ret)
+ virObjectLock(ret);
return ret;
}
@@ -214,7 +217,7 @@ virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
ret = virHashSearch(nets->objs, virNetworkObjSearchName, name);
if (ret)
- virObjectLock(ret);
+ virObjectRef(ret);
return ret;
}
@@ -227,6 +230,8 @@ virNetworkObjFindByName(virNetworkObjListPtr nets,
virObjectLock(nets);
ret = virNetworkObjFindByNameLocked(nets, name);
virObjectUnlock(nets);
+ if (ret)
+ virObjectLock(ret);
return ret;
}
@@ -491,6 +496,7 @@ virNetworkAssignDef(virNetworkObjListPtr nets,
virObjectLock(nets);
if ((network = virNetworkObjFindByNameLocked(nets, def->name))) {
virObjectUnlock(nets);
+ virObjectLock(network);
virNetworkObjAssignDef(network, def, live);
return network;
}
@@ -507,13 +513,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;
}
@@ -687,7 +693,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 97fda3f..5ef9910 100644
--- 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
@@ -3053,11 +3051,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
@@ -3314,13 +3309,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 72f40ed..90af80a 100644
--- 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:
--
2.0.5
8:39 a.m.
New subject: [libvirt] [PATCH v3 10/15] virNetworkObjFindBy*: Return an reference to found object
On Tue, Mar 10, 2015 at 17:45:16 +0100, Michal Privoznik wrote:
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(a)redhat.com>
---
src/conf/network_conf.c | 15 ++++++++++-----
src/network/bridge_driver.c | 18 +++++-------------
src/test/test_driver.c | 10 ++++------
3 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 98342f9..a2aa4dc 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -143,6 +143,7 @@ virNetworkObjEndAPI(virNetworkObjPtr *net)
return;
virObjectUnlock(*net);
+ virObjectUnref(*net);
*net = NULL;
}
@@ -175,7 +176,7 @@ virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets,
ret = virHashLookup(nets->objs, uuidstr);
if (ret)
- virObjectLock(ret);
+ virObjectRef(ret);
return ret;
Since this ...
}
@@ -188,6 +189,8 @@ virNetworkObjFindByUUID(virNetworkObjListPtr nets,
virObjectLock(nets);
ret = virNetworkObjFindByUUIDLocked(nets, uuid);
virObjectUnlock(nets);
+ if (ret)
+ virObjectLock(ret);
and this function now differ not only in the fact that they require the
@nets object to be locked or not but in the state of the returned object
they deserve a comment telling the difference.
return ret;
}
@@ -214,7 +217,7 @@ virNetworkObjFindByNameLocked(virNetworkObjListPtr nets,
ret = virHashSearch(nets->objs, virNetworkObjSearchName, name);
if (ret)
- virObjectLock(ret);
+ virObjectRef(ret);
return ret;
}
@@ -227,6 +230,8 @@ virNetworkObjFindByName(virNetworkObjListPtr nets,
virObjectLock(nets);
ret = virNetworkObjFindByNameLocked(nets, name);
virObjectUnlock(nets);
+ if (ret)
+ virObjectLock(ret);
Same here.
return ret;
}
ACK,
Peter
11:45 a.m.
New subject: [libvirt] [PATCH v3 11/15] struct _virNetworkDriverState: Annotate items
In order to drop network driver lock, lets annotate which
structure items are immutable, which have self-locking
APIs and so on.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/network/bridge_driver_platform.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
index b7492e6..d9cf6a8 100644
--- a/src/network/bridge_driver_platform.h
+++ b/src/network/bridge_driver_platform.h
@@ -34,8 +34,10 @@
struct _virNetworkDriverState {
virMutex lock;
+ /* Immutable pointer, self-locking APIs */
virNetworkObjListPtr networks;
+ /* Immutable pointer, Immutable objects */
char *networkConfigDir;
char *networkAutostartDir;
char *stateDir;
@@ -44,6 +46,7 @@ struct _virNetworkDriverState {
char *radvdStateDir;
dnsmasqCapsPtr dnsmasqCaps;
+ /* Immutable pointer, self-locking APIs */
virObjectEventStatePtr networkEventState;
};
--
2.0.5
8:44 a.m.
New subject: [libvirt] [PATCH v3 11/15] struct _virNetworkDriverState: Annotate items
On Tue, Mar 10, 2015 at 17:45:17 +0100, Michal Privoznik wrote:
In order to drop network driver lock, lets annotate which
structure items are immutable, which have self-locking
APIs and so on.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/network/bridge_driver_platform.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
index b7492e6..d9cf6a8 100644
--- a/src/network/bridge_driver_platform.h
+++ b/src/network/bridge_driver_platform.h
@@ -34,8 +34,10 @@
struct _virNetworkDriverState {
virMutex lock;
+ /* Immutable pointer, self-locking APIs */
virNetworkObjListPtr networks;
+ /* Immutable pointer, Immutable objects */
char *networkConfigDir;
char *networkAutostartDir;
char *stateDir;
@@ -44,6 +46,7 @@ struct _virNetworkDriverState {
char *radvdStateDir;
dnsmasqCapsPtr dnsmasqCaps;
Unfortunately dnsmasqCaps is not immutable. In networkStartDhcpDaemon
it's re-allocated or updated when the network is started.
+ /* Immutable pointer, self-locking APIs */
virObjectEventStatePtr networkEventState;
};
ACK if you notify that @dnsmasqCaps is not immutable.
Peter
11:45 a.m.
New subject: [libvirt] [PATCH v3 12/15] bridge_driver: Drop networkDriverLock() from everywhere
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 | 71 ------------------------------------
src/network/bridge_driver_platform.h | 2 -
tests/objectlocking.ml | 2 -
3 files changed, 75 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 5ef9910..c6957c3 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -88,16 +88,6 @@ VIR_LOG_INIT("network.bridge_driver");
static virNetworkDriverStatePtr driver;
-
-static void networkDriverLock(void)
-{
- virMutexLock(&driver->lock);
-}
-static void networkDriverUnlock(void)
-{
- virMutexUnlock(&driver->lock);
-}
-
static int networkStateCleanup(void);
static int networkStartNetwork(virNetworkObjPtr network);
@@ -129,9 +119,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);
@@ -557,12 +545,6 @@ networkStateInitialize(bool privileged,
if (VIR_ALLOC(driver) < 0)
goto error;
- if (virMutexInit(&driver->lock) < 0) {
- VIR_FREE(driver);
- goto error;
- }
- networkDriverLock();
-
/* configuration/state paths are one of
* ~/.config/libvirt/... (session/unprivileged)
* /etc/libvirt/... && /var/(run|lib)/libvirt/... (system/privileged).
@@ -648,8 +630,6 @@ networkStateInitialize(bool privileged,
driver->networkEventState = virObjectEventStateNew();
- networkDriverUnlock();
-
#ifdef HAVE_FIREWALLD
if (!(sysbus = virDBusGetSystemBus())) {
virErrorPtr err = virGetLastError();
@@ -683,8 +663,6 @@ networkStateInitialize(bool privileged,
return ret;
error:
- if (driver)
- networkDriverUnlock();
networkStateCleanup();
goto cleanup;
}
@@ -700,11 +678,9 @@ networkStateAutoStart(void)
if (!driver)
return;
- networkDriverLock();
virNetworkObjListForEach(driver->networks,
networkAutostartConfig,
NULL);
- networkDriverUnlock();
}
/**
@@ -719,7 +695,6 @@ networkStateReload(void)
if (!driver)
return 0;
- networkDriverLock();
virNetworkLoadAllState(driver->networks,
driver->stateDir);
virNetworkLoadAllConfigs(driver->networks,
@@ -730,7 +705,6 @@ networkStateReload(void)
virNetworkObjListForEach(driver->networks,
networkAutostartConfig,
NULL);
- networkDriverUnlock();
return 0;
}
@@ -746,8 +720,6 @@ networkStateCleanup(void)
if (!driver)
return -1;
- networkDriverLock();
-
virObjectEventStateFree(driver->networkEventState);
/* free inactive networks */
@@ -762,9 +734,6 @@ networkStateCleanup(void)
virObjectUnref(driver->dnsmasqCaps);
- networkDriverUnlock();
- virMutexDestroy(&driver->lock);
-
VIR_FREE(driver);
return 0;
@@ -2478,9 +2447,7 @@ static virNetworkPtr networkLookupByUUID(virConnectPtr conn,
virNetworkObjPtr network;
virNetworkPtr ret = NULL;
- networkDriverLock();
network = virNetworkObjFindByUUID(driver->networks, uuid);
- networkDriverUnlock();
if (!network) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(uuid, uuidstr);
@@ -2506,9 +2473,7 @@ static virNetworkPtr networkLookupByName(virConnectPtr conn,
virNetworkObjPtr network;
virNetworkPtr ret = NULL;
- networkDriverLock();
network = virNetworkObjFindByName(driver->networks, name);
- networkDriverUnlock();
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
_("no network with matching name '%s'"), name);
@@ -2532,12 +2497,10 @@ static int networkConnectNumOfNetworks(virConnectPtr conn)
if (virConnectNumOfNetworksEnsureACL(conn) < 0)
return -1;
- networkDriverLock();
nactive = virNetworkObjListNumOfNetworks(driver->networks,
true,
virConnectNumOfNetworksCheckACL,
conn);
- networkDriverUnlock();
return nactive;
}
@@ -2548,12 +2511,10 @@ static int networkConnectListNetworks(virConnectPtr conn, char
**const names, in
if (virConnectListNetworksEnsureACL(conn) < 0)
return -1;
- networkDriverLock();
got = virNetworkObjListGetNames(driver->networks,
true, names, nnames,
virConnectListNetworksCheckACL,
conn);
- networkDriverUnlock();
return got;
}
@@ -2565,12 +2526,10 @@ static int networkConnectNumOfDefinedNetworks(virConnectPtr conn)
if (virConnectNumOfDefinedNetworksEnsureACL(conn) < 0)
return -1;
- networkDriverLock();
ninactive = virNetworkObjListNumOfNetworks(driver->networks,
false,
virConnectNumOfDefinedNetworksCheckACL,
conn);
- networkDriverUnlock();
return ninactive;
}
@@ -2581,12 +2540,10 @@ static int networkConnectListDefinedNetworks(virConnectPtr conn,
char **const na
if (virConnectListDefinedNetworksEnsureACL(conn) < 0)
return -1;
- networkDriverLock();
got = virNetworkObjListGetNames(driver->networks,
false, names, nnames,
virConnectListDefinedNetworksCheckACL,
conn);
- networkDriverUnlock();
return got;
}
@@ -2602,11 +2559,9 @@ networkConnectListAllNetworks(virConnectPtr conn,
if (virConnectListAllNetworksEnsureACL(conn) < 0)
goto cleanup;
- networkDriverLock();
ret = virNetworkObjListExport(conn, driver->networks, nets,
virConnectListAllNetworksCheckACL,
flags);
- networkDriverUnlock();
cleanup:
return ret;
@@ -2917,8 +2872,6 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn, const char
*xml)
virNetworkPtr ret = NULL;
virObjectEventPtr event = NULL;
- networkDriverLock();
-
if (!(def = virNetworkDefParseString(xml)))
goto cleanup;
@@ -2955,7 +2908,6 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn, const char
*xml)
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
virNetworkObjEndAPI(&network);
- networkDriverUnlock();
return ret;
}
@@ -2967,8 +2919,6 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn, const char
*xml)
virNetworkPtr ret = NULL;
virObjectEventPtr event = NULL;
- networkDriverLock();
-
if (!(def = virNetworkDefParseString(xml)))
goto cleanup;
@@ -3010,7 +2960,6 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn, const char
*xml)
if (freeDef)
virNetworkDefFree(def);
virNetworkObjEndAPI(&network);
- networkDriverUnlock();
return ret;
}
@@ -3022,8 +2971,6 @@ networkUndefine(virNetworkPtr net)
bool active = false;
virObjectEventPtr event = NULL;
- networkDriverLock();
-
network = virNetworkObjFindByUUID(driver->networks, net->uuid);
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
@@ -3067,7 +3014,6 @@ networkUndefine(virNetworkPtr net)
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
virNetworkObjEndAPI(&network);
- networkDriverUnlock();
return ret;
}
@@ -3091,8 +3037,6 @@ networkUpdate(virNetworkPtr net,
VIR_NETWORK_UPDATE_AFFECT_CONFIG,
-1);
- networkDriverLock();
-
network = virNetworkObjFindByUUID(driver->networks, net->uuid);
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
@@ -3238,7 +3182,6 @@ networkUpdate(virNetworkPtr net,
ret = 0;
cleanup:
virNetworkObjEndAPI(&network);
- networkDriverUnlock();
return ret;
}
@@ -3248,7 +3191,6 @@ static int networkCreate(virNetworkPtr net)
int ret = -1;
virObjectEventPtr event = NULL;
- networkDriverLock();
network = virNetworkObjFindByUUID(driver->networks, net->uuid);
if (!network) {
@@ -3272,7 +3214,6 @@ static int networkCreate(virNetworkPtr net)
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
virNetworkObjEndAPI(&network);
- networkDriverUnlock();
return ret;
}
@@ -3282,7 +3223,6 @@ static int networkDestroy(virNetworkPtr net)
int ret = -1;
virObjectEventPtr event = NULL;
- networkDriverLock();
network = virNetworkObjFindByUUID(driver->networks, net->uuid);
if (!network) {
@@ -3319,7 +3259,6 @@ static int networkDestroy(virNetworkPtr net)
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
virNetworkObjEndAPI(&network);
- networkDriverUnlock();
return ret;
}
@@ -3401,7 +3340,6 @@ static int networkSetAutostart(virNetworkPtr net,
char *configFile = NULL, *autostartLink = NULL;
int ret = -1;
- networkDriverLock();
network = virNetworkObjFindByUUID(driver->networks, net->uuid);
if (!network) {
@@ -3458,7 +3396,6 @@ static int networkSetAutostart(virNetworkPtr net,
VIR_FREE(configFile);
VIR_FREE(autostartLink);
virNetworkObjEndAPI(&network);
- networkDriverUnlock();
return ret;
}
@@ -3729,9 +3666,7 @@ networkAllocateActualDevice(virDomainDefPtr dom,
virDomainActualNetDefFree(iface->data.network.actual);
iface->data.network.actual = NULL;
- networkDriverLock();
network = virNetworkObjFindByName(driver->networks, iface->data.network.name);
- networkDriverUnlock();
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
_("no network with matching name '%s'"),
@@ -4137,9 +4072,7 @@ networkNotifyActualDevice(virDomainDefPtr dom,
if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
return 0;
- networkDriverLock();
network = virNetworkObjFindByName(driver->networks, iface->data.network.name);
- networkDriverUnlock();
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
_("no network with matching name '%s'"),
@@ -4336,9 +4269,7 @@ networkReleaseActualDevice(virDomainDefPtr dom,
if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
return 0;
- networkDriverLock();
network = virNetworkObjFindByName(driver->networks, iface->data.network.name);
- networkDriverUnlock();
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
_("no network with matching name '%s'"),
@@ -4494,9 +4425,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr)
char *dev_name = NULL;
*netaddr = NULL;
- networkDriverLock();
network = virNetworkObjFindByName(driver->networks, netname);
- networkDriverUnlock();
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
_("no network with matching name '%s'"),
diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
index d9cf6a8..df1b7a8 100644
--- a/src/network/bridge_driver_platform.h
+++ b/src/network/bridge_driver_platform.h
@@ -32,8 +32,6 @@
/* Main driver state */
struct _virNetworkDriverState {
- virMutex lock;
-
/* Immutable pointer, self-locking APIs */
virNetworkObjListPtr networks;
diff --git a/tests/objectlocking.ml b/tests/objectlocking.ml
index fd83d6d..4fdf95b 100644
--- a/tests/objectlocking.ml
+++ b/tests/objectlocking.ml
@@ -125,7 +125,6 @@ let driverLockMethods = [
"lxcDriverLock";
"umlDriverLock";
"nodedevDriverLock";
- "networkDriverLock";
"storageDriverLock";
"oneDriverLock"
]
@@ -140,7 +139,6 @@ let driverUnlockMethods = [
"lxcDriverUnlock";
"umlDriverUnlock";
"nodedevDriverUnlock";
- "networkDriverUnlock";
"storageDriverUnlock";
"oneDriverUnlock"
]
--
2.0.5
4:42 a.m.
New subject: [libvirt] [PATCH v3 12/15] bridge_driver: Drop networkDriverLock() from everywhere
On Tue, Mar 10, 2015 at 17:45:18 +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 | 71 ------------------------------------
src/network/bridge_driver_platform.h | 2 -
tests/objectlocking.ml | 2 -
3 files changed, 75 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 5ef9910..c6957c3 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -88,16 +88,6 @@ VIR_LOG_INIT("network.bridge_driver");
static virNetworkDriverStatePtr driver;
-
-static void networkDriverLock(void)
-{
- virMutexLock(&driver->lock);
-}
-static void networkDriverUnlock(void)
-{
- virMutexUnlock(&driver->lock);
-}
-
static int networkStateCleanup(void);
static int networkStartNetwork(virNetworkObjPtr network);
@@ -129,9 +119,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);
@@ -557,12 +545,6 @@ networkStateInitialize(bool privileged,
if (VIR_ALLOC(driver) < 0)
goto error;
- if (virMutexInit(&driver->lock) < 0) {
- VIR_FREE(driver);
- goto error;
- }
- networkDriverLock();
-
/* configuration/state paths are one of
* ~/.config/libvirt/... (session/unprivileged)
* /etc/libvirt/... && /var/(run|lib)/libvirt/... (system/privileged).
@@ -648,8 +630,6 @@ networkStateInitialize(bool privileged,
driver->networkEventState = virObjectEventStateNew();
- networkDriverUnlock();
-
#ifdef HAVE_FIREWALLD
if (!(sysbus = virDBusGetSystemBus())) {
virErrorPtr err = virGetLastError();
@@ -683,8 +663,6 @@ networkStateInitialize(bool privileged,
return ret;
error:
- if (driver)
- networkDriverUnlock();
networkStateCleanup();
goto cleanup;
}
@@ -700,11 +678,9 @@ networkStateAutoStart(void)
if (!driver)
return;
- networkDriverLock();
virNetworkObjListForEach(driver->networks,
networkAutostartConfig,
Although it's not obvious as the @driver isn't passed explicitly this
internally calls networkStartDhcpDaemon which accesses
@driver->dnsmasqStateDir which isn't immutable.
Having the access to @driver hidden by the access to a global variable
is really sneaky. I'd prefer if we'd pass it explicitly in this case.
NULL);
- networkDriverUnlock();
}
/**
@@ -719,7 +695,6 @@ networkStateReload(void)
if (!driver)
return 0;
- networkDriverLock();
virNetworkLoadAllState(driver->networks,
driver->stateDir);
virNetworkLoadAllConfigs(driver->networks,
@@ -730,7 +705,6 @@ networkStateReload(void)
virNetworkObjListForEach(driver->networks,
networkAutostartConfig,
same problem as above
NULL);
- networkDriverUnlock();
return 0;
}
@@ -746,8 +720,6 @@ networkStateCleanup(void)
if (!driver)
return -1;
- networkDriverLock();
-
virObjectEventStateFree(driver->networkEventState);
/* free inactive networks */
@@ -762,9 +734,6 @@ networkStateCleanup(void)
virObjectUnref(driver->dnsmasqCaps);
- networkDriverUnlock();
- virMutexDestroy(&driver->lock);
-
VIR_FREE(driver);
return 0;
@@ -2478,9 +2447,7 @@ static virNetworkPtr networkLookupByUUID(virConnectPtr conn,
virNetworkObjPtr network;
virNetworkPtr ret = NULL;
- networkDriverLock();
network = virNetworkObjFindByUUID(driver->networks, uuid);
- networkDriverUnlock();
if (!network) {
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(uuid, uuidstr);
@@ -2506,9 +2473,7 @@ static virNetworkPtr networkLookupByName(virConnectPtr conn,
virNetworkObjPtr network;
virNetworkPtr ret = NULL;
- networkDriverLock();
network = virNetworkObjFindByName(driver->networks, name);
- networkDriverUnlock();
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
_("no network with matching name '%s'"),
name);
@@ -2532,12 +2497,10 @@ static int networkConnectNumOfNetworks(virConnectPtr conn)
if (virConnectNumOfNetworksEnsureACL(conn) < 0)
return -1;
- networkDriverLock();
nactive = virNetworkObjListNumOfNetworks(driver->networks,
true,
virConnectNumOfNetworksCheckACL,
conn);
- networkDriverUnlock();
return nactive;
}
@@ -2548,12 +2511,10 @@ static int networkConnectListNetworks(virConnectPtr conn, char
**const names, in
if (virConnectListNetworksEnsureACL(conn) < 0)
return -1;
- networkDriverLock();
got = virNetworkObjListGetNames(driver->networks,
true, names, nnames,
virConnectListNetworksCheckACL,
conn);
- networkDriverUnlock();
return got;
}
@@ -2565,12 +2526,10 @@ static int networkConnectNumOfDefinedNetworks(virConnectPtr
conn)
if (virConnectNumOfDefinedNetworksEnsureACL(conn) < 0)
return -1;
- networkDriverLock();
ninactive = virNetworkObjListNumOfNetworks(driver->networks,
false,
virConnectNumOfDefinedNetworksCheckACL,
conn);
- networkDriverUnlock();
return ninactive;
}
@@ -2581,12 +2540,10 @@ static int networkConnectListDefinedNetworks(virConnectPtr conn,
char **const na
if (virConnectListDefinedNetworksEnsureACL(conn) < 0)
return -1;
- networkDriverLock();
got = virNetworkObjListGetNames(driver->networks,
false, names, nnames,
virConnectListDefinedNetworksCheckACL,
conn);
- networkDriverUnlock();
return got;
}
@@ -2602,11 +2559,9 @@ networkConnectListAllNetworks(virConnectPtr conn,
if (virConnectListAllNetworksEnsureACL(conn) < 0)
goto cleanup;
- networkDriverLock();
ret = virNetworkObjListExport(conn, driver->networks, nets,
virConnectListAllNetworksCheckACL,
flags);
- networkDriverUnlock();
cleanup:
return ret;
@@ -2917,8 +2872,6 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn, const
char *xml)
virNetworkPtr ret = NULL;
virObjectEventPtr event = NULL;
- networkDriverLock();
This function will have the same problem as it starts the network.
-
if (!(def = virNetworkDefParseString(xml)))
goto cleanup;
@@ -2955,7 +2908,6 @@ static virNetworkPtr networkCreateXML(virConnectPtr conn, const
char *xml)
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
virNetworkObjEndAPI(&network);
- networkDriverUnlock();
return ret;
}
@@ -2967,8 +2919,6 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn, const
char *xml)
virNetworkPtr ret = NULL;
virObjectEventPtr event = NULL;
- networkDriverLock();
-
if (!(def = virNetworkDefParseString(xml)))
goto cleanup;
@@ -3010,7 +2960,6 @@ static virNetworkPtr networkDefineXML(virConnectPtr conn, const
char *xml)
if (freeDef)
virNetworkDefFree(def);
virNetworkObjEndAPI(&network);
- networkDriverUnlock();
return ret;
}
@@ -3022,8 +2971,6 @@ networkUndefine(virNetworkPtr net)
bool active = false;
virObjectEventPtr event = NULL;
- networkDriverLock();
-
network = virNetworkObjFindByUUID(driver->networks, net->uuid);
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
@@ -3067,7 +3014,6 @@ networkUndefine(virNetworkPtr net)
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
virNetworkObjEndAPI(&network);
- networkDriverUnlock();
return ret;
}
@@ -3091,8 +3037,6 @@ networkUpdate(virNetworkPtr net,
VIR_NETWORK_UPDATE_AFFECT_CONFIG,
-1);
- networkDriverLock();
-
This API is restarting the DHCP daemon in some cases so it will share
the problem above.
network = virNetworkObjFindByUUID(driver->networks,
net->uuid);
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
@@ -3238,7 +3182,6 @@ networkUpdate(virNetworkPtr net,
ret = 0;
cleanup:
virNetworkObjEndAPI(&network);
- networkDriverUnlock();
return ret;
}
@@ -3248,7 +3191,6 @@ static int networkCreate(virNetworkPtr net)
int ret = -1;
virObjectEventPtr event = NULL;
- networkDriverLock();
network = virNetworkObjFindByUUID(driver->networks, net->uuid);
here too
if (!network) {
@@ -3272,7 +3214,6 @@ static int networkCreate(virNetworkPtr net)
if (event)
virObjectEventStateQueue(driver->networkEventState, event);
virNetworkObjEndAPI(&network);
- networkDriverUnlock();
return ret;
}
The driver lock can't be dropped in that case. I think we can employ a
similar mechanism as in qemu driver, where data that changes is stored
in a reference counted object and the object pointer is replaced under a
lock in case it's required to change the data.
The rest of the driver lock dropping looks okay to me though.
Peter
11:45 a.m.
New subject: [libvirt] [PATCH v3 13/15] test_driver: Drop testDriverLock() from almost everywhere
Well, if 'everywhere' is defined as that part of the driver code
that serves virNetwork* APIs. Again, we lower layers already have
their locks, so there's no point doing big lock.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/test/test_driver.c | 56 +-------------------------------------------------
1 file changed, 1 insertion(+), 55 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 90af80a..187bd3d 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -3495,10 +3495,7 @@ static virNetworkPtr testNetworkLookupByUUID(virConnectPtr conn,
virNetworkObjPtr net;
virNetworkPtr ret = NULL;
- testDriverLock(privconn);
net = virNetworkObjFindByUUID(privconn->networks, uuid);
- testDriverUnlock(privconn);
-
if (net == NULL) {
virReportError(VIR_ERR_NO_NETWORK, NULL);
goto cleanup;
@@ -3518,10 +3515,7 @@ static virNetworkPtr testNetworkLookupByName(virConnectPtr conn,
virNetworkObjPtr net;
virNetworkPtr ret = NULL;
- testDriverLock(privconn);
net = virNetworkObjFindByName(privconn->networks, name);
- testDriverUnlock(privconn);
-
if (net == NULL) {
virReportError(VIR_ERR_NO_NETWORK, NULL);
goto cleanup;
@@ -3540,11 +3534,8 @@ static int testConnectNumOfNetworks(virConnectPtr conn)
testConnPtr privconn = conn->privateData;
int numActive;
- testDriverLock(privconn);
numActive = virNetworkObjListNumOfNetworks(privconn->networks,
true, NULL, conn);
- testDriverUnlock(privconn);
-
return numActive;
}
@@ -3552,11 +3543,8 @@ static int testConnectListNetworks(virConnectPtr conn, char **const
names, int n
testConnPtr privconn = conn->privateData;
int n;
- testDriverLock(privconn);
n = virNetworkObjListGetNames(privconn->networks,
true, names, nnames, NULL, conn);
- testDriverUnlock(privconn);
-
return n;
}
@@ -3565,11 +3553,8 @@ static int testConnectNumOfDefinedNetworks(virConnectPtr conn)
testConnPtr privconn = conn->privateData;
int numInactive;
- testDriverLock(privconn);
numInactive = virNetworkObjListNumOfNetworks(privconn->networks,
false, NULL, conn);
- testDriverUnlock(privconn);
-
return numInactive;
}
@@ -3577,11 +3562,8 @@ static int testConnectListDefinedNetworks(virConnectPtr conn, char
**const names
testConnPtr privconn = conn->privateData;
int n;
- testDriverLock(privconn);
n = virNetworkObjListGetNames(privconn->networks,
false, names, nnames, NULL, conn);
- testDriverUnlock(privconn);
-
return n;
}
@@ -3591,15 +3573,10 @@ testConnectListAllNetworks(virConnectPtr conn,
unsigned int flags)
{
testConnPtr privconn = conn->privateData;
- int ret = -1;
virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
- testDriverLock(privconn);
- ret = virNetworkObjListExport(conn, privconn->networks, nets, NULL, flags);
- testDriverUnlock(privconn);
-
- return ret;
+ return virNetworkObjListExport(conn, privconn->networks, nets, NULL, flags);
}
static int testNetworkIsActive(virNetworkPtr net)
@@ -3608,9 +3585,7 @@ static int testNetworkIsActive(virNetworkPtr net)
virNetworkObjPtr obj;
int ret = -1;
- testDriverLock(privconn);
obj = virNetworkObjFindByUUID(privconn->networks, net->uuid);
- testDriverUnlock(privconn);
if (!obj) {
virReportError(VIR_ERR_NO_NETWORK, NULL);
goto cleanup;
@@ -3628,9 +3603,7 @@ static int testNetworkIsPersistent(virNetworkPtr net)
virNetworkObjPtr obj;
int ret = -1;
- testDriverLock(privconn);
obj = virNetworkObjFindByUUID(privconn->networks, net->uuid);
- testDriverUnlock(privconn);
if (!obj) {
virReportError(VIR_ERR_NO_NETWORK, NULL);
goto cleanup;
@@ -3651,7 +3624,6 @@ static virNetworkPtr testNetworkCreateXML(virConnectPtr conn, const
char *xml)
virNetworkPtr ret = NULL;
virObjectEventPtr event = NULL;
- testDriverLock(privconn);
if ((def = virNetworkDefParseString(xml)) == NULL)
goto cleanup;
@@ -3671,7 +3643,6 @@ static virNetworkPtr testNetworkCreateXML(virConnectPtr conn, const
char *xml)
if (event)
testObjectEventQueue(privconn, event);
virNetworkObjEndAPI(&net);
- testDriverUnlock(privconn);
return ret;
}
@@ -3684,7 +3655,6 @@ virNetworkPtr testNetworkDefineXML(virConnectPtr conn, const char
*xml)
virNetworkPtr ret = NULL;
virObjectEventPtr event = NULL;
- testDriverLock(privconn);
if ((def = virNetworkDefParseString(xml)) == NULL)
goto cleanup;
@@ -3703,7 +3673,6 @@ virNetworkPtr testNetworkDefineXML(virConnectPtr conn, const char
*xml)
if (event)
testObjectEventQueue(privconn, event);
virNetworkObjEndAPI(&net);
- testDriverUnlock(privconn);
return ret;
}
@@ -3714,7 +3683,6 @@ static int testNetworkUndefine(virNetworkPtr network)
int ret = -1;
virObjectEventPtr event = NULL;
- testDriverLock(privconn);
privnet = virNetworkObjFindByName(privconn->networks, network->name);
if (privnet == NULL) {
@@ -3739,7 +3707,6 @@ static int testNetworkUndefine(virNetworkPtr network)
if (event)
testObjectEventQueue(privconn, event);
virNetworkObjEndAPI(&privnet);
- testDriverUnlock(privconn);
return ret;
}
@@ -3759,8 +3726,6 @@ testNetworkUpdate(virNetworkPtr net,
VIR_NETWORK_UPDATE_AFFECT_CONFIG,
-1);
- testDriverLock(privconn);
-
network = virNetworkObjFindByUUID(privconn->networks, net->uuid);
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
@@ -3788,7 +3753,6 @@ testNetworkUpdate(virNetworkPtr net,
ret = 0;
cleanup:
virNetworkObjEndAPI(&network);
- testDriverUnlock(privconn);
return ret;
}
@@ -3799,10 +3763,7 @@ static int testNetworkCreate(virNetworkPtr network)
int ret = -1;
virObjectEventPtr event = NULL;
- testDriverLock(privconn);
privnet = virNetworkObjFindByName(privconn->networks, network->name);
- testDriverUnlock(privconn);
-
if (privnet == NULL) {
virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
goto cleanup;
@@ -3834,9 +3795,7 @@ static int testNetworkDestroy(virNetworkPtr network)
int ret = -1;
virObjectEventPtr event = NULL;
- testDriverLock(privconn);
privnet = virNetworkObjFindByName(privconn->networks, network->name);
-
if (privnet == NULL) {
virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
goto cleanup;
@@ -3855,7 +3814,6 @@ static int testNetworkDestroy(virNetworkPtr network)
if (event)
testObjectEventQueue(privconn, event);
virNetworkObjEndAPI(&privnet);
- testDriverUnlock(privconn);
return ret;
}
@@ -3868,10 +3826,7 @@ static char *testNetworkGetXMLDesc(virNetworkPtr network,
virCheckFlags(0, NULL);
- testDriverLock(privconn);
privnet = virNetworkObjFindByName(privconn->networks, network->name);
- testDriverUnlock(privconn);
-
if (privnet == NULL) {
virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
goto cleanup;
@@ -3889,10 +3844,7 @@ static char *testNetworkGetBridgeName(virNetworkPtr network) {
char *bridge = NULL;
virNetworkObjPtr privnet;
- testDriverLock(privconn);
privnet = virNetworkObjFindByName(privconn->networks, network->name);
- testDriverUnlock(privconn);
-
if (privnet == NULL) {
virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
goto cleanup;
@@ -3919,10 +3871,7 @@ static int testNetworkGetAutostart(virNetworkPtr network,
virNetworkObjPtr privnet;
int ret = -1;
- testDriverLock(privconn);
privnet = virNetworkObjFindByName(privconn->networks, network->name);
- testDriverUnlock(privconn);
-
if (privnet == NULL) {
virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
goto cleanup;
@@ -3943,10 +3892,7 @@ static int testNetworkSetAutostart(virNetworkPtr network,
virNetworkObjPtr privnet;
int ret = -1;
- testDriverLock(privconn);
privnet = virNetworkObjFindByName(privconn->networks, network->name);
- testDriverUnlock(privconn);
-
if (privnet == NULL) {
virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__);
goto cleanup;
--
2.0.5
8:45 a.m.
New subject: [libvirt] [PATCH v3 13/15] test_driver: Drop testDriverLock() from almost everywhere
On Tue, Mar 10, 2015 at 17:45:19 +0100, Michal Privoznik wrote:
Well, if 'everywhere' is defined as that part of the driver
code
that serves virNetwork* APIs. Again, we lower layers already have
their locks, so there's no point doing big lock.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/test/test_driver.c | 56 +-------------------------------------------------
1 file changed, 1 insertion(+), 55 deletions(-)
ACK,
Peter
11:45 a.m.
New subject: [libvirt] [PATCH v3 14/15] parallels_network: Drop parallelsDriverLock() from everywhere.
While in previous commits there were some places that relied on
the big lock, in this file there's no such place and the big
driver lock can be dropped completely. Yay!
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/parallels/parallels_network.c | 33 +--------------------------------
1 file changed, 1 insertion(+), 32 deletions(-)
diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c
index 1bcd2d3..1d3b694 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -349,9 +349,7 @@ int parallelsNetworkClose(virConnectPtr conn)
if (!privconn)
return 0;
- parallelsDriverLock(privconn);
virObjectUnref(privconn->networks);
- parallelsDriverUnlock(privconn);
return 0;
}
@@ -360,11 +358,8 @@ static int parallelsConnectNumOfNetworks(virConnectPtr conn)
int nactive;
parallelsConnPtr privconn = conn->privateData;
- parallelsDriverLock(privconn);
nactive = virNetworkObjListNumOfNetworks(privconn->networks,
true, NULL, conn);
- parallelsDriverUnlock(privconn);
-
return nactive;
}
@@ -375,11 +370,8 @@ static int parallelsConnectListNetworks(virConnectPtr conn,
parallelsConnPtr privconn = conn->privateData;
int got;
- parallelsDriverLock(privconn);
got = virNetworkObjListGetNames(privconn->networks,
true, names, nnames, NULL, conn);
- parallelsDriverUnlock(privconn);
-
return got;
}
@@ -388,11 +380,8 @@ static int parallelsConnectNumOfDefinedNetworks(virConnectPtr conn)
int ninactive;
parallelsConnPtr privconn = conn->privateData;
- parallelsDriverLock(privconn);
ninactive = virNetworkObjListNumOfNetworks(privconn->networks,
false, NULL, conn);
- parallelsDriverUnlock(privconn);
-
return ninactive;
}
@@ -403,10 +392,8 @@ static int parallelsConnectListDefinedNetworks(virConnectPtr conn,
parallelsConnPtr privconn = conn->privateData;
int got;
- parallelsDriverLock(privconn);
got = virNetworkObjListGetNames(privconn->networks,
false, names, nnames, NULL, conn);
- parallelsDriverUnlock(privconn);
return got;
}
@@ -415,15 +402,10 @@ static int parallelsConnectListAllNetworks(virConnectPtr conn,
unsigned int flags)
{
parallelsConnPtr privconn = conn->privateData;
- int ret = -1;
virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1);
- parallelsDriverLock(privconn);
- ret = virNetworkObjListExport(conn, privconn->networks, nets, NULL, flags);
- parallelsDriverUnlock(privconn);
-
- return ret;
+ return virNetworkObjListExport(conn, privconn->networks, nets, NULL, flags);
}
static virNetworkPtr parallelsNetworkLookupByUUID(virConnectPtr conn,
@@ -433,9 +415,7 @@ static virNetworkPtr parallelsNetworkLookupByUUID(virConnectPtr conn,
virNetworkObjPtr network;
virNetworkPtr ret = NULL;
- parallelsDriverLock(privconn);
network = virNetworkObjFindByUUID(privconn->networks, uuid);
- parallelsDriverUnlock(privconn);
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
"%s", _("no network with matching uuid"));
@@ -456,9 +436,7 @@ static virNetworkPtr parallelsNetworkLookupByName(virConnectPtr conn,
virNetworkObjPtr network;
virNetworkPtr ret = NULL;
- parallelsDriverLock(privconn);
network = virNetworkObjFindByName(privconn->networks, name);
- parallelsDriverUnlock(privconn);
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
_("no network with matching name '%s'"), name);
@@ -481,10 +459,7 @@ static char *parallelsNetworkGetXMLDesc(virNetworkPtr net,
virCheckFlags(VIR_NETWORK_XML_INACTIVE, NULL);
- parallelsDriverLock(privconn);
network = virNetworkObjFindByUUID(privconn->networks, net->uuid);
- parallelsDriverUnlock(privconn);
-
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
"%s", _("no network with matching uuid"));
@@ -504,9 +479,7 @@ static int parallelsNetworkIsActive(virNetworkPtr net)
virNetworkObjPtr obj;
int ret = -1;
- parallelsDriverLock(privconn);
obj = virNetworkObjFindByUUID(privconn->networks, net->uuid);
- parallelsDriverUnlock(privconn);
if (!obj) {
virReportError(VIR_ERR_NO_NETWORK, NULL);
goto cleanup;
@@ -524,9 +497,7 @@ static int parallelsNetworkIsPersistent(virNetworkPtr net)
virNetworkObjPtr obj;
int ret = -1;
- parallelsDriverLock(privconn);
obj = virNetworkObjFindByUUID(privconn->networks, net->uuid);
- parallelsDriverUnlock(privconn);
if (!obj) {
virReportError(VIR_ERR_NO_NETWORK, NULL);
goto cleanup;
@@ -545,9 +516,7 @@ static int parallelsNetworkGetAutostart(virNetworkPtr net,
virNetworkObjPtr network;
int ret = -1;
- parallelsDriverLock(privconn);
network = virNetworkObjFindByUUID(privconn->networks, net->uuid);
- parallelsDriverUnlock(privconn);
if (!network) {
virReportError(VIR_ERR_NO_NETWORK,
"%s", _("no network with matching uuid"));
--
2.0.5
8:52 a.m.
New subject: [libvirt] [PATCH v3 14/15] parallels_network: Drop parallelsDriverLock() from everywhere.
On Tue, Mar 10, 2015 at 17:45:20 +0100, Michal Privoznik wrote:
While in previous commits there were some places that relied on
the big lock, in this file there's no such place and the big
driver lock can be dropped completely. Yay!
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/parallels/parallels_network.c | 33 +--------------------------------
1 file changed, 1 insertion(+), 32 deletions(-)
ACK,
Peter
11:45 a.m.
New subject: [libvirt] [PATCH v3 15/15] bridge_driver: Use more of networkObjFromNetwork
Now that the network driver lock is ash heap of history,
we can use more of networkObjFromNetwork().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/network/bridge_driver.c | 33 +++++----------------------------
1 file changed, 5 insertions(+), 28 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index c6957c3..ec6948b 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -120,7 +120,6 @@ networkObjFromNetwork(virNetworkPtr net)
char uuidstr[VIR_UUID_STRING_BUFLEN];
network = virNetworkObjFindByUUID(driver->networks, net->uuid);
-
if (!network) {
virUUIDFormat(net->uuid, uuidstr);
virReportError(VIR_ERR_NO_NETWORK,
@@ -2971,12 +2970,8 @@ networkUndefine(virNetworkPtr net)
bool active = false;
virObjectEventPtr event = NULL;
- network = virNetworkObjFindByUUID(driver->networks, net->uuid);
- if (!network) {
- virReportError(VIR_ERR_NO_NETWORK,
- "%s", _("no network with matching uuid"));
+ if (!(network = networkObjFromNetwork(net)))
goto cleanup;
- }
if (virNetworkUndefineEnsureACL(net->conn, network->def) < 0)
goto cleanup;
@@ -3037,12 +3032,8 @@ networkUpdate(virNetworkPtr net,
VIR_NETWORK_UPDATE_AFFECT_CONFIG,
-1);
- network = virNetworkObjFindByUUID(driver->networks, net->uuid);
- if (!network) {
- virReportError(VIR_ERR_NO_NETWORK,
- "%s", _("no network with matching uuid"));
+ if (!(network = networkObjFromNetwork(net)))
goto cleanup;
- }
if (virNetworkUpdateEnsureACL(net->conn, network->def, flags) < 0)
goto cleanup;
@@ -3191,13 +3182,8 @@ static int networkCreate(virNetworkPtr net)
int ret = -1;
virObjectEventPtr event = NULL;
- network = virNetworkObjFindByUUID(driver->networks, net->uuid);
-
- if (!network) {
- virReportError(VIR_ERR_NO_NETWORK,
- "%s", _("no network with matching uuid"));
+ if (!(network = networkObjFromNetwork(net)))
goto cleanup;
- }
if (virNetworkCreateEnsureACL(net->conn, network->def) < 0)
goto cleanup;
@@ -3223,13 +3209,8 @@ static int networkDestroy(virNetworkPtr net)
int ret = -1;
virObjectEventPtr event = NULL;
- network = virNetworkObjFindByUUID(driver->networks, net->uuid);
-
- if (!network) {
- virReportError(VIR_ERR_NO_NETWORK,
- "%s", _("no network with matching uuid"));
+ if (!(network = networkObjFromNetwork(net)))
goto cleanup;
- }
if (virNetworkDestroyEnsureACL(net->conn, network->def) < 0)
goto cleanup;
@@ -3340,13 +3321,9 @@ static int networkSetAutostart(virNetworkPtr net,
char *configFile = NULL, *autostartLink = NULL;
int ret = -1;
- network = virNetworkObjFindByUUID(driver->networks, net->uuid);
- if (!network) {
- virReportError(VIR_ERR_NO_NETWORK,
- "%s", _("no network with matching uuid"));
+ if (!(network = networkObjFromNetwork(net)))
goto cleanup;
- }
if (virNetworkSetAutostartEnsureACL(net->conn, network->def) < 0)
goto cleanup;
--
2.0.5
3543
days inactive
3545
days old
29 comments
2 participants
participants (2)
-
Michal Privoznik -
Peter Krempa