[Cluster-devel] [PATCH] gfs2: Fix initialisation of args for remount

[Cluster-devel] [PATCH] gfs2: Fix initialisation of args for remount

[Andrew Price]

When gfs2 was converted to use fs_context, the initialisation of the
mount args structure to the currently active args was lost with the
removal of gfs2_remount_fs(), so the checks of the new args on remount
became checks against the default values instead of the current ones.
This caused unexpected remount behaviour and test failures (xfstests
generic/294, generic/306 and generic/452).

Reinstate the args initialisation, this time in gfs2_init_fs_context()
and conditional upon fc->purpose, as that's the only time we get control
before the mount args are parsed in the remount process.

Fixes: 1f52aa08d12f ("gfs2: Convert gfs2 to fs_context")
Signed-off-by: Andrew Price <anprice@redhat.com>
---

 fs/gfs2/ops_fstype.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index dc61af2c4d5e..1f247a0ab6ef 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1544,13 +1544,18 @@ static int gfs2_init_fs_context(struct fs_context *fc)
 	if (args == NULL)
 		return -ENOMEM;
 
-	args->ar_quota = GFS2_QUOTA_DEFAULT;
-	args->ar_data = GFS2_DATA_DEFAULT;
-	args->ar_commit = 30;
-	args->ar_statfs_quantum = 30;
-	args->ar_quota_quantum = 60;
-	args->ar_errors = GFS2_ERRORS_DEFAULT;
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
+		struct gfs2_sbd *sdp = fc->root->d_sb->s_fs_info;
 
+		*args = sdp->sd_args;
+	} else {
+		args->ar_quota = GFS2_QUOTA_DEFAULT;
+		args->ar_data = GFS2_DATA_DEFAULT;
+		args->ar_commit = 30;
+		args->ar_statfs_quantum = 30;
+		args->ar_quota_quantum = 60;
+		args->ar_errors = GFS2_ERRORS_DEFAULT;
+	}
 	fc->fs_private = args;
 	fc->ops = &gfs2_context_ops;
 	return 0;
-- 
2.21.0

[Cluster-devel] [PATCH] gfs2: Fix initialisation of args for remount

[Andreas Gruenbacher]

On Tue, Oct 29, 2019 at 9:10 PM Andrew Price <anprice@redhat.com> wrote:
> When gfs2 was converted to use fs_context, the initialisation of the
> mount args structure to the currently active args was lost with the
> removal of gfs2_remount_fs(), so the checks of the new args on remount
> became checks against the default values instead of the current ones.
> This caused unexpected remount behaviour and test failures (xfstests
> generic/294, generic/306 and generic/452).
>
> Reinstate the args initialisation, this time in gfs2_init_fs_context()
> and conditional upon fc->purpose, as that's the only time we get control
> before the mount args are parsed in the remount process.
>
> Fixes: 1f52aa08d12f ("gfs2: Convert gfs2 to fs_context")
> Signed-off-by: Andrew Price <anprice@redhat.com>
> ---
>
>  fs/gfs2/ops_fstype.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index dc61af2c4d5e..1f247a0ab6ef 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -1544,13 +1544,18 @@ static int gfs2_init_fs_context(struct fs_context *fc)
>         if (args == NULL)
>                 return -ENOMEM;
>
> -       args->ar_quota = GFS2_QUOTA_DEFAULT;
> -       args->ar_data = GFS2_DATA_DEFAULT;
> -       args->ar_commit = 30;
> -       args->ar_statfs_quantum = 30;
> -       args->ar_quota_quantum = 60;
> -       args->ar_errors = GFS2_ERRORS_DEFAULT;
> +       if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
> +               struct gfs2_sbd *sdp = fc->root->d_sb->s_fs_info;
>
> +               *args = sdp->sd_args;

Since we're fully overwriting args here, shouldn't we use kmalloc to
allocate args above ...

> +       } else {

... and memset it to 0 in this case?

> +               args->ar_quota = GFS2_QUOTA_DEFAULT;
> +               args->ar_data = GFS2_DATA_DEFAULT;
> +               args->ar_commit = 30;
> +               args->ar_statfs_quantum = 30;
> +               args->ar_quota_quantum = 60;
> +               args->ar_errors = GFS2_ERRORS_DEFAULT;
> +       }
>         fc->fs_private = args;
>         fc->ops = &gfs2_context_ops;
>         return 0;
> --
> 2.21.0
>

Thanks,
Andreas

[Cluster-devel] [PATCH v2] gfs2: Fix initialisation of args for remount

[Andrew Price]

When gfs2 was converted to use fs_context, the initialisation of the
mount args structure to the currently active args was lost with the
removal of gfs2_remount_fs(), so the checks of the new args on remount
became checks against the default values instead of the current ones.
This caused unexpected remount behaviour and test failures (xfstests
generic/294, generic/306 and generic/452).

Reinstate the args initialisation, this time in gfs2_init_fs_context()
and conditional upon fc->purpose, as that's the only time we get control
before the mount args are parsed in the remount process.

Fixes: 1f52aa08d12f ("gfs2: Convert gfs2 to fs_context")
Signed-off-by: Andrew Price <anprice@redhat.com>
---

v2: Switch to kmalloc and only zero the args struct in the normal mount case. (Thanks, Andreas.)

 fs/gfs2/ops_fstype.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index dc61af2c4d5e..18daf494abab 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1540,17 +1540,23 @@ static int gfs2_init_fs_context(struct fs_context *fc)
 {
 	struct gfs2_args *args;
 
-	args = kzalloc(sizeof(*args), GFP_KERNEL);
+	args = kmalloc(sizeof(*args), GFP_KERNEL);
 	if (args == NULL)
 		return -ENOMEM;
 
-	args->ar_quota = GFS2_QUOTA_DEFAULT;
-	args->ar_data = GFS2_DATA_DEFAULT;
-	args->ar_commit = 30;
-	args->ar_statfs_quantum = 30;
-	args->ar_quota_quantum = 60;
-	args->ar_errors = GFS2_ERRORS_DEFAULT;
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
+		struct gfs2_sbd *sdp = fc->root->d_sb->s_fs_info;
 
+		*args = sdp->sd_args;
+	} else {
+		memset(args, 0, sizeof(*args));
+		args->ar_quota = GFS2_QUOTA_DEFAULT;
+		args->ar_data = GFS2_DATA_DEFAULT;
+		args->ar_commit = 30;
+		args->ar_statfs_quantum = 30;
+		args->ar_quota_quantum = 60;
+		args->ar_errors = GFS2_ERRORS_DEFAULT;
+	}
 	fc->fs_private = args;
 	fc->ops = &gfs2_context_ops;
 	return 0;
-- 
2.21.0