Re: [PATCH 6/6] fiemap: Fix semantics of max_extents (-n arguments)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux