From: Ian Kent <raven@themaw.net>
To: NeilBrown <neilb@suse.com>, autofs@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [ANNOUNCE] autofs 5.1.2 release
Date: Thu, 18 Jan 2018 10:19:57 +0800 [thread overview]
Message-ID: <0db6bdd1-d096-ceeb-9f81-779af13a3c0d@themaw.net> (raw)
In-Reply-To: <87po78khtj.fsf@notabene.neil.brown.name>
On 21/12/17 09:09, NeilBrown wrote:
> --------8<---------------
> Subject: use_hostname_for_mounts shouldn't prevent selection among replica
>
> If several replicas have been specified for a mount point, and
> use_hostname_for_mount is set to "yes", the selection between
> these replicas is currently disabled and the last in the list is always
> chosen.
>
> There is little point selecting between different interfaces on the one
> host in this case, but it is still worth selecting between different
> hosts, particularly if different weights have been specified.
>
> This patch restores the "prune_host_list()" functionality when
> use_hostname_for_mount is set, and modifies it slightly so that once
> an IP address with a given proximity has been successfully probed,
> other IP address for the same host(weight):/path and proximity are ignored.
>
> Signed-off-by: NeilBrown <neilb@suse.com>
>
> diff --git a/modules/replicated.c b/modules/replicated.c
> index 3ac4c70f4062..16cf873513ff 100644
> --- a/modules/replicated.c
> +++ b/modules/replicated.c
> @@ -714,7 +714,7 @@ done:
> int prune_host_list(unsigned logopt, struct host **list,
> unsigned int vers, int port)
> {
> - struct host *this, *last, *first;
> + struct host *this, *last, *first, *prev;
> struct host *new = NULL;
> unsigned int proximity, selected_version = 0;
> unsigned int v2_tcp_count, v3_tcp_count, v4_tcp_count;
> @@ -726,12 +726,6 @@ int prune_host_list(unsigned logopt, struct host **list,
> if (!*list)
> return 0;
>
> - /* If we're using the host name then there's no point probing
> - * avialability and respose time.
> - */
> - if (defaults_use_hostname_for_mounts())
> - return 1;
> -
> /* Use closest hosts to choose NFS version */
>
> first = *list;
> @@ -877,11 +871,18 @@ int prune_host_list(unsigned logopt, struct host **list,
>
> first = last;
> this = first;
> + prev = NULL;
> while (this) {
> struct host *next = this->next;
> if (!this->name) {
> remove_host(list, this);
> add_host(&new, this);
> + } else if (defaults_use_hostname_for_mounts() && prev &&
> + prev->proximity == this->proximity &&
> + strcmp(prev->name, this->name) == 0 &&
> + strcmp(prev->path, this->path) == 0 &&
> + prev->weight == this->weight) {
> + /* No need to probe same host(weight):/path again */
Mmm ... so maybe I'm the one that's missing the point.
You are trying to eliminate multiple occurrences of list entries that
correspond to a specific host name entry from probing.
It might be sensible to add a "this->rr" following the
defaults_use_hostname_for_mounts() check to avoid the additional
checks when the host doesn't have additional addresses, particularly
the string comparison.
There's nothing stopping people from adding this same host name with a
different weight, even though that doesn't seem like a sensible thing
to do.
I'm not sure if this exposes mounting to problems that aren't already
present with the current implementation.
I'll think a little more about that case but at first glance the DNS
round robin problem of addresses referring to different devices is
still present, a possible false negative.
But that problem exits in the current implementation too as a round
robin lookup can just as easily return an address of a host that isn't
responding at mount time.....
> } else {
> status = get_supported_ver_and_cost(logopt, this,
> selected_version, port);
> @@ -889,6 +890,7 @@ int prune_host_list(unsigned logopt, struct host **list,
> this->version = selected_version;
> remove_host(list, this);
> add_host(&new, this);
> + prev = this;
> }
> }
> this = next;
>
--
To unsubscribe from this list: send the line "unsubscribe autofs" in
next prev parent reply other threads:[~2018-01-18 2:19 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-15 3:15 [ANNOUNCE] autofs 5.1.2 release Ian Kent
2017-12-20 3:29 ` NeilBrown
2017-12-20 5:52 ` Ian Kent
2017-12-20 6:10 ` Ian Kent
2017-12-20 6:50 ` Ian Kent
2017-12-21 1:09 ` NeilBrown
2017-12-21 11:06 ` Ian Kent
2017-12-21 11:36 ` Ian Kent
2018-01-02 22:14 ` NeilBrown
2018-01-18 1:15 ` Ian Kent
2018-01-18 2:19 ` Ian Kent [this message]
2017-12-20 7:30 ` Ian Kent
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=0db6bdd1-d096-ceeb-9f81-779af13a3c0d@themaw.net \
--to=raven@themaw.net \
--cc=autofs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=neilb@suse.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 read-only IMAP folder(s) and NNTP newsgroup(s).