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

[Li Wang]

On Mon, Jan 22, 2024 at 2:27 PM Petr Vorel <pvorel@suse.cz> wrote:

> 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.
>

Yes, but it is ready there 1/4 :).
  https://lists.linux.it/pipermail/ltp/2024-January/036804.html

The rest work is just to finalize the swap_supported_fs list.
And, I agree that all of these can be postponed. Because
so far it won't be a critical/urgent-fix for a new release.


-- 
Regards,
Li Wang