On 2024-09-04, Amir Goldstein [off-list ref] wrote:

On Wed, Sep 4, 2024 at 6:31 PM Aleksa Sarai [off-list ref] wrote:

quoted

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?

Yes. Both of those suite runs succeed without issues on v6.6.49,
v6.11-rc6 and with the AT_HANDLE_MNT_ID_UNIQUE patches.

I also added skipping logic such that it _should_ work on pre-5.8
kernels (pre-STATX_MNT_ID) as well, but I can't test that at the moment
(mkosi-kernel fails to boot kernels that old it seems...).

I'll send it now.

quoted

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.

Ah okay, I missed that. That fixed the issue, thanks!

Thanks,
Amir.

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>