From nobody Fri Oct 18 08:53:24 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of lists.libvirt.org designates 8.43.85.245 as permitted sender) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 1718291658567549.9393020929678; Thu, 13 Jun 2024 08:14:18 -0700 (PDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 7E510E66; Thu, 13 Jun 2024 11:14:17 -0400 (EDT) Received: from lists.libvirt.org (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id 8A68011C1; Thu, 13 Jun 2024 11:12:15 -0400 (EDT) Received: by lists.libvirt.org (Postfix, from userid 996) id 66E631204; Thu, 13 Jun 2024 11:12:08 -0400 (EDT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id EB87B11F5 for ; Thu, 13 Jun 2024 11:11:56 -0400 (EDT) Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-649-RkxFFM9hNoK8H8X8A28TAw-1; Thu, 13 Jun 2024 11:11:55 -0400 Received: from mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.17]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 3BBC51955F02 for ; Thu, 13 Jun 2024 15:11:54 +0000 (UTC) Received: from speedmetal.lan (unknown [10.45.242.20]) by mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 5DBD31956050 for ; Thu, 13 Jun 2024 15:11:53 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: *** X-Spam-Status: No, score=3.0 required=5.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,RCVD_IN_SBL_CSS,SPF_HELO_NONE, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1718291516; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=EeGIU6d4UKnFJloMkRG0x9Xfks0iWYz71+LLPpYEQJ8=; b=YeRs+hSoLbI3Pw9iK7WJOFpz50pcdMz3XojKgTo4527yFq9CzpR6VJPR77WXpD9AvrL5qs hAP9+xYsMk5GP5/bJIRh+mQE3FfTzZZt/iPoqPis57Z908lhKqALHODpCoJwDSkZEBhezn /1W40RehZ4+Hlc7jFy0QXKBIrOltFgU= X-MC-Unique: RkxFFM9hNoK8H8X8A28TAw-1 From: Peter Krempa To: devel@lists.libvirt.org Subject: [PATCH 06/12] qemuProcessStop: Move code not depending on 'vm->def->id' after reset of the ID Date: Thu, 13 Jun 2024 17:11:38 +0200 Message-ID: <4700174e7388c856b9fea0c56dff03110f9ee0aa.1718291410.git.pkrempa@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.17 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Message-ID-Hash: BYRQUWTD7UDFDWUTACF4ZRVKU4OVEXTH X-Message-ID-Hash: BYRQUWTD7UDFDWUTACF4ZRVKU4OVEXTH X-MailFrom: pkrempa@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-ZohoMail-DKIM: fail (Header signature does not verify) X-ZM-MESSAGEID: 1718291659933100001 Content-Type: text/plain; charset="utf-8" There are few function calls done while cleaning up a stopped VM which do require the old VM id, to e.g. clean up paths containing the 'short' domain name in the path. Anything else, which doesn't strictly require it can be moved after clearing the 'id' in order to decrease likelyhood of potential bugs. This patch moves all the code which does not require the 'id' (except for the log entry and closing the monitor socket) after the statement clearing the id and adds a comment explaining that anything in the section must not unlock the VM object. Signed-off-by: Peter Krempa --- src/qemu/qemu_process.c | 85 ++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 39 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index aef83d2409..3187f0a9a2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8482,10 +8482,11 @@ void qemuProcessStop(virQEMUDriver *driver, goto endjob; } - qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, false); - - if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCa= llback) - driver->inhibitCallback(false, driver->inhibitOpaque); + /* BEWARE: At this point 'vm->def->id' is not cleared yet. Any code th= at + * requires the id (e.g. to call virDomainDefGetShortName()) must be p= laced + * between here (after the VM is killed) and the statement clearing th= e id. + * The code *MUST NOT* unlock vm, otherwise other code might be confus= ed + * about the state of the VM. */ if ((timestamp =3D virTimeStringNow()) !=3D NULL) { qemuDomainLogAppendMessage(driver, vm, "%s: shutting down, reason= =3D%s\n", @@ -8493,6 +8494,47 @@ void qemuProcessStop(virQEMUDriver *driver, virDomainShutoffReasonTypeToString(reas= on)); } + /* shut it off for sure */ + ignore_value(qemuProcessKill(vm, + VIR_QEMU_PROCESS_KILL_FORCE| + VIR_QEMU_PROCESS_KILL_NOCHECK)); + + if (priv->agent) { + g_clear_pointer(&priv->agent, qemuAgentClose); + } + priv->agentError =3D false; + + if (priv->mon) { + g_clear_pointer(&priv->mon, qemuMonitorClose); + } + + qemuProcessBuildDestroyMemoryPaths(driver, vm, NULL, false); + + /* Do this before we delete the tree and remove pidfile. */ + qemuProcessKillManagedPRDaemon(vm); + + qemuDomainCleanupRun(driver, vm); + + outgoingMigration =3D (flags & VIR_QEMU_PROCESS_STOP_MIGRATED) && + (asyncJob =3D=3D VIR_ASYNC_JOB_MIGRATION_OUT); + + qemuExtDevicesStop(driver, vm, outgoingMigration); + + qemuDBusStop(driver, vm); + + vm->def->id =3D -1; + + /* Wake up anything waiting on domain condition */ + virDomainObjBroadcast(vm); + + /* IMPORTANT: qemuDomainObjStopWorker() unlocks @vm in order to prevent + * deadlocks with the per-VM event loop thread. This MUST be done after + * marking the VM as dead */ + qemuDomainObjStopWorker(vm); + + if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCa= llback) + driver->inhibitCallback(false, driver->inhibitOpaque); + /* Clear network bandwidth */ virDomainClearNetBandwidth(vm->def); @@ -8512,15 +8554,6 @@ void qemuProcessStop(virQEMUDriver *driver, virPortAllocatorRelease(priv->nbdPort); priv->nbdPort =3D 0; - if (priv->agent) { - g_clear_pointer(&priv->agent, qemuAgentClose); - } - priv->agentError =3D false; - - if (priv->mon) { - g_clear_pointer(&priv->mon, qemuMonitorClose); - } - if (priv->monConfig) { if (priv->monConfig->type =3D=3D VIR_DOMAIN_CHR_TYPE_UNIX) unlink(priv->monConfig->data.nix.path); @@ -8530,41 +8563,15 @@ void qemuProcessStop(virQEMUDriver *driver, /* Remove the master key */ qemuDomainMasterKeyRemove(priv); - /* Do this before we delete the tree and remove pidfile. */ - qemuProcessKillManagedPRDaemon(vm); - ignore_value(virDomainChrDefForeach(vm->def, false, qemuProcessCleanupChardevDevice, NULL)); - /* shut it off for sure */ - ignore_value(qemuProcessKill(vm, - VIR_QEMU_PROCESS_KILL_FORCE| - VIR_QEMU_PROCESS_KILL_NOCHECK)); - /* Its namespace is also gone then. */ qemuDomainDestroyNamespace(driver, vm); - qemuDomainCleanupRun(driver, vm); - - outgoingMigration =3D (flags & VIR_QEMU_PROCESS_STOP_MIGRATED) && - (asyncJob =3D=3D VIR_ASYNC_JOB_MIGRATION_OUT); - qemuExtDevicesStop(driver, vm, outgoingMigration); - - qemuDBusStop(driver, vm); - - vm->def->id =3D -1; - - /* Wake up anything waiting on domain condition */ - virDomainObjBroadcast(vm); - - /* IMPORTANT: qemuDomainObjStopWorker() unlocks @vm in order to prevent - * deadlocks with the per-VM event loop thread. This MUST be done after - * marking the VM as dead */ - qemuDomainObjStopWorker(vm); - virFileDeleteTree(priv->libDir); virFileDeleteTree(priv->channelTargetDir); --=20 2.45.2