Re: [libvirt] [PATCH v2 18/24] virNetworkObjFindBy*: Return an reference to found object

On Thu, Mar 05, 2015 at 12:05:19 +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&gt; > --- > src/conf/network_conf.c | 7 +++++-- > src/network/bridge_driver.c | 18 +++++------------- > src/test/test_driver.c | 10 ++++------ > 3 files changed, 14 insertions(+), 21 deletions(-) > > diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c > index 7af303e..3d318ce 100644 > --- a/src/conf/network_conf.c > +++ b/src/conf/network_conf.c > @@ -137,6 +137,7 @@ virNetworkObjEndAPI(virNetworkObjPtr *net) > return; > > virObjectUnlock(*net); > + virObjectUnref(*net); > *net = NULL; > } > > @@ -164,6 +165,7 @@ virNetworkObjFindByUUIDLocked(virNetworkObjListPtr nets, > virObjectLock(nets->objs[i]); > if (!memcmp(nets->objs[i]->def->uuid, uuid, VIR_UUID_BUFLEN)) { > ret = nets->objs[i]; > + virObjectRef(ret); > break; > } > virObjectUnlock(nets->objs[i]); > @@ -194,6 +196,7 @@ virNetworkObjFindByNameLocked(virNetworkObjListPtr nets, > virObjectLock(nets->objs[i]); > if (STREQ(nets->objs[i]->def->name, name)) { > ret = nets->objs[i]; > + virObjectRef(ret); > break; > } > virObjectUnlock(nets->objs[i]); > @@ -491,13 +494,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; > > } > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c > index 8501603..d3f3f4a 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); As virNetworkRemoveInactive() now doesn't free the object due to the existing reference, but unlocks it ... > - network = NULL; ... and you don't invalidate the pointer, you'd touch the unlocked object in virNetworkObjEndAPI() and try to unlock it once more. > goto cleanup; > } > /* if network was active already, just undo new persistent NACK, virNetworkRemoveInactive needs to be fixed first.