All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/2] fiemap: Factor out actual fiemap call code
@ 2017-11-15 12:10 Nikolay Borisov
  2017-11-15 12:10 ` [PATCH v4 2/2] fiemap: Implement ranged query Nikolay Borisov
  2017-11-15 12:11 ` [PATCH v4 1/3] generic: Adjust generic test outputs for new fiemap imeplementation Nikolay Borisov
  0 siblings, 2 replies; 11+ messages in thread
From: Nikolay Borisov @ 2017-11-15 12:10 UTC (permalink / raw)
  To: linux-xfs; +Cc: fstests, eguan, david, darrick.wong, Nikolay Borisov

This will be needed to in a subsequent patch to avoid code duplication

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 io/fiemap.c | 39 ++++++++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/io/fiemap.c b/io/fiemap.c
index e6fd66da753d..08391f69d9bd 100644
--- a/io/fiemap.c
+++ b/io/fiemap.c
@@ -211,6 +211,31 @@ calc_print_format(
 	}
 }
 
+static int
+__fiemap(
+	 struct fiemap * fiemap,
+	 int		 mapsize,
+	 __u32		 flags,
+	 __u64		 start,
+	 __u64		 length) {
+
+	int ret;
+
+	memset(fiemap, 0, mapsize);
+	fiemap->fm_flags = flags;
+	fiemap->fm_start = start;
+	fiemap->fm_length = length;
+	fiemap->fm_extent_count = EXTENT_BATCH;
+	ret = ioctl(file->fd, FS_IOC_FIEMAP, fiemap);
+	if (ret < 0) {
+	        fprintf(stderr, "%s: ioctl(FS_IOC_FIEMAP) [\"%s\"]: "
+	                "%s\n", progname, file->name, strerror(errno));
+	        return ret;
+	}
+
+	return 0;
+}
+
 int
 fiemap_f(
 	int		argc,
@@ -266,19 +291,11 @@ fiemap_f(
 
 	while (!last && (cur_extent != max_extents)) {
 
-		memset(fiemap, 0, map_size);
-		fiemap->fm_flags = fiemap_flags;
-		fiemap->fm_start = last_logical;
-		fiemap->fm_length = -1LL;
-		fiemap->fm_extent_count = EXTENT_BATCH;
-
-		ret = ioctl(file->fd, FS_IOC_FIEMAP, (unsigned long)fiemap);
+		ret = __fiemap(fiemap, map_size, fiemap_flags, last_logical,
+			       -1LL);
 		if (ret < 0) {
-			fprintf(stderr, "%s: ioctl(FS_IOC_FIEMAP) [\"%s\"]: "
-				"%s\n", progname, file->name, strerror(errno));
-			free(fiemap);
 			exitcode = 1;
-			return 0;
+			goto out;
 		}
 
 		/* No more extents to map, exit */
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 2/2] fiemap: Implement ranged query
  2017-11-15 12:10 [PATCH v4 1/2] fiemap: Factor out actual fiemap call code Nikolay Borisov
@ 2017-11-15 12:10 ` Nikolay Borisov
  2017-11-17  2:47   ` Eric Sandeen
  2017-11-15 12:11 ` [PATCH v4 1/3] generic: Adjust generic test outputs for new fiemap imeplementation Nikolay Borisov
  1 sibling, 1 reply; 11+ messages in thread
From: Nikolay Borisov @ 2017-11-15 12:10 UTC (permalink / raw)
  To: linux-xfs; +Cc: fstests, eguan, david, darrick.wong, Nikolay Borisov

Currently the fiemap implementation of xfs_io doesn't support making ranged
queries. This patch implements two optional arguments which take the starting
offset and the length of the region to be queried. This also requires changing
of the final hole range is calculated. Namely, it's now being done as
[last_logical, logical addres of next extent] rather than being statically
determined by [last_logical, filesize].

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
V4: 
 * Don't do any fiemap processing if passed offset is past EOF. Filesystems
 might have custom handling for this. XFS for example pretends there is a 
 hole. 

 * Restore offset/len handling to using the optional params at the end of 
 getopt and not using an additional '-r' param

V3: 
 * Fixed a bug where incorrect extent index was shown if we didn't print a 
 hole. This was due to statically returning 2 at the end of print_(plain|verbose)
 Now, the actual number of printed extents inside the 2 functions is used. 
 This bug is visible only with the -r parameter

 * Fixed a bug where 1 additional extent would be printed. This was a result of 
 the aforementioned bug fix, since now printing function can return 1 and still
 have printed an extent and no hole. This can happen when you use -r parameter,
 this is now fixed and a comment explaining it is put. 

 * Reworked the handling of the new params by making them arguments to the 
 -r parameter. 

V2:
 * Incorporated Daricks feedback - removed variables which weren't introduced
  until the next patch as a result the build with this patch was broken. This is 
  fixed now
 io/fiemap.c       | 114 +++++++++++++++++++++++++++++++++++++++++++-----------
 man/man8/xfs_io.8 |   5 ++-
 2 files changed, 94 insertions(+), 25 deletions(-)

diff --git a/io/fiemap.c b/io/fiemap.c
index 08391f69d9bd..08203153b877 100644
--- a/io/fiemap.c
+++ b/io/fiemap.c
@@ -27,6 +27,9 @@
 
 static cmdinfo_t fiemap_cmd;
 static int max_extents = -1;
+static __u64 covered_length = 0;
+static __u64 len = -1LL;
+static bool range_limit = false;
 
 static void
 fiemap_help(void)
@@ -79,7 +82,7 @@ print_hole(
 		       boff_w, _("hole"), tot_w, lstart - llast);
 	   }
 
-
+	   covered_length += BBTOB(lstart - llast);
 }
 
 static int
@@ -90,7 +93,8 @@ print_verbose(
 	int			tot_w,
 	int			flg_w,
 	int			cur_extent,
-	__u64			last_logical)
+	__u64			last_logical,
+	__u64			limit)
 {
 	__u64			lstart;
 	__u64			llast;
@@ -99,6 +103,7 @@ print_verbose(
 	char			lbuf[48];
 	char			bbuf[48];
 	char			flgbuf[16];
+	int			num_printed = 0;
 
 	llast = BTOBBT(last_logical);
 	lstart = BTOBBT(extent->fe_logical);
@@ -120,10 +125,11 @@ print_verbose(
 		print_hole(foff_w, boff_w, tot_w, cur_extent, 0, false, llast,
 			   lstart);
 		cur_extent++;
+		num_printed = 1;
 	}
 
-	if (cur_extent == max_extents)
-		return 1;
+	if (cur_extent == max_extents || (range_limit && covered_length >= limit))
+		return num_printed;
 
 	snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]:", lstart,
 		 lstart + len - 1ULL);
@@ -132,7 +138,9 @@ print_verbose(
 	printf("%4d: %-*s %-*s %*llu %*s\n", cur_extent, foff_w, lbuf,
 	       boff_w, bbuf, tot_w, len, flg_w, flgbuf);
 
-	return 2;
+	num_printed++;
+
+	return num_printed;
 }
 
 static int
@@ -140,12 +148,14 @@ print_plain(
 	struct fiemap_extent	*extent,
 	int			lflag,
 	int			cur_extent,
-	__u64			last_logical)
+	__u64			last_logical,
+	__u64			limit)
 {
 	__u64			lstart;
 	__u64			llast;
 	__u64			block;
 	__u64			len;
+	int			num_printed = 0;
 
 	llast = BTOBBT(last_logical);
 	lstart = BTOBBT(extent->fe_logical);
@@ -155,20 +165,23 @@ print_plain(
 	if (lstart != llast) {
 		print_hole(0, 0, 0, cur_extent, lflag, true, llast, lstart);
 		cur_extent++;
+		num_printed = 1;
 	}
 
-	if (cur_extent == max_extents)
-		return 1;
+	if (cur_extent == max_extents || (range_limit && covered_length >= limit))
+		return num_printed;
 
 	printf("\t%d: [%llu..%llu]: %llu..%llu", cur_extent,
 	       lstart, lstart + len - 1ULL, block,
 	       block + len - 1ULL);
 
+	num_printed++;
+
 	if (lflag)
 		printf(_(" %llu blocks\n"), len);
 	else
 		printf("\n");
-	return 2;
+	return num_printed;
 }
 
 /*
@@ -256,8 +269,11 @@ fiemap_f(
 	int		tot_w = 5;	/* 5 since its just one number */
 	int		flg_w = 5;
 	__u64		last_logical = 0;
+	size_t		fsblocksize, fssectsize;
 	struct stat	st;
 
+	init_cvtnum(&fsblocksize, &fssectsize);
+
 	while ((c = getopt(argc, argv, "aln:v")) != EOF) {
 		switch (c) {
 		case 'a':
@@ -277,6 +293,34 @@ fiemap_f(
 		}
 	}
 
+	if (optind < argc) {
+		off64_t start_offset = cvtnum(fsblocksize, fssectsize, argv[optind]);
+		if (start_offset < 0) {
+			printf("non-numeric offset argument -- %s\n", argv[optind]);
+			return 0;
+		}
+		last_logical = start_offset;
+		optind++;
+	}
+
+	if (optind < argc) {
+		off64_t length = cvtnum(fsblocksize, fssectsize, argv[optind]);
+		if (length < 0) {
+			printf("non-numeric len argument -- %s\n", argv[optind]);
+			return 0;
+		}
+		len = length;
+		range_limit = true;
+	}
+
+	memset(&st, 0, sizeof(st));
+	if (fstat(file->fd, &st)) {
+		fprintf(stderr, "%s: fstat failed: %s\n", progname,
+			strerror(errno));
+		exitcode = 1;
+		return 0;
+	}
+
 	map_size = sizeof(struct fiemap) +
 		(EXTENT_BATCH * sizeof(struct fiemap_extent));
 	fiemap = malloc(map_size);
@@ -289,10 +333,13 @@ fiemap_f(
 
 	printf("%s:\n", file->name);
 
+	if (last_logical >= st.st_size)
+		goto out;
+
 	while (!last && (cur_extent != max_extents)) {
 
 		ret = __fiemap(fiemap, map_size, fiemap_flags, last_logical,
-			       -1LL);
+			       len - covered_length);
 		if (ret < 0) {
 			exitcode = 1;
 			goto out;
@@ -317,14 +364,23 @@ fiemap_f(
 				num_printed = print_verbose(extent, foff_w,
 							    boff_w, tot_w,
 							    flg_w, cur_extent,
-							    last_logical);
+							    last_logical,
+							    len);
 			} else
 				num_printed = print_plain(extent, lflag,
 							  cur_extent,
-							  last_logical);
+							  last_logical,
+							  len);
 
 			cur_extent += num_printed;
 			last_logical = extent->fe_logical + extent->fe_length;
+			/* If num_printed > 0 then we dunno if we have printed
+			 * a hole or an extent and a hole but we don't really
+			 * care. Termination of the loop is still going to be
+			 * correct
+			 */
+			if (num_printed)
+				covered_length += extent->fe_length;
 
 			if (extent->fe_flags & FIEMAP_EXTENT_LAST) {
 				last = 1;
@@ -333,24 +389,36 @@ fiemap_f(
 
 			if (cur_extent == max_extents)
 				break;
+
+			if (range_limit && covered_length >= len)
+				goto out;
 		}
 	}
 
 	if (cur_extent  == max_extents)
 		goto out;
 
-	memset(&st, 0, sizeof(st));
-	if (fstat(file->fd, &st)) {
-		fprintf(stderr, "%s: fstat failed: %s\n", progname,
-			strerror(errno));
-		free(fiemap);
-		exitcode = 1;
-		return 0;
-	}
 
-	if (cur_extent && last_logical < st.st_size)
+	if (last_logical < st.st_size &&
+	    (!range_limit || covered_length < len)) {
+		int end;
+
+		ret = __fiemap(fiemap, map_size, fiemap_flags, last_logical,
+			       st.st_size);
+		if (ret < 0) {
+			exitcode = 1;
+			goto out;
+		}
+
+		if (!fiemap->fm_mapped_extents)
+			end = BTOBBT(st.st_size);
+		else
+			end = BTOBBT(fiemap->fm_extents[0].fe_logical);
+
+
 		print_hole(foff_w, boff_w, tot_w, cur_extent, lflag, !vflag,
-			   BTOBBT(last_logical), BTOBBT(st.st_size));
+			   BTOBBT(last_logical), end);
+	}
 
 out:
 	free(fiemap);
@@ -365,7 +433,7 @@ fiemap_init(void)
 	fiemap_cmd.argmin = 0;
 	fiemap_cmd.argmax = -1;
 	fiemap_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
-	fiemap_cmd.args = _("[-alv] [-n nx]");
+	fiemap_cmd.args = _("[-alv] [-n nx] [offset [len]]");
 	fiemap_cmd.oneline = _("print block mapping for a file");
 	fiemap_cmd.help = fiemap_help;
 
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 0fd9b951199c..27f1ae163913 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -295,11 +295,12 @@ Prints the block mapping for the current open file. Refer to the
 .BR xfs_bmap (8)
 manual page for complete documentation.
 .TP
-.BI "fiemap [ \-alv ] [ \-n " nx " ]"
+.BI "fiemap [ \-alv ] [ \-n " nx " ] [ " offset " [ " len " ]]"
 Prints the block mapping for the current open file using the fiemap
 ioctl.  Options behave as described in the
 .BR xfs_bmap (8)
-manual page.
+manual page. Optionally, this command also supports passing the start offset
+from where to begin the fiemap and the length of that region.
 .TP
 .BI "fsmap [ \-d | \-l | \-r ] [ \-m | \-v ] [ \-n " nx " ] [ " start " ] [ " end " ]
 Prints the mapping of disk blocks used by the filesystem hosting the current
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 1/3] generic: Adjust generic test outputs for new fiemap imeplementation
  2017-11-15 12:10 [PATCH v4 1/2] fiemap: Factor out actual fiemap call code Nikolay Borisov
  2017-11-15 12:10 ` [PATCH v4 2/2] fiemap: Implement ranged query Nikolay Borisov
@ 2017-11-15 12:11 ` Nikolay Borisov
  2017-11-15 12:11   ` [PATCH v4 2/3] common: Implement fiemap's range query check Nikolay Borisov
                     ` (2 more replies)
  1 sibling, 3 replies; 11+ messages in thread
From: Nikolay Borisov @ 2017-11-15 12:11 UTC (permalink / raw)
  To: linux-xfs; +Cc: fstests, eguan, david, darrick.wong, Nikolay Borisov

With the new  range query support for the fiemap command,
the command also started printing hole extent for files which
consist of only a hole. So adjust generic test output accordingly.
Furthermore, this change breaks tests which are executed with a version
of xfs_io that doesn't support fiemap's range query. Fix this by implementing a
function which will fixup the output of tests which are broken by emulating
the output on older xfs_io versions

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 common/punch          | 23 +++++++++++++++++++++++
 tests/generic/012.out |  1 +
 tests/generic/016.out |  1 +
 tests/generic/021.out |  2 ++
 tests/generic/022.out |  2 ++
 tests/generic/058.out |  1 +
 tests/generic/060.out |  1 +
 tests/generic/061.out |  1 +
 tests/generic/063.out |  1 +
 tests/generic/255.out |  6 ++++++
 tests/generic/316.out |  6 ++++++
 11 files changed, 45 insertions(+)

diff --git a/common/punch b/common/punch
index c4ed261..9c39183 100644
--- a/common/punch
+++ b/common/punch
@@ -218,6 +218,27 @@ _filter_fiemap()
 	_coalesce_extents
 }
 
+#This function allows for tests which print the fiemap of a
+#file, that consists of only a hole to pass when executed with 
+#older versions of xfs_io's fiemap that didn't print anything for
+#such files
+_fiemap_range_fixup()
+{
+	#check if we support ranged query
+	$XFS_IO_PROG -c "help fiemap" | head -n 1 | grep -q "[offset [len]]"
+	local range_sup=$?
+	#check if the file consists of a single hole only
+	echo "$1" | grep -q "^0\: \[.*\]\: hole$"
+	local sole_hole=$?
+	local filesize="$(((`stat -c %s $1` / 512) - 1))"
+	local output_line_num=`$XFS_IO_PROG -c 'fiemap' $testfile | wc -l`
+
+	if [ $range_sup -eq 1 ] && [ $sole_hole -eq 1  ] && [ $output_line_num -eq 1 ]
+	then
+		echo "0: [0..$filesize]: hole"
+	fi
+}
+
 _filter_fiemap_flags()
 {
 	$AWK_PROG '
@@ -363,6 +384,7 @@ _test_generic_punch()
 	$XFS_IO_PROG -f -c "truncate $_20k" \
 		-c "$zero_cmd $_4k $_8k" \
 		-c "$map_cmd -v" $testfile | $filter_cmd
+	_fiemap_range_fixup $testfile
 	[ $? -ne 0 ] && die_now
 	_md5_checksum $testfile
 
@@ -470,6 +492,7 @@ _test_generic_punch()
 		-c "pwrite $_8k $_4k" $sync_cmd \
 		-c "$zero_cmd $_4k $_12k" \
 		-c "$map_cmd -v" $testfile | $filter_cmd
+	_fiemap_range_fixup $testfile
 	[ $? -ne 0 ] && die_now
 	_md5_checksum $testfile
 
diff --git a/tests/generic/012.out b/tests/generic/012.out
index ffbf8a3..8045471 100644
--- a/tests/generic/012.out
+++ b/tests/generic/012.out
@@ -1,5 +1,6 @@
 QA output created by 012
 	1. into a hole
+0: [0..95]: hole
 f4f35d60b3cc18aaa6d8d92f0cd3708a
 	2. into allocated space
 0: [0..95]: extent
diff --git a/tests/generic/016.out b/tests/generic/016.out
index c45a44a..1371ce7 100644
--- a/tests/generic/016.out
+++ b/tests/generic/016.out
@@ -1,5 +1,6 @@
 QA output created by 016
 	1. into a hole
+0: [0..95]: hole
 f4f35d60b3cc18aaa6d8d92f0cd3708a
 	2. into allocated space
 0: [0..95]: extent
diff --git a/tests/generic/021.out b/tests/generic/021.out
index 1137741..791b78a 100644
--- a/tests/generic/021.out
+++ b/tests/generic/021.out
@@ -1,5 +1,6 @@
 QA output created by 021
 	1. into a hole
+0: [0..95]: hole
 f4f35d60b3cc18aaa6d8d92f0cd3708a
 	2. into allocated space
 0: [0..95]: extent
@@ -34,6 +35,7 @@ f4f35d60b3cc18aaa6d8d92f0cd3708a
 1: [64..95]: hole
 d8f51c20223dbce5c7c90db87bc221b0
 	10. hole -> data -> hole
+0: [0..63]: hole
 bb7df04e1b0a2570657527a7e108ae23
 	11. data -> hole -> data
 0: [0..63]: extent
diff --git a/tests/generic/022.out b/tests/generic/022.out
index fbffa59..6dbc192 100644
--- a/tests/generic/022.out
+++ b/tests/generic/022.out
@@ -1,5 +1,6 @@
 QA output created by 022
 	1. into a hole
+0: [0..95]: hole
 f4f35d60b3cc18aaa6d8d92f0cd3708a
 	2. into allocated space
 0: [0..95]: extent
@@ -34,6 +35,7 @@ f4f35d60b3cc18aaa6d8d92f0cd3708a
 1: [64..95]: hole
 d8f51c20223dbce5c7c90db87bc221b0
 	10. hole -> data -> hole
+0: [0..63]: hole
 bb7df04e1b0a2570657527a7e108ae23
 	11. data -> hole -> data
 0: [0..63]: extent
diff --git a/tests/generic/058.out b/tests/generic/058.out
index b15308d..3bbc2a4 100644
--- a/tests/generic/058.out
+++ b/tests/generic/058.out
@@ -1,5 +1,6 @@
 QA output created by 058
 	1. into a hole
+0: [0..55]: hole
 cf845a781c107ec1346e849c9dd1b7e8
 	2. into allocated space
 0: [0..7]: extent
diff --git a/tests/generic/060.out b/tests/generic/060.out
index 909b578..210af74 100644
--- a/tests/generic/060.out
+++ b/tests/generic/060.out
@@ -1,5 +1,6 @@
 QA output created by 060
 	1. into a hole
+0: [0..55]: hole
 cf845a781c107ec1346e849c9dd1b7e8
 	2. into allocated space
 0: [0..7]: extent
diff --git a/tests/generic/061.out b/tests/generic/061.out
index 78d6c6d..6d95680 100644
--- a/tests/generic/061.out
+++ b/tests/generic/061.out
@@ -1,5 +1,6 @@
 QA output created by 061
 	1. into a hole
+0: [0..55]: hole
 cf845a781c107ec1346e849c9dd1b7e8
 	2. into allocated space
 0: [0..7]: extent
diff --git a/tests/generic/063.out b/tests/generic/063.out
index d828ff6..10db43f 100644
--- a/tests/generic/063.out
+++ b/tests/generic/063.out
@@ -1,5 +1,6 @@
 QA output created by 063
 	1. into a hole
+0: [0..55]: hole
 cf845a781c107ec1346e849c9dd1b7e8
 	2. into allocated space
 0: [0..7]: extent
diff --git a/tests/generic/255.out b/tests/generic/255.out
index 217ef3e..441fde8 100644
--- a/tests/generic/255.out
+++ b/tests/generic/255.out
@@ -1,5 +1,6 @@
 QA output created by 255
 	1. into a hole
+0: [0..39]: hole
 daa100df6e6711906b61c9ab5aa16032
 	2. into allocated space
 0: [0..7]: extent
@@ -42,6 +43,7 @@ daa100df6e6711906b61c9ab5aa16032
 3: [32..39]: hole
 cc63069677939f69a6e8f68cae6a6dac
 	10. hole -> data -> hole
+0: [0..39]: hole
 daa100df6e6711906b61c9ab5aa16032
 	11. data -> hole -> data
 0: [0..7]: extent
@@ -79,6 +81,7 @@ eecb7aa303d121835de05028751d301c
 0000400 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
 *
 	1. into a hole
+0: [0..39]: hole
 daa100df6e6711906b61c9ab5aa16032
 	2. into allocated space
 0: [0..7]: extent
@@ -121,6 +124,7 @@ daa100df6e6711906b61c9ab5aa16032
 3: [32..39]: hole
 cc63069677939f69a6e8f68cae6a6dac
 	10. hole -> data -> hole
+0: [0..39]: hole
 daa100df6e6711906b61c9ab5aa16032
 	11. data -> hole -> data
 0: [0..7]: extent
@@ -158,6 +162,7 @@ eecb7aa303d121835de05028751d301c
 0000400 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
 *
 	1. into a hole
+0: [0..39]: hole
 daa100df6e6711906b61c9ab5aa16032
 	2. into allocated space
 0: [0..7]: extent
@@ -240,6 +245,7 @@ eecb7aa303d121835de05028751d301c
 0000400 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
 *
 	1. into a hole
+0: [0..39]: hole
 daa100df6e6711906b61c9ab5aa16032
 	2. into allocated space
 0: [0..7]: extent
diff --git a/tests/generic/316.out b/tests/generic/316.out
index 383f0d1..5506198 100644
--- a/tests/generic/316.out
+++ b/tests/generic/316.out
@@ -1,5 +1,6 @@
 QA output created by 316
 	1. into a hole
+0: [0..39]: hole
 daa100df6e6711906b61c9ab5aa16032
 	2. into allocated space
 0: [0..7]: extent
@@ -16,6 +17,7 @@ cc63069677939f69a6e8f68cae6a6dac
 1: [8..39]: hole
 1b3779878366498b28c702ef88c4a773
 	10. hole -> data -> hole
+0: [0..39]: hole
 daa100df6e6711906b61c9ab5aa16032
 	11. data -> hole -> data
 0: [0..7]: extent
@@ -43,6 +45,7 @@ eecb7aa303d121835de05028751d301c
 0000400 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
 *
 	1. into a hole
+0: [0..39]: hole
 daa100df6e6711906b61c9ab5aa16032
 	2. into allocated space
 0: [0..7]: extent
@@ -59,6 +62,7 @@ cc63069677939f69a6e8f68cae6a6dac
 1: [8..39]: hole
 1b3779878366498b28c702ef88c4a773
 	10. hole -> data -> hole
+0: [0..39]: hole
 daa100df6e6711906b61c9ab5aa16032
 	11. data -> hole -> data
 0: [0..7]: extent
@@ -86,6 +90,7 @@ eecb7aa303d121835de05028751d301c
 0000400 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
 *
 	1. into a hole
+0: [0..39]: hole
 daa100df6e6711906b61c9ab5aa16032
 	2. into allocated space
 0: [0..7]: extent
@@ -133,6 +138,7 @@ eecb7aa303d121835de05028751d301c
 0000400 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
 *
 	1. into a hole
+0: [0..39]: hole
 daa100df6e6711906b61c9ab5aa16032
 	2. into allocated space
 0: [0..7]: extent
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 2/3] common: Implement fiemap's range query check
  2017-11-15 12:11 ` [PATCH v4 1/3] generic: Adjust generic test outputs for new fiemap imeplementation Nikolay Borisov
@ 2017-11-15 12:11   ` Nikolay Borisov
  2017-11-21  5:39     ` Eryu Guan
  2017-11-15 12:11   ` [PATCH v4 3/3] xfs: initial fiemap range query test Nikolay Borisov
  2017-11-21  5:29   ` [PATCH v4 1/3] generic: Adjust generic test outputs for new fiemap imeplementation Eryu Guan
  2 siblings, 1 reply; 11+ messages in thread
From: Nikolay Borisov @ 2017-11-15 12:11 UTC (permalink / raw)
  To: linux-xfs; +Cc: fstests, eguan, david, darrick.wong, Nikolay Borisov

Allow for users of xfstest to assert the presence of fiemap's
range query capabilities by requesting the "ranged" parameter.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 common/rc | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/common/rc b/common/rc
index e2a8229..c98c224 100644
--- a/common/rc
+++ b/common/rc
@@ -2053,6 +2053,12 @@ _require_xfs_io_command()
 			-c "$command 4k 8k" $testfile 2>&1`
 		;;
 	"fiemap")
+		if echo "$param" | grep -q "ranged"; then
+			param=$(echo "$param" | sed "s/ranged//")
+			$XFS_IO_PROG -F -f -c "pwrite 0 5k" -c "fsync" $testfile > /dev/null 2>&1
+			lines=$($XFS_IO_PROG -c "fiemap 6k" $testfile 2>&1 | wc -l)
+			[ $lines -eq 1 ] || _notrun "xfs_io $command ranged support is missing"
+		fi
 		testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
 			-c "fiemap -v $param" $testfile 2>&1`
 		param_checked=1
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v4 3/3] xfs: initial fiemap range query test
  2017-11-15 12:11 ` [PATCH v4 1/3] generic: Adjust generic test outputs for new fiemap imeplementation Nikolay Borisov
  2017-11-15 12:11   ` [PATCH v4 2/3] common: Implement fiemap's range query check Nikolay Borisov
@ 2017-11-15 12:11   ` Nikolay Borisov
  2017-11-21  5:45     ` Eryu Guan
  2017-11-21  5:29   ` [PATCH v4 1/3] generic: Adjust generic test outputs for new fiemap imeplementation Eryu Guan
  2 siblings, 1 reply; 11+ messages in thread
From: Nikolay Borisov @ 2017-11-15 12:11 UTC (permalink / raw)
  To: linux-xfs; +Cc: fstests, eguan, david, darrick.wong, Nikolay Borisov

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---

V4: 
 * Added test description
 * Added new test for past-eof behavior
 * Removed supported_generic_fs line
 * Switched to using the "ranged" param require
 * Revert v3 changes

V3:
 * Adjusted tests for '-r' fiemap param
 * Tests for invalid -r combination

V2: No change
V1: No change

 tests/xfs/900     |  92 +++++++++++++++++++++++++++++++++++++++++
 tests/xfs/900.out | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 211 insertions(+)
 create mode 100755 tests/xfs/900
 create mode 100644 tests/xfs/900.out

diff --git a/tests/xfs/900 b/tests/xfs/900
new file mode 100755
index 0000000..06a5c64
--- /dev/null
+++ b/tests/xfs/900
@@ -0,0 +1,92 @@
+#! /bin/bash
+# FS QA Test No. 900
+# Test for the new ranged query functionality in xfs_io's fiemap command.
+# This tests various combinations of hole + data layout being printed.
+# Also the test used 16k holes to be compatible with 16k block filesystems
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 SUSE Linux Products GmbH. All Rights Reserved.
+# Author: Nikolay Borisov <nborisov@suse.com>
+#
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/punch
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+
+# Modify as appropriate.
+_supported_os Linux
+_require_scratch
+_require_xfs_io_command "falloc"
+_require_xfs_io_command "fiemap" "ranged"
+
+_scratch_mkfs > $seqres.full 2>&1
+_scratch_mount || _fail "mount failure"
+
+file=$SCRATCH_MNT/testfile
+$XFS_IO_PROG -f -c "falloc 0 1m" $file 
+for i in {0..31}; do $XFS_IO_PROG -c "fpunch $(($i*32))k 16k" $file; done
+
+# Query 1 data extent between 16k..16k range
+echo "Basic data extent"
+$XFS_IO_PROG -c "fiemap -v 16k 16k" $file | _filter_fiemap 
+
+# Query data and hole extent
+echo "Data + Hole"
+$XFS_IO_PROG -c "fiemap -v 16k 20k" $file | _filter_fiemap
+
+echo "Hole + Data + Hole"
+$XFS_IO_PROG -c "fiemap -v 32k 40k" $file | _filter_fiemap
+
+echo "Beginning with a hole"
+$XFS_IO_PROG -c "fiemap -v 0 3k" $file | _filter_fiemap
+
+# Query for 0..160k that's 40 extents, more than the EXTENT_BATCH
+echo "Query more than 32 extents"
+$XFS_IO_PROG -c "fiemap -v 0 640k" $file | _filter_fiemap
+
+echo "Larger query than file size"
+$XFS_IO_PROG -c "fiemap -v 0 2m" $file | _filter_fiemap
+
+#mapping past eof shouldn't print anything"
+$XFS_IO_PROG -c "fiemap -v 2m" $file | _filter_fiemap
+
+#check everything without the first hole
+$XFS_IO_PROG -c "fiemap -v 16k" $file | wc -l
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/900.out b/tests/xfs/900.out
new file mode 100644
index 0000000..6ecf1b6
--- /dev/null
+++ b/tests/xfs/900.out
@@ -0,0 +1,119 @@
+QA output created by 900
+Basic data extent
+0: [32..63]: unwritten
+Data + Hole
+0: [32..63]: unwritten
+1: [64..95]: hole
+Hole + Data + Hole
+0: [64..95]: hole
+1: [96..127]: unwritten
+2: [128..159]: hole
+Beginning with a hole
+0: [0..31]: hole
+Query more than 32 extents
+0: [0..31]: hole
+1: [32..63]: unwritten
+2: [64..95]: hole
+3: [96..127]: unwritten
+4: [128..159]: hole
+5: [160..191]: unwritten
+6: [192..223]: hole
+7: [224..255]: unwritten
+8: [256..287]: hole
+9: [288..319]: unwritten
+10: [320..351]: hole
+11: [352..383]: unwritten
+12: [384..415]: hole
+13: [416..447]: unwritten
+14: [448..479]: hole
+15: [480..511]: unwritten
+16: [512..543]: hole
+17: [544..575]: unwritten
+18: [576..607]: hole
+19: [608..639]: unwritten
+20: [640..671]: hole
+21: [672..703]: unwritten
+22: [704..735]: hole
+23: [736..767]: unwritten
+24: [768..799]: hole
+25: [800..831]: unwritten
+26: [832..863]: hole
+27: [864..895]: unwritten
+28: [896..927]: hole
+29: [928..959]: unwritten
+30: [960..991]: hole
+31: [992..1023]: unwritten
+32: [1024..1055]: hole
+33: [1056..1087]: unwritten
+34: [1088..1119]: hole
+35: [1120..1151]: unwritten
+36: [1152..1183]: hole
+37: [1184..1215]: unwritten
+38: [1216..1247]: hole
+39: [1248..1279]: unwritten
+Larger query than file size
+0: [0..31]: hole
+1: [32..63]: unwritten
+2: [64..95]: hole
+3: [96..127]: unwritten
+4: [128..159]: hole
+5: [160..191]: unwritten
+6: [192..223]: hole
+7: [224..255]: unwritten
+8: [256..287]: hole
+9: [288..319]: unwritten
+10: [320..351]: hole
+11: [352..383]: unwritten
+12: [384..415]: hole
+13: [416..447]: unwritten
+14: [448..479]: hole
+15: [480..511]: unwritten
+16: [512..543]: hole
+17: [544..575]: unwritten
+18: [576..607]: hole
+19: [608..639]: unwritten
+20: [640..671]: hole
+21: [672..703]: unwritten
+22: [704..735]: hole
+23: [736..767]: unwritten
+24: [768..799]: hole
+25: [800..831]: unwritten
+26: [832..863]: hole
+27: [864..895]: unwritten
+28: [896..927]: hole
+29: [928..959]: unwritten
+30: [960..991]: hole
+31: [992..1023]: unwritten
+32: [1024..1055]: hole
+33: [1056..1087]: unwritten
+34: [1088..1119]: hole
+35: [1120..1151]: unwritten
+36: [1152..1183]: hole
+37: [1184..1215]: unwritten
+38: [1216..1247]: hole
+39: [1248..1279]: unwritten
+40: [1280..1311]: hole
+41: [1312..1343]: unwritten
+42: [1344..1375]: hole
+43: [1376..1407]: unwritten
+44: [1408..1439]: hole
+45: [1440..1471]: unwritten
+46: [1472..1503]: hole
+47: [1504..1535]: unwritten
+48: [1536..1567]: hole
+49: [1568..1599]: unwritten
+50: [1600..1631]: hole
+51: [1632..1663]: unwritten
+52: [1664..1695]: hole
+53: [1696..1727]: unwritten
+54: [1728..1759]: hole
+55: [1760..1791]: unwritten
+56: [1792..1823]: hole
+57: [1824..1855]: unwritten
+58: [1856..1887]: hole
+59: [1888..1919]: unwritten
+60: [1920..1951]: hole
+61: [1952..1983]: unwritten
+62: [1984..2015]: hole
+63: [2016..2047]: unwritten
+65
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/2] fiemap: Implement ranged query
  2017-11-15 12:10 ` [PATCH v4 2/2] fiemap: Implement ranged query Nikolay Borisov
@ 2017-11-17  2:47   ` Eric Sandeen
  2017-11-17  9:39     ` Nikolay Borisov
  0 siblings, 1 reply; 11+ messages in thread
From: Eric Sandeen @ 2017-11-17  2:47 UTC (permalink / raw)
  To: Nikolay Borisov, linux-xfs; +Cc: fstests, eguan, david, darrick.wong

On 11/15/17 6:10 AM, Nikolay Borisov wrote:
> Currently the fiemap implementation of xfs_io doesn't support making ranged
> queries. This patch implements two optional arguments which take the starting
> offset and the length of the region to be queried. This also requires changing
> of the final hole range is calculated. Namely, it's now being done as
> [last_logical, logical addres of next extent] rather than being statically
> determined by [last_logical, filesize].
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Ok, so first an apology for taking so long to really, really look at this.

When I started reviewing this, and dreading the complexity of the fiemap loop
control (why /is/ it so complicated today, anyway?) I started wondering: why
do we need a lot more logic?  Why can't we simply:

1) Set a non-zero mapping start
2) Set a non-maximal mapping length
3) Decrement that length as we map, and
4) When the kernel tells us mapped_extents for the range is 0, stop.

