From b0d3053a2be55a417661de3a3adacd8ef70e8327 Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Wed, 11 Nov 2020 13:51:21 +0100 Subject: [PATCH] lxc: Cleanup after failed startup 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 --- 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 cfg, } +typedef enum { + VIR_LXC_PROCESS_CLEANUP_RELEASE_SECLABEL = (1 << 0), + VIR_LXC_PROCESS_CLEANUP_RESTORE_SECLABEL = (1 << 1), + VIR_LXC_PROCESS_CLEANUP_REMOVE_TRANSIENT = (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 = vm->privateData; @@ -164,8 +173,11 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver, virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver); virConnectPtr conn = NULL; - VIR_DEBUG("Cleanup VM name=%s pid=%d reason=%d", - vm->def->name, (int)vm->pid, (int)reason); + VIR_DEBUG("Cleanup VM name=%s pid=%d reason=%d flags=0x%x", + vm->def->name, (int)vm->pid, (int)reason, flags); + + if (flags == 0) + flags = ~0; /* 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 driver, NULL, xml, NULL); } - 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 == VIR_DOMAIN_SECLABEL_DYNAMIC) { @@ -258,7 +276,9 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver, NULL, xml, NULL); } - virDomainObjRemoveTransientDef(vm); + if (flags & VIR_LXC_PROCESS_CLEANUP_REMOVE_TRANSIENT) + virDomainObjRemoveTransientDef(vm); + virObjectUnref(cfg); virObjectUnref(conn); } @@ -904,7 +924,7 @@ int virLXCProcessStop(virLXCDriverPtr driver, } cleanup: - virLXCProcessCleanup(driver, vm, reason); + virLXCProcessCleanup(driver, vm, reason, 0); return 0; } @@ -1198,6 +1218,7 @@ int virLXCProcessStart(virConnectPtr conn, g_autoptr(virCgroup) selfcgroup = NULL; int status; g_autofree char *pidfile = NULL; + unsigned int stopFlags = 0; 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 |= VIR_LXC_PROCESS_CLEANUP_REMOVE_TRANSIENT; /* 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 |= VIR_LXC_PROCESS_CLEANUP_RELEASE_SECLABEL; VIR_DEBUG("Setting domain security labels"); if (virSecurityManagerSetAllLabel(driver->securityManager, vm->def, NULL, false, false) < 0) goto cleanup; + stopFlags |= VIR_LXC_PROCESS_CLEANUP_RESTORE_SECLABEL; VIR_DEBUG("Setting up consoles"); for (i = 0; i < vm->def->nconsoles; i++) { @@ -1525,7 +1549,13 @@ int virLXCProcessStart(virConnectPtr conn, } if (rc != 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, stopFlags); + } } virCommandFree(cmd); for (i = 0; i < nttyFDs; i++) -- 2.47.3