[PATCH v2 0/2] Fiemap's range query

[PATCH v2 0/2] Fiemap's range query

[Nikolay Borisov]

So here is an implementation of fiemap's range query. The only major change 
apart from range support is that for files which consists of only a hole fiemap
will print a line saying so, rather than being silent. Naturally, this breaks a 
bunch of generic fstests but I've attached fixes for them. The code also comes
with xfstests. Furthermore, I've verified that none of the generic tests fail 
with this implementation apart from those tests which I've fixed. 

V2: Considered all of Darrick's review for the preivous posting, as a result 
this is being posted as the fiemap patches coupled with the xfstest patches. 

Nikolay Borisov (2):
  fiemap: Factor out actual fiemap call code
  fiemap: Implement ranged query

 io/fiemap.c       | 110 +++++++++++++++++++++++++++++++++++++++++++-----------
 man/man8/xfs_io.8 |   5 ++-
 2 files changed, 92 insertions(+), 23 deletions(-)

-- 
2.7.4

[PATCH 1/2] fiemap: Factor out actual fiemap call code

[Nikolay Borisov]

This will be needed to in a subsequent patch to avoid code duplication. No 
functional changes

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

diff --git a/io/fiemap.c b/io/fiemap.c
index e6fd66da753d..4a52bff974e8 100644
--- a/io/fiemap.c
+++ b/io/fiemap.c
@@ -211,6 +211,30 @@ 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, (unsigned long)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 +290,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,
+			       len - covered_length);
 		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

Re: [PATCH 1/2] fiemap: Factor out actual fiemap call code

[Darrick J. Wong]

On Fri, Oct 27, 2017 at 03:37:09PM +0300, Nikolay Borisov wrote:
> This will be needed to in a subsequent patch to avoid code duplication. No 
> functional changes
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  io/fiemap.c | 38 +++++++++++++++++++++++++++-----------
>  1 file changed, 27 insertions(+), 11 deletions(-)
> 
> diff --git a/io/fiemap.c b/io/fiemap.c
> index e6fd66da753d..4a52bff974e8 100644
> --- a/io/fiemap.c
> +++ b/io/fiemap.c
> @@ -211,6 +211,30 @@ calc_print_format(
>  	}
>  }
>  
> +static int
> +__fiemap(struct fiemap * fiemap,

Function parameter should be on a separate line.

> +	 int		 mapsize,
> +	 __u32		 flags,
> +	 __u64		 start,
> +	 __u64		 length) {
> +
> +	int ret;
> +
> +	memset(fiemap, 0, mapsize);
> +        fiemap->fm_flags = flags;

Space/tab indent problems.

> +        fiemap->fm_start = start;
> +        fiemap->fm_length = length;
> +        fiemap->fm_extent_count = EXTENT_BATCH;
> +	ret = ioctl(file->fd, FS_IOC_FIEMAP, (unsigned long)fiemap);

Do we still need the unsigned long cast?

--D

> +	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 +290,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,
> +			       len - covered_length);
>  		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
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] fiemap: Factor out actual fiemap call code

[Darrick J. Wong]

On Fri, Oct 27, 2017 at 09:33:20AM -0700, Darrick J. Wong wrote:
> On Fri, Oct 27, 2017 at 03:37:09PM +0300, Nikolay Borisov wrote:
> > This will be needed to in a subsequent patch to avoid code duplication. No 
> > functional changes
> > 
> > Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> > ---
> >  io/fiemap.c | 38 +++++++++++++++++++++++++++-----------
> >  1 file changed, 27 insertions(+), 11 deletions(-)
> > 
> > diff --git a/io/fiemap.c b/io/fiemap.c
> > index e6fd66da753d..4a52bff974e8 100644
> > --- a/io/fiemap.c
> > +++ b/io/fiemap.c
> > @@ -211,6 +211,30 @@ calc_print_format(
> >  	}
> >  }
> >  
> > +static int
> > +__fiemap(struct fiemap * fiemap,
> 
> Function parameter should be on a separate line.
> 
> > +	 int		 mapsize,
> > +	 __u32		 flags,
> > +	 __u64		 start,
> > +	 __u64		 length) {
> > +
> > +	int ret;
> > +
> > +	memset(fiemap, 0, mapsize);
> > +        fiemap->fm_flags = flags;
> 
> Space/tab indent problems.
> 
> > +        fiemap->fm_start = start;
> > +        fiemap->fm_length = length;
> > +        fiemap->fm_extent_count = EXTENT_BATCH;
> > +	ret = ioctl(file->fd, FS_IOC_FIEMAP, (unsigned long)fiemap);
> 
> Do we still need the unsigned long cast?
> 
> --D
> 
> > +	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 +290,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,
> > +			       len - covered_length);

Also, where did covered_length come from?  Isn't that in patch 3?

--D

> >  		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
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] initial fiemap test

[Nikolay Borisov]

Test various range queries of fiemap and ensure they produce expected output. 

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 tests/xfs/900     |  96 ++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/900.out | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 214 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..e535820
--- /dev/null
+++ b/tests/xfs/900
@@ -0,0 +1,96 @@
+#! /bin/bash
+# FS QA Test No. 900
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 SUSE Linux Products GmbH.  All Rights Reserved.
+#
+# 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_fs generic
+_supported_os Linux
+_require_scratch
+_require_xfs_io_command "falloc"
+_require_xfs_io_command "fiemap"
+
+_scratch_mkfs > $seqres.full 2>&1
+_scratch_mount || _fail "mount failure"
+
+file=$SCRATCH_MNT/testfile
+$XFS_IO_PROG -f -c "falloc 0 256k" $file 
+for i in {0..31}; do $XFS_IO_PROG -c "fpunch $(($i*8))k 4k" $file; done
+
+# Query 1 data extent between 4k..8k range
+echo "Basic data extent"
+$XFS_IO_PROG -c "fiemap -v 4k 4k" $file | _filter_fiemap 
+
+# Query data and hole extent
+echo "Data + Hole"
+$XFS_IO_PROG -c "fiemap -v 4k 5k" $file | _filter_fiemap
+
+echo "Hole + Data + Hole"
+$XFS_IO_PROG -c "fiemap -v 8k 10k" $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 160k" $file | _filter_fiemap
+
+echo "Larger query than file size"
+$XFS_IO_PROG -c "fiemap -v 0 300k" $file | _filter_fiemap
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/900.out b/tests/xfs/900.out
new file mode 100644
index 0000000..acd309e
--- /dev/null
+++ b/tests/xfs/900.out
@@ -0,0 +1,118 @@
+QA output created by 900
+Basic data extent
+0: [8..15]: unwritten
+Data + Hole
+0: [8..15]: unwritten
+1: [16..23]: hole
+Hole + Data + Hole
+0: [16..23]: hole
+1: [24..31]: unwritten
+2: [32..39]: hole
+Beginning with a hole
+0: [0..7]: hole
+Query more than 32 extents
+0: [0..7]: hole
+1: [8..15]: unwritten
+2: [16..23]: hole
+3: [24..31]: unwritten
+4: [32..39]: hole
+5: [40..47]: unwritten
+6: [48..55]: hole
+7: [56..63]: unwritten
+8: [64..71]: hole
+9: [72..79]: unwritten
+10: [80..87]: hole
+11: [88..95]: unwritten
+12: [96..103]: hole
+13: [104..111]: unwritten
+14: [112..119]: hole
+15: [120..127]: unwritten
+16: [128..135]: hole
+17: [136..143]: unwritten
+18: [144..151]: hole
+19: [152..159]: unwritten
+20: [160..167]: hole
+21: [168..175]: unwritten
+22: [176..183]: hole
+23: [184..191]: unwritten
+24: [192..199]: hole
+25: [200..207]: unwritten
+26: [208..215]: hole
+27: [216..223]: unwritten
+28: [224..231]: hole
+29: [232..239]: unwritten
+30: [240..247]: hole
+31: [248..255]: unwritten
+32: [256..263]: hole
+33: [264..271]: unwritten
+34: [272..279]: hole
+35: [280..287]: unwritten
+36: [288..295]: hole
+37: [296..303]: unwritten
+38: [304..311]: hole
+39: [312..319]: unwritten
+Larger query than file size
+0: [0..7]: hole
+1: [8..15]: unwritten
+2: [16..23]: hole
+3: [24..31]: unwritten
+4: [32..39]: hole
+5: [40..47]: unwritten
+6: [48..55]: hole
+7: [56..63]: unwritten
+8: [64..71]: hole
+9: [72..79]: unwritten
+10: [80..87]: hole
+11: [88..95]: unwritten
+12: [96..103]: hole
+13: [104..111]: unwritten
+14: [112..119]: hole
+15: [120..127]: unwritten
+16: [128..135]: hole
+17: [136..143]: unwritten
+18: [144..151]: hole
+19: [152..159]: unwritten
+20: [160..167]: hole
+21: [168..175]: unwritten
+22: [176..183]: hole
+23: [184..191]: unwritten
+24: [192..199]: hole
+25: [200..207]: unwritten
+26: [208..215]: hole
+27: [216..223]: unwritten
+28: [224..231]: hole
+29: [232..239]: unwritten
+30: [240..247]: hole
+31: [248..255]: unwritten
+32: [256..263]: hole
+33: [264..271]: unwritten
+34: [272..279]: hole
+35: [280..287]: unwritten
+36: [288..295]: hole
+37: [296..303]: unwritten
+38: [304..311]: hole
+39: [312..319]: unwritten
+40: [320..327]: hole
+41: [328..335]: unwritten
+42: [336..343]: hole
+43: [344..351]: unwritten
+44: [352..359]: hole
+45: [360..367]: unwritten
+46: [368..375]: hole
+47: [376..383]: unwritten
+48: [384..391]: hole
+49: [392..399]: unwritten
+50: [400..407]: hole
+51: [408..415]: unwritten
+52: [416..423]: hole
+53: [424..431]: unwritten
+54: [432..439]: hole
+55: [440..447]: unwritten
+56: [448..455]: hole
+57: [456..463]: unwritten
+58: [464..471]: hole
+59: [472..479]: unwritten
+60: [480..487]: hole
+61: [488..495]: unwritten
+62: [496..503]: hole
+63: [504..511]: unwritten
-- 
2.7.4

Re: [PATCH 1/2] initial fiemap test

[Eryu Guan]

On Fri, Oct 27, 2017 at 03:37:10PM +0300, Nikolay Borisov wrote:
> Test various range queries of fiemap and ensure they produce expected output. 
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  tests/xfs/900     |  96 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/900.out | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 214 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..e535820
> --- /dev/null
> +++ b/tests/xfs/900
> @@ -0,0 +1,96 @@
> +#! /bin/bash
> +# FS QA Test No. 900
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (c) 2017 SUSE Linux Products GmbH.  All Rights Reserved.
> +#
> +# 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_fs generic
> +_supported_os Linux
> +_require_scratch
> +_require_xfs_io_command "falloc"
> +_require_xfs_io_command "fiemap"

I haven't run the test yet, but from a quick look, it looks like test
fails with old xfs_io without fiemap range support. Not sure adding
range param to _require_xfs_io_command helps, e.g.

_require_xfs_io_command "fiemap" "0 4k"

Thanks,
Eryu

> +
> +_scratch_mkfs > $seqres.full 2>&1
> +_scratch_mount || _fail "mount failure"
> +
> +file=$SCRATCH_MNT/testfile
> +$XFS_IO_PROG -f -c "falloc 0 256k" $file 
> +for i in {0..31}; do $XFS_IO_PROG -c "fpunch $(($i*8))k 4k" $file; done
> +
> +# Query 1 data extent between 4k..8k range
> +echo "Basic data extent"
> +$XFS_IO_PROG -c "fiemap -v 4k 4k" $file | _filter_fiemap 
> +
> +# Query data and hole extent
> +echo "Data + Hole"
> +$XFS_IO_PROG -c "fiemap -v 4k 5k" $file | _filter_fiemap
> +
> +echo "Hole + Data + Hole"
> +$XFS_IO_PROG -c "fiemap -v 8k 10k" $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 160k" $file | _filter_fiemap
> +
> +echo "Larger query than file size"
> +$XFS_IO_PROG -c "fiemap -v 0 300k" $file | _filter_fiemap
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/900.out b/tests/xfs/900.out
> new file mode 100644
> index 0000000..acd309e
> --- /dev/null
> +++ b/tests/xfs/900.out
> @@ -0,0 +1,118 @@
> +QA output created by 900
> +Basic data extent
> +0: [8..15]: unwritten
> +Data + Hole
> +0: [8..15]: unwritten
> +1: [16..23]: hole
> +Hole + Data + Hole
> +0: [16..23]: hole
> +1: [24..31]: unwritten
> +2: [32..39]: hole
> +Beginning with a hole
> +0: [0..7]: hole
> +Query more than 32 extents
> +0: [0..7]: hole
> +1: [8..15]: unwritten
> +2: [16..23]: hole
> +3: [24..31]: unwritten
> +4: [32..39]: hole
> +5: [40..47]: unwritten
> +6: [48..55]: hole
> +7: [56..63]: unwritten
> +8: [64..71]: hole
> +9: [72..79]: unwritten
> +10: [80..87]: hole
> +11: [88..95]: unwritten
> +12: [96..103]: hole
> +13: [104..111]: unwritten
> +14: [112..119]: hole
> +15: [120..127]: unwritten
> +16: [128..135]: hole
> +17: [136..143]: unwritten
> +18: [144..151]: hole
> +19: [152..159]: unwritten
> +20: [160..167]: hole
> +21: [168..175]: unwritten
> +22: [176..183]: hole
> +23: [184..191]: unwritten
> +24: [192..199]: hole
> +25: [200..207]: unwritten
> +26: [208..215]: hole
> +27: [216..223]: unwritten
> +28: [224..231]: hole
> +29: [232..239]: unwritten
> +30: [240..247]: hole
> +31: [248..255]: unwritten
> +32: [256..263]: hole
> +33: [264..271]: unwritten
> +34: [272..279]: hole
> +35: [280..287]: unwritten
> +36: [288..295]: hole
> +37: [296..303]: unwritten
> +38: [304..311]: hole
> +39: [312..319]: unwritten
> +Larger query than file size
> +0: [0..7]: hole
> +1: [8..15]: unwritten
> +2: [16..23]: hole
> +3: [24..31]: unwritten
> +4: [32..39]: hole
> +5: [40..47]: unwritten
> +6: [48..55]: hole
> +7: [56..63]: unwritten
> +8: [64..71]: hole
> +9: [72..79]: unwritten
> +10: [80..87]: hole
> +11: [88..95]: unwritten
> +12: [96..103]: hole
> +13: [104..111]: unwritten
> +14: [112..119]: hole
> +15: [120..127]: unwritten
> +16: [128..135]: hole
> +17: [136..143]: unwritten
> +18: [144..151]: hole
> +19: [152..159]: unwritten
> +20: [160..167]: hole
> +21: [168..175]: unwritten
> +22: [176..183]: hole
> +23: [184..191]: unwritten
> +24: [192..199]: hole
> +25: [200..207]: unwritten
> +26: [208..215]: hole
> +27: [216..223]: unwritten
> +28: [224..231]: hole
> +29: [232..239]: unwritten
> +30: [240..247]: hole
> +31: [248..255]: unwritten
> +32: [256..263]: hole
> +33: [264..271]: unwritten
> +34: [272..279]: hole
> +35: [280..287]: unwritten
> +36: [288..295]: hole
> +37: [296..303]: unwritten
> +38: [304..311]: hole
> +39: [312..319]: unwritten
> +40: [320..327]: hole
> +41: [328..335]: unwritten
> +42: [336..343]: hole
> +43: [344..351]: unwritten
> +44: [352..359]: hole
> +45: [360..367]: unwritten
> +46: [368..375]: hole
> +47: [376..383]: unwritten
> +48: [384..391]: hole
> +49: [392..399]: unwritten
> +50: [400..407]: hole
> +51: [408..415]: unwritten
> +52: [416..423]: hole
> +53: [424..431]: unwritten
> +54: [432..439]: hole
> +55: [440..447]: unwritten
> +56: [448..455]: hole
> +57: [456..463]: unwritten
> +58: [464..471]: hole
> +59: [472..479]: unwritten
> +60: [480..487]: hole
> +61: [488..495]: unwritten
> +62: [496..503]: hole
> +63: [504..511]: unwritten
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] initial fiemap test

[Nikolay Borisov]

On 27.10.2017 15:44, Eryu Guan wrote:
> On Fri, Oct 27, 2017 at 03:37:10PM +0300, Nikolay Borisov wrote:
>> Test various range queries of fiemap and ensure they produce expected output. 
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>  tests/xfs/900     |  96 ++++++++++++++++++++++++++++++++++++++++++++
>>  tests/xfs/900.out | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 214 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..e535820
>> --- /dev/null
>> +++ b/tests/xfs/900
>> @@ -0,0 +1,96 @@
>> +#! /bin/bash
>> +# FS QA Test No. 900
>> +#
>> +#-----------------------------------------------------------------------
>> +# Copyright (c) 2017 SUSE Linux Products GmbH.  All Rights Reserved.
>> +#
>> +# 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_fs generic
>> +_supported_os Linux
>> +_require_scratch
>> +_require_xfs_io_command "falloc"
>> +_require_xfs_io_command "fiemap"
> 
> I haven't run the test yet, but from a quick look, it looks like test
> fails with old xfs_io without fiemap range support. Not sure adding
> range param to _require_xfs_io_command helps, e.g.
> 
> _require_xfs_io_command "fiemap" "0 4k"

Apparently it doesn't :
./io/xfs_io -c "fiemap 0 2k" ../xfstests-dev/trace.dat
../xfstests-dev/trace.dat:
	0: [0..6095]: 18477056..18483151


the xfs_io is just v4.13.1 HEAD. One other things that comes to mind is
to play tricks with the xfs_io -c "help fiemap" invocation and parse the
first line. If the range param is supported then it will be present in
the one-line help if not, it won't be. Do you think that's acceptable ?


> 
> Thanks,
> Eryu
> 
>> +
>> +_scratch_mkfs > $seqres.full 2>&1
>> +_scratch_mount || _fail "mount failure"
>> +
>> +file=$SCRATCH_MNT/testfile
>> +$XFS_IO_PROG -f -c "falloc 0 256k" $file 
>> +for i in {0..31}; do $XFS_IO_PROG -c "fpunch $(($i*8))k 4k" $file; done
>> +
>> +# Query 1 data extent between 4k..8k range
>> +echo "Basic data extent"
>> +$XFS_IO_PROG -c "fiemap -v 4k 4k" $file | _filter_fiemap 
>> +
>> +# Query data and hole extent
>> +echo "Data + Hole"
>> +$XFS_IO_PROG -c "fiemap -v 4k 5k" $file | _filter_fiemap
>> +
>> +echo "Hole + Data + Hole"
>> +$XFS_IO_PROG -c "fiemap -v 8k 10k" $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 160k" $file | _filter_fiemap
>> +
>> +echo "Larger query than file size"
>> +$XFS_IO_PROG -c "fiemap -v 0 300k" $file | _filter_fiemap
>> +
>> +# success, all done
>> +status=0
>> +exit
>> diff --git a/tests/xfs/900.out b/tests/xfs/900.out
>> new file mode 100644
>> index 0000000..acd309e
>> --- /dev/null
>> +++ b/tests/xfs/900.out
>> @@ -0,0 +1,118 @@
>> +QA output created by 900
>> +Basic data extent
>> +0: [8..15]: unwritten
>> +Data + Hole
>> +0: [8..15]: unwritten
>> +1: [16..23]: hole
>> +Hole + Data + Hole
>> +0: [16..23]: hole
>> +1: [24..31]: unwritten
>> +2: [32..39]: hole
>> +Beginning with a hole
>> +0: [0..7]: hole
>> +Query more than 32 extents
>> +0: [0..7]: hole
>> +1: [8..15]: unwritten
>> +2: [16..23]: hole
>> +3: [24..31]: unwritten
>> +4: [32..39]: hole
>> +5: [40..47]: unwritten
>> +6: [48..55]: hole
>> +7: [56..63]: unwritten
>> +8: [64..71]: hole
>> +9: [72..79]: unwritten
>> +10: [80..87]: hole
>> +11: [88..95]: unwritten
>> +12: [96..103]: hole
>> +13: [104..111]: unwritten
>> +14: [112..119]: hole
>> +15: [120..127]: unwritten
>> +16: [128..135]: hole
>> +17: [136..143]: unwritten
>> +18: [144..151]: hole
>> +19: [152..159]: unwritten
>> +20: [160..167]: hole
>> +21: [168..175]: unwritten
>> +22: [176..183]: hole
>> +23: [184..191]: unwritten
>> +24: [192..199]: hole
>> +25: [200..207]: unwritten
>> +26: [208..215]: hole
>> +27: [216..223]: unwritten
>> +28: [224..231]: hole
>> +29: [232..239]: unwritten
>> +30: [240..247]: hole
>> +31: [248..255]: unwritten
>> +32: [256..263]: hole
>> +33: [264..271]: unwritten
>> +34: [272..279]: hole
>> +35: [280..287]: unwritten
>> +36: [288..295]: hole
>> +37: [296..303]: unwritten
>> +38: [304..311]: hole
>> +39: [312..319]: unwritten
>> +Larger query than file size
>> +0: [0..7]: hole
>> +1: [8..15]: unwritten
>> +2: [16..23]: hole
>> +3: [24..31]: unwritten
>> +4: [32..39]: hole
>> +5: [40..47]: unwritten
>> +6: [48..55]: hole
>> +7: [56..63]: unwritten
>> +8: [64..71]: hole
>> +9: [72..79]: unwritten
>> +10: [80..87]: hole
>> +11: [88..95]: unwritten
>> +12: [96..103]: hole
>> +13: [104..111]: unwritten
>> +14: [112..119]: hole
>> +15: [120..127]: unwritten
>> +16: [128..135]: hole
>> +17: [136..143]: unwritten
>> +18: [144..151]: hole
>> +19: [152..159]: unwritten
>> +20: [160..167]: hole
>> +21: [168..175]: unwritten
>> +22: [176..183]: hole
>> +23: [184..191]: unwritten
>> +24: [192..199]: hole
>> +25: [200..207]: unwritten
>> +26: [208..215]: hole
>> +27: [216..223]: unwritten
>> +28: [224..231]: hole
>> +29: [232..239]: unwritten
>> +30: [240..247]: hole
>> +31: [248..255]: unwritten
>> +32: [256..263]: hole
>> +33: [264..271]: unwritten
>> +34: [272..279]: hole
>> +35: [280..287]: unwritten
>> +36: [288..295]: hole
>> +37: [296..303]: unwritten
>> +38: [304..311]: hole
>> +39: [312..319]: unwritten
>> +40: [320..327]: hole
>> +41: [328..335]: unwritten
>> +42: [336..343]: hole
>> +43: [344..351]: unwritten
>> +44: [352..359]: hole
>> +45: [360..367]: unwritten
>> +46: [368..375]: hole
>> +47: [376..383]: unwritten
>> +48: [384..391]: hole
>> +49: [392..399]: unwritten
>> +50: [400..407]: hole
>> +51: [408..415]: unwritten
>> +52: [416..423]: hole
>> +53: [424..431]: unwritten
>> +54: [432..439]: hole
>> +55: [440..447]: unwritten
>> +56: [448..455]: hole
>> +57: [456..463]: unwritten
>> +58: [464..471]: hole
>> +59: [472..479]: unwritten
>> +60: [480..487]: hole
>> +61: [488..495]: unwritten
>> +62: [496..503]: hole
>> +63: [504..511]: unwritten
>> -- 
>> 2.7.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe fstests" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/2] initial fiemap test

[Eryu Guan]

On Tue, Oct 31, 2017 at 11:22:48AM +0200, Nikolay Borisov wrote:
> 
> 
> On 27.10.2017 15:44, Eryu Guan wrote:
> > On Fri, Oct 27, 2017 at 03:37:10PM +0300, Nikolay Borisov wrote:
> >> Test various range queries of fiemap and ensure they produce expected output. 
> >>
> >> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> >> ---
> >>  tests/xfs/900     |  96 ++++++++++++++++++++++++++++++++++++++++++++
> >>  tests/xfs/900.out | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 214 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..e535820
> >> --- /dev/null
> >> +++ b/tests/xfs/900
> >> @@ -0,0 +1,96 @@
> >> +#! /bin/bash
> >> +# FS QA Test No. 900
> >> +#
> >> +#-----------------------------------------------------------------------
> >> +# Copyright (c) 2017 SUSE Linux Products GmbH.  All Rights Reserved.
> >> +#
> >> +# 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_fs generic
> >> +_supported_os Linux
> >> +_require_scratch
> >> +_require_xfs_io_command "falloc"
> >> +_require_xfs_io_command "fiemap"
> > 
> > I haven't run the test yet, but from a quick look, it looks like test
> > fails with old xfs_io without fiemap range support. Not sure adding
> > range param to _require_xfs_io_command helps, e.g.
> > 
> > _require_xfs_io_command "fiemap" "0 4k"
> 
> Apparently it doesn't :
> ./io/xfs_io -c "fiemap 0 2k" ../xfstests-dev/trace.dat
> ../xfstests-dev/trace.dat:
> 	0: [0..6095]: 18477056..18483151

Looks like fiemap ignores all non-option arguments..

> 
> 
> the xfs_io is just v4.13.1 HEAD. One other things that comes to mind is
> to play tricks with the xfs_io -c "help fiemap" invocation and parse the
> first line. If the range param is supported then it will be present in
> the one-line help if not, it won't be. Do you think that's acceptable ?

I'm fine with it, we do have similar code to do such check on help
message.

Thanks,
Eryu

Re: [PATCH 1/2] initial fiemap test

[Nikolay Borisov]

On 31.10.2017 11:32, Eryu Guan wrote:
> On Tue, Oct 31, 2017 at 11:22:48AM +0200, Nikolay Borisov wrote:
>>
>>
>> On 27.10.2017 15:44, Eryu Guan wrote:
>>> On Fri, Oct 27, 2017 at 03:37:10PM +0300, Nikolay Borisov wrote:
>>>> Test various range queries of fiemap and ensure they produce expected output. 
>>>>
>>>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>>>> ---
>>>>  tests/xfs/900     |  96 ++++++++++++++++++++++++++++++++++++++++++++
>>>>  tests/xfs/900.out | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 214 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..e535820
>>>> --- /dev/null
>>>> +++ b/tests/xfs/900
>>>> @@ -0,0 +1,96 @@
>>>> +#! /bin/bash
>>>> +# FS QA Test No. 900
>>>> +#
>>>> +#-----------------------------------------------------------------------
>>>> +# Copyright (c) 2017 SUSE Linux Products GmbH.  All Rights Reserved.
>>>> +#
>>>> +# 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_fs generic
>>>> +_supported_os Linux
>>>> +_require_scratch
>>>> +_require_xfs_io_command "falloc"
>>>> +_require_xfs_io_command "fiemap"
>>>
>>> I haven't run the test yet, but from a quick look, it looks like test
>>> fails with old xfs_io without fiemap range support. Not sure adding
>>> range param to _require_xfs_io_command helps, e.g.
>>>
>>> _require_xfs_io_command "fiemap" "0 4k"
>>
>> Apparently it doesn't :
>> ./io/xfs_io -c "fiemap 0 2k" ../xfstests-dev/trace.dat
>> ../xfstests-dev/trace.dat:
>> 	0: [0..6095]: 18477056..18483151
> 
> Looks like fiemap ignores all non-option arguments..
> 
>>
>>
>> the xfs_io is just v4.13.1 HEAD. One other things that comes to mind is
>> to play tricks with the xfs_io -c "help fiemap" invocation and parse the
>> first line. If the range param is supported then it will be present in
>> the one-line help if not, it won't be. Do you think that's acceptable ?
> 
> I'm fine with it, we do have similar code to do such check on help
> message.

This is what I got I just want to run it past you before resending the whole series with the fixes incorporated: 

diff --git a/common/rc b/common/rc
index e2a8229..673f9ef 100644
--- a/common/rc
+++ b/common/rc
@@ -2053,8 +2053,15 @@ _require_xfs_io_command()
                        -c "$command 4k 8k" $testfile 2>&1`
                ;;
        "fiemap")
+               if [ ! -z "$param" ]
+               then
+                       $XFS_IO_PROG -c "help fiemap" | head -n 1 | grep -q "[offset [len]]" || \
+                               _notrun "xfs_io $command range param 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
                ;;
        "flink" )



> 
> Thanks,
> Eryu
> 

[PATCH 2/2] fiemap: Implement ranged query

[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 how 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]. Also now fiemap explicitly 
prints if a file consists of only a hole rather than staying silent.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 io/fiemap.c       | 72 +++++++++++++++++++++++++++++++++++++++++++++++--------
 man/man8/xfs_io.8 |  5 ++--
 2 files changed, 65 insertions(+), 12 deletions(-)

diff --git a/io/fiemap.c b/io/fiemap.c
index 4a52bff974e8..56bfb1a5f2d5 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;
@@ -122,7 +126,7 @@ print_verbose(
 		cur_extent++;
 	}
 
-	if (cur_extent == max_extents)
+	if (cur_extent == max_extents || (range_limit && covered_length >= limit))
 		return 1;
 
 	snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]:", lstart,
@@ -140,7 +144,8 @@ print_plain(
 	struct fiemap_extent	*extent,
 	int			lflag,
 	int			cur_extent,
-	__u64			last_logical)
+	__u64			last_logical,
+	__u64			limit)
 {
 	__u64			lstart;
 	__u64			llast;
@@ -157,7 +162,7 @@ print_plain(
 		cur_extent++;
 	}
 
-	if (cur_extent == max_extents)
+	if (cur_extent == max_extents || (range_limit && covered_length >= limit))
 		return 1;
 
 	printf("\t%d: [%llu..%llu]: %llu..%llu", cur_extent,
@@ -255,8 +260,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':
@@ -276,6 +284,26 @@ 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;
+	}
+
 	map_size = sizeof(struct fiemap) +
 		(EXTENT_BATCH * sizeof(struct fiemap_extent));
 	fiemap = malloc(map_size);
@@ -316,14 +344,18 @@ 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 == 2)
+				covered_length += extent->fe_length;
 
 			if (extent->fe_flags & FIEMAP_EXTENT_LAST) {
 				last = 1;
@@ -332,6 +364,9 @@ fiemap_f(
 
 			if (cur_extent == max_extents)
 				break;
+
+			if (range_limit && covered_length >= len)
+				goto out;
 		}
 	}
 
@@ -347,9 +382,26 @@ fiemap_f(
 		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);
@@ -364,7 +416,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

[PATCH 2/2] generic: Adjust generic test ouputs for new fiemap implementation

[Nikolay Borisov]

 Since xfs_io's fiemap implementation was changed to explicitly show a whole
 when a file consists only of it adjust the output of various test accordingly. 
 Essentially only test 1 and 10 of _test_generic_punch require changes

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 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 ++++++
 10 files changed, 22 insertions(+)

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

Re: [PATCH 2/2] generic: Adjust generic test ouputs for new fiemap implementation

[Eryu Guan]

On Fri, Oct 27, 2017 at 03:37:12PM +0300, Nikolay Borisov wrote:
>  Since xfs_io's fiemap implementation was changed to explicitly show a whole
>  when a file consists only of it adjust the output of various test accordingly. 
>  Essentially only test 1 and 10 of _test_generic_punch require changes
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  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 ++++++
>  10 files changed, 22 insertions(+)
> 
> 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

I suspect this will break test runs with old xfs_io too (and again, I
haven't confirmed yet..). Perhaps we need some kind of filter?

Thanks,
Eryu

Re: [PATCH 2/2] generic: Adjust generic test ouputs for new fiemap implementation

[Nikolay Borisov]

On 27.10.2017 15:46, Eryu Guan wrote:
> On Fri, Oct 27, 2017 at 03:37:12PM +0300, Nikolay Borisov wrote:
>>  Since xfs_io's fiemap implementation was changed to explicitly show a whole
>>  when a file consists only of it adjust the output of various test accordingly. 
>>  Essentially only test 1 and 10 of _test_generic_punch require changes
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>  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 ++++++
>>  10 files changed, 22 insertions(+)
>>
>> 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
> 
> I suspect this will break test runs with old xfs_io too (and again, I
> haven't confirmed yet..). Perhaps we need some kind of filter?

You are right it will indeed break with old xfs_io so some solution need
to be created. At this time I'm more interested to see if people will
accept the fiemap code and if there is agreement I will focus on
creating such a filter.

> 
> Thanks,
> Eryu
> 

[PATCH v2 1/2] fiemap: Factor out actual fiemap call code

[Nikolay Borisov]

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

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
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 | 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

[PATCH v2 2/2] fiemap: Implement ranged query

[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>
---
 io/fiemap.c       | 72 +++++++++++++++++++++++++++++++++++++++++++++++--------
 man/man8/xfs_io.8 |  5 ++--
 2 files changed, 65 insertions(+), 12 deletions(-)

diff --git a/io/fiemap.c b/io/fiemap.c
index 08391f69d9bd..668f8fe4e732 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;
@@ -122,7 +126,7 @@ print_verbose(
 		cur_extent++;
 	}
 
-	if (cur_extent == max_extents)
+	if (cur_extent == max_extents || (range_limit && covered_length >= limit))
 		return 1;
 
 	snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]:", lstart,
@@ -140,7 +144,8 @@ print_plain(
 	struct fiemap_extent	*extent,
 	int			lflag,
 	int			cur_extent,
-	__u64			last_logical)
+	__u64			last_logical,
+	__u64			limit)
 {
 	__u64			lstart;
 	__u64			llast;
@@ -157,7 +162,7 @@ print_plain(
 		cur_extent++;
 	}
 
-	if (cur_extent == max_extents)
+	if (cur_extent == max_extents || (range_limit && covered_length >= limit))
 		return 1;
 
 	printf("\t%d: [%llu..%llu]: %llu..%llu", cur_extent,
@@ -256,8 +261,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 +285,26 @@ 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;
+	}
+
 	map_size = sizeof(struct fiemap) +
 		(EXTENT_BATCH * sizeof(struct fiemap_extent));
 	fiemap = malloc(map_size);
@@ -317,14 +345,18 @@ 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 == 2)
+				covered_length += extent->fe_length;
 
 			if (extent->fe_flags & FIEMAP_EXTENT_LAST) {
 				last = 1;
@@ -333,6 +365,9 @@ fiemap_f(
 
 			if (cur_extent == max_extents)
 				break;
+
+			if (range_limit && covered_length >= len)
+				goto out;
 		}
 	}
 
@@ -348,9 +383,26 @@ fiemap_f(
 		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 +417,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

[PATCH v2 1/4] common: Check for fiemap range argument support

[Nikolay Borisov]

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

diff --git a/common/rc b/common/rc
index e2a8229..673f9ef 100644
--- a/common/rc
+++ b/common/rc
@@ -2053,8 +2053,15 @@ _require_xfs_io_command()
 			-c "$command 4k 8k" $testfile 2>&1`
 		;;
 	"fiemap")
+		if [ ! -z "$param" ]
+		then
+			$XFS_IO_PROG -c "help fiemap" | head -n 1 | grep -q "[offset [len]]" || \
+				_notrun "xfs_io $command range param 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
 		;;
 	"flink" )
-- 
2.7.4

[PATCH v2 2/4] punch: Implement fixup for fiemap range queries

[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. 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 | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/common/punch b/common/punch
index c4ed261..b20dc30 100644
--- a/common/punch
+++ b/common/punch
@@ -218,6 +218,23 @@ _filter_fiemap()
 	_coalesce_extents
 }
 
+_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 +380,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 +488,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
 
-- 
2.7.4

Re: [PATCH v2 2/4] punch: Implement fixup for fiemap range queries

[Eryu Guan]

On Tue, Oct 31, 2017 at 04:11:34PM +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. 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 | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/common/punch b/common/punch
> index c4ed261..b20dc30 100644
> --- a/common/punch
> +++ b/common/punch
> @@ -218,6 +218,23 @@ _filter_fiemap()
>  	_coalesce_extents
>  }
>  
> +_fiemap_range_fixup()

Good to have some comments too on what this function does.

> +{
> +	#check if we support ranged query
> +	$XFS_IO_PROG -c "help fiemap" | head -n 1 | grep -q "[offset [len]]"

return immediately if fiemap supports range query?

Thanks,
Eryu

> +	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 +380,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 +488,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
>  
> -- 
> 2.7.4
> 

[PATCH v2 3/4] generic: Adjust generic test ouputs for new fiemap implementation

[Nikolay Borisov]

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 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 ++++++
 10 files changed, 22 insertions(+)

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

Re: [PATCH v2 3/4] generic: Adjust generic test ouputs for new fiemap implementation

[Eryu Guan]

On Tue, Oct 31, 2017 at 04:11:35PM +0200, Nikolay Borisov wrote:
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

I think patch 2 and 3 should be merged to a single patch, so we don't
have a tiny window to break the affected tests with old fiemap command.

Thanks,
Eryu

> ---
>  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 ++++++
>  10 files changed, 22 insertions(+)
> 
> 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
> 

[PATCH v2 4/4] xfs: initial fiemap range query test

[Nikolay Borisov]

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 tests/xfs/900     |  96 ++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/900.out | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 214 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..a81f85f
--- /dev/null
+++ b/tests/xfs/900
@@ -0,0 +1,96 @@
+#! /bin/bash
+# FS QA Test No. 900
+#
+# Regression test for an XFS NULL xattr buffer problem during unlink. XFS had a
+# bug where the attr fork walk during file removal could go off the rails due to
+# a stale reference to content of a released buffer. Memory pressure could cause
+# this reference to point to free or reused memory and cause subsequent
+# attribute fork lookups to fail, return a NULL buffer and possibly crash.
+#
+# This test emulates this behavior using an error injection knob to explicitly
+# disable buffer LRU caching. This forces the attr walk to execute under
+# conditions where each buffer is immediately freed on release.
+#
+# Commit f35c5e10c6ed ("xfs: reinit btree pointer on attr tree inactivation
+# walk") fixed the bug.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2017 SUSE Linux Products GmbH.  All Rights Reserved.
+#
+# 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_fs generic
+_supported_os Linux
+_require_scratch
+_require_xfs_io_command "falloc"
+_require_xfs_io_command "fiemap" "0 4k"
+
+_scratch_mkfs > $seqres.full 2>&1
+_scratch_mount || _fail "mount failure"
+
+file=$SCRATCH_MNT/testfile
+$XFS_IO_PROG -f -c "falloc 0 256k" $file 
+for i in {0..31}; do $XFS_IO_PROG -c "fpunch $(($i*8))k 4k" $file; done
+
+# Query 1 data extent between 4k..8k range
+echo "Basic data extent"
+$XFS_IO_PROG -c "fiemap -v 4k 4k" $file | _filter_fiemap 
+
+# Query data and hole extent
+echo "Data + Hole"
+$XFS_IO_PROG -c "fiemap -v 4k 5k" $file | _filter_fiemap
+
+echo "Hole + Data + Hole"
+$XFS_IO_PROG -c "fiemap -v 8k 10k" $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 160k" $file | _filter_fiemap
+
+echo "Larger query than file size"
+$XFS_IO_PROG -c "fiemap -v 0 300k" $file | _filter_fiemap
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/900.out b/tests/xfs/900.out
new file mode 100644
index 0000000..acd309e
--- /dev/null
+++ b/tests/xfs/900.out
@@ -0,0 +1,118 @@
+QA output created by 900
+Basic data extent
+0: [8..15]: unwritten
+Data + Hole
+0: [8..15]: unwritten
+1: [16..23]: hole
+Hole + Data + Hole
+0: [16..23]: hole
+1: [24..31]: unwritten
+2: [32..39]: hole
+Beginning with a hole
+0: [0..7]: hole
+Query more than 32 extents
+0: [0..7]: hole
+1: [8..15]: unwritten
+2: [16..23]: hole
+3: [24..31]: unwritten
+4: [32..39]: hole
+5: [40..47]: unwritten
+6: [48..55]: hole
+7: [56..63]: unwritten
+8: [64..71]: hole
+9: [72..79]: unwritten
+10: [80..87]: hole
+11: [88..95]: unwritten
+12: [96..103]: hole
+13: [104..111]: unwritten
+14: [112..119]: hole
+15: [120..127]: unwritten
+16: [128..135]: hole
+17: [136..143]: unwritten
+18: [144..151]: hole
+19: [152..159]: unwritten
+20: [160..167]: hole
+21: [168..175]: unwritten
+22: [176..183]: hole
+23: [184..191]: unwritten
+24: [192..199]: hole
+25: [200..207]: unwritten
+26: [208..215]: hole
+27: [216..223]: unwritten
+28: [224..231]: hole
+29: [232..239]: unwritten
+30: [240..247]: hole
+31: [248..255]: unwritten
+32: [256..263]: hole
+33: [264..271]: unwritten
+34: [272..279]: hole
+35: [280..287]: unwritten
+36: [288..295]: hole
+37: [296..303]: unwritten
+38: [304..311]: hole
+39: [312..319]: unwritten
+Larger query than file size
+0: [0..7]: hole
+1: [8..15]: unwritten
+2: [16..23]: hole
+3: [24..31]: unwritten
+4: [32..39]: hole
+5: [40..47]: unwritten
+6: [48..55]: hole
+7: [56..63]: unwritten
+8: [64..71]: hole
+9: [72..79]: unwritten
+10: [80..87]: hole
+11: [88..95]: unwritten
+12: [96..103]: hole
+13: [104..111]: unwritten
+14: [112..119]: hole
+15: [120..127]: unwritten
+16: [128..135]: hole
+17: [136..143]: unwritten
+18: [144..151]: hole
+19: [152..159]: unwritten
+20: [160..167]: hole
+21: [168..175]: unwritten
+22: [176..183]: hole
+23: [184..191]: unwritten
+24: [192..199]: hole
+25: [200..207]: unwritten
+26: [208..215]: hole
+27: [216..223]: unwritten
+28: [224..231]: hole
+29: [232..239]: unwritten
+30: [240..247]: hole
+31: [248..255]: unwritten
+32: [256..263]: hole
+33: [264..271]: unwritten
+34: [272..279]: hole
+35: [280..287]: unwritten
+36: [288..295]: hole
+37: [296..303]: unwritten
+38: [304..311]: hole
+39: [312..319]: unwritten
+40: [320..327]: hole
+41: [328..335]: unwritten
+42: [336..343]: hole
+43: [344..351]: unwritten
+44: [352..359]: hole
+45: [360..367]: unwritten
+46: [368..375]: hole
+47: [376..383]: unwritten
+48: [384..391]: hole
+49: [392..399]: unwritten
+50: [400..407]: hole
+51: [408..415]: unwritten
+52: [416..423]: hole
+53: [424..431]: unwritten
+54: [432..439]: hole
+55: [440..447]: unwritten
+56: [448..455]: hole
+57: [456..463]: unwritten
+58: [464..471]: hole
+59: [472..479]: unwritten
+60: [480..487]: hole
+61: [488..495]: unwritten
+62: [496..503]: hole
+63: [504..511]: unwritten
-- 
2.7.4

Re: [PATCH v2 1/4] common: Check for fiemap range argument support

[Eryu Guan]

On Tue, Oct 31, 2017 at 04:11:33PM +0200, Nikolay Borisov wrote:
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  common/rc | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index e2a8229..673f9ef 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2053,8 +2053,15 @@ _require_xfs_io_command()
>  			-c "$command 4k 8k" $testfile 2>&1`
>  		;;
>  	"fiemap")
> +		if [ ! -z "$param" ]
> +		then
> +			$XFS_IO_PROG -c "help fiemap" | head -n 1 | grep -q "[offset [len]]" || \
                                                                            ^^^^^^^^^^^^^^^^
this doesn't look correct to me, the "bracket expression" is a
"Character Classes", grep matches any single char in the char set. A
quick test shows it only matches the last two chars, that's "n]"

$ echo "[offset len]" | grep  "[offset [len]]"
[offset len]
          ^^ highlighted

fgrep should work, or grep -q "\[offset \[len\]\]"

But now the fiemap check in _require_xfs_io_command checks range support
on whatever $param it takes, even the '[-al] [-n nx]' param that have
nothing to do with range.

Thanks,
Eryu

> +				_notrun "xfs_io $command range param 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
>  		;;
>  	"flink" )
> -- 
> 2.7.4
> 

Re: [PATCH v2 1/4] common: Check for fiemap range argument support

[Nikolay Borisov]

On  2.11.2017 05:16, Eryu Guan wrote:
> On Tue, Oct 31, 2017 at 04:11:33PM +0200, Nikolay Borisov wrote:
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>  common/rc | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/common/rc b/common/rc
>> index e2a8229..673f9ef 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -2053,8 +2053,15 @@ _require_xfs_io_command()
>>  			-c "$command 4k 8k" $testfile 2>&1`
>>  		;;
>>  	"fiemap")
>> +		if [ ! -z "$param" ]
>> +		then
>> +			$XFS_IO_PROG -c "help fiemap" | head -n 1 | grep -q "[offset [len]]" || \
>                                                                             ^^^^^^^^^^^^^^^^
> this doesn't look correct to me, the "bracket expression" is a
> "Character Classes", grep matches any single char in the char set. A
> quick test shows it only matches the last two chars, that's "n]"
> 
> $ echo "[offset len]" | grep  "[offset [len]]"
> [offset len]
>           ^^ highlighted
> 
> fgrep should work, or grep -q "\[offset \[len\]\]"
> 
> But now the fiemap check in _require_xfs_io_command checks range support
> on whatever $param it takes, even the '[-al] [-n nx]' param that have
> nothing to do with range.

Fair enough, my testing shows that something like that ought to work:
grep "[[:digit:]]\+[bskmgtpe]\? [[:digit:]]\+[bskmgtpe]\?$" to match
only the last two numbers

> 
> Thanks,
> Eryu
> 
>> +				_notrun "xfs_io $command range param 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
>>  		;;
>>  	"flink" )
>> -- 
>> 2.7.4
>>
> 

[PATCH v3] common: Check for fiemap range argument support

[Nikolay Borisov]

From: Nikolay Borsiov <nborisov@suse.com>

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
v3:
 * Changed the way we detect ranged args. Now use a regexp which checks 
 explicitly for the ranged args
 common/rc | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/common/rc b/common/rc
index e2a8229..f7a5fe9 100644
--- a/common/rc
+++ b/common/rc
@@ -2053,8 +2053,15 @@ _require_xfs_io_command()
 			-c "$command 4k 8k" $testfile 2>&1`
 		;;
 	"fiemap")
+		if echo "$param" | egrep -q "[[:digit:]]+[bskmgtpe]? [[:digit:]]+[bskmgtpe]?$"
+		then
+			$XFS_IO_PROG -c "help fiemap" | head -n 1 | grep -q "[offset [len]]" || \
+				_notrun "xfs_io $command range param 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
 		;;
 	"flink" )
-- 
2.7.4

Re: [PATCH v3] common: Check for fiemap range argument support

[Dave Chinner]

On Thu, Nov 02, 2017 at 10:13:41AM +0200, Nikolay Borisov wrote:
> From: Nikolay Borsiov <nborisov@suse.com>
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> v3:
>  * Changed the way we detect ranged args. Now use a regexp which checks 
>  explicitly for the ranged args
>  common/rc | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index e2a8229..f7a5fe9 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2053,8 +2053,15 @@ _require_xfs_io_command()
>  			-c "$command 4k 8k" $testfile 2>&1`
>  		;;
>  	"fiemap")
> +		if echo "$param" | egrep -q "[[:digit:]]+[bskmgtpe]? [[:digit:]]+[bskmgtpe]?$"

What is that for?

If you're going to dump crazy complex regexes to match something,
you need comments to explain it in both the commit message and the
code.

And, really, if we need to add complex or tricky code to check a
command exists, we've done somethign wrong. Don't make the "fiemap
range" command by triggered/detected by an optional parameter, add a
specific option instead e.g. "-r" for "range query":

$ xfs_io -c "fiemap -r 0 10k" /mnt/foo

Then you can test it like the falloc command tests it's different
parameters....

-Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v3] common: Check for fiemap range argument support

[Nikolay Borisov]

On  2.11.2017 23:12, Dave Chinner wrote:
> On Thu, Nov 02, 2017 at 10:13:41AM +0200, Nikolay Borisov wrote:
>> From: Nikolay Borsiov <nborisov@suse.com>
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>> v3:
>>  * Changed the way we detect ranged args. Now use a regexp which checks 
>>  explicitly for the ranged args
>>  common/rc | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/common/rc b/common/rc
>> index e2a8229..f7a5fe9 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -2053,8 +2053,15 @@ _require_xfs_io_command()
>>  			-c "$command 4k 8k" $testfile 2>&1`
>>  		;;
>>  	"fiemap")
>> +		if echo "$param" | egrep -q "[[:digit:]]+[bskmgtpe]? [[:digit:]]+[bskmgtpe]?$"
> 
> What is that for?
> 
> If you're going to dump crazy complex regexes to match something,
> you need comments to explain it in both the commit message and the
> code.
> 
> And, really, if we need to add complex or tricky code to check a
> command exists, we've done somethign wrong. Don't make the "fiemap
> range" command by triggered/detected by an optional parameter, add a
> specific option instead e.g. "-r" for "range query":
> 
> $ xfs_io -c "fiemap -r 0 10k" /mnt/foo

Do you mean having the range params as arguments to the -r option (which
would be a bitch to parse since getopt doesn't support multiple args to
an options). Or having the -r serve as a boolean indicating we'd need to
parse the final 2 args (as it's done currently)?

> 
> Then you can test it like the falloc command tests it's different
> parameters....
> 
> -Dave.
> 

Re: [PATCH v3] common: Check for fiemap range argument support

[Darrick J. Wong]

On Fri, Nov 03, 2017 at 04:05:37PM +0200, Nikolay Borisov wrote:
> 
> 
> On  2.11.2017 23:12, Dave Chinner wrote:
> > On Thu, Nov 02, 2017 at 10:13:41AM +0200, Nikolay Borisov wrote:
> >> From: Nikolay Borsiov <nborisov@suse.com>
> >>
> >> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> >> ---
> >> v3:
> >>  * Changed the way we detect ranged args. Now use a regexp which checks 
> >>  explicitly for the ranged args
> >>  common/rc | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/common/rc b/common/rc
> >> index e2a8229..f7a5fe9 100644
> >> --- a/common/rc
> >> +++ b/common/rc
> >> @@ -2053,8 +2053,15 @@ _require_xfs_io_command()
> >>  			-c "$command 4k 8k" $testfile 2>&1`
> >>  		;;
> >>  	"fiemap")
> >> +		if echo "$param" | egrep -q "[[:digit:]]+[bskmgtpe]? [[:digit:]]+[bskmgtpe]?$"
> > 
> > What is that for?
> > 
> > If you're going to dump crazy complex regexes to match something,
> > you need comments to explain it in both the commit message and the
> > code.
> > 
> > And, really, if we need to add complex or tricky code to check a
> > command exists, we've done somethign wrong. Don't make the "fiemap
> > range" command by triggered/detected by an optional parameter, add a
> > specific option instead e.g. "-r" for "range query":
> > 
> > $ xfs_io -c "fiemap -r 0 10k" /mnt/foo
> 
> Do you mean having the range params as arguments to the -r option (which
> would be a bitch to parse since getopt doesn't support multiple args to
> an options). Or having the -r serve as a boolean indicating we'd need to
> parse the final 2 args (as it's done currently)?

That wouldn't be a bad way to proceed -- -r tells fiemap to look for two
range arguments; lack of -r tells it to expect zero arguments.  Apparently
we don't really validate the freeform arguments...

$ xfs_io -c 'fiemap mugga wugga fribba gribba ding dong' VERSION
VERSION:
        0: [0..7]: 287903056..287903063

Heh.

--D

> 
> > 
> > Then you can test it like the falloc command tests it's different
> > parameters....
> > 
> > -Dave.
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] common: Check for fiemap range argument support

[Eric Sandeen]

On 11/2/17 3:13 AM, Nikolay Borisov wrote:
> From: Nikolay Borsiov <nborisov@suse.com>
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> v3:
>  * Changed the way we detect ranged args. Now use a regexp which checks 
>  explicitly for the ranged args
>  common/rc | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/common/rc b/common/rc
> index e2a8229..f7a5fe9 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -2053,8 +2053,15 @@ _require_xfs_io_command()
>  			-c "$command 4k 8k" $testfile 2>&1`
>  		;;
>  	"fiemap")
> +		if echo "$param" | egrep -q "[[:digit:]]+[bskmgtpe]? [[:digit:]]+[bskmgtpe]?$"
> +		then
> +			$XFS_IO_PROG -c "help fiemap" | head -n 1 | grep -q "[offset [len]]" || \
> +				_notrun "xfs_io $command range param support is missing"
> +		fi
> +

What if, rather than difficult to read regexps, we actually checked the functionality?
The check already writes a testfile; you could try fiemapping past EOF and see what
you get, i.e. for a file in a single 4k block:

$ io/xfs_io -c "fiemap" short
short:
	0: [0..7]: 2788960..2788967

Mapping past EOF will give you no extents (if the range values are honored by the
command):

$ io/xfs_io -c "fiemap 4k 4k" short
short:

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

so I think you could switch based on [ -n $param ] about whether we were asked
to check a range and if so, map past EOF, and return success or fail based
on what you get back.  The only slight weirdness is that you're asking to check
a specific range (_require_xfs_io_command "fiemap" "0 4k") but possibly testing
a different one ("20k 4k") but not sure if that matters.

It'd be a bit of a departure but with enough comments, maybe ok to do it as:

_require_xfs_io_command "fiemap" "ranged"

and use the "ranged" param to DTRT in the test.

-Eric

> +
>  		param_checked=1
>  		;;
>  	"flink" )
> 

Re: [PATCH v3] common: Check for fiemap range argument support

[Eryu Guan]

On Tue, Nov 14, 2017 at 09:09:21AM -0600, Eric Sandeen wrote:
> 
> 
> On 11/2/17 3:13 AM, Nikolay Borisov wrote:
> > From: Nikolay Borsiov <nborisov@suse.com>
> > 
> > Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> > ---
> > v3:
> >  * Changed the way we detect ranged args. Now use a regexp which checks 
> >  explicitly for the ranged args
> >  common/rc | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/common/rc b/common/rc
> > index e2a8229..f7a5fe9 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -2053,8 +2053,15 @@ _require_xfs_io_command()
> >  			-c "$command 4k 8k" $testfile 2>&1`
> >  		;;
> >  	"fiemap")
> > +		if echo "$param" | egrep -q "[[:digit:]]+[bskmgtpe]? [[:digit:]]+[bskmgtpe]?$"
> > +		then
> > +			$XFS_IO_PROG -c "help fiemap" | head -n 1 | grep -q "[offset [len]]" || \
> > +				_notrun "xfs_io $command range param support is missing"
> > +		fi
> > +
> 
> What if, rather than difficult to read regexps, we actually checked the functionality?
> The check already writes a testfile; you could try fiemapping past EOF and see what
> you get, i.e. for a file in a single 4k block:
> 
> $ io/xfs_io -c "fiemap" short
> short:
> 	0: [0..7]: 2788960..2788967
> 
> Mapping past EOF will give you no extents (if the range values are honored by the
> command):
> 
> $ io/xfs_io -c "fiemap 4k 4k" short
> short:

Yeah, I'm OK with it, looks better than the complicated regexp and could
keep the fiemap command interface simpler.

> 
> >  		testio=`$XFS_IO_PROG -F -f -c "pwrite 0 20k" -c "fsync" \
> >  			-c "fiemap -v $param" $testfile 2>&1`
> 
> so I think you could switch based on [ -n $param ] about whether we were asked
> to check a range and if so, map past EOF, and return success or fail based
> on what you get back.  The only slight weirdness is that you're asking to check
> a specific range (_require_xfs_io_command "fiemap" "0 4k") but possibly testing
> a different one ("20k 4k") but not sure if that matters.
> 
> It'd be a bit of a departure but with enough comments, maybe ok to do it as:
> 
> _require_xfs_io_command "fiemap" "ranged"
> 
> and use the "ranged" param to DTRT in the test.

This looks fine too to me.

Thanks,
Eryu

> 
> -Eric
> 
> > +
> >  		param_checked=1
> >  		;;
> >  	"flink" )
> > 

Re: [PATCH v2 1/2] fiemap: Factor out actual fiemap call code

[Darrick J. Wong]

> Subject: Re: [PATCH v2 1/2] fiemap: Factor out actual fiemap call code

I thought this was v3....

On Tue, Oct 31, 2017 at 04:10:10PM +0200, Nikolay Borisov wrote:
> This will be needed to in a subsequent patch to avoid code duplication
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> 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 | 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,

	struct fiemap	*fiemap,

> +	 int		 mapsize,
> +	 __u32		 flags,
> +	 __u64		 start,
> +	 __u64		 length) {
> +
> +	int ret;

	int		ret;

(Yeah, I know...)

> +
> +	memset(fiemap, 0, mapsize);
> +	fiemap->fm_flags = flags;
> +	fiemap->fm_start = start;
> +	fiemap->fm_length = length;
> +	fiemap->fm_extent_count = EXTENT_BATCH;

It's odd that we pass in both the struct* and its initializers here, but
we still have to have a way to pass fm_flags and fm_mapped_extents back
to the __fiemap() caller.

(TBH I'm wondering if this patch is necessary...)

--D

> +	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
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/2] fiemap: Factor out actual fiemap call code

[Nikolay Borisov]

On 31.10.2017 22:29, Darrick J. Wong wrote:
>> Subject: Re: [PATCH v2 1/2] fiemap: Factor out actual fiemap call code
> 
> I thought this was v3....
> 
> On Tue, Oct 31, 2017 at 04:10:10PM +0200, Nikolay Borisov wrote:
>> This will be needed to in a subsequent patch to avoid code duplication
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>> 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 | 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,
> 
> 	struct fiemap	*fiemap,
> 
>> +	 int		 mapsize,
>> +	 __u32		 flags,
>> +	 __u64		 start,
>> +	 __u64		 length) {
>> +
>> +	int ret;
> 
> 	int		ret;
> 
> (Yeah, I know...)
> 
>> +
>> +	memset(fiemap, 0, mapsize);
>> +	fiemap->fm_flags = flags;
>> +	fiemap->fm_start = start;
>> +	fiemap->fm_length = length;
>> +	fiemap->fm_extent_count = EXTENT_BATCH;
> 
> It's odd that we pass in both the struct* and its initializers here, but
> we still have to have a way to pass fm_flags and fm_mapped_extents back
> to the __fiemap() caller.
> 
> (TBH I'm wondering if this patch is necessary...)

Well,

I'd rather have this function and make the final hole handling in the
next one being a bit more clear, than duplicating the setup + ioctl
statements :)

> 
> --D
> 
>> +	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
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>