4:19 p.m.
Current processing requires a "fire dance" unlocking the @obj,
adding an @obj ref, locking the @secrets, and relocking @obj
in order to ensure proper lock ordering.
This can be avoided by changing virSecretObjListRemove to take
a @uuidstr instead of @obj. Then, we can lock the @secrets list,
look up the @obj by @uuidstr (like we do when adding), and remove
the @obj from the list. This removes the last reference to the
object effectively reaping it.
NB: Since prior to calling we remove the reference to the object
we cannot pass anything contained within the object (such as the
obj->def) because it's possible that the object could be reaped
by two competing remove threads.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/conf/virsecretobj.c | 38 +++++++++++++++++---------------------
src/conf/virsecretobj.h | 2 +-
src/secret/secret_driver.c | 15 +++++++++------
3 files changed, 27 insertions(+), 28 deletions(-)
diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
index 4aaf47b5d..09f6ead64 100644
--- a/src/conf/virsecretobj.c
+++ b/src/conf/virsecretobj.c
@@ -284,32 +284,25 @@ virSecretObjListFindByUsage(virSecretObjListPtr secrets,
/*
* virSecretObjListRemove:
* @secrets: list of secret objects
- * @secret: a secret object
+ * @uuidstr: secret uuid to find
+ *
+ * Find the object by the @uuidstr in the list, remove the object from
+ * the list hash table, and free the 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.
+ * Upon entry it's expected that prior to entry any locks on
+ * the object related to @uuidstr will have been removed.
*/
void
virSecretObjListRemove(virSecretObjListPtr secrets,
- virSecretObjPtr obj)
+ const char *uuidstr)
{
- char uuidstr[VIR_UUID_STRING_BUFLEN];
- virSecretDefPtr def;
-
- if (!obj)
- return;
- def = obj->def;
-
- virUUIDFormat(def->uuid, uuidstr);
- virObjectRef(obj);
- virObjectUnlock(obj);
+ virSecretObjPtr obj;
virObjectRWLockWrite(secrets);
- virObjectLock(obj);
- virHashRemoveEntry(secrets->objs, uuidstr);
- virObjectUnlock(obj);
- virObjectUnref(obj);
+ if ((obj = virSecretObjListFindByUUIDLocked(secrets, uuidstr))) {
+ virHashRemoveEntry(secrets->objs, uuidstr);
+ virSecretObjEndAPI(&obj);
+ }
virObjectRWUnlock(secrets);
}
@@ -927,8 +920,11 @@ virSecretLoad(virSecretObjListPtr secrets,
def = NULL;
if (virSecretLoadValue(obj) < 0) {
- virSecretObjListRemove(secrets, obj);
- virObjectLock(obj);
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+ virUUIDFormat(obj->def->uuid, uuidstr);
+ virSecretObjEndAPI(&obj);
+ virSecretObjListRemove(secrets, uuidstr);
goto cleanup;
}
diff --git a/src/conf/virsecretobj.h b/src/conf/virsecretobj.h
index d412ee6a7..68bafc718 100644
--- a/src/conf/virsecretobj.h
+++ b/src/conf/virsecretobj.h
@@ -49,7 +49,7 @@ virSecretObjListFindByUsage(virSecretObjListPtr secrets,
void
virSecretObjListRemove(virSecretObjListPtr secrets,
- virSecretObjPtr obj);
+ const char *uuidstr);
virSecretObjPtr
virSecretObjListAdd(virSecretObjListPtr secrets,
diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index 23a3c9bda..75bc2b0cf 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -270,9 +270,11 @@ secretDefineXML(virConnectPtr conn,
virSecretObjSetDef(obj, backup);
VIR_STEAL_PTR(def, objDef);
} else {
- virSecretObjListRemove(driver->secrets, obj);
- virObjectUnref(obj);
- obj = NULL;
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+ virUUIDFormat(objDef->uuid, uuidstr);
+ virSecretObjEndAPI(&obj);
+ virSecretObjListRemove(driver->secrets, uuidstr);
}
cleanup:
@@ -392,6 +394,7 @@ secretUndefine(virSecretPtr secret)
int ret = -1;
virSecretObjPtr obj;
virSecretDefPtr def;
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
virObjectEventPtr event = NULL;
if (!(obj = secretObjFromSecret(secret)))
@@ -412,9 +415,9 @@ secretUndefine(virSecretPtr secret)
virSecretObjDeleteData(obj);
- virSecretObjListRemove(driver->secrets, obj);
- virObjectUnref(obj);
- obj = NULL;
+ virUUIDFormat(def->uuid, uuidstr);
+ virSecretObjEndAPI(&obj);
+ virSecretObjListRemove(driver->secrets, uuidstr);
ret = 0;
--
2.13.6