On Wed, Aug 27, 2014 at 09:22:53PM -0700, Iustin Pop wrote:
> Currently, the ioctl handling code for XFS_IOC_FSSETXATTR treats all
> targets as regular files: it refuses to change the extent size if
> extents are allocated. This is wrong for directories, as there the
> extent size is only used as a default for children.
>
> The patch fixes this by only checking for allocated extents for regular
> files; it leaves undefined what it means to set an extent size on a
> non-regular, non-directory inode. Additionally, when setting a non-zero
> extent size, the appropriate flags (EXTSIZE, respectively EXTSZINHERIT)
> are enforced for regular files and directories.
>
> Signed-off-by: Iustin Pop <iusty@xxxxxxxxx>
> ---
> A patch against xfstests to test for the fixed behaviour will follow
> shortly.
>
> fs/xfs/xfs_ioctl.c | 34 ++++++++++++++++++++++++++--------
> 1 file changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 8bc1bbc..5b9acd2 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -1116,14 +1116,32 @@ xfs_ioctl_setattr(
> }
>
> if (mask & FSX_EXTSIZE) {
> - /*
> - * Can't change extent size if any extents are allocated.
> - */
> - if (ip->i_d.di_nextents &&
> - ((ip->i_d.di_extsize << mp->m_sb.sb_blocklog) !=
> - fa->fsx_extsize)) {
> - code = XFS_ERROR(EINVAL); /* EFBIG? */
> - goto error_return;
Doesn't apply to a current 3.17-rc1 tree. Can you update the patch?
> + if (S_ISDIR(ip->i_d.di_mode)) {
> + /*
> + * Enforce setting the EXTSZINHERIT flag when
> + * a non-zero extent size has been specified.
> + */
> + if (fa->fsx_extsize) {
> + fa->fsx_xflags |= XFS_XFLAG_EXTSZINHERIT;
> + }
> + } else if (S_ISREG(ip->i_d.di_mode)) {
> + /*
> + * For a regular file, we can't change extent
> + * size if any extents are allocated.
> + */
> + if (ip->i_d.di_nextents &&
> + ((ip->i_d.di_extsize << mp->m_sb.sb_blocklog) !=
> + fa->fsx_extsize)) {
> + code = XFS_ERROR(EINVAL); /* EFBIG? */
> + goto error_return;
> + }
> + /*
> + * Enforce setting the EXTSIZE flag when
> + * a non-zero extent size has been specified.
> + */
> + if (fa->fsx_extsize) {
> + fa->fsx_xflags |= XFS_XFLAG_EXTSIZE;
> + }
> }
Hmmmm. That's not validating/enforcing the correct use of
XFS_XFLAG_EXTSIZE or XFS_XFLAG_EXTSZINHERIT, that's setting it
implicitly based on the type of inode.
If we are going to enforce this properly, then XFS_XFLAG_EXTSIZE is
only valid for a regular file, and XFS_XFLAG_EXTSZINHERIT is only
valid on a directory, and the flags on th einode should only be set
if the hint is not zero. i.e:
if (mask & FSX_EXTSIZE) {
error = -EINVAL;
/* validate the flags are set appropriately */
if ((fa->fsx_xflags & XFS_XFLAG_EXTSIZE) &&
!S_ISREG(ip->i_d.di_mode))
goto error_return;
if ((fa->fsx_xflags & XFS_XFLAG_EXTSZINHERIT) &&
!S_ISDIR(ip->i_d.di_mode))
goto error_return;
/* Can't change extent size on regular files with allocated extents */
if (S_ISREG(ip->i_d.di_mode) && ip->i_d.di_nextents &&
((ip->i_d.di_extsize << mp->m_sb.sb_blocklog) !=
fa->fsx_extsize))
goto error_return;
/* if the extent size is zero, clear the inode flags */
if (fs->fsx_extsize == 0)
fa->fsx_xflags &= ~(XFS_XFLAG_EXTSIZE | XFS_XFLAG_EXTSZINHERIT);
} else {
/* existing size check code */
....
}
}
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
