On Wed, Jul 30, 2025 at 08:04:09AM -0700, Darrick J. Wong wrote: > On Wed, Jul 30, 2025 at 04:40:39PM +0200, Luca Di Maio wrote: > > Thanks Darrick for the review! > > Sorry again for this indentation mess, I'm going to basically align all > > function arguments and all the top-function declarations > > I was a bit confused because elsewhere in the code is not like that so > > it's a bit difficult to infere > > > > Offtopic: maybe we could also introduce an editorconfig setup? so that > > all various editors will be correctly set to see the tabs/spaces as > > needed (https://editorconfig.org/) > > Hrmm, that /would/ be useful. > Nice, will try to put something together for a future patch > > Back on topic: > > > > On Tue, Jul 29, 2025 at 02:43:22PM -0700, Darrick J. Wong wrote: > > <snipping> > > > > > + if (!S_ISSOCK(file_stat.st_mode) && > > > > + !S_ISLNK(file_stat.st_mode) && > > > > + !S_ISFIFO(file_stat.st_mode)) { > > > > + close(fd); > > > > + fd = openat(pathfd, entry->d_name, > > > > + O_NOFOLLOW | O_RDONLY | O_NOATIME); > > > > > > Just out of curiosity, does O_NOATIME not work in the previous openat? > > [narrator: it doesn't] > > > Actually on my test setup (mainly using docker/podman to test), opening > > with and without O_NOATIME when using O_PATH, does not change accesstime > > checking with `stat`, but also it works if I add it. > > As a precautionary measure (not sure if podman/docker is messing with > > noatime) I'll add it, as it seems to work correctly. > > On second thought I think you might leave the double opens because > O_NOATIME is only allowed if the current user owns source file, or has > CAP_FOWNER. If you're running mkfs as an unprivileged user trying to > capture a rootfs (with uid 0 files) then O_NOATIME won't be allowed. > > Maybe something along the lines of: > > /* > * Try to open the source file noatime to avoid a flood of > * writes to the source fs, but we can fall back to plain > * readonly mode if we don't have enough permission. > */ > fd = openat(pathfd, entry->d_name, O_NOFOLLOW | O_RDONLY | O_NOATIME); > if (fd < 0) > fd = openat(pathfd, entry->d_name, O_NOFOLLOW | O_RDONLY); > if (fd < 0) > /* whine and exit */ > > Just to see if you can open the source file without touching atime? > > --D Right, will do like this, didn't think about rootless/unprivileged mkfs L.
