[libvirt] [PATCH 3/3] Avoid rare race when undefining domain
When one domain is being undefined and at the same time started, for
example, there is a possibility of a rare problem occuring.
- Thread 1 does virDomainUndefine(), has the lock, checks that the
domain is active and because it's not, calls
virDomainObjListRemove().
- Thread 2 does virDomainCreate() and tries to lock the domain.
- Thread 1 needs to lock domain list in order to remove the domain from
it, but must unlock domain first (proper order is to lock domain list
first and the domain itself second).
- Thread 2 grabs the lock, starts the domain and releases the lock.
- Thread 1 grabs the lock and removes the domain from list.
With this patch:
- The undefining domain gets marked as "to undefine" before it is
unlocked.
- If domain is found in any of the search APIs, it's returned only if
it is not marked as "to undefine". The check is done while the
domain is locked.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1150505
Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
---
src/conf/domain_conf.c | 23 ++++++++++++++++++++---
src/conf/domain_conf.h | 1 +
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 43574e1..b92a58a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1054,8 +1054,13 @@ virDomainObjPtr virDomainObjListFindByID(virDomainObjListPtr doms,
virDomainObjPtr obj;
virObjectLock(doms);
obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id);
- if (obj)
+ if (obj) {
virObjectLock(obj);
+ if (obj->undefining) {
+ virObjectUnlock(obj);
+ obj = NULL;
+ }
+ }
virObjectUnlock(doms);
return obj;
}
@@ -1071,8 +1076,13 @@ virDomainObjPtr virDomainObjListFindByUUID(virDomainObjListPtr
doms,
virUUIDFormat(uuid, uuidstr);
obj = virHashLookup(doms->objs, uuidstr);
- if (obj)
+ if (obj) {
virObjectLock(obj);
+ if (obj->undefining) {
+ virObjectUnlock(obj);
+ obj = NULL;
+ }
+ }
virObjectUnlock(doms);
return obj;
}
@@ -1097,8 +1107,13 @@ virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr
doms,
virDomainObjPtr obj;
virObjectLock(doms);
obj = virHashSearch(doms->objs, virDomainObjListSearchName, name);
- if (obj)
+ if (obj) {
virObjectLock(obj);
+ if (obj->undefining) {
+ virObjectUnlock(obj);
+ obj = NULL;
+ }
+ }
virObjectUnlock(doms);
return obj;
}
@@ -2538,6 +2553,7 @@ void virDomainObjListRemove(virDomainObjListPtr doms,
virUUIDFormat(dom->def->uuid, uuidstr);
virObjectRef(dom);
+ dom->undefining = true;
virObjectUnlock(dom);
VIR_WARN("NOW!!!");
@@ -2563,6 +2579,7 @@ void virDomainObjListRemoveLocked(virDomainObjListPtr doms,
char uuidstr[VIR_UUID_STRING_BUFLEN];
virUUIDFormat(dom->def->uuid, uuidstr);
+ dom->undefining = true;
virObjectUnlock(dom);
virHashRemoveEntry(doms->objs, uuidstr);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9908d88..1d28ed0 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2155,6 +2155,7 @@ struct _virDomainObj {
unsigned int autostart : 1;
unsigned int persistent : 1;
unsigned int updated : 1;
+ unsigned int undefining : 1; /* the domain is about to be undefined */
virDomainDefPtr def; /* The current definition */
virDomainDefPtr newDef; /* New definition to activate at shutdown */
--
2.1.2