And this is what I ended up with, which seems a lot simpler.  Is there any
problem with this approach?

This /almost/ passes your test; what it doesn't do is show holes at the edges
of the mapping range, but I think that's ok.

The interface itself says nothing about holes, it only maps allocated ranges.

If we ask for a ranged query, I think it's appropriate to /not/ print holes
on either side of the requested range.


Thoughts?



diff --git a/io/fiemap.c b/io/fiemap.c
index bdcfacd..bbf6d63 100644
--- a/io/fiemap.c
+++ b/io/fiemap.c
@@ -49,6 +49,8 @@ fiemap_help(void)
 " -l -- also displays the length of each extent in 512-byte blocks.\n"
 " -n -- query n extents.\n"
 " -v -- Verbose information\n"
+" offset is the starting offset to map, and is optional.  If offset is\n"
+" specified, mapping length may (optionally) be specified as well."
 "\n"));
 }
 
@@ -235,9 +237,15 @@ fiemap_f(
 	int		boff_w = 16;
 	int		tot_w = 5;	/* 5 since its just one number */
 	int		flg_w = 5;
-	__u64		last_logical = 0;
+	__u64		last_logical = 0;	/* last extent offset handled */
+	off64_t		start_offset = 0;	/* mapping start */
+	off64_t		length = -1LL;		/* mapping length */
+	off64_t		range_end = -1LL;	/* mapping end*/
+	size_t		fsblocksize, fssectsize;
 	struct stat	st;
 
+	init_cvtnum(&fsblocksize, &fssectsize);
+
 	while ((c = getopt(argc, argv, "aln:v")) != EOF) {
 		switch (c) {
 		case 'a':
@@ -257,6 +265,27 @@ fiemap_f(
 		}
 	}
 
+	/* Range start (optional) */
+	if (optind < argc) {
+		start_offset = cvtnum(fsblocksize, fssectsize, argv[optind]);
+		if (start_offset < 0) {
+			printf("non-numeric offset argument -- %s\n", argv[optind]);
+			return 0;
+		}
+		last_logical = start_offset;
+		optind++;
+	}
+
+	/* Range length (optional if range start was specified) */
+	if (optind < argc) {
+		length = cvtnum(fsblocksize, fssectsize, argv[optind]);
+		if (length < 0) {
+			printf("non-numeric len argument -- %s\n", argv[optind]);
+			return 0;
+		}
+		range_end = start_offset + length;
+	}
+
 	map_size = sizeof(struct fiemap) +
 		(EXTENT_BATCH * sizeof(struct fiemap_extent));
 	fiemap = malloc(map_size);
@@ -274,7 +303,7 @@ fiemap_f(
 		memset(fiemap, 0, map_size);
 		fiemap->fm_flags = fiemap_flags;
 		fiemap->fm_start = last_logical;
-		fiemap->fm_length = -1LL;
+		fiemap->fm_length = range_end - last_logical;
 		fiemap->fm_extent_count = EXTENT_BATCH;
 
 		ret = ioctl(file->fd, FS_IOC_FIEMAP, (unsigned long)fiemap);
@@ -336,7 +365,8 @@ fiemap_f(
 		return 0;
 	}
 
-	if (cur_extent && last_logical < st.st_size)
+	/* Print last hole to EOF only if range end was not specified */
+	if (cur_extent && (range_end == -1LL) && (last_logical < st.st_size))
 		print_hole(foff_w, boff_w, tot_w, cur_extent, lflag, !vflag,
 			   BTOBBT(last_logical), BTOBBT(st.st_size));
 
@@ -353,7 +383,7 @@ fiemap_init(void)
 	fiemap_cmd.argmin = 0;
 	fiemap_cmd.argmax = -1;
 	fiemap_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
-	fiemap_cmd.args = _("[-alv] [-n nx]");
+	fiemap_cmd.args = _("[-alv] [-n nx] [offset [len]]");
 	fiemap_cmd.oneline = _("print block mapping for a file");
 	fiemap_cmd.help = fiemap_help;
 


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/2] fiemap: Implement ranged query
  2017-11-17  2:47   ` Eric Sandeen
@ 2017-11-17  9:39     ` Nikolay Borisov
  0 siblings, 0 replies; 11+ messages in thread
From: Nikolay Borisov @ 2017-11-17  9:39 UTC (permalink / raw)
  To: Eric Sandeen, linux-xfs; +Cc: fstests, eguan, david, darrick.wong



On 17.11.2017 04:47, Eric Sandeen wrote:
> On 11/15/17 6:10 AM, Nikolay Borisov wrote:
>> Currently the fiemap implementation of xfs_io doesn't support making ranged
>> queries. This patch implements two optional arguments which take the starting
>> offset and the length of the region to be queried. This also requires changing
>> of the final hole range is calculated. Namely, it's now being done as
>> [last_logical, logical addres of next extent] rather than being statically
>> determined by [last_logical, filesize].
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> 
> Ok, so first an apology for taking so long to really, really look at this.
> 
> When I started reviewing this, and dreading the complexity of the fiemap loop
> control (why /is/ it so complicated today, anyway?) I started wondering: why
> do we need a lot more logic?  Why can't we simply:
> 
> 1) Set a non-zero mapping start
> 2) Set a non-maximal mapping length
> 3) Decrement that length as we map, and
> 4) When the kernel tells us mapped_extents for the range is 0, stop.
> 
> And this is what I ended up with, which seems a lot simpler.  Is there any
> problem with this approach?
> 
> This /almost/ passes your test; what it doesn't do is show holes at the edges
> of the mapping range, but I think that's ok.
> 
> The interface itself says nothing about holes, it only maps allocated ranges.
> 
> If we ask for a ranged query, I think it's appropriate to /not/ print holes
> on either side of the requested range.
> 
> 
> Thoughts?

I definitely like the simplicity and am happy for this to replace my
patches. But with this do we require the fixup for existing hole tests
(I don't think so)?

> 
> 
> 
> diff --git a/io/fiemap.c b/io/fiemap.c
> index bdcfacd..bbf6d63 100644
> --- a/io/fiemap.c
> +++ b/io/fiemap.c
> @@ -49,6 +49,8 @@ fiemap_help(void)
>  " -l -- also displays the length of each extent in 512-byte blocks.\n"
>  " -n -- query n extents.\n"
>  " -v -- Verbose information\n"
> +" offset is the starting offset to map, and is optional.  If offset is\n"
> +" specified, mapping length may (optionally) be specified as well."
>  "\n"));
>  }
>  
> @@ -235,9 +237,15 @@ fiemap_f(
>  	int		boff_w = 16;
>  	int		tot_w = 5;	/* 5 since its just one number */
>  	int		flg_w = 5;
> -	__u64		last_logical = 0;
> +	__u64		last_logical = 0;	/* last extent offset handled */
> +	off64_t		start_offset = 0;	/* mapping start */
> +	off64_t		length = -1LL;		/* mapping length */
> +	off64_t		range_end = -1LL;	/* mapping end*/
> +	size_t		fsblocksize, fssectsize;
>  	struct stat	st;
>  
> +	init_cvtnum(&fsblocksize, &fssectsize);
> +
>  	while ((c = getopt(argc, argv, "aln:v")) != EOF) {
>  		switch (c) {
>  		case 'a':
> @@ -257,6 +265,27 @@ fiemap_f(
>  		}
>  	}
>  
> +	/* Range start (optional) */
> +	if (optind < argc) {
> +		start_offset = cvtnum(fsblocksize, fssectsize, argv[optind]);
> +		if (start_offset < 0) {
> +			printf("non-numeric offset argument -- %s\n", argv[optind]);
> +			return 0;
> +		}
> +		last_logical = start_offset;
> +		optind++;
> +	}
> +
> +	/* Range length (optional if range start was specified) */
> +	if (optind < argc) {
> +		length = cvtnum(fsblocksize, fssectsize, argv[optind]);
> +		if (length < 0) {
> +			printf("non-numeric len argument -- %s\n", argv[optind]);
> +			return 0;
> +		}
> +		range_end = start_offset + length;
> +	}
> +
>  	map_size = sizeof(struct fiemap) +
>  		(EXTENT_BATCH * sizeof(struct fiemap_extent));
>  	fiemap = malloc(map_size);
> @@ -274,7 +303,7 @@ fiemap_f(
>  		memset(fiemap, 0, map_size);
>  		fiemap->fm_flags = fiemap_flags;
>  		fiemap->fm_start = last_logical;
> -		fiemap->fm_length = -1LL;
> +		fiemap->fm_length = range_end - last_logical;
>  		fiemap->fm_extent_count = EXTENT_BATCH;
>  
>  		ret = ioctl(file->fd, FS_IOC_FIEMAP, (unsigned long)fiemap);
> @@ -336,7 +365,8 @@ fiemap_f(
>  		return 0;
>  	}
>  
> -	if (cur_extent && last_logical < st.st_size)
> +	/* Print last hole to EOF only if range end was not specified */
> +	if (cur_extent && (range_end == -1LL) && (last_logical < st.st_size))
>  		print_hole(foff_w, boff_w, tot_w, cur_extent, lflag, !vflag,
>  			   BTOBBT(last_logical), BTOBBT(st.st_size));
>  
> @@ -353,7 +383,7 @@ fiemap_init(void)
>  	fiemap_cmd.argmin = 0;
>  	fiemap_cmd.argmax = -1;
>  	fiemap_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
> -	fiemap_cmd.args = _("[-alv] [-n nx]");
> +	fiemap_cmd.args = _("[-alv] [-n nx] [offset [len]]");
>  	fiemap_cmd.oneline = _("print block mapping for a file");
>  	fiemap_cmd.help = fiemap_help;
>  
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 1/3] generic: Adjust generic test outputs for new fiemap imeplementation
  2017-11-15 12:11 ` [PATCH v4 1/3] generic: Adjust generic test outputs for new fiemap imeplementation Nikolay Borisov
  2017-11-15 12:11   ` [PATCH v4 2/3] common: Implement fiemap's range query check Nikolay Borisov
  2017-11-15 12:11   ` [PATCH v4 3/3] xfs: initial fiemap range query test Nikolay Borisov
@ 2017-11-21  5:29   ` Eryu Guan
  2 siblings, 0 replies; 11+ messages in thread
From: Eryu Guan @ 2017-11-21  5:29 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: linux-xfs, fstests, david, darrick.wong

On Wed, Nov 15, 2017 at 02:11:38PM +0200, Nikolay Borisov wrote:
> With the new  range query support for the fiemap command,
> the command also started printing hole extent for files which
> consist of only a hole. So adjust generic test output accordingly.
> Furthermore, this change breaks tests which are executed with a version
> of xfs_io that doesn't support fiemap's range query. Fix this by implementing a
> function which will fixup the output of tests which are broken by emulating
> the output on older xfs_io versions
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  common/punch          | 23 +++++++++++++++++++++++
>  tests/generic/012.out |  1 +
>  tests/generic/016.out |  1 +
>  tests/generic/021.out |  2 ++
>  tests/generic/022.out |  2 ++
>  tests/generic/058.out |  1 +
>  tests/generic/060.out |  1 +
>  tests/generic/061.out |  1 +
>  tests/generic/063.out |  1 +
>  tests/generic/255.out |  6 ++++++
>  tests/generic/316.out |  6 ++++++
>  11 files changed, 45 insertions(+)
> 
> diff --git a/common/punch b/common/punch
> index c4ed261..9c39183 100644
> --- a/common/punch
> +++ b/common/punch
> @@ -218,6 +218,27 @@ _filter_fiemap()
>  	_coalesce_extents
>  }
>  
> +#This function allows for tests which print the fiemap of a

Please add a space after "#" for comments.

> +#file, that consists of only a hole to pass when executed with 

Trailing whitespace in above line.

> +#older versions of xfs_io's fiemap that didn't print anything for
> +#such files
> +_fiemap_range_fixup()
> +{
> +	#check if we support ranged query
> +	$XFS_IO_PROG -c "help fiemap" | head -n 1 | grep -q "[offset [len]]"

regexp doesn't look correct, either use fgrep or escape "[".

> +	local range_sup=$?

I think we can return immediately if xfs_io supports ranged query, so we
don't have to do the "$range_sup -eq 1" check later, which results in a
shorter line.

Thanks,
Eryu

> +	#check if the file consists of a single hole only
> +	echo "$1" | grep -q "^0\: \[.*\]\: hole$"
> +	local sole_hole=$?
> +	local filesize="$(((`stat -c %s $1` / 512) - 1))"
> +	local output_line_num=`$XFS_IO_PROG -c 'fiemap' $testfile | wc -l`
> +
> +	if [ $range_sup -eq 1 ] && [ $sole_hole -eq 1  ] && [ $output_line_num -eq 1 ]
> +	then
> +		echo "0: [0..$filesize]: hole"
> +	fi
> +}
> +
>  _filter_fiemap_flags()
>  {
>  	$AWK_PROG '
> @@ -363,6 +384,7 @@ _test_generic_punch()
>  	$XFS_IO_PROG -f -c "truncate $_20k" \
>  		-c "$zero_cmd $_4k $_8k" \
>  		-c "$map_cmd -v" $testfile | $filter_cmd
> +	_fiemap_range_fixup $testfile
>  	[ $? -ne 0 ] && die_now
>  	_md5_checksum $testfile
>  
> @@ -470,6 +492,7 @@ _test_generic_punch()
>  		-c "pwrite $_8k $_4k" $sync_cmd \
>  		-c "$zero_cmd $_4k $_12k" \
>  		-c "$map_cmd -v" $testfile | $filter_cmd
> +	_fiemap_range_fixup $testfile
>  	[ $? -ne 0 ] && die_now
>  	_md5_checksum $testfile
>  
> diff --git a/tests/generic/012.out b/tests/generic/012.out
> index ffbf8a3..8045471 100644
> --- a/tests/generic/012.out
> +++ b/tests/generic/012.out
> @@ -1,5 +1,6 @@
>  QA output created by 012
>  	1. into a hole
> +0: [0..95]: hole
>  f4f35d60b3cc18aaa6d8d92f0cd3708a
>  	2. into allocated space
>  0: [0..95]: extent
> diff --git a/tests/generic/016.out b/tests/generic/016.out
> index c45a44a..1371ce7 100644
> --- a/tests/generic/016.out
> +++ b/tests/generic/016.out
> @@ -1,5 +1,6 @@
>  QA output created by 016
>  	1. into a hole
> +0: [0..95]: hole
>  f4f35d60b3cc18aaa6d8d92f0cd3708a
>  	2. into allocated space
>  0: [0..95]: extent
> diff --git a/tests/generic/021.out b/tests/generic/021.out
> index 1137741..791b78a 100644
> --- a/tests/generic/021.out
> +++ b/tests/generic/021.out
> @@ -1,5 +1,6 @@
>  QA output created by 021
>  	1. into a hole
> +0: [0..95]: hole
>  f4f35d60b3cc18aaa6d8d92f0cd3708a
>  	2. into allocated space
>  0: [0..95]: extent
> @@ -34,6 +35,7 @@ f4f35d60b3cc18aaa6d8d92f0cd3708a
>  1: [64..95]: hole
>  d8f51c20223dbce5c7c90db87bc221b0
>  	10. hole -> data -> hole
> +0: [0..63]: hole
>  bb7df04e1b0a2570657527a7e108ae23
>  	11. data -> hole -> data
>  0: [0..63]: extent
> diff --git a/tests/generic/022.out b/tests/generic/022.out
> index fbffa59..6dbc192 100644
> --- a/tests/generic/022.out
> +++ b/tests/generic/022.out
> @@ -1,5 +1,6 @@
>  QA output created by 022
>  	1. into a hole
> +0: [0..95]: hole
>  f4f35d60b3cc18aaa6d8d92f0cd3708a
>  	2. into allocated space
>  0: [0..95]: extent
> @@ -34,6 +35,7 @@ f4f35d60b3cc18aaa6d8d92f0cd3708a
>  1: [64..95]: hole
>  d8f51c20223dbce5c7c90db87bc221b0
>  	10. hole -> data -> hole
> +0: [0..63]: hole
>  bb7df04e1b0a2570657527a7e108ae23
>  	11. data -> hole -> data
>  0: [0..63]: extent
> diff --git a/tests/generic/058.out b/tests/generic/058.out
> index b15308d..3bbc2a4 100644
> --- a/tests/generic/058.out
> +++ b/tests/generic/058.out
> @@ -1,5 +1,6 @@
>  QA output created by 058
>  	1. into a hole
> +0: [0..55]: hole
>  cf845a781c107ec1346e849c9dd1b7e8
>  	2. into allocated space
>  0: [0..7]: extent
> diff --git a/tests/generic/060.out b/tests/generic/060.out
> index 909b578..210af74 100644
> --- a/tests/generic/060.out
> +++ b/tests/generic/060.out
> @@ -1,5 +1,6 @@
>  QA output created by 060
>  	1. into a hole
> +0: [0..55]: hole
>  cf845a781c107ec1346e849c9dd1b7e8
>  	2. into allocated space
>  0: [0..7]: extent
> diff --git a/tests/generic/061.out b/tests/generic/061.out
> index 78d6c6d..6d95680 100644
> --- a/tests/generic/061.out
> +++ b/tests/generic/061.out
> @@ -1,5 +1,6 @@
>  QA output created by 061
>  	1. into a hole
> +0: [0..55]: hole
>  cf845a781c107ec1346e849c9dd1b7e8
>  	2. into allocated space
>  0: [0..7]: extent
> diff --git a/tests/generic/063.out b/tests/generic/063.out
> index d828ff6..10db43f 100644
> --- a/tests/generic/063.out
> +++ b/tests/generic/063.out
> @@ -1,5 +1,6 @@
>  QA output created by 063
>  	1. into a hole
> +0: [0..55]: hole
>  cf845a781c107ec1346e849c9dd1b7e8
>  	2. into allocated space
>  0: [0..7]: extent
> diff --git a/tests/generic/255.out b/tests/generic/255.out
> index 217ef3e..441fde8 100644
> --- a/tests/generic/255.out
> +++ b/tests/generic/255.out
> @@ -1,5 +1,6 @@
>  QA output created by 255
>  	1. into a hole
> +0: [0..39]: hole
>  daa100df6e6711906b61c9ab5aa16032
>  	2. into allocated space
>  0: [0..7]: extent
> @@ -42,6 +43,7 @@ daa100df6e6711906b61c9ab5aa16032
>  3: [32..39]: hole
>  cc63069677939f69a6e8f68cae6a6dac
>  	10. hole -> data -> hole
> +0: [0..39]: hole
>  daa100df6e6711906b61c9ab5aa16032
>  	11. data -> hole -> data
>  0: [0..7]: extent
> @@ -79,6 +81,7 @@ eecb7aa303d121835de05028751d301c
>  0000400 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
>  *
>  	1. into a hole
> +0: [0..39]: hole
>  daa100df6e6711906b61c9ab5aa16032
>  	2. into allocated space
>  0: [0..7]: extent
> @@ -121,6 +124,7 @@ daa100df6e6711906b61c9ab5aa16032
>  3: [32..39]: hole
>  cc63069677939f69a6e8f68cae6a6dac
>  	10. hole -> data -> hole
> +0: [0..39]: hole
>  daa100df6e6711906b61c9ab5aa16032
>  	11. data -> hole -> data
>  0: [0..7]: extent
> @@ -158,6 +162,7 @@ eecb7aa303d121835de05028751d301c
>  0000400 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
>  *
>  	1. into a hole
> +0: [0..39]: hole
>  daa100df6e6711906b61c9ab5aa16032
>  	2. into allocated space
>  0: [0..7]: extent
> @@ -240,6 +245,7 @@ eecb7aa303d121835de05028751d301c
>  0000400 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
>  *
>  	1. into a hole
> +0: [0..39]: hole
>  daa100df6e6711906b61c9ab5aa16032
>  	2. into allocated space
>  0: [0..7]: extent
> diff --git a/tests/generic/316.out b/tests/generic/316.out
> index 383f0d1..5506198 100644
> --- a/tests/generic/316.out
> +++ b/tests/generic/316.out
> @@ -1,5 +1,6 @@
>  QA output created by 316
>  	1. into a hole
> +0: [0..39]: hole
>  daa100df6e6711906b61c9ab5aa16032
>  	2. into allocated space
>  0: [0..7]: extent
> @@ -16,6 +17,7 @@ cc63069677939f69a6e8f68cae6a6dac
>  1: [8..39]: hole
>  1b3779878366498b28c702ef88c4a773
>  	10. hole -> data -> hole
> +0: [0..39]: hole
>  daa100df6e6711906b61c9ab5aa16032
>  	11. data -> hole -> data
>  0: [0..7]: extent
> @@ -43,6 +45,7 @@ eecb7aa303d121835de05028751d301c
>  0000400 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
>  *
>  	1. into a hole
> +0: [0..39]: hole
>  daa100df6e6711906b61c9ab5aa16032
>  	2. into allocated space
>  0: [0..7]: extent
> @@ -59,6 +62,7 @@ cc63069677939f69a6e8f68cae6a6dac
>  1: [8..39]: hole
>  1b3779878366498b28c702ef88c4a773
>  	10. hole -> data -> hole
> +0: [0..39]: hole
>  daa100df6e6711906b61c9ab5aa16032
>  	11. data -> hole -> data
>  0: [0..7]: extent
> @@ -86,6 +90,7 @@ eecb7aa303d121835de05028751d301c
>  0000400 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
>  *
>  	1. into a hole
> +0: [0..39]: hole
>  daa100df6e6711906b61c9ab5aa16032
>  	2. into allocated space
>  0: [0..7]: extent
> @@ -133,6 +138,7 @@ eecb7aa303d121835de05028751d301c
>  0000400 cdcd cdcd cdcd cdcd cdcd cdcd cdcd cdcd
>  *
>  	1. into a hole
> +0: [0..39]: hole
>  daa100df6e6711906b61c9ab5aa16032
>  	2. into allocated space
>  0: [0..7]: extent
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 2/3] common: Implement fiemap's range query check
  2017-11-15 12:11   ` [PATCH v4 2/3] common: Implement fiemap's range query check Nikolay Borisov
@ 2017-11-21  5:39     ` Eryu Guan
  0 siblings, 0 replies; 11+ messages in thread
From: Eryu Guan @ 2017-11-21  5:39 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: linux-xfs, fstests, david, darrick.wong

On Wed, Nov 15, 2017 at 02:11:39PM +0200, Nikolay Borisov wrote:
> Allow for users of xfstest to assert the presence of fiemap's
> range query capabilities by requesting the "ranged" parameter.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

I think this patch could be folded into patch 3, the change comes with
users of the change.

> ---
>  common/rc | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index e2a8229..c98c224 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2053,6 +2053,12 @@ _require_xfs_io_command()
>  			-c "$command 4k 8k" $testfile 2>&1`
>  		;;
>  	"fiemap")
> +		if echo "$param" | grep -q "ranged"; then

Please add comments on the special "ranged" param.

> +			param=$(echo "$param" | sed "s/ranged//")
> +			$XFS_IO_PROG -F -f -c "pwrite 0 5k" -c "fsync" $testfile > /dev/null 2>&1
> +			lines=$($XFS_IO_PROG -c "fiemap 6k" $testfile 2>&1 | wc -l)
> +			[ $lines -eq 1 ] || _notrun "xfs_io $command ranged support is missing"

I don't see why we're expecting $lines to be 1, maybe I was testing
Eric's version of ranged query patch, I always see file name followed by
a extent description, so that's two lines. e.g.

testfile:
        0: [8..15]: 1300882584..1300882591

But how about just checking for "[offset [len]]" string in help message?
Because we know the test is asking for fiemap ranged support at this
point due to the special "ranged" param.

Thanks,
Eryu

> +		fi
>  		testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
>  			-c "fiemap -v $param" $testfile 2>&1`
>  		param_checked=1
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/3] xfs: initial fiemap range query test
  2017-11-15 12:11   ` [PATCH v4 3/3] xfs: initial fiemap range query test Nikolay Borisov
@ 2017-11-21  5:45     ` Eryu Guan
  2017-11-21 15:10       ` Nikolay Borisov
  0 siblings, 1 reply; 11+ messages in thread
From: Eryu Guan @ 2017-11-21  5:45 UTC (permalink / raw)
  To: Nikolay Borisov; +Cc: linux-xfs, fstests, david, darrick.wong

On Wed, Nov 15, 2017 at 02:11:40PM +0200, Nikolay Borisov wrote:
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Please add test description to commit log too.

> ---
> 
> V4: 
>  * Added test description
>  * Added new test for past-eof behavior
>  * Removed supported_generic_fs line
>  * Switched to using the "ranged" param require
>  * Revert v3 changes
> 
> V3:
>  * Adjusted tests for '-r' fiemap param
>  * Tests for invalid -r combination
> 
> V2: No change
> V1: No change
> 
>  tests/xfs/900     |  92 +++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/900.out | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 211 insertions(+)
>  create mode 100755 tests/xfs/900
>  create mode 100644 tests/xfs/900.out
> 
> diff --git a/tests/xfs/900 b/tests/xfs/900
> new file mode 100755
> index 0000000..06a5c64
> --- /dev/null
> +++ b/tests/xfs/900
> @@ -0,0 +1,92 @@
> +#! /bin/bash
> +# FS QA Test No. 900

Add an empty line after the first line.

> +# Test for the new ranged query functionality in xfs_io's fiemap command.
> +# This tests various combinations of hole + data layout being printed.
> +# Also the test used 16k holes to be compatible with 16k block filesystems

Empty line here too.

> +#-----------------------------------------------------------------------
> +# Copyright (c) 2017 SUSE Linux Products GmbH. All Rights Reserved.
> +# Author: Nikolay Borisov <nborisov@suse.com>
> +#
> +#

Extra empty line here.

> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +here=`pwd`
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/punch
> +
> +# remove previous $seqres.full before test
> +rm -f $seqres.full
> +
> +# real QA test starts here
> +
> +# Modify as appropriate.
> +_supported_os Linux
> +_require_scratch
> +_require_xfs_io_command "falloc"
> +_require_xfs_io_command "fiemap" "ranged"

Better to add comments about the "ranged" too in the test.

> +
> +_scratch_mkfs > $seqres.full 2>&1
> +_scratch_mount || _fail "mount failure"
> +
> +file=$SCRATCH_MNT/testfile
> +$XFS_IO_PROG -f -c "falloc 0 1m" $file 

Trailing whitespace in above line.

> +for i in {0..31}; do $XFS_IO_PROG -c "fpunch $(($i*32))k 16k" $file; done
> +
> +# Query 1 data extent between 16k..16k range
> +echo "Basic data extent"
> +$XFS_IO_PROG -c "fiemap -v 16k 16k" $file | _filter_fiemap 

Trailing whitespace in above line.

> +
> +# Query data and hole extent
> +echo "Data + Hole"
> +$XFS_IO_PROG -c "fiemap -v 16k 20k" $file | _filter_fiemap
> +
> +echo "Hole + Data + Hole"
> +$XFS_IO_PROG -c "fiemap -v 32k 40k" $file | _filter_fiemap

Test "Only hole", "Hole + Data" and "Data + Hole + Data" too? As well as
some tests to exercise range boundaries are not aligned with extent
boundaries?

> +
> +echo "Beginning with a hole"
> +$XFS_IO_PROG -c "fiemap -v 0 3k" $file | _filter_fiemap
> +
> +# Query for 0..160k that's 40 extents, more than the EXTENT_BATCH
> +echo "Query more than 32 extents"
> +$XFS_IO_PROG -c "fiemap -v 0 640k" $file | _filter_fiemap
> +
> +echo "Larger query than file size"
> +$XFS_IO_PROG -c "fiemap -v 0 2m" $file | _filter_fiemap
> +
> +#mapping past eof shouldn't print anything"

Add a space after "#" for comments.

> +$XFS_IO_PROG -c "fiemap -v 2m" $file | _filter_fiemap
> +
> +#check everything without the first hole

Same here.

Thanks,
Eryu

> +$XFS_IO_PROG -c "fiemap -v 16k" $file | wc -l
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/900.out b/tests/xfs/900.out
> new file mode 100644
> index 0000000..6ecf1b6
> --- /dev/null
> +++ b/tests/xfs/900.out
> @@ -0,0 +1,119 @@
> +QA output created by 900
> +Basic data extent
> +0: [32..63]: unwritten
> +Data + Hole
> +0: [32..63]: unwritten
> +1: [64..95]: hole
> +Hole + Data + Hole
> +0: [64..95]: hole
> +1: [96..127]: unwritten
> +2: [128..159]: hole
> +Beginning with a hole
> +0: [0..31]: hole
> +Query more than 32 extents
> +0: [0..31]: hole
> +1: [32..63]: unwritten
> +2: [64..95]: hole
> +3: [96..127]: unwritten
> +4: [128..159]: hole
> +5: [160..191]: unwritten
> +6: [192..223]: hole
> +7: [224..255]: unwritten
> +8: [256..287]: hole
> +9: [288..319]: unwritten
> +10: [320..351]: hole
> +11: [352..383]: unwritten
> +12: [384..415]: hole
> +13: [416..447]: unwritten
> +14: [448..479]: hole
> +15: [480..511]: unwritten
> +16: [512..543]: hole
> +17: [544..575]: unwritten
> +18: [576..607]: hole
> +19: [608..639]: unwritten
> +20: [640..671]: hole
> +21: [672..703]: unwritten
> +22: [704..735]: hole
> +23: [736..767]: unwritten
> +24: [768..799]: hole
> +25: [800..831]: unwritten
> +26: [832..863]: hole
> +27: [864..895]: unwritten
> +28: [896..927]: hole
> +29: [928..959]: unwritten
> +30: [960..991]: hole
> +31: [992..1023]: unwritten
> +32: [1024..1055]: hole
> +33: [1056..1087]: unwritten
> +34: [1088..1119]: hole
> +35: [1120..1151]: unwritten
> +36: [1152..1183]: hole
> +37: [1184..1215]: unwritten
> +38: [1216..1247]: hole
> +39: [1248..1279]: unwritten
> +Larger query than file size
> +0: [0..31]: hole
> +1: [32..63]: unwritten
> +2: [64..95]: hole
> +3: [96..127]: unwritten
> +4: [128..159]: hole
> +5: [160..191]: unwritten
> +6: [192..223]: hole
> +7: [224..255]: unwritten
> +8: [256..287]: hole
> +9: [288..319]: unwritten
> +10: [320..351]: hole
> +11: [352..383]: unwritten
> +12: [384..415]: hole
> +13: [416..447]: unwritten
> +14: [448..479]: hole
> +15: [480..511]: unwritten
> +16: [512..543]: hole
> +17: [544..575]: unwritten
> +18: [576..607]: hole
> +19: [608..639]: unwritten
> +20: [640..671]: hole
> +21: [672..703]: unwritten
> +22: [704..735]: hole
> +23: [736..767]: unwritten
> +24: [768..799]: hole
> +25: [800..831]: unwritten
> +26: [832..863]: hole
> +27: [864..895]: unwritten
> +28: [896..927]: hole
> +29: [928..959]: unwritten
> +30: [960..991]: hole
> +31: [992..1023]: unwritten
> +32: [1024..1055]: hole
> +33: [1056..1087]: unwritten
> +34: [1088..1119]: hole
> +35: [1120..1151]: unwritten
> +36: [1152..1183]: hole
> +37: [1184..1215]: unwritten
> +38: [1216..1247]: hole
> +39: [1248..1279]: unwritten
> +40: [1280..1311]: hole
> +41: [1312..1343]: unwritten
> +42: [1344..1375]: hole
> +43: [1376..1407]: unwritten
> +44: [1408..1439]: hole
> +45: [1440..1471]: unwritten
> +46: [1472..1503]: hole
> +47: [1504..1535]: unwritten
> +48: [1536..1567]: hole
> +49: [1568..1599]: unwritten
> +50: [1600..1631]: hole
> +51: [1632..1663]: unwritten
> +52: [1664..1695]: hole
> +53: [1696..1727]: unwritten
> +54: [1728..1759]: hole
> +55: [1760..1791]: unwritten
> +56: [1792..1823]: hole
> +57: [1824..1855]: unwritten
> +58: [1856..1887]: hole
> +59: [1888..1919]: unwritten
> +60: [1920..1951]: hole
> +61: [1952..1983]: unwritten
> +62: [1984..2015]: hole
> +63: [2016..2047]: unwritten
> +65
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v4 3/3] xfs: initial fiemap range query test
  2017-11-21  5:45     ` Eryu Guan
