[LTP] [PATCH v3 1/2] libswap: add two methods to create swapfile
Li Wang
liwang@redhat.com
Fri Mar 22 06:26:11 CET 2024
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
More information about the ltp
mailing list