ceph: skip existing superblocks that are blocklisted or shut down when mounting

PATCH In review Jeff Layton 2 participants 4 messages Oct 12, 2021

Subsystems (2)

Stable branch Ceph client

lore

Share & tools

lore

Jeff Layton

author

2021-10-12 13:49 UTC #0 lore raw

PATCH v3 1/1 ceph: skip existing superblocks that are blocklisted or shut down when mounting

Currently when mounting, we may end up finding an existing superblock that corresponds to a blocklisted MDS client. This means that the new mount ends up being unusable.

If we've found an existing superblock with a client that is already blocklisted, and the client is not configured to recover on its own, fail the match. Ditto if the superblock has been forcibly unmounted.

While we're in here, also rename "other" to the more conventional "fsc".

Cc: Patrick Donnelly <[email protected]>
Cc: Niels de Vos <[email protected]>
Cc: "Yan, Zheng" <[email protected]>
Cc: [email protected]
URL: https://bugzilla.redhat.com/show_bug.cgi?id=1901499

Reviewed-by Xiubo Li <[email protected]>

Signed-off-by Jeff Layton <[email protected]>

ceph: when comparing superblocks, skip ones that have been forcibly unmounted

Signed-off-by Jeff Layton <[email protected]>

---
 fs/ceph/super.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

v3: also handle the case where we have a forcibly unmounted superblock
that is detached but still extant

fs/ceph/super.c +14 -3

index f517ad9eeb26..b9ba50c9dc95 100644
@@ -1123,16 +1123,16 @@ static int ceph_compare_super(struct super_block *sb, struct fs_context *fc)
 	struct ceph_fs_client *new = fc->s_fs_info;
 	struct ceph_mount_options *fsopt = new->mount_options;
 	struct ceph_options *opt = new->client->options;
-	struct ceph_fs_client *other = ceph_sb_to_client(sb);
+	struct ceph_fs_client *fsc = ceph_sb_to_client(sb);
 
 	dout("ceph_compare_super %p\n", sb);
 
-	if (compare_mount_options(fsopt, opt, other)) {
+	if (compare_mount_options(fsopt, opt, fsc)) {
 		dout("monitor(s)/mount options don't match\n");
 		return 0;
 	}
 	if ((opt->flags & CEPH_OPT_FSID) &&
-	    ceph_fsid_compare(&opt->fsid, &other->client->fsid)) {
+	    ceph_fsid_compare(&opt->fsid, &fsc->client->fsid)) {
 		dout("fsid doesn't match\n");
 		return 0;
 	}

@@ -1140,6 +1140,17 @@ static int ceph_compare_super(struct super_block *sb, struct fs_context *fc)
 		dout("flags differ\n");
 		return 0;
 	}
+	/* Exclude any blocklisted superblocks */
+	if (fsc->blocklisted && !ceph_test_mount_opt(fsc, CLEANRECOVER)) {
+		dout("client is blocklisted (and CLEANRECOVER is not set)\n");
+		return 0;
+	}
+
+	if (fsc->mount_state == CEPH_MOUNT_SHUTDOWN) {
+		dout("client has been forcibly unmounted\n");
+		return 0;
+	}
+
 	return 1;
 }

Signature · 1 line

2.31.1

Ilya Dryomov

maintainer

2021-10-12 19:06 UTC #1 lore raw

Quoted 16 lines · Jeff Layton

Currently when mounting, we may end up finding an existing superblock that corresponds to a blocklisted MDS client. This means that the new mount ends up being unusable.
If we've found an existing superblock with a client that is already blocklisted, and the client is not configured to recover on its own, fail the match. Ditto if the superblock has been forcibly unmounted.
While we're in here, also rename "other" to the more conventional "fsc".
Cc: Patrick Donnelly <[email protected]>
Cc: Niels de Vos <[email protected]>
Cc: "Yan, Zheng" <[email protected]>
Cc: [email protected]
URL: https://bugzilla.redhat.com/show_bug.cgi?id=1901499

Quoted 6 lines

Reviewed-by Xiubo Li <[email protected]>
Signed-off-by Jeff Layton <[email protected]>
ceph: when comparing superblocks, skip ones that have been forcibly unmounted
Signed-off-by Jeff Layton <[email protected]>

