Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs: Add utimens and lutimens #4300

Merged
merged 2 commits into from
Aug 9, 2021
Merged

Conversation

xiaoxiang781216
Copy link
Contributor

@gustavonihei
Copy link
Contributor

I am a bit confused here. I am not familiar with these APIs.
According to the latest release of the OpenGroup standard, neither utimens nor lutimens are standardized functions.
I saw that utimes has been moved the XSI and been somewhat deprecated.
But, the standard defines utimensat as an equivalent function for utimes.
Isn't utimensat the implementation you intended to add instead of utimens?

@xiaoxiang781216
Copy link
Contributor Author

I am a bit confused here. I am not familiar with these APIs.
According to the latest release of the OpenGroup standard, neither utimens nor lutimens are standardized functions.
I saw that utimes has been moved the XSI and been somewhat deprecated.
But, the standard defines utimensat as an equivalent function for utimes.
Isn't utimensat the implementation you intended to add instead of utimens?

Yes, utimensat is the final goal: @Donny9 is preparing a patch series to support utimensat(and other similar function e.g. openat, unlinkat...). This PR could be used before that patch finish. Once utimensat is ready, all these time related functions will simply forward to utimensat.

@xiaoxiang781216
Copy link
Contributor Author

xiaoxiang781216 commented Aug 9, 2021

@gustavonihei I append a new patch to this PR, you can see from it that:

  1. The sim implementation of hostfs call utimensat:
    d7631be#diff-4f28509098f7178398166fad417074658b353f7e72d7a4fddb93e917cb91e055R646
  2. The rpmsg implmentation of hostfs call utimens:
    d7631be#diff-9cefb67caeab4d76ef979687e08c0faed73fc127bf2c5ac128fe63ee2164dbd3R779

Without utimens, it look ulgy to convert timespec to timeval and invoke utimes instead in the second implementation. On the other hand, when utimensat is ready, rpmsg's version will change to call utimensat too.

@gustavonihei
Copy link
Contributor

@gustavonihei I append a new patch to this PR, you can see from it that:

  1. The sim implementation of hostfs call utimensat:
    d7631be#diff-4f28509098f7178398166fad417074658b353f7e72d7a4fddb93e917cb91e055R646
  2. The rpmsg implmentation of hostfs call utimens:
    d7631be#diff-9cefb67caeab4d76ef979687e08c0faed73fc127bf2c5ac128fe63ee2164dbd3R779

Without utimens, it look ulgy to convert timespec to timeval and invoke utimes instead in the second implementation. On the other hand, when utimensat is ready, rpmsg's version will change to call utimensat too.

Ok, I got your point, I think it makes sense.
So utimens and lutimens usage will later be restricted as just internal helper functions.
LGTM, but the CI is still failing on some compiler errors.

@xiaoxiang781216
Copy link
Contributor Author

@gustavonihei I append a new patch to this PR, you can see from it that:

  1. The sim implementation of hostfs call utimensat:
    d7631be#diff-4f28509098f7178398166fad417074658b353f7e72d7a4fddb93e917cb91e055R646
  2. The rpmsg implmentation of hostfs call utimens:
    d7631be#diff-9cefb67caeab4d76ef979687e08c0faed73fc127bf2c5ac128fe63ee2164dbd3R779

Without utimens, it look ulgy to convert timespec to timeval and invoke utimes instead in the second implementation. On the other hand, when utimensat is ready, rpmsg's version will change to call utimensat too.

Ok, I got your point, I think it makes sense.
So utimens and lutimens usage will later be restricted as just internal helper functions.

Yes, it will be replace with utimensat.

LGTM, but the CI is still failing on some compiler errors.

It should OK, now.

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
@gustavonihei gustavonihei merged commit 7764581 into apache:master Aug 9, 2021
@xiaoxiang781216 xiaoxiang781216 deleted the timens branch August 10, 2021 02:50
@yamt
Copy link
Contributor

yamt commented Apr 11, 2022

I am a bit confused here. I am not familiar with these APIs.
According to the latest release of the OpenGroup standard, neither utimens nor lutimens are standardized functions.
I saw that utimes has been moved the XSI and been somewhat deprecated.
But, the standard defines utimensat as an equivalent function for utimes.
Isn't utimensat the implementation you intended to add instead of utimens?

Yes, utimensat is the final goal: @Donny9 is preparing a patch series to support utimensat(and other similar function e.g. openat, unlinkat...). This PR could be used before that patch finish. Once utimensat is ready, all these time related functions will simply forward to utimensat.

@Donny9
are you (or someone) still working on openat and friends?

@xiaoxiang781216
Copy link
Contributor Author

yes, we have a draft huge patchset.

@xiaoxiang781216
Copy link
Contributor Author

yes, we have a draft huge patchset.

@yamt
Copy link
Contributor

yamt commented Apr 12, 2022

yes, we have a draft huge patchset.

do you have a plan to submit it anytime soon?

@Donny9
Copy link
Contributor

Donny9 commented Apr 12, 2022

I'm free at the end of this month to organize this patch, there are other missions recently.

@yamt
Copy link
Contributor

yamt commented Apr 17, 2022

ok. thank you for an update.

@yamt
Copy link
Contributor

yamt commented Apr 25, 2022

btw, my interest in the openat stuff is for wasm-micro-runtime libc-wasi.

@xiaoxiang781216
Copy link
Contributor Author

Yes, we use this runtime too.

@yamt
Copy link
Contributor

yamt commented Apr 26, 2022

Yes, we use this runtime too.

good to hear.
do you have patches for nuttx apps and product-mini/platforms/nuttx/wamr.mk etc as well?
(just asking because my colleagues seem plan to write them by themselves)

@xiaoxiang781216
Copy link
Contributor Author

@no1wudi do you know?

@no1wudi
Copy link
Contributor

no1wudi commented Apr 26, 2022

We plan to support libc-wasi in future, but we don't have such patches now.

@yamt
Copy link
Contributor

yamt commented Apr 26, 2022

ok. thank you.

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.

MISSING FILE SYSTEM FEATURES
5 participants