diff mbox series

statvfs01: Convert to new LTP API

Message ID 20221124114204.990-1-akumar@suse.de
State Changes Requested
Headers show
Series statvfs01: Convert to new LTP API | expand

Commit Message

Avinesh Kumar Nov. 24, 2022, 11:42 a.m. UTC
Also I've removed the TINFO statements, I'm not sure if only
printing the data in logs is helpful in anyway.

Signed-off-by: Avinesh Kumar <akumar@suse.de>
---
 testcases/kernel/syscalls/statvfs/statvfs01.c | 96 +++++--------------
 1 file changed, 22 insertions(+), 74 deletions(-)

Comments

Li Wang Nov. 25, 2022, 2:18 a.m. UTC | #1
On Thu, Nov 24, 2022 at 7:42 PM Avinesh Kumar <akumar@suse.de> wrote:

> Also I've removed the TINFO statements, I'm not sure if only
> printing the data in logs is helpful in anyway.
>

Removing the printing is OK, but I am just wondering that
can we find a way to check if the returned value in 'buf' is
indeed correct?

As you can see the ‘struct statvfs‘ includes many fs key
values that we need to trust when using them.

struct statvfs {
        unsigned long  f_bsize;    /* Filesystem block size */
        unsigned long  f_frsize;   /* Fragment size */
        fsblkcnt_t     f_blocks;   /* Size of fs in f_frsize units */
        fsblkcnt_t     f_bfree;    /* Number of free blocks */
        fsblkcnt_t     f_bavail;   /* Number of free blocks for
                                             unprivileged users */
        fsfilcnt_t     f_files;    /* Number of inodes */
        fsfilcnt_t     f_ffree;    /* Number of free inodes */
        fsfilcnt_t     f_favail;   /* Number of free inodes for
                                             unprivileged users */
        unsigned long  f_fsid;     /* Filesystem ID */
        unsigned long  f_flag;     /* Mount flags */
        unsigned long  f_namemax;  /* Maximum filename length */
 };


>
> Signed-off-by: Avinesh Kumar <akumar@suse.de>
> ---
>  testcases/kernel/syscalls/statvfs/statvfs01.c | 96 +++++--------------
>  1 file changed, 22 insertions(+), 74 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/statvfs/statvfs01.c
> b/testcases/kernel/syscalls/statvfs/statvfs01.c
> index e3b356c93..89ca4e960 100644
> --- a/testcases/kernel/syscalls/statvfs/statvfs01.c
> +++ b/testcases/kernel/syscalls/statvfs/statvfs01.c
> @@ -1,92 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0
>  /*
>   * Copyright (c) Wipro Technologies Ltd, 2005.  All Rights Reserved.
>   *    AUTHOR: Prashant P Yendigeri <prashant.yendigeri@wipro.com>
> - *
> - * This program is free software; you can redistribute it and/or modify it
> - * under the terms of version 2 of the GNU General Public License as
> - * published by the Free Software Foundation.
> - *
> - * This program is distributed in the hope that it would be useful, but
> - * WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> - *
> - * You should have received a copy of the GNU General Public License along
> - * with this program; if not, write the Free Software Foundation, Inc.,
> - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
> - *
> + * Copyright (c) 2022 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com>
>   */
> -/*
> - *    DESCRIPTION
> - *      This is a Phase I test for the statvfs(2) system call.
> - *      It is intended to provide a limited exposure of the system call.
> - *     This call behaves similar to statfs.
> +
> +/*\
> + * [Description]
> + *
> + * Verify that statvfs() executes successfully for all
> + * available filesystems.
>   */
>
> -#include <stdio.h>
> -#include <unistd.h>
> -#include <errno.h>
>  #include <sys/statvfs.h>
> -#include <stdint.h>
> -
> -#include "test.h"
> -
> -#define TEST_PATH "/"
> +#include "tst_test.h"
>
> -static void setup(void);
> -static void cleanup(void);
> +#define MNT_POINT "mntpoint"
> +#define TEST_PATH MNT_POINT"/testfile"
>
> -char *TCID = "statvfs01";
> -int TST_TOTAL = 1;
> -
> -int main(int ac, char **av)
> +static void run(void)
>  {
>         struct statvfs buf;
> -       int lc;
> -
> -       tst_parse_opts(ac, av, NULL, NULL);
>
> -       setup();
> -
> -       for (lc = 0; TEST_LOOPING(lc); lc++) {
> -
> -               tst_count = 0;
> -
> -               TEST(statvfs(TEST_PATH, &buf));
> -
> -               if (TEST_RETURN == -1) {
> -                       tst_resm(TFAIL | TTERRNO, "statvfs(%s, ...)
> failed",
> -                                TEST_PATH);
> -               } else {
> -                       tst_resm(TPASS, "statvfs(%s, ...) passed",
> TEST_PATH);
> -               }
> -
> -       }
> -
> -       tst_resm(TINFO, "This call is similar to statfs");
> -       tst_resm(TINFO, "Extracting info about the '%s' file system",
> -                TEST_PATH);
> -       tst_resm(TINFO, "file system block size = %lu bytes", buf.f_bsize);
> -       tst_resm(TINFO, "file system fragment size = %lu bytes",
> buf.f_frsize);
> -       tst_resm(TINFO, "file system free blocks = %ju",
> -                (uintmax_t) buf.f_bfree);
> -       tst_resm(TINFO, "file system total inodes = %ju",
> -                (uintmax_t) buf.f_files);
> -       tst_resm(TINFO, "file system free inodes = %ju",
> -                (uintmax_t) buf.f_ffree);
> -       tst_resm(TINFO, "file system id = %lu", buf.f_fsid);
> -       tst_resm(TINFO, "file system max filename length = %lu",
> buf.f_namemax);
> -
> -       cleanup();
> -       tst_exit();
> +       TST_EXP_PASS(statvfs(TEST_PATH, &buf));
>  }
>
>  static void setup(void)
>  {
> -       tst_sig(NOFORK, DEF_HANDLER, cleanup);
> -
> -       TEST_PAUSE;
> +       SAFE_TOUCH(TEST_PATH, 0666, NULL);
>  }
>
> -static void cleanup(void)
> -{
> -}
> +static struct tst_test test = {
> +       .test_all = run,
> +       .setup = setup,
> +       .needs_root = 1,
> +       .mount_device = 1,
> +       .mntpoint = MNT_POINT,
> +       .all_filesystems = 1
> +};
> --
> 2.38.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>
Richard Palethorpe Nov. 29, 2022, 10:58 a.m. UTC | #2
Hello,

