seek_sanity_test: Repair check for unwritten extent support
diff mbox series

Message ID 20190807112514.1903-1-agruenba@redhat.com
State New
Headers show
Series
  • seek_sanity_test: Repair check for unwritten extent support
Related show

Commit Message

Andreas Gruenbacher Aug. 7, 2019, 11:25 a.m. UTC
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>
---
 src/seek_sanity_test.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

Comments

Eryu Guan Aug. 11, 2019, 2:33 p.m. UTC | #1
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
>

Patch
diff mbox series

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");