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 >