[PATCH v2] overlay/081: add missing _require_scratch check

[PATCH v2] overlay/081: add missing _require_scratch check

[Prabhakar Pujeri]

The test runs _scratch_mkfs without first requiring a scratch device, so it fails messily instead of skipping when no scratch device is configured.  Add _require_scratch so the test skips cleanly when no scratch device is present.

Signed-off-by: Prabhakar Pujeri <prabhakar.pujeri@dell.com>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
---
Changes in v2:
- Use _require_scratch instead of _require_scratch_nocheck and drop the
  comment copied from overlay/079/080, which does not apply to this test.
  Add Amir's Reviewed-by.
v1: https://lore.kernel.org/fstests/20260831104154.2256-1-prabhakar.pujeri@dell.com/

 tests/overlay/081 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/overlay/081 b/tests/overlay/081
index 2270a04..e0ab558 100755
--- a/tests/overlay/081
+++ b/tests/overlay/081
@@ -14,6 +14,7 @@ _begin_fstest auto quick
 . ./common/filter
 . ./common/attr
 
+_require_scratch
 
 _scratch_mkfs >>$seqres.full 2>&1
 
-- 
2.55.0

Re: [PATCH v2] overlay/081: add missing _require_scratch check

[Zorro Lang]

On Tue, Sep 01, 2026 at 01:24:38PM +0000, Prabhakar Pujeri wrote:
> The test runs _scratch_mkfs without first requiring a scratch device, so it fails messily instead of skipping when no scratch device is configured.  Add _require_scratch so the test skips cleanly when no scratch device is present.
> 
> Signed-off-by: Prabhakar Pujeri <prabhakar.pujeri@dell.com>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> ---
> Changes in v2:
> - Use _require_scratch instead of _require_scratch_nocheck and drop the
>   comment copied from overlay/079/080, which does not apply to this test.
>   Add Amir's Reviewed-by.
> v1: https://lore.kernel.org/fstests/20260831104154.2256-1-prabhakar.pujeri@dell.com/
> 
>  tests/overlay/081 | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/overlay/081 b/tests/overlay/081
> index 2270a04..e0ab558 100755
> --- a/tests/overlay/081
> +++ b/tests/overlay/081
> @@ -14,6 +14,7 @@ _begin_fstest auto quick
>  . ./common/filter
>  . ./common/attr
>  
> +_require_scratch

As you're fixing this patch, I'm wondering if we need a _require_* helper to
make sure the mount options *uuid=null/auto/on* is supported by current system?
Due to the comment says "mount options uuid=null/auto/on introduced in kernel
v6.6" :)

>  
>  _scratch_mkfs >>$seqres.full 2>&1
>  
> -- 
> 2.55.0
> 

Re: [PATCH v2] overlay/081: add missing _require_scratch check

[Prabhakar Pujeri]

On Wed, Sep 02, 2026 at 01:51:08PM +0800, Zorro Lang wrote:
> As you're fixing this patch, I'm wondering if we need a _require_* helper to
> make sure the mount options *uuid=null/auto/on* is supported by current system?
> Due to the comment says "mount options uuid=null/auto/on introduced in kernel
> v6.6" :)

Thanks for looking.  That case is already covered a few lines below the
change, at the first use of the uuid= option (line 40):

	_overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o uuid=null \
		2>/dev/null || \
		_notrun "Overlayfs does not support unique fsid feature"

overlayfs rejects unknown mount options with EINVAL, so on kernels older
than v6.6 that probe mount fails and the test skips with the _notrun
reason instead of failing.  The uuid= options are also exercised only by
this test (it is the sole user of uuid=null/on/auto in the tree), so a
dedicated _require_* helper would have exactly one call site.  Happy to
factor it out anyway if you would rather have an explicit requirement.

Cheers,
Prabhakar

Re: [PATCH v2] overlay/081: add missing _require_scratch check

[Amir Goldstein]

On Wed, Sep 2, 2026 at 9:35 AM Prabhakar Pujeri
<prabhakar.pujeri@dell.com> wrote:
>
> On Wed, Sep 02, 2026 at 01:51:08PM +0800, Zorro Lang wrote:
> > As you're fixing this patch, I'm wondering if we need a _require_* helper to
> > make sure the mount options *uuid=null/auto/on* is supported by current system?
> > Due to the comment says "mount options uuid=null/auto/on introduced in kernel
> > v6.6" :)
>
> Thanks for looking.  That case is already covered a few lines below the
> change, at the first use of the uuid= option (line 40):
>
>         _overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o uuid=null \
>                 2>/dev/null || \
>                 _notrun "Overlayfs does not support unique fsid feature"
>
> overlayfs rejects unknown mount options with EINVAL, so on kernels older
> than v6.6 that probe mount fails and the test skips with the _notrun
> reason instead of failing.  The uuid= options are also exercised only by
> this test (it is the sole user of uuid=null/on/auto in the tree), so a
> dedicated _require_* helper would have exactly one call site.  Happy to
> factor it out anyway if you would rather have an explicit requirement.
>

