[LibOS] Implement utime, utimes, utimensat #2131

efu39 · 2025-05-20T12:28:40Z

Description of the changes

This PR implements support for the utime(), utimes(), and utimensat() syscalls in Gramine to provide file timestamp update functionality. The utime() syscall is fully implemented, while utimes() and utimensat() are stubbed with equivalent behavior. These additions enhance Gramine's syscall compatibility and enable several LTP test cases that were previously skipped due to the lack of utimensat() support.
The implementation retains the existing data types for atime, mtime, and ctime in the LibOS file structure, resulting in a timestamp resolution of one second. I took reference of prior discussions (PR #1144 and discussion #942) and implemented with lower resolution to keep the implementation minimal.

How to test this PR?

Tested with LTP Test.

LTP Before PR:
422 passed, 906 skipped

LTP After PR:
436 passed, 892 skipped

This change is

Signed-off-by: Erica Fu <[email protected]>

donporter · 2025-05-20T20:24:48Z

Add to whitelist.

donporter

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions, not enough approvals from maintainers (3 more required), not enough approvals from different teams (2 more required, approved so far: )

libos/src/sys/libos_file.c line 269 at r1 (raw file):

        return ret;

    if (flags != 0 && flags != AT_SYMLINK_NOFOLLOW)

This fix is not immediately germane - the issue is that some of the LTP comments were updated to reflect addition of utime*, and this revealed a trivial fix for this case.

libos/src/sys/libos_file.c line 638 at r1 (raw file):

    return nsec >= 0 && nsec <= 999999999;
}

Add a comment that this is stubbed - does not support any flags, only a subset equivalent to utimes.

libos/src/sys/libos_file.c line 664 at r1 (raw file):

    struct libos_dentry* dent = NULL;

    if (*pathname != '/' && (ret = get_dirfd_dentry(dirfd, &dir)) < 0)

Move above the time query, for a faster failure?

libos/src/sys/libos_file.c line 672 at r1 (raw file):

        goto out;

    time_t atime = 0, mtime = 0;

Shouldn't this check_permissions to write the file?

libos/test/ltp/ltp.cfg line 387 at r1 (raw file):

# no symlink()
[fchownat02]

I see - I think this is why the other change is here. Small enough to retain, or worth splitting the PR?

Signed-off-by: Don Porter <[email protected]>

efu39

Reviewable status: 5 of 6 files reviewed, 5 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @donporter)

libos/src/sys/libos_file.c line 269 at r1 (raw file):

Previously, donporter (Don Porter) wrote…

This fix is not immediately germane - the issue is that some of the LTP comments were updated to reflect addition of utime*, and this revealed a trivial fix for this case.

This is to fix fchownat(). We are able to enable the LTP fchownat testcase after stubbing utimensat(). I can split to another commit if that is preferred. but i think it might be better to be one PR.

libos/src/sys/libos_file.c line 638 at r1 (raw file):

Previously, donporter (Don Porter) wrote…

Add a comment that this is stubbed - does not support any flags, only a subset equivalent to utimes.

Thanks for adding the comment

libos/src/sys/libos_file.c line 664 at r1 (raw file):

Previously, donporter (Don Porter) wrote…

Move above the time query, for a faster failure?

Thanks for updating for me.

libos/src/sys/libos_file.c line 672 at r1 (raw file):

Previously, donporter (Don Porter) wrote…

Shouldn't this check_permissions to write the file?

Thanks again.

libos/test/ltp/ltp.cfg line 387 at r1 (raw file):

Previously, donporter (Don Porter) wrote…

I see - I think this is why the other change is here. Small enough to retain, or worth splitting the PR?

I think same PR is better so fchownat can be enabled and won't fail. but let me know if separate commit is preferred.

donporter

Reviewed 1 of 6 files at r1, all commit messages.
Reviewable status: 5 of 6 files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @efu39)

libos/src/sys/libos_file.c line 269 at r1 (raw file):

Previously, efu39 (Erica Fu) wrote…

This is to fix fchownat(). We are able to enable the LTP fchownat testcase after stubbing utimensat(). I can split to another commit if that is preferred. but i think it might be better to be one PR.

I agree - I think this is small enough to merge in one PR, but others can weigh in if they disagree.

libos/test/ltp/ltp.cfg line 387 at r1 (raw file):

Previously, efu39 (Erica Fu) wrote…

I think same PR is better so fchownat can be enabled and won't fail. but let me know if separate commit is preferred.

Agreed.

a discussion (no related file):
Failures in gramine direct mode:

15:21:33 FAILED test_ltp.py::test_ltp[creat01] - AssertionError: Command ['gramine-dir...
15:21:33 FAILED test_ltp.py::test_ltp[mmap14] - Failed: ['gramine-direct', 'mmap14'] e...
15:21:33 FAILED test_ltp.py::test_ltp[utime01] - Failed: ['gramine-direct', 'utime01']...
15:21:33 FAILED test_ltp.py::test_ltp[utime02] - Failed: ['gramine-direct', 'utime02']...
15:21:33 FAILED test_ltp.py::test_ltp[utime04] - Failed: ['gramine-direct', 'utime04']...
15:21:33 FAILED test_ltp.py::test_ltp[utime05] - Failed: ['gramine-direct', 'utime05']...

efu39

Reviewable status: 5 of 6 files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "fixup! " found in commit messages' one-liners (waiting on @donporter)

a discussion (no related file):

Previously, donporter (Don Porter) wrote…

Failures in gramine direct mode:

15:21:33 FAILED test_ltp.py::test_ltp[creat01] - AssertionError: Command ['gramine-dir...
15:21:33 FAILED test_ltp.py::test_ltp[mmap14] - Failed: ['gramine-direct', 'mmap14'] e...
15:21:33 FAILED test_ltp.py::test_ltp[utime01] - Failed: ['gramine-direct', 'utime01']...
15:21:33 FAILED test_ltp.py::test_ltp[utime02] - Failed: ['gramine-direct', 'utime02']...
15:21:33 FAILED test_ltp.py::test_ltp[utime04] - Failed: ['gramine-direct', 'utime04']...
15:21:33 FAILED test_ltp.py::test_ltp[utime05] - Failed: ['gramine-direct', 'utime05']...

Not sure why I didn't have these failures when I tested it. I'll be out of town next 10 days, I'll check it after coming back.

Signed-off-by: Erica Fu <[email protected]>

efu39

Reviewable status: 4 of 6 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "Merge branch '" and "fixup! " found in commit messages' one-liners (waiting on @donporter)

a discussion (no related file):

Previously, efu39 (Erica Fu) wrote…

Not sure why I didn't have these failures when I tested it. I'll be out of town next 10 days, I'll check it after coming back.

The test failures were caused by the check_permissions() introduced in the fixup commit, so I’ve reverted that change.

libos/src/sys/libos_file.c line 672 at r1 (raw file):

Previously, efu39 (Erica Fu) wrote…

Thanks again.

The test failures were caused by the check_permissions() introduced in the fixup commit, so I’ve reverted that change.

"Gramine has dummy support for file owner and group manipulations. In Gramine, users and groups are dummy; see the “User and group identifiers” section for details. Therefore, chown(), fchownat(), fchown() system calls update UID and GID inside the Gramine environment, but not on host files.
Gramine supports checking permissions on the file via access() and faccessat() system calls. Recall however that users and groups are dummy in Gramine, thus the checks are also largely irrelevant." Given this context from Gramine documentation, it seems that check_permissions() is not required here, as is the case with many other file system-related syscalls.

efu39

Reviewable status: 4 of 6 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "Merge branch '" and "fixup! " found in commit messages' one-liners (waiting on @donporter)

a discussion (no related file):

Previously, efu39 (Erica Fu) wrote…

Not sure why I didn't have these failures when I tested it. I'll be out of town next 10 days, I'll check it after coming back.

The test failures were caused by the check_permissions() introduced in the fixup commit, so I’ve reverted that change.

libos/src/sys/libos_file.c line 672 at r1 (raw file):

Previously, efu39 (Erica Fu) wrote…

Thanks again.

The test failures were caused by the check_permissions() introduced in the fixup commit, so I’ve reverted that change.

"Gramine has dummy support for file owner and group manipulations. In Gramine, users and groups are dummy; see the “User and group identifiers” section for details. Therefore, chown(), fchownat(), fchown() system calls update UID and GID inside the Gramine environment, but not on host files.
Gramine supports checking permissions on the file via access() and faccessat() system calls. Recall however that users and groups are dummy in Gramine, thus the checks are also largely irrelevant." Given this context from Gramine documentation, it seems that check_permissions() is not required here, as is the case with many other file system-related syscalls.

efu39 · 2025-06-17T11:32:02Z

Jenkins, retest this please

donporter

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: OSCAR Lab), "Merge branch '" and "fixup! " found in commit messages' one-liners

donporter

Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (2 more required, approved so far: ), "Merge branch '" and "fixup! " found in commit messages' one-liners

libos/test/ltp/ltp.cfg line 1109 at r3 (raw file):

[mmap12]
skip = yes

This test fails because the VmLck field in /proc/pid/status is dummy. I suggest we re-skip.

https://github.com/linux-test-project/ltp/blob/a908cff70f9389c2dd2bf535976cb179bfa8f340/testcases/kernel/syscalls/mmap/mmap14.c#L25

https://gramine.readthedocs.io/en/latest/devel/features.html#process-and-thread-identifiers

Signed-off-by: Don Porter <[email protected]>

donporter · 2025-07-29T12:05:29Z

Retest Jenkins-Direct-22.04, please. Timeout on fdatasync01 I cannot reproduce locally.

donporter · 2025-07-29T12:06:34Z

Jenkins, retest Jenkins-Direct-22.04 please. Timeout on fdatasync01 I cannot reproduce locally.

[LibOS] Implement utime, utimes, utimensat

856c320

Signed-off-by: Erica Fu <[email protected]>

donporter requested changes May 20, 2025

View reviewed changes

fixup! [LibOS] Implement utime, utimes, utimensat

6a9fd83

Signed-off-by: Don Porter <[email protected]>

donporter force-pushed the erica/utime branch from d2818ee to 6a9fd83 Compare May 22, 2025 14:24

fixup! fixup! [LibOS] Implement utime, utimes, utimensat

1959848

Signed-off-by: Don Porter <[email protected]>

efu39 commented May 23, 2025

View reviewed changes

donporter reviewed May 23, 2025

View reviewed changes

efu39 commented May 25, 2025

View reviewed changes

efu39 and others added 2 commits June 16, 2025 22:53

Merge branch 'gramineproject:master' into erica/utime

e02d522

fixup! fixup! fixup! [LibOS] Implement utime, utimes, utimensat

db94331

Signed-off-by: Erica Fu <[email protected]>

efu39 commented Jun 17, 2025

View reviewed changes

efu39 requested a review from donporter June 17, 2025 11:27

donporter approved these changes Jul 1, 2025

View reviewed changes

donporter requested changes Jul 1, 2025

View reviewed changes

fixup! fixup! fixup! fixup! [LibOS] Implement utime, utimes, utimensat

8e0313a

Signed-off-by: Don Porter <[email protected]>

donporter force-pushed the erica/utime branch from 7fd2617 to 8e0313a Compare July 1, 2025 13:07

forkthus mentioned this pull request Jul 11, 2025

[LibOS] Child process fails during libos_init when all TCS are in use #2145

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[LibOS] Implement utime, utimes, utimensat #2131

[LibOS] Implement utime, utimes, utimensat #2131

Uh oh!

efu39 commented May 20, 2025 •

edited by mkow

Loading

Uh oh!

donporter commented May 20, 2025

Uh oh!

donporter left a comment

Uh oh!

efu39 left a comment

Uh oh!

donporter left a comment

Uh oh!

efu39 left a comment

Uh oh!

efu39 left a comment

Uh oh!

efu39 left a comment

Uh oh!

efu39 commented Jun 17, 2025

Uh oh!

donporter left a comment

Uh oh!

donporter left a comment

Uh oh!

donporter commented Jul 29, 2025

Uh oh!

donporter commented Jul 29, 2025

Uh oh!

Uh oh!

[LibOS] Implement utime, utimes, utimensat #2131

Are you sure you want to change the base?

[LibOS] Implement utime, utimes, utimensat #2131

Uh oh!

Conversation

efu39 commented May 20, 2025 • edited by mkow Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of the changes

How to test this PR?

Uh oh!

donporter commented May 20, 2025

Uh oh!

donporter left a comment

Choose a reason for hiding this comment

Uh oh!

efu39 left a comment

Choose a reason for hiding this comment

Uh oh!

donporter left a comment

Choose a reason for hiding this comment

Uh oh!

efu39 left a comment

Choose a reason for hiding this comment

Uh oh!

efu39 left a comment

Choose a reason for hiding this comment

Uh oh!

efu39 left a comment

Choose a reason for hiding this comment

Uh oh!

efu39 commented Jun 17, 2025

Uh oh!

donporter left a comment

Choose a reason for hiding this comment

Uh oh!

donporter left a comment

Choose a reason for hiding this comment

Uh oh!

donporter commented Jul 29, 2025

Uh oh!

donporter commented Jul 29, 2025

Uh oh!

Uh oh!

efu39 commented May 20, 2025 •

edited by mkow

Loading