Re: [PATCH v11 1/1] proto: add ability to populate a filesystem from a directory

["Darrick J. Wong" <djwong@kernel.org>]

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.
>