Package: release.debian.org Severity: normal Tags: bullseye User: release.debian....@packages.debian.org Usertags: pu
[ Reason ] Fix lxc container reboots and shutdown (#983871, #991773). [ Tests ] On top of reports that the issues are fixed I tested that libvirt's main use case qemu is still functional. [ Risks ] The cgroup code is used by other bits of libvirt as well (like qemu) but see above. The change also introduces a intermittent (but harmless) log message Nov 25 15:01:55 honk libvirtd[1761464]: unable to open '/sys/fs/cgroup/machine.slice/machine*scope/': No such file or directory on shutdown. Given the risks of changing more code I'd consider that the lesser evil. [ Checklist ] [x] *all* changes are documented in the d/changelog [x] I reviewed all changes and I approve them [x] attach debdiff against the package in (old)stable [x] the issue is verified as fixed in unstable [ Changes ] The code is a backport of upstream commits. Cheers, -- Guido
diff --git a/debian/changelog b/debian/changelog index 5b82057454..28579ccd7e 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,17 @@ +libvirt (7.0.0-3+deb11u1) bullseye; urgency=medium + + [ Guido Günther ] + * [eb0956b] d/salsa-ci: Switch to bullseye + * [dfcaecc] d/gbp.conf: Switch to bullseye + * [7decb27] vircgroup: Fix virCgroupKillRecursive() wrt nested controllers. + Thanks to Dio Putra (Closes: #983871) + + [ Joachim Falk ] + * [fcfceec] lxc: Fix reboot command + (Closes: #991773) + + -- Guido Günther <a...@sigxcpu.org> Thu, 24 Nov 2022 21:59:50 +0100 + libvirt (7.0.0-3) unstable; urgency=medium * Team upload diff --git a/debian/gbp.conf b/debian/gbp.conf index 83b38b3bdb..b474d29e6f 100644 --- a/debian/gbp.conf +++ b/debian/gbp.conf @@ -1,6 +1,6 @@ [DEFAULT] upstream-branch = upstream/latest -debian-branch = debian/master +debian-branch = debian/bullseye pristine-tar = True upstream-signatures = on diff --git a/debian/patches/backport/Fix-reboot-command-for-LXC-containers.patch b/debian/patches/backport/Fix-reboot-command-for-LXC-containers.patch new file mode 100644 index 0000000000..82db391755 --- /dev/null +++ b/debian/patches/backport/Fix-reboot-command-for-LXC-containers.patch @@ -0,0 +1,92 @@ +From: Joachim Falk <joachim.f...@gmx.de> +Date: Thu, 2 Dec 2021 19:56:07 +0100 +Subject: Fix reboot command for LXC containers (Closes: #991773) + +The virNetDaemonQuit(dmn) command in virLXCControllerSignalChildIO triggers an +early close of all clients of lxc_controller. Here, libvirtd itself is a client +of this controller, and the client connection is used to notify libvirtd if a +reboot of the container is required. However, the client connection was closed +before such a status could be sent to libvirtd. To fix this bug, we will +immediately send the reboot or shutdown status of the container to libvirtd, and +only after client disconnect will we trigger virNetDaemonQuit (Closes: #991773). + +Fixes: https://gitlab.com/libvirt/libvirt/-/issues/237 +Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=991773 +Signed-off-by: Joachim Falk <joachim.f...@gmx.de> +Reviewed-by: Michal Privoznik <mpriv...@redhat.com> + +(cherry picked from commit 93c47e2c39521aba760486f0238458ef1a37490c) + +In order to cleanly apply to libvirt 7.0.0, this patch needed some minor +adjustments, e.g., "virNetDaemon *dmn" vs "virNetDaemonPtr dmn" in libvirt 7.0.0. +--- + src/lxc/lxc_controller.c | 18 +++++++++++------- + 1 file changed, 11 insertions(+), 7 deletions(-) + +Index: libvirt/src/lxc/lxc_controller.c +=================================================================== +--- libvirt.orig/src/lxc/lxc_controller.c ++++ libvirt/src/lxc/lxc_controller.c +@@ -897,8 +897,10 @@ static void virLXCControllerClientCloseH + virLXCControllerPtr ctrl = virNetServerClientGetPrivateData(client); + + VIR_DEBUG("Client %p has closed", client); +- if (ctrl->client == client) ++ if (ctrl->client == client) { + ctrl->client = NULL; ++ VIR_DEBUG("Client has gone away"); ++ } + if (ctrl->inShutdown) { + VIR_DEBUG("Arm timer to quit event loop"); + virEventUpdateTimeout(ctrl->timerShutdown, 0); +@@ -1009,8 +1011,11 @@ static int lxcControllerClearCapabilitie + static bool wantReboot; + static virMutex lock = VIR_MUTEX_INITIALIZER; + ++static int ++virLXCControllerEventSendExit(virLXCController *ctrl, ++ int exitstatus); + +-static void virLXCControllerSignalChildIO(virNetDaemonPtr dmn, ++static void virLXCControllerSignalChildIO(virNetDaemonPtr dmn G_GNUC_UNUSED, + siginfo_t *info G_GNUC_UNUSED, + void *opaque) + { +@@ -1021,7 +1026,6 @@ static void virLXCControllerSignalChildI + ret = waitpid(-1, &status, WNOHANG); + VIR_DEBUG("Got sig child %d vs %lld", ret, (long long)ctrl->initpid); + if (ret == ctrl->initpid) { +- virNetDaemonQuit(dmn); + virMutexLock(&lock); + if (WIFSIGNALED(status) && + WTERMSIG(status) == SIGHUP) { +@@ -1029,6 +1033,7 @@ static void virLXCControllerSignalChildI + wantReboot = true; + } + virMutexUnlock(&lock); ++ virLXCControllerEventSendExit(ctrl, wantReboot ? 1 : 0); + } + } + +@@ -2279,9 +2284,10 @@ virLXCControllerEventSendExit(virLXCCont + VIR_DEBUG("Waiting for client to complete dispatch"); + ctrl->inShutdown = true; + virNetServerClientDelayedClose(ctrl->client); +- virNetDaemonRun(ctrl->daemon); ++ } else { ++ VIR_DEBUG("Arm timer to quit event loop"); ++ virEventUpdateTimeout(ctrl->timerShutdown, 0); + } +- VIR_DEBUG("Client has gone away"); + return 0; + } + +@@ -2423,8 +2429,6 @@ virLXCControllerRun(virLXCControllerPtr + + rc = virLXCControllerMain(ctrl); + +- virLXCControllerEventSendExit(ctrl, rc); +- + cleanup: + VIR_FORCE_CLOSE(control[0]); + VIR_FORCE_CLOSE(control[1]); diff --git a/debian/patches/backport/vircgroup-Fix-virCgroupKillRecursive-wrt-nested-controlle.patch b/debian/patches/backport/vircgroup-Fix-virCgroupKillRecursive-wrt-nested-controlle.patch new file mode 100644 index 0000000000..3a04e71ee5 --- /dev/null +++ b/debian/patches/backport/vircgroup-Fix-virCgroupKillRecursive-wrt-nested-controlle.patch @@ -0,0 +1,184 @@ +From: Michal Privoznik <mpriv...@redhat.com> +Date: Fri, 16 Apr 2021 16:39:14 +0200 +Subject: vircgroup: Fix virCgroupKillRecursive() wrt nested controllers +MIME-Version: 1.0 +Content-Type: text/plain; charset="utf-8" +Content-Transfer-Encoding: 8bit + +I've encountered the following bug, but only on Gentoo with +systemd and CGroupsV2. I've started an LXC container successfully +but destroying it reported the following error: + + error: Failed to destroy domain 'amd64' + error: internal error: failed to get cgroup backend for 'pathOfController' + +Debugging showed, that CGroup hierarchy is full of surprises: + +/sys/fs/cgroup/machine.slice/machine-lxc\x2d861\x2damd64.scope/ +└── libvirt + ├── dev-hugepages.mount + ├── dev-mqueue.mount + ├── init.scope + ├── sys-fs-fuse-connections.mount + ├── sys-kernel-config.mount + ├── sys-kernel-debug.mount + ├── sys-kernel-tracing.mount + ├── system.slice + │ ├── console-getty.service + │ ├── dbus.service + │ ├── system-getty.slice + │ ├── system-modprobe.slice + │ ├── systemd-journald.service + │ ├── systemd-logind.service + │ └── tmp.mount + └── user.slice + +For comparison, here's the same container on recent Rawhide: + +/sys/fs/cgroup/machine.slice/machine-lxc\x2d13550\x2damd64.scope/ +└── libvirt + +Anyway, those nested directories should not be a problem, because +virCgroupKillRecursiveInternal() removes them recursively, right? +Sort of. The function really does remove nested directories, but +it assumes that every directory has the same controller as the +rest. Just take a look at virCgroupV2KillRecursive() - it gets +'Any' controller (the first one it found in ".scope") and then +passes it to virCgroupKillRecursiveInternal(). + +This assumption is not true though. The controllers found in +".scope" are the following: + + cpuset cpu io memory pids + +while "libvirt" has fewer: + + cpuset cpu io memory + +Up until now it's not problem, because of how we order +controllers internally - "cpu" is the first and thus picking +"Any" controller returns just that. But the rest of directories +has no controllers, their "cgroup.controllers" is just empty. + +What fixes the bug is dropping @controller argument from +virCgroupKillRecursiveInternal() and letting each iteration work +pick its own controller. + +Signed-off-by: Michal Privoznik <mpriv...@redhat.com> +Reviewed-by: Pavel Hrdina <phrd...@redhat.com> +(cherry picked from commit ea7d0ca37cce76e1327945c4864b996d7fd6d2e6) +--- + src/util/vircgroup.c | 25 +++++++++++++++++++++++-- + src/util/vircgrouppriv.h | 1 - + src/util/vircgroupv1.c | 7 +------ + src/util/vircgroupv2.c | 7 +------ + 4 files changed, 25 insertions(+), 15 deletions(-) + +diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c +index 15071d8..2bd3a17 100644 +--- a/src/util/vircgroup.c ++++ b/src/util/vircgroup.c +@@ -1380,6 +1380,24 @@ virCgroupHasController(virCgroupPtr cgroup, int controller) + } + + ++static int ++virCgroupGetAnyController(virCgroup *cgroup) ++{ ++ size_t i; ++ ++ for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { ++ if (!cgroup->backends[i]) ++ continue; ++ ++ return cgroup->backends[i]->getAnyController(cgroup); ++ } ++ ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", ++ _("Unable to get any controller")); ++ return -1; ++} ++ ++ + int + virCgroupPathOfController(virCgroupPtr group, + unsigned int controller, +@@ -2548,11 +2566,11 @@ int + virCgroupKillRecursiveInternal(virCgroupPtr group, + int signum, + GHashTable *pids, +- int controller, + const char *taskFile, + bool dormdir) + { + int rc; ++ int controller; + bool killedAny = false; + g_autofree char *keypath = NULL; + g_autoptr(DIR) dp = NULL; +@@ -2561,6 +2579,9 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, + VIR_DEBUG("group=%p signum=%d pids=%p", + group, signum, pids); + ++ if ((controller = virCgroupGetAnyController(group)) < 0) ++ return -1; ++ + if (virCgroupPathOfController(group, controller, "", &keypath) < 0) + return -1; + +@@ -2593,7 +2614,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group, + return -1; + + if ((rc = virCgroupKillRecursiveInternal(subgroup, signum, pids, +- controller, taskFile, true)) < 0) ++ taskFile, true)) < 0) + return -1; + if (rc == 1) + killedAny = true; +diff --git a/src/util/vircgrouppriv.h b/src/util/vircgrouppriv.h +index 85ba539..84f1104 100644 +--- a/src/util/vircgrouppriv.h ++++ b/src/util/vircgrouppriv.h +@@ -128,6 +128,5 @@ int virCgroupRemoveRecursively(char *grppath); + int virCgroupKillRecursiveInternal(virCgroupPtr group, + int signum, + GHashTable *pids, +- int controller, + const char *taskFile, + bool dormdir); +diff --git a/src/util/vircgroupv1.c b/src/util/vircgroupv1.c +index 2b4d625..5bbfa2e 100644 +--- a/src/util/vircgroupv1.c ++++ b/src/util/vircgroupv1.c +@@ -771,12 +771,7 @@ virCgroupV1KillRecursive(virCgroupPtr group, + int signum, + GHashTable *pids) + { +- int controller = virCgroupV1GetAnyController(group); +- +- if (controller < 0) +- return -1; +- +- return virCgroupKillRecursiveInternal(group, signum, pids, controller, ++ return virCgroupKillRecursiveInternal(group, signum, pids, + "tasks", false); + } + +diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c +index 4a239f0..acc1bd6 100644 +--- a/src/util/vircgroupv2.c ++++ b/src/util/vircgroupv2.c +@@ -543,12 +543,7 @@ virCgroupV2KillRecursive(virCgroupPtr group, + int signum, + GHashTable *pids) + { +- int controller = virCgroupV2GetAnyController(group); +- +- if (controller < 0) +- return -1; +- +- return virCgroupKillRecursiveInternal(group, signum, pids, controller, ++ return virCgroupKillRecursiveInternal(group, signum, pids, + "cgroup.threads", false); + } + diff --git a/debian/patches/series b/debian/patches/series index 9fb796e49d..dae748e4ee 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -1,5 +1,6 @@ backport/apparmor-let-image-label-setting-loop-over-backing-files.patch backport/meson-Fix-cross-building-of-dtrace-probes.patch +backport/Fix-reboot-command-for-LXC-containers.patch revert/systemd-Revert-remote-Add-libvirtd-dependency-to-virt-gue.patch forward/Skip-vircgrouptest.patch forward/Reduce-udevadm-settle-timeout-to-10-seconds.patch @@ -11,3 +12,4 @@ debian/apparmor_profiles_local_include.patch debian/Set-defaults-for-zfs-tools.patch debian/Revert-m4-virt-xdr-rewrite-XDR-check.patch debian/Use-sensible-editor-by-default.patch +backport/vircgroup-Fix-virCgroupKillRecursive-wrt-nested-controlle.patch diff --git a/debian/salsa-ci.yml b/debian/salsa-ci.yml index 8be0072d23..d2748c73c2 100644 --- a/debian/salsa-ci.yml +++ b/debian/salsa-ci.yml @@ -4,7 +4,7 @@ stages: variables: # Default docker image to use - LV_DOCKER_IMAGE: debian:unstable + LV_DOCKER_IMAGE: debian:bullseye LV_WORKING_DIR: $CI_PROJECT_DIR/debian/output build-debian-package:
