Re: [LTP] [PATCH V4 5/6] libswap: Introduce file contiguity check

[Petr Vorel <pvorel@suse.cz>]

Hi Li,

> This patch introduces a new function file_is_contiguous to the
> libltpswap library to determine if a swap file is stored in a
> contiguous block of disk space, which is a typical requirement
> for swap files in Linux. The function performs a series of checks
> using the fiemap structure to assess the contiguity of the file
> and logs the result.

> It is integrated into the is_swap_supported function to replace
> the previous tst_fibmap check, providing a more reliable method
> for verifying that a file suitable for swap is indeed contiguous.

...
> +static int file_is_contiguous(const char *filename)
> +{
> +	int fd, contiguous = 0;
> +	struct fiemap *fiemap;
> +
> +	if (tst_fibmap(filename) == 0) {
> +		contiguous = 1;
> +		goto out;
> +	}
> +
> +	if (tst_fs_type(filename) == TST_TMPFS_MAGIC) {
> +		contiguous = 0;
nit: contiguous is already 0. Also I like that tst_fibmap is tested first (in
case tmpfs gets that support one day.
> +		goto out;
> +	}
> +
> +	fd = SAFE_OPEN(filename, O_RDONLY);
> +
> +	fiemap = (struct fiemap *)SAFE_MALLOC(sizeof(struct fiemap) + sizeof(struct fiemap_extent));
> +	memset(fiemap, 0, sizeof(struct fiemap) + sizeof(struct fiemap_extent));
> +
> +	fiemap->fm_start = 0;
> +	fiemap->fm_length = ~0;
> +	fiemap->fm_flags = 0;
> +	fiemap->fm_extent_count = 1;
> +
> +	SAFE_IOCTL(fd, FS_IOC_FIEMAP, fiemap);
> +
> +	/*
> +	 * fiemap->fm_mapped_extents != 1:
> +	 *   This checks if the file does not have exactly one extent. If there are more
> +	 *   or zero extents, the file is not stored in a single contiguous block.
> +	 *
> +	 * fiemap->fm_extents[0].fe_logical != 0:
> +	 *   This checks if the first extent does not start at the logical offset 0 of
> +	 *   the file. If it doesn't, it indicates that the file's first block of data
> +	 *   is not at the beginning of the file, which implies non-contiguity.
> +	 *
> +	 * (fiemap->fm_extents[0].fe_flags & FIEMAP_EXTENT_LAST) != FIEMAP_EXTENT_LAST:
> +	 *   This checks if the first extent does not have the FIEMAP_EXTENT_LAST flag set.
> +	 *   If the flag isn't set, it means that this extent is not the last one, suggesting
> +	 *   that there are more extents and the file is not contiguous.
> +	 */

Interesting, thanks for the doc!

LGTM.
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp