Skip to content

Add I/O priority#3783

Merged
lifubang merged 1 commit intoopencontainers:mainfrom
utam0k:io-prio
Mar 31, 2024
Merged

Add I/O priority#3783
lifubang merged 1 commit intoopencontainers:mainfrom
utam0k:io-prio

Conversation

@utam0k
Copy link
Copy Markdown
Member

@utam0k utam0k commented Mar 24, 2023

@AkihiroSuda
Copy link
Copy Markdown
Member

Could you squash the commits?

@utam0k
Copy link
Copy Markdown
Member Author

utam0k commented Jun 12, 2023

@AkihiroSuda Sure. Done.

Copy link
Copy Markdown
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left a few comments, mostly nits

@utam0k utam0k force-pushed the io-prio branch 5 times, most recently from 223043c to ca36f1e Compare June 15, 2023 12:17

**Be very careful when passing file descriptors to a container process.** Due
to some Linux kernel (mis)features, a container with access to certain types of
to some Linux kernel misfeatures, a container with access to certain types of
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to fix it to pass the checking code spell.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem relevant to the PR

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But CI won't allow this word... if I edit this file, CI won't go through unless I fix this part.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this hunk. We'll deal with CI later.

@utam0k utam0k force-pushed the io-prio branch 2 times, most recently from 076fa1b to 9f7d118 Compare June 15, 2023 12:30
@utam0k utam0k requested a review from AkihiroSuda June 15, 2023 23:36
@utam0k utam0k force-pushed the io-prio branch 3 times, most recently from a1980b1 to cff9be2 Compare October 21, 2023 05:50
@utam0k utam0k requested a review from AkihiroSuda October 21, 2023 06:12
@lifubang
Copy link
Copy Markdown
Member

lifubang commented Jan 1, 2024

This PR can be merged once you do a rebase and resolve the conflicts, and I can take over this PR if you are in busy status.

@kolyshkin
Copy link
Copy Markdown
Contributor

@utam0k please avoid merge commits -- do a rebase instead

@utam0k
Copy link
Copy Markdown
Member Author

utam0k commented Feb 20, 2024

@utam0k please avoid merge commits -- do a rebase instead

Done ✅

@AkihiroSuda
Copy link
Copy Markdown
Member

@kolyshkin PTAL

@lifubang lifubang dismissed kolyshkin’s stale review March 16, 2024 01:03

All request changes have been addressed.

@lifubang
Copy link
Copy Markdown
Member

@utam0k Please help to do a rebase to main and solve the conflicts. Thanks.

@@ -9,6 +9,7 @@ Spec version | Feature | PR
-------------|------------------------------------------|----------------------------------------------------------
v1.1.0 | `SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV` | [#3862](https://github.com/opencontainers/runc/pull/3862)
v1.1.0 | `.process.ioPriority` | [#3783](https://github.com/opencontainers/runc/pull/3783)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line should be now removed

-------------|------------------------------------------|----------------------------------------------------------
v1.1.0 | `SECCOMP_FILTER_FLAG_WAIT_KILLABLE_RECV` | [#3862](https://github.com/opencontainers/runc/pull/3862)
v1.1.0 | `.process.ioPriority` | [#3783](https://github.com/opencontainers/runc/pull/3783)
v1.1.0 | rsvd hugetlb cgroup | TODO ([#3859](https://github.com/opencontainers/runc/issues/3859))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a separate PR?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to have been implemented in:

Signed-off-by: utam0k <k0ma@utam0k.jp>
Priority: tc.priority,
}
config := &configs.Config{
Rootfs: "/var",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This value looks unusual, but it doesn’t matter here)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems we need to set an existing dir. I tried to delete this line and got following error.

 validator_test.go|869| iopriority: 0, expected nil, got error invalid rootfs: stat : no such file or directory

Copy link
Copy Markdown
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Copy link
Copy Markdown
Member

@lifubang lifubang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank!

@lifubang lifubang merged commit ea38465 into opencontainers:main Mar 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants