| Message ID | 20191108052303.15052-1-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | xfs: remove XFS_IOC_FSSETDM and XFS_IOC_FSSETDM_BY_HANDLE | expand |
On Fri, Nov 08, 2019 at 06:23:03AM +0100, Christoph Hellwig wrote: > Thes ioctls set DMAPI specific flags in the on-disk inode, but there is > no way to actually ever query those flags. The only known user is > xfsrestore with the -D option, which is documented to be only useful > inside a DMAPI enviroment, which isn't supported by upstream XFS. Hmm, shouldn't we deprecate this at least for one or two releases? Even if it's functionally pointless... --D > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > fs/xfs/xfs_ioctl.c | 94 -------------------------------------------- > fs/xfs/xfs_ioctl.h | 6 --- > fs/xfs/xfs_ioctl32.c | 40 ------------------- > fs/xfs/xfs_ioctl32.h | 9 ----- > 4 files changed, 149 deletions(-) > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > index 287f83eb791f..a979d730203a 100644 > --- a/fs/xfs/xfs_ioctl.c > +++ b/fs/xfs/xfs_ioctl.c > @@ -291,82 +291,6 @@ xfs_readlink_by_handle( > return error; > } > > -int > -xfs_set_dmattrs( > - xfs_inode_t *ip, > - uint evmask, > - uint16_t state) > -{ > - xfs_mount_t *mp = ip->i_mount; > - xfs_trans_t *tp; > - int error; > - > - if (!capable(CAP_SYS_ADMIN)) > - return -EPERM; > - > - if (XFS_FORCED_SHUTDOWN(mp)) > - return -EIO; > - > - error = xfs_trans_alloc(mp, &M_RES(mp)->tr_ichange, 0, 0, 0, &tp); > - if (error) > - return error; > - > - xfs_ilock(ip, XFS_ILOCK_EXCL); > - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); > - > - ip->i_d.di_dmevmask = evmask; > - ip->i_d.di_dmstate = state; > - > - xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); > - error = xfs_trans_commit(tp); > - > - return error; > -} > - > -STATIC int > -xfs_fssetdm_by_handle( > - struct file *parfilp, > - void __user *arg) > -{ > - int error; > - struct fsdmidata fsd; > - xfs_fsop_setdm_handlereq_t dmhreq; > - struct dentry *dentry; > - > - if (!capable(CAP_MKNOD)) > - return -EPERM; > - if (copy_from_user(&dmhreq, arg, sizeof(xfs_fsop_setdm_handlereq_t))) > - return -EFAULT; > - > - error = mnt_want_write_file(parfilp); > - if (error) > - return error; > - > - dentry = xfs_handlereq_to_dentry(parfilp, &dmhreq.hreq); > - if (IS_ERR(dentry)) { > - mnt_drop_write_file(parfilp); > - return PTR_ERR(dentry); > - } > - > - if (IS_IMMUTABLE(d_inode(dentry)) || IS_APPEND(d_inode(dentry))) { > - error = -EPERM; > - goto out; > - } > - > - if (copy_from_user(&fsd, dmhreq.data, sizeof(fsd))) { > - error = -EFAULT; > - goto out; > - } > - > - error = xfs_set_dmattrs(XFS_I(d_inode(dentry)), fsd.fsd_dmevmask, > - fsd.fsd_dmstate); > - > - out: > - mnt_drop_write_file(parfilp); > - dput(dentry); > - return error; > -} > - > STATIC int > xfs_attrlist_by_handle( > struct file *parfilp, > @@ -2126,22 +2050,6 @@ xfs_file_ioctl( > case XFS_IOC_SETXFLAGS: > return xfs_ioc_setxflags(ip, filp, arg); > > - case XFS_IOC_FSSETDM: { > - struct fsdmidata dmi; > - > - if (copy_from_user(&dmi, arg, sizeof(dmi))) > - return -EFAULT; > - > - error = mnt_want_write_file(filp); > - if (error) > - return error; > - > - error = xfs_set_dmattrs(ip, dmi.fsd_dmevmask, > - dmi.fsd_dmstate); > - mnt_drop_write_file(filp); > - return error; > - } > - > case XFS_IOC_GETBMAP: > case XFS_IOC_GETBMAPA: > case XFS_IOC_GETBMAPX: > @@ -2169,8 +2077,6 @@ xfs_file_ioctl( > return -EFAULT; > return xfs_open_by_handle(filp, &hreq); > } > - case XFS_IOC_FSSETDM_BY_HANDLE: > - return xfs_fssetdm_by_handle(filp, arg); > > case XFS_IOC_READLINK_BY_HANDLE: { > xfs_fsop_handlereq_t hreq; > diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h > index 25ef178cbb74..420bd95dc326 100644 > --- a/fs/xfs/xfs_ioctl.h > +++ b/fs/xfs/xfs_ioctl.h > @@ -70,12 +70,6 @@ xfs_file_compat_ioctl( > unsigned int cmd, > unsigned long arg); > > -extern int > -xfs_set_dmattrs( > - struct xfs_inode *ip, > - uint evmask, > - uint16_t state); > - > struct xfs_ibulk; > struct xfs_bstat; > struct xfs_inogrp; > diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c > index 3c0d518e1039..c4c4f09113d3 100644 > --- a/fs/xfs/xfs_ioctl32.c > +++ b/fs/xfs/xfs_ioctl32.c > @@ -500,44 +500,6 @@ xfs_compat_attrmulti_by_handle( > return error; > } > > -STATIC int > -xfs_compat_fssetdm_by_handle( > - struct file *parfilp, > - void __user *arg) > -{ > - int error; > - struct fsdmidata fsd; > - compat_xfs_fsop_setdm_handlereq_t dmhreq; > - struct dentry *dentry; > - > - if (!capable(CAP_MKNOD)) > - return -EPERM; > - if (copy_from_user(&dmhreq, arg, > - sizeof(compat_xfs_fsop_setdm_handlereq_t))) > - return -EFAULT; > - > - dentry = xfs_compat_handlereq_to_dentry(parfilp, &dmhreq.hreq); > - if (IS_ERR(dentry)) > - return PTR_ERR(dentry); > - > - if (IS_IMMUTABLE(d_inode(dentry)) || IS_APPEND(d_inode(dentry))) { > - error = -EPERM; > - goto out; > - } > - > - if (copy_from_user(&fsd, compat_ptr(dmhreq.data), sizeof(fsd))) { > - error = -EFAULT; > - goto out; > - } > - > - error = xfs_set_dmattrs(XFS_I(d_inode(dentry)), fsd.fsd_dmevmask, > - fsd.fsd_dmstate); > - > -out: > - dput(dentry); > - return error; > -} > - > long > xfs_file_compat_ioctl( > struct file *filp, > @@ -646,8 +608,6 @@ xfs_file_compat_ioctl( > return xfs_compat_attrlist_by_handle(filp, arg); > case XFS_IOC_ATTRMULTI_BY_HANDLE_32: > return xfs_compat_attrmulti_by_handle(filp, arg); > - case XFS_IOC_FSSETDM_BY_HANDLE_32: > - return xfs_compat_fssetdm_by_handle(filp, arg); > default: > /* try the native version */ > return xfs_file_ioctl(filp, cmd, (unsigned long)arg); > diff --git a/fs/xfs/xfs_ioctl32.h b/fs/xfs/xfs_ioctl32.h > index 7985344d3aa6..7cbd6a0ee3e9 100644 > --- a/fs/xfs/xfs_ioctl32.h > +++ b/fs/xfs/xfs_ioctl32.h > @@ -143,15 +143,6 @@ typedef struct compat_xfs_fsop_attrmulti_handlereq { > #define XFS_IOC_ATTRMULTI_BY_HANDLE_32 \ > _IOW('X', 123, struct compat_xfs_fsop_attrmulti_handlereq) > > -typedef struct compat_xfs_fsop_setdm_handlereq { > - struct compat_xfs_fsop_handlereq hreq; /* handle information */ > - /* ptr to struct fsdmidata */ > - compat_uptr_t data; /* DMAPI data */ > -} compat_xfs_fsop_setdm_handlereq_t; > - > -#define XFS_IOC_FSSETDM_BY_HANDLE_32 \ > - _IOW('X', 121, struct compat_xfs_fsop_setdm_handlereq) > - > #ifdef BROKEN_X86_ALIGNMENT > /* on ia32 l_start is on a 32-bit boundary */ > typedef struct compat_xfs_flock64 { > -- > 2.20.1 >
On Thu, Nov 07, 2019 at 10:48:01PM -0800, Darrick J. Wong wrote: > On Fri, Nov 08, 2019 at 06:23:03AM +0100, Christoph Hellwig wrote: > > Thes ioctls set DMAPI specific flags in the on-disk inode, but there is > > no way to actually ever query those flags. The only known user is > > xfsrestore with the -D option, which is documented to be only useful > > inside a DMAPI enviroment, which isn't supported by upstream XFS. > > Hmm, shouldn't we deprecate this at least for one or two releases? > > Even if it's functionally pointless... It sets a value we can't even retreive. Not sure what the deprecation would help with.
On Fri, Nov 08, 2019 at 07:50:32AM +0100, Christoph Hellwig wrote: > On Thu, Nov 07, 2019 at 10:48:01PM -0800, Darrick J. Wong wrote: > > On Fri, Nov 08, 2019 at 06:23:03AM +0100, Christoph Hellwig wrote: > > > Thes ioctls set DMAPI specific flags in the on-disk inode, but there is > > > no way to actually ever query those flags. The only known user is > > > xfsrestore with the -D option, which is documented to be only useful > > > inside a DMAPI enviroment, which isn't supported by upstream XFS. > > > > Hmm, shouldn't we deprecate this at least for one or two releases? > > > > Even if it's functionally pointless... > > It sets a value we can't even retreive. Not sure what the deprecation > would help with. Dotting i's and crossing t's. IOWs, not getting ourselves yelled at for killing something without any warning / bureaucracy. :/ --D
On Thu, Nov 07, 2019 at 11:49:07PM -0800, Darrick J. Wong wrote: > On Fri, Nov 08, 2019 at 07:50:32AM +0100, Christoph Hellwig wrote: > > On Thu, Nov 07, 2019 at 10:48:01PM -0800, Darrick J. Wong wrote: > > > On Fri, Nov 08, 2019 at 06:23:03AM +0100, Christoph Hellwig wrote: > > > > Thes ioctls set DMAPI specific flags in the on-disk inode, but there is > > > > no way to actually ever query those flags. The only known user is > > > > xfsrestore with the -D option, which is documented to be only useful > > > > inside a DMAPI enviroment, which isn't supported by upstream XFS. > > > > > > Hmm, shouldn't we deprecate this at least for one or two releases? > > > > > > Even if it's functionally pointless... > > > > It sets a value we can't even retreive. Not sure what the deprecation > > would help with. > > Dotting i's and crossing t's. IOWs, not getting ourselves yelled at for > killing something without any warning / bureaucracy. :/ OTOH Dave said the same thing, so: Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> --D > --D
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c index 287f83eb791f..a979d730203a 100644 --- a/fs/xfs/xfs_ioctl.c +++ b/fs/xfs/xfs_ioctl.c @@ -291,82 +291,6 @@ xfs_readlink_by_handle( return error; } -int -xfs_set_dmattrs( - xfs_inode_t *ip, - uint evmask, - uint16_t state) -{ - xfs_mount_t *mp = ip->i_mount; - xfs_trans_t *tp; - int error; - - if (!capable(CAP_SYS_ADMIN)) - return -EPERM; - - if (XFS_FORCED_SHUTDOWN(mp)) - return -EIO; - - error = xfs_trans_alloc(mp, &M_RES(mp)->tr_ichange, 0, 0, 0, &tp); - if (error) - return error; - - xfs_ilock(ip, XFS_ILOCK_EXCL); - xfs_trans_ijoin(tp, ip, XFS_ILOCK_EXCL); - - ip->i_d.di_dmevmask = evmask; - ip->i_d.di_dmstate = state; - - xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE); - error = xfs_trans_commit(tp); - - return error; -} - -STATIC int -xfs_fssetdm_by_handle( - struct file *parfilp, - void __user *arg) -{ - int error; - struct fsdmidata fsd; - xfs_fsop_setdm_handlereq_t dmhreq; - struct dentry *dentry; - - if (!capable(CAP_MKNOD)) - return -EPERM; - if (copy_from_user(&dmhreq, arg, sizeof(xfs_fsop_setdm_handlereq_t))) - return -EFAULT; - - error = mnt_want_write_file(parfilp); - if (error) - return error; - - dentry = xfs_handlereq_to_dentry(parfilp, &dmhreq.hreq); - if (IS_ERR(dentry)) { - mnt_drop_write_file(parfilp); - return PTR_ERR(dentry); - } - - if (IS_IMMUTABLE(d_inode(dentry)) || IS_APPEND(d_inode(dentry))) { - error = -EPERM; - goto out; - } - - if (copy_from_user(&fsd, dmhreq.data, sizeof(fsd))) { - error = -EFAULT; - goto out; - } - - error = xfs_set_dmattrs(XFS_I(d_inode(dentry)), fsd.fsd_dmevmask, - fsd.fsd_dmstate); - - out: - mnt_drop_write_file(parfilp); - dput(dentry); - return error; -} - STATIC int xfs_attrlist_by_handle( struct file *parfilp, @@ -2126,22 +2050,6 @@ xfs_file_ioctl( case XFS_IOC_SETXFLAGS: return xfs_ioc_setxflags(ip, filp, arg); - case XFS_IOC_FSSETDM: { - struct fsdmidata dmi; - - if (copy_from_user(&dmi, arg, sizeof(dmi))) - return -EFAULT; - - error = mnt_want_write_file(filp); - if (error) - return error; - - error = xfs_set_dmattrs(ip, dmi.fsd_dmevmask, - dmi.fsd_dmstate); - mnt_drop_write_file(filp); - return error; - } - case XFS_IOC_GETBMAP: case XFS_IOC_GETBMAPA: case XFS_IOC_GETBMAPX: @@ -2169,8 +2077,6 @@ xfs_file_ioctl( return -EFAULT; return xfs_open_by_handle(filp, &hreq); } - case XFS_IOC_FSSETDM_BY_HANDLE: - return xfs_fssetdm_by_handle(filp, arg); case XFS_IOC_READLINK_BY_HANDLE: { xfs_fsop_handlereq_t hreq; diff --git a/fs/xfs/xfs_ioctl.h b/fs/xfs/xfs_ioctl.h index 25ef178cbb74..420bd95dc326 100644 --- a/fs/xfs/xfs_ioctl.h +++ b/fs/xfs/xfs_ioctl.h @@ -70,12 +70,6 @@ xfs_file_compat_ioctl( unsigned int cmd, unsigned long arg); -extern int -xfs_set_dmattrs( - struct xfs_inode *ip, - uint evmask, - uint16_t state); - struct xfs_ibulk; struct xfs_bstat; struct xfs_inogrp; diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c index 3c0d518e1039..c4c4f09113d3 100644 --- a/fs/xfs/xfs_ioctl32.c +++ b/fs/xfs/xfs_ioctl32.c @@ -500,44 +500,6 @@ xfs_compat_attrmulti_by_handle( return error; } -STATIC int -xfs_compat_fssetdm_by_handle( - struct file *parfilp, - void __user *arg) -{ - int error; - struct fsdmidata fsd; - compat_xfs_fsop_setdm_handlereq_t dmhreq; - struct dentry *dentry; - - if (!capable(CAP_MKNOD)) - return -EPERM; - if (copy_from_user(&dmhreq, arg, - sizeof(compat_xfs_fsop_setdm_handlereq_t))) - return -EFAULT; - - dentry = xfs_compat_handlereq_to_dentry(parfilp, &dmhreq.hreq); - if (IS_ERR(dentry)) - return PTR_ERR(dentry); - - if (IS_IMMUTABLE(d_inode(dentry)) || IS_APPEND(d_inode(dentry))) { - error = -EPERM; - goto out; - } - - if (copy_from_user(&fsd, compat_ptr(dmhreq.data), sizeof(fsd))) { - error = -EFAULT; - goto out; - } - - error = xfs_set_dmattrs(XFS_I(d_inode(dentry)), fsd.fsd_dmevmask, - fsd.fsd_dmstate); - -out: - dput(dentry); - return error; -} - long xfs_file_compat_ioctl( struct file *filp, @@ -646,8 +608,6 @@ xfs_file_compat_ioctl( return xfs_compat_attrlist_by_handle(filp, arg); case XFS_IOC_ATTRMULTI_BY_HANDLE_32: return xfs_compat_attrmulti_by_handle(filp, arg); - case XFS_IOC_FSSETDM_BY_HANDLE_32: - return xfs_compat_fssetdm_by_handle(filp, arg); default: /* try the native version */ return xfs_file_ioctl(filp, cmd, (unsigned long)arg); diff --git a/fs/xfs/xfs_ioctl32.h b/fs/xfs/xfs_ioctl32.h index 7985344d3aa6..7cbd6a0ee3e9 100644 --- a/fs/xfs/xfs_ioctl32.h +++ b/fs/xfs/xfs_ioctl32.h @@ -143,15 +143,6 @@ typedef struct compat_xfs_fsop_attrmulti_handlereq { #define XFS_IOC_ATTRMULTI_BY_HANDLE_32 \ _IOW('X', 123, struct compat_xfs_fsop_attrmulti_handlereq) -typedef struct compat_xfs_fsop_setdm_handlereq { - struct compat_xfs_fsop_handlereq hreq; /* handle information */ - /* ptr to struct fsdmidata */ - compat_uptr_t data; /* DMAPI data */ -} compat_xfs_fsop_setdm_handlereq_t; - -#define XFS_IOC_FSSETDM_BY_HANDLE_32 \ - _IOW('X', 121, struct compat_xfs_fsop_setdm_handlereq) - #ifdef BROKEN_X86_ALIGNMENT /* on ia32 l_start is on a 32-bit boundary */ typedef struct compat_xfs_flock64 {
Thes ioctls set DMAPI specific flags in the on-disk inode, but there is no way to actually ever query those flags. The only known user is xfsrestore with the -D option, which is documented to be only useful inside a DMAPI enviroment, which isn't supported by upstream XFS. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/xfs/xfs_ioctl.c | 94 -------------------------------------------- fs/xfs/xfs_ioctl.h | 6 --- fs/xfs/xfs_ioctl32.c | 40 ------------------- fs/xfs/xfs_ioctl32.h | 9 ----- 4 files changed, 149 deletions(-)