[PATCH v3] ceph: skip existing superblocks that are blocklisted or shut down when mounting

[PATCH v3] ceph: skip existing superblocks that are blocklisted or shut down when mounting

[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 <pdonnell@redhat.com>
Cc: Niels de Vos <ndevos@redhat.com>
Cc: "Yan, Zheng" <ukernel@gmail.com>
Cc: stable@vger.kernel.org
URL: https://bugzilla.redhat.com/show_bug.cgi?id=1901499
Reviewed-by: Xiubo Li <xiubli@redhat.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>

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

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 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

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index f517ad9eeb26..b9ba50c9dc95 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -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;
 }
 
-- 
2.31.1

Re: [PATCH v3] ceph: skip existing superblocks that are blocklisted or shut down when mounting

[Ilya Dryomov]

On Tue, Oct 12, 2021 at 3:49 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> 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 <pdonnell@redhat.com>
> Cc: Niels de Vos <ndevos@redhat.com>
> Cc: "Yan, Zheng" <ukernel@gmail.com>
> Cc: stable@vger.kernel.org
> URL: https://bugzilla.redhat.com/show_bug.cgi?id=1901499
> Reviewed-by: Xiubo Li <xiubli@redhat.com>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>
> ceph: when comparing superblocks, skip ones that have been forcibly unmounted
>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>

Hi Jeff,

This looks like a squashing left-over.

> ---
>  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
>
> diff --git a/fs/ceph/super.c b/fs/ceph/super.c
> index f517ad9eeb26..b9ba50c9dc95 100644
> --- a/fs/ceph/super.c
> +++ b/fs/ceph/super.c
> @@ -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 */

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

Thanks,

                Ilya

Re: [PATCH v3] ceph: skip existing superblocks that are blocklisted or shut down when mounting

[Jeff Layton]

On Tue, 2021-10-12 at 21:05 +0200, Ilya Dryomov wrote:
> On Tue, Oct 12, 2021 at 3:49 PM Jeff Layton <jlayton@kernel.org> wrote:
> > 
> > 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 <pdonnell@redhat.com>
> > Cc: Niels de Vos <ndevos@redhat.com>
> > Cc: "Yan, Zheng" <ukernel@gmail.com>
> > Cc: stable@vger.kernel.org
> > URL: https://bugzilla.redhat.com/show_bug.cgi?id=1901499
> > Reviewed-by: Xiubo Li <xiubli@redhat.com>
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > 
> > ceph: when comparing superblocks, skip ones that have been forcibly unmounted
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> 
> Hi Jeff,
> 
> This looks like a squashing left-over.
> 

Yep, that can be removed.

> > ---
> >  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
> > 
> > diff --git a/fs/ceph/super.c b/fs/ceph/super.c
> > index f517ad9eeb26..b9ba50c9dc95 100644
> > --- a/fs/ceph/super.c
> > +++ b/fs/ceph/super.c
> > @@ -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 */
> 
> Nit: given the dout message that follows, this comment seems useless.
> 
> 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,
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH v3] ceph: skip existing superblocks that are blocklisted or shut down when mounting

[Ilya Dryomov]

On Tue, Oct 12, 2021 at 9:42 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> On Tue, 2021-10-12 at 21:05 +0200, Ilya Dryomov wrote:
> > On Tue, Oct 12, 2021 at 3:49 PM Jeff Layton <jlayton@kernel.org> wrote:
> > >
> > > 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 <pdonnell@redhat.com>
> > > Cc: Niels de Vos <ndevos@redhat.com>
> > > Cc: "Yan, Zheng" <ukernel@gmail.com>
> > > Cc: stable@vger.kernel.org
> > > URL: https://bugzilla.redhat.com/show_bug.cgi?id=1901499
> > > Reviewed-by: Xiubo Li <xiubli@redhat.com>
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > >
> > > ceph: when comparing superblocks, skip ones that have been forcibly unmounted
> > >
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> >
> > Hi Jeff,
> >
> > This looks like a squashing left-over.
> >
>
> Yep, that can be removed.
>
> > > ---
> > >  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
> > >
> > > diff --git a/fs/ceph/super.c b/fs/ceph/super.c
> > > index f517ad9eeb26..b9ba50c9dc95 100644
> > > --- a/fs/ceph/super.c
> > > +++ b/fs/ceph/super.c
> > > @@ -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 */
> >
> > Nit: given the dout message that follows, this comment seems useless.
> >
> > 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