Re: [RFC][PATCH 0/5] Fiemap, an extent mapping ioctl

[Christoph Hellwig <hch@infradead.org>]

On Mon, May 26, 2008 at 04:59:44AM -0600, Andreas Dilger wrote:
> > A LUN doesn't make any sense in filesystem context.  That's a
> > scsi-centric acronym that doesn't even make sense in a scsi-centric
> > filesystem universe because a LUN can of course contain multiple
> > partitions.  It's also extremly ill-defined when using volume managers.
> 
> What else do you propose calling this?  It isn't a LUN in the SCSI sense
> of course, but there is definitely a need to be able to identify multiple
> disks.  Regardless of whether there is a single disk or multiple disks
> involved, it is generally called a LUN.  It is a better than calling it
> a "disk" or a "partition".

See below because the naming really depends on defining the semantics
of the this field.

> I don't see why we need a different ioctl for mapping extents on a
> filesystem that support direct access to multiple disks.  Having one
> mechanism that returns the file mapping is much more simple for user
> space applications (filefrag, cp, tar, gzip, etc) than having to use
> different ioctls for different backing filesystems.

Well, we could add a dev field that contains the dev_t for the
underlying block device.  That would work for the current XFS realtime
device aswell as for my work to map different XFS AGs to different
devices.  It wouldn't work for btrfs with integrated raid code where
a single extent can span multiple underlying devices, the same probably
applies to pnfs.

> > Why is this passes a structure instead of individual arguments?
> 
> Saves on passing this around as arguments on the stack?  Also, for ext4
> there is an iterator function which needs a private data struct passed,
> and it doesn't make sense to require duplicating all of this information
> again.

Ok.

> > > If the request has the FIEMAP_FLAG_NUM_EXTENTS flag set, then calling
> > > this helper is not necessary and fi_extents_mapped can be set
> > > directly.
> > 
> > Sounds like the count number of extents request should be a separate
> > ioctl and separate filesystem entry point instead of overloading FIEMAP.
> 
> I don't see that at all.  The operations that the filesystem has to do
> are basically the same whether it is counting extents or returning them.
> All that would result from having separate ioctl and filesystem methods
> would be a lot of code duplication.
> 
> The fiemap_fill_next_extents() call will handle the NUM_EXTENTS operation
> internally, and the filesystem code doesn't need to special case this
> at all.  The only time the NUM_EXTENTS case would be handled by the
> filesystem specially would be if it tracks the count of extents itself
> for some reason.

It just special cases it.  As does for example the ext4 handler.  Please
keep the API simple instead of overloading it already from the start.
And when you look at the ext4 implementation with the extent walker it's
pretty simple to implement a fiecount by having a second callback with
a trivial shared helper.