xfs: remove XFS_IOC_FSSETDM and XFS_IOC_FSSETDM_BY_HANDLE
diff mbox series

Message ID 20191108052303.15052-1-hch@lst.de
State Accepted
Headers show
Series
  • xfs: remove XFS_IOC_FSSETDM and XFS_IOC_FSSETDM_BY_HANDLE
Related show

Commit Message

Christoph Hellwig Nov. 8, 2019, 5:23 a.m. UTC
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(-)

Comments

Darrick J. Wong Nov. 8, 2019, 6:48 a.m. UTC | #1
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
>
Christoph Hellwig Nov. 8, 2019, 6:50 a.m. UTC | #2
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.
Darrick J. Wong Nov. 8, 2019, 7:49 a.m. UTC | #3
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
Darrick J. Wong Nov. 13, 2019, 5:09 a.m. UTC | #4
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

Patch
diff mbox series

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 {