On 5/10/23 09:50, 胡玮文 wrote:
On Wed, May 10, 2023 at 08:42:10AM +0800, Xiubo Li wrote:
On 5/9/23 21:55, 胡玮文 wrote:
On Tue, May 09, 2023 at 09:36:16AM +0800, Xiubo Li wrote:
On 5/8/23 01:55, Hu Weiwen wrote:
From: Hu Weiwen <sehuww@xxxxxxxxxxxxxxxx>
We have name and fsid in the new device syntax. It is confusing that
the kernel accept these info but do not take them into account when
connecting to the cluster.
Although the mount.ceph helper program will extract the name from device
spec and pass it as name options, these changes are still useful if we
don't have that program installed, or if we want to call `mount()'
directly.
Signed-off-by: Hu Weiwen <sehuww@xxxxxxxxxxxxxxxx>
---
fs/ceph/super.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 4e1f4031e888..74636b9383b8 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -267,6 +267,7 @@ static int ceph_parse_new_source(const char *dev_name, const char *dev_name_end,
struct ceph_fsid fsid;
struct ceph_parse_opts_ctx *pctx = fc->fs_private;
struct ceph_mount_options *fsopt = pctx->opts;
+ struct ceph_options *copts = pctx->copts;
char *fsid_start, *fs_name_start;
if (*dev_name_end != '=') {
@@ -285,6 +286,12 @@ static int ceph_parse_new_source(const char *dev_name, const char *dev_name_end,
if (ceph_parse_fsid(fsid_start, &fsid))
return invalfc(fc, "Invalid FSID");
+ if (!(copts->flags & CEPH_OPT_FSID)) {
+ copts->fsid = fsid;
+ copts->flags |= CEPH_OPT_FSID;
+ } else if (ceph_fsid_compare(&fsid, &copts->fsid)) {
+ return invalfc(fc, "Mismatching cluster FSID");
+ }
++fs_name_start; /* start of file system name */
len = dev_name_end - fs_name_start;
@@ -298,6 +305,16 @@ static int ceph_parse_new_source(const char *dev_name, const char *dev_name_end,
}
dout("file system (mds namespace) '%s'\n", fsopt->mds_namespace);
+ len = fsid_start - dev_name - 1;
+ if (!copts->name) {
+ copts->name = kstrndup(dev_name, len, GFP_KERNEL);
+ if (!copts->name)
+ return -ENOMEM;
Shouldn't we kfree the 'copts->mds_namespace' here ?
Seems not necessary. ceph_free_fc() will take care of releasing the
whole 'struct ceph_parse_opts_ctx', including 'copts->mds_namespace'.
Besides, the mds_namespace may already be set before we parse the source
here.
ceph_free_fc() is called from:
put_fs_context
457 void put_fs_context(struct fs_context *fc)
458 {
459 struct super_block *sb;
460
461 if (fc->root) {
462 sb = fc->root->d_sb;
463 dput(fc->root);
464 fc->root = NULL;
465 deactivate_super(sb);
466 }
467
468 if (fc->need_free && fc->ops && fc->ops->free)
469 fc->ops->free(fc);
But are u sure the 'fc->need_free' is correctly set ?
It seems not from my reading if I didn't miss something.
'fc->need_free' is initialized to true just after init_fs_context() is
called, see 'alloc_fs_context()'. And it is only reset to false after
calling free().
I've verified with gdb that ceph_free_fc() got called if
ceph_parse_new_source() returns an error.
Anyway, if ceph_free_fc() is not invoked, then we are leaking a lot of
things, not just mds_namespace. The whole fs_context need to be freed
unconditionally after the mount is finished.
Okay, you are right. I just missed that.
Thanks
Thanks
- Xiubo
do_new_mount
do_mount
+ } else if (!strstrn_equals(copts->name, dev_name, len)) {
+ return invalfc(fc, "Mismatching cephx name");
+ }
+ dout("cephx name '%s'\n", copts->name);
+
fsopt->new_dev_syntax = true;
return 0;
}
