[LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Li Wang]

Hi Petr,

On Tue, Jan 23, 2024 at 4:23 AM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> > Great, and FYI.
>
> > I just pushed a new commit to resolve the FIBMAP (unsupported on BTRFS)
> > problem on my ltp:libswap branch.
>
> >
> https://github.com/wangli5665/ltp/commit/699711bfe8c8dbc3597c46587345fa1197c054c9
>
> > Hope you can try on your side and give some feedback :).
>
> Unfortunately regardless of kernel version it fails:
>
> # LTP_SINGLE_FS_TYPE=btrfs ./swapon01
> ...
> tst_test.c:1669: TINFO: === Testing on btrfs ===
> tst_test.c:1117: TINFO: Formatting /dev/loop0 with btrfs opts='' extra
> opts=''
> tst_test.c:1131: TINFO: Mounting /dev/loop0 to /tmp/LTP_swa497AKp/mntpoint
> fstyp=btrfs flags=0
> tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
> libswap.c:93: TINFO: File 'mntpoint/swapfile01' is not contiguous
> libswap.c:34: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01
> libswap.c:169: TFAIL: swapon() on btrfs failed: EINVAL (22)
>


Thanks for the quick feedback, very useful.

After looking over the code, I think it's because the tst_fill_file didn't
create a contiguous file at the beginning, so the solution is probably
to define a new function just used by creating a contiguous file for btrfs
in the libswap.c.

After a tiny amend, it works on a fedora38.

$ df -Th |grep btrfs
/dev/nvme0n1p3 btrfs     476G  112G  362G  24% /
/dev/nvme0n1p3 btrfs     476G  112G  362G  24% /home

$ sudo LTP_SINGLE_FS_TYPE=btrfs ./swapon01
tst_device.c:96: TINFO: Found free device 0 '/dev/loop0'
tst_test.c:1709: TINFO: LTP version: 20230929-292-g699711bfe
tst_test.c:1593: TINFO: Timeout per run is 0h 00m 30s
tst_supported_fs_types.c:161: TINFO: WARNING: testing only btrfs
tst_supported_fs_types.c:97: TINFO: Kernel supports btrfs
tst_supported_fs_types.c:62: TINFO: mkfs.btrfs does exist

tst_test.c:1669: TINFO: === Testing on btrfs ===
tst_test.c:1117: TINFO: Formatting /dev/loop0 with btrfs opts='' extra
opts=''
tst_test.c:1131: TINFO: Mounting /dev/loop0 to /tmp/LTP_swaMqVXj2/mntpoint
fstyp=btrfs flags=0
libswap.c:34: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:34: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01
swapon01.c:27: TPASS: tst_syscall(__NR_swapon, SWAP_FILE, 0) passed
swapon01.c:30: TINFO: SwapCached: 0 Kb

Summary:
passed   1
failed   0
broken   0
skipped  0
warnings 0





>
> Also, tst_brk() in is_swap_supported() causes test to fail quickly.
> Given similar Martin's fix to shell API I merged today, which use tst_res +
> return instead of tst_brk [1] I suggest to use tst_res + change return
> function
> from void to int and handle another return in the test. That will allow to
> test
> other filesystems (which is useful to see, if particular filesystem code is
> broken or or VFS or some common code reused by all filesystems). This is
> the
> reason why I don't think "quit as early as possible" is a good approach
> when
> testing on all filesystems.
>

Yes, good advice.


> Kind regards,
> Petr
>
> [1]
> https://github.com/linux-test-project/ltp/commit/5c73ad84f3e6db9a965df3ed4a846352136ed990
>
> > > > https://github.com/wangli5665/ltp/tree/libswap
>
> > > > And, I prefer to wait for Cryil's feedback before posting them in
> ML:)
>
> > > +1
>
> > > Kind regards,
> > > Petr
>
>

-- 
Regards,
Li Wang