On Fri, Oct 07, 2016 at 04:08:03PM -0700, Darrick J. Wong wrote: > Since we can only turn on the rt flag if there are no data extents, > we can safely turn off the reflink flag if the rt flag is being > turned on. > > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx> > Reported-by: Brian Foster <bfoster@xxxxxxxxxx> > --- > fs/xfs/xfs_ioctl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > index 8b9f31c5..598b97b 100644 > --- a/fs/xfs/xfs_ioctl.c > +++ b/fs/xfs/xfs_ioctl.c > @@ -1034,9 +1034,9 @@ xfs_ioctl_setattr_xflags( > return -EINVAL; > } > > - /* Don't allow us to set realtime mode for a reflinked file. */ > + /* Clear reflink if we are actually able to set the rt flag. */ > if ((fa->fsx_xflags & FS_XFLAG_REALTIME) && xfs_is_reflink_inode(ip)) > - return -EINVAL; > + ip->i_d.di_flags2 &= ~XFS_DIFLAG2_REFLINK; > > /* Don't allow us to set DAX mode for a reflinked file for now. */ > if ((fa->fsx_xflags & FS_XFLAG_DAX) && xfs_is_reflink_inode(ip)) This seems fine to me, but don't we still have the original problem in the code that shortly follows with regard to the DAX flag? In other words, the fundamental issue was that we clear the reflink flag lazily (which by itself is perfectly fine), but we have a few spots where we trust that the inode absolutely has shared extents when the flag is set. That also might be fine logic in certain contexts, but in these couple cases we fail requested operations from the user (setting RT, DAX, etc.). It's not a big deal in that I suppose one can always run an unshare fallocate, but just could be confusing if somebody is aware of the already unshared state of a file. Could we invoke xfs_reflink_clear_inode_flag() somewhere earlier in this path if the reflink flag is set and any of the conflicting flags have been requested by the user? Brian > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
