| Message ID | 20240122072948.2568801-1-liwang@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/4] libswap: add known swap supported fs check | expand |
Hi Li, Cyril, > This introduce an enhancement to the library's is_swap_supported > function to check for filesystem compatibility before attempting > to create and enable a swap file. A list of supported filesystems > is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check > against this list is performed to ensure that the swap operations > are only attempted on known compatible filesystems. > If the make_swapfile function fails, the error handling is now > more descriptive: it distinguishes between failures due to the > filesystem not supporting swap files and other types of failures. > Similarly, when attempting to enable the swap file with swapon, > the patch ensures that clearer error messages are provided in > cases where the operation is not supported by the filesystem. > Signed-off-by: Li Wang <liwang@redhat.com> > --- > libs/libltpswap/libswap.c | 33 ++++++++++++++++++++++++++------- > 1 file changed, 26 insertions(+), 7 deletions(-) > diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c > index 13610709e..623f2fb3c 100644 > --- a/libs/libltpswap/libswap.c > +++ b/libs/libltpswap/libswap.c > @@ -12,6 +12,17 @@ > #include "libswap.h" > #include "lapi/syscalls.h" > +static const char *const swap_supported_fs[] = { > + "ext2", > + "ext3", > + "ext4", > + "xfs", > + "vfat", > + "exfat", > + "ntfs", > + NULL > +}; > + > /* > * Make a swap file > */ > @@ -40,23 +51,31 @@ int make_swapfile(const char *swapfile, int safe) > */ > void is_swap_supported(const char *filename) > { > + int i, sw_support = 0; > int fibmap = tst_fibmap(filename); Just a note unrelated to this patchset. When testing on SLES kernel based on 5.3.18 we still get TCONF due missing FIBMAP ioctl support: tst_test.c:1669: TINFO: === Testing on btrfs === tst_test.c:1118: TINFO: Formatting /dev/loop0 with btrfs opts='' extra opts='' tst_test.c:1132: TINFO: Mounting /dev/loop0 to /tmp/LTP_swazaqF1L/mntpoint fstyp=btrfs flags=0 tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22) libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01 libswap.c:114: TCONF: Swapfile on btrfs not implemented Am I wrong or could it be solved with FIEMAP (<linux/fiemap.h>)? If yes, I wonder if we should fallback on btrfs when FIBMAP is missing https://www.kernel.org/doc/Documentation/filesystems/fiemap.txt https://unix.stackexchange.com/questions/623859/how-do-you-find-the-physical-offset-for-a-file-in-btrfs Kind regards, Petr
On Mon, Jan 22, 2024 at 5:03 PM Petr Vorel <pvorel@suse.cz> wrote: > Hi Li, Cyril, > > > This introduce an enhancement to the library's is_swap_supported > > function to check for filesystem compatibility before attempting > > to create and enable a swap file. A list of supported filesystems > > is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check > > against this list is performed to ensure that the swap operations > > are only attempted on known compatible filesystems. > > > If the make_swapfile function fails, the error handling is now > > more descriptive: it distinguishes between failures due to the > > filesystem not supporting swap files and other types of failures. > > Similarly, when attempting to enable the swap file with swapon, > > the patch ensures that clearer error messages are provided in > > cases where the operation is not supported by the filesystem. > > > Signed-off-by: Li Wang <liwang@redhat.com> > > --- > > libs/libltpswap/libswap.c | 33 ++++++++++++++++++++++++++------- > > 1 file changed, 26 insertions(+), 7 deletions(-) > > > diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c > > index 13610709e..623f2fb3c 100644 > > --- a/libs/libltpswap/libswap.c > > +++ b/libs/libltpswap/libswap.c > > @@ -12,6 +12,17 @@ > > #include "libswap.h" > > #include "lapi/syscalls.h" > > > +static const char *const swap_supported_fs[] = { > > + "ext2", > > + "ext3", > > + "ext4", > > + "xfs", > > + "vfat", > > + "exfat", > > + "ntfs", > > + NULL > > +}; > > + > > /* > > * Make a swap file > > */ > > @@ -40,23 +51,31 @@ int make_swapfile(const char *swapfile, int safe) > > */ > > void is_swap_supported(const char *filename) > > { > > + int i, sw_support = 0; > > int fibmap = tst_fibmap(filename); > Just a note unrelated to this patchset. When testing on SLES kernel based > on > 5.3.18 we still get TCONF due missing FIBMAP ioctl support: > > tst_test.c:1669: TINFO: === Testing on btrfs === > tst_test.c:1118: TINFO: Formatting /dev/loop0 with btrfs opts='' extra > opts='' > tst_test.c:1132: TINFO: Mounting /dev/loop0 to /tmp/LTP_swazaqF1L/mntpoint > fstyp=btrfs flags=0 > tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22) > libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01 > > libswap.c:114: TCONF: Swapfile on btrfs not implemented > Interesting, can you try with the below command manually to see if swapfile is supported correctly on the BTRFS? "cut from man 5 btrfs" # truncate -s 0 swapfile # chattr +C swapfile # fallocate -l 2G swapfile # chmod 0600 swapfile # mkswap swapfile # swapon swapfile > > Am I wrong or could it be solved with FIEMAP (<linux/fiemap.h>)? > If yes, I wonder if we should fallback on btrfs when FIBMAP is missing > > https://www.kernel.org/doc/Documentation/filesystems/fiemap.txt > > https://unix.stackexchange.com/questions/623859/how-do-you-find-the-physical-offset-for-a-file-in-btrfs > > Kind regards, > Petr > >
Hi Li, > This introduce an enhancement to the library's is_swap_supported > function to check for filesystem compatibility before attempting > to create and enable a swap file. A list of supported filesystems > is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check > against this list is performed to ensure that the swap operations > are only attempted on known compatible filesystems. > If the make_swapfile function fails, the error handling is now > more descriptive: it distinguishes between failures due to the > filesystem not supporting swap files and other types of failures. > Similarly, when attempting to enable the swap file with swapon, > the patch ensures that clearer error messages are provided in > cases where the operation is not supported by the filesystem. +1 Reviewed-by: Petr Vorel <pvorel@suse.cz> formatting issues (tabs) are actually mostly from this commit, please fix them before merge. libswap.c:40: WARNING: Missing a blank line after declarations libswap.c:66: WARNING: please, no spaces at the start of a line libswap.c:67: WARNING: Missing a blank line after declarations libswap.c:67: WARNING: please, no spaces at the start of a line libswap.c:67: WARNING: suspect code indent for conditional statements (7, 15) libswap.c:68: ERROR: code indent should use tabs where possible libswap.c:68: WARNING: please, no spaces at the start of a line libswap.c:68: WARNING: suspect code indent for conditional statements (15, 23) libswap.c:69: ERROR: code indent should use tabs where possible libswap.c:69: WARNING: please, no spaces at the start of a line libswap.c:70: ERROR: code indent should use tabs where possible libswap.c:70: WARNING: please, no spaces at the start of a line libswap.c:70: WARNING: suspect code indent for conditional statements (15, 23) libswap.c:71: ERROR: code indent should use tabs where possible libswap.c:71: WARNING: please, no spaces at the start of a line libswap.c:72: WARNING: please, no spaces at the start of a line Kind regards, Petr
Hi Petr, On Mon, Jan 22, 2024 at 5:14 PM Petr Vorel <pvorel@suse.cz> wrote: > Hi Li, > > > This introduce an enhancement to the library's is_swap_supported > > function to check for filesystem compatibility before attempting > > to create and enable a swap file. A list of supported filesystems > > is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check > > against this list is performed to ensure that the swap operations > > are only attempted on known compatible filesystems. > > > If the make_swapfile function fails, the error handling is now > > more descriptive: it distinguishes between failures due to the > > filesystem not supporting swap files and other types of failures. > > Similarly, when attempting to enable the swap file with swapon, > > the patch ensures that clearer error messages are provided in > > cases where the operation is not supported by the filesystem. > > +1 > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > formatting issues (tabs) are actually mostly from this commit, please fix > them > before merge. > Thanks very much, Petr. The patchset V2 (based on your suggestions) is published in my git branch. In case you want to do more tests today. https://github.com/wangli5665/ltp/tree/libswap And, I prefer to wait for Cryil's feedback before posting them in ML:)
Hi Li, Cyril, > Hi Li, Cyril, ... > > void is_swap_supported(const char *filename) > > { > > + int i, sw_support = 0; > > int fibmap = tst_fibmap(filename); > Just a note unrelated to this patchset. When testing on SLES kernel based on > 5.3.18 we still get TCONF due missing FIBMAP ioctl support: > tst_test.c:1669: TINFO: === Testing on btrfs === > tst_test.c:1118: TINFO: Formatting /dev/loop0 with btrfs opts='' extra opts='' > tst_test.c:1132: TINFO: Mounting /dev/loop0 to /tmp/LTP_swazaqF1L/mntpoint fstyp=btrfs flags=0 > tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22) > libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01 > libswap.c:114: TCONF: Swapfile on btrfs not implemented Hm, what makes me wonder is that btrfs does not support FIBMAP even on current openSUSE Tumbleweed with 6.7.0 kernel: # TMPDIR=/var/tmp/ 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 /var/tmp/LTP_swaMBctpq/mntpoint fstyp=btrfs flags=0 tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22) libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01 libswap.c:114: TCONF: Swapfile on btrfs not implemented $ df -hT /var/tmp Filesystem Type Size Used Avail Use% Mounted on /dev/vda2 btrfs 28G 19G 3.3G 86% /var $ uname -r 6.7.0-9.gaedda80-default I thought the problem is that underlying fs is btrfs, but testing on Debian on 6.6.x with TMPDIR on ext4 does not bring FIBMAP support: # TMPDIR=/var/tmp LTP_SINGLE_FS_TYPE=btrfs /opt/ltp/testcases/bin/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 /var/tmp/LTP_swaZf8FN6/mntpoint fstyp=btrfs flags=0 tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22) libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01 libswap.c:114: TCONF: Swapfile on btrfs not implemented ... Kind regards, Petr
On Mon, Jan 22, 2024 at 5:41 PM Petr Vorel <pvorel@suse.cz> wrote: > Hi Li, Cyril, > > > Hi Li, Cyril, > ... > > > void is_swap_supported(const char *filename) > > > { > > > + int i, sw_support = 0; > > > int fibmap = tst_fibmap(filename); > > Just a note unrelated to this patchset. When testing on SLES kernel > based on > > 5.3.18 we still get TCONF due missing FIBMAP ioctl support: > > > tst_test.c:1669: TINFO: === Testing on btrfs === > > tst_test.c:1118: TINFO: Formatting /dev/loop0 with btrfs opts='' extra > opts='' > > tst_test.c:1132: TINFO: Mounting /dev/loop0 to > /tmp/LTP_swazaqF1L/mntpoint fstyp=btrfs flags=0 > > tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22) > > libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01 > > > libswap.c:114: TCONF: Swapfile on btrfs not implemented > > Hm, what makes me wonder is that btrfs does not support FIBMAP even on > current > openSUSE Tumbleweed with 6.7.0 kernel: > What we made use of FIBMAP is just to determine the file's blocks are contiguous on the disk then we can assume the swapfile support or not with that filesystem. As you mentioned, maybe FIEMAP is an additional way to check if a file's blocks are contiguous, I guess we might need to improve the check in a simple function with both FIEMAP and FIBMAP to guarantee we get the necessary info. And yes, it can be achieved in a separate patch. > > # TMPDIR=/var/tmp/ 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 > /var/tmp/LTP_swaMBctpq/mntpoint fstyp=btrfs flags=0 > tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22) > libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01 > > libswap.c:114: TCONF: Swapfile on btrfs not implemented > > $ df -hT /var/tmp > Filesystem Type Size Used Avail Use% Mounted on > /dev/vda2 btrfs 28G 19G 3.3G 86% /var > > $ uname -r > 6.7.0-9.gaedda80-default > > I thought the problem is that underlying fs is btrfs, but testing on Debian > on 6.6.x with TMPDIR on ext4 does not bring FIBMAP support: > > # TMPDIR=/var/tmp LTP_SINGLE_FS_TYPE=btrfs /opt/ltp/testcases/bin/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 > /var/tmp/LTP_swaZf8FN6/mntpoint fstyp=btrfs flags=0 > tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22) > libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01 > > libswap.c:114: TCONF: Swapfile on btrfs not implemented > > ... > > Kind regards, > Petr > >
> On Mon, Jan 22, 2024 at 5:03 PM Petr Vorel <pvorel@suse.cz> wrote: > > Hi Li, Cyril, > > > This introduce an enhancement to the library's is_swap_supported > > > function to check for filesystem compatibility before attempting > > > to create and enable a swap file. A list of supported filesystems > > > is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check > > > against this list is performed to ensure that the swap operations > > > are only attempted on known compatible filesystems. > > > If the make_swapfile function fails, the error handling is now > > > more descriptive: it distinguishes between failures due to the > > > filesystem not supporting swap files and other types of failures. > > > Similarly, when attempting to enable the swap file with swapon, > > > the patch ensures that clearer error messages are provided in > > > cases where the operation is not supported by the filesystem. > > > Signed-off-by: Li Wang <liwang@redhat.com> > > > --- > > > libs/libltpswap/libswap.c | 33 ++++++++++++++++++++++++++------- > > > 1 file changed, 26 insertions(+), 7 deletions(-) > > > diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c > > > index 13610709e..623f2fb3c 100644 > > > --- a/libs/libltpswap/libswap.c > > > +++ b/libs/libltpswap/libswap.c > > > @@ -12,6 +12,17 @@ > > > #include "libswap.h" > > > #include "lapi/syscalls.h" > > > +static const char *const swap_supported_fs[] = { > > > + "ext2", > > > + "ext3", > > > + "ext4", > > > + "xfs", > > > + "vfat", > > > + "exfat", > > > + "ntfs", > > > + NULL > > > +}; > > > + > > > /* > > > * Make a swap file > > > */ > > > @@ -40,23 +51,31 @@ int make_swapfile(const char *swapfile, int safe) > > > */ > > > void is_swap_supported(const char *filename) > > > { > > > + int i, sw_support = 0; > > > int fibmap = tst_fibmap(filename); > > Just a note unrelated to this patchset. When testing on SLES kernel based > > on > > 5.3.18 we still get TCONF due missing FIBMAP ioctl support: > > tst_test.c:1669: TINFO: === Testing on btrfs === > > tst_test.c:1118: TINFO: Formatting /dev/loop0 with btrfs opts='' extra > > opts='' > > tst_test.c:1132: TINFO: Mounting /dev/loop0 to /tmp/LTP_swazaqF1L/mntpoint > > fstyp=btrfs flags=0 > > tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22) > > libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01 > > libswap.c:114: TCONF: Swapfile on btrfs not implemented > Interesting, can you try with the below command manually to see if swapfile > is supported correctly on the BTRFS? > "cut from man 5 btrfs" > # truncate -s 0 swapfile > # chattr +C swapfile > # fallocate -l 2G swapfile > # chmod 0600 swapfile > # mkswap swapfile > # swapon swapfile It works (tested on openSUSE Tumbleweed with 6.6.11 which also reported FIBMAP ioctl is NOT supported), on file created with: dd if=/dev/zero of=swapfile bs=400M count=1 # swapon -s Filename Type Size Used Priority /dev/vda3 partition 2098152 34148 -2 /root/swapfile file 2097148 0 -3 # df -hT . Filesystem Type Size Used Avail Use% Mounted on /dev/vda2 btrfs 28G 21G 1.4G 95% /root Kind regards, Petr > > Am I wrong or could it be solved with FIEMAP (<linux/fiemap.h>)? > > If yes, I wonder if we should fallback on btrfs when FIBMAP is missing > > https://www.kernel.org/doc/Documentation/filesystems/fiemap.txt > > https://unix.stackexchange.com/questions/623859/how-do-you-find-the-physical-offset-for-a-file-in-btrfs > > Kind regards, > > Petr
> Hi Petr, > On Mon, Jan 22, 2024 at 5:14 PM Petr Vorel <pvorel@suse.cz> wrote: > > Hi Li, > > > This introduce an enhancement to the library's is_swap_supported > > > function to check for filesystem compatibility before attempting > > > to create and enable a swap file. A list of supported filesystems > > > is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check > > > against this list is performed to ensure that the swap operations > > > are only attempted on known compatible filesystems. > > > If the make_swapfile function fails, the error handling is now > > > more descriptive: it distinguishes between failures due to the > > > filesystem not supporting swap files and other types of failures. > > > Similarly, when attempting to enable the swap file with swapon, > > > the patch ensures that clearer error messages are provided in > > > cases where the operation is not supported by the filesystem. > > +1 > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > formatting issues (tabs) are actually mostly from this commit, please fix > > them > > before merge. > Thanks very much, Petr. > The patchset V2 (based on your suggestions) is published in my git branch. > In case you want to do more tests today. Thanks, I already scheduled tests, let you know later today. > 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
On Mon, Jan 22, 2024 at 7:03 PM Petr Vorel <pvorel@suse.cz> wrote: > > Hi Petr, > > > On Mon, Jan 22, 2024 at 5:14 PM Petr Vorel <pvorel@suse.cz> wrote: > > > > Hi Li, > > > > > This introduce an enhancement to the library's is_swap_supported > > > > function to check for filesystem compatibility before attempting > > > > to create and enable a swap file. A list of supported filesystems > > > > is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check > > > > against this list is performed to ensure that the swap operations > > > > are only attempted on known compatible filesystems. > > > > > If the make_swapfile function fails, the error handling is now > > > > more descriptive: it distinguishes between failures due to the > > > > filesystem not supporting swap files and other types of failures. > > > > Similarly, when attempting to enable the swap file with swapon, > > > > the patch ensures that clearer error messages are provided in > > > > cases where the operation is not supported by the filesystem. > > > > +1 > > > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > > > formatting issues (tabs) are actually mostly from this commit, please > fix > > > them > > > before merge. > > > > Thanks very much, Petr. > > > The patchset V2 (based on your suggestions) is published in my git > branch. > > In case you want to do more tests today. > > Thanks, I already scheduled tests, let you know later today. > 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 :). > > 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 > >
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) 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. 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
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 > >
On Tue, Jan 23, 2024 at 1:55 PM Li Wang <liwang@redhat.com> wrote: > 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. > V3 is here: https://github.com/wangli5665/ltp/commits/libswap-v3 It get passed on my fedora38(BTRFS), rhel9(XFS) platforms, I would post out in ML, just n case people can review the latest version. (it made quite a lot of changes vs the v1, so I don't want people to see the buggy version to fall into confusion :)
Hi Li, > V3 is here: https://github.com/wangli5665/ltp/commits/libswap-v3 I've just scheduled new runs for both Tumbleweed and all SLES versions (various kernel versions). > It get passed on my fedora38(BTRFS), rhel9(XFS) platforms, I would > post out in ML, just n case people can review the latest version. > (it made quite a lot of changes vs the v1, so I don't want people to see > the > buggy version to fall into confusion :) +1 Thanks! Kind regards, Petr
On Tue, Jan 23, 2024 at 3:48 PM Petr Vorel <pvorel@suse.cz> wrote: > Hi Li, > > > V3 is here: https://github.com/wangli5665/ltp/commits/libswap-v3 > > I've just scheduled new runs for both Tumbleweed and all SLES versions > (various kernel versions). > Thanks Petr, and sorry for posting to ML so later. I did some polishing work and fixed a few tiny issues based on above V3. I'm not sure if the libswap-v3 branch will test well (with tiny issues). So I pushed the latest to my main branch: https://github.com/wangli5665/ltp And, I deployed it with a general test (against RHEL) for the coming release work. Will share my test result with you and Cryil later.
> On Tue, Jan 23, 2024 at 3:48 PM Petr Vorel <pvorel@suse.cz> wrote: > > Hi Li, > > > V3 is here: https://github.com/wangli5665/ltp/commits/libswap-v3 > > I've just scheduled new runs for both Tumbleweed and all SLES versions > > (various kernel versions). > Thanks Petr, and sorry for posting to ML so later. > I did some polishing work and fixed a few tiny issues based on above V3. > I'm not sure if the libswap-v3 branch will test well (with tiny issues). > So I pushed the latest to my main branch: > https://github.com/wangli5665/ltp > And, I deployed it with a general test (against RHEL) for the coming > release work. Thanks Li! As I wrote to 4th patch [1] there will be v4 needed, but fix should be easy. Kind regards, Petr [1] https://lore.kernel.org/ltp/20240123121156.GA175806@pevik/ > Will share my test result with you and Cryil later.
On Tue, Jan 23, 2024 at 8:31 PM Petr Vorel <pvorel@suse.cz> wrote: > > On Tue, Jan 23, 2024 at 3:48 PM Petr Vorel <pvorel@suse.cz> wrote: > > > > Hi Li, > > > > > V3 is here: https://github.com/wangli5665/ltp/commits/libswap-v3 > > > > I've just scheduled new runs for both Tumbleweed and all SLES versions > > > (various kernel versions). > > > > Thanks Petr, and sorry for posting to ML so later. > > I did some polishing work and fixed a few tiny issues based on above V3. > > > I'm not sure if the libswap-v3 branch will test well (with tiny issues). > > So I pushed the latest to my main branch: > > https://github.com/wangli5665/ltp > > And, I deployed it with a general test (against RHEL) for the coming > > release work. > > Thanks Li! > As I wrote to 4th patch [1] there will be v4 needed, but fix should be > easy. > The fix is needed, but it does not belong to my libswap patch set. Without applying the five patches, swapoff still fails, right? (it originally caused by the tst_fill_file() but not other thing) It probably needs to be fixed in a separate patch in following up.
> On Tue, Jan 23, 2024 at 8:31 PM Petr Vorel <pvorel@suse.cz> wrote: > > > On Tue, Jan 23, 2024 at 3:48 PM Petr Vorel <pvorel@suse.cz> wrote: > > > > Hi Li, > > > > > V3 is here: https://github.com/wangli5665/ltp/commits/libswap-v3 > > > > I've just scheduled new runs for both Tumbleweed and all SLES versions > > > > (various kernel versions). > > > Thanks Petr, and sorry for posting to ML so later. > > > I did some polishing work and fixed a few tiny issues based on above V3. > > > I'm not sure if the libswap-v3 branch will test well (with tiny issues). > > > So I pushed the latest to my main branch: > > > https://github.com/wangli5665/ltp > > > And, I deployed it with a general test (against RHEL) for the coming > > > release work. > > Thanks Li! > > As I wrote to 4th patch [1] there will be v4 needed, but fix should be > > easy. > The fix is needed, but it does not belong to my libswap patch set. > Without applying the five patches, swapoff still fails, right? No, that was introduced by 4th patch which added support for btrfs (before it TCONF - swap not implemented). Kind regards, Petr > (it originally caused by the tst_fill_file() but not other thing) > It probably needs to be fixed in a separate patch in following up.
diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c index 13610709e..623f2fb3c 100644 --- a/libs/libltpswap/libswap.c +++ b/libs/libltpswap/libswap.c @@ -12,6 +12,17 @@ #include "libswap.h" #include "lapi/syscalls.h" +static const char *const swap_supported_fs[] = { + "ext2", + "ext3", + "ext4", + "xfs", + "vfat", + "exfat", + "ntfs", + NULL +}; + /* * Make a swap file */ @@ -40,23 +51,31 @@ int make_swapfile(const char *swapfile, int safe) */ void is_swap_supported(const char *filename) { + int i, sw_support = 0; int fibmap = tst_fibmap(filename); long fs_type = tst_fs_type(filename); const char *fstype = tst_fs_type_name(fs_type); - int ret = make_swapfile(filename, 1); - if (ret != 0) { - if (fibmap == 1) - tst_brk(TCONF, "mkswap on %s not supported", fstype); - else - tst_brk(TFAIL, "mkswap on %s failed", fstype); + for (i = 0; swap_supported_fs[i]; i++) { + if (!strcmp(fstype, swap_supported_fs[i])) { + sw_support = 1; + break; + } } + int ret = make_swapfile(filename, 1); + if (ret != 0) { + if (fibmap == 1 && sw_support == 0) + tst_brk(TCONF, "mkswap on %s not supported", fstype); + else + tst_brk(TFAIL, "mkswap on %s failed", fstype); + } + TEST(tst_syscall(__NR_swapon, filename, 0)); if (TST_RET == -1) { if (errno == EPERM) tst_brk(TCONF, "Permission denied for swapon()"); - else if (fibmap == 1 && errno == EINVAL) + else if (fibmap == 1 && errno == EINVAL && sw_support == 0) tst_brk(TCONF, "Swapfile on %s not implemented", fstype); else tst_brk(TFAIL | TTERRNO, "swapon() on %s failed", fstype);
This introduce an enhancement to the library's is_swap_supported function to check for filesystem compatibility before attempting to create and enable a swap file. A list of supported filesystems is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check against this list is performed to ensure that the swap operations are only attempted on known compatible filesystems. If the make_swapfile function fails, the error handling is now more descriptive: it distinguishes between failures due to the filesystem not supporting swap files and other types of failures. Similarly, when attempting to enable the swap file with swapon, the patch ensures that clearer error messages are provided in cases where the operation is not supported by the filesystem. Signed-off-by: Li Wang <liwang@redhat.com> --- libs/libltpswap/libswap.c | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-)