|
 |
c313de |
From 874617d278b58554feee567b2e79d208aaef214c Mon Sep 17 00:00:00 2001
|
|
|
c313de |
Message-Id: <874617d278b58554feee567b2e79d208aaef214c@dist-git>
|
|
|
c313de |
From: Michal Privoznik <mprivozn@redhat.com>
|
|
|
c313de |
Date: Tue, 6 Aug 2019 13:29:22 +0200
|
|
|
c313de |
Subject: [PATCH] virDomainObjListAddLocked: Produce better error message than
|
|
|
c313de |
'Duplicate key'
|
|
|
c313de |
MIME-Version: 1.0
|
|
|
c313de |
Content-Type: text/plain; charset=UTF-8
|
|
|
c313de |
Content-Transfer-Encoding: 8bit
|
|
|
c313de |
|
|
|
c313de |
If there are two concurrent threads, one of which is removing a
|
|
|
c313de |
domain from the list and the other is trying to add it back they
|
|
|
c313de |
may serialize in the following order:
|
|
|
c313de |
|
|
|
c313de |
1) vm->removing is set and @vm is unlocked.
|
|
|
c313de |
2) The tread that's trying to add the domain onto the list locks
|
|
|
c313de |
the list and tries to find, if the domain already exists.
|
|
|
c313de |
3) Our lookup functions say it doesn't, so the thread proceeds to
|
|
|
c313de |
virHashAddEntry() which fails with 'Duplicate key' error.
|
|
|
c313de |
|
|
|
c313de |
This is obviously not helpful error message at all.
|
|
|
c313de |
|
|
|
c313de |
The problem lies in our lookup functions
|
|
|
c313de |
(virDomainObjListFindByUUIDLocked() and
|
|
|
c313de |
virDomainObjListFindByNameLocked()) which return NULL even if the
|
|
|
c313de |
object is still on the list. They do this so that the object is
|
|
|
c313de |
not mistakenly looked up by some driver. The fix consists of
|
|
|
c313de |
moving 'removing' check one level up and thus allowing
|
|
|
c313de |
virDomainObjListAddLocked() to produce meaningful error message.
|
|
|
c313de |
|
|
|
c313de |
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
|
|
|
c313de |
Reviewed-by: Cole Robinson <crobinso@redhat.com>
|
|
|
c313de |
(cherry picked from commit a5c71129bf2c12a827f1bc00149acd1c572ffe9c)
|
|
|
c313de |
|
|
|
c313de |
Signed-off-by: Ján Tomko <jtomko@redhat.com>
|
|
|
c313de |
RHEL-8.1.0: https://bugzilla.redhat.com/show_bug.cgi?id=1737790
|
|
|
c313de |
Message-Id: <c63ca487bf56af5240e273b5a112a753c2bc85a3.1565090956.git.jtomko@redhat.com>
|
|
|
c313de |
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
|
|
c313de |
---
|
|
|
c313de |
src/conf/virdomainobjlist.c | 35 +++++++++++++++++++++--------------
|
|
|
c313de |
1 file changed, 21 insertions(+), 14 deletions(-)
|
|
|
c313de |
|
|
|
c313de |
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
|
|
|
c313de |
index e7c3e326ca..3b388e2ba2 100644
|
|
|
c313de |
--- a/src/conf/virdomainobjlist.c
|
|
|
c313de |
+++ b/src/conf/virdomainobjlist.c
|
|
|
c313de |
@@ -142,14 +142,9 @@ virDomainObjListFindByUUIDLocked(virDomainObjListPtr doms,
|
|
|
c313de |
|
|
|
c313de |
virUUIDFormat(uuid, uuidstr);
|
|
|
c313de |
obj = virHashLookup(doms->objs, uuidstr);
|
|
|
c313de |
- virObjectRef(obj);
|
|
|
c313de |
if (obj) {
|
|
|
c313de |
+ virObjectRef(obj);
|
|
|
c313de |
virObjectLock(obj);
|
|
|
c313de |
- if (obj->removing) {
|
|
|
c313de |
- virObjectUnlock(obj);
|
|
|
c313de |
- virObjectUnref(obj);
|
|
|
c313de |
- obj = NULL;
|
|
|
c313de |
- }
|
|
|
c313de |
}
|
|
|
c313de |
return obj;
|
|
|
c313de |
}
|
|
|
c313de |
@@ -173,6 +168,12 @@ virDomainObjListFindByUUID(virDomainObjListPtr doms,
|
|
|
c313de |
obj = virDomainObjListFindByUUIDLocked(doms, uuid);
|
|
|
c313de |
virObjectRWUnlock(doms);
|
|
|
c313de |
|
|
|
c313de |
+ if (obj && obj->removing) {
|
|
|
c313de |
+ virObjectUnlock(obj);
|
|
|
c313de |
+ virObjectUnref(obj);
|
|
|
c313de |
+ obj = NULL;
|
|
|
c313de |
+ }
|
|
|
c313de |
+
|
|
|
c313de |
return obj;
|
|
|
c313de |
}
|
|
|
c313de |
|
|
|
c313de |
@@ -184,14 +185,9 @@ virDomainObjListFindByNameLocked(virDomainObjListPtr doms,
|
|
|
c313de |
virDomainObjPtr obj;
|
|
|
c313de |
|
|
|
c313de |
obj = virHashLookup(doms->objsName, name);
|
|
|
c313de |
- virObjectRef(obj);
|
|
|
c313de |
if (obj) {
|
|
|
c313de |
+ virObjectRef(obj);
|
|
|
c313de |
virObjectLock(obj);
|
|
|
c313de |
- if (obj->removing) {
|
|
|
c313de |
- virObjectUnlock(obj);
|
|
|
c313de |
- virObjectUnref(obj);
|
|
|
c313de |
- obj = NULL;
|
|
|
c313de |
- }
|
|
|
c313de |
}
|
|
|
c313de |
return obj;
|
|
|
c313de |
}
|
|
|
c313de |
@@ -215,6 +211,12 @@ virDomainObjListFindByName(virDomainObjListPtr doms,
|
|
|
c313de |
obj = virDomainObjListFindByNameLocked(doms, name);
|
|
|
c313de |
virObjectRWUnlock(doms);
|
|
|
c313de |
|
|
|
c313de |
+ if (obj && obj->removing) {
|
|
|
c313de |
+ virObjectUnlock(obj);
|
|
|
c313de |
+ virObjectUnref(obj);
|
|
|
c313de |
+ obj = NULL;
|
|
|
c313de |
+ }
|
|
|
c313de |
+
|
|
|
c313de |
return obj;
|
|
|
c313de |
}
|
|
|
c313de |
|
|
|
c313de |
@@ -286,8 +288,13 @@ virDomainObjListAddLocked(virDomainObjListPtr doms,
|
|
|
c313de |
|
|
|
c313de |
/* See if a VM with matching UUID already exists */
|
|
|
c313de |
if ((vm = virDomainObjListFindByUUIDLocked(doms, def->uuid))) {
|
|
|
c313de |
- /* UUID matches, but if names don't match, refuse it */
|
|
|
c313de |
- if (STRNEQ(vm->def->name, def->name)) {
|
|
|
c313de |
+ if (vm->removing) {
|
|
|
c313de |
+ virReportError(VIR_ERR_OPERATION_FAILED,
|
|
|
c313de |
+ _("domain '%s' is already being removed"),
|
|
|
c313de |
+ vm->def->name);
|
|
|
c313de |
+ goto error;
|
|
|
c313de |
+ } else if (STRNEQ(vm->def->name, def->name)) {
|
|
|
c313de |
+ /* UUID matches, but if names don't match, refuse it */
|
|
|
c313de |
virUUIDFormat(vm->def->uuid, uuidstr);
|
|
|
c313de |
virReportError(VIR_ERR_OPERATION_FAILED,
|
|
|
c313de |
_("domain '%s' is already defined with uuid %s"),
|
|
|
c313de |
--
|
|
|
c313de |
2.22.1
|
|
|
c313de |
|