On Thu, Jul 4, 2024 at 10:25 AM Zhihao Cheng <chengzhihao1@xxxxxxxxxx> wrote:
>
> 在 2024/7/3 23:18, Amir Goldstein 写道:
> > On Wed, Jul 3, 2024 at 4:48 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> >>
> [...]
> >>>
> >>> This fix looks correct, but it is not pretty IMO.
> >>> The cleanup on error is much cleaner in ovl_parse_layer() -> ovl_add_layer()
> >>> I wonder if we can reuse some of those helpers instead of the current code.
> >>>
> >>> Christian, what do you think?
> >>
> >> Yeah, sounds good. Something like the completely untested below.
> >> Feel free to reuse in whatever form.
>
> Thanks Christian, it's so nice of you.
> >
> > This looks much nicer!
> > I think you unintentionally dropped incrementing of ctx->nr_data
> > for the notorious case of ::<lowerdatadir>.
>
> Hi, Amir, thanks for reminding, I modify some points based on the
> patches from Christian:
> 1. Use ovl_mount_dir() to parse parameter string for Opt_lowerdir,
> because I found the character '\' can be filtered from parameter string
> for Opt_lowerdir, so I want to keep it(Not sure whether it's right).
Good catch - we definitely don't want to change escaping of legacy
mount options.
>
> mount("none", "/root/tmp/merge", "overlay", 0, "lowerdir=low\\er:lower2")
> 2. Keep 'ctx->nr_data' updating in ovl_parse_param_lowerdir(). I was
> going to move 'ctx->nr_data' updating into ovl_add_layer() by passing
> 'Opt_datadir_add' into ovl_parse_layer(), but I found
> ovl_mount_dir_check() use this opt to do some check, the opt should be
> Opt_lowerdir.
> 3. Remove 'out_put' error handling branch from
> ovl_parse_param_lowerdir(), because ovl_fs_context_free is invoked if
> something error happens.
> >
> > Zhihao,
> >
> > Please make sure to run the fstests overlay test for lowerdatadirs
> > overlay/079 overlay/085 overlay/086
>
> BTW, I get overlay/061 failed even before applying my patches. Maybe I
> will take some time to analyze it.
It always failed.
that's why it is not in the "auto" group, but in the "posix" group
to signify that this is a non compliant behavior of overlayfs.
Thanks,
Amir.
>
> overlay/061 - output mismatch (see
> /root/git/xfstests-dev/results//overlay/061.out.bad)
> --- tests/overlay/061.out 2023-04-01 14:14:58.354052795 +0800
> +++ /root/git/xfstests-dev/results//overlay/061.out.bad 2024-07-04
> 14:52:44.993000000 +0800
> @@ -1,4 +1,4 @@
> QA output created by 061
> -00000000: 61 61 61 61 61 61 61 61 61 61 61 61 61 61 61 61
> aaaaaaaaaaaaaaaa
> +00000000: 54 68 69 73 20 69 73 20 6f 6c 64 20 6e 65 77 73
> This.is.old.news
> After mount cycle:
> 00000000: 61 61 61 61 61 61 61 61 61 61 61 61 61 61 61 61
> aaaaaaaaaaaaaaaa
> ...
> (Run 'diff -u /root/git/xfstests-dev/tests/overlay/061.out
> /root/git/xfstests-dev/results//overlay/061.out.bad' to see the entire
> diff)
>
> v2 pacthes:
> https://lore.kernel.org/linux-unionfs/20240704070323.3365042-1-chengzhihao1@xxxxxxxxxx/T/#mdaedba39d7f261cd2555ea6773ed3611c02e7a4e
>