Hi Jeff,

This looks like a squashing left-over.

Quoted 32 lines

---
 fs/ceph/super.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

v3: also handle the case where we have a forcibly unmounted superblock
that is detached but still extant

fs/ceph/super.c +3 -3

index f517ad9eeb26..b9ba50c9dc95 100644
@@ -1123,16 +1123,16 @@ static int ceph_compare_super(struct super_block *sb, struct fs_context *fc)
        struct ceph_fs_client *new = fc->s_fs_info;
        struct ceph_mount_options *fsopt = new->mount_options;
        struct ceph_options *opt = new->client->options;
-       struct ceph_fs_client *other = ceph_sb_to_client(sb);
+       struct ceph_fs_client *fsc = ceph_sb_to_client(sb);
 
        dout("ceph_compare_super %p\n", sb);
 
-       if (compare_mount_options(fsopt, opt, other)) {
+       if (compare_mount_options(fsopt, opt, fsc)) {
                dout("monitor(s)/mount options don't match\n");
                return 0;
        }
        if ((opt->flags & CEPH_OPT_FSID) &&
-           ceph_fsid_compare(&opt->fsid, &other->client->fsid)) {
+           ceph_fsid_compare(&opt->fsid, &fsc->client->fsid)) {
                dout("fsid doesn't match\n");
                return 0;
        }

fs/ceph/super.c static int ceph_compare_super(struct super_block *sb, struct fs_context *fc) · line 1143

Quoted 4 lines

fs/ceph/super.c +1 -0

@@ -1140,6 +1140,17 @@ static int ceph_compare_super(struct super_block *sb, struct fs_context *fc)
                dout("flags differ\n");
                return 0;
        }
+       /* Exclude any blocklisted superblocks */

Nit: given the dout message that follows, this comment seems useless.

Thanks,

Ilya

Jeff Layton

author

2021-10-12 19:42 UTC #2 lore raw

Quoted 20 lines · Ilya Dryomov

Quoted 19 lines · Jeff Layton
Currently when mounting, we may end up finding an existing superblock that corresponds to a blocklisted MDS client. This means that the new mount ends up being unusable.
If we've found an existing superblock with a client that is already blocklisted, and the client is not configured to recover on its own, fail the match. Ditto if the superblock has been forcibly unmounted.
While we're in here, also rename "other" to the more conventional "fsc".
Cc: Patrick Donnelly <[email protected]>
Cc: Niels de Vos <[email protected]>
Cc: "Yan, Zheng" <[email protected]>
Cc: [email protected]
URL: https://bugzilla.redhat.com/show_bug.cgi?id=1901499
Reviewed-by Xiubo Li <[email protected]>
Signed-off-by Jeff Layton <[email protected]>

Quoted 8 lines

Quoted 3 lines
ceph: when comparing superblocks, skip ones that have been forcibly unmounted
Signed-off-by Jeff Layton <[email protected]>
Hi Jeff,
This looks like a squashing left-over.

Yep, that can be removed.

Quoted 39 lines

Quoted 32 lines

---
 fs/ceph/super.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

v3: also handle the case where we have a forcibly unmounted superblock
that is detached but still extant

fs/ceph/super.c +3 -3

index f517ad9eeb26..b9ba50c9dc95 100644
@@ -1123,16 +1123,16 @@ static int ceph_compare_super(struct super_block *sb, struct fs_context *fc)
        struct ceph_fs_client *new = fc->s_fs_info;
        struct ceph_mount_options *fsopt = new->mount_options;
        struct ceph_options *opt = new->client->options;
-       struct ceph_fs_client *other = ceph_sb_to_client(sb);
+       struct ceph_fs_client *fsc = ceph_sb_to_client(sb);
 
        dout("ceph_compare_super %p\n", sb);
 
-       if (compare_mount_options(fsopt, opt, other)) {
+       if (compare_mount_options(fsopt, opt, fsc)) {
                dout("monitor(s)/mount options don't match\n");
                return 0;
        }
        if ((opt->flags & CEPH_OPT_FSID) &&
-           ceph_fsid_compare(&opt->fsid, &other->client->fsid)) {
+           ceph_fsid_compare(&opt->fsid, &fsc->client->fsid)) {
                dout("fsid doesn't match\n");
                return 0;
        }

fs/ceph/super.c static int ceph_compare_super(struct super_block *sb, struct fs_context *fc) · line 1143

Quoted 4 lines

fs/ceph/super.c +1 -0

@@ -1140,6 +1140,17 @@ static int ceph_compare_super(struct super_block *sb, struct fs_context *fc)
                dout("flags differ\n");
                return 0;
        }
+       /* Exclude any blocklisted superblocks */

Nit: given the dout message that follows, this comment seems useless.

Quoted 3 lines

Thanks,
Ilya

I'm fine with removing that comment. Would you rather I resend, or do you just want to fix up the nits in-tree?

Thanks,

Signature · 1 line

Jeff Layton <[email protected]>

Ilya Dryomov

maintainer

2021-10-12 20:06 UTC #3 lore raw

Quoted 72 lines · Jeff Layton

Quoted 20 lines · Ilya Dryomov
Quoted 19 lines · Jeff Layton
Currently when mounting, we may end up finding an existing superblock that corresponds to a blocklisted MDS client. This means that the new mount ends up being unusable.
If we've found an existing superblock with a client that is already blocklisted, and the client is not configured to recover on its own, fail the match. Ditto if the superblock has been forcibly unmounted.
While we're in here, also rename "other" to the more conventional "fsc".
Cc: Patrick Donnelly <[email protected]>
Cc: Niels de Vos <[email protected]>
Cc: "Yan, Zheng" <[email protected]>
Cc: [email protected]
URL: https://bugzilla.redhat.com/show_bug.cgi?id=1901499
Reviewed-by Xiubo Li <[email protected]>
Signed-off-by Jeff Layton <[email protected]>
Quoted 8 lines
Quoted 3 lines
ceph: when comparing superblocks, skip ones that have been forcibly unmounted
Signed-off-by Jeff Layton <[email protected]>
Hi Jeff,
This looks like a squashing left-over.
Yep, that can be removed.
Quoted 39 lines
Quoted 32 lines
---
 fs/ceph/super.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)
v3: also handle the case where we have a forcibly unmounted superblock
that is detached but still extant
fs/ceph/super.c +3 -3
index f517ad9eeb26..b9ba50c9dc95 100644
@@ -1123,16 +1123,16 @@ static int ceph_compare_super(struct super_block *sb, struct fs_context *fc)
        struct ceph_fs_client *new = fc->s_fs_info;
        struct ceph_mount_options *fsopt = new->mount_options;
        struct ceph_options *opt = new->client->options;
-       struct ceph_fs_client *other = ceph_sb_to_client(sb);
+       struct ceph_fs_client *fsc = ceph_sb_to_client(sb);
 
        dout("ceph_compare_super %p\n", sb);
 
-       if (compare_mount_options(fsopt, opt, other)) {
+       if (compare_mount_options(fsopt, opt, fsc)) {
                dout("monitor(s)/mount options don't match\n");
                return 0;
        }
        if ((opt->flags & CEPH_OPT_FSID) &&
-           ceph_fsid_compare(&opt->fsid, &other->client->fsid)) {
+           ceph_fsid_compare(&opt->fsid, &fsc->client->fsid)) {
                dout("fsid doesn't match\n");
                return 0;
        }
fs/ceph/super.c static int ceph_compare_super(struct super_block *sb, struct fs_context *fc) · line 1143
Quoted 4 lines
fs/ceph/super.c +1 -0
@@ -1140,6 +1140,17 @@ static int ceph_compare_super(struct super_block *sb, struct fs_context *fc)
                dout("flags differ\n");
                return 0;
        }
+       /* Exclude any blocklisted superblocks */
Nit: given the dout message that follows, this comment seems useless.

Quoted 6 lines

Quoted 3 lines
Thanks,
Ilya
I'm fine with removing that comment. Would you rather I resend, or do you just want to fix up the nits in-tree?

The squashing left-over is already fixed. I'll remove the comment later.

Thanks,

Ilya