diff mbox series

overlay: add test for lowerdir mount option parsing

Message ID 20231017101145.2348571-1-amir73il@gmail.com (mailing list archive)
State New, archived
Headers show
Series overlay: add test for lowerdir mount option parsing | expand

Commit Message

Amir Goldstein Oct. 17, 2023, 10:11 a.m. UTC
Check parsing and display of spaces and escaped colons and commans in
lowerdir mount option.

This is a regression test for two bugs introduced in v6.5 with the
conversion to new mount api.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Zorro,

This is a test for two regressions in kernel v6.5.
The two fixes were merged in 6.6-rc6 and have been picked for
the upcoming LTS 6.5.y release.

Thanks,
Amir.

 tests/overlay/083     | 54 +++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/083.out |  2 ++
 2 files changed, 56 insertions(+)
 create mode 100755 tests/overlay/083
 create mode 100644 tests/overlay/083.out

Comments

Zorro Lang Oct. 19, 2023, 5:50 p.m. UTC | #1
On Tue, Oct 17, 2023 at 01:11:45PM +0300, Amir Goldstein wrote:
> Check parsing and display of spaces and escaped colons and commans in
> lowerdir mount option.
> 
> This is a regression test for two bugs introduced in v6.5 with the
> conversion to new mount api.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
> 
> Zorro,
> 
> This is a test for two regressions in kernel v6.5.
> The two fixes were merged in 6.6-rc6 and have been picked for
> the upcoming LTS 6.5.y release.


> 
> Thanks,
> Amir.
> 
>  tests/overlay/083     | 54 +++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/083.out |  2 ++
>  2 files changed, 56 insertions(+)
>  create mode 100755 tests/overlay/083
>  create mode 100644 tests/overlay/083.out
> 
> diff --git a/tests/overlay/083 b/tests/overlay/083
> new file mode 100755
> index 00000000..071b4b84
> --- /dev/null
> +++ b/tests/overlay/083
> @@ -0,0 +1,54 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2023 CTERA Networks. All Rights Reserved.
> +#
> +# FS QA Test 083
> +#
> +# Test regressions in parsing and display of special chars in mount options.
> +#
> +# The following kernel commits from v6.5 introduced regressions:
> +#  b36a5780cb44 ("ovl: modify layer parameter parsing")
> +#  1784fbc2ed9c ("ovl: port to new mount api")
> +#
> +
> +. ./common/preamble
> +_begin_fstest auto quick mount
> +
> +# Import common functions.
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs overlay
> +_fixed_by_kernel_commit 32db51070850 \
> +	"ovl: fix regression in showing lowerdir mount option"
> +_fixed_by_kernel_commit c34706acf40b \
> +	"ovl: fix regression in parsing of mount options with escaped comma"

Hi Amir,

I tried this case on the latest linux kernel which contains the
two commits, but still hit below failure:

FSTYP         -- overlay
PLATFORM      -- Linux/x86_64 hp-dl380pg8-01 6.6.0-rc6-mainline+ #7 SMP PREEMPT_DYNAMIC Thu Oct 19 22:34:28 CST 2023
MKFS_OPTIONS  -- /mnt/scratch
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /mnt/scratch /mnt/scratch/ovl-mnt

overlay/083       - output mismatch (see /root/git/xfstests/results//overlay/083.out.bad)
    --- tests/overlay/083.out   2023-10-19 14:07:18.099496414 +0800
    +++ /root/git/xfstests/results//overlay/083.out.bad 2023-10-20 00:25:47.682874383 +0800
    @@ -1,2 +1,4 @@
     QA output created by 083
    +mount: /mnt/scratch/ovl-mnt: special device ovl_esc_test does not exist.
    +       dmesg(1) may have more information after failed mount system call.
     Silence is golden

Thanks,
Zorro

> +
> +# _overlay_check_* helpers do not handle special chars well
> +_require_scratch_nocheck
> +
> +# Remove all files from previous tests
> +_scratch_mkfs
> +
> +# Create lowerdirs with special characters
> +lowerdir1="$OVL_BASE_SCRATCH_MNT/lower1 with  spaces"
> +lowerdir2="$OVL_BASE_SCRATCH_MNT/lower2:with::colons"
> +lowerdir3="$OVL_BASE_SCRATCH_MNT/lower3,with,,commas"
> +lowerdir2_esc="$OVL_BASE_SCRATCH_MNT/lower2\:with\:\:colons"
> +lowerdir3_esc="$OVL_BASE_SCRATCH_MNT/lower3\,with\,\,commas"
> +upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
> +workdir=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
> +mkdir -p "$lowerdir1" "$lowerdir2" "$lowerdir3"
> +
> +# _overlay_mount_* helpers do not handle special chars well, so execute mount directly.
> +# if escaped colons and commas are not parsed correctly, mount will fail.
> +$MOUNT_PROG -t overlay ovl_esc_test $SCRATCH_MNT \
> +	-o"upperdir=$upperdir,workdir=$workdir,lowerdir=$lowerdir3_esc:$lowerdir2_esc:$lowerdir1"
> +
> +# if spaces are not escaped when showing mount options,
> +# mount command will not show the word 'spaces' after the spaces
> +$MOUNT_PROG -t overlay | grep ovl_esc_test  | tee -a $seqres.full | grep -v spaces
> +
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/overlay/083.out b/tests/overlay/083.out
> new file mode 100644
> index 00000000..0beba309
> --- /dev/null
> +++ b/tests/overlay/083.out
> @@ -0,0 +1,2 @@
> +QA output created by 083
> +Silence is golden
> -- 
> 2.34.1
>
Amir Goldstein Oct. 20, 2023, 4:18 p.m. UTC | #2
On Thu, Oct 19, 2023 at 8:50 PM Zorro Lang <zlang@redhat.com> wrote:
>
> On Tue, Oct 17, 2023 at 01:11:45PM +0300, Amir Goldstein wrote:
> > Check parsing and display of spaces and escaped colons and commans in
> > lowerdir mount option.
> >
> > This is a regression test for two bugs introduced in v6.5 with the
> > conversion to new mount api.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >
> > Zorro,
> >
> > This is a test for two regressions in kernel v6.5.
> > The two fixes were merged in 6.6-rc6 and have been picked for
> > the upcoming LTS 6.5.y release.
>
>
> >
> > Thanks,
> > Amir.
> >
> >  tests/overlay/083     | 54 +++++++++++++++++++++++++++++++++++++++++++
> >  tests/overlay/083.out |  2 ++
> >  2 files changed, 56 insertions(+)
> >  create mode 100755 tests/overlay/083
> >  create mode 100644 tests/overlay/083.out
> >
> > diff --git a/tests/overlay/083 b/tests/overlay/083
> > new file mode 100755
> > index 00000000..071b4b84
> > --- /dev/null
> > +++ b/tests/overlay/083
> > @@ -0,0 +1,54 @@
> > +#! /bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (C) 2023 CTERA Networks. All Rights Reserved.
> > +#
> > +# FS QA Test 083
> > +#
> > +# Test regressions in parsing and display of special chars in mount options.
> > +#
> > +# The following kernel commits from v6.5 introduced regressions:
> > +#  b36a5780cb44 ("ovl: modify layer parameter parsing")
> > +#  1784fbc2ed9c ("ovl: port to new mount api")
> > +#
> > +
> > +. ./common/preamble
> > +_begin_fstest auto quick mount
> > +
> > +# Import common functions.
> > +. ./common/filter
> > +
> > +# real QA test starts here
> > +_supported_fs overlay
> > +_fixed_by_kernel_commit 32db51070850 \
> > +     "ovl: fix regression in showing lowerdir mount option"
> > +_fixed_by_kernel_commit c34706acf40b \
> > +     "ovl: fix regression in parsing of mount options with escaped comma"
>
> Hi Amir,
>
> I tried this case on the latest linux kernel which contains the
> two commits, but still hit below failure:
>
> FSTYP         -- overlay
> PLATFORM      -- Linux/x86_64 hp-dl380pg8-01 6.6.0-rc6-mainline+ #7 SMP PREEMPT_DYNAMIC Thu Oct 19 22:34:28 CST 2023
> MKFS_OPTIONS  -- /mnt/scratch
> MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /mnt/scratch /mnt/scratch/ovl-mnt
>
> overlay/083       - output mismatch (see /root/git/xfstests/results//overlay/083.out.bad)
>     --- tests/overlay/083.out   2023-10-19 14:07:18.099496414 +0800
>     +++ /root/git/xfstests/results//overlay/083.out.bad 2023-10-20 00:25:47.682874383 +0800
>     @@ -1,2 +1,4 @@
>      QA output created by 083
>     +mount: /mnt/scratch/ovl-mnt: special device ovl_esc_test does not exist.
>     +       dmesg(1) may have more information after failed mount system call.
>      Silence is golden
>

Strange.
I was under the impression that the 'dev' argument to mount command
of overlayfs is a completely opaque string.

Maybe you are using a different libmount version that I do.
I have libmount 2.36.1.

Anyway, can you please try if this variation works for you:

--- a/tests/overlay/083
+++ b/tests/overlay/083
@@ -42,12 +42,12 @@ mkdir -p "$lowerdir1" "$lowerdir2" "$lowerdir3"

 # _overlay_mount_* helpers do not handle special chars well, so
execute mount directly.
 # if escaped colons and commas are not parsed correctly, mount will fail.
-$MOUNT_PROG -t overlay ovl_esc_test $SCRATCH_MNT \
+$MOUNT_PROG -t overlay $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT \
        -o"upperdir=$upperdir,workdir=$workdir,lowerdir=$lowerdir3_esc:$lowerdir2_esc:$lowerdir1"

 # if spaces are not escaped when showing mount options,
 # mount command will not show the word 'spaces' after the spaces
-$MOUNT_PROG -t overlay | grep ovl_esc_test  | tee -a $seqres.full |
grep -v spaces
+$MOUNT_PROG -t overlay | grep lower3 | tee -a $seqres.full | grep -v spaces

Thanks,
Amir.

>
> > +
> > +# _overlay_check_* helpers do not handle special chars well
> > +_require_scratch_nocheck
> > +
> > +# Remove all files from previous tests
> > +_scratch_mkfs
> > +
> > +# Create lowerdirs with special characters
> > +lowerdir1="$OVL_BASE_SCRATCH_MNT/lower1 with  spaces"
> > +lowerdir2="$OVL_BASE_SCRATCH_MNT/lower2:with::colons"
> > +lowerdir3="$OVL_BASE_SCRATCH_MNT/lower3,with,,commas"
> > +lowerdir2_esc="$OVL_BASE_SCRATCH_MNT/lower2\:with\:\:colons"
> > +lowerdir3_esc="$OVL_BASE_SCRATCH_MNT/lower3\,with\,\,commas"
> > +upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
> > +workdir=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
> > +mkdir -p "$lowerdir1" "$lowerdir2" "$lowerdir3"
> > +
> > +# _overlay_mount_* helpers do not handle special chars well, so execute mount directly.
> > +# if escaped colons and commas are not parsed correctly, mount will fail.
> > +$MOUNT_PROG -t overlay ovl_esc_test $SCRATCH_MNT \
> > +     -o"upperdir=$upperdir,workdir=$workdir,lowerdir=$lowerdir3_esc:$lowerdir2_esc:$lowerdir1"
> > +
> > +# if spaces are not escaped when showing mount options,
> > +# mount command will not show the word 'spaces' after the spaces
> > +$MOUNT_PROG -t overlay | grep ovl_esc_test  | tee -a $seqres.full | grep -v spaces
> > +
> > +echo "Silence is golden"
> > +status=0
> > +exit
> > diff --git a/tests/overlay/083.out b/tests/overlay/083.out
> > new file mode 100644
> > index 00000000..0beba309
> > --- /dev/null
> > +++ b/tests/overlay/083.out
> > @@ -0,0 +1,2 @@
> > +QA output created by 083
> > +Silence is golden
> > --
> > 2.34.1
> >
>
Zorro Lang Oct. 21, 2023, 12:30 p.m. UTC | #3
On Fri, Oct 20, 2023 at 07:18:55PM +0300, Amir Goldstein wrote:
> On Thu, Oct 19, 2023 at 8:50 PM Zorro Lang <zlang@redhat.com> wrote:
> >
> > On Tue, Oct 17, 2023 at 01:11:45PM +0300, Amir Goldstein wrote:
> > > Check parsing and display of spaces and escaped colons and commans in
> > > lowerdir mount option.
> > >
> > > This is a regression test for two bugs introduced in v6.5 with the
> > > conversion to new mount api.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> > >
> > > Zorro,
> > >
> > > This is a test for two regressions in kernel v6.5.
> > > The two fixes were merged in 6.6-rc6 and have been picked for
> > > the upcoming LTS 6.5.y release.
> >
> >
> > >
> > > Thanks,
> > > Amir.
> > >
> > >  tests/overlay/083     | 54 +++++++++++++++++++++++++++++++++++++++++++
> > >  tests/overlay/083.out |  2 ++
> > >  2 files changed, 56 insertions(+)
> > >  create mode 100755 tests/overlay/083
> > >  create mode 100644 tests/overlay/083.out
> > >
> > > diff --git a/tests/overlay/083 b/tests/overlay/083
> > > new file mode 100755
> > > index 00000000..071b4b84
> > > --- /dev/null
> > > +++ b/tests/overlay/083
> > > @@ -0,0 +1,54 @@
> > > +#! /bin/bash
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +# Copyright (C) 2023 CTERA Networks. All Rights Reserved.
> > > +#
> > > +# FS QA Test 083
> > > +#
> > > +# Test regressions in parsing and display of special chars in mount options.
> > > +#
> > > +# The following kernel commits from v6.5 introduced regressions:
> > > +#  b36a5780cb44 ("ovl: modify layer parameter parsing")
> > > +#  1784fbc2ed9c ("ovl: port to new mount api")
> > > +#
> > > +
> > > +. ./common/preamble
> > > +_begin_fstest auto quick mount
> > > +
> > > +# Import common functions.
> > > +. ./common/filter
> > > +
> > > +# real QA test starts here
> > > +_supported_fs overlay
> > > +_fixed_by_kernel_commit 32db51070850 \
> > > +     "ovl: fix regression in showing lowerdir mount option"
> > > +_fixed_by_kernel_commit c34706acf40b \
> > > +     "ovl: fix regression in parsing of mount options with escaped comma"
> >
> > Hi Amir,
> >
> > I tried this case on the latest linux kernel which contains the
> > two commits, but still hit below failure:
> >
> > FSTYP         -- overlay
> > PLATFORM      -- Linux/x86_64 hp-dl380pg8-01 6.6.0-rc6-mainline+ #7 SMP PREEMPT_DYNAMIC Thu Oct 19 22:34:28 CST 2023
> > MKFS_OPTIONS  -- /mnt/scratch
> > MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /mnt/scratch /mnt/scratch/ovl-mnt
> >
> > overlay/083       - output mismatch (see /root/git/xfstests/results//overlay/083.out.bad)
> >     --- tests/overlay/083.out   2023-10-19 14:07:18.099496414 +0800
> >     +++ /root/git/xfstests/results//overlay/083.out.bad 2023-10-20 00:25:47.682874383 +0800
> >     @@ -1,2 +1,4 @@
> >      QA output created by 083
> >     +mount: /mnt/scratch/ovl-mnt: special device ovl_esc_test does not exist.
> >     +       dmesg(1) may have more information after failed mount system call.
> >      Silence is golden
> >
> 
> Strange.
> I was under the impression that the 'dev' argument to mount command
> of overlayfs is a completely opaque string.
> 
> Maybe you are using a different libmount version that I do.
> I have libmount 2.36.1.

I'm using Fedora rawhide, the libmount version is libmount-2.39.2-1.fc40.

> 
> Anyway, can you please try if this variation works for you:
> 
> --- a/tests/overlay/083
> +++ b/tests/overlay/083
> @@ -42,12 +42,12 @@ mkdir -p "$lowerdir1" "$lowerdir2" "$lowerdir3"
> 
>  # _overlay_mount_* helpers do not handle special chars well, so
> execute mount directly.
>  # if escaped colons and commas are not parsed correctly, mount will fail.
> -$MOUNT_PROG -t overlay ovl_esc_test $SCRATCH_MNT \
> +$MOUNT_PROG -t overlay $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT \

I doubt it works. This looks like to try to mount /mnt/scratch on
/mnt/scratch/ovl-mnt.

Thanks,
Zorro

>         -o"upperdir=$upperdir,workdir=$workdir,lowerdir=$lowerdir3_esc:$lowerdir2_esc:$lowerdir1"
> 
>  # if spaces are not escaped when showing mount options,
>  # mount command will not show the word 'spaces' after the spaces
> -$MOUNT_PROG -t overlay | grep ovl_esc_test  | tee -a $seqres.full |
> grep -v spaces
> +$MOUNT_PROG -t overlay | grep lower3 | tee -a $seqres.full | grep -v spaces
> 
> Thanks,
> Amir.
> 
> >
> > > +
> > > +# _overlay_check_* helpers do not handle special chars well
> > > +_require_scratch_nocheck
> > > +
> > > +# Remove all files from previous tests
> > > +_scratch_mkfs
> > > +
> > > +# Create lowerdirs with special characters
> > > +lowerdir1="$OVL_BASE_SCRATCH_MNT/lower1 with  spaces"
> > > +lowerdir2="$OVL_BASE_SCRATCH_MNT/lower2:with::colons"
> > > +lowerdir3="$OVL_BASE_SCRATCH_MNT/lower3,with,,commas"
> > > +lowerdir2_esc="$OVL_BASE_SCRATCH_MNT/lower2\:with\:\:colons"
> > > +lowerdir3_esc="$OVL_BASE_SCRATCH_MNT/lower3\,with\,\,commas"
> > > +upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
> > > +workdir=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
> > > +mkdir -p "$lowerdir1" "$lowerdir2" "$lowerdir3"
> > > +
> > > +# _overlay_mount_* helpers do not handle special chars well, so execute mount directly.
> > > +# if escaped colons and commas are not parsed correctly, mount will fail.
> > > +$MOUNT_PROG -t overlay ovl_esc_test $SCRATCH_MNT \
> > > +     -o"upperdir=$upperdir,workdir=$workdir,lowerdir=$lowerdir3_esc:$lowerdir2_esc:$lowerdir1"
> > > +
> > > +# if spaces are not escaped when showing mount options,
> > > +# mount command will not show the word 'spaces' after the spaces
> > > +$MOUNT_PROG -t overlay | grep ovl_esc_test  | tee -a $seqres.full | grep -v spaces
> > > +
> > > +echo "Silence is golden"
> > > +status=0
> > > +exit
> > > diff --git a/tests/overlay/083.out b/tests/overlay/083.out
> > > new file mode 100644
> > > index 00000000..0beba309
> > > --- /dev/null
> > > +++ b/tests/overlay/083.out
> > > @@ -0,0 +1,2 @@
> > > +QA output created by 083
> > > +Silence is golden
> > > --
> > > 2.34.1
> > >
> >
>
Amir Goldstein Oct. 22, 2023, 7:09 a.m. UTC | #4
On Sun, Oct 22, 2023 at 9:26 AM Zorro Lang <zlang@redhat.com> wrote:
>
> On Sat, Oct 21, 2023 at 03:48:09PM +0300, Amir Goldstein wrote:
> >    On Sat, Oct 21, 2023, 3:30 PM Zorro Lang <[1]zlang@redhat.com> wrote:
> >
> >      On Fri, Oct 20, 2023 at 07:18:55PM +0300, Amir Goldstein wrote:
> >      > On Thu, Oct 19, 2023 at 8:50 PM Zorro Lang <[2]zlang@redhat.com>
> >      wrote:
> >      > >
> >      > > On Tue, Oct 17, 2023 at 01:11:45PM +0300, Amir Goldstein wrote:
> >      > > > Check parsing and display of spaces and escaped colons and
> >      commans in
> >      > > > lowerdir mount option.
> >      > > >
> >      > > > This is a regression test for two bugs introduced in v6.5 with
> >      the
> >      > > > conversion to new mount api.
> >      > > >
> >      > > > Signed-off-by: Amir Goldstein <[3]amir73il@gmail.com>
> >      > > > ---
> >      > > >
> >      > > > Zorro,
> >      > > >
> >      > > > This is a test for two regressions in kernel v6.5.
> >      > > > The two fixes were merged in 6.6-rc6 and have been picked for
> >      > > > the upcoming LTS 6.5.y release.
> >      > >
> >      > >
> >      > > >
> >      > > > Thanks,
> >      > > > Amir.
> >      > > >
> >      > > >  tests/overlay/083     | 54
> >      +++++++++++++++++++++++++++++++++++++++++++
> >      > > >  tests/overlay/083.out |  2 ++
> >      > > >  2 files changed, 56 insertions(+)
> >      > > >  create mode 100755 tests/overlay/083
> >      > > >  create mode 100644 tests/overlay/083.out
> >      > > >
> >      > > > diff --git a/tests/overlay/083 b/tests/overlay/083
> >      > > > new file mode 100755
> >      > > > index 00000000..071b4b84
> >      > > > --- /dev/null
> >      > > > +++ b/tests/overlay/083
> >      > > > @@ -0,0 +1,54 @@
> >      > > > +#! /bin/bash
> >      > > > +# SPDX-License-Identifier: GPL-2.0
> >      > > > +# Copyright (C) 2023 CTERA Networks. All Rights Reserved.
> >      > > > +#
> >      > > > +# FS QA Test 083
> >      > > > +#
> >      > > > +# Test regressions in parsing and display of special chars in
> >      mount options.
> >      > > > +#
> >      > > > +# The following kernel commits from v6.5 introduced
> >      regressions:
> >      > > > +#  b36a5780cb44 ("ovl: modify layer parameter parsing")
> >      > > > +#  1784fbc2ed9c ("ovl: port to new mount api")
> >      > > > +#
> >      > > > +
> >      > > > +. ./common/preamble
> >      > > > +_begin_fstest auto quick mount
> >      > > > +
> >      > > > +# Import common functions.
> >      > > > +. ./common/filter
> >      > > > +
> >      > > > +# real QA test starts here
> >      > > > +_supported_fs overlay
> >      > > > +_fixed_by_kernel_commit 32db51070850 \
> >      > > > +     "ovl: fix regression in showing lowerdir mount option"
> >      > > > +_fixed_by_kernel_commit c34706acf40b \
> >      > > > +     "ovl: fix regression in parsing of mount options with
> >      escaped comma"
> >      > >
> >      > > Hi Amir,
> >      > >
> >      > > I tried this case on the latest linux kernel which contains the
> >      > > two commits, but still hit below failure:
> >      > >
> >      > > FSTYP         -- overlay
> >      > > PLATFORM      -- Linux/x86_64 hp-dl380pg8-01 6.6.0-rc6-mainline+
> >      #7 SMP PREEMPT_DYNAMIC Thu Oct 19 22:34:28 CST 2023
> >      > > MKFS_OPTIONS  -- /mnt/scratch
> >      > > MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0
> >      /mnt/scratch /mnt/scratch/ovl-mnt
> >      > >
> >      > > overlay/083       - output mismatch (see
> >      /root/git/xfstests/results//overlay/083.out.bad)
> >      > >     --- tests/overlay/083.out   2023-10-19 14:07:18.099496414
> >      +0800
> >      > >     +++ /root/git/xfstests/results//overlay/083.out.bad
> >      2023-10-20 00:25:47.682874383 +0800
> >      > >     @@ -1,2 +1,4 @@
> >      > >      QA output created by 083
> >      > >     +mount: /mnt/scratch/ovl-mnt: special device ovl_esc_test
> >      does not exist.
> >      > >     +       dmesg(1) may have more information after failed
> >      mount system call.
> >      > >      Silence is golden
> >      > >
> >      >
> >      > Strange.
> >      > I was under the impression that the 'dev' argument to mount
> >      command
> >      > of overlayfs is a completely opaque string.
> >      >
> >      > Maybe you are using a different libmount version that I do.
> >      > I have libmount 2.36.1.
> >      I'm using Fedora rawhide, the libmount version is
> >      libmount-2.39.2-1.fc40.
> >      >
> >      > Anyway, can you please try if this variation works for you:
> >      >
> >      > --- a/tests/overlay/083
> >      > +++ b/tests/overlay/083
> >      > @@ -42,12 +42,12 @@ mkdir -p "$lowerdir1" "$lowerdir2"
> >      "$lowerdir3"
> >      >
> >      >  # _overlay_mount_* helpers do not handle special chars well, so
> >      > execute mount directly.
> >      >  # if escaped colons and commas are not parsed correctly, mount
> >      will fail.
> >      > -$MOUNT_PROG -t overlay ovl_esc_test $SCRATCH_MNT \
> >      > +$MOUNT_PROG -t overlay $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT \
> >      I doubt it works. This looks like to try to mount /mnt/scratch on
> >      /mnt/scratch/ovl-mnt.
> >
> >    Please try.
> >    You are making assumption that the dev argument is meaningful to
> >    overlayfs - it is not.
> >    Every one of the overlayfs tests in fstests uses the exact same dev
> >    argument as above inside the ovl mount helpers.
>
> Sure Amir, but it's still failed as below [1]. I think I've changed it as
> you wish [2]. The dmesg shows [3].
>
> Thanks,
> Zorro
>
> [1]
> # ./check -overlay overlay/083
> FSTYP         -- overlay
> PLATFORM      -- Linux/x86_64 hp-dl380pg8-01 6.6.0-rc6-mainline+ #7 SMP PREEMPT_DYNAMIC Thu Oct 19 22:34:28 CST 2023
> MKFS_OPTIONS  -- /mnt/scratch
> MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /mnt/scratch /mnt/scratch/ovl-mnt
>
> overlay/083       - output mismatch (see /root/git/xfstests/results//overlay/083.out.bad)
>     --- tests/overlay/083.out   2023-10-22 14:21:11.446520444 +0800
>     +++ /root/git/xfstests/results//overlay/083.out.bad 2023-10-22 14:23:06.642681119 +0800
>     @@ -1,2 +1,4 @@
>      QA output created by 083
>     +mount: /mnt/scratch/ovl-mnt: mount(2) system call failed: No such file or directory.
>     +       dmesg(1) may have more information after failed mount system call.
>      Silence is golden
>     ...
>     (Run 'diff -u /root/git/xfstests/tests/overlay/083.out /root/git/xfstests/results//overlay/083.out.bad'  to see the entire diff)
>
> HINT: You _MAY_ be missing kernel fix:
>       32db51070850 ovl: fix regression in showing lowerdir mount option
>
> HINT: You _MAY_ be missing kernel fix:
>       c34706acf40b ovl: fix regression in parsing of mount options with escaped comma
>
> Ran: overlay/083
> Failures: overlay/083
> Failed 1 of 1 tests
>
> [2]
> # git diff
> diff --git a/tests/overlay/083 b/tests/overlay/083
> index 071b4b84..3a6d6809 100755
> --- a/tests/overlay/083
> +++ b/tests/overlay/083
> @@ -42,12 +42,12 @@ mkdir -p "$lowerdir1" "$lowerdir2" "$lowerdir3"
>
>  # _overlay_mount_* helpers do not handle special chars well, so execute mount directly.
>  # if escaped colons and commas are not parsed correctly, mount will fail.
> -$MOUNT_PROG -t overlay ovl_esc_test $SCRATCH_MNT \
> +$MOUNT_PROG -t overlay $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT \
>         -o"upperdir=$upperdir,workdir=$workdir,lowerdir=$lowerdir3_esc:$lowerdir2_esc:$lowerdir1"
>
>  # if spaces are not escaped when showing mount options,
>  # mount command will not show the word 'spaces' after the spaces
> -$MOUNT_PROG -t overlay | grep ovl_esc_test  | tee -a $seqres.full | grep -v spaces
> +$MOUNT_PROG -t overlay | grep lower3 | tee -a $seqres.full | grep -v spaces
>
>  echo "Silence is golden"
>  status=0
>
> [3]
> [227208.314968] run fstests overlay/083 at 2023-10-22 14:23:06
> [227208.725135] XFS (loop0): Mounting V5 Filesystem 433b1a77-697c-4a83-abbe-85471466b6ca
> [227208.730987] XFS (loop0): Ending clean mount
> [227208.755377] overlayfs: failed to resolve '/mnt/scratch/lower3': -2
> [227208.792994] XFS (loop1): Unmounting Filesystem 0d4e26b6-3202-437c-962c-62c15c4763d7
> [227208.809166] XFS (loop0): Unmounting Filesystem 433b1a77-697c-4a83-abbe-85471466b6ca
>

This is exactly the "libmount regression" I was warning about:

https://lore.kernel.org/linux-unionfs/20231017101118.5h7pj26vos32h63u@ws.net.home/

With libmount upgraded to 2.39, the kernel doesn't get a chance
to parse the arguments correctly:

fsconfig(3, FSCONFIG_SET_STRING, "source", "ovl_esc_test", 0) = 0
fsconfig(3, FSCONFIG_SET_STRING, "upperdir", "/home/amir/upper", 0) = 0
fsconfig(3, FSCONFIG_SET_STRING, "workdir", "/home/amir/work", 0) = 0
fsconfig(3, FSCONFIG_SET_STRING, "lowerdir", "/home/amir/lower3\\", 0) = 0
fsconfig(3, FSCONFIG_SET_FLAG, "with\\", NULL, 0) = 0
fsconfig(3, FSCONFIG_SET_FLAG, "\\", NULL, 0) = 0
fsconfig(3, FSCONFIG_SET_FLAG,
"commas:/home/amir/lower2\\:with\\:"..., NULL, 0) = 0
fsconfig(3, FSCONFIG_CMD_CREATE, NULL, NULL, 0) = 0

Have no mistake, I am not trying to play the blaming game between
kernel and libmount - we simply have regressions that involve them both.

Kernel c34706acf40b "ovl: fix regression in parsing of mount options with"
solves a regression with comma separated list parsing when using the
old mount API.

Even if kernel could workaround the problem above by re-assembling
the options into a monolithic lowerdir list and splitting them up again, it
would be terribly wrong to do that.

The saner solution, as Karel already suggested it for libmount to
parse \, as a non-separator as overlayfs does with old mount api.
Either that, or, default to old mount api for overlayfs with
LIBMOUNT_FORCE_MOUNT2=auto

As for the test in question.
The purpose of this test is to validate that the kernel comma splitting
regression for old mount api has been fixed.
The fix patches are already in the stable kernel and we need to merge
the test, so that distros will also pick the kernel fix.

To meet this goal we could add to this test:
export LIBMOUNT_FORCE_MOUNT2=never
so that it can test what it was meant to test.

Can you please try that?

Regarding the fix to libmount, even if a fix to libmount comma separated
list would land in future libmount version, is it the goal of fstests to
test fixes to libmount?

Thanks,
Amir.
Zorro Lang Oct. 22, 2023, 2:34 p.m. UTC | #5
On Sun, Oct 22, 2023 at 10:09:55AM +0300, Amir Goldstein wrote:
> On Sun, Oct 22, 2023 at 9:26 AM Zorro Lang <zlang@redhat.com> wrote:
> >
> > On Sat, Oct 21, 2023 at 03:48:09PM +0300, Amir Goldstein wrote:
> > >    On Sat, Oct 21, 2023, 3:30 PM Zorro Lang <[1]zlang@redhat.com> wrote:
> > >
> > >      On Fri, Oct 20, 2023 at 07:18:55PM +0300, Amir Goldstein wrote:
> > >      > On Thu, Oct 19, 2023 at 8:50 PM Zorro Lang <[2]zlang@redhat.com>
> > >      wrote:
> > >      > >
> > >      > > On Tue, Oct 17, 2023 at 01:11:45PM +0300, Amir Goldstein wrote:
> > >      > > > Check parsing and display of spaces and escaped colons and
> > >      commans in
> > >      > > > lowerdir mount option.
> > >      > > >
> > >      > > > This is a regression test for two bugs introduced in v6.5 with
> > >      the
> > >      > > > conversion to new mount api.
> > >      > > >
> > >      > > > Signed-off-by: Amir Goldstein <[3]amir73il@gmail.com>
> > >      > > > ---
> > >      > > >
> > >      > > > Zorro,
> > >      > > >
> > >      > > > This is a test for two regressions in kernel v6.5.
> > >      > > > The two fixes were merged in 6.6-rc6 and have been picked for
> > >      > > > the upcoming LTS 6.5.y release.
> > >      > >
> > >      > >
> > >      > > >
> > >      > > > Thanks,
> > >      > > > Amir.
> > >      > > >
> > >      > > >  tests/overlay/083     | 54
> > >      +++++++++++++++++++++++++++++++++++++++++++
> > >      > > >  tests/overlay/083.out |  2 ++
> > >      > > >  2 files changed, 56 insertions(+)
> > >      > > >  create mode 100755 tests/overlay/083
> > >      > > >  create mode 100644 tests/overlay/083.out
> > >      > > >
> > >      > > > diff --git a/tests/overlay/083 b/tests/overlay/083
> > >      > > > new file mode 100755
> > >      > > > index 00000000..071b4b84
> > >      > > > --- /dev/null
> > >      > > > +++ b/tests/overlay/083
> > >      > > > @@ -0,0 +1,54 @@
> > >      > > > +#! /bin/bash
> > >      > > > +# SPDX-License-Identifier: GPL-2.0
> > >      > > > +# Copyright (C) 2023 CTERA Networks. All Rights Reserved.
> > >      > > > +#
> > >      > > > +# FS QA Test 083
> > >      > > > +#
> > >      > > > +# Test regressions in parsing and display of special chars in
> > >      mount options.
> > >      > > > +#
> > >      > > > +# The following kernel commits from v6.5 introduced
> > >      regressions:
> > >      > > > +#  b36a5780cb44 ("ovl: modify layer parameter parsing")
> > >      > > > +#  1784fbc2ed9c ("ovl: port to new mount api")
> > >      > > > +#
> > >      > > > +
> > >      > > > +. ./common/preamble
> > >      > > > +_begin_fstest auto quick mount
> > >      > > > +
> > >      > > > +# Import common functions.
> > >      > > > +. ./common/filter
> > >      > > > +
> > >      > > > +# real QA test starts here
> > >      > > > +_supported_fs overlay
> > >      > > > +_fixed_by_kernel_commit 32db51070850 \
> > >      > > > +     "ovl: fix regression in showing lowerdir mount option"
> > >      > > > +_fixed_by_kernel_commit c34706acf40b \
> > >      > > > +     "ovl: fix regression in parsing of mount options with
> > >      escaped comma"
> > >      > >
> > >      > > Hi Amir,
> > >      > >
> > >      > > I tried this case on the latest linux kernel which contains the
> > >      > > two commits, but still hit below failure:
> > >      > >
> > >      > > FSTYP         -- overlay
> > >      > > PLATFORM      -- Linux/x86_64 hp-dl380pg8-01 6.6.0-rc6-mainline+
> > >      #7 SMP PREEMPT_DYNAMIC Thu Oct 19 22:34:28 CST 2023
> > >      > > MKFS_OPTIONS  -- /mnt/scratch
> > >      > > MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0
> > >      /mnt/scratch /mnt/scratch/ovl-mnt
> > >      > >
> > >      > > overlay/083       - output mismatch (see
> > >      /root/git/xfstests/results//overlay/083.out.bad)
> > >      > >     --- tests/overlay/083.out   2023-10-19 14:07:18.099496414
> > >      +0800
> > >      > >     +++ /root/git/xfstests/results//overlay/083.out.bad
> > >      2023-10-20 00:25:47.682874383 +0800
> > >      > >     @@ -1,2 +1,4 @@
> > >      > >      QA output created by 083
> > >      > >     +mount: /mnt/scratch/ovl-mnt: special device ovl_esc_test
> > >      does not exist.
> > >      > >     +       dmesg(1) may have more information after failed
> > >      mount system call.
> > >      > >      Silence is golden
> > >      > >
> > >      >
> > >      > Strange.
> > >      > I was under the impression that the 'dev' argument to mount
> > >      command
> > >      > of overlayfs is a completely opaque string.
> > >      >
> > >      > Maybe you are using a different libmount version that I do.
> > >      > I have libmount 2.36.1.
> > >      I'm using Fedora rawhide, the libmount version is
> > >      libmount-2.39.2-1.fc40.
> > >      >
> > >      > Anyway, can you please try if this variation works for you:
> > >      >
> > >      > --- a/tests/overlay/083
> > >      > +++ b/tests/overlay/083
> > >      > @@ -42,12 +42,12 @@ mkdir -p "$lowerdir1" "$lowerdir2"
> > >      "$lowerdir3"
> > >      >
> > >      >  # _overlay_mount_* helpers do not handle special chars well, so
> > >      > execute mount directly.
> > >      >  # if escaped colons and commas are not parsed correctly, mount
> > >      will fail.
> > >      > -$MOUNT_PROG -t overlay ovl_esc_test $SCRATCH_MNT \
> > >      > +$MOUNT_PROG -t overlay $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT \
> > >      I doubt it works. This looks like to try to mount /mnt/scratch on
> > >      /mnt/scratch/ovl-mnt.
> > >
> > >    Please try.
> > >    You are making assumption that the dev argument is meaningful to
> > >    overlayfs - it is not.
> > >    Every one of the overlayfs tests in fstests uses the exact same dev
> > >    argument as above inside the ovl mount helpers.
> >
> > Sure Amir, but it's still failed as below [1]. I think I've changed it as
> > you wish [2]. The dmesg shows [3].
> >
> > Thanks,
> > Zorro
> >
> > [1]
> > # ./check -overlay overlay/083
> > FSTYP         -- overlay
> > PLATFORM      -- Linux/x86_64 hp-dl380pg8-01 6.6.0-rc6-mainline+ #7 SMP PREEMPT_DYNAMIC Thu Oct 19 22:34:28 CST 2023
> > MKFS_OPTIONS  -- /mnt/scratch
> > MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /mnt/scratch /mnt/scratch/ovl-mnt
> >
> > overlay/083       - output mismatch (see /root/git/xfstests/results//overlay/083.out.bad)
> >     --- tests/overlay/083.out   2023-10-22 14:21:11.446520444 +0800
> >     +++ /root/git/xfstests/results//overlay/083.out.bad 2023-10-22 14:23:06.642681119 +0800
> >     @@ -1,2 +1,4 @@
> >      QA output created by 083
> >     +mount: /mnt/scratch/ovl-mnt: mount(2) system call failed: No such file or directory.
> >     +       dmesg(1) may have more information after failed mount system call.
> >      Silence is golden
> >     ...
> >     (Run 'diff -u /root/git/xfstests/tests/overlay/083.out /root/git/xfstests/results//overlay/083.out.bad'  to see the entire diff)
> >
> > HINT: You _MAY_ be missing kernel fix:
> >       32db51070850 ovl: fix regression in showing lowerdir mount option
> >
> > HINT: You _MAY_ be missing kernel fix:
> >       c34706acf40b ovl: fix regression in parsing of mount options with escaped comma
> >
> > Ran: overlay/083
> > Failures: overlay/083
> > Failed 1 of 1 tests
> >
> > [2]
> > # git diff
> > diff --git a/tests/overlay/083 b/tests/overlay/083
> > index 071b4b84..3a6d6809 100755
> > --- a/tests/overlay/083
> > +++ b/tests/overlay/083
> > @@ -42,12 +42,12 @@ mkdir -p "$lowerdir1" "$lowerdir2" "$lowerdir3"
> >
> >  # _overlay_mount_* helpers do not handle special chars well, so execute mount directly.
> >  # if escaped colons and commas are not parsed correctly, mount will fail.
> > -$MOUNT_PROG -t overlay ovl_esc_test $SCRATCH_MNT \
> > +$MOUNT_PROG -t overlay $OVL_BASE_SCRATCH_MNT $SCRATCH_MNT \
> >         -o"upperdir=$upperdir,workdir=$workdir,lowerdir=$lowerdir3_esc:$lowerdir2_esc:$lowerdir1"
> >
> >  # if spaces are not escaped when showing mount options,
> >  # mount command will not show the word 'spaces' after the spaces
> > -$MOUNT_PROG -t overlay | grep ovl_esc_test  | tee -a $seqres.full | grep -v spaces
> > +$MOUNT_PROG -t overlay | grep lower3 | tee -a $seqres.full | grep -v spaces
> >
> >  echo "Silence is golden"
> >  status=0
> >
> > [3]
> > [227208.314968] run fstests overlay/083 at 2023-10-22 14:23:06
> > [227208.725135] XFS (loop0): Mounting V5 Filesystem 433b1a77-697c-4a83-abbe-85471466b6ca
> > [227208.730987] XFS (loop0): Ending clean mount
> > [227208.755377] overlayfs: failed to resolve '/mnt/scratch/lower3': -2
> > [227208.792994] XFS (loop1): Unmounting Filesystem 0d4e26b6-3202-437c-962c-62c15c4763d7
> > [227208.809166] XFS (loop0): Unmounting Filesystem 433b1a77-697c-4a83-abbe-85471466b6ca
> >
> 
> This is exactly the "libmount regression" I was warning about:
> 
> https://lore.kernel.org/linux-unionfs/20231017101118.5h7pj26vos32h63u@ws.net.home/
> 
> With libmount upgraded to 2.39, the kernel doesn't get a chance
> to parse the arguments correctly:
> 
> fsconfig(3, FSCONFIG_SET_STRING, "source", "ovl_esc_test", 0) = 0
> fsconfig(3, FSCONFIG_SET_STRING, "upperdir", "/home/amir/upper", 0) = 0
> fsconfig(3, FSCONFIG_SET_STRING, "workdir", "/home/amir/work", 0) = 0
> fsconfig(3, FSCONFIG_SET_STRING, "lowerdir", "/home/amir/lower3\\", 0) = 0
> fsconfig(3, FSCONFIG_SET_FLAG, "with\\", NULL, 0) = 0
> fsconfig(3, FSCONFIG_SET_FLAG, "\\", NULL, 0) = 0
> fsconfig(3, FSCONFIG_SET_FLAG,
> "commas:/home/amir/lower2\\:with\\:"..., NULL, 0) = 0
> fsconfig(3, FSCONFIG_CMD_CREATE, NULL, NULL, 0) = 0
> 
> Have no mistake, I am not trying to play the blaming game between
> kernel and libmount - we simply have regressions that involve them both.
> 
> Kernel c34706acf40b "ovl: fix regression in parsing of mount options with"
> solves a regression with comma separated list parsing when using the
> old mount API.
> 
> Even if kernel could workaround the problem above by re-assembling
> the options into a monolithic lowerdir list and splitting them up again, it
> would be terribly wrong to do that.
> 
> The saner solution, as Karel already suggested it for libmount to
> parse \, as a non-separator as overlayfs does with old mount api.
> Either that, or, default to old mount api for overlayfs with
> LIBMOUNT_FORCE_MOUNT2=auto
> 
> As for the test in question.
> The purpose of this test is to validate that the kernel comma splitting
> regression for old mount api has been fixed.
> The fix patches are already in the stable kernel and we need to merge
> the test, so that distros will also pick the kernel fix.
> 
> To meet this goal we could add to this test:
> export LIBMOUNT_FORCE_MOUNT2=never

Thanks for this details! But by reading your explanation above, I think
you mean setting "LIBMOUNT_FORCE_MOUNT2=always", to force to use classic
mount APIs.

By doing that, this case test passed:

# ./check -overlay overlay/083
FSTYP         -- overlay
PLATFORM      -- Linux/x86_64 hp-dl380pg8-01 6.6.0-rc6-mainline+ #7 SMP PREEMPT_DYNAMIC Thu Oct 19 22:34:28 CST 2023
MKFS_OPTIONS  -- /mnt/scratch
MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /mnt/scratch /mnt/scratch/ovl-mnt

overlay/083        0s
Ran: overlay/083
Passed all 1 tests

Thanks,
Zorro

> so that it can test what it was meant to test.
> 
> Can you please try that?
> 
> Regarding the fix to libmount, even if a fix to libmount comma separated
> list would land in future libmount version, is it the goal of fstests to
> test fixes to libmount?
> 
> Thanks,
> Amir.
>
diff mbox series

Patch

diff --git a/tests/overlay/083 b/tests/overlay/083
new file mode 100755
index 00000000..071b4b84
--- /dev/null
+++ b/tests/overlay/083
@@ -0,0 +1,54 @@ 
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2023 CTERA Networks. All Rights Reserved.
+#
+# FS QA Test 083
+#
+# Test regressions in parsing and display of special chars in mount options.
+#
+# The following kernel commits from v6.5 introduced regressions:
+#  b36a5780cb44 ("ovl: modify layer parameter parsing")
+#  1784fbc2ed9c ("ovl: port to new mount api")
+#
+
+. ./common/preamble
+_begin_fstest auto quick mount
+
+# Import common functions.
+. ./common/filter
+
+# real QA test starts here
+_supported_fs overlay
+_fixed_by_kernel_commit 32db51070850 \
+	"ovl: fix regression in showing lowerdir mount option"
+_fixed_by_kernel_commit c34706acf40b \
+	"ovl: fix regression in parsing of mount options with escaped comma"
+
+# _overlay_check_* helpers do not handle special chars well
+_require_scratch_nocheck
+
+# Remove all files from previous tests
+_scratch_mkfs
+
+# Create lowerdirs with special characters
+lowerdir1="$OVL_BASE_SCRATCH_MNT/lower1 with  spaces"
+lowerdir2="$OVL_BASE_SCRATCH_MNT/lower2:with::colons"
+lowerdir3="$OVL_BASE_SCRATCH_MNT/lower3,with,,commas"
+lowerdir2_esc="$OVL_BASE_SCRATCH_MNT/lower2\:with\:\:colons"
+lowerdir3_esc="$OVL_BASE_SCRATCH_MNT/lower3\,with\,\,commas"
+upperdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
+workdir=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
+mkdir -p "$lowerdir1" "$lowerdir2" "$lowerdir3"
+
+# _overlay_mount_* helpers do not handle special chars well, so execute mount directly.
+# if escaped colons and commas are not parsed correctly, mount will fail.
+$MOUNT_PROG -t overlay ovl_esc_test $SCRATCH_MNT \
+	-o"upperdir=$upperdir,workdir=$workdir,lowerdir=$lowerdir3_esc:$lowerdir2_esc:$lowerdir1"
+
+# if spaces are not escaped when showing mount options,
+# mount command will not show the word 'spaces' after the spaces
+$MOUNT_PROG -t overlay | grep ovl_esc_test  | tee -a $seqres.full | grep -v spaces
+
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/overlay/083.out b/tests/overlay/083.out
new file mode 100644
index 00000000..0beba309
--- /dev/null
+++ b/tests/overlay/083.out
@@ -0,0 +1,2 @@ 
+QA output created by 083
+Silence is golden