linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] gfs2: Switch to the new mount API
@ 2019-03-19 16:04 Andrew Price
  2019-03-19 16:04 ` [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context Andrew Price
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Andrew Price @ 2019-03-19 16:04 UTC (permalink / raw)
  To: cluster-devel; +Cc: dhowells, linux-fsdevel

These patches convert gfs2 and gfs2meta to use fs_context.

Tested with xfstests -g quick, a bunch of targeted mount commands to exercise
individual options, and gfs2_grow to test the gfs2meta mount.

v2: Use sget_fc() instead of sget()

Andrew Price (2):
  gfs2: Convert gfs2 to fs_context
  gfs2: Convert gfs2meta to fs_context

 fs/gfs2/incore.h     |  11 +-
 fs/gfs2/ops_fstype.c | 443 +++++++++++++++++++++++++++++++++++++------
 fs/gfs2/super.c      | 335 +-------------------------------
 fs/gfs2/super.h      |   3 +-
 4 files changed, 394 insertions(+), 398 deletions(-)

-- 
2.20.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context
  2019-03-19 16:04 [PATCH v2 0/2] gfs2: Switch to the new mount API Andrew Price
@ 2019-03-19 16:04 ` Andrew Price
  2019-03-19 16:04 ` [PATCH v2 2/2] gfs2: Convert gfs2meta " Andrew Price
  2019-03-19 17:05 ` [PATCH v2 1/2] gfs2: Convert gfs2 " David Howells
  2 siblings, 0 replies; 10+ messages in thread
From: Andrew Price @ 2019-03-19 16:04 UTC (permalink / raw)
  To: cluster-devel; +Cc: dhowells, linux-fsdevel

Switch to using fs_context ops for mount/remount.

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 fs/gfs2/incore.h     |  11 +-
 fs/gfs2/ops_fstype.c | 405 ++++++++++++++++++++++++++++++++++++++-----
 fs/gfs2/super.c      | 335 +----------------------------------
 fs/gfs2/super.h      |   3 +-
 4 files changed, 370 insertions(+), 384 deletions(-)

diff --git a/fs/gfs2/incore.h b/fs/gfs2/incore.h
index cdf07b408f54..ee474201f3b5 100644
--- a/fs/gfs2/incore.h
+++ b/fs/gfs2/incore.h
@@ -587,10 +587,13 @@ struct gfs2_args {
 	unsigned int ar_rgrplvb:1;		/* use lvbs for rgrp info */
 	unsigned int ar_loccookie:1;		/* use location based readdir
 						   cookies */
-	int ar_commit;				/* Commit interval */
-	int ar_statfs_quantum;			/* The fast statfs interval */
-	int ar_quota_quantum;			/* The quota interval */
-	int ar_statfs_percent;			/* The % change to force sync */
+	s32 ar_commit;				/* Commit interval */
+	s32 ar_statfs_quantum;			/* The fast statfs interval */
+	s32 ar_quota_quantum;			/* The quota interval */
+	s32 ar_statfs_percent;			/* The % change to force sync */
+
+	/* This is just for propagating the bdev through sget_fc() */
+	struct block_device *ar_bdev;
 };
 
 struct gfs2_tune {
diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index b041cb8ae383..ad2c98a37bd4 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -24,6 +24,7 @@
 #include <linux/lockdep.h>
 #include <linux/module.h>
 #include <linux/backing-dev.h>
+#include <linux/fs_parser.h>
 
 #include "gfs2.h"
 #include "incore.h"
@@ -1200,50 +1201,49 @@ static int fill_super(struct super_block *sb, struct gfs2_args *args, int silent
 	return error;
 }
 
-static int set_gfs2_super(struct super_block *s, void *data)
+static int set_gfs2_super(struct super_block *s, struct fs_context *fc)
 {
-	s->s_bdev = data;
+	struct gfs2_args *args = fc->fs_private;
+
+	s->s_bdev = args->ar_bdev;
 	s->s_dev = s->s_bdev->bd_dev;
 	s->s_bdi = bdi_get(s->s_bdev->bd_bdi);
 	return 0;
 }
 
-static int test_gfs2_super(struct super_block *s, void *ptr)
+static int test_gfs2_super(struct super_block *s, struct fs_context *fc)
 {
-	struct block_device *bdev = ptr;
-	return (bdev == s->s_bdev);
+	struct gfs2_args *args = fc->fs_private;
+
+	return (args->ar_bdev == s->s_bdev);
 }
 
 /**
- * gfs2_mount - Get the GFS2 superblock
- * @fs_type: The GFS2 filesystem type
- * @flags: Mount flags
- * @dev_name: The name of the device
- * @data: The mount arguments
+ * gfs2_get_tree - Get the GFS2 superblock and root directory
+ * @fc: The filesystem context
  *
  * Q. Why not use get_sb_bdev() ?
  * A. We need to select one of two root directories to mount, independent
  *    of whether this is the initial, or subsequent, mount of this sb
  *
- * Returns: 0 or -ve on error
+ * Returns: 0 or -errno on error
  */
 
-static struct dentry *gfs2_mount(struct file_system_type *fs_type, int flags,
-		       const char *dev_name, void *data)
+static int gfs2_get_tree(struct fs_context *fc)
 {
+	struct gfs2_args *args = fc->fs_private;
+	fmode_t mode = FMODE_READ | FMODE_EXCL;
 	struct block_device *bdev;
 	struct super_block *s;
-	fmode_t mode = FMODE_READ | FMODE_EXCL;
-	int error;
-	struct gfs2_args args;
 	struct gfs2_sbd *sdp;
+	int error;
 
-	if (!(flags & SB_RDONLY))
+	if (!(fc->sb_flags & SB_RDONLY))
 		mode |= FMODE_WRITE;
 
-	bdev = blkdev_get_by_path(dev_name, mode, fs_type);
+	bdev = blkdev_get_by_path(fc->source, mode, fc->fs_type);
 	if (IS_ERR(bdev))
-		return ERR_CAST(bdev);
+		return PTR_ERR(bdev);
 
 	/*
 	 * once the super is inserted into the list by sget, s_umount
@@ -1256,7 +1256,8 @@ static struct dentry *gfs2_mount(struct file_system_type *fs_type, int flags,
 		error = -EBUSY;
 		goto error_bdev;
 	}
-	s = sget(fs_type, test_gfs2_super, set_gfs2_super, flags, bdev);
+	args->ar_bdev = bdev;
+	s = sget_fc(fc, test_gfs2_super, set_gfs2_super);
 	mutex_unlock(&bdev->bd_fsfreeze_mutex);
 	error = PTR_ERR(s);
 	if (IS_ERR(s))
@@ -1278,28 +1279,14 @@ static struct dentry *gfs2_mount(struct file_system_type *fs_type, int flags,
 		s->s_mode = mode;
 	}
 
-	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;
-
-	error = gfs2_mount_args(&args, data);
-	if (error) {
-		pr_warn("can't parse mount arguments\n");
-		goto error_super;
-	}
-
 	if (s->s_root) {
 		error = -EBUSY;
-		if ((flags ^ s->s_flags) & SB_RDONLY)
+		if ((fc->sb_flags ^ s->s_flags) & SB_RDONLY)
 			goto error_super;
 	} else {
 		snprintf(s->s_id, sizeof(s->s_id), "%pg", bdev);
 		sb_set_blocksize(s, block_size(bdev));
-		error = fill_super(s, &args, flags & SB_SILENT ? 1 : 0);
+		error = fill_super(s, args, fc->sb_flags & SB_SILENT ? 1 : 0);
 		if (error)
 			goto error_super;
 		s->s_flags |= SB_ACTIVE;
@@ -1307,17 +1294,339 @@ static struct dentry *gfs2_mount(struct file_system_type *fs_type, int flags,
 	}
 
 	sdp = s->s_fs_info;
-	if (args.ar_meta)
-		return dget(sdp->sd_master_dir);
+	if (args->ar_meta)
+		fc->root = dget(sdp->sd_master_dir);
 	else
-		return dget(sdp->sd_root_dir);
-
+		fc->root = dget(sdp->sd_root_dir);
+	return 0;
 error_super:
 	deactivate_locked_super(s);
-	return ERR_PTR(error);
+	return error;
 error_bdev:
 	blkdev_put(bdev, mode);
-	return ERR_PTR(error);
+	return error;
+}
+
+static void gfs2_fc_free(struct fs_context *fc)
+{
+	struct gfs2_args *args = fc->fs_private;
+
+	kfree(args);
+}
+
+enum gfs2_param {
+	Opt_lockproto,
+	Opt_locktable,
+	Opt_hostdata,
+	Opt_spectator,
+	Opt_ignore_local_fs,
+	Opt_localflocks,
+	Opt_localcaching,
+	Opt_debug,
+	Opt_upgrade,
+	Opt_acl,
+	Opt_quota,
+	Opt_suiddir,
+	Opt_data,
+	Opt_meta,
+	Opt_discard,
+	Opt_commit,
+	Opt_errors,
+	Opt_statfs_quantum,
+	Opt_statfs_percent,
+	Opt_quota_quantum,
+	Opt_barrier,
+	Opt_rgrplvb,
+	Opt_loccookie,
+};
+
+enum opt_quota {
+	Opt_quota_unset = 0,
+	Opt_quota_off,
+	Opt_quota_account,
+	Opt_quota_on,
+};
+
+static const unsigned int opt_quota_values[] = {
+	[Opt_quota_off]     = GFS2_QUOTA_OFF,
+	[Opt_quota_account] = GFS2_QUOTA_ACCOUNT,
+	[Opt_quota_on]      = GFS2_QUOTA_ON,
+};
+
+enum opt_data {
+	Opt_data_writeback = GFS2_DATA_WRITEBACK,
+	Opt_data_ordered   = GFS2_DATA_ORDERED,
+};
+
+enum opt_errors {
+	Opt_errors_withdraw = GFS2_ERRORS_WITHDRAW,
+	Opt_errors_panic    = GFS2_ERRORS_PANIC,
+};
+
+static const struct fs_parameter_spec gfs2_param_specs[] = {
+	fsparam_string ("lockproto",          Opt_lockproto),
+	fsparam_string ("locktable",          Opt_locktable),
+	fsparam_string ("hostdata",           Opt_hostdata),
+	fsparam_flag   ("spectator",          Opt_spectator),
+	fsparam_flag   ("norecovery",         Opt_spectator),
+	fsparam_flag   ("ignore_local_fs",    Opt_ignore_local_fs),
+	fsparam_flag   ("localflocks",        Opt_localflocks),
+	fsparam_flag   ("localcaching",       Opt_localcaching),
+	fsparam_flag_no("debug",              Opt_debug),
+	fsparam_flag   ("upgrade",            Opt_upgrade),
+	fsparam_flag_no("acl",                Opt_acl),
+	fsparam_flag_no("suiddir",            Opt_suiddir),
+	fsparam_enum   ("data",               Opt_data),
+	fsparam_flag   ("meta",               Opt_meta),
+	fsparam_flag_no("discard",            Opt_discard),
+	fsparam_s32    ("commit",             Opt_commit),
+	fsparam_enum   ("errors",             Opt_errors),
+	fsparam_s32    ("statfs_quantum",     Opt_statfs_quantum),
+	fsparam_s32    ("statfs_percent",     Opt_statfs_percent),
+	fsparam_s32    ("quota_quantum",      Opt_quota_quantum),
+	fsparam_flag_no("barrier",            Opt_barrier),
+	fsparam_flag_no("rgrplvb",            Opt_rgrplvb),
+	fsparam_flag_no("loccookie",          Opt_loccookie),
+	/* quota can be a flag or an enum so it gets special treatment */
+	__fsparam(fs_param_is_enum, "quota", Opt_quota, fs_param_neg_with_no|fs_param_v_optional),
+	{}
+};
+
+static const struct fs_parameter_enum gfs2_param_enums[] = {
+	{ Opt_quota,    "off",        Opt_quota_off },
+	{ Opt_quota,    "account",    Opt_quota_account },
+	{ Opt_quota,    "on",         Opt_quota_on },
+	{ Opt_data,     "writeback",  Opt_data_writeback },
+	{ Opt_data,     "ordered",    Opt_data_ordered },
+	{ Opt_errors,   "withdraw",   Opt_errors_withdraw },
+	{ Opt_errors,   "panic",      Opt_errors_panic },
+	{}
+};
+
+const struct fs_parameter_description gfs2_fs_parameters = {
+	.name = "gfs2",
+	.specs = gfs2_param_specs,
+	.enums = gfs2_param_enums,
+};
+
+/* Parse a single mount parameter */
+static int gfs2_parse_param(struct fs_context *fc, struct fs_parameter *param)
+{
+	struct gfs2_args *args = fc->fs_private;
+	struct fs_parse_result result;
+	int o;
+
+	o = fs_parse(fc, &gfs2_fs_parameters, param, &result);
+	if (o < 0)
+		return o;
+
+	switch (o) {
+	case Opt_lockproto:
+		strlcpy(args->ar_lockproto, param->string, GFS2_LOCKNAME_LEN);
+		break;
+	case Opt_locktable:
+		strlcpy(args->ar_locktable, param->string, GFS2_LOCKNAME_LEN);
+		break;
+	case Opt_hostdata:
+		strlcpy(args->ar_hostdata, param->string, GFS2_LOCKNAME_LEN);
+		break;
+	case Opt_spectator:
+		args->ar_spectator = 1;
+		break;
+	case Opt_ignore_local_fs:
+		/* Retained for backwards compat only */
+		break;
+	case Opt_localflocks:
+		args->ar_localflocks = 1;
+		break;
+	case Opt_localcaching:
+		/* Retained for backwards compat only */
+		break;
+	case Opt_debug:
+		if (result.boolean && args->ar_errors == GFS2_ERRORS_PANIC) {
+			pr_warn("-o debug and -o errors=panic are mutually exclusive\n");
+			return -EINVAL;
+		}
+		args->ar_debug = result.boolean;
+		break;
+	case Opt_upgrade:
+		/* Retained for backwards compat only */
+		break;
+	case Opt_acl:
+		args->ar_posix_acl = result.boolean;
+		break;
+	case Opt_quota:
+		/* The quota option can be a flag or an enum. A non-zero int_32
+		   result means that we have an enum index. Otherwise we have
+		   to rely on the 'negated' flag to tell us whether 'quota' or
+		   'noquota' was specified. */
+		if (result.int_32 > 0)
+			args->ar_quota = opt_quota_values[result.int_32];
+		else if (result.negated)
+			args->ar_quota = GFS2_QUOTA_OFF;
+		else
+			args->ar_quota = GFS2_QUOTA_ON;
+		break;
+	case Opt_suiddir:
+		args->ar_suiddir = result.boolean;
+		break;
+	case Opt_data:
+		/* The uint_32 result maps directly to GFS2_DATA_* */
+		args->ar_data = result.uint_32;
+		break;
+	case Opt_meta:
+		args->ar_meta = 1;
+		break;
+	case Opt_discard:
+		args->ar_discard = result.boolean;
+		break;
+	case Opt_commit:
+		if (result.int_32 <= 0) {
+			pr_warn("commit mount option requires a positive numeric argument\n");
+			return -EINVAL;
+		}
+		args->ar_commit = result.int_32;
+		break;
+	case Opt_statfs_quantum:
+		if (result.int_32 < 0) {
+			pr_warn("statfs_quantum mount option requires a non-negative numeric argument\n");
+			return -EINVAL;
+		}
+		args->ar_statfs_quantum = result.int_32;
+		break;
+	case Opt_quota_quantum:
+		if (result.int_32 <= 0) {
+			pr_warn("quota_quantum mount option requires a positive numeric argument\n");
+			return -EINVAL;
+		}
+		args->ar_quota_quantum = result.int_32;
+		break;
+	case Opt_statfs_percent:
+		if (result.int_32 < 0 || result.int_32 > 100) {
+			pr_warn("statfs_percent mount option requires a numeric argument between 0 and 100\n");
+			return -EINVAL;
+		}
+		args->ar_statfs_percent = result.int_32;
+		break;
+	case Opt_errors:
+		if (args->ar_debug && result.uint_32 == GFS2_ERRORS_PANIC) {
+			pr_warn("-o debug and -o errors=panic are mutually exclusive\n");
+			return -EINVAL;
+		}
+		args->ar_errors = result.uint_32;
+		break;
+	case Opt_barrier:
+		args->ar_nobarrier = result.boolean;
+		break;
+	case Opt_rgrplvb:
+		args->ar_rgrplvb = result.boolean;
+		break;
+	case Opt_loccookie:
+		args->ar_loccookie = result.boolean;
+		break;
+	default:
+		pr_warn("invalid mount option: %s\n", param->key);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int gfs2_reconfigure(struct fs_context *fc)
+{
+	struct super_block *sb = fc->root->d_sb;
+	struct gfs2_sbd *sdp = sb->s_fs_info;
+	struct gfs2_args *oldargs = &sdp->sd_args;
+	struct gfs2_args *newargs = fc->fs_private;
+	struct gfs2_tune *gt = &sdp->sd_tune;
+	int error;
+
+	sync_filesystem(sb);
+
+	spin_lock(&gt->gt_spin);
+	oldargs->ar_commit = gt->gt_logd_secs;
+	oldargs->ar_quota_quantum = gt->gt_quota_quantum;
+	if (gt->gt_statfs_slow)
+		oldargs->ar_statfs_quantum = 0;
+	else
+		oldargs->ar_statfs_quantum = gt->gt_statfs_quantum;
+	spin_unlock(&gt->gt_spin);
+
+	/* Not allowed to change locking details */
+	if (strcmp(newargs->ar_lockproto, oldargs->ar_lockproto) ||
+	    strcmp(newargs->ar_locktable, oldargs->ar_locktable) ||
+	    strcmp(newargs->ar_hostdata, oldargs->ar_hostdata))
+		return -EINVAL;
+
+	/* Some flags must not be changed */
+	if (newargs->ar_spectator != oldargs->ar_spectator ||
+	    newargs->ar_localflocks != oldargs->ar_localflocks ||
+	    newargs->ar_meta != oldargs->ar_meta)
+		return -EINVAL;
+
+	if (oldargs->ar_spectator)
+		fc->sb_flags |= SB_RDONLY;
+
+	if ((sb->s_flags ^ fc->sb_flags) & SB_RDONLY) {
+		if (fc->sb_flags & SB_RDONLY)
+			error = gfs2_make_fs_ro(sdp);
+		else
+			error = gfs2_make_fs_rw(sdp);
+		if (error)
+			return error;
+	}
+	sdp->sd_args = *newargs;
+
+	if (sdp->sd_args.ar_posix_acl)
+		sb->s_flags |= SB_POSIXACL;
+	else
+		sb->s_flags &= ~SB_POSIXACL;
+	if (sdp->sd_args.ar_nobarrier)
+		set_bit(SDF_NOBARRIERS, &sdp->sd_flags);
+	else
+		clear_bit(SDF_NOBARRIERS, &sdp->sd_flags);
+	spin_lock(&gt->gt_spin);
+	gt->gt_logd_secs = newargs->ar_commit;
+	gt->gt_quota_quantum = newargs->ar_quota_quantum;
+	if (newargs->ar_statfs_quantum) {
+		gt->gt_statfs_slow = 0;
+		gt->gt_statfs_quantum = newargs->ar_statfs_quantum;
+	}
+	else {
+		gt->gt_statfs_slow = 1;
+		gt->gt_statfs_quantum = 30;
+	}
+	spin_unlock(&gt->gt_spin);
+
+	gfs2_online_uevent(sdp);
+	return 0;
+}
+
+static const struct fs_context_operations gfs2_context_ops = {
+	.free        = gfs2_fc_free,
+	.parse_param = gfs2_parse_param,
+	.get_tree    = gfs2_get_tree,
+	.reconfigure = gfs2_reconfigure,
+};
+
+/* Set up the filesystem mount context */
+static int gfs2_init_fs_context(struct fs_context *fc)
+{
+	struct gfs2_args *args;
+
+	args = kzalloc(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;
+
+	fc->fs_private = args;
+	fc->ops = &gfs2_context_ops;
+	return 0;
 }
 
 static int set_meta_super(struct super_block *s, void *ptr)
@@ -1325,6 +1634,13 @@ static int set_meta_super(struct super_block *s, void *ptr)
 	return -EINVAL;
 }
 
+static int test_meta_super(struct super_block *s, void *ptr)
+{
+	struct block_device *bdev = ptr;
+	return (bdev == s->s_bdev);
+}
+
+
 static struct dentry *gfs2_mount_meta(struct file_system_type *fs_type,
 			int flags, const char *dev_name, void *data)
 {
@@ -1342,7 +1658,7 @@ static struct dentry *gfs2_mount_meta(struct file_system_type *fs_type,
 			dev_name, error);
 		return ERR_PTR(error);
 	}
-	s = sget(&gfs2_fs_type, test_gfs2_super, set_meta_super, flags,
+	s = sget(&gfs2_fs_type, test_meta_super, set_meta_super, flags,
 		 path.dentry->d_sb->s_bdev);
 	path_put(&path);
 	if (IS_ERR(s)) {
@@ -1379,7 +1695,8 @@ static void gfs2_kill_sb(struct super_block *sb)
 struct file_system_type gfs2_fs_type = {
 	.name = "gfs2",
 	.fs_flags = FS_REQUIRES_DEV,
-	.mount = gfs2_mount,
+	.init_fs_context = gfs2_init_fs_context,
+	.parameters = &gfs2_fs_parameters,
 	.kill_sb = gfs2_kill_sb,
 	.owner = THIS_MODULE,
 };
diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c
index ca71163ff7cf..18917f8ae7ea 100644
--- a/fs/gfs2/super.c
+++ b/fs/gfs2/super.c
@@ -46,258 +46,6 @@
 #include "sys.h"
 #include "xattr.h"
 
-#define args_neq(a1, a2, x) ((a1)->ar_##x != (a2)->ar_##x)
-
-enum {
-	Opt_lockproto,
-	Opt_locktable,
-	Opt_hostdata,
-	Opt_spectator,
-	Opt_ignore_local_fs,
-	Opt_localflocks,
-	Opt_localcaching,
-	Opt_debug,
-	Opt_nodebug,
-	Opt_upgrade,
-	Opt_acl,
-	Opt_noacl,
-	Opt_quota_off,
-	Opt_quota_account,
-	Opt_quota_on,
-	Opt_quota,
-	Opt_noquota,
-	Opt_suiddir,
-	Opt_nosuiddir,
-	Opt_data_writeback,
-	Opt_data_ordered,
-	Opt_meta,
-	Opt_discard,
-	Opt_nodiscard,
-	Opt_commit,
-	Opt_err_withdraw,
-	Opt_err_panic,
-	Opt_statfs_quantum,
-	Opt_statfs_percent,
-	Opt_quota_quantum,
-	Opt_barrier,
-	Opt_nobarrier,
-	Opt_rgrplvb,
-	Opt_norgrplvb,
-	Opt_loccookie,
-	Opt_noloccookie,
-	Opt_error,
-};
-
-static const match_table_t tokens = {
-	{Opt_lockproto, "lockproto=%s"},
-	{Opt_locktable, "locktable=%s"},
-	{Opt_hostdata, "hostdata=%s"},
-	{Opt_spectator, "spectator"},
-	{Opt_spectator, "norecovery"},
-	{Opt_ignore_local_fs, "ignore_local_fs"},
-	{Opt_localflocks, "localflocks"},
-	{Opt_localcaching, "localcaching"},
-	{Opt_debug, "debug"},
-	{Opt_nodebug, "nodebug"},
-	{Opt_upgrade, "upgrade"},
-	{Opt_acl, "acl"},
-	{Opt_noacl, "noacl"},
-	{Opt_quota_off, "quota=off"},
-	{Opt_quota_account, "quota=account"},
-	{Opt_quota_on, "quota=on"},
-	{Opt_quota, "quota"},
-	{Opt_noquota, "noquota"},
-	{Opt_suiddir, "suiddir"},
-	{Opt_nosuiddir, "nosuiddir"},
-	{Opt_data_writeback, "data=writeback"},
-	{Opt_data_ordered, "data=ordered"},
-	{Opt_meta, "meta"},
-	{Opt_discard, "discard"},
-	{Opt_nodiscard, "nodiscard"},
-	{Opt_commit, "commit=%d"},
-	{Opt_err_withdraw, "errors=withdraw"},
-	{Opt_err_panic, "errors=panic"},
-	{Opt_statfs_quantum, "statfs_quantum=%d"},
-	{Opt_statfs_percent, "statfs_percent=%d"},
-	{Opt_quota_quantum, "quota_quantum=%d"},
-	{Opt_barrier, "barrier"},
-	{Opt_nobarrier, "nobarrier"},
-	{Opt_rgrplvb, "rgrplvb"},
-	{Opt_norgrplvb, "norgrplvb"},
-	{Opt_loccookie, "loccookie"},
-	{Opt_noloccookie, "noloccookie"},
-	{Opt_error, NULL}
-};
-
-/**
- * gfs2_mount_args - Parse mount options
- * @args: The structure into which the parsed options will be written
- * @options: The options to parse
- *
- * Return: errno
- */
-
-int gfs2_mount_args(struct gfs2_args *args, char *options)
-{
-	char *o;
-	int token;
-	substring_t tmp[MAX_OPT_ARGS];
-	int rv;
-
-	/* Split the options into tokens with the "," character and
-	   process them */
-
-	while (1) {
-		o = strsep(&options, ",");
-		if (o == NULL)
-			break;
-		if (*o == '\0')
-			continue;
-
-		token = match_token(o, tokens, tmp);
-		switch (token) {
-		case Opt_lockproto:
-			match_strlcpy(args->ar_lockproto, &tmp[0],
-				      GFS2_LOCKNAME_LEN);
-			break;
-		case Opt_locktable:
-			match_strlcpy(args->ar_locktable, &tmp[0],
-				      GFS2_LOCKNAME_LEN);
-			break;
-		case Opt_hostdata:
-			match_strlcpy(args->ar_hostdata, &tmp[0],
-				      GFS2_LOCKNAME_LEN);
-			break;
-		case Opt_spectator:
-			args->ar_spectator = 1;
-			break;
-		case Opt_ignore_local_fs:
-			/* Retained for backwards compat only */
-			break;
-		case Opt_localflocks:
-			args->ar_localflocks = 1;
-			break;
-		case Opt_localcaching:
-			/* Retained for backwards compat only */
-			break;
-		case Opt_debug:
-			if (args->ar_errors == GFS2_ERRORS_PANIC) {
-				pr_warn("-o debug and -o errors=panic are mutually exclusive\n");
-				return -EINVAL;
-			}
-			args->ar_debug = 1;
-			break;
-		case Opt_nodebug:
-			args->ar_debug = 0;
-			break;
-		case Opt_upgrade:
-			/* Retained for backwards compat only */
-			break;
-		case Opt_acl:
-			args->ar_posix_acl = 1;
-			break;
-		case Opt_noacl:
-			args->ar_posix_acl = 0;
-			break;
-		case Opt_quota_off:
-		case Opt_noquota:
-			args->ar_quota = GFS2_QUOTA_OFF;
-			break;
-		case Opt_quota_account:
-			args->ar_quota = GFS2_QUOTA_ACCOUNT;
-			break;
-		case Opt_quota_on:
-		case Opt_quota:
-			args->ar_quota = GFS2_QUOTA_ON;
-			break;
-		case Opt_suiddir:
-			args->ar_suiddir = 1;
-			break;
-		case Opt_nosuiddir:
-			args->ar_suiddir = 0;
-			break;
-		case Opt_data_writeback:
-			args->ar_data = GFS2_DATA_WRITEBACK;
-			break;
-		case Opt_data_ordered:
-			args->ar_data = GFS2_DATA_ORDERED;
-			break;
-		case Opt_meta:
-			args->ar_meta = 1;
-			break;
-		case Opt_discard:
-			args->ar_discard = 1;
-			break;
-		case Opt_nodiscard:
-			args->ar_discard = 0;
-			break;
-		case Opt_commit:
-			rv = match_int(&tmp[0], &args->ar_commit);
-			if (rv || args->ar_commit <= 0) {
-				pr_warn("commit mount option requires a positive numeric argument\n");
-				return rv ? rv : -EINVAL;
-			}
-			break;
-		case Opt_statfs_quantum:
-			rv = match_int(&tmp[0], &args->ar_statfs_quantum);
-			if (rv || args->ar_statfs_quantum < 0) {
-				pr_warn("statfs_quantum mount option requires a non-negative numeric argument\n");
-				return rv ? rv : -EINVAL;
-			}
-			break;
-		case Opt_quota_quantum:
-			rv = match_int(&tmp[0], &args->ar_quota_quantum);
-			if (rv || args->ar_quota_quantum <= 0) {
-				pr_warn("quota_quantum mount option requires a positive numeric argument\n");
-				return rv ? rv : -EINVAL;
-			}
-			break;
-		case Opt_statfs_percent:
-			rv = match_int(&tmp[0], &args->ar_statfs_percent);
-			if (rv || args->ar_statfs_percent < 0 ||
-			    args->ar_statfs_percent > 100) {
-				pr_warn("statfs_percent mount option requires a numeric argument between 0 and 100\n");
-				return rv ? rv : -EINVAL;
-			}
-			break;
-		case Opt_err_withdraw:
-			args->ar_errors = GFS2_ERRORS_WITHDRAW;
-			break;
-		case Opt_err_panic:
-			if (args->ar_debug) {
-				pr_warn("-o debug and -o errors=panic are mutually exclusive\n");
-				return -EINVAL;
-			}
-			args->ar_errors = GFS2_ERRORS_PANIC;
-			break;
-		case Opt_barrier:
-			args->ar_nobarrier = 0;
-			break;
-		case Opt_nobarrier:
-			args->ar_nobarrier = 1;
-			break;
-		case Opt_rgrplvb:
-			args->ar_rgrplvb = 1;
-			break;
-		case Opt_norgrplvb:
-			args->ar_rgrplvb = 0;
-			break;
-		case Opt_loccookie:
-			args->ar_loccookie = 1;
-			break;
-		case Opt_noloccookie:
-			args->ar_loccookie = 0;
-			break;
-		case Opt_error:
-		default:
-			pr_warn("invalid mount option: %s\n", o);
-			return -EINVAL;
-		}
-	}
-
-	return 0;
-}
-
 /**
  * gfs2_jindex_free - Clear all the journal index information
  * @sdp: The GFS2 superblock
@@ -844,7 +592,7 @@ static void gfs2_dirty_inode(struct inode *inode, int flags)
  * Returns: errno
  */
 
-static int gfs2_make_fs_ro(struct gfs2_sbd *sdp)
+int gfs2_make_fs_ro(struct gfs2_sbd *sdp)
 {
 	struct gfs2_holder freeze_gh;
 	int error;
@@ -1223,86 +971,6 @@ static int gfs2_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return 0;
 }
 
-/**
- * gfs2_remount_fs - called when the FS is remounted
- * @sb:  the filesystem
- * @flags:  the remount flags
- * @data:  extra data passed in (not used right now)
- *
- * Returns: errno
- */
-
-static int gfs2_remount_fs(struct super_block *sb, int *flags, char *data)
-{
-	struct gfs2_sbd *sdp = sb->s_fs_info;
-	struct gfs2_args args = sdp->sd_args; /* Default to current settings */
-	struct gfs2_tune *gt = &sdp->sd_tune;
-	int error;
-
-	sync_filesystem(sb);
-
-	spin_lock(&gt->gt_spin);
-	args.ar_commit = gt->gt_logd_secs;
-	args.ar_quota_quantum = gt->gt_quota_quantum;
-	if (gt->gt_statfs_slow)
-		args.ar_statfs_quantum = 0;
-	else
-		args.ar_statfs_quantum = gt->gt_statfs_quantum;
-	spin_unlock(&gt->gt_spin);
-	error = gfs2_mount_args(&args, data);
-	if (error)
-		return error;
-
-	/* Not allowed to change locking details */
-	if (strcmp(args.ar_lockproto, sdp->sd_args.ar_lockproto) ||
-	    strcmp(args.ar_locktable, sdp->sd_args.ar_locktable) ||
-	    strcmp(args.ar_hostdata, sdp->sd_args.ar_hostdata))
-		return -EINVAL;
-
-	/* Some flags must not be changed */
-	if (args_neq(&args, &sdp->sd_args, spectator) ||
-	    args_neq(&args, &sdp->sd_args, localflocks) ||
-	    args_neq(&args, &sdp->sd_args, meta))
-		return -EINVAL;
-
-	if (sdp->sd_args.ar_spectator)
-		*flags |= SB_RDONLY;
-
-	if ((sb->s_flags ^ *flags) & SB_RDONLY) {
-		if (*flags & SB_RDONLY)
-			error = gfs2_make_fs_ro(sdp);
-		else
-			error = gfs2_make_fs_rw(sdp);
-		if (error)
-			return error;
-	}
-
-	sdp->sd_args = args;
-	if (sdp->sd_args.ar_posix_acl)
-		sb->s_flags |= SB_POSIXACL;
-	else
-		sb->s_flags &= ~SB_POSIXACL;
-	if (sdp->sd_args.ar_nobarrier)
-		set_bit(SDF_NOBARRIERS, &sdp->sd_flags);
-	else
-		clear_bit(SDF_NOBARRIERS, &sdp->sd_flags);
-	spin_lock(&gt->gt_spin);
-	gt->gt_logd_secs = args.ar_commit;
-	gt->gt_quota_quantum = args.ar_quota_quantum;
-	if (args.ar_statfs_quantum) {
-		gt->gt_statfs_slow = 0;
-		gt->gt_statfs_quantum = args.ar_statfs_quantum;
-	}
-	else {
-		gt->gt_statfs_slow = 1;
-		gt->gt_statfs_quantum = 30;
-	}
-	spin_unlock(&gt->gt_spin);
-
-	gfs2_online_uevent(sdp);
-	return 0;
-}
-
 /**
  * gfs2_drop_inode - Drop an inode (test for remote unlink)
  * @inode: The inode to drop
@@ -1758,7 +1426,6 @@ const struct super_operations gfs2_super_ops = {
 	.freeze_super		= gfs2_freeze,
 	.thaw_super		= gfs2_unfreeze,
 	.statfs			= gfs2_statfs,
-	.remount_fs		= gfs2_remount_fs,
 	.drop_inode		= gfs2_drop_inode,
 	.show_options		= gfs2_show_options,
 };
diff --git a/fs/gfs2/super.h b/fs/gfs2/super.h
index 73c97dccae21..b9dc2065dd72 100644
--- a/fs/gfs2/super.h
+++ b/fs/gfs2/super.h
@@ -27,8 +27,6 @@ static inline unsigned int gfs2_jindex_size(struct gfs2_sbd *sdp)
 
 extern void gfs2_jindex_free(struct gfs2_sbd *sdp);
 
-extern int gfs2_mount_args(struct gfs2_args *args, char *data);
-
 extern struct gfs2_jdesc *gfs2_jdesc_find(struct gfs2_sbd *sdp, unsigned int jid);
 extern int gfs2_jdesc_check(struct gfs2_jdesc *jd);
 
@@ -36,6 +34,7 @@ extern int gfs2_lookup_in_master_dir(struct gfs2_sbd *sdp, char *filename,
 				     struct gfs2_inode **ipp);
 
 extern int gfs2_make_fs_rw(struct gfs2_sbd *sdp);
+extern int gfs2_make_fs_ro(struct gfs2_sbd *sdp);
 extern void gfs2_online_uevent(struct gfs2_sbd *sdp);
 extern int gfs2_statfs_init(struct gfs2_sbd *sdp);
 extern void gfs2_statfs_change(struct gfs2_sbd *sdp, s64 total, s64 free,
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] gfs2: Convert gfs2meta to fs_context
  2019-03-19 16:04 [PATCH v2 0/2] gfs2: Switch to the new mount API Andrew Price
  2019-03-19 16:04 ` [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context Andrew Price
@ 2019-03-19 16:04 ` Andrew Price
  2019-03-19 17:05 ` [PATCH v2 1/2] gfs2: Convert gfs2 " David Howells
  2 siblings, 0 replies; 10+ messages in thread
From: Andrew Price @ 2019-03-19 16:04 UTC (permalink / raw)
  To: cluster-devel; +Cc: dhowells, linux-fsdevel

Signed-off-by: Andrew Price <anprice@redhat.com>
---
 fs/gfs2/ops_fstype.c | 54 ++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
index ad2c98a37bd4..460d31403a7c 100644
--- a/fs/gfs2/ops_fstype.c
+++ b/fs/gfs2/ops_fstype.c
@@ -1629,48 +1629,58 @@ static int gfs2_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
-static int set_meta_super(struct super_block *s, void *ptr)
+static int set_meta_super(struct super_block *s, struct fs_context *fc)
 {
 	return -EINVAL;
 }
 
-static int test_meta_super(struct super_block *s, void *ptr)
-{
-	struct block_device *bdev = ptr;
-	return (bdev == s->s_bdev);
-}
-
-
-static struct dentry *gfs2_mount_meta(struct file_system_type *fs_type,
-			int flags, const char *dev_name, void *data)
+static int gfs2_meta_get_tree(struct fs_context *fc)
 {
+	struct gfs2_args *args = fc->fs_private;
 	struct super_block *s;
 	struct gfs2_sbd *sdp;
 	struct path path;
 	int error;
 
-	if (!dev_name || !*dev_name)
-		return ERR_PTR(-EINVAL);
+	if (!fc->source || !*fc->source)
+		return -EINVAL;
 
-	error = kern_path(dev_name, LOOKUP_FOLLOW, &path);
+	error = kern_path(fc->source, LOOKUP_FOLLOW, &path);
 	if (error) {
 		pr_warn("path_lookup on %s returned error %d\n",
-			dev_name, error);
-		return ERR_PTR(error);
+		        fc->source, error);
+		return error;
 	}
-	s = sget(&gfs2_fs_type, test_meta_super, set_meta_super, flags,
-		 path.dentry->d_sb->s_bdev);
+	fc->fs_type = &gfs2_fs_type;
+	args->ar_bdev = path.dentry->d_sb->s_bdev;
+	s = sget_fc(fc, test_gfs2_super, set_meta_super);
 	path_put(&path);
 	if (IS_ERR(s)) {
 		pr_warn("gfs2 mount does not exist\n");
-		return ERR_CAST(s);
+		return PTR_ERR(s);
 	}
-	if ((flags ^ s->s_flags) & SB_RDONLY) {
+	if ((fc->sb_flags ^ s->s_flags) & SB_RDONLY) {
 		deactivate_locked_super(s);
-		return ERR_PTR(-EBUSY);
+		return -EBUSY;
 	}
 	sdp = s->s_fs_info;
-	return dget(sdp->sd_master_dir);
+	fc->root = dget(sdp->sd_master_dir);
+	return 0;
+}
+
+static const struct fs_context_operations gfs2_meta_context_ops = {
+	.get_tree    = gfs2_meta_get_tree,
+};
+
+static int gfs2_meta_init_fs_context(struct fs_context *fc)
+{
+	int ret = gfs2_init_fs_context(fc);
+
+	if (ret)
+		return ret;
+
+	fc->ops = &gfs2_meta_context_ops;
+	return 0;
 }
 
 static void gfs2_kill_sb(struct super_block *sb)
@@ -1705,7 +1715,7 @@ MODULE_ALIAS_FS("gfs2");
 struct file_system_type gfs2meta_fs_type = {
 	.name = "gfs2meta",
 	.fs_flags = FS_REQUIRES_DEV,
-	.mount = gfs2_mount_meta,
+	.init_fs_context = gfs2_meta_init_fs_context,
 	.owner = THIS_MODULE,
 };
 MODULE_ALIAS_FS("gfs2meta");
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context
  2019-03-19 16:04 [PATCH v2 0/2] gfs2: Switch to the new mount API Andrew Price
  2019-03-19 16:04 ` [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context Andrew Price
  2019-03-19 16:04 ` [PATCH v2 2/2] gfs2: Convert gfs2meta " Andrew Price
@ 2019-03-19 17:05 ` David Howells
  2019-03-19 17:53   ` Andrew Price
  2 siblings, 1 reply; 10+ messages in thread
From: David Howells @ 2019-03-19 17:05 UTC (permalink / raw)
  To: Andrew Price; +Cc: dhowells, cluster-devel, linux-fsdevel

Andrew Price <anprice@redhat.com> wrote:

> +			pr_warn("-o debug and -o errors=panic are mutually exclusive\n");
> +			return -EINVAL;

return invalf(fc, "gfs2: -o debug and -o errors=panic are mutually exclusive"); 

(Note: no "\n")

> +		if (result.int_32 > 0)
> +			args->ar_quota = opt_quota_values[result.int_32];
> +		else if (result.negated)
> +			args->ar_quota = GFS2_QUOTA_OFF;
> +		else
> +			args->ar_quota = GFS2_QUOTA_ON;

I recommend checking result.negated first.

> +	/* Not allowed to change locking details */
> +	if (strcmp(newargs->ar_lockproto, oldargs->ar_lockproto) ||
> +	    strcmp(newargs->ar_locktable, oldargs->ar_locktable) ||
> +	    strcmp(newargs->ar_hostdata, oldargs->ar_hostdata))
> +		return -EINVAL;

Use errorf().  (Not invalf - the parameter isn't exactly invalid, it's just
that you're not allowed to do this operation).

> +			error = gfs2_make_fs_ro(sdp);
> +		else
> +			error = gfs2_make_fs_rw(sdp);
> +		if (error)
> +			return error;

Might want to call errorf() here too.

> -	s = sget(&gfs2_fs_type, test_gfs2_super, set_meta_super, flags,
> +	s = sget(&gfs2_fs_type, test_meta_super, set_meta_super, flags,

Try and use sget_fc() please.  If you look at the fuse patchset I cc'd you on,
there's a commit there that adds a ->bdev and ->bdev_mode to fs_context that
may be of use to you.

Can you use vfs_get_block_super()?  Would it be of use to export
test_bdev_super_fc() and set_bdev_super_fc()?

David

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context
  2019-03-19 17:05 ` [PATCH v2 1/2] gfs2: Convert gfs2 " David Howells
@ 2019-03-19 17:53   ` Andrew Price
  2019-03-21 12:57     ` Andrew Price
  2019-03-21 17:08     ` David Howells
  0 siblings, 2 replies; 10+ messages in thread
From: Andrew Price @ 2019-03-19 17:53 UTC (permalink / raw)
  To: David Howells; +Cc: cluster-devel, linux-fsdevel

On 19/03/2019 17:05, David Howells wrote:
> Andrew Price <anprice@redhat.com> wrote:
> 
>> +			pr_warn("-o debug and -o errors=panic are mutually exclusive\n");
>> +			return -EINVAL;
> 
> return invalf(fc, "gfs2: -o debug and -o errors=panic are mutually exclusive");

Thanks.

> 
> (Note: no "\n")
> 
>> +		if (result.int_32 > 0)
>> +			args->ar_quota = opt_quota_values[result.int_32];
>> +		else if (result.negated)
>> +			args->ar_quota = GFS2_QUOTA_OFF;
>> +		else
>> +			args->ar_quota = GFS2_QUOTA_ON;
> 
> I recommend checking result.negated first.

OK

> 
>> +	/* Not allowed to change locking details */
>> +	if (strcmp(newargs->ar_lockproto, oldargs->ar_lockproto) ||
>> +	    strcmp(newargs->ar_locktable, oldargs->ar_locktable) ||
>> +	    strcmp(newargs->ar_hostdata, oldargs->ar_hostdata))
>> +		return -EINVAL;
> 
> Use errorf().  (Not invalf - the parameter isn't exactly invalid, it's just
> that you're not allowed to do this operation).

Yes, that makes more sense.

>> +			error = gfs2_make_fs_ro(sdp);
>> +		else
>> +			error = gfs2_make_fs_rw(sdp);
>> +		if (error)
>> +			return error;
> 
> Might want to call errorf() here too.
> 
>> -	s = sget(&gfs2_fs_type, test_gfs2_super, set_meta_super, flags,
>> +	s = sget(&gfs2_fs_type, test_meta_super, set_meta_super, flags,
> 
> Try and use sget_fc() please.

That happens in patch 2/2 for gfs2meta. I might just roll both patches 
together in v3 so there's no intermediate churn.

> If you look at the fuse patchset I cc'd you on,
> there's a commit there that adds a ->bdev and ->bdev_mode to fs_context that
> may be of use to you.

Yes, that looks useful - thanks.

> Can you use vfs_get_block_super()?

It might be possible if we can rearrange things so that this can be done 
outside of the function:

         if (args->ar_meta)
                 fc->root = dget(sdp->sd_master_dir);
         else
                 fc->root = dget(sdp->sd_root_dir);

but we can't do that in our fill_super() because it needs to be selected 
whether we have an existing mount or not.

> Would it be of use to export
> test_bdev_super_fc() and set_bdev_super_fc()?

There's only a little maintenance value in it, I think, but we could 
certainly use them in gfs2 if we can't solve the fc->root selection issue.

Andy

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context
  2019-03-19 17:53   ` Andrew Price
@ 2019-03-21 12:57     ` Andrew Price
  2019-03-21 17:08     ` David Howells
  1 sibling, 0 replies; 10+ messages in thread
From: Andrew Price @ 2019-03-21 12:57 UTC (permalink / raw)
  To: David Howells; +Cc: cluster-devel, linux-fsdevel

On 19/03/2019 17:53, Andrew Price wrote:
> On 19/03/2019 17:05, David Howells wrote:

>> Can you use vfs_get_block_super()?
> 
> It might be possible if we can rearrange things so that this can be done 
> outside of the function:
> 
>          if (args->ar_meta)
>                  fc->root = dget(sdp->sd_master_dir);
>          else
>                  fc->root = dget(sdp->sd_root_dir);
> 
> but we can't do that in our fill_super() because it needs to be selected 
> whether we have an existing mount or not.

Would this fly?

diff --git a/fs/super.c b/fs/super.c
index 6d8dbf309241..0cdeaf28126f 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1232,11 +1232,12 @@ static int test_bdev_super_fc(struct super_block 
*s, struct fs_context *fc)
   * @fill_super: Helper to initialise a new superblock
   */
  int vfs_get_block_super(struct fs_context *fc,
-                       int (*fill_super)(struct super_block *,
-                                         struct fs_context *))
+      int (*fill_super)(struct super_block *, struct fs_context *),
+      struct dentry* (*select_root)(struct fs_context *, struct 
super_block *))
  {
         struct block_device *bdev;
         struct super_block *s;
+       struct dentry *root;
         int error = 0;

         fc->bdev_mode = FMODE_READ | FMODE_EXCL;
@@ -1302,7 +1303,11 @@ int vfs_get_block_super(struct fs_context *fc,
         }

         BUG_ON(fc->root);
-       fc->root = dget(s->s_root);
+       if (select_root)
+               root = select_root(fc, s);
+       else
+               root = s->s_root;
+       fc->root = dget(root);
         return 0;

  error_sb:
diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h
index 8233c873af73..c2e9dc5826ce 100644
--- a/include/linux/fs_context.h
+++ b/include/linux/fs_context.h
@@ -145,8 +145,8 @@ extern int vfs_get_super(struct fs_context *fc,
                                            struct fs_context *fc));

  extern int vfs_get_block_super(struct fs_context *fc,
-                              int (*fill_super)(struct super_block *sb,
-                                                struct fs_context *fc));
+      int (*fill_super)(struct super_block *, struct fs_context *),
+      struct dentry* (*select_root)(struct fs_context *, struct 
super_block *));

  extern const struct file_operations fscontext_fops;


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context
  2019-03-19 17:53   ` Andrew Price
  2019-03-21 12:57     ` Andrew Price
@ 2019-03-21 17:08     ` David Howells
  2019-03-21 17:26       ` [PATCH] vfs: Allow selection of fs root independent of sb Andrew Price
  2019-03-21 17:59       ` David Howells
  1 sibling, 2 replies; 10+ messages in thread
From: David Howells @ 2019-03-21 17:08 UTC (permalink / raw)
  To: Andrew Price; +Cc: dhowells, cluster-devel, linux-fsdevel

Andrew Price <anprice@redhat.com> wrote:

> Would this fly?

Can you update the comment to document what it's for?

David

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] vfs: Allow selection of fs root independent of sb
  2019-03-21 17:08     ` David Howells
@ 2019-03-21 17:26       ` Andrew Price
  2019-03-21 18:36         ` Matthew Wilcox
  2019-03-21 17:59       ` David Howells
  1 sibling, 1 reply; 10+ messages in thread
From: Andrew Price @ 2019-03-21 17:26 UTC (permalink / raw)
  To: cluster-devel, dhowells; +Cc: linux-fsdevel

Take a function pointer 'select_root' in vfs_get_block_super() to allow
callers to select the root dentry based on the context.

This is intended for gfs2 to select between the normal root and the
'meta' root, which must be done whether a root already exists for a
superblock or not, i.e. it cannot be decided in fill_super().

This will allow gfs2 to avoid duplicating a function from the vfs just
to special-case the root dentry selection.

Signed-off-by: Andrew Price <anprice@redhat.com>
---

Depends on "vfs: Create fs_context-aware mount_bdev() replacement"

 fs/super.c                 | 11 ++++++++---
 include/linux/fs_context.h |  4 ++--
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index 6d8dbf309241..0cdeaf28126f 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1232,11 +1232,12 @@ static int test_bdev_super_fc(struct super_block *s, struct fs_context *fc)
  * @fill_super: Helper to initialise a new superblock
  */
 int vfs_get_block_super(struct fs_context *fc,
-			int (*fill_super)(struct super_block *,
-					  struct fs_context *))
+      int (*fill_super)(struct super_block *, struct fs_context *),
+      struct dentry* (*select_root)(struct fs_context *, struct super_block *))
 {
 	struct block_device *bdev;
 	struct super_block *s;
+	struct dentry *root;
 	int error = 0;
 
 	fc->bdev_mode = FMODE_READ | FMODE_EXCL;
@@ -1302,7 +1303,11 @@ int vfs_get_block_super(struct fs_context *fc,
 	}
 
 	BUG_ON(fc->root);
-	fc->root = dget(s->s_root);
+	if (select_root)
+		root = select_root(fc, s);
+	else
+		root = s->s_root;
+	fc->root = dget(root);
 	return 0;
 
 error_sb:
diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h
index 8233c873af73..c2e9dc5826ce 100644
--- a/include/linux/fs_context.h
+++ b/include/linux/fs_context.h
@@ -145,8 +145,8 @@ extern int vfs_get_super(struct fs_context *fc,
 					   struct fs_context *fc));
 
 extern int vfs_get_block_super(struct fs_context *fc,
-			       int (*fill_super)(struct super_block *sb,
-						 struct fs_context *fc));
+      int (*fill_super)(struct super_block *, struct fs_context *),
+      struct dentry* (*select_root)(struct fs_context *, struct super_block *));
 
 extern const struct file_operations fscontext_fops;
 
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] vfs: Allow selection of fs root independent of sb
  2019-03-21 17:08     ` David Howells
  2019-03-21 17:26       ` [PATCH] vfs: Allow selection of fs root independent of sb Andrew Price
@ 2019-03-21 17:59       ` David Howells
  1 sibling, 0 replies; 10+ messages in thread
From: David Howells @ 2019-03-21 17:59 UTC (permalink / raw)
  To: Andrew Price; +Cc: dhowells, cluster-devel, linux-fsdevel

That's not what I meant.  There's a kerneldoc comment on vfs_get_block_super()
that you need to modify.  I would in any case roll your patch into mine.

> This is intended for gfs2 to select between the normal root and the
> 'meta' root, which must be done whether a root already exists for a
> superblock or not, i.e. it cannot be decided in fill_super().

Can this be done in gfs2_get_tree, where it sets fc->root?

David

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] vfs: Allow selection of fs root independent of sb
  2019-03-21 17:26       ` [PATCH] vfs: Allow selection of fs root independent of sb Andrew Price
@ 2019-03-21 18:36         ` Matthew Wilcox
  0 siblings, 0 replies; 10+ messages in thread
From: Matthew Wilcox @ 2019-03-21 18:36 UTC (permalink / raw)
  To: Andrew Price; +Cc: cluster-devel, dhowells, linux-fsdevel

On Thu, Mar 21, 2019 at 05:26:44PM +0000, Andrew Price wrote:
> Take a function pointer 'select_root' in vfs_get_block_super() to allow
> callers to select the root dentry based on the context.

Can't this be a fs_context_operations pointer?


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-03-21 18:36 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-19 16:04 [PATCH v2 0/2] gfs2: Switch to the new mount API Andrew Price
2019-03-19 16:04 ` [PATCH v2 1/2] gfs2: Convert gfs2 to fs_context Andrew Price
2019-03-19 16:04 ` [PATCH v2 2/2] gfs2: Convert gfs2meta " Andrew Price
2019-03-19 17:05 ` [PATCH v2 1/2] gfs2: Convert gfs2 " David Howells
2019-03-19 17:53   ` Andrew Price
2019-03-21 12:57     ` Andrew Price
2019-03-21 17:08     ` David Howells
2019-03-21 17:26       ` [PATCH] vfs: Allow selection of fs root independent of sb Andrew Price
2019-03-21 18:36         ` Matthew Wilcox
2019-03-21 17:59       ` David Howells

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).