Linux Overlay Filesystem development
 help / color / mirror / Atom feed
* [PATCH fstests v2] overlay: add a test for redirect_dir=followdata
@ 2026-11-09 13:38 Giuseppe Scrivano
  2026-11-10 11:43 ` Amir Goldstein
  0 siblings, 1 reply; 4+ messages in thread
From: Giuseppe Scrivano @ 2026-11-09 13:38 UTC (permalink / raw)
  To: fstests; +Cc: linux-unionfs, amir73il

Test that data-only layers select redirect_dir=followdata, which follows
redirects from lower to lowerdata from any lower layer, with userxattr
and trusted xattrs, and that redirect_dir=nofollow is refused with
data-only layers.

Also test following an absolute redirect past a lower layer that has the
parent directory of the target, which failed with EIO before the fix
"ovl: do not always clear absolute_redirect".

Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
 common/overlay        |  15 +++++
 tests/overlay/090     | 149 ++++++++++++++++++++++++++++++++++++++++++
 tests/overlay/090.out |  43 ++++++++++++
 3 files changed, 207 insertions(+)
 create mode 100755 tests/overlay/090
 create mode 100644 tests/overlay/090.out

diff --git a/common/overlay b/common/overlay
index 8e89d6e4..382960c3 100644
--- a/common/overlay
+++ b/common/overlay
@@ -286,6 +286,21 @@ _require_scratch_overlay_datadir_without_metacopy()
 
 }
 
+# Check kernel support for the "redirect_dir=followdata" mount option
+_require_scratch_overlay_redirect_followdata()
+{
+	local lowerdir="$OVL_BASE_SCRATCH_MNT/$OVL_UPPER"
+	local datadir="$OVL_BASE_SCRATCH_MNT/$OVL_LOWER"
+
+	_scratch_mkfs > /dev/null 2>&1
+	_overlay_scratch_mount_opts \
+		-o"lowerdir+=$lowerdir,datadir+=$datadir" \
+		-o"userxattr,redirect_dir=followdata" > /dev/null 2>&1 || \
+		_notrun "overlay redirect_dir=followdata not supported on ${SCRATCH_DEV}"
+
+	_scratch_unmount
+}
+
 
 # Helper function to check underlying dirs of overlay filesystem
 _overlay_fsck_dirs()
diff --git a/tests/overlay/090 b/tests/overlay/090
new file mode 100755
index 00000000..6eeeacdf
--- /dev/null
+++ b/tests/overlay/090
@@ -0,0 +1,149 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2026 Giuseppe Scrivano.  All Rights Reserved.
+#
+# FS QA Test 090
+#
+# Test the redirect_dir=followdata mount option.
+#
+# Data-only layers select redirect_dir=followdata, which follows redirects
+# from any lower layer to a data-only layer, and cannot be used with
+# redirect_dir=nofollow.
+#
+# Also test following an absolute redirect past a lower layer that has the
+# parent directory of the redirect target, with metacopy=on.
+#
+. ./common/preamble
+_begin_fstest auto quick metacopy redirect attr
+
+# Import common functions.
+. ./common/filter
+. ./common/attr
+
+# We use non-default scratch underlying overlay dirs, we need to check
+# them explicitly after test.
+_require_scratch_nocheck
+_require_attrs user
+_require_attrs trusted
+_require_scratch_overlay_redirect_followdata
+_require_scratch_overlay_features redirect_dir metacopy
+_require_scratch_overlay_lowerdata_layers
+
+data=$OVL_BASE_SCRATCH_MNT/data
+l1=$OVL_BASE_SCRATCH_MNT/lower1
+l2=$OVL_BASE_SCRATCH_MNT/lower2
+l3=$OVL_BASE_SCRATCH_MNT/lower3
+upper=$OVL_BASE_SCRATCH_MNT/upper
+work=$OVL_BASE_SCRATCH_MNT/work
+layers="$l3:$l2:$l1::$data"
+
+# Make a metacopy file <file> with the redirect <redirect>, for both
+# userxattr and trusted xattr mounts
+make_stub()
+{
+	local file=$1 redirect=$2 x
+
+	touch $file
+	for x in user.overlay. ${OVL_XATTR_METACOPY%metacopy}; do
+		$SETFATTR_PROG -n ${x}metacopy $file
+		$SETFATTR_PROG -n ${x}redirect -v $redirect $file
+	done
+	truncate -s 4 $file
+}
+
+# Mount the layers $layers with the options <opts>, print redirect_dir=followdata
+# if the mount shows it and <opts> has userxattr or redirect_dir (otherwise it
+# depends on the kernel config), and the content of each of the following files,
+# or the error reading it
+check()
+{
+	local opts=$1 f
+
+	shift
+	echo "== $opts"
+	_overlay_scratch_mount_dirs "$layers" $upper $work -o $opts
+	case $opts in
+	*userxattr*|*redirect_dir=*)
+		_fs_options $OVL_BASE_SCRATCH_MNT | grep -o redirect_dir=followdata
+		;;
+	esac
+	for f; do
+		echo "$f: $(cat $SCRATCH_MNT/$f 2>&1 | sed 's/^cat: .*: //')"
+	done
+	_unmount $SCRATCH_MNT
+}
+
+# Mounting the layers $layers with the options <opts> must fail
+check_fail()
+{
+	local opts=$1
+
+	echo "== $opts"
+	_overlay_scratch_mount_dirs "$layers" $upper $work -o $opts \
+		>> $seqres.full 2>&1 && echo "mount succeeded" && \
+		_unmount $SCRATCH_MNT
+}
+
+_scratch_mkfs
+mkdir -p $data/base $l1/base $l2/base $l3 $upper $work
+echo -n "data" > $data/base/file
+
+make_stub $l1/s1 /base/file
+make_stub $l2/s2 /base/file
+make_stub $l3/s3 /base/file
+# The redirect target in a lower layer below s3, which is not looked up
+echo -n "lowr" > $l2/base/file
+# Redirects that are not followed: relative, directory, upper
+echo -n "lowr" > $l2/reltarget
+make_stub $l3/rel reltarget
+mkdir $l3/dir
+$SETFATTR_PROG -n user.overlay.redirect -v /base $l3/dir
+$SETFATTR_PROG -n $OVL_XATTR_REDIRECT -v /base $l3/dir
+make_stub $upper/up /base/file
+
+# followdata is selected by default with userxattr and a data-only layer
+check userxattr s1 s2 s3
+check userxattr,redirect_dir=followdata s1 s2 s3 rel dir up
+# a data-only layer cannot be used without following redirects
+check_fail userxattr,redirect_dir=nofollow
+check_fail userxattr,redirect_dir=off
+echo -n "no data-only layer "
+layers="$l3:$l2:$l1" check userxattr
+
+check metacopy=off s1 s2 s3
+check redirect_dir=followdata s1 s2 s3 rel dir up
+check redirect_dir=follow,metacopy=off s1 s2 s3
+check redirect_dir=on,metacopy=off s1 s2 s3
+check_fail redirect_dir=nofollow
+echo -n "no data-only layer "
+layers="$l3:$l2:$l1" check redirect_dir=nofollow
+
+# An explicit option requires a data-only layer and conflicts with metacopy=on
+_overlay_scratch_mount_dirs "$l3:$l2:$l1" $upper $work \
+	-o userxattr,redirect_dir=followdata >> $seqres.full 2>&1 && \
+	echo "mount redirect_dir=followdata without a data-only layer succeeded"
+_overlay_scratch_mount_dirs "$layers" $upper $work \
+	-o redirect_dir=followdata,metacopy=on >> $seqres.full 2>&1 && \
+	echo "mount with redirect_dir=followdata,metacopy=on succeeded"
+
+# The directory "/base" in a lower layer without a redirect must not make the
+# lookup forget that the redirect of the metacopy file is absolute.
+echo "== metacopy=on,redirect_dir=follow"
+_scratch_mkfs
+mkdir -p $data/base $l1 $l2
+echo -n "data" > $data/base/file
+touch $l1/file
+$SETFATTR_PROG -n $OVL_XATTR_METACOPY $l1/file
+$SETFATTR_PROG -n $OVL_XATTR_REDIRECT -v /base/file $l1/file
+truncate -s 4 $l1/file
+for f in "" base base/file; do
+	[ "$f" = base ] && mkdir $l2/base
+	[ "$f" = base/file ] && echo -n "lowr" > $l2/base/file
+	_overlay_scratch_mount_dirs "$l1:$l2::$data" - - \
+		-o ro,redirect_dir=follow,metacopy=on
+	echo "lower2 /$f: $(cat $SCRATCH_MNT/file 2>&1 | sed 's/^cat: .*: //')"
+	_unmount $SCRATCH_MNT
+done
+
+# success, all done
+_exit 0
diff --git a/tests/overlay/090.out b/tests/overlay/090.out
new file mode 100644
index 00000000..c2f576e2
--- /dev/null
+++ b/tests/overlay/090.out
@@ -0,0 +1,43 @@
+QA output created by 090
+== userxattr
+redirect_dir=followdata
+s1: data
+s2: data
+s3: data
+== userxattr,redirect_dir=followdata
+redirect_dir=followdata
+s1: data
+s2: data
+s3: data
+rel: Operation not permitted
+dir: Operation not permitted
+up: Operation not permitted
+== userxattr,redirect_dir=nofollow
+== userxattr,redirect_dir=off
+no data-only layer == userxattr
+== metacopy=off
+s1: data
+s2: data
+s3: data
+== redirect_dir=followdata
+redirect_dir=followdata
+s1: data
+s2: data
+s3: data
+rel: Operation not permitted
+dir: Operation not permitted
+up: Operation not permitted
+== redirect_dir=follow,metacopy=off
+s1: data
+s2: data
+s3: data
+== redirect_dir=on,metacopy=off
+s1: data
+s2: data
+s3: data
+== redirect_dir=nofollow
+no data-only layer == redirect_dir=nofollow
+== metacopy=on,redirect_dir=follow
+lower2 /: data
+lower2 /base: data
+lower2 /base/file: lowr
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH fstests v2] overlay: add a test for redirect_dir=followdata
  2026-11-09 13:38 [PATCH fstests v2] overlay: add a test for redirect_dir=followdata Giuseppe Scrivano
@ 2026-11-10 11:43 ` Amir Goldstein
  2026-11-10 14:02   ` Giuseppe Scrivano
  0 siblings, 1 reply; 4+ messages in thread
From: Amir Goldstein @ 2026-11-10 11:43 UTC (permalink / raw)
  To: Giuseppe Scrivano; +Cc: fstests, linux-unionfs

On Mon, Nov 9, 2026 at 2:40 PM Giuseppe Scrivano <giuseppe@scrivano.org> wrote:
>
> Test that data-only layers select redirect_dir=followdata, which follows
> redirects from lower to lowerdata from any lower layer, with userxattr
> and trusted xattrs, and that redirect_dir=nofollow is refused with
> data-only layers.
>
> Also test following an absolute redirect past a lower layer that has the
> parent directory of the target, which failed with EIO before the fix
> "ovl: do not always clear absolute_redirect".
>
> Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  common/overlay        |  15 +++++
>  tests/overlay/090     | 149 ++++++++++++++++++++++++++++++++++++++++++
>  tests/overlay/090.out |  43 ++++++++++++
>  3 files changed, 207 insertions(+)
>  create mode 100755 tests/overlay/090
>  create mode 100644 tests/overlay/090.out
>
> diff --git a/common/overlay b/common/overlay
> index 8e89d6e4..382960c3 100644
> --- a/common/overlay
> +++ b/common/overlay
> @@ -286,6 +286,21 @@ _require_scratch_overlay_datadir_without_metacopy()
>
>  }
>
> +# Check kernel support for the "redirect_dir=followdata" mount option
> +_require_scratch_overlay_redirect_followdata()
> +{
> +       local lowerdir="$OVL_BASE_SCRATCH_MNT/$OVL_UPPER"
> +       local datadir="$OVL_BASE_SCRATCH_MNT/$OVL_LOWER"
> +
> +       _scratch_mkfs > /dev/null 2>&1
> +       _overlay_scratch_mount_opts \
> +               -o"lowerdir+=$lowerdir,datadir+=$datadir" \
> +               -o"userxattr,redirect_dir=followdata" > /dev/null 2>&1 || \
> +               _notrun "overlay redirect_dir=followdata not supported on ${SCRATCH_DEV}"
> +
> +       _scratch_unmount
> +}
> +
>
>  # Helper function to check underlying dirs of overlay filesystem
>  _overlay_fsck_dirs()
> diff --git a/tests/overlay/090 b/tests/overlay/090
> new file mode 100755
> index 00000000..6eeeacdf
> --- /dev/null
> +++ b/tests/overlay/090
> @@ -0,0 +1,149 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2026 Giuseppe Scrivano.  All Rights Reserved.
> +#
> +# FS QA Test 090
> +#
> +# Test the redirect_dir=followdata mount option.
> +#
> +# Data-only layers select redirect_dir=followdata, which follows redirects
> +# from any lower layer to a data-only layer, and cannot be used with
> +# redirect_dir=nofollow.
> +#
> +# Also test following an absolute redirect past a lower layer that has the
> +# parent directory of the redirect target, with metacopy=on.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick metacopy redirect attr
> +
> +# Import common functions.
> +. ./common/filter
> +. ./common/attr
> +
> +# We use non-default scratch underlying overlay dirs, we need to check
> +# them explicitly after test.
> +_require_scratch_nocheck
> +_require_attrs user
> +_require_attrs trusted
> +_require_scratch_overlay_redirect_followdata
> +_require_scratch_overlay_features redirect_dir metacopy
> +_require_scratch_overlay_lowerdata_layers
> +
> +data=$OVL_BASE_SCRATCH_MNT/data
> +l1=$OVL_BASE_SCRATCH_MNT/lower1
> +l2=$OVL_BASE_SCRATCH_MNT/lower2
> +l3=$OVL_BASE_SCRATCH_MNT/lower3
> +upper=$OVL_BASE_SCRATCH_MNT/upper
> +work=$OVL_BASE_SCRATCH_MNT/work
> +layers="$l3:$l2:$l1::$data"
> +
> +# Make a metacopy file <file> with the redirect <redirect>, for both
> +# userxattr and trusted xattr mounts
> +make_stub()
> +{
> +       local file=$1 redirect=$2 x
> +
> +       touch $file
> +       for x in user.overlay. ${OVL_XATTR_METACOPY%metacopy}; do
> +               $SETFATTR_PROG -n ${x}metacopy $file
> +               $SETFATTR_PROG -n ${x}redirect -v $redirect $file
> +       done
> +       truncate -s 4 $file
> +}
> +
> +# Mount the layers $layers with the options <opts>, print redirect_dir=followdata
> +# if the mount shows it and <opts> has userxattr or redirect_dir (otherwise it
> +# depends on the kernel config), and the content of each of the following files,
> +# or the error reading it
> +check()
> +{
> +       local opts=$1 f
> +
> +       shift
> +       echo "== $opts"
> +       _overlay_scratch_mount_dirs "$layers" $upper $work -o $opts
> +       case $opts in
> +       *userxattr*|*redirect_dir=*)
> +               _fs_options $OVL_BASE_SCRATCH_MNT | grep -o redirect_dir=followdata
> +               ;;
> +       esac
> +       for f; do
> +               echo "$f: $(cat $SCRATCH_MNT/$f 2>&1 | sed 's/^cat: .*: //')"
> +       done
> +       _unmount $SCRATCH_MNT
> +}
> +
> +# Mounting the layers $layers with the options <opts> must fail
> +check_fail()
> +{
> +       local opts=$1
> +
> +       echo "== $opts"
> +       _overlay_scratch_mount_dirs "$layers" $upper $work -o $opts \
> +               >> $seqres.full 2>&1 && echo "mount succeeded" && \
> +               _unmount $SCRATCH_MNT
> +}
> +
> +_scratch_mkfs
> +mkdir -p $data/base $l1/base $l2/base $l3 $upper $work
> +echo -n "data" > $data/base/file
> +
> +make_stub $l1/s1 /base/file
> +make_stub $l2/s2 /base/file
> +make_stub $l3/s3 /base/file
> +# The redirect target in a lower layer below s3, which is not looked up
> +echo -n "lowr" > $l2/base/file
> +# Redirects that are not followed: relative, directory, upper
> +echo -n "lowr" > $l2/reltarget
> +make_stub $l3/rel reltarget
> +mkdir $l3/dir
> +$SETFATTR_PROG -n user.overlay.redirect -v /base $l3/dir
> +$SETFATTR_PROG -n $OVL_XATTR_REDIRECT -v /base $l3/dir
> +make_stub $upper/up /base/file
> +
> +# followdata is selected by default with userxattr and a data-only layer
> +check userxattr s1 s2 s3
> +check userxattr,redirect_dir=followdata s1 s2 s3 rel dir up
> +# a data-only layer cannot be used without following redirects
> +check_fail userxattr,redirect_dir=nofollow
> +check_fail userxattr,redirect_dir=off

The test looks good but this expectation of failure is problematic.

First of all the test would fail with redirect_always_follow=N
because that makes redirect_dir=off => followdata

But I also think that redirect_dir=off should not fail with
redirect_always_follow=Y.

I will explain with the documentation.
You documentation v2 patch says:

:A data-only layer selects "redirect_dir=followdata", unless
"redirect_dir=follow"
:or "redirect_dir=on" is in effect.

So it implies redirect_dir=off => followdata

All that redirect_dir=off really means is "don't create redirects".

I amended the document as follows to clarify that:

- "redirect_dir=on":
    Redirects are created and followed.
- "redirect_dir=follow":
    Redirects are not created, but followed.
- "redirect_dir=followdata":
    Redirects are not created.
    Only redirects from lower to lowerdata are followed.
- "redirect_dir=nofollow":
    Redirects are not created and not followed.
- "redirect_dir=off":
    Redirects are not created.
    Which redirects are followed depends on "redirect_always_follow" in the
    kernel/module config, on data-only lower layers and on the "userxattr"
    mount option.

Made this off => followdata change in the code and pushed to ovl-fixes:

-static int ovl_redirect_follow_mode(size_t nr_data)
+int ovl_redirect_follow_mode(bool follow_lower, size_t nr_data)
 {
-       return ovl_redirect_always_follow ? OVL_REDIRECT_FOLLOW :
-                                           ovl_redirect_nofollow_mode(nr_data);
+       if (ovl_redirect_always_follow && follow_lower)
+               return OVL_REDIRECT_FOLLOW;
+
+       /* Data-only layers cannot be used without following redirects
to them */
+       return nr_data ? OVL_REDIRECT_FOLLOWDATA : OVL_REDIRECT_NOFOLLOW;
 }

 /*
@@ -137,7 +140,7 @@ static int ovl_redirect_follow_mode(size_t nr_data)
 static int ovl_redirect_mode_def(void)
 {
        return ovl_redirect_dir_def ? OVL_REDIRECT_ON :
-                                     ovl_redirect_follow_mode(0);
+                                     ovl_redirect_follow_mode(true, 0);
 }

 static const struct constant_table ovl_parameter_verity[] = {
@@ -896,6 +899,9 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
        struct ovl_opt_set set = ctx->set;
        /* The requested redirect_dir=, for reporting conflicts with it */
        int redirect_opt = config->redirect_mode;
+       /* The redirect follow mode for redirect_dir=off and for the default */
+       int follow_mode = ovl_redirect_follow_mode(!config->userxattr,
+                                                  ctx->nr_data);

        /* Workdir/index are useless in non-upper mount */
        if (!config->upperdir) {
@@ -922,9 +928,12 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
                config->uuid = OVL_UUID_NULL;
        }

-       /* Resolve redirect_dir=off to the default redirect follow mode */
+       /*
+        * Resolve redirect_dir=off to the default redirect follow mode.
+        * With userxattr, redirects are not followed to lower by default.
+        */
        if (config->redirect_mode == OVL_REDIRECT_OFF)
-               config->redirect_mode = ovl_redirect_follow_mode(ctx->nr_data);
+               config->redirect_mode = follow_mode;

        /*
         * This is to make the logic below simpler.  It doesn't make any other
@@ -1038,7 +1047,7 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
                 * userxattr.
                 */
                if (!set.redirect)
-                       config->redirect_mode =
ovl_redirect_nofollow_mode(ctx->nr_data);
+                       config->redirect_mode = follow_mode;
                config->metacopy = false;
        }


Please amend the test and test with the latest ovl-fixes:
d0d57557b76de ovl: follow lower to lowerdata redirects from any lower layer

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH fstests v2] overlay: add a test for redirect_dir=followdata
  2026-11-10 11:43 ` Amir Goldstein
