From nobody Sun Feb 8 19:59:38 2026 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 1552645448999605.0182505483523; Fri, 15 Mar 2019 03:24:08 -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 1335481DF6; Fri, 15 Mar 2019 10:24:07 +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 DEF826A979; Fri, 15 Mar 2019 10:24:06 +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 94C80181A009; Fri, 15 Mar 2019 10:24:06 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x2FAO3Rw009035 for ; Fri, 15 Mar 2019 06:24:03 -0400 Received: by smtp.corp.redhat.com (Postfix) id 518B717CEB; Fri, 15 Mar 2019 10:24:03 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.30]) by smtp.corp.redhat.com (Postfix) with ESMTP id CE880614C0 for ; Fri, 15 Mar 2019 10:24:02 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Fri, 15 Mar 2019 11:23:54 +0100 Message-Id: <6f969844f8458e9f7db9935abf8628deacf220fa.1552645270.git.mprivozn@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v3 4/5] qemu_hotplug: Fix a rare race condition when detaching a device twice 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.25]); Fri, 15 Mar 2019 10:24:07 +0000 (UTC) Content-Type: text/plain; charset="utf-8" https://bugzilla.redhat.com/show_bug.cgi?id=3D1623389 If a device is detached twice from the same domain the following race condition may happen: 1) The first DetachDevice() call will issue "device_del" on qemu monitor, but since the DEVICE_DELETED event did not arrive in time, the API ends claiming "Device detach request sent successfully". 2) The second DetachDevice() therefore still find the device in the domain and thus proceeds to detaching it again. It calls EnterMonitor() and qemuMonitorSend() trying to issue "device_del" command again. This gets both domain lock and monitor lock released. 3) At this point, qemu sends us the DEVICE_DELETED event which is going to be handled by the event loop which ends up calling qemuDomainSignalDeviceRemoval() to determine who is going to remove the device from domain definition. Whether it is the caller that marked the device for removal or whether it is going to be the event processing thread. 4) Because the device was marked for removal, qemuDomainSignalDeviceRemoval() returns true, which means the event is to be processed by the thread that has marked the device for removal (and is currently still trying to issue "device_del" command) 5) The thread finally issues the "device_del" command, which fails (obviously) and therefore it calls qemuDomainResetDeviceRemoval() to reset the device marking and quits immediately after, NOT removing any device from the domain definition. At this point, the device is still present in the domain definition but doesn't exist in qemu anymore. Worse, there is no way to remove it from the domain definition. Solution is to note down that we've seen the event and if the second "device_del" fails, not take it as a failure but carry on with the usual execution. Signed-off-by: Michal Privoznik --- src/qemu/qemu_domain.h | 1 + src/qemu/qemu_hotplug.c | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 9f468e5661..fb361515ba 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -218,6 +218,7 @@ typedef qemuDomainUnpluggingDevice *qemuDomainUnpluggin= gDevicePtr; struct _qemuDomainUnpluggingDevice { const char *alias; qemuDomainUnpluggingDeviceStatus status; + bool eventSeen; /* True if DEVICE_DELETED event arrived. */ }; =20 =20 diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index c4a0971a65..99abf051bd 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -67,6 +67,9 @@ VIR_LOG_INIT("qemu.qemu_hotplug"); unsigned long long qemuDomainRemoveDeviceWaitTime =3D 1000ull * 5; =20 =20 +static void +qemuDomainResetDeviceRemoval(virDomainObjPtr vm); + /** * qemuDomainDeleteDevice: * @vm: domain object @@ -104,8 +107,31 @@ qemuDomainDeleteDevice(virDomainObjPtr vm, virObjectLock(priv->mon); } =20 - if (qemuDomainObjExitMonitor(driver, vm) < 0) - rc =3D -1; + if (qemuDomainObjExitMonitor(driver, vm) < 0) { + /* Domain is no longer running. No cleanup needed. */ + return -1; + } + + if (rc < 0) { + /* Deleting device failed. Let's check if DEVICE_DELETED + * even arrived. If it did, we need to claim success to + * make the caller remove device from domain XML. */ + + if (priv->unplug.eventSeen) { + /* The event arrived. Return success. */ + VIR_DEBUG("Detaching of device %s failed, but event arrived", = alias); + qemuDomainResetDeviceRemoval(vm); + rc =3D 0; + } else if (rc =3D=3D -2) { + /* The device does not exist in qemu, but it still + * exists in libvirt. Claim success to make caller + * qemuDomainWaitForDeviceRemoval(). Otherwise if + * domain XML is queried right after detach API the + * device would still be there. */ + VIR_DEBUG("Detaching of device %s failed and no event arrived"= , alias); + rc =3D 0; + } + } =20 return rc; } @@ -5187,6 +5213,7 @@ qemuDomainResetDeviceRemoval(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv =3D vm->privateData; priv->unplug.alias =3D NULL; + priv->unplug.eventSeen =3D false; } =20 /* Returns: @@ -5246,6 +5273,7 @@ qemuDomainSignalDeviceRemoval(virDomainObjPtr vm, VIR_DEBUG("Removal of device '%s' continues in waiting thread", de= vAlias); qemuDomainResetDeviceRemoval(vm); priv->unplug.status =3D status; + priv->unplug.eventSeen =3D true; virDomainObjBroadcast(vm); return true; } --=20 2.19.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list