*IF* we would want this, I would not recommend a dedicated require_
for uuid feature.

I would recommend teaching _require_scratch_overlay_features/
_check_overlay_feature to deal with mount options (e.g. uuid=on)
which do not have a module parameter to tune the default.

So if /sys/module/overlay/parameters/uuid does not exist, instead of
assuming it is not supported, assume that it defaults to off.

I am not saying this is worth the trouble, which is probably why I took the
easy lane and added the local _notrun condition adhoc in the test.

Overlayfs enum mount options that can be tested with the generic
features helper: redirect_dir, index, nfs_export, metacopy

Overlayfs enum mount options that cannot be tested with the
generic features helper because they do not have a corresponding
module param of same name:
uuid, xino, verity, fsync

verity - has a dedicated helper because it has special conditions.
fsync - test 087 has _require_scratch_shutdown_and_syncfs
            which is technically enough to test support for -o volatile
uuid - can benefit from generalization of features helper
xino - all these tests 041, 043, 044, 067, 070, 071 practically open
          code the suggested improvement of the generic helper

So for the uuid test alone it might not be worth it, but unless I am mistaken
for all the xino tests, this would be a nice cleanup/improvement.

But of course, this should not block this trivial and correct fix patch.

Thanks,
Amir.

Re: [PATCH v2] overlay/081: add missing _require_scratch check

[Zorro Lang]

On Wed, Sep 02, 2026 at 10:25:57AM +0200, Amir Goldstein wrote:
> On Wed, Sep 2, 2026 at 9:35 AM Prabhakar Pujeri
> <prabhakar.pujeri@dell.com> wrote:
> >
> > On Wed, Sep 02, 2026 at 01:51:08PM +0800, Zorro Lang wrote:
> > > As you're fixing this patch, I'm wondering if we need a _require_* helper to
> > > make sure the mount options *uuid=null/auto/on* is supported by current system?
> > > Due to the comment says "mount options uuid=null/auto/on introduced in kernel
> > > v6.6" :)
> >
> > Thanks for looking.  That case is already covered a few lines below the
> > change, at the first use of the uuid= option (line 40):
> >
> >         _overlay_scratch_mount_dirs $lowerdir $upperdir $workdir -o uuid=null \
> >                 2>/dev/null || \
> >                 _notrun "Overlayfs does not support unique fsid feature"
> >
> > overlayfs rejects unknown mount options with EINVAL, so on kernels older
> > than v6.6 that probe mount fails and the test skips with the _notrun
> > reason instead of failing.  The uuid= options are also exercised only by
> > this test (it is the sole user of uuid=null/on/auto in the tree), so a
> > dedicated _require_* helper would have exactly one call site.  Happy to
> > factor it out anyway if you would rather have an explicit requirement.
> >
> 
> *IF* we would want this, I would not recommend a dedicated require_
> for uuid feature.
> 
> I would recommend teaching _require_scratch_overlay_features/
> _check_overlay_feature to deal with mount options (e.g. uuid=on)
> which do not have a module parameter to tune the default.
> 
> So if /sys/module/overlay/parameters/uuid does not exist, instead of
> assuming it is not supported, assume that it defaults to off.
> 
> I am not saying this is worth the trouble, which is probably why I took the
> easy lane and added the local _notrun condition adhoc in the test.
> 
> Overlayfs enum mount options that can be tested with the generic
> features helper: redirect_dir, index, nfs_export, metacopy
> 
> Overlayfs enum mount options that cannot be tested with the
> generic features helper because they do not have a corresponding
> module param of same name:
> uuid, xino, verity, fsync
> 
> verity - has a dedicated helper because it has special conditions.
> fsync - test 087 has _require_scratch_shutdown_and_syncfs
>             which is technically enough to test support for -o volatile
> uuid - can benefit from generalization of features helper
> xino - all these tests 041, 043, 044, 067, 070, 071 practically open
>           code the suggested improvement of the generic helper
> 
> So for the uuid test alone it might not be worth it, but unless I am mistaken
> for all the xino tests, this would be a nice cleanup/improvement.
> 
> But of course, this should not block this trivial and correct fix patch.


Thanks Amir! Sure, I've already applied this patch locally. I just wanted
to use this opportunity to bring it up for discussion, as overlayfs is quite
a unique filesystem and its features have indeed been growing steadily.

I completely agree that having a unified feature check/require helper makes
sense. It would definitely make writing future test cases much cleaner and
simpler. Any patches in this direction are definitely more than welcome :)

Thanks,
Zorro

> 
> Thanks,
> Amir.