runtime-rs: Introduce kata-sys-util library crate #3306

jiangliu · 2021-12-19T15:35:00Z

With the runtime-rs related work, there will be code shared by multiple Kata Containers components, or even some crates may be published and consumed by other projects. So introduce kata-types and kata-sys-util to crates to host shared data structures and common code to access system services.

liubin · 2021-12-20T12:37:21Z

@jiangliu please rebase to include #3315 to pass the CI.

src/libs/types-rs/src/container.rs

src/libs/types-rs/src/k8s.rs

src/libs/types-rs/src/lib.rs

src/libs/sys-util-rs/src/numa.rs

src/libs/sys-util-rs/Cargo.toml

src/libs/sys-util-rs/src/cgroup.rs

src/libs/sys-util-rs/src/mount.rs

src/libs/sys-util-rs/README.md

src/libs/sys-util-rs/src/cgroup.rs

fidencio · 2022-01-26T12:39:56Z

A head's up that may affect your PR.

In the Architecture Committee meeting from January 25th, 2022, the Architecture Committee has agreed on using the "Dismiss stale pull request approvals when new commits are pushed" configuration from GitHub. It basically means that if your PR has been rebased or updated, the approvals given will be erased.

In order to minimize traumas due to the new approach, please, consider adding a note on the changes done before the rebase / force-push, and also pinging the reviewers for a subsequent round of reviews.

Thanks for your understanding!

Related issue: kata-containers/community#249

src/libs/kata-sys-util/src/cgroup.rs

jodh-intel

Thanks @jiangliu. The only blocking comment I have is the missing wait() call in handle_exit_status().

src/libs/kata-sys-util/src/fs.rs

src/libs/kata-sys-util/src/hooks.rs

jodh-intel · 2022-01-27T15:01:10Z

src/libs/kata-sys-util/src/hooks.rs

+        }
+        executor.execute_and_wait(&mut popen)?;
+        info!(sl!(), "hook {} finished", hook.path);
+        self.states.insert(hook.into(), HookState::Done);


Idea: How about logging how long the hook took to run (since random hooks could have a huge impact on performance)?

src/libs/kata-sys-util/src/hooks.rs

src/libs/kata-sys-util/src/mount.rs

jodh-intel · 2022-02-10T16:12:46Z

Hi @jiangliu - any update on this?

jiangliu · 2022-02-11T02:44:27Z

Hi @jiangliu - any update on this?

Back to work now, will update it soon:)

jiangliu · 2022-02-12T03:02:33Z

Hi @jiangliu - any update on this?

@jodh-intel @dgibson @lifupan @liubin Sorry for delay, I have updated the PR according to review comments:)

bergwolf

lgtm! @dgibson @jodh-intel Do you have more comments on the PR?

lifupan · 2022-02-18T07:10:34Z

Hi @jodh-intel @dgibson @fidencio , do you guys have any comments?

jodh-intel

Thanks @jiangliu - A few comments, mostly just nits. I'm assuming some of the code here will change when #3462 lands?

src/libs/kata-sys-util/src/fs.rs

src/libs/kata-sys-util/src/hooks.rs

jodh-intel · 2022-02-14T16:59:38Z

src/libs/kata-sys-util/src/hooks.rs

+                info!(sl!(), "hook {} exec stdout: {}", self.hook.path, out);
+            }
+        }
+        self.timeout = 1;


Add a comment

// Give a grace period for `execute_and_wait()`.

jodh-intel · 2022-02-14T17:01:02Z

src/libs/kata-sys-util/src/hooks.rs

+        };
+        let key1 = HookKey::from(&hook);
+
+        let hook = oci::Hook {


These tests would be clearer and more consistent if you used the table based approach.

src/libs/kata-sys-util/src/mount.rs

jodh-intel · 2022-02-18T09:33:52Z

src/libs/kata-sys-util/src/mount.rs

+pub fn create_mount_destination<S: AsRef<Path>, D: AsRef<Path>, R: AsRef<Path>>(
+    src: S,
+    dst: D,
+    _root: R,


Is this going to be enabled when #3462 lands?

Yes, there are several TODOs in mount.rs, which will be handled after merging #3462.

jodh-intel · 2022-02-18T09:34:14Z

src/libs/kata-sys-util/src/mount.rs

+/// To ensure security, the `create_mount_destination()` function takes an extra parameter `root`,
+/// which is used to ensure that `dst` is within the specified directory. And a safe version of
+/// `PathBuf` is returned to avoid TOCTTOU type of flaws.
+pub fn create_mount_destination<S: AsRef<Path>, D: AsRef<Path>, R: AsRef<Path>>(


For safety, shouldn't this function require that all specified paths are absolute?

it's designed to support the container rootfs mount scenario and simplify the caller:)

src/libs/kata-sys-util/src/mount.rs

jiangliu · 2022-02-19T02:39:02Z

Thanks @jiangliu - A few comments, mostly just nits. I'm assuming some of the code here will change when #3462 lands?

Yes, we found the secure issue when developing this PR, and then developed #3462. So let's get it done step by step:)

The kata-sys-util crate is a collection of modules that provides helpers and utilities used by multiple Kata Containers components. Fixes: kata-containers#3305 Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>

Add some wrappers for mount and fs syscall. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com> Signed-off-by: Bin Liu <bin@hyper.sh> Signed-off-by: Fupan Li <lifupan@gmail.com> Signed-off-by: Huamin Tang <huamin.thm@alibaba-inc.com> Signed-off-by: Lei Wang <wllenyj@linux.alibaba.com> Signed-off-by: Quanwei Zhou <quanweiZhou@linux.alibaba.com>

Add utilities to manipulate cgroup, currently only v1 is supported. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com> Signed-off-by: He Rongguang <herongguang@linux.alibaba.com> Signed-off-by: Jiahuan Chao <jhchao@linux.alibaba.com> Signed-off-by: Qingyuan Hou <qingyuan.hou@linux.alibaba.com> Signed-off-by: Quanwei Zhou <quanweiZhou@linux.alibaba.com> Signed-off-by: Tim Zhang <tim@hyper.sh>

Add utilities to parse NUMA information. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com> Signed-off-by: Qingyuan Hou <qingyuan.hou@linux.alibaba.com> Signed-off-by: Simon Guo <wei.guo.simon@linux.alibaba.com>

Implement reflink_copy() to copy file by reflink, and fallback to normal file copy. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com> Signed-off-by: Eryu Guan <eguan@linux.alibaba.com>

Introduce get_devid() to get major/minor number of a block device. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com> Signed-off-by: Eryu Guan <eguan@linux.alibaba.com>

Add function to detect and update K8s emptyDir volume. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com> Signed-off-by: Qingyuan Hou <qingyuan.hou@linux.alibaba.com>

Provide functions to execute OCI hooks. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com> Signed-off-by: Bin Liu <bin@hyper.sh> Signed-off-by: Huamin Tang <huamin.thm@alibaba-inc.com> Signed-off-by: Lei Wang <wllenyj@linux.alibaba.com> Signed-off-by: Quanwei Zhou <quanweiZhou@linux.alibaba.com>

jodh-intel

Thanks @jiangliu.

lgtm

bergwolf · 2022-02-21T11:48:23Z

@dgibson has been inactive on the PR for more than a month since his comments were last addressed. I'm going to dismiss his review and merge the PR. @dgibson when you come back, please open new issues on any changes that you would like to see happening in this area. Thanks!

jiangliu requested a review from a team as a code owner December 19, 2021 15:35

fidencio changed the title ~~Introduce kata-types and kata-sys-util crates~~ runtime-rs: Introduce kata-types and kata-sys-util crates Dec 20, 2021

dgibson linked an issue Dec 21, 2021 that may be closed by this pull request

Introduce rust crates to host common code #3305

Closed

dgibson reviewed Dec 21, 2021

View reviewed changes

jodh-intel reviewed Dec 24, 2021

View reviewed changes

jiangliu force-pushed the sys-util branch 7 times, most recently from ad0cff6 to 61b5f83 Compare December 25, 2021 12:59

jiangliu force-pushed the sys-util branch 2 times, most recently from f1aca2e to c540afc Compare January 6, 2022 02:10

jiangliu closed this Jan 6, 2022

jiangliu reopened this Jan 6, 2022

jiangliu force-pushed the sys-util branch 13 times, most recently from 98f12a9 to d16bcf8 Compare January 6, 2022 10:06

bergwolf reviewed Jan 27, 2022

View reviewed changes

src/libs/kata-sys-util/src/cgroup.rs Show resolved Hide resolved

jodh-intel reviewed Jan 27, 2022

View reviewed changes

jiangliu force-pushed the sys-util branch from 56a4bf1 to 981a73a Compare February 11, 2022 08:27

jiangliu requested review from jodh-intel, bergwolf and lifupan February 14, 2022 06:05

bergwolf approved these changes Feb 15, 2022

View reviewed changes

lifupan approved these changes Feb 16, 2022

View reviewed changes

jodh-intel reviewed Feb 18, 2022

View reviewed changes

jiangliu added 8 commits February 19, 2022 10:44

libs/sys-util: add kata-sys-util crate under src/libs

fab70eb

The kata-sys-util crate is a collection of modules that provides helpers and utilities used by multiple Kata Containers components. Fixes: kata-containers#3305 Signed-off-by: Liu Jiang <gerry@linux.alibaba.com>

libs/sys-util: add utilities to parse NUMA information

5597bac

Add utilities to parse NUMA information. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com> Signed-off-by: Qingyuan Hou <qingyuan.hou@linux.alibaba.com> Signed-off-by: Simon Guo <wei.guo.simon@linux.alibaba.com>

libs/sys-util: implement reflink_copy()

505d0ad

Implement reflink_copy() to copy file by reflink, and fallback to normal file copy. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com> Signed-off-by: Eryu Guan <eguan@linux.alibaba.com>

libs/sys-util: introduce function to get device id

ce046f9

Introduce get_devid() to get major/minor number of a block device. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com> Signed-off-by: Eryu Guan <eguan@linux.alibaba.com>

libs/sys-util: add function to detect and update K8s emptyDir volume

ff265d6

Add function to detect and update K8s emptyDir volume. Signed-off-by: Liu Jiang <gerry@linux.alibaba.com> Signed-off-by: Qingyuan Hou <qingyuan.hou@linux.alibaba.com>

jiangliu force-pushed the sys-util branch from 981a73a to 8d29d85 Compare February 19, 2022 07:36

jiangliu requested a review from jodh-intel February 21, 2022 07:11

jodh-intel approved these changes Feb 21, 2022

View reviewed changes

lifupan merged commit 076e658 into kata-containers:runtime-rs Feb 21, 2022

runtime-rs automation moved this from WiP to Done Feb 21, 2022

jiangliu mentioned this pull request Feb 21, 2022

Introduce rust crates to host common code #3305

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runtime-rs: Introduce kata-sys-util library crate #3306

runtime-rs: Introduce kata-sys-util library crate #3306

jiangliu commented Dec 19, 2021 •

edited

liubin commented Dec 20, 2021

fidencio commented Jan 26, 2022

jodh-intel left a comment

jodh-intel Jan 27, 2022

jodh-intel commented Feb 10, 2022

jiangliu commented Feb 11, 2022

jiangliu commented Feb 12, 2022

bergwolf left a comment

lifupan commented Feb 18, 2022

jodh-intel left a comment

jodh-intel Feb 14, 2022

jiangliu Feb 19, 2022

jodh-intel Feb 14, 2022

jodh-intel Feb 18, 2022

jiangliu Feb 19, 2022

jodh-intel Feb 18, 2022

jiangliu Feb 19, 2022

jiangliu commented Feb 19, 2022

jodh-intel left a comment

bergwolf commented Feb 21, 2022

runtime-rs: Introduce kata-sys-util library crate #3306

runtime-rs: Introduce kata-sys-util library crate #3306

Conversation

jiangliu commented Dec 19, 2021 • edited

liubin commented Dec 20, 2021

fidencio commented Jan 26, 2022

jodh-intel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jodh-intel commented Feb 10, 2022

jiangliu commented Feb 11, 2022

jiangliu commented Feb 12, 2022

bergwolf left a comment

Choose a reason for hiding this comment

lifupan commented Feb 18, 2022

jodh-intel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiangliu commented Feb 19, 2022

jodh-intel left a comment

Choose a reason for hiding this comment

bergwolf commented Feb 21, 2022

jiangliu commented Dec 19, 2021 •

edited