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 <raven@xxxxxxxxxx>
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 <raven@xxxxxxxxxx>
---
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;