@ 2026-11-10 14:02   ` Giuseppe Scrivano
  2026-11-10 14:27     ` Amir Goldstein
  0 siblings, 1 reply; 4+ messages in thread
From: Giuseppe Scrivano @ 2026-11-10 14:02 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: fstests, linux-unionfs

Hi Amir,

Amir Goldstein <amir73il@gmail.com> writes:

> On Mon, Nov 9, 2026 at 2:40 PM Giuseppe Scrivano <giuseppe@scrivano.org> wrote:
>>
>> Test that data-only layers select redirect_dir=followdata, which follows
>> redirects from lower to lowerdata from any lower layer, with userxattr
>> and trusted xattrs, and that redirect_dir=nofollow is refused with
>> data-only layers.
>>
>> Also test following an absolute redirect past a lower layer that has the
>> parent directory of the target, which failed with EIO before the fix
>> "ovl: do not always clear absolute_redirect".
>>
>> Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>
>> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
>> ---
>>  common/overlay        |  15 +++++
>>  tests/overlay/090     | 149 ++++++++++++++++++++++++++++++++++++++++++
>>  tests/overlay/090.out |  43 ++++++++++++
>>  3 files changed, 207 insertions(+)
>>  create mode 100755 tests/overlay/090
>>  create mode 100644 tests/overlay/090.out
>>
>> diff --git a/common/overlay b/common/overlay
>> index 8e89d6e4..382960c3 100644
>> --- a/common/overlay
>> +++ b/common/overlay
>> @@ -286,6 +286,21 @@ _require_scratch_overlay_datadir_without_metacopy()
>>
>>  }
>>
>> +# Check kernel support for the "redirect_dir=followdata" mount option
>> +_require_scratch_overlay_redirect_followdata()
>> +{
>> +       local lowerdir="$OVL_BASE_SCRATCH_MNT/$OVL_UPPER"
>> +       local datadir="$OVL_BASE_SCRATCH_MNT/$OVL_LOWER"
>> +
>> +       _scratch_mkfs > /dev/null 2>&1
>> +       _overlay_scratch_mount_opts \
>> +               -o"lowerdir+=$lowerdir,datadir+=$datadir" \
>> +               -o"userxattr,redirect_dir=followdata" > /dev/null 2>&1 || \
>> +               _notrun "overlay redirect_dir=followdata not supported on ${SCRATCH_DEV}"
>> +
>> +       _scratch_unmount
>> +}
>> +
>>
>>  # Helper function to check underlying dirs of overlay filesystem
>>  _overlay_fsck_dirs()
>> diff --git a/tests/overlay/090 b/tests/overlay/090
>> new file mode 100755
>> index 00000000..6eeeacdf
>> --- /dev/null
>> +++ b/tests/overlay/090
>> @@ -0,0 +1,149 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2026 Giuseppe Scrivano.  All Rights Reserved.
>> +#
>> +# FS QA Test 090
>> +#
>> +# Test the redirect_dir=followdata mount option.
>> +#
>> +# Data-only layers select redirect_dir=followdata, which follows redirects
>> +# from any lower layer to a data-only layer, and cannot be used with
>> +# redirect_dir=nofollow.
>> +#
>> +# Also test following an absolute redirect past a lower layer that has the
>> +# parent directory of the redirect target, with metacopy=on.
>> +#
>> +. ./common/preamble
>> +_begin_fstest auto quick metacopy redirect attr
>> +
>> +# Import common functions.
>> +. ./common/filter
>> +. ./common/attr
>> +
>> +# We use non-default scratch underlying overlay dirs, we need to check
>> +# them explicitly after test.
>> +_require_scratch_nocheck
>> +_require_attrs user
>> +_require_attrs trusted
>> +_require_scratch_overlay_redirect_followdata
>> +_require_scratch_overlay_features redirect_dir metacopy
>> +_require_scratch_overlay_lowerdata_layers
>> +
>> +data=$OVL_BASE_SCRATCH_MNT/data
>> +l1=$OVL_BASE_SCRATCH_MNT/lower1
>> +l2=$OVL_BASE_SCRATCH_MNT/lower2
>> +l3=$OVL_BASE_SCRATCH_MNT/lower3
>> +upper=$OVL_BASE_SCRATCH_MNT/upper
>> +work=$OVL_BASE_SCRATCH_MNT/work
>> +layers="$l3:$l2:$l1::$data"
>> +
>> +# Make a metacopy file <file> with the redirect <redirect>, for both
>> +# userxattr and trusted xattr mounts
>> +make_stub()
>> +{
>> +       local file=$1 redirect=$2 x
>> +
>> +       touch $file
>> +       for x in user.overlay. ${OVL_XATTR_METACOPY%metacopy}; do
>> +               $SETFATTR_PROG -n ${x}metacopy $file
>> +               $SETFATTR_PROG -n ${x}redirect -v $redirect $file
>> +       done
>> +       truncate -s 4 $file
>> +}
>> +
>> +# Mount the layers $layers with the options <opts>, print redirect_dir=followdata
>> +# if the mount shows it and <opts> has userxattr or redirect_dir (otherwise it
>> +# depends on the kernel config), and the content of each of the following files,
>> +# or the error reading it
>> +check()
>> +{
>> +       local opts=$1 f
>> +
>> +       shift
>> +       echo "== $opts"
>> +       _overlay_scratch_mount_dirs "$layers" $upper $work -o $opts
>> +       case $opts in
>> +       *userxattr*|*redirect_dir=*)
>> +               _fs_options $OVL_BASE_SCRATCH_MNT | grep -o redirect_dir=followdata
>> +               ;;
>> +       esac
>> +       for f; do
>> +               echo "$f: $(cat $SCRATCH_MNT/$f 2>&1 | sed 's/^cat: .*: //')"
>> +       done
>> +       _unmount $SCRATCH_MNT
>> +}
>> +
>> +# Mounting the layers $layers with the options <opts> must fail
>> +check_fail()
>> +{
>> +       local opts=$1
>> +
>> +       echo "== $opts"
>> +       _overlay_scratch_mount_dirs "$layers" $upper $work -o $opts \
>> +               >> $seqres.full 2>&1 && echo "mount succeeded" && \
>> +               _unmount $SCRATCH_MNT
>> +}
>> +
>> +_scratch_mkfs
>> +mkdir -p $data/base $l1/base $l2/base $l3 $upper $work
>> +echo -n "data" > $data/base/file
>> +
>> +make_stub $l1/s1 /base/file
>> +make_stub $l2/s2 /base/file
>> +make_stub $l3/s3 /base/file
>> +# The redirect target in a lower layer below s3, which is not looked up
>> +echo -n "lowr" > $l2/base/file
>> +# Redirects that are not followed: relative, directory, upper
>> +echo -n "lowr" > $l2/reltarget
>> +make_stub $l3/rel reltarget
>> +mkdir $l3/dir
>> +$SETFATTR_PROG -n user.overlay.redirect -v /base $l3/dir
>> +$SETFATTR_PROG -n $OVL_XATTR_REDIRECT -v /base $l3/dir
>> +make_stub $upper/up /base/file
>> +
>> +# followdata is selected by default with userxattr and a data-only layer
>> +check userxattr s1 s2 s3
>> +check userxattr,redirect_dir=followdata s1 s2 s3 rel dir up
>> +# a data-only layer cannot be used without following redirects
>> +check_fail userxattr,redirect_dir=nofollow
>> +check_fail userxattr,redirect_dir=off
>
> The test looks good but this expectation of failure is problematic.
>
> First of all the test would fail with redirect_always_follow=N
> because that makes redirect_dir=off => followdata
>
> But I also think that redirect_dir=off should not fail with
> redirect_always_follow=Y.
>
> I will explain with the documentation.
> You documentation v2 patch says:
>
> :A data-only layer selects "redirect_dir=followdata", unless
> "redirect_dir=follow"
> :or "redirect_dir=on" is in effect.
>
> So it implies redirect_dir=off => followdata
>
> All that redirect_dir=off really means is "don't create redirects".
>
> I amended the document as follows to clarify that:
>
> - "redirect_dir=on":
>     Redirects are created and followed.
> - "redirect_dir=follow":
>     Redirects are not created, but followed.
> - "redirect_dir=followdata":
>     Redirects are not created.
>     Only redirects from lower to lowerdata are followed.
> - "redirect_dir=nofollow":
>     Redirects are not created and not followed.
> - "redirect_dir=off":
>     Redirects are not created.
>     Which redirects are followed depends on "redirect_always_follow" in the
>     kernel/module config, on data-only lower layers and on the "userxattr"
>     mount option.
>
> Made this off => followdata change in the code and pushed to ovl-fixes:
>
> -static int ovl_redirect_follow_mode(size_t nr_data)
> +int ovl_redirect_follow_mode(bool follow_lower, size_t nr_data)
>  {
> -       return ovl_redirect_always_follow ? OVL_REDIRECT_FOLLOW :
> -                                           ovl_redirect_nofollow_mode(nr_data);
> +       if (ovl_redirect_always_follow && follow_lower)
> +               return OVL_REDIRECT_FOLLOW;
> +
> +       /* Data-only layers cannot be used without following redirects
> to them */
> +       return nr_data ? OVL_REDIRECT_FOLLOWDATA : OVL_REDIRECT_NOFOLLOW;
>  }
>
>  /*
> @@ -137,7 +140,7 @@ static int ovl_redirect_follow_mode(size_t nr_data)
>  static int ovl_redirect_mode_def(void)
>  {
>         return ovl_redirect_dir_def ? OVL_REDIRECT_ON :
> -                                     ovl_redirect_follow_mode(0);
> +                                     ovl_redirect_follow_mode(true, 0);
>  }
>
>  static const struct constant_table ovl_parameter_verity[] = {
> @@ -896,6 +899,9 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
>         struct ovl_opt_set set = ctx->set;
>         /* The requested redirect_dir=, for reporting conflicts with it */
>         int redirect_opt = config->redirect_mode;
> +       /* The redirect follow mode for redirect_dir=off and for the default */
> +       int follow_mode = ovl_redirect_follow_mode(!config->userxattr,
> +                                                  ctx->nr_data);
>
>         /* Workdir/index are useless in non-upper mount */
>         if (!config->upperdir) {
> @@ -922,9 +928,12 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
>                 config->uuid = OVL_UUID_NULL;
>         }
>
> -       /* Resolve redirect_dir=off to the default redirect follow mode */
> +       /*
> +        * Resolve redirect_dir=off to the default redirect follow mode.
> +        * With userxattr, redirects are not followed to lower by default.
> +        */
>         if (config->redirect_mode == OVL_REDIRECT_OFF)
> -               config->redirect_mode = ovl_redirect_follow_mode(ctx->nr_data);
> +               config->redirect_mode = follow_mode;
>
>         /*
>          * This is to make the logic below simpler.  It doesn't make any other
> @@ -1038,7 +1047,7 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
>                  * userxattr.
>                  */
>                 if (!set.redirect)
> -                       config->redirect_mode =
> ovl_redirect_nofollow_mode(ctx->nr_data);
> +                       config->redirect_mode = follow_mode;
>                 config->metacopy = false;
>         }
>
>
> Please amend the test and test with the latest ovl-fixes:
> d0d57557b76de ovl: follow lower to lowerdata redirects from any lower layer

