| Message ID | 20211012134915.38994-1-jlayton@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] ceph: skip existing superblocks that are blocklisted or shut down when mounting | expand |
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
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,
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
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; }