Re: [PATCH RFC v2 2/4] ovl: specify layers via file descriptors

[Amir Goldstein <amir73il@gmail.com>]

On Sat, Oct 12, 2024 at 12:37 PM Christian Brauner <brauner@kernel.org> wrote:
>
> On Sat, Oct 12, 2024 at 10:25:38AM +0200, Amir Goldstein wrote:
> > On Fri, Oct 11, 2024 at 11:46 PM Christian Brauner <brauner@kernel.org> wrote:
> > >
> >
> > nit: if you can avoid using the exact same title for the cover letter and
> > a patch that would be nice (gmail client collapses them together).
>
> Fine, but fwiw, the solution to this problem is to use a proper email
> client. ;)
>

touché :)

> >
> > > Currently overlayfs only allows specifying layers through path names.
> > > This is inconvenient for users such as systemd that want to assemble an
> > > overlayfs mount purely based on file descriptors.
> > >
> > > This enables user to specify both:
> > >
> > >     fsconfig(fd_overlay, FSCONFIG_SET_FD, "upperdir+", NULL, fd_upper);
> > >     fsconfig(fd_overlay, FSCONFIG_SET_FD, "workdir+",  NULL, fd_work);
> > >     fsconfig(fd_overlay, FSCONFIG_SET_FD, "lowerdir+", NULL, fd_lower1);
> > >     fsconfig(fd_overlay, FSCONFIG_SET_FD, "lowerdir+", NULL, fd_lower2);
> > >
> > > in addition to:
> > >
> > >     fsconfig(fd_overlay, FSCONFIG_SET_STRING, "upperdir+", "/upper",  0);
> > >     fsconfig(fd_overlay, FSCONFIG_SET_STRING, "workdir+",  "/work",   0);
> > >     fsconfig(fd_overlay, FSCONFIG_SET_STRING, "lowerdir+", "/lower1", 0);
> > >     fsconfig(fd_overlay, FSCONFIG_SET_STRING, "lowerdir+", "/lower2", 0);
> > >
> >
> > Please add a minimal example with FSCONFIG_SET_FD to overlayfs.rst.
> > I am not looking for a user manual, just one example to complement the
> > FSCONFIG_SET_STRING examples.
> >
> > I don't mind adding config types on a per need basis, but out of curiosity
> > do you think the need will arise to support FSCONFIG_SET_PATH{,_EMPTY}
> > in the future? It is going to be any more challenging than just adding
> > support for
> > just FSCONFIG_SET_FD?
>
> This could also be made to work rather easily but I wouldn't know why we
> would want to add it. The current overlayfs FSCONFIG_SET_STRING variant
> is mostly equivalent. Imho, it's a lot saner to let userspace do the
> required open via regular openat{2}() and then use FSCONFIG_SET_FD, then
> force *at() based semantics down into the filesystem via fsconfig().

Fine be me. I am less familiar with the relevant use cases.

> U_PATH{_EMPTY} is unused and we could probably also get rid of it.
>

Oh. I didn't know that.

Thanks,
Amir.