Message ID | 20191011130316.13373-2-hch@lst.de (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
Series | [1/2] xfs: disable xfs_ioc_space for always COW inodes | expand |
On Fri, Oct 11, 2019 at 03:03:15PM +0200, Christoph Hellwig wrote: > If we always have to write out of place preallocating blocks is > pointless. We already check for this in the normal falloc path, but > the check was missig in the legacy ALLOCSP path. This function handles other things than preallocation, such as XFS_IOC_ZERO_RANGE and XFS_IOC_UNRESVSP, which call xfs_zero_file_space and xfs_free_file_space, respectively. We don't prohibit fallocate from calling those two functions on an always_cow inode, so why do that here? --D > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/xfs/xfs_ioctl.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > index d58f0d6a699e..abf7a102376f 100644 > --- a/fs/xfs/xfs_ioctl.c > +++ b/fs/xfs/xfs_ioctl.c > @@ -33,6 +33,7 @@ > #include "xfs_sb.h" > #include "xfs_ag.h" > #include "xfs_health.h" > +#include "xfs_reflink.h" > > #include <linux/mount.h> > #include <linux/namei.h> > @@ -607,6 +608,9 @@ xfs_ioc_space( > if (!S_ISREG(inode->i_mode)) > return -EINVAL; > > + if (xfs_is_always_cow_inode(ip)) > + return -EOPNOTSUPP; > + > if (filp->f_flags & O_DSYNC) > flags |= XFS_PREALLOC_SYNC; > if (filp->f_mode & FMODE_NOCMTIME) > -- > 2.20.1 >
On Fri, Oct 11, 2019 at 05:29:54PM -0700, Darrick J. Wong wrote: > On Fri, Oct 11, 2019 at 03:03:15PM +0200, Christoph Hellwig wrote: > > If we always have to write out of place preallocating blocks is > > pointless. We already check for this in the normal falloc path, but > > the check was missig in the legacy ALLOCSP path. > > This function handles other things than preallocation, such as > XFS_IOC_ZERO_RANGE and XFS_IOC_UNRESVSP, which call xfs_zero_file_space > and xfs_free_file_space, respectively. We don't prohibit fallocate > from calling those two functions on an always_cow inode, so why do that > here? True. I actually have a patch in my tree that switches those to be handled in the core so that they enter XFS through ->fallocate. It didn't make any sense to send this patch before that other change, sorry.
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c index d58f0d6a699e..abf7a102376f 100644 --- a/fs/xfs/xfs_ioctl.c +++ b/fs/xfs/xfs_ioctl.c @@ -33,6 +33,7 @@ #include "xfs_sb.h" #include "xfs_ag.h" #include "xfs_health.h" +#include "xfs_reflink.h" #include <linux/mount.h> #include <linux/namei.h> @@ -607,6 +608,9 @@ xfs_ioc_space( if (!S_ISREG(inode->i_mode)) return -EINVAL; + if (xfs_is_always_cow_inode(ip)) + return -EOPNOTSUPP; + if (filp->f_flags & O_DSYNC) flags |= XFS_PREALLOC_SYNC; if (filp->f_mode & FMODE_NOCMTIME)
If we always have to write out of place preallocating blocks is pointless. We already check for this in the normal falloc path, but the check was missig in the legacy ALLOCSP path. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_ioctl.c | 4 ++++ 1 file changed, 4 insertions(+)