Re: [PATCH v2 1/2] shared: move btrfs clone device testcase to the shared group

David Sterba
2024-03-19T17:34:54Z

On Tue, Mar 19, 2024 at 12:16:33PM +0800, Zorro Lang wrote:
> On Mon, Mar 18, 2024 at 11:02:19PM +0100, David Sterba wrote:
> > On Sat, Mar 16, 2024 at 10:32:33PM +0530, Anand Jain wrote:
> > > Given that ext4 also allows mounting of a cloned filesystem, the btrfs
> > > test case btrfs/312, which assesses the functionality of cloned filesystem
> > > support, can be refactored to be under the shared group.
> > >
> > > Signed-off-by: Anand Jain <anand.jain@oracle.com>
> > > ---
> > > v2:
> > > Move to shared testcase instead of generic.
> >
> > What's the purpose of shared/ ? We have tests that make sense for a
> > subset of supported filesystems in generic/, with proper _required and
> > other the checks it works fine.
> >
> > I see that v1 did the move to generic/ but then the 'shared' got
> > suggested, which is IMHO the wrong direction. I remember some distant
> > past discussions about shared/ and what to put there. Right now there
> > are 3 remaining tests which I think is a good opportunity to make it 0.
>
> I didn't suggest to make it a shared case directly, I asked if there's a
> _require_xxxx helper to make this case notrun on "not proper" fs, not
> just use "btrfs ext4" to be whitelist :
>
> https://lore.kernel.org/fstests/20240312044629.hpaqdkl24nxaa3dv@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com/
>
> In my personal opinion, the "shared" directory is a place to store the cases
> which are nearly to be generic, but not ready. It's a place to remind us
> there're still some cases use something likes "supported btrfs ext4" as the
> hard condition of _notrun, rather than a flexible _require_xxx helper. These
> cases in shared better to be moved to generic, if we can improve it in one day.

Well, I disagree with that, we don't need to track the nearly-generic
state in a different way than we have right now with the _supported_fs
and ^exceptions.

> It more likes a "TODO" list of generic. If we just write it in generic/
> directory, I'm afraid we'll leave it in hundreds of generic cases then forget it.

A TODO for who? If I take the current state of shared/ as example, the
test 298, there's "_supported_fs ext4 xfs btrfs", this can live in
generic as it covers the current filesystems. If this does not apply to
NFS then it's IMHO fine to explicitly list the supported filesystems
rather than do long list of exceptions.

Support for btrfs to that test was added in 2019 in commit
0680ff2ea5313b3. If the state hasn't changed and is still in TODO then I
don't think this works (although back then the todo-status of the
shared/ was not defined as such).

> What do you think?

What I suggest:

- move everything from shared/ to generic
- document (as guidelines) what to do if there's a generic test that
applies only to subset of filesystems, what helpers to use and
possibly comment in the test cases why this canont be fully generic so
that a new filesystems to add can be evaluated as needed
- good example is shared/002, one could decide if eg. ext4 is missing
or not
- bad example is shared/032, there's xfs and btrfs but from the test
it looks like it's relevant for any local filesystem