2008-07-04 19:03:16

by Andreas Dilger

[permalink] [raw]
Subject: Re: [PATCH 4/4] ext4: implement ext4_fiemap

On Jul 03, 2008 22:31 +0530, Kalpak Shah wrote:
> On Wed, 2008-06-25 at 15:19 -0700, Mark Fasheh wrote:
> > +int ext4_xattr_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo)
> > +{
> > + /* in-inode? */
> > + if (EXT4_I(inode)->i_state & EXT4_STATE_XATTR) {
> > + struct ext4_iloc iloc;
> > + int offset; /* offset of xattr in inode */
> > +
> > + physical = iloc.bh->b_blocknr << blockbits;
> > + offset = EXT4_GOOD_OLD_INODE_SIZE +
> > + EXT4_I(inode)->i_extra_isize;
>
> @@ -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);

It seems a bit unpleasant to have both logical offsets be 0 in this case.
I'd suggest having the in-inode logical offset be "offset", and the external
EA logical offset be "s_inode_size" or "s_blocksize".

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.