10:53 a.m.
Using comments from recent changes/reviews w/ libxl and usage of
the virDomainObjListRemove which note that it's "a problem" that
the Remove API expects a locked @obj on input, but then returns
the @obj unlocked.
Although adjusting domainobjlist is a "future" task, we can at least
modify the other drivers/vir*obj modules to be consistent at least
with respect to expecting and returning a locked (and reffed) object.
Thus, modify interface, secret, nodedev, and storage pool. The network
is already done this way and nwfilter appears to be a lost cause as
we cannot come to a concensus over how to use hash objects and thus
the code still uses forward linked list object manipulation.
Changes for virdomainobjlist require a number of other changes to
happen first with respect to consistent usage of Find and Add API's
to return locked and ref counted objects, then being able to have
the virDomainObjListRemove be consistent as well.
John Ferlan (4):
secret: Return with locked obj from virSecretObjListRemove
interface: Return with locked obj from virInterfaceObjListRemove
nodedev: Return with locked obj from virNodeDeviceObjListRemove
storagepool: Return with locked obj from virStoragePoolObjRemove
src/conf/virinterfaceobj.c | 13 ++++++++++++-
src/conf/virnodedeviceobj.c | 13 ++++++++++++-
src/conf/virsecretobj.c | 15 ++++++++-------
src/conf/virstorageobj.c | 17 ++++++++++++++---
src/node_device/node_device_hal.c | 4 ++--
src/node_device/node_device_udev.c | 2 +-
src/secret/secret_driver.c | 4 ----
src/storage/storage_driver.c | 12 +-----------
src/test/test_driver.c | 20 +++++---------------
9 files changed, 55 insertions(+), 45 deletions(-)
--
2.13.6
10:53 a.m.
New subject: [libvirt] [PATCH 1/4] secret: Return with locked obj from virSecretObjListRemove
Rather than unlock the object that was expected to be locked on
input, let the caller perform the unlock or more succinctly a
virSecretObjEndAPI on the object which will perform the Unref
and Unlock and clear the @obj.
Also clean up the virSecretObjListRemove function comments.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virsecretobj.c | 15 ++++++++-------
src/secret/secret_driver.c | 4 ----
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 47e0b28968..49c341484b 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -284,11 +284,14 @@ virSecretObjListFindByUsage(virSecretObjListPtr secrets,
/*
* virSecretObjListRemove:
* @secrets: list of secret objects
- * @secret: a secret object
+ * @obj: a secret object
*
- * Remove the object from the hash table. The caller must hold the lock
- * on the driver owning @secrets and must have also locked @secret to
- * ensure no one else is either waiting for @secret or still using it.
+ * Remove @obj from the secret obj list hash table. The caller must hold
+ * the lock on @obj to ensure no one else is either waiting for @obj or
+ * still using it.
+ *
+ * Upon return the @obj remains locked with at least 1 reference and
+ * the caller is expected to use virSecretObjEndAPI on it.
*/
void
virSecretObjListRemove(virSecretObjListPtr secrets,
@@ -308,7 +311,6 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
virObjectRWLockWrite(secrets);
virObjectLock(obj);
virHashRemoveEntry(secrets->objs, uuidstr);
- virObjectUnlock(obj);
virObjectUnref(obj);
virObjectRWUnlock(secrets);
}
@@ -927,8 +929,7 @@ virSecretLoad(virSecretObjListPtr secrets,
if (virSecretLoadValue(obj) < 0) {
virSecretObjListRemove(secrets, obj);
- virObjectUnref(obj);
- obj = NULL;
+ virSecretObjEndAPI(&obj); /* clears obj */
}
cleanup:
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index 23a3c9bdad..7c611253aa 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -271,8 +271,6 @@ secretDefineXML(virConnectPtr conn,
VIR_STEAL_PTR(def, objDef);
} else {
virSecretObjListRemove(driver->secrets, obj);
- virObjectUnref(obj);
- obj = NULL;
}
cleanup:
@@ -413,8 +411,6 @@ secretUndefine(virSecretPtr secret)
virSecretObjDeleteData(obj);
virSecretObjListRemove(driver->secrets, obj);
- virObjectUnref(obj);
- obj = NULL;
ret = 0;
--
2.13.6
4:44 a.m.
New subject: [libvirt] [PATCH 1/4] secret: Return with locked obj from virSecretObjListRemove
On Wed, Mar 21, 2018 at 11:53:32AM -0400, John Ferlan wrote:
Rather than unlock the object that was expected to be locked on
input, let the caller perform the unlock or more succinctly a
virSecretObjEndAPI on the object which will perform the Unref
and Unlock and clear the @obj.
I understand the incentive for this change and I agree with it, but I do have a
question below.
Also clean up the virSecretObjListRemove function comments.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virsecretobj.c | 15 ++++++++-------
src/secret/secret_driver.c | 4 ----
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 47e0b28968..49c341484b 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -284,11 +284,14 @@ virSecretObjListFindByUsage(virSecretObjListPtr secrets,
/*
* virSecretObjListRemove:
* @secrets: list of secret objects
- * @secret: a secret object
+ * @obj: a secret object
*
- * Remove the object from the hash table. The caller must hold the lock
- * on the driver owning @secrets and must have also locked @secret to
- * ensure no one else is either waiting for @secret or still using it.
+ * Remove @obj from the secret obj list hash table. The caller must hold
+ * the lock on @obj to ensure no one else is either waiting for @obj or
+ * still using it.
+ *
+ * Upon return the @obj remains locked with at least 1 reference and
+ * the caller is expected to use virSecretObjEndAPI on it.
*/
void
virSecretObjListRemove(virSecretObjListPtr secrets,
@@ -308,7 +311,6 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
virObjectRWLockWrite(secrets);
virObjectLock(obj);
virHashRemoveEntry(secrets->objs, uuidstr);
- virObjectUnlock(obj);
virObjectUnref(obj);
virObjectRWUnlock(secrets);
So, why do we require the caller to hold a lock on the object when the very
first thing we do is that we ref the @obj, UNLOCK it to avoid a deadlock with
someone else already holding a lock on @secrets and waiting on @obj and only
when we have the lock on @secrets, we re-lock @obj. Why isn't it sufficient to
just ref @obj since that's an atomic op and then only lock it once we acquire a
lock on @secrets too. In this case, you don't have to accept and return a locked
object, you only lock when absolutely necessary, what am I missing? In my
opinion this lock-unlock-lock (with occasional ref) fire dance is quite
cumbersome, but I guess I might be missing some important aspect here which I
don't see at the first glance.
}
@@ -927,8 +929,7 @@ virSecretLoad(virSecretObjListPtr secrets,
if (virSecretLoadValue(obj) < 0) {
virSecretObjListRemove(secrets, obj);
- virObjectUnref(obj);
- obj = NULL;
+ virSecretObjEndAPI(&obj); /* clears obj */
This can be further simplified by moving the ObjEndAPI to the cleanup section
and removing the braces on this 'if' block.
Erik
8:32 a.m.
New subject: [libvirt] [PATCH 1/4] secret: Return with locked obj from virSecretObjListRemove
On 03/22/2018 05:44 AM, Erik Skultety wrote:
On Wed, Mar 21, 2018 at 11:53:32AM -0400, John Ferlan wrote:
> Rather than unlock the object that was expected to be locked on
> input, let the caller perform the unlock or more succinctly a
> virSecretObjEndAPI on the object which will perform the Unref
> and Unlock and clear the @obj.
I understand the incentive for this change and I agree with it, but I do have a
question below.
>
> Also clean up the virSecretObjListRemove function comments.
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/conf/virsecretobj.c | 15 ++++++++-------
> src/secret/secret_driver.c | 4 ----
> 2 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
> index 47e0b28968..49c341484b 100644
> --- a/src/conf/virsecretobj.c
> +++ b/src/conf/virsecretobj.c
> @@ -284,11 +284,14 @@ virSecretObjListFindByUsage(virSecretObjListPtr secrets,
> /*
> * virSecretObjListRemove:
> * @secrets: list of secret objects
> - * @secret: a secret object
> + * @obj: a secret object
> *
> - * Remove the object from the hash table. The caller must hold the lock
> - * on the driver owning @secrets and must have also locked @secret to
> - * ensure no one else is either waiting for @secret or still using it.
> + * Remove @obj from the secret obj list hash table. The caller must hold
> + * the lock on @obj to ensure no one else is either waiting for @obj or
> + * still using it.
> + *
> + * Upon return the @obj remains locked with at least 1 reference and
> + * the caller is expected to use virSecretObjEndAPI on it.
> */
> void
> virSecretObjListRemove(virSecretObjListPtr secrets,
> @@ -308,7 +311,6 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
> virObjectRWLockWrite(secrets);
> virObjectLock(obj);
> virHashRemoveEntry(secrets->objs, uuidstr);
> - virObjectUnlock(obj);
> virObjectUnref(obj);
> virObjectRWUnlock(secrets);
So, why do we require the caller to hold a lock on the object when the very
first thing we do is that we ref the @obj, UNLOCK it to avoid a deadlock with
someone else already holding a lock on @secrets and waiting on @obj and only
when we have the lock on @secrets, we re-lock @obj. Why isn't it sufficient to
just ref @obj since that's an atomic op and then only lock it once we acquire a
lock on @secrets too. In this case, you don't have to accept and return a locked
object, you only lock when absolutely necessary, what am I missing? In my
opinion this lock-unlock-lock (with occasional ref) fire dance is quite
cumbersome, but I guess I might be missing some important aspect here which I
don't see at the first glance.
I have no compelling argument other than history (it's how it's been
done) and a desire to not change too much code, but I can take that path
to see what happens.
Ironically as I was making these changes I thought - why even do that
ref since we enter with the @obj reffed and locked. Adding one more ref
doesn't help or hurt when it comes to unlock as we enter (this code)
with 2 refs and 1 lock. We RemoveEntry losing 1 ref and then have the
caller unref when it's done. The lock fire dance is only because of
lock ordering.
I think part of the issue is that there's mostly error path callers to
ListRemove and one non error path caller (Undefine). For each of those
callers, we grab the list lock, fetch the object, and return it
locked/reffed - changing that would have the same fire dance problem you
note in the current Remove path, but perhaps for larger swaths of code.
Another part of the issue is that we don't grab the List lock in the
driver, rather we grab it in the vir*obj.c code. That's a design
decision propagated from the domain/network change to use hash tables
that I just followed when altering secrets, interface, nodedev, storage,
nwfilter, etc.
So in order to not have the fire dance, the list locking would need to
move to the driver and would be held for longer periods which is what
IIRC we're trying to avoid originally (think domain code and how many
threads can hammer that). Of course with rwlock/read locks that perhaps
is now a non issue.
NB: consider the possibility that two threads try to Undefine at the
same time... One wins the list lock race and does the hash table
removal, but since there's still a refcnt from the other thread, the
memory isn't freed. When the second thread gets the lock, it'll call the
RemoveEntry (and fail, but not error). Eventually it unrefs the last ref
and poof the memory is gone.
> }
> @@ -927,8 +929,7 @@ virSecretLoad(virSecretObjListPtr secrets,
>
> if (virSecretLoadValue(obj) < 0) {
> virSecretObjListRemove(secrets, obj);
> - virObjectUnref(obj);
> - obj = NULL;
> + virSecretObjEndAPI(&obj); /* clears obj */
This can be further simplified by moving the ObjEndAPI to the cleanup section
and removing the braces on this 'if' block.
True, that'd be an extra "before" patch I think though.
John
10:53 a.m.
New subject: [libvirt] [PATCH 2/4] interface: Return with locked obj from virInterfaceObjListRemove
Rather than unlock the object that was expected to be locked on
input, let the caller perform the unlock or more succinctly a
virInterfaceObjEndAPI on the object which will perform the Unref
and Unlock and clear the @obj.
Also add comments to the virInterfaceObjListRemove.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virinterfaceobj.c | 13 ++++++++++++-
src/test/test_driver.c | 2 +-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/src/conf/virinterfaceobj.c b/src/conf/virinterfaceobj.c
index f90c0bd9c4..28fbbaa08b 100644
--- a/src/conf/virinterfaceobj.c
+++ b/src/conf/virinterfaceobj.c
@@ -358,6 +358,18 @@ virInterfaceObjListAssignDef(virInterfaceObjListPtr interfaces,
}
+/*
+ * virInterfaceObjListRemove:
+ * @interfaces: list of interface objects
+ * @obj: an interface object
+ *
+ * Remove @obj from the interface obj list hash table. The caller must hold
+ * the lock on @obj to ensure no one else is either waiting for @obj or
+ * still using it.
+ *
+ * Upon return the @obj remains locked with at least 1 reference and
+ * the caller is expected to use virInterfaceObjEndAPI on it.
+ */
void
virInterfaceObjListRemove(virInterfaceObjListPtr interfaces,
virInterfaceObjPtr obj)
@@ -370,7 +382,6 @@ virInterfaceObjListRemove(virInterfaceObjListPtr interfaces,
virObjectRWLockWrite(interfaces);
virObjectLock(obj);
virHashRemoveEntry(interfaces->objsName, obj->def->name);
- virObjectUnlock(obj);
virObjectUnref(obj);
virObjectRWUnlock(interfaces);
}
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 4de0cc5333..b0aa350d95 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4085,7 +4085,7 @@ testInterfaceUndefine(virInterfacePtr iface)
return -1;
virInterfaceObjListRemove(privconn->ifaces, obj);
- virObjectUnref(obj);
+ virInterfaceObjEndAPI(&obj);
return 0;
}
--
2.13.6
10:53 a.m.
New subject: [libvirt] [PATCH 3/4] nodedev: Return with locked obj from virNodeDeviceObjListRemove
Rather than unlock the object that was expected to be locked on
input, let the caller perform the unlock or more succinctly a
virNodeDeviceObjEndAPI on the object which will perform the Unref
and Unlock and clear the @obj.
Also add comments for virNodeDeviceObjListRemove.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virnodedeviceobj.c | 13 ++++++++++++-
src/node_device/node_device_hal.c | 4 ++--
src/node_device/node_device_udev.c | 2 +-
src/test/test_driver.c | 4 +---
4 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/src/conf/virnodedeviceobj.c b/src/conf/virnodedeviceobj.c
index ad0f27ee47..a5939c5de5 100644
--- a/src/conf/virnodedeviceobj.c
+++ b/src/conf/virnodedeviceobj.c
@@ -470,6 +470,18 @@ virNodeDeviceObjListAssignDef(virNodeDeviceObjListPtr devs,
}
+/*
+ * virNodeDeviceObjListRemove:
+ * @devs: list of node device objects
+ * @obj: a node device object
+ *
+ * Remove @obj from the node device obj list hash table. The caller must hold
+ * the lock on @obj to ensure no one else is either waiting for @obj or
+ * still using it.
+ *
+ * Upon return the @obj remains locked with at least 1 reference and
+ * the caller is expected to use virNodeDeviceObjEndAPI on it.
+ */
void
virNodeDeviceObjListRemove(virNodeDeviceObjListPtr devs,
virNodeDeviceObjPtr obj)
@@ -485,7 +497,6 @@ virNodeDeviceObjListRemove(virNodeDeviceObjListPtr devs,
virObjectRWLockWrite(devs);
virObjectLock(obj);
virHashRemoveEntry(devs->objs, def->name);
- virObjectUnlock(obj);
virObjectUnref(obj);
virObjectRWUnlock(devs);
}
diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c
index 6ad56f4166..8aac806a64 100644
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -512,7 +512,7 @@ dev_refresh(const char *udi)
* to sub-capabilities (like net.80203) is nasty ... so avoid it.
*/
virNodeDeviceObjListRemove(driver->devs, obj);
- virObjectUnref(obj);
+ virNodeDeviceObjEndAPI(&obj);
dev_create(udi);
} else {
VIR_DEBUG("no device named %s", name);
@@ -541,7 +541,7 @@ device_removed(LibHalContext *ctx ATTRIBUTE_UNUSED,
virNodeDeviceObjListRemove(driver->devs, obj);
else
VIR_DEBUG("no device named %s", name);
- virObjectUnref(obj);
+ virNodeDeviceObjEndAPI(&obj);
}
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index e87eb32a85..1151b04ecb 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1298,7 +1298,7 @@ udevRemoveOneDevice(struct udev_device *device)
VIR_DEBUG("Removing device '%s' with sysfs path '%s'",
def->name, name);
virNodeDeviceObjListRemove(driver->devs, obj);
- virObjectUnref(obj);
+ virNodeDeviceObjEndAPI(&obj);
if (event)
virObjectEventStateQueue(driver->nodeDeviceEventState, event);
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index b0aa350d95..a7aaabe09b 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4716,7 +4716,7 @@ testDestroyVport(testDriverPtr privconn,
0);
virNodeDeviceObjListRemove(privconn->devs, obj);
- virObjectUnref(obj);
+ virNodeDeviceObjEndAPI(&obj);
testObjectEventQueue(privconn, event);
return 0;
@@ -5695,8 +5695,6 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
virObjectLock(obj);
virNodeDeviceObjListRemove(driver->devs, obj);
- virObjectUnref(obj);
- obj = NULL;
cleanup:
virNodeDeviceObjEndAPI(&obj);
--
2.13.6
10:53 a.m.
New subject: [libvirt] [PATCH 4/4] storagepool: Return with locked obj from virStoragePoolObjRemove
Rather than unlock the object that was expected to be locked on
input, let the caller perform the unlock or more succinctly a
virStoragePoolObjEndAPI on the object which will perform the Unref
and Unlock and clear the @obj.
Also add comments for virStoragePoolObjRemove.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virstorageobj.c | 17 ++++++++++++++---
src/storage/storage_driver.c | 12 +-----------
src/test/test_driver.c | 14 +++-----------
3 files changed, 18 insertions(+), 25 deletions(-)
diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c
index 799b8c9fa3..c057c6b3ed 100644
--- a/src/conf/virstorageobj.c
+++ b/src/conf/virstorageobj.c
@@ -517,6 +517,18 @@ virStoragePoolObjListSearch(virStoragePoolObjListPtr pools,
}
+/*
+ * virStoragePoolObjRemove:
+ * @pools: list of storage pool objects
+ * @obj: a storage pool object
+ *
+ * Remove @obj from the storage pool obj list hash tables. The caller must
+ * hold the lock on @obj to ensure no one else is either waiting for @obj or
+ * still using it.
+ *
+ * Upon return the @obj remains locked with at least 1 reference and
+ * the caller is expected to use virStoragePoolObjEndAPI on it.
+ */
void
virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
virStoragePoolObjPtr obj)
@@ -530,7 +542,6 @@ virStoragePoolObjRemove(virStoragePoolObjListPtr pools,
virObjectLock(obj);
virHashRemoveEntry(pools->objs, uuidstr);
virHashRemoveEntry(pools->objsName, obj->def->name);
- virObjectUnlock(obj);
virObjectUnref(obj);
virObjectRWUnlock(pools);
}
@@ -1138,13 +1149,13 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools,
VIR_FREE(obj->configFile); /* for driver reload */
if (VIR_STRDUP(obj->configFile, path) < 0) {
virStoragePoolObjRemove(pools, obj);
- virObjectUnref(obj);
+ virStoragePoolObjEndAPI(&obj);
return NULL;
}
VIR_FREE(obj->autostartLink); /* for driver reload */
if (VIR_STRDUP(obj->autostartLink, autostartLink) < 0) {
virStoragePoolObjRemove(pools, obj);
- virObjectUnref(obj);
+ virStoragePoolObjEndAPI(&obj);
return NULL;
}
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index d5e38af5aa..b2946d0509 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -94,7 +94,7 @@ virStoragePoolUpdateInactive(virStoragePoolObjPtr *objptr)
if (!virStoragePoolObjGetConfigFile(obj)) {
virStoragePoolObjRemove(driver->pools, obj);
- virObjectUnref(obj);
+ virStoragePoolObjEndAPI(&obj);
*objptr = NULL;
} else if (virStoragePoolObjGetNewDef(obj)) {
virStoragePoolObjDefUseNewDef(obj);
@@ -746,8 +746,6 @@ storagePoolCreateXML(virConnectPtr conn,
(flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD)) {
if (backend->buildPool(obj, build_flags) < 0) {
virStoragePoolObjRemove(driver->pools, obj);
- virObjectUnref(obj);
- obj = NULL;
goto cleanup;
}
}
@@ -756,8 +754,6 @@ storagePoolCreateXML(virConnectPtr conn,
if (backend->startPool &&
backend->startPool(obj) < 0) {
virStoragePoolObjRemove(driver->pools, obj);
- virObjectUnref(obj);
- obj = NULL;
goto cleanup;
}
@@ -771,8 +767,6 @@ storagePoolCreateXML(virConnectPtr conn,
if (backend->stopPool)
backend->stopPool(obj);
virStoragePoolObjRemove(driver->pools, obj);
- virObjectUnref(obj);
- obj = NULL;
goto cleanup;
}
@@ -833,8 +827,6 @@ storagePoolDefineXML(virConnectPtr conn,
if (virStoragePoolObjSaveDef(driver, obj, def) < 0) {
virStoragePoolObjRemove(driver->pools, obj);
- virObjectUnref(obj);
- obj = NULL;
goto cleanup;
}
@@ -901,8 +893,6 @@ storagePoolUndefine(virStoragePoolPtr pool)
VIR_INFO("Undefining storage pool '%s'", def->name);
virStoragePoolObjRemove(driver->pools, obj);
- virObjectUnref(obj);
- obj = NULL;
ret = 0;
cleanup:
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index a7aaabe09b..4b7f8c4244 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -4556,16 +4556,12 @@ testStoragePoolCreateXML(virConnectPtr conn,
def->source.adapter.data.fchost.wwnn,
def->source.adapter.data.fchost.wwpn) < 0) {
virStoragePoolObjRemove(privconn->pools, obj);
- virObjectUnref(obj);
- obj = NULL;
goto cleanup;
}
}
if (testStoragePoolObjSetDefaults(obj) == -1) {
virStoragePoolObjRemove(privconn->pools, obj);
- virObjectUnref(obj);
- obj = NULL;
goto cleanup;
}
@@ -4627,8 +4623,6 @@ testStoragePoolDefineXML(virConnectPtr conn,
if (testStoragePoolObjSetDefaults(obj) == -1) {
virStoragePoolObjRemove(privconn->pools, obj);
- virObjectUnref(obj);
- obj = NULL;
goto cleanup;
}
@@ -4658,7 +4652,7 @@ testStoragePoolUndefine(virStoragePoolPtr pool)
0);
virStoragePoolObjRemove(privconn->pools, obj);
- virObjectUnref(obj);
+ virStoragePoolObjEndAPI(&obj);
testObjectEventQueue(privconn, event);
return 0;
@@ -4750,11 +4744,9 @@ testStoragePoolDestroy(virStoragePoolPtr pool)
VIR_STORAGE_POOL_EVENT_STOPPED,
0);
- if (!(virStoragePoolObjGetConfigFile(obj))) {
+ if (!(virStoragePoolObjGetConfigFile(obj)))
virStoragePoolObjRemove(privconn->pools, obj);
- virObjectUnref(obj);
- obj = NULL;
- }
+
ret = 0;
cleanup:
--
2.13.6
2471
days inactive
2473
days old
6 comments
2 participants
participants (2)
-
Erik Skultety -
John Ferlan