ceph-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Luis Henriques <lhenriques@suse.de>
To: Jeff Layton <jlayton@redhat.com>
Cc: Venky Shankar <vshankar@redhat.com>,
	idryomov@gmail.com, Xiubo Li <xiubli@redhat.com>,
	pdonnell@redhat.com, ceph-devel@vger.kernel.org
Subject: Re: [PATCH v4 4/5] ceph: record updated mon_addr on remount
Date: Wed, 14 Jul 2021 17:35:51 +0100	[thread overview]
Message-ID: <YO8SZ+Q3LaCt3K+V@suse.de> (raw)
In-Reply-To: <848d919c6a791ab9b7c61d7cb89f759b55195c18.camel@redhat.com>

On Wed, Jul 14, 2021 at 12:17:33PM -0400, Jeff Layton wrote:
> On Wed, 2021-07-14 at 15:35 +0530, Venky Shankar wrote:
> > Note that the new monitors are just shown in /proc/mounts.
> > Ceph does not (re)connect to new monitors yet.
> > 
> > Signed-off-by: Venky Shankar <vshankar@redhat.com>
> > ---
> >  fs/ceph/super.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/fs/ceph/super.c b/fs/ceph/super.c
> > index d8c6168b7fcd..d3a5a3729c5b 100644
> > --- a/fs/ceph/super.c
> > +++ b/fs/ceph/super.c
> > @@ -1268,6 +1268,13 @@ static int ceph_reconfigure_fc(struct fs_context *fc)
> >  	else
> >  		ceph_clear_mount_opt(fsc, ASYNC_DIROPS);
> >  
> > +	if (strcmp(fsc->mount_options->mon_addr, fsopt->mon_addr)) {
> > +		kfree(fsc->mount_options->mon_addr);
> > +		fsc->mount_options->mon_addr = fsopt->mon_addr;
> > +		fsopt->mon_addr = NULL;
> > +		printk(KERN_NOTICE "ceph: monitor addresses recorded, but not used for reconnection");
> 
> It's currently more in-vogue to use pr_notice() for this. I'll plan to
> make that (minor) change before I merge. No need to resend.

Yeah, this was the only comment I had too.  I saw some issues in the
previous revision but the changes to ceph_parse_source() seem to fix it in
this revision.

The other annoying thing I found isn't related with this patchset but with
a change that's been done some time ago by Xiubo (added to CC): it looks
like that if we have an invalid parameter (for example, wrong secret)
we'll always get -EHOSTUNREACH.

See below a possible fix (although I'm not entirely sure that's the correct
one).

Cheers,
--
Luís

From a988d24d8e72fc4933459f3dd5d303cbc9a566ed Mon Sep 17 00:00:00 2001
From: Luis Henriques <lhenriques@suse.de>
Date: Wed, 14 Jul 2021 16:56:36 +0100
Subject: [PATCH] ceph: don't hide error code if we don't have mdsmap

Since commit 97820058fb28 ("ceph: check availability of mds cluster on mount
after wait timeout") we're returning -EHOSTUNREACH, even if the error isn't
related with the MDSs availability.  For example, we'll get it even if we're
trying to mounting a filesystem with an invalid username or secret.

Only return this error if we get -EIO.

Fixes: 97820058fb28 ("ceph: check availability of mds cluster on mount after wait timeout")
Signed-off-by: Luis Henriques <lhenriques@suse.de>
---
 fs/ceph/super.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 086a1ceec9d8..67d70059ce9f 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -1230,7 +1230,8 @@ static int ceph_get_tree(struct fs_context *fc)
 	return 0;
 
 out_splat:
-	if (!ceph_mdsmap_is_cluster_available(fsc->mdsc->mdsmap)) {
+	if ((err == -EIO) &&
+	    !ceph_mdsmap_is_cluster_available(fsc->mdsc->mdsmap)) {
 		pr_info("No mds server is up or the cluster is laggy\n");
 		err = -EHOSTUNREACH;
 	}

  reply	other threads:[~2021-07-14 16:35 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-14 10:05 [PATCH v4 0/5] ceph: new mount device syntax Venky Shankar
2021-07-14 10:05 ` [PATCH v4 1/5] ceph: generalize addr/ip parsing based on delimiter Venky Shankar
2021-07-14 10:05 ` [PATCH v4 2/5] ceph: rename parse_fsid() to ceph_parse_fsid() and export Venky Shankar
2021-07-14 10:05 ` [PATCH v4 3/5] ceph: new device mount syntax Venky Shankar
2021-07-14 10:05 ` [PATCH v4 4/5] ceph: record updated mon_addr on remount Venky Shankar
2021-07-14 16:17   ` Jeff Layton
2021-07-14 16:35     ` Luis Henriques [this message]
2021-07-14 18:15       ` Jeff Layton
2021-07-15 11:42         ` Luis Henriques
2021-07-15  5:52     ` Venky Shankar
2021-07-14 18:05   ` Jeff Layton
2021-07-14 10:05 ` [PATCH v4 5/5] doc: document new CephFS mount device syntax Venky Shankar
2021-07-14 15:33 ` [PATCH v4 0/5] ceph: new " Jeff Layton
2021-07-16 18:47 ` Jeff Layton
2021-07-19  7:03   ` Venky Shankar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YO8SZ+Q3LaCt3K+V@suse.de \
    --to=lhenriques@suse.de \
    --cc=ceph-devel@vger.kernel.org \
    --cc=idryomov@gmail.com \
    --cc=jlayton@redhat.com \
    --cc=pdonnell@redhat.com \
    --cc=vshankar@redhat.com \
    --cc=xiubli@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).