Re: [PATCH] xfs: Forbid FIEMAP on RT devices

[Carlos Maiolino <cmaiolino@redhat.com>]

On Tue, Nov 19, 2019 at 10:10:54AM -0600, Eric Sandeen wrote:
> On 11/19/19 9:44 AM, Carlos Maiolino wrote:
> > By now, FIEMAP users have no way to identify which device contains the
> > mapping being reported by the ioctl, so, let's forbid FIEMAP on RT
> > devices/files until FIEMAP can properly report the device containing the
> > returned mappings.
> 
> I'm not sure I agree with this - we don't return any device information with
> any mapping, ever. It's always up to the caller to understand what they have
> asked to have mapped. I'm not sure why RT files should be singled out, as long
> as the mapping is correct.

It's been a long time discussion already on this subject, it all started back
with btrfs and their virtual address spaces and how a fiemap should behave on
filesystems such as btrfs, which, by instance, the caller can't tell which
device the mapping refers to.

XFS RT device is the simplest of this kind of case, where the mapping returned
does not match with the mounted device, but by now that's the one blocking my
FIEMAP work, which I want to fix to move on to the next steps :P

> 
> -Eric
> 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> > 
> > Hi folks, this change has been previously suggested by Christoph while the
> > fibmap->fiemap work was being discussed on the last version [1] of that set.
> > And after some thought I do think RT devices shouldn't allow fiemap calls
> > either, giving the file blocks will actually be on a different device than that
> > displayed on /proc/mounts which can lead to erroneous assumptions.
> > 
> >  fs/xfs/xfs_iops.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> > index e532db27d0dc..ec7749cbd3ca 100644
> > --- a/fs/xfs/xfs_iops.c
> > +++ b/fs/xfs/xfs_iops.c
> > @@ -1138,6 +1138,9 @@ xfs_vn_fiemap(
> >  {
> >  	int			error;
> >  
> > +	if (XFS_IS_REALTIME_INODE(XFS_I(inode)))
> > +		return -EOPNOTSUPP;
> > +
> >  	xfs_ilock(XFS_I(inode), XFS_IOLOCK_SHARED);
> >  	if (fieinfo->fi_flags & FIEMAP_FLAG_XATTR) {
> >  		fieinfo->fi_flags &= ~FIEMAP_FLAG_XATTR;
> > 
> 

-- 
Carlos