|
 |
9119d9 |
From eeb166cea4a966f01d9241eba1d465d8466e7768 Mon Sep 17 00:00:00 2001
|
|
|
9119d9 |
Message-Id: <eeb166cea4a966f01d9241eba1d465d8466e7768@dist-git>
|
|
|
9119d9 |
From: Martin Kletzander <mkletzan@redhat.com>
|
|
|
9119d9 |
Date: Sat, 13 Dec 2014 10:10:52 +0100
|
|
|
9119d9 |
Subject: [PATCH] qemu: avoid rare race when undefining domain
|
|
|
9119d9 |
|
|
|
9119d9 |
When one domain is being undefined and at the same time started, for
|
|
|
9119d9 |
example, there is a possibility of a rare problem occuring.
|
|
|
9119d9 |
|
|
|
9119d9 |
- Thread 1 does virDomainUndefine(), has the lock, checks that the
|
|
|
9119d9 |
domain is active and because it's not, calls
|
|
|
9119d9 |
virDomainObjListRemove().
|
|
|
9119d9 |
|
|
|
9119d9 |
- Thread 2 does virDomainCreate() and tries to lock the domain.
|
|
|
9119d9 |
|
|
|
9119d9 |
- Thread 1 needs to lock domain list in order to remove the domain from
|
|
|
9119d9 |
it, but must unlock domain first (proper order is to lock domain list
|
|
|
9119d9 |
first and the domain itself second).
|
|
|
9119d9 |
|
|
|
9119d9 |
- Thread 2 grabs the lock, starts the domain and releases the lock.
|
|
|
9119d9 |
|
|
|
9119d9 |
- Thread 1 grabs the lock and removes the domain from list.
|
|
|
9119d9 |
|
|
|
9119d9 |
With this patch:
|
|
|
9119d9 |
|
|
|
9119d9 |
- The undefining domain gets marked as "to undefine" before it is
|
|
|
9119d9 |
unlocked.
|
|
|
9119d9 |
|
|
|
9119d9 |
- If domain is found in any of the search APIs, it's returned only if
|
|
|
9119d9 |
it is not marked as "to undefine". The check is done while the
|
|
|
9119d9 |
domain is locked.
|
|
|
9119d9 |
|
|
|
9119d9 |
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1150505
|
|
|
9119d9 |
|
|
|
9119d9 |
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
|
|
|
9119d9 |
(cherry picked from commit c7d1c139ca3402e875002753952e80ce8054374e)
|
|
|
9119d9 |
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
|
|
|
9119d9 |
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
|
|
|
9119d9 |
---
|
|
|
9119d9 |
src/conf/domain_conf.c | 22 +++++++++++++++++++---
|
|
|
9119d9 |
src/conf/domain_conf.h | 1 +
|
|
|
9119d9 |
2 files changed, 20 insertions(+), 3 deletions(-)
|
|
|
9119d9 |
|
|
|
9119d9 |
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
|
|
|
9119d9 |
index 25f20f8..20ae4e7 100644
|
|
|
9119d9 |
--- a/src/conf/domain_conf.c
|
|
|
9119d9 |
+++ b/src/conf/domain_conf.c
|
|
|
9119d9 |
@@ -1056,8 +1056,13 @@ virDomainObjPtr virDomainObjListFindByID(virDomainObjListPtr doms,
|
|
|
9119d9 |
virDomainObjPtr obj;
|
|
|
9119d9 |
virObjectLock(doms);
|
|
|
9119d9 |
obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id;;
|
|
|
9119d9 |
- if (obj)
|
|
|
9119d9 |
+ if (obj) {
|
|
|
9119d9 |
virObjectLock(obj);
|
|
|
9119d9 |
+ if (obj->removing) {
|
|
|
9119d9 |
+ virObjectUnlock(obj);
|
|
|
9119d9 |
+ obj = NULL;
|
|
|
9119d9 |
+ }
|
|
|
9119d9 |
+ }
|
|
|
9119d9 |
virObjectUnlock(doms);
|
|
|
9119d9 |
return obj;
|
|
|
9119d9 |
}
|
|
|
9119d9 |
@@ -1073,8 +1078,13 @@ virDomainObjPtr virDomainObjListFindByUUID(virDomainObjListPtr doms,
|
|
|
9119d9 |
virUUIDFormat(uuid, uuidstr);
|
|
|
9119d9 |
|
|
|
9119d9 |
obj = virHashLookup(doms->objs, uuidstr);
|
|
|
9119d9 |
- if (obj)
|
|
|
9119d9 |
+ if (obj) {
|
|
|
9119d9 |
virObjectLock(obj);
|
|
|
9119d9 |
+ if (obj->removing) {
|
|
|
9119d9 |
+ virObjectUnlock(obj);
|
|
|
9119d9 |
+ obj = NULL;
|
|
|
9119d9 |
+ }
|
|
|
9119d9 |
+ }
|
|
|
9119d9 |
virObjectUnlock(doms);
|
|
|
9119d9 |
return obj;
|
|
|
9119d9 |
}
|
|
|
9119d9 |
@@ -1099,8 +1109,13 @@ virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
|
|
|
9119d9 |
virDomainObjPtr obj;
|
|
|
9119d9 |
virObjectLock(doms);
|
|
|
9119d9 |
obj = virHashSearch(doms->objs, virDomainObjListSearchName, name);
|
|
|
9119d9 |
- if (obj)
|
|
|
9119d9 |
+ if (obj) {
|
|
|
9119d9 |
virObjectLock(obj);
|
|
|
9119d9 |
+ if (obj->removing) {
|
|
|
9119d9 |
+ virObjectUnlock(obj);
|
|
|
9119d9 |
+ obj = NULL;
|
|
|
9119d9 |
+ }
|
|
|
9119d9 |
+ }
|
|
|
9119d9 |
virObjectUnlock(doms);
|
|
|
9119d9 |
return obj;
|
|
|
9119d9 |
}
|
|
|
9119d9 |
@@ -2537,6 +2552,7 @@ void virDomainObjListRemove(virDomainObjListPtr doms,
|
|
|
9119d9 |
{
|
|
|
9119d9 |
char uuidstr[VIR_UUID_STRING_BUFLEN];
|
|
|
9119d9 |
|
|
|
9119d9 |
+ dom->removing = true;
|
|
|
9119d9 |
virUUIDFormat(dom->def->uuid, uuidstr);
|
|
|
9119d9 |
virObjectRef(dom);
|
|
|
9119d9 |
virObjectUnlock(dom);
|
|
|
9119d9 |
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
|
|
|
9119d9 |
index 04cee80..01d5aeb 100644
|
|
|
9119d9 |
--- a/src/conf/domain_conf.h
|
|
|
9119d9 |
+++ b/src/conf/domain_conf.h
|
|
|
9119d9 |
@@ -2149,6 +2149,7 @@ struct _virDomainObj {
|
|
|
9119d9 |
unsigned int autostart : 1;
|
|
|
9119d9 |
unsigned int persistent : 1;
|
|
|
9119d9 |
unsigned int updated : 1;
|
|
|
9119d9 |
+ unsigned int removing : 1;
|
|
|
9119d9 |
|
|
|
9119d9 |
virDomainDefPtr def; /* The current definition */
|
|
|
9119d9 |
virDomainDefPtr newDef; /* New definition to activate at shutdown */
|
|
|
9119d9 |
--
|
|
|
9119d9 |
2.2.0
|
|
|
9119d9 |
|