Re: [PATCH v2] xfs: Make fiemap works with sparse file.

[Alex Elder <aelder@sgi.com>]

On Thu, 2010-06-17 at 16:53 +0800, Tao Ma wrote:
> Hi Dave,
> On 06/14/2010 08:29 PM, Dave Chinner wrote:
> > I just had a thought - if you want to avoid holes being reported to
> > fiemap, then add a BMV_IF_NO_HOLES flag to xfs_getbmap() and skip
> > holes in the mappin gloop when this flag is set. That will make
> > fiemap fill in the full number of extents without hacking the
> > extent count...
> Here is the updated one. I have used BVM_IF_NO_HOLES in xfs_getbmap
> to skip increasing index 'cur_ext'. It is a bit ugly, see my commit
> log. I guess maybe we can add another flag in xfs_bmapi so that it
> don't even give us the holes?

Dave said he would commit this but it hasn't been done
yet, so there's still time to comment!

. . .

> diff --git a/fs/xfs/xfs_bmap.c b/fs/xfs/xfs_bmap.c
> index 99587de..d49107d 100644
> --- a/fs/xfs/xfs_bmap.c
> +++ b/fs/xfs/xfs_bmap.c
> @@ -5744,12 +5744,24 @@ xfs_getbmap(
>  					map[i].br_startblock))
>  				goto out_free_map;
>  
> -			nexleft--;
>  			bmv->bmv_offset =
>  				out[cur_ext].bmv_offset +
>  				out[cur_ext].bmv_length;
>  			bmv->bmv_length =
>  				max_t(__int64_t, 0, bmvend - bmv->bmv_offset);
> +
> +			/*
> +			 * In case we don't want to return the hole,
> +			 * don't increase cur_ext so that we can reuse
> +			 * it in the next loop.
> +			 */
> +			if ((iflags & BMV_IF_NO_HOLES) &&
> +			    out[cur_ext].bmv_block == -1LL) {

I personally would prefer this to use:
		if ((iflags & BMV_IF_NO_HOLES) &&
			map[i].br_startblock == HOLESTARTBLOCK) {

I think this is more obvious that this "extent" represents
a hole (as opposed to bmv_block == -1, which is how
xfs_getbmapx_fix_eof_hole() encodes this).

Otherwise looks good.

Reviewed-by: Alex Elder <aelder@sgi.com>



> +				memset(&out[cur_ext], 0, sizeof(out[cur_ext]));
> +				continue;
> +			}
> +
> +			nexleft--;
>  			bmv->bmv_entries++;
>  			cur_ext++;
>  		}
> diff --git a/fs/xfs/xfs_fs.h b/fs/xfs/xfs_fs.h
> index 7cf7220..87c2e9d 100644
> --- a/fs/xfs/xfs_fs.h
> +++ b/fs/xfs/xfs_fs.h
> @@ -114,8 +114,10 @@ struct getbmapx {
>  #define BMV_IF_NO_DMAPI_READ	0x2	/* Do not generate DMAPI read event  */
>  #define BMV_IF_PREALLOC		0x4	/* rtn status BMV_OF_PREALLOC if req */
>  #define BMV_IF_DELALLOC		0x8	/* rtn status BMV_OF_DELALLOC if req */
> +#define BMV_IF_NO_HOLES		0x10	/* Do not return holes */
>  #define BMV_IF_VALID	\
> -	(BMV_IF_ATTRFORK|BMV_IF_NO_DMAPI_READ|BMV_IF_PREALLOC|BMV_IF_DELALLOC)
> +	(BMV_IF_ATTRFORK|BMV_IF_NO_DMAPI_READ|BMV_IF_PREALLOC|	\
> +	 BMV_IF_DELALLOC|BMV_IF_NO_HOLES)
>  
>  /*	bmv_oflags values - returned for each non-header segment */
>  #define BMV_OF_PREALLOC		0x1	/* segment = unwritten pre-allocation */



_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs