Re: [PATCH 4/4] ext4: implement ext4_fiemap

[Kalpak Shah <Kalpak.Shah@Sun.COM>]

On Wed, 2008-06-25 at 15:19 -0700, Mark Fasheh wrote:
> From: Eric Sandeen <sandeen@redhat.com>
> +/* fiemap flags we can handle specified here */
> +#define EXT4_FIEMAP_FLAGS	(FIEMAP_FLAG_SYNC|FIEMAP_FLAG_XATTR)
> +
> +int ext4_xattr_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
> +{
> +	__u64 physical = 0;
> +	__u64 length;
> +	__u32 flags = FIEMAP_EXTENT_LAST;
> +	int blockbits = inode->i_sb->s_blocksize_bits;
> +	int error = 0;
> +
> +	/* in-inode? */
> +	if (EXT4_I(inode)->i_state & EXT4_STATE_XATTR) {
> +		struct ext4_iloc iloc;
> +		int offset;	/* offset of xattr in inode */
> +
> +		error = ext4_get_inode_loc(inode, &iloc);
> +		if (error)
> +			return error;
> +		physical = iloc.bh->b_blocknr << blockbits;
> +		offset = EXT4_GOOD_OLD_INODE_SIZE +
> +				EXT4_I(inode)->i_extra_isize;
> +		physical += offset;
> +		length = EXT4_SB(inode->i_sb)->s_inode_size - offset;
> +		flags |= FIEMAP_EXTENT_DATA_INLINE;
> +	} else { /* external block */
> +		physical = EXT4_I(inode)->i_file_acl << blockbits;
> +		length = inode->i_sb->s_blocksize;
> +	}
> +
> +	if (physical)
> +		error = fiemap_fill_next_extent(fieinfo, 0, physical,
> +					length, flags, inode->i_sb->s_dev);
> +	return (error < 0 ? error : 0);
> +}
> +

Extended attributes can be present in the inode as well as the external block at the same time and we should be returning extents for both them in that case. Below patch fixes this problem:

Signed-off-by: Kalpak Shah <kalpak.shah@sun.com> 

Index: linux-2.6/fs/ext4/extents.c
===================================================================
--- linux-2.6.orig/fs/ext4/extents.c
+++ linux-2.6/fs/ext4/extents.c
@@ -3130,7 +3130,7 @@ int ext4_xattr_fiemap(struct inode *inod
 {
 	__u64 physical = 0;
 	__u64 length;
-	__u32 flags = FIEMAP_EXTENT_LAST;
+	__u32 flags = 0;
 	int blockbits = inode->i_sb->s_blocksize_bits;
 	int error = 0;
 
@@ -3148,14 +3148,24 @@ int ext4_xattr_fiemap(struct inode *inod
 		physical += offset;
 		length = EXT4_SB(inode->i_sb)->s_inode_size - offset;
 		flags |= FIEMAP_EXTENT_DATA_INLINE;
-	} else { /* external block */
-		physical = EXT4_I(inode)->i_file_acl << blockbits;
-		length = inode->i_sb->s_blocksize;
+		if (EXT4_I(inode)->i_file_acl == 0)
+			flags |= FIEMAP_EXTENT_LAST;
+
+		error = fiemap_fill_next_extent(fieinfo, 0, physical, length,
+						flags, inode->i_sb->s_dev);
+		if (error)
+			goto out;
 	}
 
+	/* external block */
+	flags = FIEMAP_EXTENT_LAST;
+	physical = EXT4_I(inode)->i_file_acl << blockbits;
+	length = inode->i_sb->s_blocksize;
 	if (physical)
-		error = fiemap_fill_next_extent(fieinfo, 0, physical,
-					length, flags, inode->i_sb->s_dev);
+		error = fiemap_fill_next_extent(fieinfo, 0, physical, length,
+						flags, inode->i_sb->s_dev);
+
+out:
 	return (error < 0 ? error : 0);
 }
 

Thanks,
Kalpak