[v5,13/17] xfs: mount api - add xfs_reconfigure()
diff mbox series

Message ID 157062067944.32346.8228418435930532076.stgit@fedora-28
State New
Headers show
Series
  • xfs: mount API patch series
Related show

Commit Message

Ian Kent Oct. 9, 2019, 11:31 a.m. UTC
Add the fs_context_operations method .reconfigure that performs
remount validation as previously done by the super_operations
.remount_fs method.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/xfs/xfs_super.c |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

Comments

Brian Foster Oct. 9, 2019, 12:51 p.m. UTC | #1
On Wed, Oct 09, 2019 at 07:31:19PM +0800, Ian Kent wrote:
> Add the fs_context_operations method .reconfigure that performs
> remount validation as previously done by the super_operations
> .remount_fs method.
> 
> Signed-off-by: Ian Kent <raven@themaw.net>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_super.c |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index 7e634706626b..230b0e2a184c 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1544,6 +1544,67 @@ xfs_fs_remount(
>  	return 0;
>  }
>  
> +/*
> + * Logically we would return an error here to prevent users
> + * from believing they might have changed mount options using
> + * remount which can't be changed.
> + *
> + * But unfortunately mount(8) adds all options from mtab and
> + * fstab to the mount arguments in some cases so we can't
> + * blindly reject options, but have to check for each specified
> + * option if it actually differs from the currently set option
> + * and only reject it if that's the case.
> + *
> + * Until that is implemented we return success for every remount
> + * request, and silently ignore all options that we can't actually
> + * change.
> + */
> +STATIC int
> +xfs_reconfigure(
> +	struct fs_context *fc)
> +{
> +	struct xfs_fs_context	*ctx = fc->fs_private;
> +	struct xfs_mount	*mp = XFS_M(fc->root->d_sb);
> +	struct xfs_mount        *new_mp = fc->s_fs_info;
> +	xfs_sb_t		*sbp = &mp->m_sb;
> +	int			flags = fc->sb_flags;
> +	int			error;
> +
> +	error = xfs_validate_params(new_mp, ctx, false);
> +	if (error)
> +		return error;
> +
> +	/* inode32 -> inode64 */
> +	if ((mp->m_flags & XFS_MOUNT_SMALL_INUMS) &&
> +	    !(new_mp->m_flags & XFS_MOUNT_SMALL_INUMS)) {
> +		mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
> +		mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
> +	}
> +
> +	/* inode64 -> inode32 */
> +	if (!(mp->m_flags & XFS_MOUNT_SMALL_INUMS) &&
> +	    (new_mp->m_flags & XFS_MOUNT_SMALL_INUMS)) {
> +		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
> +		mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
> +	}
> +
> +	/* ro -> rw */
> +	if ((mp->m_flags & XFS_MOUNT_RDONLY) && !(flags & SB_RDONLY)) {
> +		error = xfs_remount_rw(mp);
> +		if (error)
> +			return error;
> +	}
> +
> +	/* rw -> ro */
> +	if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (flags & SB_RDONLY)) {
> +		error = xfs_remount_ro(mp);
> +		if (error)
> +			return error;
> +	}
> +
> +	return 0;
> +}
> +
>  /*
>   * Second stage of a freeze. The data is already frozen so we only
>   * need to take care of the metadata. Once that's done sync the superblock
> @@ -2069,6 +2130,7 @@ static const struct super_operations xfs_super_operations = {
>  static const struct fs_context_operations xfs_context_ops = {
>  	.parse_param = xfs_parse_param,
>  	.get_tree    = xfs_get_tree,
> +	.reconfigure = xfs_reconfigure,
>  };
>  
>  static struct file_system_type xfs_fs_type = {
>
hch@infradead.org Oct. 9, 2019, 3:05 p.m. UTC | #2
> +/*
> + * Logically we would return an error here to prevent users
> + * from believing they might have changed mount options using
> + * remount which can't be changed.
> + *
> + * But unfortunately mount(8) adds all options from mtab and
> + * fstab to the mount arguments in some cases so we can't
> + * blindly reject options, but have to check for each specified
> + * option if it actually differs from the currently set option
> + * and only reject it if that's the case.
> + *
> + * Until that is implemented we return success for every remount
> + * request, and silently ignore all options that we can't actually
> + * change.
> + */

Please use up all 80 chars available for your comments.
Ian Kent Oct. 10, 2019, 1:12 a.m. UTC | #3
On Wed, 2019-10-09 at 08:05 -0700, Christoph Hellwig wrote:
> > +/*
> > + * Logically we would return an error here to prevent users
> > + * from believing they might have changed mount options using
> > + * remount which can't be changed.
> > + *
> > + * But unfortunately mount(8) adds all options from mtab and
> > + * fstab to the mount arguments in some cases so we can't
> > + * blindly reject options, but have to check for each specified
> > + * option if it actually differs from the currently set option
> > + * and only reject it if that's the case.
> > + *
> > + * Until that is implemented we return success for every remount
> > + * request, and silently ignore all options that we can't actually
> > + * change.
> > + */
> 
> Please use up all 80 chars available for your comments.

Will do.

Ian

Patch
diff mbox series

diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
index 7e634706626b..230b0e2a184c 100644
--- a/fs/xfs/xfs_super.c
+++ b/fs/xfs/xfs_super.c
@@ -1544,6 +1544,67 @@  xfs_fs_remount(
 	return 0;
 }
 
+/*
+ * Logically we would return an error here to prevent users
+ * from believing they might have changed mount options using
+ * remount which can't be changed.
+ *
+ * But unfortunately mount(8) adds all options from mtab and
+ * fstab to the mount arguments in some cases so we can't
+ * blindly reject options, but have to check for each specified
+ * option if it actually differs from the currently set option
+ * and only reject it if that's the case.
+ *
+ * Until that is implemented we return success for every remount
+ * request, and silently ignore all options that we can't actually
+ * change.
+ */
+STATIC int
+xfs_reconfigure(
+	struct fs_context *fc)
+{
+	struct xfs_fs_context	*ctx = fc->fs_private;
+	struct xfs_mount	*mp = XFS_M(fc->root->d_sb);
+	struct xfs_mount        *new_mp = fc->s_fs_info;
+	xfs_sb_t		*sbp = &mp->m_sb;
+	int			flags = fc->sb_flags;
+	int			error;
+
+	error = xfs_validate_params(new_mp, ctx, false);
+	if (error)
+		return error;
+
+	/* inode32 -> inode64 */
+	if ((mp->m_flags & XFS_MOUNT_SMALL_INUMS) &&
+	    !(new_mp->m_flags & XFS_MOUNT_SMALL_INUMS)) {
+		mp->m_flags &= ~XFS_MOUNT_SMALL_INUMS;
+		mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
+	}
+
+	/* inode64 -> inode32 */
+	if (!(mp->m_flags & XFS_MOUNT_SMALL_INUMS) &&
+	    (new_mp->m_flags & XFS_MOUNT_SMALL_INUMS)) {
+		mp->m_flags |= XFS_MOUNT_SMALL_INUMS;
+		mp->m_maxagi = xfs_set_inode_alloc(mp, sbp->sb_agcount);
+	}
+
+	/* ro -> rw */
+	if ((mp->m_flags & XFS_MOUNT_RDONLY) && !(flags & SB_RDONLY)) {
+		error = xfs_remount_rw(mp);
+		if (error)
+			return error;
+	}
+
+	/* rw -> ro */
+	if (!(mp->m_flags & XFS_MOUNT_RDONLY) && (flags & SB_RDONLY)) {
+		error = xfs_remount_ro(mp);
+		if (error)
+			return error;
+	}
+
+	return 0;
+}
+
 /*
  * Second stage of a freeze. The data is already frozen so we only
  * need to take care of the metadata. Once that's done sync the superblock
@@ -2069,6 +2130,7 @@  static const struct super_operations xfs_super_operations = {
 static const struct fs_context_operations xfs_context_ops = {
 	.parse_param = xfs_parse_param,
 	.get_tree    = xfs_get_tree,
+	.reconfigure = xfs_reconfigure,
 };
 
 static struct file_system_type xfs_fs_type = {