From nobody Wed Apr 24 17:36:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1553003388415937.8476322226144; Tue, 19 Mar 2019 06:49:48 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 907DB66979; Tue, 19 Mar 2019 13:49:46 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 55B2F611C2; Tue, 19 Mar 2019 13:49:46 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 8D6DD181A13B; Tue, 19 Mar 2019 13:49:45 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x2JDnhSv012114 for ; Tue, 19 Mar 2019 09:49:43 -0400 Received: by smtp.corp.redhat.com (Postfix) id 1A39819C71; Tue, 19 Mar 2019 13:49:43 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.30]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9715B2854C for ; Tue, 19 Mar 2019 13:49:42 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Tue, 19 Mar 2019 14:49:34 +0100 Message-Id: <56439c2c0fbb69e9ba58fffbf34f1df92fdd7a5b.1553003328.git.mprivozn@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 1/3] DO NOT APPLY: Simple reproducer for virDomainObjListRemove X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Tue, 19 Mar 2019 13:49:47 +0000 (UTC) Content-Type: text/plain; charset="utf-8" This bug demonstrates itself for other objects too, but let's show it for domains. Assume that dummy.xml contains a valid domain definition. Then 1) virsh create dummy.xml && sleep 1 2) virsh destroy dummy & 3) virsh create ../dummy.xml Observe error: error: Failed to create domain from dummy.xml error: internal error: Duplicate key Signed-off-by: Michal Privoznik --- src/conf/virdomainobjlist.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index a814fc10a3..3631cf16d0 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -396,6 +396,7 @@ virDomainObjListRemove(virDomainObjListPtr doms, dom->removing =3D true; virObjectRef(dom); virObjectUnlock(dom); + sleep(60); virObjectRWLockWrite(doms); virObjectLock(dom); virDomainObjListRemoveLocked(doms, dom); --=20 2.19.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Wed Apr 24 17:36:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1553003395841558.4085238348005; Tue, 19 Mar 2019 06:49:55 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0B78881E10; Tue, 19 Mar 2019 13:49:54 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D3E49601A4; Tue, 19 Mar 2019 13:49:53 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 95CA43FA4A; Tue, 19 Mar 2019 13:49:53 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x2JDnhKh012120 for ; Tue, 19 Mar 2019 09:49:43 -0400 Received: by smtp.corp.redhat.com (Postfix) id E15AE282FC; Tue, 19 Mar 2019 13:49:43 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.30]) by smtp.corp.redhat.com (Postfix) with ESMTP id 688BE19C71 for ; Tue, 19 Mar 2019 13:49:43 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Tue, 19 Mar 2019 14:49:35 +0100 Message-Id: <66da3d9965c32581adcb331c1c674a219e137fdd.1553003328.git.mprivozn@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 2/3] virDomainObjListAddLocked: Produce better error message than 'Duplicate key' X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Tue, 19 Mar 2019 13:49:54 +0000 (UTC) Content-Type: text/plain; charset="utf-8" If there are two concurrent threads, one of which is removing a domain from the list and the other is trying to add it back they may serialize in the following order: 1) vm->removing is set and @vm is unlocked. 2) The tread that's trying to add the domain onto the list locks the list and tries to find, if the domain already exists. 3) Our lookup functions say it doesn't, so the thread proceeds to virHashAddEntry() which fails with 'Duplicate key' error. This is obviously not helpful error message at all. The problem lies in our lookup functions (virDomainObjListFindByUUIDLocked() and virDomainObjListFindByNameLocked()) which return NULL even if the object is still on the list. They do this so that the object is not mistakenly looked up by some driver. The fix consists of moving 'removing' check one level up and thus allowing virDomainObjListAddLocked() to produce meaningful error message. Signed-off-by: Michal Privoznik Reviewed-by: Cole Robinson --- src/conf/virdomainobjlist.c | 35 +++++++++++++++++++++-------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 3631cf16d0..23734ad815 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -141,14 +141,9 @@ virDomainObjListFindByUUIDLocked(virDomainObjListPtr d= oms, =20 virUUIDFormat(uuid, uuidstr); obj =3D virHashLookup(doms->objs, uuidstr); - virObjectRef(obj); if (obj) { + virObjectRef(obj); virObjectLock(obj); - if (obj->removing) { - virObjectUnlock(obj); - virObjectUnref(obj); - obj =3D NULL; - } } return obj; } @@ -172,6 +167,12 @@ virDomainObjListFindByUUID(virDomainObjListPtr doms, obj =3D virDomainObjListFindByUUIDLocked(doms, uuid); virObjectRWUnlock(doms); =20 + if (obj && obj->removing) { + virObjectUnlock(obj); + virObjectUnref(obj); + obj =3D NULL; + } + return obj; } =20 @@ -183,14 +184,9 @@ virDomainObjListFindByNameLocked(virDomainObjListPtr d= oms, virDomainObjPtr obj; =20 obj =3D virHashLookup(doms->objsName, name); - virObjectRef(obj); if (obj) { + virObjectRef(obj); virObjectLock(obj); - if (obj->removing) { - virObjectUnlock(obj); - virObjectUnref(obj); - obj =3D NULL; - } } return obj; } @@ -214,6 +210,12 @@ virDomainObjListFindByName(virDomainObjListPtr doms, obj =3D virDomainObjListFindByNameLocked(doms, name); virObjectRWUnlock(doms); =20 + if (obj && obj->removing) { + virObjectUnlock(obj); + virObjectUnref(obj); + obj =3D NULL; + } + return obj; } =20 @@ -285,8 +287,13 @@ virDomainObjListAddLocked(virDomainObjListPtr doms, =20 /* See if a VM with matching UUID already exists */ if ((vm =3D virDomainObjListFindByUUIDLocked(doms, def->uuid))) { - /* UUID matches, but if names don't match, refuse it */ - if (STRNEQ(vm->def->name, def->name)) { + if (vm->removing) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("domain '%s' is already being removed"), + vm->def->name); + goto error; + } else if (STRNEQ(vm->def->name, def->name)) { + /* UUID matches, but if names don't match, refuse it */ virUUIDFormat(vm->def->uuid, uuidstr); virReportError(VIR_ERR_OPERATION_FAILED, _("domain '%s' is already defined with uuid %s"= ), --=20 2.19.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Wed Apr 24 17:36:49 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1553003388556751.3136938451701; Tue, 19 Mar 2019 06:49:48 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9BE54C057F50; Tue, 19 Mar 2019 13:49:46 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4D5A12854E; Tue, 19 Mar 2019 13:49:46 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id A630B3FA46; Tue, 19 Mar 2019 13:49:45 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x2JDniVp012128 for ; Tue, 19 Mar 2019 09:49:44 -0400 Received: by smtp.corp.redhat.com (Postfix) id B2CAE19C71; Tue, 19 Mar 2019 13:49:44 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.30]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3B31F28550 for ; Tue, 19 Mar 2019 13:49:44 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Tue, 19 Mar 2019 14:49:36 +0100 Message-Id: <2d616eae1f7402181674739d94937e2852cffdf1.1553003328.git.mprivozn@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 3/3] virNWFilterBindingObjListAddLocked: Produce better error message than 'Duplicate key' X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Tue, 19 Mar 2019 13:49:47 +0000 (UTC) Content-Type: text/plain; charset="utf-8" If there are two concurrent threads, one of which is removing an nwfilter from the list and the other is trying to add it back they may serialize in the following order: 1) obj->removing is set and @obj is unlocked. 2) The tread that's trying to add the nwfilter onto the list locks the list and tries to find, if the nwfilter already exists. 3) Our lookup functions say it doesn't, so the thread proceeds to virHashAddEntry() which fails with 'Duplicate key' error. This is obviously not helpful error message at all. The problem lies in our lookup function (virNWFilterBindingObjListFindByPortDevLocked()) which return NULL even if the object is still on the list. They do this so that the object is not mistakenly looked up by some API. The fix consists of moving 'removing' check one level up and thus allowing virNWFilterBindingObjListAddLocked() to produce meaningful error message. Signed-off-by: Michal Privoznik Reviewed-by: Cole Robinson --- src/conf/virnwfilterbindingobjlist.c | 29 +++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/conf/virnwfilterbindingobjlist.c b/src/conf/virnwfilterbin= dingobjlist.c index 06ccbf53af..dbeee0d55e 100644 --- a/src/conf/virnwfilterbindingobjlist.c +++ b/src/conf/virnwfilterbindingobjlist.c @@ -91,14 +91,9 @@ virNWFilterBindingObjListFindByPortDevLocked(virNWFilter= BindingObjListPtr bindin virNWFilterBindingObjPtr obj; =20 obj =3D virHashLookup(bindings->objs, name); - virObjectRef(obj); if (obj) { + virObjectRef(obj); virObjectLock(obj); - if (virNWFilterBindingObjGetRemoving(obj)) { - virObjectUnlock(obj); - virObjectUnref(obj); - obj =3D NULL; - } } return obj; } @@ -122,6 +117,12 @@ virNWFilterBindingObjListFindByPortDev(virNWFilterBind= ingObjListPtr bindings, obj =3D virNWFilterBindingObjListFindByPortDevLocked(bindings, name); virObjectRWUnlock(bindings); =20 + if (obj && virNWFilterBindingObjGetRemoving(obj)) { + virObjectUnlock(obj); + virObjectUnref(obj); + obj =3D NULL; + } + return obj; } =20 @@ -169,11 +170,17 @@ virNWFilterBindingObjListAddLocked(virNWFilterBinding= ObjListPtr bindings, virNWFilterBindingObjPtr binding; =20 /* See if a binding with matching portdev already exists */ - if ((binding =3D virNWFilterBindingObjListFindByPortDevLocked( - bindings, def->portdevname))) { - virReportError(VIR_ERR_OPERATION_FAILED, - _("binding '%s' already exists"), - def->portdevname); + binding =3D virNWFilterBindingObjListFindByPortDevLocked(bindings, def= ->portdevname); + if (binding) { + if (virNWFilterBindingObjGetRemoving(binding)) { + virReportError(VIR_ERR_OPERATION_FAILED, + _("binding '%s' is already being removed"), + def->portdevname); + } else { + virReportError(VIR_ERR_OPERATION_FAILED, + _("binding '%s' already exists"), + def->portdevname); + } goto error; } =20 --=20 2.19.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list