Li Wang <liwang@redhat.com> writes:

> On Thu, Nov 24, 2022 at 7:42 PM Avinesh Kumar <akumar@suse.de> wrote:
>
>  Also I've removed the TINFO statements, I'm not sure if only
>  printing the data in logs is helpful in anyway.
>
> Removing the printing is OK, but I am just wondering that
> can we find a way to check if the returned value in 'buf' is
> indeed correct?
>
> As you can see the ‘struct statvfs‘ includes many fs key
> values that we need to trust when using them.
>
> struct statvfs {
>         unsigned long  f_bsize;    /* Filesystem block size */
>         unsigned long  f_frsize;   /* Fragment size */
>         fsblkcnt_t     f_blocks;   /* Size of fs in f_frsize units */
>         fsblkcnt_t     f_bfree;    /* Number of free blocks */
>         fsblkcnt_t     f_bavail;   /* Number of free blocks for
>                                              unprivileged users */
>         fsfilcnt_t     f_files;    /* Number of inodes */
>         fsfilcnt_t     f_ffree;    /* Number of free inodes */
>         fsfilcnt_t     f_favail;   /* Number of free inodes for
>                                              unprivileged users */
>         unsigned long  f_fsid;     /* Filesystem ID */
>         unsigned long  f_flag;     /* Mount flags */
>         unsigned long  f_namemax;  /* Maximum filename length */
>  };

I suppose previously printing the values at least accessed the memory.
Actually validating the values could be a separate patch though.

We (probably) know that maximum filename should be less than 255 chars
(for e.g.), but I think there is a good chance that trying to validate
this will result in false positives and stuff we might want to revert.
Li Wang Nov. 30, 2022, 7:20 a.m. UTC | #3
On Tue, Nov 29, 2022 at 7:09 PM Richard Palethorpe <rpalethorpe@suse.de>
wrote:

> Hello,
>
> Li Wang <liwang@redhat.com> writes:
>
> > On Thu, Nov 24, 2022 at 7:42 PM Avinesh Kumar <akumar@suse.de> wrote:
> >
> >  Also I've removed the TINFO statements, I'm not sure if only
> >  printing the data in logs is helpful in anyway.
> >
> > Removing the printing is OK, but I am just wondering that
> > can we find a way to check if the returned value in 'buf' is
> > indeed correct?
> >
> > As you can see the ‘struct statvfs‘ includes many fs key
> > values that we need to trust when using them.
> >
> > struct statvfs {
> >         unsigned long  f_bsize;    /* Filesystem block size */
> >         unsigned long  f_frsize;   /* Fragment size */
> >         fsblkcnt_t     f_blocks;   /* Size of fs in f_frsize units */
> >         fsblkcnt_t     f_bfree;    /* Number of free blocks */
> >         fsblkcnt_t     f_bavail;   /* Number of free blocks for
> >                                              unprivileged users */
> >         fsfilcnt_t     f_files;    /* Number of inodes */
> >         fsfilcnt_t     f_ffree;    /* Number of free inodes */
> >         fsfilcnt_t     f_favail;   /* Number of free inodes for
> >                                              unprivileged users */
> >         unsigned long  f_fsid;     /* Filesystem ID */
> >         unsigned long  f_flag;     /* Mount flags */
> >         unsigned long  f_namemax;  /* Maximum filename length */
> >  };
>
> I suppose previously printing the values at least accessed the memory.
>

Sounds reasonable.



> Actually validating the values could be a separate patch though.
>

+1 Absolutely.



>
> We (probably) know that maximum filename should be less than 255 chars
> (for e.g.), but I think there is a good chance that trying to validate
> this will result in false positives and stuff we might want to revert.
>

Maybe we can create a concrete size of the device and mount
it with a designated FS (e.g. ext4), then extracting the known FS
data into `struct statvfs` and validating them.

Does this make sense?
Li Wang Dec. 1, 2022, 6 a.m. UTC | #4
Li Wang <liwang@redhat.com> wrote:


> We (probably) know that maximum filename should be less than 255 chars
>> (for e.g.), but I think there is a good chance that trying to validate
>> this will result in false positives and stuff we might want to revert.
>>
>
> Maybe we can create a concrete size of the device and mount
> it with a designated FS (e.g. ext4), then extracting the known FS
> data into `struct statvfs` and validating them.
>

As some of the data are determined by the FS attribute and
device size. (e.g. block size, total block, fragment size, and
total inodes). We can check them directly by known values.

For those easy change data, e.g. FS free blocks, which
determined by the real system status, maybe just check
that is no large than the total block number.

Based on that I draft something below to validate the fields.
what do you think?


#include <sys/statvfs.h>
#include "tst_test.h"

#define MNT_POINT "mntpoint"
#define TEST_PATH MNT_POINT"/testfile"

static void run(void)
{
        int type;
        struct statfs buf;
        struct statvfs vbuf;

        TST_EXP_PASS_SILENT(statfs(TEST_PATH, &buf));
        TST_EXP_PASS(statvfs(TEST_PATH, &vbuf));

        tst_res(TINFO, "Extracting FS info from the '%s' file", MNT_POINT);
        tst_res(TINFO, "vbuf.f_fsid == %lu", vbuf.f_fsid);

        long fs_type = tst_fs_type(TEST_PATH);

        switch (fs_type) {
        case TST_EXT2_OLD_MAGIC:
        case TST_EXT234_MAGIC:
                TST_EXP_EQ_LI(vbuf.f_bsize, 1024);
                TST_EXP_EQ_LI(vbuf.f_frsize, 1024);
                ttype = (vbuf.f_bfree <= buf.f_blocks) ? TPASS : TFAIL;
                tst_res(ttype, "vbuf.f_bfree == %ju", (uintmax_t)
vbuf.f_bfree);
                TST_EXP_EQ_LI(vbuf.f_files, 76912);
                ttype = (vbuf.f_ffree <= vbuf.f_files) ? TPASS : TFAIL;
                tst_res(ttype, "vbuf.f_ffree == %ju", (uintmax_t)
vbuf.f_ffree);
                TST_EXP_EQ_LI(vbuf.f_flag, 4096);
                TST_EXP_EQ_LI(vbuf.f_namemax, 255);
                break;
        case TST_XFS_MAGIC:
                TST_EXP_EQ_LI(vbuf.f_bsize, 4096);
                TST_EXP_EQ_LI(vbuf.f_frsize, 4096);
                ttype = (vbuf.f_bfree <= buf.f_blocks) ? TPASS : TFAIL;
                tst_res(ttype, "vbuf.f_bfree == %ju", (uintmax_t)
vbuf.f_bfree);
                TST_EXP_EQ_LI(vbuf.f_files, 153600);
                ttype = (vbuf.f_ffree <= vbuf.f_files) ? TPASS : TFAIL;
                tst_res(ttype, "vbuf.f_ffree == %ju", (uintmax_t)
vbuf.f_ffree);
                TST_EXP_EQ_LI(vbuf.f_flag, 4096);
                TST_EXP_EQ_LI(vbuf.f_namemax, 255);
                break;
        case TST_BTRFS_MAGIC:
                TST_EXP_EQ_LI(vbuf.f_bsize, 4096);
                TST_EXP_EQ_LI(vbuf.f_frsize, 4096);
                ttype = (vbuf.f_bfree <= buf.f_blocks) ? TPASS : TFAIL;
                tst_res(ttype, "vbuf.f_bfree == %ju", (uintmax_t)
vbuf.f_bfree);
                TST_EXP_EQ_LI(vbuf.f_files, 0);
                ttype = (vbuf.f_ffree <= vbuf.f_files) ? TPASS : TFAIL;
                tst_res(ttype, "vbuf.f_ffree == %ju", (uintmax_t)
vbuf.f_ffree);
                TST_EXP_EQ_LI(vbuf.f_flag, 4096);
                TST_EXP_EQ_LI(vbuf.f_namemax, 255);
                break;
        default:
                tst_res(TINFO, "vbuf.f_bsize == %lu bytes", vbuf.f_bsize);
                tst_res(TINFO, "vbuf.f_frsize == %lu bytes", vbuf.f_frsize);
                tst_res(TINFO, "vbuf.f_bfree == %ju", (uintmax_t)
vbuf.f_bfree);
                tst_res(TINFO, "vbuf.f_files == %ju", (uintmax_t)
vbuf.f_files);
                tst_res(TINFO, "vbuf.f_ffree == %ju", (uintmax_t)
vbuf.f_ffree);
                tst_res(TINFO, "vbuf.f_namemax == %lu", vbuf.f_namemax);
                break;
    }
}

