On Mon, 16 Oct 2023 at 13:56, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> On Mon, Oct 16, 2023 at 12:27 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> >
> > On Sun, 15 Oct 2023 at 08:58, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> >
> > > + for (nr = 0; nr < nr_added_lower; nr++, lowerdirs++) {
> > > + if (nr < nr_merged_lower)
> > > + seq_show_option(m, "lowerdir+", *lowerdirs);
> > > + else
> > > + seq_show_option(m, "datadir+", *lowerdirs);
> >
> > Good.
> >
> > I did some testing and it turns out libmount still regresses on
> > 6.6-rc6 for the escaped comma case. The reason is that libmount
> > doesn't understand escaping of commas, hence the '-oupper=upper\,1'
> > will result in two fsconfig() calls: 'upper=upper\' and '1'. Prior
> > to 6.5 these were nicely reconstructed into the original
> > 'upper=upper\,1' by legacy_parse_param().
> >
>
> Technically, I think this is a libmount regression, not a kernel regression.
> Since libmount 2.39, libmount will split the commas differently than
> overlayfs always did.
Ah, but it's not a regression after all, since the kernel un-split the
same commas until 6.5, so there was no way the libmount devs would
have observed any regression in overlayfs mount. But arguing about
which component is the cause of the regression is not very productive.
Indeed libmount can be fixed parse overlayfs options the same way as
the kernel parsed them before 6.5, which is probably a much better
fix, than a kernel one.
Karel, is doing such filesystem specific option handling feasible?
If so, then for overlayfs please please pass an un-escaped (\char ->
char) string to fsconfig for "upperdir=" and "workdir=" options.
> I am ok with doing this experiment in 6.7.
>
> I saw that your patch converted upperdir/workdir to path type
> without unescaping. Don't we need to keep the support for
> string type params for ->parse_monolothic()?
Yes.
> Or is this the reason for the change in fs_lookup_param()?
That just a bug. It already supported the string type, and in that
case lookup must be done against AT_FDCWD and not against zero fd
(which is what it previously did).
Thanks,
Miklos
