From nobody Sat May 4 06:52:37 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) client-ip=216.205.24.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 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=1626448030; cv=none; d=zohomail.com; s=zohoarc; b=lApywgJyKmKPFVHj1n88deZNkGNn3ADH7pPjaEbmhK8frJTVEGPWxwWXvGPwg0LuQDu6tq193HANlGb9F0uoYw/VfbIYpT6eazr/nKmzRZJBwRisOrLyq6e0i4a0485AFYRJk0FEpvoAmfwlj4I+olSZ4b5CngyeS0CPatgNZjg= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1626448030; h=Content-Type:Content-Transfer-Encoding:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=W2nWZZfFrH2Jp4heaRUYbd5bIxq5YzLVVlA3nd+kaYw=; b=kwSMnPR5iO9es3M4vtAaX59r5XjcgxoY4tq401jeHBcmMukhp0OwXIumfLqdJz7pSimKYlQg7zCUEJmL8HUCsi+ZWJBj8jiVTQ8L7rzKtSMq/46yZAJHub0mwAavmjC/z3a0+R5r/o+QsxLjiVAQxV9A2a2GY+WbG4TzyM/mcPM= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.zohomail.com with SMTPS id 1626448030684586.585759847255; Fri, 16 Jul 2021 08:07:10 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-243-tBiyh6zmOzy6zoywT28yEg-1; Fri, 16 Jul 2021 11:07:07 -0400 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 mimecast-mx01.redhat.com (Postfix) with ESMTPS id CCB53101C8AA; Fri, 16 Jul 2021 15:07:01 +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 6785C60864; Fri, 16 Jul 2021 15:07:01 +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 75E394EA29; Fri, 16 Jul 2021 15:07:00 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 16GF6wJ7031814 for ; Fri, 16 Jul 2021 11:06:58 -0400 Received: by smtp.corp.redhat.com (Postfix) id 5DDA418A50; Fri, 16 Jul 2021 15:06:58 +0000 (UTC) Received: from virval.usersys.redhat.com (unknown [10.40.194.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 315C360657 for ; Fri, 16 Jul 2021 15:06:54 +0000 (UTC) Received: by virval.usersys.redhat.com (Postfix, from userid 500) id 7DD95241F64; Fri, 16 Jul 2021 17:06:53 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1626448029; h=from:from:sender:sender: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:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=W2nWZZfFrH2Jp4heaRUYbd5bIxq5YzLVVlA3nd+kaYw=; b=GhmsWX+nz4QmxdAFGVEYmW7Fo+hpuKzinNqBLCMD4g+KSJRJe4fAf3DJTLLm/N7GzVNcFl xUNnKriw8OE+w/+DQxddfiwTQ3p5sHreexg2BLQpCdO7X1CrP94tG7xpkEfko9D3ZpZ0rz NYo65rfDEOltJGDIDAMJVPMU4zsHkyA= X-MC-Unique: tBiyh6zmOzy6zoywT28yEg-1 From: Jiri Denemark To: libvir-list@redhat.com Subject: [libvirt PATCH] qemu: Signal domain condition in qemuProcessStop a bit later Date: Fri, 16 Jul 2021 17:06:44 +0200 Message-Id: <3bd8d3fdf3c977696c5bbeffd1fc4a4e76294aa2.1626448004.git.jdenemar@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-loop: libvir-list@redhat.com 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: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) X-ZM-MESSAGEID: 1626448031336100005 Content-Type: text/plain; charset="utf-8" Signaling the condition before vm->def->id is reset to -1 is dangerous: in case a waiting thread wakes up, it does not see anything interesting (the domain is still marked as running) and just enters virDomainObjWait where it waits forever because the condition will never be signalled again. Originally it was impossible to get into such situation because the vm object was locked all the time between signaling the condition and resetting vm->def->id, but after commit 860a999802 released in 6.8.0, qemuDomainObjStopWorker called in qemuProcessStop between virDomainObjBroadcast and setting vm->def->id to -1 unlocks the vm object giving other threads a chance to wake up and possibly hang. In real world, this can be easily reproduced by killing, destroying, or just shutting down (from the guest OS) a domain while it is being migrated somewhere else. The migration job would never finish. We can't fix this by reseting vm->def->id earlier because other functions (such as qemuProcessKill) do nothing when the domain is already marked as inactive. So let's make sure we delay signaling the domain condition to the point when a woken up thread can detect the domain is not active anymore. https://bugzilla.redhat.com/show_bug.cgi?id=3D1949869 Signed-off-by: Jiri Denemark Reviewed-by: Michal Privoznik --- src/qemu/qemu_process.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index c972c90801..914f936e45 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7852,9 +7852,6 @@ void qemuProcessStop(virQEMUDriver *driver, if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCa= llback) driver->inhibitCallback(false, driver->inhibitOpaque); =20 - /* Wake up anything waiting on domain condition */ - virDomainObjBroadcast(vm); - if ((timestamp =3D virTimeStringNow()) !=3D NULL) { qemuDomainLogAppendMessage(driver, vm, "%s: shutting down, reason= =3D%s\n", timestamp, @@ -7925,6 +7922,9 @@ void qemuProcessStop(virQEMUDriver *driver, =20 vm->def->id =3D -1; =20 + /* Wake up anything waiting on domain condition */ + virDomainObjBroadcast(vm); + virFileDeleteTree(priv->libDir); virFileDeleteTree(priv->channelTargetDir); =20 --=20 2.32.0