From 8019f7322d6b4af0be11c246b6a53670ea156883 Mon Sep 17 00:00:00 2001 From: bin Date: Thu, 27 May 2021 14:57:29 +0800 Subject: [PATCH 1/3] virtiofsd: Fix file descriptors leak and return correct PID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit will fix two problems: - Virtiofsd process ID returned to the caller will always be 0, the pid var is never being assigned a value. - Socket listen fd may leak in case of failure of starting virtiofsd process. This is a port of https://github.com/kata-containers/kata-containers/commit/be9ca0d58b2b777beda02f59e443c1dc67d0b5f7 Fixes: #1931 Signed-off-by: bin (cherry picked from commit 773deca2f6507a22f66843a2a7c9fbafe6a06cb2) Signed-off-by: Fabiano Fidêncio --- src/runtime/virtcontainers/virtiofsd.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/runtime/virtcontainers/virtiofsd.go b/src/runtime/virtcontainers/virtiofsd.go index 9b4bc27aa..be2c0e069 100644 --- a/src/runtime/virtcontainers/virtiofsd.go +++ b/src/runtime/virtcontainers/virtiofsd.go @@ -75,6 +75,8 @@ func (v *virtiofsd) getSocketFD() (*os.File, error) { if err != nil { return nil, err } + + // no longer needed since fd is a dup defer listener.Close() listener.SetUnlinkOnClose(false) @@ -98,6 +100,7 @@ func (v *virtiofsd) Start(ctx context.Context) (int, error) { if err != nil { return 0, err } + defer socketFD.Close() cmd.ExtraFiles = append(cmd.ExtraFiles, socketFD) @@ -128,7 +131,7 @@ func (v *virtiofsd) Start(ctx context.Context) (int, error) { v.wait = waitVirtiofsReady } - return pid, socketFD.Close() + return cmd.Process.Pid, nil } func (v *virtiofsd) Stop(ctx context.Context) error { From a05e137710d4f92bc454eed839ca7ef10ea34f1f Mon Sep 17 00:00:00 2001 From: "fupan.lfp" Date: Fri, 28 May 2021 15:06:39 +0800 Subject: [PATCH 2/3] agent: re-enable the standard SIGPIPE behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Rust standard library had suppressed the default SIGPIPE behavior, see https://github.com/rust-lang/rust/pull/13158. Since the parent's signal handler would be inherited by it's child process, thus we should re-enable the standard SIGPIPE behavior as a workaround. Fixes: #1887 Signed-off-by: fupan.lfp (cherry picked from commit 0ae364c8ebbe1ce7af2965f47f6b5cd7768cc0ae) Signed-off-by: Fabiano Fidêncio --- src/agent/src/main.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/agent/src/main.rs b/src/agent/src/main.rs index 595951bc5..d7d2aaf19 100644 --- a/src/agent/src/main.rs +++ b/src/agent/src/main.rs @@ -248,6 +248,7 @@ fn main() -> std::result::Result<(), Box> { } if args.len() == 2 && args[1] == "init" { + reset_sigpipe(); rustjail::container::init_child(); exit(0); } @@ -358,5 +359,16 @@ fn sethostname(hostname: &OsStr) -> Result<()> { } } +// The Rust standard library had suppressed the default SIGPIPE behavior, +// see https://github.com/rust-lang/rust/pull/13158. +// Since the parent's signal handler would be inherited by it's child process, +// thus we should re-enable the standard SIGPIPE behavior as a workaround to +// fix the issue of https://github.com/kata-containers/kata-containers/issues/1887. +fn reset_sigpipe() { + unsafe { + libc::signal(libc::SIGPIPE, libc::SIG_DFL); + } +} + use crate::config::AgentConfig; use std::os::unix::io::{FromRawFd, RawFd}; From 915fea7b1f8781ff251b5387785863e17668ffc1 Mon Sep 17 00:00:00 2001 From: "fupan.lfp" Date: Fri, 21 May 2021 17:55:44 +0800 Subject: [PATCH 3/3] cgroup: fix the issue of set mem.limit and mem.swap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When update memory limit, we should adapt the write sequence for memory and swap memory, so it won't fail because the new value and the old value don't fit kernel's validation. Fixes: #1917 Signed-off-by: fupan.lfp (cherry picked from commit 30f4834c5bc7769bdc4d88f1115b7b16f850f289) Signed-off-by: Fabiano Fidêncio --- src/agent/rustjail/src/cgroups/fs/mod.rs | 38 +++++++++++++++++++----- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/src/agent/rustjail/src/cgroups/fs/mod.rs b/src/agent/rustjail/src/cgroups/fs/mod.rs index 7f41cb4dd..04aa57537 100644 --- a/src/agent/rustjail/src/cgroups/fs/mod.rs +++ b/src/agent/rustjail/src/cgroups/fs/mod.rs @@ -349,14 +349,34 @@ fn set_memory_resources(cg: &cgroups::Cgroup, memory: &LinuxMemory, update: bool mem_controller.set_kmem_limit(-1)?; } - set_resource!(mem_controller, set_limit, memory, limit); - set_resource!(mem_controller, set_soft_limit, memory, reservation); - set_resource!(mem_controller, set_kmem_limit, memory, kernel); - set_resource!(mem_controller, set_tcp_limit, memory, kernel_tcp); + // If the memory update is set to -1 we should also + // set swap to -1, it means unlimited memory. + let mut swap = memory.swap.unwrap_or(0); + if memory.limit == Some(-1) { + swap = -1; + } - if let Some(swap) = memory.swap { - // set memory swap - let swap = if cg.v2() { + if memory.limit.is_some() && swap != 0 { + let memstat = get_memory_stats(cg) + .into_option() + .ok_or_else(|| anyhow!("failed to get the cgroup memory stats"))?; + let memusage = memstat.get_usage(); + + // When update memory limit, the kernel would check the current memory limit + // set against the new swap setting, if the current memory limit is large than + // the new swap, then set limit first, otherwise the kernel would complain and + // refused to set; on the other hand, if the current memory limit is smaller than + // the new swap, then we should set the swap first and then set the memor limit. + if swap == -1 || memusage.get_limit() < swap as u64 { + mem_controller.set_memswap_limit(swap)?; + set_resource!(mem_controller, set_limit, memory, limit); + } else { + set_resource!(mem_controller, set_limit, memory, limit); + mem_controller.set_memswap_limit(swap)?; + } + } else { + set_resource!(mem_controller, set_limit, memory, limit); + swap = if cg.v2() { convert_memory_swap_to_v2_value(swap, memory.limit.unwrap_or(0))? } else { swap @@ -366,6 +386,10 @@ fn set_memory_resources(cg: &cgroups::Cgroup, memory: &LinuxMemory, update: bool } } + set_resource!(mem_controller, set_soft_limit, memory, reservation); + set_resource!(mem_controller, set_kmem_limit, memory, kernel); + set_resource!(mem_controller, set_tcp_limit, memory, kernel_tcp); + if let Some(swappiness) = memory.swappiness { if (0..=100).contains(&swappiness) { mem_controller.set_swappiness(swappiness as u64)?;