I've tested this modified version and it works fine.  Do you want me to
send a v3 or will you amend it?

Thanks,
Giuseppe

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH fstests v2] overlay: add a test for redirect_dir=followdata
  2026-11-10 14:02   ` Giuseppe Scrivano
@ 2026-11-10 14:27     ` Amir Goldstein
  0 siblings, 0 replies; 4+ messages in thread
From: Amir Goldstein @ 2026-11-10 14:27 UTC (permalink / raw)
  To: Giuseppe Scrivano; +Cc: fstests, linux-unionfs

On Tue, Nov 10, 2026 at 3:02 PM Giuseppe Scrivano <giuseppe@scrivano.org> wrote:
>
> Hi Amir,
>
> Amir Goldstein <amir73il@gmail.com> writes:
>
> > On Mon, Nov 9, 2026 at 2:40 PM Giuseppe Scrivano <giuseppe@scrivano.org> wrote:
> >>
> >> Test that data-only layers select redirect_dir=followdata, which follows
> >> redirects from lower to lowerdata from any lower layer, with userxattr
> >> and trusted xattrs, and that redirect_dir=nofollow is refused with
> >> data-only layers.
> >>
> >> Also test following an absolute redirect past a lower layer that has the
> >> parent directory of the target, which failed with EIO before the fix
> >> "ovl: do not always clear absolute_redirect".
> >>
> >> Signed-off-by: Giuseppe Scrivano <giuseppe@scrivano.org>
> >> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> >> ---
> >>  common/overlay        |  15 +++++
> >>  tests/overlay/090     | 149 ++++++++++++++++++++++++++++++++++++++++++
> >>  tests/overlay/090.out |  43 ++++++++++++
> >>  3 files changed, 207 insertions(+)
> >>  create mode 100755 tests/overlay/090
> >>  create mode 100644 tests/overlay/090.out
> >>
> >> diff --git a/common/overlay b/common/overlay
> >> index 8e89d6e4..382960c3 100644
> >> --- a/common/overlay
> >> +++ b/common/overlay
> >> @@ -286,6 +286,21 @@ _require_scratch_overlay_datadir_without_metacopy()
> >>
> >>  }
> >>
> >> +# Check kernel support for the "redirect_dir=followdata" mount option
> >> +_require_scratch_overlay_redirect_followdata()
> >> +{
> >> +       local lowerdir="$OVL_BASE_SCRATCH_MNT/$OVL_UPPER"
> >> +       local datadir="$OVL_BASE_SCRATCH_MNT/$OVL_LOWER"
> >> +
> >> +       _scratch_mkfs > /dev/null 2>&1
> >> +       _overlay_scratch_mount_opts \
> >> +               -o"lowerdir+=$lowerdir,datadir+=$datadir" \
> >> +               -o"userxattr,redirect_dir=followdata" > /dev/null 2>&1 || \
> >> +               _notrun "overlay redirect_dir=followdata not supported on ${SCRATCH_DEV}"
> >> +
> >> +       _scratch_unmount
> >> +}
> >> +
> >>
> >>  # Helper function to check underlying dirs of overlay filesystem
> >>  _overlay_fsck_dirs()
> >> diff --git a/tests/overlay/090 b/tests/overlay/090
> >> new file mode 100755
> >> index 00000000..6eeeacdf
> >> --- /dev/null
> >> +++ b/tests/overlay/090
> >> @@ -0,0 +1,149 @@
> >> +#! /bin/bash
> >> +# SPDX-License-Identifier: GPL-2.0
> >> +# Copyright (c) 2026 Giuseppe Scrivano.  All Rights Reserved.
> >> +#
> >> +# FS QA Test 090
> >> +#
> >> +# Test the redirect_dir=followdata mount option.
> >> +#
> >> +# Data-only layers select redirect_dir=followdata, which follows redirects
> >> +# from any lower layer to a data-only layer, and cannot be used with
> >> +# redirect_dir=nofollow.
> >> +#
> >> +# Also test following an absolute redirect past a lower layer that has the
> >> +# parent directory of the redirect target, with metacopy=on.
> >> +#
> >> +. ./common/preamble
> >> +_begin_fstest auto quick metacopy redirect attr
> >> +
> >> +# Import common functions.
> >> +. ./common/filter
> >> +. ./common/attr
> >> +
> >> +# We use non-default scratch underlying overlay dirs, we need to check
> >> +# them explicitly after test.
> >> +_require_scratch_nocheck
> >> +_require_attrs user
> >> +_require_attrs trusted
> >> +_require_scratch_overlay_redirect_followdata
> >> +_require_scratch_overlay_features redirect_dir metacopy
> >> +_require_scratch_overlay_lowerdata_layers
> >> +
> >> +data=$OVL_BASE_SCRATCH_MNT/data
> >> +l1=$OVL_BASE_SCRATCH_MNT/lower1
> >> +l2=$OVL_BASE_SCRATCH_MNT/lower2
> >> +l3=$OVL_BASE_SCRATCH_MNT/lower3
> >> +upper=$OVL_BASE_SCRATCH_MNT/upper
> >> +work=$OVL_BASE_SCRATCH_MNT/work
> >> +layers="$l3:$l2:$l1::$data"
> >> +
> >> +# Make a metacopy file <file> with the redirect <redirect>, for both
> >> +# userxattr and trusted xattr mounts
> >> +make_stub()
> >> +{
> >> +       local file=$1 redirect=$2 x
> >> +
> >> +       touch $file
> >> +       for x in user.overlay. ${OVL_XATTR_METACOPY%metacopy}; do
> >> +               $SETFATTR_PROG -n ${x}metacopy $file
> >> +               $SETFATTR_PROG -n ${x}redirect -v $redirect $file
> >> +       done
> >> +       truncate -s 4 $file
> >> +}
> >> +
> >> +# Mount the layers $layers with the options <opts>, print redirect_dir=followdata
> >> +# if the mount shows it and <opts> has userxattr or redirect_dir (otherwise it
> >> +# depends on the kernel config), and the content of each of the following files,
> >> +# or the error reading it
> >> +check()
> >> +{
> >> +       local opts=$1 f
> >> +
> >> +       shift
> >> +       echo "== $opts"
> >> +       _overlay_scratch_mount_dirs "$layers" $upper $work -o $opts
> >> +       case $opts in
> >> +       *userxattr*|*redirect_dir=*)
> >> +               _fs_options $OVL_BASE_SCRATCH_MNT | grep -o redirect_dir=followdata
> >> +               ;;
> >> +       esac
> >> +       for f; do
> >> +               echo "$f: $(cat $SCRATCH_MNT/$f 2>&1 | sed 's/^cat: .*: //')"
> >> +       done
> >> +       _unmount $SCRATCH_MNT
> >> +}
> >> +
> >> +# Mounting the layers $layers with the options <opts> must fail
> >> +check_fail()
> >> +{
> >> +       local opts=$1
> >> +
> >> +       echo "== $opts"
> >> +       _overlay_scratch_mount_dirs "$layers" $upper $work -o $opts \
> >> +               >> $seqres.full 2>&1 && echo "mount succeeded" && \
> >> +               _unmount $SCRATCH_MNT
> >> +}
> >> +
> >> +_scratch_mkfs
> >> +mkdir -p $data/base $l1/base $l2/base $l3 $upper $work
> >> +echo -n "data" > $data/base/file
> >> +
> >> +make_stub $l1/s1 /base/file
> >> +make_stub $l2/s2 /base/file
> >> +make_stub $l3/s3 /base/file
> >> +# The redirect target in a lower layer below s3, which is not looked up
> >> +echo -n "lowr" > $l2/base/file
> >> +# Redirects that are not followed: relative, directory, upper
> >> +echo -n "lowr" > $l2/reltarget
> >> +make_stub $l3/rel reltarget
> >> +mkdir $l3/dir
> >> +$SETFATTR_PROG -n user.overlay.redirect -v /base $l3/dir
> >> +$SETFATTR_PROG -n $OVL_XATTR_REDIRECT -v /base $l3/dir
> >> +make_stub $upper/up /base/file
> >> +
> >> +# followdata is selected by default with userxattr and a data-only layer
> >> +check userxattr s1 s2 s3
> >> +check userxattr,redirect_dir=followdata s1 s2 s3 rel dir up
> >> +# a data-only layer cannot be used without following redirects
> >> +check_fail userxattr,redirect_dir=nofollow
> >> +check_fail userxattr,redirect_dir=off
> >
> > The test looks good but this expectation of failure is problematic.
> >
> > First of all the test would fail with redirect_always_follow=N
> > because that makes redirect_dir=off => followdata
> >
> > But I also think that redirect_dir=off should not fail with
> > redirect_always_follow=Y.
> >
> > I will explain with the documentation.
> > You documentation v2 patch says:
> >
> > :A data-only layer selects "redirect_dir=followdata", unless
> > "redirect_dir=follow"
> > :or "redirect_dir=on" is in effect.
> >
> > So it implies redirect_dir=off => followdata
> >
> > All that redirect_dir=off really means is "don't create redirects".
> >
> > I amended the document as follows to clarify that:
> >
> > - "redirect_dir=on":
> >     Redirects are created and followed.
> > - "redirect_dir=follow":
> >     Redirects are not created, but followed.
> > - "redirect_dir=followdata":
> >     Redirects are not created.
> >     Only redirects from lower to lowerdata are followed.
> > - "redirect_dir=nofollow":
> >     Redirects are not created and not followed.
> > - "redirect_dir=off":
> >     Redirects are not created.
> >     Which redirects are followed depends on "redirect_always_follow" in the
> >     kernel/module config, on data-only lower layers and on the "userxattr"
> >     mount option.
> >
> > Made this off => followdata change in the code and pushed to ovl-fixes:
> >
> > -static int ovl_redirect_follow_mode(size_t nr_data)
> > +int ovl_redirect_follow_mode(bool follow_lower, size_t nr_data)
> >  {
> > -       return ovl_redirect_always_follow ? OVL_REDIRECT_FOLLOW :
> > -                                           ovl_redirect_nofollow_mode(nr_data);
> > +       if (ovl_redirect_always_follow && follow_lower)
> > +               return OVL_REDIRECT_FOLLOW;
> > +
> > +       /* Data-only layers cannot be used without following redirects
> > to them */
> > +       return nr_data ? OVL_REDIRECT_FOLLOWDATA : OVL_REDIRECT_NOFOLLOW;
> >  }
> >
> >  /*
> > @@ -137,7 +140,7 @@ static int ovl_redirect_follow_mode(size_t nr_data)
> >  static int ovl_redirect_mode_def(void)
> >  {
> >         return ovl_redirect_dir_def ? OVL_REDIRECT_ON :
> > -                                     ovl_redirect_follow_mode(0);
> > +                                     ovl_redirect_follow_mode(true, 0);
> >  }
> >
> >  static const struct constant_table ovl_parameter_verity[] = {
> > @@ -896,6 +899,9 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
> >         struct ovl_opt_set set = ctx->set;
> >         /* The requested redirect_dir=, for reporting conflicts with it */
> >         int redirect_opt = config->redirect_mode;
> > +       /* The redirect follow mode for redirect_dir=off and for the default */
> > +       int follow_mode = ovl_redirect_follow_mode(!config->userxattr,
> > +                                                  ctx->nr_data);
> >
> >         /* Workdir/index are useless in non-upper mount */
> >         if (!config->upperdir) {
> > @@ -922,9 +928,12 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
> >                 config->uuid = OVL_UUID_NULL;
> >         }
> >
> > -       /* Resolve redirect_dir=off to the default redirect follow mode */
> > +       /*
> > +        * Resolve redirect_dir=off to the default redirect follow mode.
> > +        * With userxattr, redirects are not followed to lower by default.
> > +        */
> >         if (config->redirect_mode == OVL_REDIRECT_OFF)
> > -               config->redirect_mode = ovl_redirect_follow_mode(ctx->nr_data);
> > +               config->redirect_mode = follow_mode;
> >
> >         /*
> >          * This is to make the logic below simpler.  It doesn't make any other
> > @@ -1038,7 +1047,7 @@ int ovl_fs_params_verify(const struct ovl_fs_context *ctx,
> >                  * userxattr.
> >                  */
> >                 if (!set.redirect)
> > -                       config->redirect_mode =
> > ovl_redirect_nofollow_mode(ctx->nr_data);
> > +                       config->redirect_mode = follow_mode;
> >                 config->metacopy = false;
> >         }
> >
> >
> > Please amend the test and test with the latest ovl-fixes:
> > d0d57557b76de ovl: follow lower to lowerdata redirects from any lower layer
>
> I've tested this modified version and it works fine.  Do you want me to
> send a v3 or will you amend it?

Please send v3 to Zoro/fstests because he will be applying it to fstests
after his review, not me.

Please keep my Reviewed-by on v3 I trust that you modified the test case
correctly and fixed the golden output accordingly.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-10 14:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-09 13:38 [PATCH fstests v2] overlay: add a test for redirect_dir=followdata Giuseppe Scrivano
2026-11-10 11:43 ` Amir Goldstein
2026-11-10 14:02   ` Giuseppe Scrivano
2026-11-10 14:27     ` Amir Goldstein

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox