diff mbox series

[V2] Convert coda to use the new mount API

Message ID 97650eeb-94c7-4041-b58c-90e81e76b699@redhat.com (mailing list archive)
State New, archived
Headers show
Series [V2] Convert coda to use the new mount API | expand

Commit Message

Eric Sandeen Feb. 21, 2024, 3:40 p.m. UTC
From: David Howells <dhowells@redhat.com>

Convert the coda filesystem to the new internal mount API as the old
one will be obsoleted and removed.  This allows greater flexibility in
communication of mount parameters between userspace, the VFS and the
filesystem.

See Documentation/filesystems/mount_api.rst for more information.

Note this is slightly tricky as coda currently only has a binary mount data
interface.  This is handled through the parse_monolithic hook.

Also add a more conventional interface with a parameter named "fd" that
takes an fd that refers to a coda psdev, thereby specifying the index to
use.

Signed-off-by: David Howells <dhowells@redhat.com>
Co-developed-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
[sandeen: forward port to current upstream mount API interfaces]
Tested-by: Jan Harkes <jaharkes@cs.cmu.edu>
cc: coda@cs.cmu.edu
---

V2: Remove extra task_active_pid_ns check from fill_super() that I missed
(note that Jan did not test with this change)

NB: This updated patch is based on 
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=mount-api-viro&id=4aec2ba3ca543e39944604774b8cab9c4d592651

hence the From: David above.

 fs/coda/inode.c | 143 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 98 insertions(+), 45 deletions(-)

Comments

Ian Kent Feb. 21, 2024, 11:48 p.m. UTC | #1
On 21/2/24 23:40, Eric Sandeen wrote:
> From: David Howells <dhowells@redhat.com>
>
> Convert the coda filesystem to the new internal mount API as the old
> one will be obsoleted and removed.  This allows greater flexibility in
> communication of mount parameters between userspace, the VFS and the
> filesystem.
>
> See Documentation/filesystems/mount_api.rst for more information.
>
> Note this is slightly tricky as coda currently only has a binary mount data
> interface.  This is handled through the parse_monolithic hook.
>
> Also add a more conventional interface with a parameter named "fd" that
> takes an fd that refers to a coda psdev, thereby specifying the index to
> use.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Co-developed-by: Eric Sandeen <sandeen@redhat.com>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> [sandeen: forward port to current upstream mount API interfaces]
> Tested-by: Jan Harkes <jaharkes@cs.cmu.edu>
> cc: coda@cs.cmu.edu
> ---
>
> V2: Remove extra task_active_pid_ns check from fill_super() that I missed
> (note that Jan did not test with this change)
>
> NB: This updated patch is based on
> https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/commit/?h=mount-api-viro&id=4aec2ba3ca543e39944604774b8cab9c4d592651
>
> hence the From: David above.
>
>   fs/coda/inode.c | 143 +++++++++++++++++++++++++++++++++---------------
>   1 file changed, 98 insertions(+), 45 deletions(-)
>
> diff --git a/fs/coda/inode.c b/fs/coda/inode.c
> index 0c7c2528791e..a50356c541f6 100644
> --- a/fs/coda/inode.c
> +++ b/fs/coda/inode.c
> @@ -24,6 +24,8 @@
>   #include <linux/pid_namespace.h>
>   #include <linux/uaccess.h>
>   #include <linux/fs.h>
> +#include <linux/fs_context.h>
> +#include <linux/fs_parser.h>
>   #include <linux/vmalloc.h>
>   
>   #include <linux/coda.h>
> @@ -87,10 +89,10 @@ void coda_destroy_inodecache(void)
>   	kmem_cache_destroy(coda_inode_cachep);
>   }
>   
> -static int coda_remount(struct super_block *sb, int *flags, char *data)
> +static int coda_reconfigure(struct fs_context *fc)
>   {
> -	sync_filesystem(sb);
> -	*flags |= SB_NOATIME;
> +	sync_filesystem(fc->root->d_sb);
> +	fc->sb_flags |= SB_NOATIME;
>   	return 0;
>   }
>   
> @@ -102,78 +104,102 @@ static const struct super_operations coda_super_operations =
>   	.evict_inode	= coda_evict_inode,
>   	.put_super	= coda_put_super,
>   	.statfs		= coda_statfs,
> -	.remount_fs	= coda_remount,
>   };
>   
> -static int get_device_index(struct coda_mount_data *data)
> +struct coda_fs_context {
> +	int	idx;
> +};
> +
> +enum {
> +	Opt_fd,
> +};
> +
> +static const struct fs_parameter_spec coda_param_specs[] = {
> +	fsparam_fd	("fd",	Opt_fd),
> +	{}
> +};
> +
> +static int coda_parse_fd(struct fs_context *fc, int fd)
>   {
> +	struct coda_fs_context *ctx = fc->fs_private;
>   	struct fd f;
>   	struct inode *inode;
>   	int idx;
>   
> -	if (data == NULL) {
> -		pr_warn("%s: Bad mount data\n", __func__);
> -		return -1;
> -	}
> -
> -	if (data->version != CODA_MOUNT_VERSION) {
> -		pr_warn("%s: Bad mount version\n", __func__);
> -		return -1;
> -	}
> -
> -	f = fdget(data->fd);
> +	f = fdget(fd);
>   	if (!f.file)
> -		goto Ebadf;
> +		return -EBADF;
>   	inode = file_inode(f.file);
>   	if (!S_ISCHR(inode->i_mode) || imajor(inode) != CODA_PSDEV_MAJOR) {
>   		fdput(f);
> -		goto Ebadf;
> +		return invalf(fc, "code: Not coda psdev");
>   	}
>   
>   	idx = iminor(inode);
>   	fdput(f);
>   
> -	if (idx < 0 || idx >= MAX_CODADEVS) {
> -		pr_warn("%s: Bad minor number\n", __func__);
> -		return -1;
> +	if (idx < 0 || idx >= MAX_CODADEVS)
> +		return invalf(fc, "coda: Bad minor number");
> +	ctx->idx = idx;
> +	return 0;
> +}
> +
> +static int coda_parse_param(struct fs_context *fc, struct fs_parameter *param)
> +{
> +	struct fs_parse_result result;
> +	int opt;
> +
> +	opt = fs_parse(fc, coda_param_specs, param, &result);
> +	if (opt < 0)
> +		return opt;
> +
> +	switch (opt) {
> +	case Opt_fd:
> +		return coda_parse_fd(fc, result.uint_32);
>   	}
>   
> -	return idx;
> -Ebadf:
> -	pr_warn("%s: Bad file\n", __func__);
> -	return -1;
> +	return 0;
> +}
> +
> +/*
> + * Parse coda's binary mount data form.  We ignore any errors and go with index
> + * 0 if we get one for backward compatibility.
> + */
> +static int coda_parse_monolithic(struct fs_context *fc, void *_data)
> +{
> +	struct coda_mount_data *data = _data;
> +
> +	if (!data)
> +		return invalf(fc, "coda: Bad mount data");
> +
> +	if (data->version != CODA_MOUNT_VERSION)
> +		return invalf(fc, "coda: Bad mount version");
> +
> +	coda_parse_fd(fc, data->fd);
> +	return 0;
>   }
>   
> -static int coda_fill_super(struct super_block *sb, void *data, int silent)
> +static int coda_fill_super(struct super_block *sb, struct fs_context *fc)
>   {
> +	struct coda_fs_context *ctx = fc->fs_private;
>   	struct inode *root = NULL;
>   	struct venus_comm *vc;
>   	struct CodaFid fid;
>   	int error;
> -	int idx;
> -
> -	if (task_active_pid_ns(current) != &init_pid_ns)
> -		return -EINVAL;
> -
> -	idx = get_device_index((struct coda_mount_data *) data);
>   
> -	/* Ignore errors in data, for backward compatibility */
> -	if(idx == -1)
> -		idx = 0;
> -	
> -	pr_info("%s: device index: %i\n", __func__,  idx);
> +	infof(fc, "coda: device index: %i\n", ctx->idx);
>   
> -	vc = &coda_comms[idx];
> +	vc = &coda_comms[ctx->idx];
>   	mutex_lock(&vc->vc_mutex);
>   
>   	if (!vc->vc_inuse) {
> -		pr_warn("%s: No pseudo device\n", __func__);
> +		errorf(fc, "coda: No pseudo device");
>   		error = -EINVAL;
>   		goto unlock_out;
>   	}
>   
>   	if (vc->vc_sb) {
> -		pr_warn("%s: Device already mounted\n", __func__);
> +		errorf(fc, "coda: Device already mounted");
>   		error = -EBUSY;
>   		goto unlock_out;
>   	}
> @@ -313,18 +339,45 @@ static int coda_statfs(struct dentry *dentry, struct kstatfs *buf)
>   	return 0;
>   }
>   
> -/* init_coda: used by filesystems.c to register coda */
> +static int coda_get_tree(struct fs_context *fc)
> +{
> +	if (task_active_pid_ns(current) != &init_pid_ns)
> +		return -EINVAL;
>   
> -static struct dentry *coda_mount(struct file_system_type *fs_type,
> -	int flags, const char *dev_name, void *data)
> +	return get_tree_nodev(fc, coda_fill_super);
> +}
> +
> +static void coda_free_fc(struct fs_context *fc)
>   {
> -	return mount_nodev(fs_type, flags, data, coda_fill_super);
> +	kfree(fc->fs_private);
> +}
> +
> +static const struct fs_context_operations coda_context_ops = {
> +	.free		= coda_free_fc,
> +	.parse_param	= coda_parse_param,
> +	.parse_monolithic = coda_parse_monolithic,
> +	.get_tree	= coda_get_tree,
> +	.reconfigure	= coda_reconfigure,
> +};
> +
> +static int coda_init_fs_context(struct fs_context *fc)
> +{
> +	struct coda_fs_context *ctx;
> +
> +	ctx = kzalloc(sizeof(struct coda_fs_context), GFP_KERNEL);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	fc->fs_private = ctx;
> +	fc->ops = &coda_context_ops;
> +	return 0;
>   }
>   
>   struct file_system_type coda_fs_type = {
>   	.owner		= THIS_MODULE,
>   	.name		= "coda",
> -	.mount		= coda_mount,
> +	.init_fs_context = coda_init_fs_context,
> +	.parameters	= coda_param_specs,
>   	.kill_sb	= kill_anon_super,
>   	.fs_flags	= FS_BINARY_MOUNTDATA,
>   };


Looks good to me.

Reviewed-by: Ian Kent <raven@themaw.net>
Christian Brauner Feb. 22, 2024, 9:03 a.m. UTC | #2
On Wed, 21 Feb 2024 09:40:03 -0600, Eric Sandeen wrote:
> Convert the coda filesystem to the new internal mount API as the old
> one will be obsoleted and removed.  This allows greater flexibility in
> communication of mount parameters between userspace, the VFS and the
> filesystem.
> 
> See Documentation/filesystems/mount_api.rst for more information.
> 
> [...]

Applied to the vfs.misc branch of the vfs/vfs.git tree.
Patches in the vfs.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.misc

[1/1] Convert coda to use the new mount API
      https://git.kernel.org/vfs/vfs/c/62e3038211db
diff mbox series

Patch

diff --git a/fs/coda/inode.c b/fs/coda/inode.c
index 0c7c2528791e..a50356c541f6 100644
--- a/fs/coda/inode.c
+++ b/fs/coda/inode.c
@@ -24,6 +24,8 @@ 
 #include <linux/pid_namespace.h>
 #include <linux/uaccess.h>
 #include <linux/fs.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 #include <linux/vmalloc.h>
 
 #include <linux/coda.h>
@@ -87,10 +89,10 @@  void coda_destroy_inodecache(void)
 	kmem_cache_destroy(coda_inode_cachep);
 }
 
-static int coda_remount(struct super_block *sb, int *flags, char *data)
+static int coda_reconfigure(struct fs_context *fc)
 {
-	sync_filesystem(sb);
-	*flags |= SB_NOATIME;
+	sync_filesystem(fc->root->d_sb);
+	fc->sb_flags |= SB_NOATIME;
 	return 0;
 }
 
@@ -102,78 +104,102 @@  static const struct super_operations coda_super_operations =
 	.evict_inode	= coda_evict_inode,
 	.put_super	= coda_put_super,
 	.statfs		= coda_statfs,
-	.remount_fs	= coda_remount,
 };
 
-static int get_device_index(struct coda_mount_data *data)
+struct coda_fs_context {
+	int	idx;
+};
+
+enum {
+	Opt_fd,
+};
+
+static const struct fs_parameter_spec coda_param_specs[] = {
+	fsparam_fd	("fd",	Opt_fd),
+	{}
+};
+
+static int coda_parse_fd(struct fs_context *fc, int fd)
 {
+	struct coda_fs_context *ctx = fc->fs_private;
 	struct fd f;
 	struct inode *inode;
 	int idx;
 
-	if (data == NULL) {
-		pr_warn("%s: Bad mount data\n", __func__);
-		return -1;
-	}
-
-	if (data->version != CODA_MOUNT_VERSION) {
-		pr_warn("%s: Bad mount version\n", __func__);
-		return -1;
-	}
-
-	f = fdget(data->fd);
+	f = fdget(fd);
 	if (!f.file)
-		goto Ebadf;
+		return -EBADF;
 	inode = file_inode(f.file);
 	if (!S_ISCHR(inode->i_mode) || imajor(inode) != CODA_PSDEV_MAJOR) {
 		fdput(f);
-		goto Ebadf;
+		return invalf(fc, "code: Not coda psdev");
 	}
 
 	idx = iminor(inode);
 	fdput(f);
 
-	if (idx < 0 || idx >= MAX_CODADEVS) {
-		pr_warn("%s: Bad minor number\n", __func__);
-		return -1;
+	if (idx < 0 || idx >= MAX_CODADEVS)
+		return invalf(fc, "coda: Bad minor number");
+	ctx->idx = idx;
+	return 0;
+}
+
+static int coda_parse_param(struct fs_context *fc, struct fs_parameter *param)
+{
+	struct fs_parse_result result;
+	int opt;
+
+	opt = fs_parse(fc, coda_param_specs, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case Opt_fd:
+		return coda_parse_fd(fc, result.uint_32);
 	}
 
-	return idx;
-Ebadf:
-	pr_warn("%s: Bad file\n", __func__);
-	return -1;
+	return 0;
+}
+
+/*
+ * Parse coda's binary mount data form.  We ignore any errors and go with index
+ * 0 if we get one for backward compatibility.
+ */
+static int coda_parse_monolithic(struct fs_context *fc, void *_data)
+{
+	struct coda_mount_data *data = _data;
+
+	if (!data)
+		return invalf(fc, "coda: Bad mount data");
+
+	if (data->version != CODA_MOUNT_VERSION)
+		return invalf(fc, "coda: Bad mount version");
+
+	coda_parse_fd(fc, data->fd);
+	return 0;
 }
 
-static int coda_fill_super(struct super_block *sb, void *data, int silent)
+static int coda_fill_super(struct super_block *sb, struct fs_context *fc)
 {
+	struct coda_fs_context *ctx = fc->fs_private;
 	struct inode *root = NULL;
 	struct venus_comm *vc;
 	struct CodaFid fid;
 	int error;
-	int idx;
-
-	if (task_active_pid_ns(current) != &init_pid_ns)
-		return -EINVAL;
-
-	idx = get_device_index((struct coda_mount_data *) data);
 
-	/* Ignore errors in data, for backward compatibility */
-	if(idx == -1)
-		idx = 0;
-	
-	pr_info("%s: device index: %i\n", __func__,  idx);
+	infof(fc, "coda: device index: %i\n", ctx->idx);
 
-	vc = &coda_comms[idx];
+	vc = &coda_comms[ctx->idx];
 	mutex_lock(&vc->vc_mutex);
 
 	if (!vc->vc_inuse) {
-		pr_warn("%s: No pseudo device\n", __func__);
+		errorf(fc, "coda: No pseudo device");
 		error = -EINVAL;
 		goto unlock_out;
 	}
 
 	if (vc->vc_sb) {
-		pr_warn("%s: Device already mounted\n", __func__);
+		errorf(fc, "coda: Device already mounted");
 		error = -EBUSY;
 		goto unlock_out;
 	}
@@ -313,18 +339,45 @@  static int coda_statfs(struct dentry *dentry, struct kstatfs *buf)
 	return 0; 
 }
 
-/* init_coda: used by filesystems.c to register coda */
+static int coda_get_tree(struct fs_context *fc)
+{
+	if (task_active_pid_ns(current) != &init_pid_ns)
+		return -EINVAL;
 
-static struct dentry *coda_mount(struct file_system_type *fs_type,
-	int flags, const char *dev_name, void *data)
+	return get_tree_nodev(fc, coda_fill_super);
+}
+
+static void coda_free_fc(struct fs_context *fc)
 {
-	return mount_nodev(fs_type, flags, data, coda_fill_super);
+	kfree(fc->fs_private);
+}
+
+static const struct fs_context_operations coda_context_ops = {
+	.free		= coda_free_fc,
+	.parse_param	= coda_parse_param,
+	.parse_monolithic = coda_parse_monolithic,
+	.get_tree	= coda_get_tree,
+	.reconfigure	= coda_reconfigure,
+};
+
+static int coda_init_fs_context(struct fs_context *fc)
+{
+	struct coda_fs_context *ctx;
+
+	ctx = kzalloc(sizeof(struct coda_fs_context), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	fc->fs_private = ctx;
+	fc->ops = &coda_context_ops;
+	return 0;
 }
 
 struct file_system_type coda_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "coda",
-	.mount		= coda_mount,
+	.init_fs_context = coda_init_fs_context,
+	.parameters	= coda_param_specs,
 	.kill_sb	= kill_anon_super,
 	.fs_flags	= FS_BINARY_MOUNTDATA,
 };