Re: [PATCH] seek_sanity_test: Repair check for unwritten extent support

[Eryu Guan <guaneryu@gmail.com>]

On Wed, Aug 07, 2019 at 01:25:14PM +0200, Andreas Gruenbacher wrote:
> In test_basic_support, commit f3c1bca7fb25 ("generic: Test that
> SEEK_HOLE can find a punched hole") cleverly punched a hole in the test
> file in the middle of the check for unwritten extent support, making
> sure we would never detect when unwritten extent support is missing.
> Fix that.
> 
> While at it, explicitly check for SEEK_DATA support as well:  so far, we
> were assuming that SEEK_HOLE support implies SEEK_DATA support, but it
> won't hurt to actually check.
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>

Thanks for the fix! I missed the bug when reviewing f3c1bca7fb25, I
think that's because it's not so obvious that the fallocate and the
subsequent SEEK_DATA as a whole are for check for unwritten extent
support. So I did minor updates based on this patch to add more
comments, to make it clear what we're checking for at each stage.

Thanks,
Eryu

> ---
>  src/seek_sanity_test.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/src/seek_sanity_test.c b/src/seek_sanity_test.c
> index 30e996e2..080ca7eb 100644
> --- a/src/seek_sanity_test.c
> +++ b/src/seek_sanity_test.c
> @@ -1156,7 +1156,16 @@ static int test_basic_support(void)
>  	if (ret)
>  		goto out;
>  
> -	/* Is SEEK_DATA and SEEK_HOLE supported in the kernel? */
> +	/* Is SEEK_DATA supported in the kernel? */
> +	pos = lseek(fd, 0, SEEK_DATA);
> +	if (pos == -1) {
> +		fprintf(stderr, "Kernel does not support llseek(2) extension "
> +			"SEEK_DATA. Aborting.\n");
> +		ret = -1;
> +		goto out;
> +	}
> +
> +	/* Is SEEK_HOLE supported in the kernel? */
>  	pos = lseek(fd, 0, SEEK_HOLE);
>  	if (pos == -1) {
>  		fprintf(stderr, "Kernel does not support llseek(2) extension "
> @@ -1186,19 +1195,21 @@ static int test_basic_support(void)
>  			ret = -1;
>  		}
>  		goto out;
> -	} else if (fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
> -			     0, alloc_size) == -1) {
> -		fprintf(stderr, "File system does not support punch hole.\n");
> -	} else {
> -		punch_hole = 1;
>  	}
>  
>  	pos = lseek(fd, 0, SEEK_DATA);
>  	if (pos == 0) {
>  		fprintf(stderr, "File system does not support unwritten extents.\n");
> -		goto out;
> +	} else {
> +		unwritten_extents = 1;
> +	}
> +
> +	if (fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
> +			     0, alloc_size) == -1) {
> +		fprintf(stderr, "File system does not support punch hole.\n");
> +	} else {
> +		punch_hole = 1;
>  	}
> -	unwritten_extents = 1;
>  
>  	printf("\n");
>  
> -- 
> 2.20.1
>