static void setup(void)
{
        SAFE_TOUCH(TEST_PATH, 0666, NULL);
}

static struct tst_test test = {
        .test_all = run,
        .setup = setup,
        .needs_root = 1,
        .mount_device = 1,
        .mntpoint = MNT_POINT,
        .dev_min_size = 300,
        .all_filesystems = 1
};
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/statvfs/statvfs01.c b/testcases/kernel/syscalls/statvfs/statvfs01.c
index e3b356c93..89ca4e960 100644
--- a/testcases/kernel/syscalls/statvfs/statvfs01.c
+++ b/testcases/kernel/syscalls/statvfs/statvfs01.c
@@ -1,92 +1,40 @@ 
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (c) Wipro Technologies Ltd, 2005.  All Rights Reserved.
  *    AUTHOR: Prashant P Yendigeri <prashant.yendigeri@wipro.com>
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of version 2 of the GNU General Public License as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- *
+ * Copyright (c) 2022 SUSE LLC Avinesh Kumar <avinesh.kumar@suse.com>
  */
-/*
- *    DESCRIPTION
- *      This is a Phase I test for the statvfs(2) system call.
- *      It is intended to provide a limited exposure of the system call.
- *	This call behaves similar to statfs.
+
+/*\
+ * [Description]
+ *
+ * Verify that statvfs() executes successfully for all
+ * available filesystems.
  */
 
-#include <stdio.h>
-#include <unistd.h>
-#include <errno.h>
 #include <sys/statvfs.h>
-#include <stdint.h>
-
-#include "test.h"
-
-#define TEST_PATH "/"
+#include "tst_test.h"
 
-static void setup(void);
-static void cleanup(void);
+#define MNT_POINT "mntpoint"
+#define TEST_PATH MNT_POINT"/testfile"
 
-char *TCID = "statvfs01";
-int TST_TOTAL = 1;
-
-int main(int ac, char **av)
+static void run(void)
 {
 	struct statvfs buf;
-	int lc;
-
-	tst_parse_opts(ac, av, NULL, NULL);
 
-	setup();
-
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-
-		tst_count = 0;
-
-		TEST(statvfs(TEST_PATH, &buf));
-
-		if (TEST_RETURN == -1) {
-			tst_resm(TFAIL | TTERRNO, "statvfs(%s, ...) failed",
-				 TEST_PATH);
-		} else {
-			tst_resm(TPASS, "statvfs(%s, ...) passed", TEST_PATH);
-		}
-
-	}
-
-	tst_resm(TINFO, "This call is similar to statfs");
-	tst_resm(TINFO, "Extracting info about the '%s' file system",
-		 TEST_PATH);
-	tst_resm(TINFO, "file system block size = %lu bytes", buf.f_bsize);
-	tst_resm(TINFO, "file system fragment size = %lu bytes", buf.f_frsize);
-	tst_resm(TINFO, "file system free blocks = %ju",
-		 (uintmax_t) buf.f_bfree);
-	tst_resm(TINFO, "file system total inodes = %ju",
-		 (uintmax_t) buf.f_files);
-	tst_resm(TINFO, "file system free inodes = %ju",
-		 (uintmax_t) buf.f_ffree);
-	tst_resm(TINFO, "file system id = %lu", buf.f_fsid);
-	tst_resm(TINFO, "file system max filename length = %lu", buf.f_namemax);
-
-	cleanup();
-	tst_exit();
+	TST_EXP_PASS(statvfs(TEST_PATH, &buf));
 }
 
 static void setup(void)
 {
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
-
-	TEST_PAUSE;
+	SAFE_TOUCH(TEST_PATH, 0666, NULL);
 }
 
-static void cleanup(void)
-{
-}
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.needs_root = 1,
+	.mount_device = 1,
+	.mntpoint = MNT_POINT,
+	.all_filesystems = 1
+};