From nobody Tue Apr 16 21:11:40 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 ARC-Seal: i=1; a=rsa-sha256; t=1570526167; cv=none; d=zoho.com; s=zohoarc; b=hmZ5s9KUmvS3K0qjHXqpglk9QEsf0NzWBakLT2EnaxLwF9DuQyJsgk2EOUZyjTM68lfdSpKstyQiuRwZwToNCOuJ7bpJ7Mj3XbSU4W0CdL68MWzOrhpEEWIe3BxXuglLSBDtFwUEn79a0H2SyTtc6a9OetfeyNxZbAyoD0hyGO0= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1570526167; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=JhYP1SnSzesqJ/0Y9sUKDzf79uXrjIn7ndkHW7JdSoo=; b=AW1Ajmn9eONwokCTebvbHxWx54ZaMAsvvFycdIvnFAVatjuvdTzp/o/5S8tOe6fmllpRy01PTVNSXTk5d41sHzD+ETlLN5Vuhjb/AjgboIBOS5gGq9GB4GByiVuQ4Uvk530vPAg3bpoG1dbg41p0b7aGGBS4aBIqFu9uIdWAGac= ARC-Authentication-Results: i=1; mx.zoho.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 header.from= (p=none dis=none) header.from= Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1570526167510865.003008812936; Tue, 8 Oct 2019 02:16:07 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8E10A4E938; Tue, 8 Oct 2019 09:16:05 +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 215175C1D4; Tue, 8 Oct 2019 09:16:05 +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 6911D1803518; Tue, 8 Oct 2019 09:16:02 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x989G0i9015472 for ; Tue, 8 Oct 2019 05:16:00 -0400 Received: by smtp.corp.redhat.com (Postfix) id DA3EC5D772; Tue, 8 Oct 2019 09:16:00 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.30]) by smtp.corp.redhat.com (Postfix) with ESMTP id C20565D70D; Tue, 8 Oct 2019 09:15:59 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Tue, 8 Oct 2019 11:15:51 +0200 Message-Id: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-loop: libvir-list@redhat.com Cc: xuyandong2@huawei.com, danielhb413@gmail.com, zhang.zhanghailiang@huawei.com Subject: [libvirt] [PATCH 1/2] qemu: Fix @vm locking issue when connecting to the monitor 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.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Tue, 08 Oct 2019 09:16:06 +0000 (UTC) Content-Type: text/plain; charset="utf-8" When connecting to qemu's monitor the @vm object is unlocked. This is justified - connecting may take a long time and we don't want to wait with the domain object locked. However, just before the domain object is locked again, the monitor's FD is registered in the event loop. Therefore, there is a small window where the event loop has a chance to call a handler for an event that occurred on the monitor FD but vm is not initalized properly just yet (i.e. priv->mon is not set). For instance, if there's an incoming migration, qemu creates its socket but then fails to initialize (for various reasons, I'm reproducing this by using hugepages but leaving the HP pool empty) then the following may happen: 1) qemuConnectMonitor() unlocks @vm 2) qemuMonitorOpen() connects to the monitor socket and by calling qemuMonitorOpenInternal() which subsequently calls qemuMonitorRegister() the event handler is installed 3) qemu fails to initialize and exit()-s, which closes the monitor 4) The even loop sees EOF on the monitor and the control gets to qemuProcessEventHandler() which locks @vm and calls processMonitorEOFEvent() which then calls qemuMonitorLastError(priv->mon). But priv->mon is not set just yet. 5) qemuMonitorLastError() dereferences NULL pointer The solution is to unlock the domain object for a shorter time and most importantly, register event handler with domain object locked so that any possible event processing is done only after @vm's private data was properly initialized. This issue is also mentioned in v4.2.0-99-ga5a777a8ba. Since we are unlocking @vm and locking it back, another thread might have destroyed the domain meanwhile. Therefore we have to check if domain is still activem, and we have to do it at the same place where domain lock is acquired back, i.e. in qemuMonitorOpen(). This creates a small problem for our test suite which calls qemuMonitorOpen() directly and passes @vm which has no definition. This makes virDomainObjIsActive() call crash. Fortunately, allocating empty domain definition is sufficient. Signed-off-by: Michal Privoznik Reviewed-by: Daniel Henrique Barboza --- This is an alternative approach to: https://www.redhat.com/archives/libvir-list/2019-September/msg00749.html src/qemu/qemu_monitor.c | 33 +++++++++++++++++++++++++-------- src/qemu/qemu_process.c | 12 ------------ tests/qemumonitortestutils.c | 2 ++ 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 58de26a276..a53d3b1d60 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -810,35 +810,52 @@ qemuMonitorOpen(virDomainObjPtr vm, qemuMonitorCallbacksPtr cb, void *opaque) { - int fd; + int fd =3D -1; bool hasSendFD =3D false; - qemuMonitorPtr ret; + qemuMonitorPtr ret =3D NULL; =20 timeout +=3D QEMU_DEFAULT_MONITOR_WAIT; =20 + /* Hold an extra reference because we can't allow 'vm' to be + * deleted until the monitor gets its own reference. */ + virObjectRef(vm); + switch (config->type) { case VIR_DOMAIN_CHR_TYPE_UNIX: hasSendFD =3D true; - if ((fd =3D qemuMonitorOpenUnix(config->data.nix.path, - vm->pid, retry, timeout)) < 0) - return NULL; + virObjectUnlock(vm); + fd =3D qemuMonitorOpenUnix(config->data.nix.path, + vm->pid, retry, timeout); + virObjectLock(vm); break; =20 case VIR_DOMAIN_CHR_TYPE_PTY: - if ((fd =3D qemuMonitorOpenPty(config->data.file.path)) < 0) - return NULL; + virObjectUnlock(vm); + fd =3D qemuMonitorOpenPty(config->data.file.path); + virObjectLock(vm); break; =20 default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unable to handle monitor type: %s"), virDomainChrTypeToString(config->type)); - return NULL; + break; + } + + if (fd < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("domain is not running")); + goto cleanup; } =20 ret =3D qemuMonitorOpenInternal(vm, fd, hasSendFD, cb, opaque); + cleanup: if (!ret) VIR_FORCE_CLOSE(fd); + virObjectUnref(vm); return ret; } =20 diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a50cd54393..b303e5ba05 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1955,13 +1955,8 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomai= nObjPtr vm, int asyncJob, * 1GiB of guest RAM. */ timeout =3D vm->def->mem.total_memory / (1024 * 1024); =20 - /* Hold an extra reference because we can't allow 'vm' to be - * deleted until the monitor gets its own reference. */ - virObjectRef(vm); - ignore_value(virTimeMillisNow(&priv->monStart)); monConfig =3D virObjectRef(priv->monConfig); - virObjectUnlock(vm); =20 mon =3D qemuMonitorOpen(vm, monConfig, @@ -1978,15 +1973,8 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomai= nObjPtr vm, int asyncJob, qemuProcessMonitorLogFree); } =20 - virObjectLock(vm); virObjectUnref(monConfig); - virObjectUnref(vm); priv->monStart =3D 0; - - if (!virDomainObjIsActive(vm)) { - qemuMonitorClose(mon); - mon =3D NULL; - } priv->mon =3D mon; =20 if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0= ) { diff --git a/tests/qemumonitortestutils.c b/tests/qemumonitortestutils.c index e9dff123f8..c7580c5f28 100644 --- a/tests/qemumonitortestutils.c +++ b/tests/qemumonitortestutils.c @@ -1085,6 +1085,8 @@ qemuMonitorCommonTestNew(virDomainXMLOptionPtr xmlopt, test->vm =3D virDomainObjNew(xmlopt); if (!test->vm) goto error; + if (!(test->vm->def =3D virDomainDefNew())) + goto error; } =20 if (virNetSocketNewListenUNIX(path, 0700, geteuid(), getegid(), --=20 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list From nobody Tue Apr 16 21:11:40 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 ARC-Seal: i=1; a=rsa-sha256; t=1570526186; cv=none; d=zoho.com; s=zohoarc; b=aecHFkoNXWOicz8LkbLdtPq01lHIS3Hk8Jibezu51se+qhIUCKfunuvompfwLYoFLvs8FfIXHLi1R9AXy+ep06UxY1Fzc5x3cAaKJM1OYI7jtEGzUiKOz73x0s3RNj9/h2oFTOidRA+wl/k1oEwURGW6KZq/B/9VRTK2FEJ8OIM= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1570526186; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=yn1ochl+Z1vHfTm8L1kGbyg0s4edU4u9iRYYLu1CmeU=; b=FZbcl+zJ/I3DTtfu3pzNKRcvr98SO6mM1Aoxk1ixyljjOWYNpne2TZ6hPPIQI1QgfqGdrpxnY8939fkfM2Mqg3dthRdcLLu+5Pvv+qOo8Q6ZfMp0cn5gyyBbXsW9DTxK2WkCLaGapV0TOl7l8qXS9KXalNCHI8M9towGyJou4CE= ARC-Authentication-Results: i=1; mx.zoho.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 header.from= (p=none dis=none) header.from= Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1570526186124742.5970084188009; Tue, 8 Oct 2019 02:16:26 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2EC108AC6F9; Tue, 8 Oct 2019 09:16:24 +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 F3BA35D9E2; Tue, 8 Oct 2019 09:16:23 +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 ACD434EE96; Tue, 8 Oct 2019 09:16:23 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x989G2PF015477 for ; Tue, 8 Oct 2019 05:16:02 -0400 Received: by smtp.corp.redhat.com (Postfix) id 29DA35D772; Tue, 8 Oct 2019 09:16:02 +0000 (UTC) Received: from moe.brq.redhat.com (unknown [10.43.2.30]) by smtp.corp.redhat.com (Postfix) with ESMTP id 32F475D70D; Tue, 8 Oct 2019 09:16:01 +0000 (UTC) From: Michal Privoznik To: libvir-list@redhat.com Date: Tue, 8 Oct 2019 11:15:52 +0200 Message-Id: <9e6da7b7543cf8d6cc57265b97893e608554adc5.1570525995.git.mprivozn@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-loop: libvir-list@redhat.com Cc: xuyandong2@huawei.com, danielhb413@gmail.com, zhang.zhanghailiang@huawei.com Subject: [libvirt] [PATCH 2/2] Revert "qemu: Obtain reference on monConfig" 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.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.6.2 (mx1.redhat.com [10.5.110.69]); Tue, 08 Oct 2019 09:16:24 +0000 (UTC) Content-Type: text/plain; charset="utf-8" This reverts commit a5a777a8bae61cb9e41c4dcd12d2962ad1a65a0d. After previous commit the domain won't disappear while connecting to monitor. There's no need to ref monitor config then. Signed-off-by: Michal Privoznik Reviewed-by: Daniel Henrique Barboza --- src/qemu/qemu_process.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b303e5ba05..4d26b5a305 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1941,7 +1941,6 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomain= ObjPtr vm, int asyncJob, qemuDomainObjPrivatePtr priv =3D vm->privateData; qemuMonitorPtr mon =3D NULL; unsigned long long timeout =3D 0; - virDomainChrSourceDefPtr monConfig; =20 if (qemuSecuritySetDaemonSocketLabel(driver->securityManager, vm->def)= < 0) { VIR_ERROR(_("Failed to set security context for monitor for %s"), @@ -1956,10 +1955,9 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomai= nObjPtr vm, int asyncJob, timeout =3D vm->def->mem.total_memory / (1024 * 1024); =20 ignore_value(virTimeMillisNow(&priv->monStart)); - monConfig =3D virObjectRef(priv->monConfig); =20 mon =3D qemuMonitorOpen(vm, - monConfig, + priv->monConfig, retry, timeout, &monitorCallbacks, @@ -1973,7 +1971,6 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomain= ObjPtr vm, int asyncJob, qemuProcessMonitorLogFree); } =20 - virObjectUnref(monConfig); priv->monStart =3D 0; priv->mon =3D mon; =20 --=20 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list