From patchwork Wed Dec 20 06:50:59 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Kent X-Patchwork-Id: 10125043 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 80FC76019C for ; Wed, 20 Dec 2017 06:51:20 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6C98C2960C for ; Wed, 20 Dec 2017 06:51:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6141029612; Wed, 20 Dec 2017 06:51:20 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E53622960C for ; Wed, 20 Dec 2017 06:51:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932405AbdLTGvG (ORCPT ); Wed, 20 Dec 2017 01:51:06 -0500 Received: from out3-smtp.messagingengine.com ([66.111.4.27]:51127 "EHLO out3-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932288AbdLTGvE (ORCPT ); Wed, 20 Dec 2017 01:51:04 -0500 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 3267920A52; Wed, 20 Dec 2017 01:51:04 -0500 (EST) Received: from frontend2 ([10.202.2.161]) by compute1.internal (MEProxy); Wed, 20 Dec 2017 01:51:04 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=themaw.net; h=cc :content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc; s=fm1; bh=MIF4AI+Lc5M9bqF8OlE5jfUgNrUOL jT4BP1fTlLFH5g=; b=hvnsxKwA6dH7zXw0YF6DCbpW2LYVCjNVmmL9AWVle+2OP TH0wI1dvdub0V7jKV0hpBVJTeJYsHZXdgLAc5Xl2om5Mcx+NJjyV8tdmufdbMpZ3 ZxnvkCsXaIUYmRO2s6m9a9OGTjotzU0Yi1UPeaXRYcq5dphXEuDNQX8uZxymEL+5 pTXAGrLZszhLveAVrBgCBdPpMQSfZe48k5HnhlwrslvcyBNkQXXywrL0ffk9jFJZ 99rfmkzsbQ9uzw3TleAz2USbgK6k9qioJkEqJ42/qVgAiM2/Bwdqox9OFIP7Ba1W LyScHF1XKWi8u1/xbVrNxc1I8ljA0AxpiPj5QmCOQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=MIF4AI +Lc5M9bqF8OlE5jfUgNrUOLjT4BP1fTlLFH5g=; b=Kbx4XfhIiNEHdskRGSHzFk 4eDoyrW5X2KMOO34FD5l0dBAqcHjQq0u7pzyvz4G7d0bEWlPrPf3T90YaO1ZbGx3 ZHfNG7KyHR+1qhHvfQUemNcxrMwB5hOmuOjNJPXE/raOlBSDlqMtCorWhXtrXGPO Z1iIPtarhxNo0/aAn1IpnS+VbpypSg0zwN20IRQc2uCht1Kgr7oqMVSxDtnCo2WO ptlMcsmPZSzqbuqASsQxs2OkEuuGq6tfwkabNXeRPv2IvCPiXx1Zb+CawjPvhBAz 7CaDwzsLOCmVwpbAhQlPJ1tDArrxrlLfJuEQltvEeaFhv+T4kRdJkYdcrg+0cAVA == X-ME-Sender: Received: from [192.168.1.28] (220-253-172-215.dyn.iinet.net.au [220.253.172.215]) by mail.messagingengine.com (Postfix) with ESMTPA id 6CEE0244DF; Wed, 20 Dec 2017 01:51:02 -0500 (EST) Subject: Re: [ANNOUNCE] autofs 5.1.2 release From: Ian Kent To: NeilBrown , autofs@vger.kernel.org Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org References: <1465960537.3164.1.camel@themaw.net> <876092krew.fsf@notabene.neil.brown.name> <5aee2303-d1c3-49fd-0ef9-362fb0256fd5@themaw.net> Message-ID: <5f98a504-90f1-0dd5-f536-38b24de2912e@themaw.net> Date: Wed, 20 Dec 2017 14:50:59 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 20/12/17 14:10, Ian Kent wrote: > On 20/12/17 13:52, Ian Kent wrote: >> On 20/12/17 11:29, NeilBrown wrote: >>> >>> Hi Ian, >>> I've been looking at: >>> >>>> - add configuration option to use fqdn in mounts. >>> >>> (commit 9aeef772604) because using this new option causes a regression. >>> If you are using the "replicated server" functionality, then >>> use_hostname_for_mounts = yes >>> completely disables it. >> >> Yes, that's not quite right. >> >> It disables the probe and proximity check for each distinct host >> name used. >> >> Each of the entries in the list of hosts should still be >> attempted and given that NFS ping is also now used in the NFS >> mount module what's lost is the preferred ordering of the hosts >> list. >> >>> >>> This is caused by: >>> >>> diff --git a/modules/replicated.c b/modules/replicated.c >>> index 32860d5fe245..8437f5f3d5b2 100644 >>> --- a/modules/replicated.c >>> +++ b/modules/replicated.c >>> @@ -667,6 +667,12 @@ 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 */ >>> >>> My question is: why what this particular change made. >> >> It was a while ago but there were complains about using the IP >> address for mounts. It was requested to provide a way to prevent >> that and force the use of the host name in mounts. >> >>> Why can't prune_host_list() be allowed to do it's thing >>> when use_hostname_for_mounts is set. >> >> We could if each host name resolved to a single IP address. >> >> I'd need to check that use_hostname_for_mounts doesn't get >> in the road but the host struct should have ->rr set to true >> if it has multiple addresses so changing it to work the way >> your recommending shouldn't be hard. I think there's a couple >> of places that would need to be checked. >> >> If the host does resolve to multiple addresses the situation >> is different. There's no way to stop the actual mount from >> trying an IP address that's not responding and proximity >> doesn't make sense either again because every time a lookup >> is done on the host name (eg. at mount time) the next address >> in its list will be returned which can and usually is different >> from what would have been checked. >> >>> I understand that it would be pointless choosing between >>> the different interfaces of a multi-homed host, but there is still value >>> in choosing between multiple distinct hosts. >>> >>> What, if anything, might go wrong if I simply reverse this chunk of the >>> patch? >> >> You'll get IP addresses in the logs in certain cases but that >> should be all. >> >> It would probably be better to ensure that the checks are done >> if the host name resolves to a single IP address. > > I think that should be "if the host names in the list each resolve > to a single IP address", otherwise the round robin behavior would > probably still get in the road. I think maybe this is sufficient .... autofs-5.1.4 - use proximity check if all host names are simple From: Ian Kent Currently if the configuration option use_hostname_for_mounts is set then the proximity calcualtion is not done for the list of hosts. But if each host name in the host list resolves to a single IP address then performing the proximity check still makes sense. Signed-off-by: Ian Kent --- modules/replicated.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/modules/replicated.c b/modules/replicated.c index 3ac4c70f..e5c2276d 100644 --- a/modules/replicated.c +++ b/modules/replicated.c @@ -711,6 +711,24 @@ done: return 0; } +static unsigned int is_hosts_list_simple(struct host *list) +{ + struct host *this = list; + unsigned int ret = 1; + + while (this) { + struct host *next = this->next; + + if (this->rr) { + ret = 0; + break; + } + this = next; + } + + return ret; +} + int prune_host_list(unsigned logopt, struct host **list, unsigned int vers, int port) { @@ -726,12 +744,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; @@ -767,6 +779,14 @@ int prune_host_list(unsigned logopt, struct host **list, return 1; } + /* If we're using the host name then there's no point probing + * avialability and respose time unless all host names in the + * list each resolve to a single address. + */ + if (defaults_use_hostname_for_mounts() && + !is_hosts_list_simple(this)) + return 1; + proximity = this->proximity; while (this) { struct host *next = this->next;