From nobody Fri Mar 29 08:48:13 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.37 as permitted sender) client-ip=209.132.183.37; envelope-from=libvir-list-bounces@redhat.com; helo=mx5-phx2.redhat.com; Authentication-Results: mx.zoho.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.37 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; Return-Path: Received: from mx5-phx2.redhat.com (mx5-phx2.redhat.com [209.132.183.37]) by mx.zohomail.com with SMTPS id 1487679307004504.25858295731564; Tue, 21 Feb 2017 04:15:07 -0800 (PST) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx5-phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1LCBQEb019907; Tue, 21 Feb 2017 07:11:26 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id v1LCBP5V022449 for ; Tue, 21 Feb 2017 07:11:25 -0500 Received: from mx1.redhat.com (ext-mx07.extmail.prod.ext.phx2.redhat.com [10.5.110.31]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1LCBP0G018324 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO) for ; Tue, 21 Feb 2017 07:11:25 -0500 Received: from mx0a-001b2d01.pphosted.com (mx0b-001b2d01.pphosted.com [148.163.158.5]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A398DC00AFC4 for ; Tue, 21 Feb 2017 12:11:23 +0000 (UTC) Received: from pps.filterd (m0098420.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.20/8.16.0.20) with SMTP id v1LC3sL3012359 for ; Tue, 21 Feb 2017 07:11:22 -0500 Received: from e06smtp12.uk.ibm.com (e06smtp12.uk.ibm.com [195.75.94.108]) by mx0b-001b2d01.pphosted.com with ESMTP id 28rj2r8h18-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Tue, 21 Feb 2017 07:11:21 -0500 Received: from localhost by e06smtp12.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 21 Feb 2017 12:11:19 -0000 Received: from d06dlp02.portsmouth.uk.ibm.com (9.149.20.14) by e06smtp12.uk.ibm.com (192.168.101.142) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Tue, 21 Feb 2017 12:11:10 -0000 Received: from b06cxnps3074.portsmouth.uk.ibm.com (d06relay09.portsmouth.uk.ibm.com [9.149.109.194]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id E83572190023; Tue, 21 Feb 2017 12:10:11 +0000 (GMT) Received: from d06av25.portsmouth.uk.ibm.com (d06av25.portsmouth.uk.ibm.com [9.149.105.61]) by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id v1LCB9VQ11731446; Tue, 21 Feb 2017 12:11:09 GMT Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id D5C7F11C054; Tue, 21 Feb 2017 11:08:51 +0000 (GMT) Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id B2F5C11C058; Tue, 21 Feb 2017 11:08:51 +0000 (GMT) Received: from marc-ibm.boeblingen.de.ibm.com (unknown [9.152.224.51]) by d06av25.portsmouth.uk.ibm.com (Postfix) with ESMTP; Tue, 21 Feb 2017 11:08:51 +0000 (GMT) From: Marc Hartmayer To: Libvirt Mailing List Date: Tue, 21 Feb 2017 13:11:00 +0100 X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17022112-0008-0000-0000-000003ED025F X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17022112-0009-0000-0000-00001C89F4D0 Message-Id: <20170221121100.1428-1-mhartmay@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-02-21_10:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1702210118 X-Greylist: Sender passed SPF test, Sender IP whitelisted by DNSRBL, ACL 200 matched, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Tue, 21 Feb 2017 12:11:23 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Tue, 21 Feb 2017 12:11:23 +0000 (UTC) for IP:'148.163.158.5' DOMAIN:'mx0b-001b2d01.pphosted.com' HELO:'mx0a-001b2d01.pphosted.com' FROM:'mhartmay@linux.vnet.ibm.com' RCPT:'' X-RedHat-Spam-Score: -0.29 (BAYES_50, RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2) 148.163.158.5 mx0b-001b2d01.pphosted.com 148.163.158.5 mx0b-001b2d01.pphosted.com X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Scanned-By: MIMEDefang 2.78 on 10.5.110.31 X-loop: libvir-list@redhat.com Cc: Michal Privoznik , Marc Hartmayer Subject: [libvirt] [PATCH] qemu: Fix deadlock across fork() in QEMU driver 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: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" The functions in virCommand() after fork() must be careful with regard to accessing any mutexes that may have been locked by other threads in the parent process. It is possible that another thread in the parent process holds the lock for the virQEMUDriver while fork() is called. This leads to a deadlock in the child process when 'virQEMUDriverGetConfig(driver)' is called and therefore the handshake never completes between the child and the parent process. Ultimately the virDomainObjectPtr will never be unlocked. It gets much worse if the other thread of the parent process, that holds the lock for the virQEMUDriver, tries to lock the already locked virDomainObject. This leads to a completely unresponsive libvirtd. It's possible to reproduce this case with calling 'virsh start XXX' and 'virsh managedsave XXX' in a tight loop for multiple domains. This commit fixes the deadlock in the same way as it is described in commit 61b52d2e3813cc8c9ff3ab67f232bd0c65f7318d. Signed-off-by: Marc Hartmayer Reviewed-by: Boris Fiuczynski --- src/qemu/qemu_domain.c | 73 +++++++++++++++++++++++----------------------= ---- src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_process.c | 2 +- 3 files changed, 37 insertions(+), 41 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ea4b282..c187214 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -7045,13 +7045,12 @@ qemuDomainGetHostdevPath(virDomainDefPtr def, * Returns 0 on success, -1 otherwise (with error reported) */ static int -qemuDomainGetPreservedMounts(virQEMUDriverPtr driver, +qemuDomainGetPreservedMounts(virQEMUDriverConfigPtr cfg, virDomainObjPtr vm, char ***devPath, char ***devSavePath, size_t *ndevPath) { - virQEMUDriverConfigPtr cfg =3D virQEMUDriverGetConfig(driver); char **paths =3D NULL, **mounts =3D NULL; size_t i, nmounts; =20 @@ -7092,13 +7091,11 @@ qemuDomainGetPreservedMounts(virQEMUDriverPtr drive= r, if (ndevPath) *ndevPath =3D nmounts; =20 - virObjectUnref(cfg); return 0; =20 error: virStringListFreeCount(mounts, nmounts); virStringListFreeCount(paths, nmounts); - virObjectUnref(cfg); return -1; } =20 @@ -7310,11 +7307,10 @@ qemuDomainCreateDevice(const char *device, =20 =20 static int -qemuDomainPopulateDevices(virQEMUDriverPtr driver, +qemuDomainPopulateDevices(virQEMUDriverConfigPtr cfg, virDomainObjPtr vm ATTRIBUTE_UNUSED, const char *path) { - virQEMUDriverConfigPtr cfg =3D virQEMUDriverGetConfig(driver); const char *const *devices =3D (const char *const *) cfg->cgroupDevice= ACL; size_t i; int ret =3D -1; @@ -7329,13 +7325,13 @@ qemuDomainPopulateDevices(virQEMUDriverPtr driver, =20 ret =3D 0; cleanup: - virObjectUnref(cfg); return ret; } =20 =20 static int -qemuDomainSetupDev(virQEMUDriverPtr driver, +qemuDomainSetupDev(virQEMUDriverConfigPtr cfg, + virSecurityManagerPtr mgr, virDomainObjPtr vm, const char *path) { @@ -7345,7 +7341,7 @@ qemuDomainSetupDev(virQEMUDriverPtr driver, =20 VIR_DEBUG("Setting up /dev/ for domain %s", vm->def->name); =20 - mount_options =3D virSecurityManagerGetMountOptions(driver->securityMa= nager, + mount_options =3D virSecurityManagerGetMountOptions(mgr, vm->def); =20 if (!mount_options && @@ -7363,7 +7359,7 @@ qemuDomainSetupDev(virQEMUDriverPtr driver, if (virFileSetupDev(path, opts) < 0) goto cleanup; =20 - if (qemuDomainPopulateDevices(driver, vm, path) < 0) + if (qemuDomainPopulateDevices(cfg, vm, path) < 0) goto cleanup; =20 ret =3D 0; @@ -7375,7 +7371,7 @@ qemuDomainSetupDev(virQEMUDriverPtr driver, =20 =20 static int -qemuDomainSetupDisk(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, +qemuDomainSetupDisk(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED, virDomainDiskDefPtr disk, const char *devPath) { @@ -7401,7 +7397,7 @@ qemuDomainSetupDisk(virQEMUDriverPtr driver ATTRIBUTE= _UNUSED, =20 =20 static int -qemuDomainSetupAllDisks(virQEMUDriverPtr driver, +qemuDomainSetupAllDisks(virQEMUDriverConfigPtr cfg, virDomainObjPtr vm, const char *devPath) { @@ -7409,7 +7405,7 @@ qemuDomainSetupAllDisks(virQEMUDriverPtr driver, VIR_DEBUG("Setting up disks"); =20 for (i =3D 0; i < vm->def->ndisks; i++) { - if (qemuDomainSetupDisk(driver, + if (qemuDomainSetupDisk(cfg, vm->def->disks[i], devPath) < 0) return -1; @@ -7421,7 +7417,7 @@ qemuDomainSetupAllDisks(virQEMUDriverPtr driver, =20 =20 static int -qemuDomainSetupHostdev(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, +qemuDomainSetupHostdev(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED, virDomainHostdevDefPtr dev, const char *devPath) { @@ -7447,7 +7443,7 @@ qemuDomainSetupHostdev(virQEMUDriverPtr driver ATTRIB= UTE_UNUSED, =20 =20 static int -qemuDomainSetupAllHostdevs(virQEMUDriverPtr driver, +qemuDomainSetupAllHostdevs(virQEMUDriverConfigPtr cfg, virDomainObjPtr vm, const char *devPath) { @@ -7455,7 +7451,7 @@ qemuDomainSetupAllHostdevs(virQEMUDriverPtr driver, =20 VIR_DEBUG("Setting up hostdevs"); for (i =3D 0; i < vm->def->nhostdevs; i++) { - if (qemuDomainSetupHostdev(driver, + if (qemuDomainSetupHostdev(cfg, vm->def->hostdevs[i], devPath) < 0) return -1; @@ -7480,7 +7476,7 @@ qemuDomainSetupChardev(virDomainDefPtr def ATTRIBUTE_= UNUSED, =20 =20 static int -qemuDomainSetupAllChardevs(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, +qemuDomainSetupAllChardevs(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED, virDomainObjPtr vm, const char *devPath) { @@ -7498,7 +7494,7 @@ qemuDomainSetupAllChardevs(virQEMUDriverPtr driver AT= TRIBUTE_UNUSED, =20 =20 static int -qemuDomainSetupTPM(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, +qemuDomainSetupTPM(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED, virDomainObjPtr vm, const char *devPath) { @@ -7527,7 +7523,7 @@ qemuDomainSetupTPM(virQEMUDriverPtr driver ATTRIBUTE_= UNUSED, =20 =20 static int -qemuDomainSetupGraphics(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, +qemuDomainSetupGraphics(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED, virDomainGraphicsDefPtr gfx, const char *devPath) { @@ -7543,7 +7539,7 @@ qemuDomainSetupGraphics(virQEMUDriverPtr driver ATTRI= BUTE_UNUSED, =20 =20 static int -qemuDomainSetupAllGraphics(virQEMUDriverPtr driver, +qemuDomainSetupAllGraphics(virQEMUDriverConfigPtr cfg, virDomainObjPtr vm, const char *devPath) { @@ -7551,7 +7547,7 @@ qemuDomainSetupAllGraphics(virQEMUDriverPtr driver, =20 VIR_DEBUG("Setting up graphics"); for (i =3D 0; i < vm->def->ngraphics; i++) { - if (qemuDomainSetupGraphics(driver, + if (qemuDomainSetupGraphics(cfg, vm->def->graphics[i], devPath) < 0) return -1; @@ -7563,7 +7559,7 @@ qemuDomainSetupAllGraphics(virQEMUDriverPtr driver, =20 =20 static int -qemuDomainSetupInput(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, +qemuDomainSetupInput(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED, virDomainInputDefPtr input, const char *devPath) { @@ -7590,7 +7586,7 @@ qemuDomainSetupInput(virQEMUDriverPtr driver ATTRIBUT= E_UNUSED, =20 =20 static int -qemuDomainSetupAllInputs(virQEMUDriverPtr driver, +qemuDomainSetupAllInputs(virQEMUDriverConfigPtr cfg, virDomainObjPtr vm, const char *devPath) { @@ -7598,7 +7594,7 @@ qemuDomainSetupAllInputs(virQEMUDriverPtr driver, =20 VIR_DEBUG("Setting up inputs"); for (i =3D 0; i < vm->def->ninputs; i++) { - if (qemuDomainSetupInput(driver, + if (qemuDomainSetupInput(cfg, vm->def->inputs[i], devPath) < 0) return -1; @@ -7609,7 +7605,7 @@ qemuDomainSetupAllInputs(virQEMUDriverPtr driver, =20 =20 static int -qemuDomainSetupRNG(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, +qemuDomainSetupRNG(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED, virDomainRNGDefPtr rng, const char *devPath) { @@ -7629,7 +7625,7 @@ qemuDomainSetupRNG(virQEMUDriverPtr driver ATTRIBUTE_= UNUSED, =20 =20 static int -qemuDomainSetupAllRNGs(virQEMUDriverPtr driver, +qemuDomainSetupAllRNGs(virQEMUDriverConfigPtr cfg, virDomainObjPtr vm, const char *devPath) { @@ -7637,7 +7633,7 @@ qemuDomainSetupAllRNGs(virQEMUDriverPtr driver, =20 VIR_DEBUG("Setting up RNGs"); for (i =3D 0; i < vm->def->nrngs; i++) { - if (qemuDomainSetupRNG(driver, + if (qemuDomainSetupRNG(cfg, vm->def->rngs[i], devPath) < 0) return -1; @@ -7649,10 +7645,10 @@ qemuDomainSetupAllRNGs(virQEMUDriverPtr driver, =20 =20 int -qemuDomainBuildNamespace(virQEMUDriverPtr driver, +qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg, + virSecurityManagerPtr mgr, virDomainObjPtr vm) { - virQEMUDriverConfigPtr cfg =3D virQEMUDriverGetConfig(driver); char *devPath =3D NULL; char **devMountsPath =3D NULL, **devMountsSavePath =3D NULL; size_t ndevMountsPath =3D 0, i; @@ -7663,7 +7659,7 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver, goto cleanup; } =20 - if (qemuDomainGetPreservedMounts(driver, vm, + if (qemuDomainGetPreservedMounts(cfg, vm, &devMountsPath, &devMountsSavePath, &ndevMountsPath) < 0) goto cleanup; @@ -7684,7 +7680,7 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver, if (virProcessSetupPrivateMountNS() < 0) goto cleanup; =20 - if (qemuDomainSetupDev(driver, vm, devPath) < 0) + if (qemuDomainSetupDev(cfg, mgr, vm, devPath) < 0) goto cleanup; =20 /* Save some mount points because we want to share them with the host = */ @@ -7703,25 +7699,25 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver, goto cleanup; } =20 - if (qemuDomainSetupAllDisks(driver, vm, devPath) < 0) + if (qemuDomainSetupAllDisks(cfg, vm, devPath) < 0) goto cleanup; =20 - if (qemuDomainSetupAllHostdevs(driver, vm, devPath) < 0) + if (qemuDomainSetupAllHostdevs(cfg, vm, devPath) < 0) goto cleanup; =20 - if (qemuDomainSetupAllChardevs(driver, vm, devPath) < 0) + if (qemuDomainSetupAllChardevs(cfg, vm, devPath) < 0) goto cleanup; =20 - if (qemuDomainSetupTPM(driver, vm, devPath) < 0) + if (qemuDomainSetupTPM(cfg, vm, devPath) < 0) goto cleanup; =20 - if (qemuDomainSetupAllGraphics(driver, vm, devPath) < 0) + if (qemuDomainSetupAllGraphics(cfg, vm, devPath) < 0) goto cleanup; =20 - if (qemuDomainSetupAllInputs(driver, vm, devPath) < 0) + if (qemuDomainSetupAllInputs(cfg, vm, devPath) < 0) goto cleanup; =20 - if (qemuDomainSetupAllRNGs(driver, vm, devPath) < 0) + if (qemuDomainSetupAllRNGs(cfg, vm, devPath) < 0) goto cleanup; =20 if (virFileMoveMount(devPath, "/dev") < 0) @@ -7743,7 +7739,6 @@ qemuDomainBuildNamespace(virQEMUDriverPtr driver, =20 ret =3D 0; cleanup: - virObjectUnref(cfg); for (i =3D 0; i < ndevMountsPath; i++) rmdir(devMountsSavePath[i]); virStringListFreeCount(devMountsPath, ndevMountsPath); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 8ba807c..72efa33 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -809,7 +809,8 @@ int qemuDomainGetHostdevPath(virDomainDefPtr def, char ***path, int **perms); =20 -int qemuDomainBuildNamespace(virQEMUDriverPtr driver, +int qemuDomainBuildNamespace(virQEMUDriverConfigPtr cfg, + virSecurityManagerPtr mgr, virDomainObjPtr vm); =20 int qemuDomainCreateNamespace(virQEMUDriverPtr driver, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 522f49d..e1a738c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2636,7 +2636,7 @@ static int qemuProcessHook(void *data) if (virSecurityManagerClearSocketLabel(h->driver->securityManager, h->= vm->def) < 0) goto cleanup; =20 - if (qemuDomainBuildNamespace(h->driver, h->vm) < 0) + if (qemuDomainBuildNamespace(h->cfg, h->driver->securityManager, h->vm= ) < 0) goto cleanup; =20 if (virDomainNumatuneGetMode(h->vm->def->numa, -1, &mode) =3D=3D 0) { --=20 2.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list