@ 2017-11-21 15:10       ` Nikolay Borisov
  0 siblings, 0 replies; 11+ messages in thread
From: Nikolay Borisov @ 2017-11-21 15:10 UTC (permalink / raw)
  To: Eryu Guan; +Cc: linux-xfs, fstests, david, darrick.wong



On 21.11.2017 07:45, Eryu Guan wrote:
> On Wed, Nov 15, 2017 at 02:11:40PM +0200, Nikolay Borisov wrote:
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> 
> Please add test description to commit log too.
> 
>> ---
>>
>> V4: 
>>  * Added test description
>>  * Added new test for past-eof behavior
>>  * Removed supported_generic_fs line
>>  * Switched to using the "ranged" param require
>>  * Revert v3 changes
>>
>> V3:
>>  * Adjusted tests for '-r' fiemap param
>>  * Tests for invalid -r combination
>>
>> V2: No change
>> V1: No change
>>
>>  tests/xfs/900     |  92 +++++++++++++++++++++++++++++++++++++++++
>>  tests/xfs/900.out | 119 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 211 insertions(+)
>>  create mode 100755 tests/xfs/900
>>  create mode 100644 tests/xfs/900.out
>>
>> diff --git a/tests/xfs/900 b/tests/xfs/900
>> new file mode 100755
>> index 0000000..06a5c64
>> --- /dev/null
>> +++ b/tests/xfs/900
>> @@ -0,0 +1,92 @@
>> +#! /bin/bash
>> +# FS QA Test No. 900
> 
> Add an empty line after the first line.
> 
>> +# Test for the new ranged query functionality in xfs_io's fiemap command.
>> +# This tests various combinations of hole + data layout being printed.
>> +# Also the test used 16k holes to be compatible with 16k block filesystems
> 
> Empty line here too.
> 
>> +#-----------------------------------------------------------------------
>> +# Copyright (c) 2017 SUSE Linux Products GmbH. All Rights Reserved.
>> +# Author: Nikolay Borisov <nborisov@suse.com>
>> +#
>> +#
> 
> Extra empty line here.
> 
>> +# This program is free software; you can redistribute it and/or
>> +# modify it under the terms of the GNU General Public License as
>> +# published by the Free Software Foundation.
>> +#
>> +# This program is distributed in the hope that it would be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program; if not, write the Free Software Foundation,
>> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>> +#-----------------------------------------------------------------------
>> +#
>> +
>> +seq=`basename $0`
>> +seqres=$RESULT_DIR/$seq
>> +echo "QA output created by $seq"
>> +
>> +here=`pwd`
>> +tmp=/tmp/$$
>> +status=1	# failure is the default!
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +_cleanup()
>> +{
>> +	cd /
>> +	rm -f $tmp.*
>> +}
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +. ./common/punch
>> +
>> +# remove previous $seqres.full before test
>> +rm -f $seqres.full
>> +
>> +# real QA test starts here
>> +
>> +# Modify as appropriate.
>> +_supported_os Linux
>> +_require_scratch
>> +_require_xfs_io_command "falloc"
>> +_require_xfs_io_command "fiemap" "ranged"
> 
> Better to add comments about the "ranged" too in the test.
> 
>> +
>> +_scratch_mkfs > $seqres.full 2>&1
>> +_scratch_mount || _fail "mount failure"
>> +
>> +file=$SCRATCH_MNT/testfile
>> +$XFS_IO_PROG -f -c "falloc 0 1m" $file 
> 
> Trailing whitespace in above line.
> 
>> +for i in {0..31}; do $XFS_IO_PROG -c "fpunch $(($i*32))k 16k" $file; done
>> +
>> +# Query 1 data extent between 16k..16k range
>> +echo "Basic data extent"
>> +$XFS_IO_PROG -c "fiemap -v 16k 16k" $file | _filter_fiemap 
> 
> Trailing whitespace in above line.
> 
>> +
>> +# Query data and hole extent
>> +echo "Data + Hole"
>> +$XFS_IO_PROG -c "fiemap -v 16k 20k" $file | _filter_fiemap
>> +
>> +echo "Hole + Data + Hole"
>> +$XFS_IO_PROG -c "fiemap -v 32k 40k" $file | _filter_fiemap
> 
> Test "Only hole", "Hole + Data" and "Data + Hole + Data" too? As well as
> some tests to exercise range boundaries are not aligned with extent
> boundaries?

Only hole is covered by "Begining with a hole", I've added the next 2
suggestions. Also some tests already have boundaries which are not
aligned and the behavior there is pretty well defined by the kernel so
no point in adding specific tests for those.

> 
>> +
>> +echo "Beginning with a hole"
>> +$XFS_IO_PROG -c "fiemap -v 0 3k" $file | _filter_fiemap
>> +
>> +# Query for 0..160k that's 40 extents, more than the EXTENT_BATCH
>> +echo "Query more than 32 extents"
>> +$XFS_IO_PROG -c "fiemap -v 0 640k" $file | _filter_fiemap
>> +
>> +echo "Larger query than file size"
>> +$XFS_IO_PROG -c "fiemap -v 0 2m" $file | _filter_fiemap
>> +
>> +#mapping past eof shouldn't print anything"
> 
> Add a space after "#" for comments.
> 
>> +$XFS_IO_PROG -c "fiemap -v 2m" $file | _filter_fiemap
>> +
>> +#check everything without the first hole
> 
> Same here.
> 
> Thanks,
> Eryu
> 
>> +$XFS_IO_PROG -c "fiemap -v 16k" $file | wc -l
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/xfs/900.out b/tests/xfs/900.out
>> new file mode 100644
>> index 0000000..6ecf1b6
>> --- /dev/null
>> +++ b/tests/xfs/900.out
>> @@ -0,0 +1,119 @@
>> +QA output created by 900
>> +Basic data extent
>> +0: [32..63]: unwritten
>> +Data + Hole
>> +0: [32..63]: unwritten
>> +1: [64..95]: hole
>> +Hole + Data + Hole
>> +0: [64..95]: hole
>> +1: [96..127]: unwritten
>> +2: [128..159]: hole
>> +Beginning with a hole
>> +0: [0..31]: hole
>> +Query more than 32 extents
>> +0: [0..31]: hole
>> +1: [32..63]: unwritten
>> +2: [64..95]: hole
>> +3: [96..127]: unwritten
>> +4: [128..159]: hole
>> +5: [160..191]: unwritten
>> +6: [192..223]: hole
>> +7: [224..255]: unwritten
>> +8: [256..287]: hole
>> +9: [288..319]: unwritten
>> +10: [320..351]: hole
>> +11: [352..383]: unwritten
>> +12: [384..415]: hole
>> +13: [416..447]: unwritten
>> +14: [448..479]: hole
>> +15: [480..511]: unwritten
>> +16: [512..543]: hole
>> +17: [544..575]: unwritten
>> +18: [576..607]: hole
>> +19: [608..639]: unwritten
>> +20: [640..671]: hole
>> +21: [672..703]: unwritten
>> +22: [704..735]: hole
>> +23: [736..767]: unwritten
>> +24: [768..799]: hole
>> +25: [800..831]: unwritten
>> +26: [832..863]: hole
>> +27: [864..895]: unwritten
>> +28: [896..927]: hole
>> +29: [928..959]: unwritten
>> +30: [960..991]: hole
>> +31: [992..1023]: unwritten
>> +32: [1024..1055]: hole
>> +33: [1056..1087]: unwritten
>> +34: [1088..1119]: hole
>> +35: [1120..1151]: unwritten
>> +36: [1152..1183]: hole
>> +37: [1184..1215]: unwritten
>> +38: [1216..1247]: hole
>> +39: [1248..1279]: unwritten
>> +Larger query than file size
>> +0: [0..31]: hole
>> +1: [32..63]: unwritten
>> +2: [64..95]: hole
>> +3: [96..127]: unwritten
>> +4: [128..159]: hole
>> +5: [160..191]: unwritten
>> +6: [192..223]: hole
>> +7: [224..255]: unwritten
>> +8: [256..287]: hole
>> +9: [288..319]: unwritten
>> +10: [320..351]: hole
>> +11: [352..383]: unwritten
>> +12: [384..415]: hole
>> +13: [416..447]: unwritten
>> +14: [448..479]: hole
>> +15: [480..511]: unwritten
>> +16: [512..543]: hole
>> +17: [544..575]: unwritten
>> +18: [576..607]: hole
>> +19: [608..639]: unwritten
>> +20: [640..671]: hole
>> +21: [672..703]: unwritten
>> +22: [704..735]: hole
>> +23: [736..767]: unwritten
>> +24: [768..799]: hole
>> +25: [800..831]: unwritten
>> +26: [832..863]: hole
>> +27: [864..895]: unwritten
>> +28: [896..927]: hole
>> +29: [928..959]: unwritten
>> +30: [960..991]: hole
>> +31: [992..1023]: unwritten
>> +32: [1024..1055]: hole
>> +33: [1056..1087]: unwritten
>> +34: [1088..1119]: hole
>> +35: [1120..1151]: unwritten
>> +36: [1152..1183]: hole
>> +37: [1184..1215]: unwritten
>> +38: [1216..1247]: hole
>> +39: [1248..1279]: unwritten
>> +40: [1280..1311]: hole
>> +41: [1312..1343]: unwritten
>> +42: [1344..1375]: hole
>> +43: [1376..1407]: unwritten
>> +44: [1408..1439]: hole
>> +45: [1440..1471]: unwritten
>> +46: [1472..1503]: hole
>> +47: [1504..1535]: unwritten
>> +48: [1536..1567]: hole
>> +49: [1568..1599]: unwritten
>> +50: [1600..1631]: hole
>> +51: [1632..1663]: unwritten
>> +52: [1664..1695]: hole
>> +53: [1696..1727]: unwritten
>> +54: [1728..1759]: hole
>> +55: [1760..1791]: unwritten
>> +56: [1792..1823]: hole
>> +57: [1824..1855]: unwritten
>> +58: [1856..1887]: hole
>> +59: [1888..1919]: unwritten
>> +60: [1920..1951]: hole
>> +61: [1952..1983]: unwritten
>> +62: [1984..2015]: hole
>> +63: [2016..2047]: unwritten
>> +65
>> -- 
>> 2.7.4
>>
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2017-11-21 15:10 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-15 12:10 [PATCH v4 1/2] fiemap: Factor out actual fiemap call code Nikolay Borisov
2017-11-15 12:10 ` [PATCH v4 2/2] fiemap: Implement ranged query Nikolay Borisov
2017-11-17  2:47   ` Eric Sandeen
2017-11-17  9:39     ` Nikolay Borisov
2017-11-15 12:11 ` [PATCH v4 1/3] generic: Adjust generic test outputs for new fiemap imeplementation Nikolay Borisov
2017-11-15 12:11   ` [PATCH v4 2/3] common: Implement fiemap's range query check Nikolay Borisov
2017-11-21  5:39     ` Eryu Guan
2017-11-15 12:11   ` [PATCH v4 3/3] xfs: initial fiemap range query test Nikolay Borisov
2017-11-21  5:45     ` Eryu Guan
2017-11-21 15:10       ` Nikolay Borisov
2017-11-21  5:29   ` [PATCH v4 1/3] generic: Adjust generic test outputs for new fiemap imeplementation Eryu Guan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.