Thread (28 messages) 28 messages, 4 authors, 2024-09-04

Re: [PATCH xfstests v2 2/2] open_by_handle: add tests for u64 mount ID

From: Amir Goldstein <amir73il@gmail.com>
Date: 2024-09-04 16:45:07
Also in: fstests, linux-fsdevel, linux-nfs, linux-perf-users, lkml

On Wed, Sep 4, 2024 at 6:31 PM Aleksa Sarai [off-list ref] wrote:
On 2024-09-03, Amir Goldstein [off-list ref] wrote:
quoted
On Tue, Sep 3, 2024 at 8:41 AM Aleksa Sarai [off-list ref] wrote:
quoted
On 2024-09-02, Amir Goldstein [off-list ref] wrote:
quoted
On Mon, Sep 2, 2024 at 6:46 PM Aleksa Sarai [off-list ref] wrote:
quoted
Now that open_by_handle_at(2) can return u64 mount IDs, do some tests to
make sure they match properly as part of the regular open_by_handle
tests.

Link: https://lore.kernel.org/all/20240828-exportfs-u64-mount-id-v3-0-10c2c4c16708@cyphar.com/ (local)
Signed-off-by: Aleksa Sarai <redacted>
---
v2:
- Remove -M argument and always do the mount ID tests. [Amir Goldstein]
- Do not error out if the kernel doesn't support STATX_MNT_ID_UNIQUE
  or AT_HANDLE_MNT_ID_UNIQUE. [Amir Goldstein]
- v1: <https://lore.kernel.org/all/20240828103706.2393267-1-cyphar@cyphar.com/ (local)>
Looks good.

You may add:

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

It'd be nice to get a verification that this is indeed tested on the latest
upstream and does not regress the tests that run the open_by_handle program.
I've tested that the fallback works on mainline and correctly does the
test on patched kernels (by running open_by_handle directly) but I
haven't run the suite yet (still getting my mkosi testing setup working
to run fstests...).
I am afraid this has to be tested.
I started testing myself and found that it breaks existing tests.
Even if you make the test completely opt-in as in v1 it need to be
tested and _notrun on old kernels.

If you have a new version, I can test it until you get your fstests setup
ready, because anyway I would want to check that your test also
works with overlayfs which has some specialized exportfs tests.
Test by running ./check -overlay -g exportfs, but I can also do that for you.
I managed to get fstests running, sorry about that...

For the v3 I have ready (which includes a new test using -M), the
following runs work in my VM:

 - ./check -g exportfs
 - ./check -overlay -g exportfs

Should I check anything else before sending it?
That should be enough.
So you have one new test that does not run on upstream kernel
and runs and passes on patched kernel?
quoted hunk ↗ jump to hunk
Also, when running the tests I think I may have found a bug? Using
overlayfs+xfs leads to the following error when doing ./check -overlay
if the scratch device is XFS:

  ./common/rc: line 299: _xfs_has_feature: command not found
    not run: upper fs needs to support d_type

The fix I applied was simply:
diff --git a/common/rc b/common/rc
index 0beaf2ff1126..e6af1b16918f 100644
--- a/common/rc
+++ b/common/rc
@@ -296,6 +296,7 @@ _supports_filetype()
        local fstyp=`$DF_PROG $dir | tail -1 | $AWK_PROG '{print $2}'`
        case "$fstyp" in
        xfs)
+               . common/xfs
                _xfs_has_feature $dir ftype
                ;;
        ext2|ext3|ext4)
Should I include this patch as well, or did I make a mistake somewhere?
(I could add the import to the top instead if you'd prefer that.)
This should already be handled by
if [ -n "$OVL_BASE_FSTYP" ];then
        _source_specific_fs $OVL_BASE_FSTYP
fi

in common/overlay

I think what you are missing is to
export FSTYP=xfs
as README.overlay suggests.

It's true that ./check does not *require* defining FSTYP
and can auto detect the test filesystem, but for running -overlay
is it a requirement to define the base FSTYP.

Thanks,
Amir.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help