fix: ratime and rnostrictatime mount options failing with EINVAL#3467
fix: ratime and rnostrictatime mount options failing with EINVAL#3467kechigon wants to merge 2 commits intoyouki-dev:mainfrom
Conversation
|
Todo: Create or update the corresponding unit tests within the modified module. |
Signed-off-by: Keigo Kurita <keig0xurita@gmail.com>
Signed-off-by: Keigo Kurita <keig0xurita@gmail.com>
kechigon
force-pushed
the
fix_issue_3399
branch
from
8b05e0d to
a5bc94b
Compare
saku3
left a comment
saku3
left a comment
There was a problem hiding this comment.
Thanks for the PR.
I left some comments, so could you please take a look?
Also, would it be possible to add an e2e test as well?
Please refer to the following README:
https://github.com/youki-dev/youki/tree/main/tests/contest/contest
I think it should be sufficient to add ratime and rnostrictatime cases to the existing mount_setattr test cases.
https://github.com/youki-dev/youki/blob/main/tests/contest/contest/src/tests/mounts_recursive/mod.rs
| if let Some(mount_attr) = &mut mount_attr { | ||
| if is_clear { | ||
| mount_attr.attr_clr |= flag; | ||
| if flag != 0 && flag & linux::MOUNT_ATTR__ATIME == flag { |
There was a problem hiding this comment.
I think it would be better to align this with runc’s implementation.
In other words, rnorelatime should be treated as the recursive form of “clear relatime”.
I do not think we need to make the low-level runtime more complex by handling it as a no-op and skipping it there.
That would also allow us to consolidate the if logic the way runc does.
https://github.com/lifubang/runc/blob/main/libcontainer/specconv/spec_linux.go#L1168
| } | ||
|
|
||
| #[test] | ||
| fn test_parse_mount_atime_set_includes_full_atime_mask_in_attr_clr() -> Result<()> { |
There was a problem hiding this comment.
I think the rnoatime and rstrictatime tests should be split into separate test cases.
|
We have decided not to implement end-to-end testing on my end. |
2 participants
Description
Type of Change
Testing
Related Issues
Related #3399
Additional Context