[LTP] [PATCH v3 1/2] libswap: add two methods to create swapfile

[Li Wang]

On Fri, Mar 22, 2024 at 1:00 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> ...
> > Signed-off-by: Li Wang <liwang@redhat.com>
> > Signed-off-by: Wei Gao <wegao@suse.com>
> > ---
> ...
> -int make_swapfile(const char *swapfile, int blocks, int safe)
> +int make_swapfile_(const char *file, const int lineno,
> +                       const char *swapfile, unsigned int num,
> +                       int safe, enum swapfile_method method)
>  {
>         struct statvfs fs_info;
> -       unsigned long blk_size, bs;
> +       unsigned long blk_size;
> +       unsigned int blocks = 0;
>         size_t pg_size = sysconf(_SC_PAGESIZE);
> -       char mnt_path[100];
> +       char mnt_path[128];
>
> nit: why this increase to 128? Why not PATH_MAX?
>


No special meaning, I just thought of using 2 raised to the nth power.
But you're right, PATCH_MAX will be safer.



>
> >       /* To guarantee at least one page can be swapped out */
> > -     if (blk_size * blocks < pg_size)
> > -             bs = pg_size;
> > -     else
> > -             bs = blk_size;
> > +     if (blk_size * blocks < pg_size) {
> > +             tst_res(TWARN, "Swapfile size is less than the system page
> size. \
> > +                     Using page size (%lu bytes) instead of block size
> (%lu bytes).",
>
> libswap.c:163: WARNING: Avoid line continuations in quoted strings
>
> This will fix it:
>
>                 tst_res(TWARN, "Swapfile size is less than the system page
> size. "
>                         "Using page size (%lu bytes) instead of block size
> (%lu bytes).",
>

+1


> > +                     (unsigned long)pg_size, blk_size);
> > +             blk_size = pg_size;
> > +     }
>
> >       if (sscanf(swapfile, "%[^/]", mnt_path) != 1)
> > -             tst_brk(TBROK, "sscanf failed");
> > +             tst_brk_(file, lineno, TBROK, "sscanf failed");
>
> > -     if (!tst_fs_has_free(mnt_path, bs * blocks, TST_BYTES))
> > -             tst_brk(TCONF, "Insufficient disk space to create swap
> file");
> > +     if (!tst_fs_has_free(mnt_path, blk_size * blocks, TST_BYTES))
> > +             tst_brk_(file, lineno, TCONF, "Insufficient disk space to
> create swap file");
>
> >       /* create file */
> > -     if (prealloc_contiguous_file(swapfile, bs, blocks) != 0)
> > -             tst_brk(TBROK, "Failed to create swapfile");
> > +     if (prealloc_contiguous_file(swapfile, blk_size, blocks) != 0)
> > +             tst_brk_(file, lineno, TBROK, "Failed to create swapfile");
>
> >       /* Fill the file if needed (specific to old xfs filesystems) */
> >       if (tst_fs_type(swapfile) == TST_XFS_MAGIC) {
> > -             if (tst_fill_file(swapfile, 0, bs, blocks) != 0)
> > -                     tst_brk(TBROK, "Failed to fill swapfile");
> > +             if (tst_fill_file(swapfile, 0, blk_size, blocks) != 0)
> > +                     tst_brk_(file, lineno, TBROK, "Failed to fill
> swapfile");
> >       }
>
> >       /* make the file swapfile */
> > -     const char *argv[2 + 1];
> > -
> > -     argv[0] = "mkswap";
> > -     argv[1] = swapfile;
> > -     argv[2] = NULL;
> > +     const char *argv[] = {"mkswap", swapfile, NULL};
>
> libswap.c:186: WARNING: char * array declaration might be better as static
> const
>
> This will fix it:
>
>         const char *const argv[] = {"mkswap", swapfile, NULL};
>

+1


> >       return tst_cmd(argv, "/dev/null", "/dev/null", safe ?
> > -                                TST_CMD_PASS_RETVAL |
> TST_CMD_TCONF_ON_MISSING : 0);
> > +                     TST_CMD_PASS_RETVAL | TST_CMD_TCONF_ON_MISSING :
> 0);
>
> The rest LGTM.
>


Thanks, I would give the patch set more time in case others have comments.



> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> Kind regards,
> Petr
>
>

-- 
Regards,
Li Wang