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

[Zorro Lang <zlang@kernel.org>]

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.