From mboxrd@z Thu Jan  1 00:00:00 1970
From: Eric Sandeen <sandeen@redhat.com>
Subject: Re: [PATCH 4/5] ext4: implement ext4_fiemap
Date: Sun, 25 May 2008 10:26:34 -0500
Message-ID: <4839852A.5040902@redhat.com>
References: <20080525000224.GN8325@wotan.suse.de> <20080525075139.GJ3516@webber.adilger.int>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Cc: Mark Fasheh <mfasheh@suse.com>, linux-fsdevel@vger.kernel.org
To: Andreas Dilger <adilger@sun.com>
Return-path: <linux-fsdevel-owner@vger.kernel.org>
Received: from mx1.redhat.com ([66.187.233.31]:54685 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1753252AbYEYP0m (ORCPT <rfc822;linux-fsdevel@vger.kernel.org>);
	Sun, 25 May 2008 11:26:42 -0400
In-Reply-To: <20080525075139.GJ3516@webber.adilger.int>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

Andreas Dilger wrote:
> On May 24, 2008  17:02 -0700, Mark Fasheh wrote:
>> +#define EXT4_FIEMAP_FLAG_INCOMPAT_UNSUPP (FIEMAP_FLAG_INCOMPAT &	\
>> +					  ~(FIEMAP_FLAG_LUN_OFFSET))
> 
> Per discussion with Eric, this needs to be changed:
> 
> #define EXT4_FIEMAP_FLAG_SUPP (FIEMAP_FLAG_NUM_EXTENTS | FIEMAP_FLAG_SYNC | \
> 			       FIEMAP_FLAG_HSM_READ | FIEMAP_FLAG_LUN_ORDER)
> 

Yep, I agree with this and the others, probably should have put "proof
of concept" in the original message; I knew it had warts but didn't want
to hold up Mark's RFE.

Thanks Andreas,

-Eric

>> +struct fiemap_internal {
>> +	struct fiemap		*fiemap_s;
>> +	struct fiemap_extent	fm_extent;
>> +	char			*cur_ext_ptr;
>> +	int			err;
>> +};
> 
> This is no longer used and can be removed (now uses fiemap_extent_info).
> 
>> +/*
>> + * Callback function called for each extent to gather FIEMAP information.
>> + */
>> +int ext4_ext_fiemap_cb(struct inode *inode, struct ext4_ext_path *path,
>> +		       struct ext4_ext_cache *newex, struct ext4_extent *ex,
>> +		       void *data)
>> +{
>> +	/* Just counting extents? */
>> +	if (fieinfo->fi_flags & FIEMAP_FLAG_NUM_EXTENTS) {
>> +		fieinfo->fi_extents_mapped++;
>> +		return EXT_CONTINUE;
>> +	}
> 
> This isn't needed anymore, fiemap_fill_next_extent() handles it.
> 
> 
>> +int ext4_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>> +		__u64 start, __u64 len)
>> +{
>> +	/* fallback to generic here if not extents */
>> +#warning fix bitmap fallback
>> +	if (!(EXT4_I(inode)->i_flags & EXT4_EXTENTS_FL))
>> +		return -EOPNOTSUPP;
> 
> 	if (!(EXT4_I(inode)->i_flags & EXT4_EXTENTS_FL))
> 		return generic_block_fiemap(inode, fieinfo, start, len,
> 					    ext4_get_block);
> 
>> +#if 0
>> +	/* bail on unsupported flags for this fs */
>> +	if (fiemap_s->fm_flags & EXT4_FIEMAP_FLAG_INCOMPAT_UNSUPP)
>> +		return -EOPNOTSUPP;
>> +#endif
> 
> This should be reinstated now that we have come to a decision on the flags:
> 
> 	if (fieinfo->fi_flags & ~EXT4_FIEMAP_FLAG_SUPP) {
> 		fieinfo->fi_flags &= ~EXT4_FIEMAP_FLAG_SUPP;
> 		return -EBADF;
> 	}
> 
> 
> Cheers, Andreas
> --
> Andreas Dilger
> Sr. Staff Engineer, Lustre Group
> Sun Microsystems of Canada, Inc.
>