From f8e3b11496bd6d602a690535c4a3bb32bb8e9744 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Wed, 14 Jun 2023 17:20:13 +0200 Subject: Drop FEATURES=cgroup, i.e., v1 cgroup usage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove portage's usage of Linux version 1 cgroups, which are itself superseded by version 2 cgroups. This basically reverts b01a1b90d8c5 ("Add FEATURES=cgroup to isolate phase processes."). Portage's usage of version 1 cgroups has caused some issues in the past. For example https://bugs.gentoo.org/894398, where LXD got confused by the existence of the version 1 cgroup created by portage. Arguably, this could be considered a bug in LXD, but with FEATURES=pid-sandbox, as better alternative to FEATURES=cgroup exists. And removing the code for FEATURES=cgroup reduces portage's code size, which is also a plus. Bug: https://bugs.gentoo.org/894398 Signed-off-by: Florian Schmaus Reviewed-by: Michał Górny Closes: https://github.com/gentoo/portage/pull/1057 Signed-off-by: Sam James --- NEWS | 5 ++ lib/_emerge/AbstractEbuildProcess.py | 88 ------------------------------------ lib/_emerge/SpawnProcess.py | 63 -------------------------- lib/portage/const.py | 1 - lib/portage/process.py | 14 ------ man/make.conf.5 | 4 -- 6 files changed, 5 insertions(+), 170 deletions(-) diff --git a/NEWS b/NEWS index 4c54ba382..82e4c8780 100644 --- a/NEWS +++ b/NEWS @@ -18,6 +18,11 @@ Bug fixes: portage-3.0.49 (2023-06-21) -------------- +Breaking changes: +* FEATURES=cgroup was removed since it was based on version 1 cgroups, which + caused some issues and version 1 cgroups are obsolete. Use + FEATURES=pid-sandbox instead. + Bug fixes: * Adjust write tests for DISTDIR and PORTAGE_TMPDIR to work with automount directories (bug #485100, bug #890812). diff --git a/lib/_emerge/AbstractEbuildProcess.py b/lib/_emerge/AbstractEbuildProcess.py index be257a620..97408806c 100644 --- a/lib/_emerge/AbstractEbuildProcess.py +++ b/lib/_emerge/AbstractEbuildProcess.py @@ -3,10 +3,7 @@ import functools import io -import platform import stat -import subprocess -import tempfile import textwrap from _emerge.SpawnProcess import SpawnProcess from _emerge.EbuildBuildDir import EbuildBuildDir @@ -80,91 +77,6 @@ class AbstractEbuildProcess(SpawnProcess): self._async_wait() return - # Check if the cgroup hierarchy is in place. If it's not, mount it. - if ( - os.geteuid() == 0 - and platform.system() == "Linux" - and "cgroup" in self.settings.features - and self.phase not in _global_pid_phases - ): - cgroup_root = "/sys/fs/cgroup" - cgroup_portage = os.path.join(cgroup_root, "portage") - - try: - # cgroup tmpfs - if not os.path.ismount(cgroup_root): - # we expect /sys/fs to be there already - if not os.path.isdir(cgroup_root): - os.mkdir(cgroup_root, 0o755) - subprocess.check_call( - [ - "mount", - "-t", - "tmpfs", - "-o", - "rw,nosuid,nodev,noexec,mode=0755", - "tmpfs", - cgroup_root, - ] - ) - - # portage subsystem - if not os.path.ismount(cgroup_portage): - if not os.path.isdir(cgroup_portage): - os.mkdir(cgroup_portage, 0o755) - subprocess.check_call( - [ - "mount", - "-t", - "cgroup", - "-o", - "rw,nosuid,nodev,noexec,none,name=portage", - "tmpfs", - cgroup_portage, - ] - ) - with open(os.path.join(cgroup_portage, "release_agent"), "w") as f: - f.write( - os.path.join( - self.settings["PORTAGE_BIN_PATH"], - "cgroup-release-agent", - ) - ) - with open( - os.path.join(cgroup_portage, "notify_on_release"), "w" - ) as f: - f.write("1") - else: - # Update release_agent if it no longer exists, because - # it refers to a temporary path when portage is updating - # itself. - release_agent = os.path.join(cgroup_portage, "release_agent") - try: - with open(release_agent) as f: - release_agent_path = f.readline().rstrip("\n") - except OSError: - release_agent_path = None - - if release_agent_path is None or not os.path.exists( - release_agent_path - ): - with open(release_agent, "w") as f: - f.write( - os.path.join( - self.settings["PORTAGE_BIN_PATH"], - "cgroup-release-agent", - ) - ) - - cgroup_path = tempfile.mkdtemp( - dir=cgroup_portage, - prefix=f"{self.settings['CATEGORY']}:{self.settings['PF']}.", - ) - except (subprocess.CalledProcessError, OSError): - pass - else: - self.cgroup = cgroup_path - if self.background: # Automatically prevent color codes from showing up in logs, # since we're not displaying to a terminal anyway. diff --git a/lib/_emerge/SpawnProcess.py b/lib/_emerge/SpawnProcess.py index db812a790..8f21f05c0 100644 --- a/lib/_emerge/SpawnProcess.py +++ b/lib/_emerge/SpawnProcess.py @@ -1,10 +1,7 @@ # Copyright 2008-2021 Gentoo Authors # Distributed under the terms of the GNU General Public License v2 -import errno import functools -import logging -import signal import sys from _emerge.SubProcess import SubProcess @@ -12,7 +9,6 @@ import portage from portage import os from portage.const import BASH_BINARY from portage.output import EOutput -from portage.util import writemsg_level from portage.util._async.BuildLogger import BuildLogger from portage.util._async.PipeLogger import PipeLogger from portage.util._pty import _create_pty_or_pipe @@ -39,7 +35,6 @@ class SpawnProcess(SubProcess): "path_lookup", "pre_exec", "close_fds", - "cgroup", "unshare_ipc", "unshare_mount", "unshare_pid", @@ -235,9 +230,6 @@ class SpawnProcess(SubProcess): def _unregister(self): SubProcess._unregister(self) - if self.cgroup is not None: - self._cgroup_cleanup() - self.cgroup = None if self._main_task is not None: self._main_task.done() or self._main_task.cancel() @@ -249,61 +241,6 @@ class SpawnProcess(SubProcess): self._main_task_cancel = None self._main_task.cancel() SubProcess._cancel(self) - self._cgroup_cleanup() - - def _cgroup_cleanup(self): - if self.cgroup: - - def get_pids(cgroup): - try: - with open(os.path.join(cgroup, "cgroup.procs")) as f: - return [int(p) for p in f.read().split()] - except OSError: - # removed by cgroup-release-agent - return [] - - def kill_all(pids, sig): - for p in pids: - try: - os.kill(p, sig) - except OSError as e: - if e.errno == errno.EPERM: - # Reported with hardened kernel (bug #358211). - writemsg_level( - f"!!! kill: ({p}) - Operation not permitted\n", - level=logging.ERROR, - noiselevel=-1, - ) - elif e.errno != errno.ESRCH: - raise - - # step 1: kill all orphans (loop in case of new forks) - remaining = self._CGROUP_CLEANUP_RETRY_MAX - while remaining: - remaining -= 1 - pids = get_pids(self.cgroup) - if pids: - kill_all(pids, signal.SIGKILL) - else: - break - - if pids: - msg = [] - msg.append( - "Failed to kill pid(s) in " - f"'{os.path.join(self.cgroup, 'cgroup.procs')}': " - f"{' '.join(str(pid) for pid in pids)}" - ) - - self._elog("eerror", msg) - - # step 2: remove the cgroup - try: - os.rmdir(self.cgroup) - except OSError: - # it may be removed already, or busy - # we can't do anything good about it - pass def _elog(self, elog_funcname, lines): elog_func = getattr(EOutput(), elog_funcname) diff --git a/lib/portage/const.py b/lib/portage/const.py index 10a208ceb..f7168d996 100644 --- a/lib/portage/const.py +++ b/lib/portage/const.py @@ -139,7 +139,6 @@ SUPPORTED_FEATURES = frozenset( "candy", "case-insensitive-fs", "ccache", - "cgroup", "chflags", "clean-logs", "collision-protect", diff --git a/lib/portage/process.py b/lib/portage/process.py index 4a1baedc6..63656cfbe 100644 --- a/lib/portage/process.py +++ b/lib/portage/process.py @@ -295,7 +295,6 @@ def spawn( unshare_ipc=False, unshare_mount=False, unshare_pid=False, - cgroup=None, warn_on_large_env=False, ): """ @@ -344,8 +343,6 @@ def spawn( @type unshare_mount: Boolean @param unshare_pid: If True, PID ns will be unshared from the spawned process @type unshare_pid: Boolean - @param cgroup: CGroup path to bind the process to - @type cgroup: String logfile requires stdout and stderr to be assigned to this process (ie not pointed somewhere else.) @@ -479,7 +476,6 @@ def spawn( unshare_mount, unshare_pid, unshare_flags, - cgroup, ) except SystemExit: raise @@ -656,7 +652,6 @@ def _exec( unshare_mount, unshare_pid, unshare_flags, - cgroup, ): """ Execute a given binary with options @@ -694,8 +689,6 @@ def _exec( @type unshare_pid: Boolean @param unshare_flags: Flags for the unshare(2) function @type unshare_flags: Integer - @param cgroup: CGroup path to bind the process to - @type cgroup: String @rtype: None @return: Never returns (calls os.execve) """ @@ -743,13 +736,6 @@ def _exec( _setup_pipes(fd_pipes, close_fds=close_fds, inheritable=True) - # Add to cgroup - # it's better to do it from the child since we can guarantee - # it is done before we start forking children - if cgroup: - with open(os.path.join(cgroup, "cgroup.procs"), "a") as f: - f.write("%d\n" % portage.getpid()) - # Unshare (while still uid==0) if unshare_net or unshare_ipc or unshare_mount or unshare_pid: filename = find_library("c") diff --git a/man/make.conf.5 b/man/make.conf.5 index 85ee88c05..dbab292fb 100644 --- a/man/make.conf.5 +++ b/man/make.conf.5 @@ -434,10 +434,6 @@ like "File not recognized: File truncated"), try recompiling the application with ccache disabled before reporting a bug. Unless you are doing development work, do not enable ccache. .TP -.B cgroup -Use Linux control group to control processes spawned by ebuilds. This allows -emerge to safely kill all subprocesses when ebuild phase exits. -.TP .B clean\-logs Enable automatic execution of the command specified by the PORTAGE_LOGDIR_CLEAN variable. The default PORTAGE_LOGDIR_CLEAN setting will -- cgit v1.2.3-65-gdbad