From nobody Wed May 15 14:43:05 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=1607028602; cv=none; d=zohomail.com; s=zohoarc; b=ENKbnPZO2KF9by7iiXNlCYPtXPndQUpp6yBr9PvqXTVg1mKRiswtF5ngrcgQrDdXViYdyci8/vs8tncsuFdFEZPrvXjvIay/bq3g+doQ1fJYsjDks3eGIRqJ8vQVBQlINqn6wHR4+TmkpcKr2Z/DSsO6Ey8Zmk2wrq9X/yAPCWs= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1607028602; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=NgqcBOwKiCWDtfBsiHllNzeEsiASE9afWX8ODU8mpxI=; b=M9RssFbo6Apzk4tJSwDMTH+55+LHxbFCXUUAiS931Y83tC2dKNNEkGE4WPc0OuDS/blL2nZhYdpIRx643PL45cOiimnr7hVMZZsZ1Qj7Tu2fy6dhCoGlH7hbyxMUDeWpxQq0ObrD+46n1tQRas+v96Zwe23fOlWtQybyYzyXN3w= 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) header.from= 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 160702860260657.50382726633973; Thu, 3 Dec 2020 12:50:02 -0800 (PST) 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-387-ORDcAf3eNCOIAZUqpXqAFQ-1; Thu, 03 Dec 2020 15:49:59 -0500 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 C6358107ACE4; Thu, 3 Dec 2020 20:49:52 +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 917DD60873; Thu, 3 Dec 2020 20:49:52 +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 B964F4BB7B; Thu, 3 Dec 2020 20:49:50 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 0B3KkVDf030996 for ; Thu, 3 Dec 2020 15:46:31 -0500 Received: by smtp.corp.redhat.com (Postfix) id 67C325C1D0; Thu, 3 Dec 2020 20:46:31 +0000 (UTC) Received: from localhost.localdomain (unknown [10.40.192.239]) by smtp.corp.redhat.com (Postfix) with ESMTP id F2B1A5C1B4; Thu, 3 Dec 2020 20:46:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607028601; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc: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=NgqcBOwKiCWDtfBsiHllNzeEsiASE9afWX8ODU8mpxI=; b=dO8+cXjzNzK7xMNy1ExCuO4De/KMnmtw8m7W97FnC8aF0BF53eac01VS1Ei4G5whc8gr2u w9rchbeKAn2LXTbIV/KJzuTH36obEdQp2cknPCD2yll2XoDrj4d+sYkkJSWtvZba/79vDD J+n3nOALdzf/AjtUYd5hqASvQOQADBA= X-MC-Unique: ORDcAf3eNCOIAZUqpXqAFQ-1 From: Michal Privoznik To: mkletzan@redhat.com Subject: [PATCH v2 RESEND] lxc: Cleanup after failed startup Date: Thu, 3 Dec 2020 21:46:20 +0100 Message-Id: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Cc: 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) Content-Type: text/plain; charset="utf-8" If starting an container fails, the virLXCProcessStop() is called. But since vm->def->id is not set until libvirt_lxc is spawned (the domain's ID is PID of that process), virLXCProcessStop() returns early as virDomainObjIsActive() returns false. But doing so leaves behind resources reserved for the containers during the startup process. Most notably, hostdevs are not re-attached to the host, the domain's transient XML is not removed, etc. To resolve this, virLXCProcessCleanup() is called in this case. However, it is modified to accept @flags which allows caller to run only specific cleanups (depending how far in container creation the failure occurred). There is plenty of cleanups which don't need this guard because either they detect a NULL pointer or try to release an unique resource. Signed-off-by: Michal Privoznik Reviewed-by: Martin Kletzander --- This is just a resend of this patch: https://www.redhat.com/archives/libvir-list/2020-November/msg00525.html src/lxc/lxc_process.c | 52 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 41 insertions(+), 11 deletions(-) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 7e07f49f9a..0f818e2ee0 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -145,18 +145,27 @@ lxcProcessRemoveDomainStatus(virLXCDriverConfigPtr cf= g, } =20 =20 +typedef enum { + VIR_LXC_PROCESS_CLEANUP_RELEASE_SECLABEL =3D (1 << 0), + VIR_LXC_PROCESS_CLEANUP_RESTORE_SECLABEL =3D (1 << 1), + VIR_LXC_PROCESS_CLEANUP_REMOVE_TRANSIENT =3D (1 << 2), +} virLXCProcessCleanupFlags; + /** * virLXCProcessCleanup: * @driver: pointer to driver structure * @vm: pointer to VM to clean up * @reason: reason for switching the VM to shutoff state + * @flags: allows to run selective cleanups only * - * Cleanout resources associated with the now dead VM - * + * Clean out resources associated with the now dead VM. + * If @flags is zero then whole cleanup process is done, + * otherwise only selected sections are run. */ static void virLXCProcessCleanup(virLXCDriverPtr driver, virDomainObjPtr vm, - virDomainShutoffReason reason) + virDomainShutoffReason reason, + unsigned int flags) { size_t i; virLXCDomainObjPrivatePtr priv =3D vm->privateData; @@ -164,8 +173,11 @@ static void virLXCProcessCleanup(virLXCDriverPtr drive= r, virLXCDriverConfigPtr cfg =3D virLXCDriverGetConfig(driver); virConnectPtr conn =3D NULL; =20 - VIR_DEBUG("Cleanup VM name=3D%s pid=3D%d reason=3D%d", - vm->def->name, (int)vm->pid, (int)reason); + VIR_DEBUG("Cleanup VM name=3D%s pid=3D%d reason=3D%d flags=3D0x%x", + vm->def->name, (int)vm->pid, (int)reason, flags); + + if (flags =3D=3D 0) + flags =3D ~0; =20 /* now that we know it's stopped call the hook if present */ if (virHookPresent(VIR_HOOK_DRIVER_LXC)) { @@ -177,9 +189,15 @@ static void virLXCProcessCleanup(virLXCDriverPtr drive= r, NULL, xml, NULL); } =20 - virSecurityManagerRestoreAllLabel(driver->securityManager, - vm->def, false, false); - virSecurityManagerReleaseLabel(driver->securityManager, vm->def); + if (flags & VIR_LXC_PROCESS_CLEANUP_RESTORE_SECLABEL) { + virSecurityManagerRestoreAllLabel(driver->securityManager, + vm->def, false, false); + } + + if (flags & VIR_LXC_PROCESS_CLEANUP_RELEASE_SECLABEL) { + virSecurityManagerReleaseLabel(driver->securityManager, vm->def); + } + /* Clear out dynamically assigned labels */ if (vm->def->nseclabels && vm->def->seclabels[0]->type =3D=3D VIR_DOMAIN_SECLABEL_DYNAMIC) { @@ -258,7 +276,9 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver, NULL, xml, NULL); } =20 - virDomainObjRemoveTransientDef(vm); + if (flags & VIR_LXC_PROCESS_CLEANUP_REMOVE_TRANSIENT) + virDomainObjRemoveTransientDef(vm); + virObjectUnref(cfg); virObjectUnref(conn); } @@ -904,7 +924,7 @@ int virLXCProcessStop(virLXCDriverPtr driver, } =20 cleanup: - virLXCProcessCleanup(driver, vm, reason); + virLXCProcessCleanup(driver, vm, reason, 0); =20 return 0; } @@ -1198,6 +1218,7 @@ int virLXCProcessStart(virConnectPtr conn, g_autoptr(virCgroup) selfcgroup =3D NULL; int status; g_autofree char *pidfile =3D NULL; + unsigned int stopFlags =3D 0; =20 if (virCgroupNewSelf(&selfcgroup) < 0) return -1; @@ -1265,6 +1286,7 @@ int virLXCProcessStart(virConnectPtr conn, VIR_DEBUG("Setting current domain def as transient"); if (virDomainObjSetDefTransient(driver->xmlopt, vm, NULL) < 0) goto cleanup; + stopFlags |=3D VIR_LXC_PROCESS_CLEANUP_REMOVE_TRANSIENT; =20 /* Run an early hook to set-up missing devices */ if (virHookPresent(VIR_HOOK_DRIVER_LXC)) { @@ -1312,11 +1334,13 @@ int virLXCProcessStart(virConnectPtr conn, goto cleanup; } virDomainAuditSecurityLabel(vm, true); + stopFlags |=3D VIR_LXC_PROCESS_CLEANUP_RELEASE_SECLABEL; =20 VIR_DEBUG("Setting domain security labels"); if (virSecurityManagerSetAllLabel(driver->securityManager, vm->def, NULL, false, false) < 0) goto cleanup; + stopFlags |=3D VIR_LXC_PROCESS_CLEANUP_RESTORE_SECLABEL; =20 VIR_DEBUG("Setting up consoles"); for (i =3D 0; i < vm->def->nconsoles; i++) { @@ -1525,7 +1549,13 @@ int virLXCProcessStart(virConnectPtr conn, } if (rc !=3D 0) { virErrorPreserveLast(&err); - virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); + if (virDomainObjIsActive(vm)) { + virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); + } else { + /* virLXCProcessStop() is NOP if the container is not active. + * If there was a failure whilst creating it, cleanup manually= . */ + virLXCProcessCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, st= opFlags); + } } virCommandFree(cmd); for (i =3D 0; i < nttyFDs; i++) --=20 2.26.2