Re: [LTP] [PATCH 1/1] swapon01: Test on all filesystems

[Petr Vorel <pvorel@suse.cz>]

Hi Li, Cyril,

> Hi Petr, Cyril,

> On Fri, Jan 19, 2024 at 10:43 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Test on all filesystems to increase coverage.
> > Skip filesystems which does not support swap (currently bcachefs, btrfs
> > and tmpfs).

> > Tested on 5.10, 6.6 and 6.7.

> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > Hi Li, Cyril,

> > feel free to postpone this after the release.
> > If we are for adding this, I'll retest on Monday older filesystems.

> > man swapon(8) mentions:

> >    Btrfs
> >        Swap files on Btrfs are supported since Linux 5.0 on files with
> > nocow
> >        attribute. See the btrfs(5) manual page for more details.

> > => Can we pass "nocow" just to btrfs? Or should it be added to single
> > test, which uses just btrfs with nocow?

> >    NFS
> >        Swap over NFS may not work.

> > => I guess this is not important for us, as NFS is not part of
> > filesystems in .all_filesystems, but maybe we should still add it to the
> > whitelist?

> > Kind regards,
> > Petr

> >  testcases/kernel/syscalls/swapon/swapon01.c | 21 +++++++++++++++------
> >  1 file changed, 15 insertions(+), 6 deletions(-)

> > diff --git a/testcases/kernel/syscalls/swapon/swapon01.c
> > b/testcases/kernel/syscalls/swapon/swapon01.c
> > index e59fb20a1..ef4ebfdcf 100644
> > --- a/testcases/kernel/syscalls/swapon/swapon01.c
> > +++ b/testcases/kernel/syscalls/swapon/swapon01.c
> > @@ -8,6 +8,7 @@
> >   * [Description]
> >   *
> >   * Checks that swapon() succeds with swapfile.
> > + * Testing on all filesystems which support swap file.
> >   */

> >  #include <unistd.h>
> > @@ -17,13 +18,14 @@
> >  #include "lapi/syscalls.h"
> >  #include "libswap.h"

> > -#define SWAP_FILE "swapfile01"
> > +#define MNTPOINT       "mntpoint"
> > +#define SWAP_FILE      MNTPOINT"/swapfile01"

> >  static void verify_swapon(void)
> >  {
> >         TST_EXP_PASS(tst_syscall(__NR_swapon, SWAP_FILE, 0));

> > -       if (TST_PASS && tst_syscall(__NR_swapoff, SWAP_FILE) != 0) {
> > +       if (tst_syscall(__NR_swapoff, SWAP_FILE) != 0) {
> >                 tst_brk(TBROK | TERRNO,
> >                                 "Failed to turn off swapfile, system
> > reboot recommended");
> >         }
> > @@ -31,13 +33,20 @@ static void verify_swapon(void)

> >  static void setup(void)
> >  {
> > -       is_swap_supported(SWAP_FILE);
> >         make_swapfile(SWAP_FILE, 0);
> >  }

> >  static struct tst_test test = {
> > -       .needs_root = 1,
> > -       .needs_tmpdir = 1,
> > +       .mntpoint = MNTPOINT,
> > +       .mount_device = 1,
> > +       .needs_root = 1, /* for swapon() */
> > +       .all_filesystems = 1,
> >         .test_all = verify_swapon,
> > -       .setup = setup
> > +       .setup = setup,
> > +       .skip_filesystems = (const char *[]) {
> > +               "bcachefs",
> > +               "btrfs",
> > +               "tmpfs",
> > +               NULL
> > +       },



> IMHO, I hate to add the skip_filesystem in this test, because if we do,
> then we have to add to all of the related swap tests that, it add more
> tedious work to people.

First I thought it'd be ok to have just single test which uses all_filesystems
(better one than none), but sure, having all of them would be better.

> So, if we decide to postpone this patch to release, we can just refactor
> the is_swap_supported() function to make things easier. I will send an RFC
> patch to show something in my mind, what we need to do later is just
> to finalize the swap_supported_fs[] struct list.

Sure, this is not critical, it can wait if you have better solution. Thanks
for spending time on it.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp