On 8/24/17 6:47 AM, Nikolay Borisov wrote:
> Currently the semantics of the -n argument are a bit idiosyncratic. We want the
> argument to be the limit of extents that are going to be output by the tool. This
> is clearly broken now as evident from the following example on a fragmented file:
>
> xfs_io -c "fiemap -v -n 5" test-dir/fragmented-file
> test-dir/fragmented-file:
> EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
> 0: [0..15]: hole 16
> 1: [16..23]: 897847296..897847303 8 0x0
> 2: [24..31]: hole 8
> 3: [32..39]: 897851392..897851399 8 0x0
>
> So we want at most 5 extents printed, yet we get 4. So we always print n - 1
> extents.
>
> With this modification the output looks like:
>
> xfs_io -c "fiemap -v -n 5" test-dir/fragmented-file
> test-dir/fragmented-file:
> EXT: FILE-OFFSET BLOCK-RANGE TOTAL FLAGS
> 0: [0..15]: hole 16
> 1: [16..23]: 897847296..897847303 8 0x0
> 2: [24..31]: hole 8
> 3: [32..39]: 897851392..897851399 8 0x0
> 4: [40..47]: hole 8
>
> Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx>
> ---
> io/fiemap.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/io/fiemap.c b/io/fiemap.c
> index 44a64870d711..7b275275465d 100644
> --- a/io/fiemap.c
> +++ b/io/fiemap.c
> @@ -122,7 +122,7 @@ print_verbose(
> cur_extent++;
> }
>
> - if ((cur_extent + 1) == max_extents)
> + if (cur_extent == max_extents)
> return 1;
>
> snprintf(lbuf, sizeof(lbuf), "[%llu..%llu]:", lstart,
> @@ -157,7 +157,7 @@ print_plain(
> cur_extent++;
> }
>
> - if ((cur_extent + 1) == max_extents)
> + if (cur_extent == max_extents)
> return 1;
>
> printf("\t%d: [%llu..%llu]: %llu..%llu", cur_extent,
> @@ -264,7 +264,7 @@ fiemap_f(
>
> printf("%s:\n", file->name);
>
> - while (!last && ((cur_extent + 1) != max_extents)) {
> + while (!last && (cur_extent != max_extents)) {
In my testing, for a bare fiemap (no -n) we get here with
cur_extent == max_extents == 0 and so nothing happens:
# xfs_io -c fiemap testfile
testfile:
#
(this causes every xfstest which invokes fiemap to fail).
I think initializing max_extents to -1 is a simple way to fix this.
>
> memset(fiemap, 0, map_size);
> fiemap->fm_flags = fiemap_flags;
> @@ -314,12 +314,12 @@ fiemap_f(
> break;
> }
>
> - if ((cur_extent + 1) == max_extents)
> + if (cur_extent == max_extents)
> break;
> }
> }
>
> - if ((cur_extent + 1) == max_extents)
> + if (cur_extent == max_extents)
^ extra space ;)
Thanks,
-Eric
> goto out;
>
> memset(&st, 0, sizeof(st));
>
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
