On Wed, Jul 30, 2025 at 05:20:42PM +0200, Luca Di Maio wrote: > 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 Either that or a indent/clang-format wrapper? The kernel has scripts/Lindent, maybe someone should try to write one for xfs style if I don't get to it first. <sigh>rustfmt<duck> > > > 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. >
