[ANNOUNCE] autofs 5.1.2 release

[ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

Hi all,

An update is overdue so here it is.

It's mostly a bug fix update.

autofs
======

The package can be found at:

ftp://ftp.kernel.org/pub/linux/daemons/autofs/v5

It is autofs-5.1.2.tar.[gz|xz]

No source rpm is there as it can be produced by using:

rpmbuild -ts autofs-5.1.2.tar.gz

and the binary rpm by using:

rpmbuild -tb autofs-5.1.2.tar.gz

See the README.amd-maps file for information about using amd format
maps.

Here are the entries from the CHANGELOG which outline the updates:

15/06/2015 autofs-5.1.2
=======================
- update libtirpc workaround for new soname.
- revert fix libtirpc name clash.
- fix left mount count return from umount_multi_triggers().
- fix return handling in sss lookup module.
- move query dn calculation from do_bind() to do_connect().
- make do_connect() return a status.
- make connect_to_server() return a status.
- make find_dc_server() return a status.
- make find_server() return a status.
- fix return handling of do_reconnect() in ldap module.
- fix rwlock unlock crash.
- fix config old name lookup.
- fix error handling on ldap bind fail.
- fix direct mount stale instance flag reset.
- fix direct map expire not set for initail empty map.
- fix missing source sss in multi map lookup.
- fix update_hosts_mounts() return.
- change lookup to use reinit instead of reopen.
- update map_hash_table_size description.
- add configuration option to use fqdn in mounts.
- fix out of order call in program map lookup.
- fix error handling of is_mounted().
- Add a mode option for master map entries.
- define monotonic clock helper functions.
- use monotonic clock for alarm thread condition wait.
- define pending condition init helper function.
- use monotonic clock for direct mount condition.
- use monotonic clock for indirect mount condition.
- change remaining gettimeofday() to use clock_gettime().
- change time() to use monotonic_clock().
- remove unused function elapsed().
- fix unbind sasl external mech.
- fix sasl connection concurrancy problem.
- fix memory leak in nisplus lookup_reinit().
- fix memory leak in ldap do_init().
- fix use after free in sun parser parse_init().
- fix use after free in open_lookup().
- fix typo in autofs_sasl_bind().
- fix memory leak in get_network_proximity().
- fix use after free in match_my_name().
- improve scalability of direct mount path component.
- always set direct mounts catatonic at exit.
- fix use-after-free in st_queue_handler().
- log pipe read errors.
- fix handle_mounts() termination condition check.
- fix Makefile linking dependencies.
- fix modules make clean target.
- fix autofs(5) description of supported map sources.
- add autofs(5) note of IPv6 libtirpc requirement.
- add remote-fs.target systemd dependency.
- fix typo in autofs.conf.
- fix yp map age not updated during map lookup.
- add config option to supress not found log message.
- fix possible memory leak in nfs mount.

Ian

[ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

Hi all,

An update is overdue so here it is.

It's mostly a bug fix update.

autofs
======

The package can be found at:

ftp://ftp.kernel.org/pub/linux/daemons/autofs/v5

It is autofs-5.1.2.tar.[gz|xz]

No source rpm is there as it can be produced by using:

rpmbuild -ts autofs-5.1.2.tar.gz

and the binary rpm by using:

rpmbuild -tb autofs-5.1.2.tar.gz

See the README.amd-maps file for information about using amd format
maps.

Here are the entries from the CHANGELOG which outline the updates:

15/06/2015 autofs-5.1.2
=======================
- update libtirpc workaround for new soname.
- revert fix libtirpc name clash.
- fix left mount count return from umount_multi_triggers().
- fix return handling in sss lookup module.
- move query dn calculation from do_bind() to do_connect().
- make do_connect() return a status.
- make connect_to_server() return a status.
- make find_dc_server() return a status.
- make find_server() return a status.
- fix return handling of do_reconnect() in ldap module.
- fix rwlock unlock crash.
- fix config old name lookup.
- fix error handling on ldap bind fail.
- fix direct mount stale instance flag reset.
- fix direct map expire not set for initail empty map.
- fix missing source sss in multi map lookup.
- fix update_hosts_mounts() return.
- change lookup to use reinit instead of reopen.
- update map_hash_table_size description.
- add configuration option to use fqdn in mounts.
- fix out of order call in program map lookup.
- fix error handling of is_mounted().
- Add a mode option for master map entries.
- define monotonic clock helper functions.
- use monotonic clock for alarm thread condition wait.
- define pending condition init helper function.
- use monotonic clock for direct mount condition.
- use monotonic clock for indirect mount condition.
- change remaining gettimeofday() to use clock_gettime().
- change time() to use monotonic_clock().
- remove unused function elapsed().
- fix unbind sasl external mech.
- fix sasl connection concurrancy problem.
- fix memory leak in nisplus lookup_reinit().
- fix memory leak in ldap do_init().
- fix use after free in sun parser parse_init().
- fix use after free in open_lookup().
- fix typo in autofs_sasl_bind().
- fix memory leak in get_network_proximity().
- fix use after free in match_my_name().
- improve scalability of direct mount path component.
- always set direct mounts catatonic at exit.
- fix use-after-free in st_queue_handler().
- log pipe read errors.
- fix handle_mounts() termination condition check.
- fix Makefile linking dependencies.
- fix modules make clean target.
- fix autofs(5) description of supported map sources.
- add autofs(5) note of IPv6 libtirpc requirement.
- add remote-fs.target systemd dependency.
- fix typo in autofs.conf.
- fix yp map age not updated during map lookup.
- add config option to supress not found log message.
- fix possible memory leak in nfs mount.

Ian
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Re: [ANNOUNCE] autofs 5.1.2 release

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1290 bytes --]


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.

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.
Why can't prune_host_list() be allowed to do it's thing
when use_hostname_for_mounts is set.
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?

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

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.

Ian

Re: [ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

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.

Ian
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Re: [ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

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.

Ian

Re: [ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

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@themaw.net>

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@themaw.net>
---
 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;

Re: [ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

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@themaw.net>

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@themaw.net>
---
 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;
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Re: [ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

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.

Mmm .... that's also not right.

An NFS ping is only done on failed local bind mount to check
the NFS server is running on the local machine.

So that availability check needs to be done at mount time if
the proximity check is not done ....

Ian

Re: [ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

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.

Mmm .... that's also not right.

An NFS ping is only done on failed local bind mount to check
the NFS server is running on the local machine.

So that availability check needs to be done at mount time if
the proximity check is not done ....

Ian
--
To unsubscribe from this list: send the line "unsubscribe autofs" in

Re: [ANNOUNCE] autofs 5.1.2 release

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 6333 bytes --]

On Wed, Dec 20 2017, 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 cannot see why the round-robin behavior would get in the road.
It might be pointless to probe each IP address on a multi-homed host if
we are just going to mount by host name, but I don't see how it hurts.

So this is what I'm thinking.  Some simple testing suggests that
it does the right things.

If a host has addresses with different proximity they will still be
probed separately, but this won't affect the final choice.

Thanks,
NeilBrown

--------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 */
 		} 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;

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

On 21/12/17 09:09, NeilBrown wrote:
> On Wed, Dec 20 2017, 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 cannot see why the round-robin behavior would get in the road.
> It might be pointless to probe each IP address on a multi-homed host if
> we are just going to mount by host name, but I don't see how it hurts.

I was wondering whether I had considered the configuration option
when setting ->rr of the host struct. I haven't so it should be set
if the name resolves to multiple addresses.

> 
> So this is what I'm thinking.  Some simple testing suggests that
> it does the right things.
> 
> If a host has addresses with different proximity they will still be
> probed separately, but this won't affect the final choice.

Well no but ...

The prune_host_list() function is meant to put the list of hosts
in proximity order with hosts ordered by response time within proximity.

It's also meant to remove hosts that don't respond from the list so that
mount attempts are not made to them (hence the need to use address).

When use_hostname_for_mounts is set there will be multiple host structures
with the same host name and each would be probed by name so all would be
considered.

But the probe is not tied to IP address so each probe failure would remove
one of the structures from the list resulting in a reduced list of hosts
to try to mount.

So it's then quite possible a mount attempt will be made to a host that
isn't responding with an associated long delay, around 3 minutes IIRC.
That's not good for the interactive nature of autofs.

It's also possible that a number of the hosts in the list are not
responding causing a portion of them to be removed making the list
smaller and the likelihood that the mount will fail when one of the
hosts actually would succeed but isn't tried because of the reduced
list size. Worse, is the long delay for each host that's not responding.

What I was trying to get at in my previous reply is that when
use_hostname_for_mounts is set and the hosts haven't been probed then
an nfs_ping should be done before attempting the mount to avoid the long
delay.

Before calling the prune function all the potential hosts are present on
the list so I think pruning the list isn't going to be of any benefit and
will cause problems of the type I described.

You have certainly identified a problem and I'd like to fix it but what is
the problem your seeing and trying to resolve?

Maybe we are approaching this the wrong way.

> 
> Thanks,
> NeilBrown
> 
> --------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 */
>  		} 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;
> 

Re: [ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

On 21/12/17 19:06, Ian Kent wrote:
> On 21/12/17 09:09, NeilBrown wrote:
>> On Wed, Dec 20 2017, 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 cannot see why the round-robin behavior would get in the road.
>> It might be pointless to probe each IP address on a multi-homed host if
>> we are just going to mount by host name, but I don't see how it hurts.
> 
> I was wondering whether I had considered the configuration option
> when setting ->rr of the host struct. I haven't so it should be set
> if the name resolves to multiple addresses.
> 
>>
>> So this is what I'm thinking.  Some simple testing suggests that
>> it does the right things.
>>
>> If a host has addresses with different proximity they will still be
>> probed separately, but this won't affect the final choice.
> 
> Well no but ...
> 
> The prune_host_list() function is meant to put the list of hosts
> in proximity order with hosts ordered by response time within proximity.
> 
> It's also meant to remove hosts that don't respond from the list so that
> mount attempts are not made to them (hence the need to use address).
> 
> When use_hostname_for_mounts is set there will be multiple host structures
> with the same host name and each would be probed by name so all would be
> considered.
> 
> But the probe is not tied to IP address so each probe failure would remove
> one of the structures from the list resulting in a reduced list of hosts
> to try to mount.
> 
> So it's then quite possible a mount attempt will be made to a host that
> isn't responding with an associated long delay, around 3 minutes IIRC.
> That's not good for the interactive nature of autofs.
> 
> It's also possible that a number of the hosts in the list are not
> responding causing a portion of them to be removed making the list
> smaller and the likelihood that the mount will fail when one of the
> hosts actually would succeed but isn't tried because of the reduced
> list size. Worse, is the long delay for each host that's not responding.
> 
> What I was trying to get at in my previous reply is that when
> use_hostname_for_mounts is set and the hosts haven't been probed then
> an nfs_ping should be done before attempting the mount to avoid the long
> delay.
> 
> Before calling the prune function all the potential hosts are present on
> the list so I think pruning the list isn't going to be of any benefit and
> will cause problems of the type I described.
> 
> You have certainly identified a problem and I'd like to fix it but what is
> the problem your seeing and trying to resolve?

Oh wait, sorry, I didn't read far enough.

You concern is hosts with weighting ..... I'll need to look at that
and the patch you've offered, ;)

> 
> Maybe we are approaching this the wrong way.
> 
>>
>> Thanks,
>> NeilBrown
>>
>> --------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.

I don't understand your thinking wrt. interfaces.

I don't think the possibility a target machine offers the file system on
different interfaces can be considered as the only possibility in this
scenario.

The replicated server machines might offer the same file system
on multiple interfaces but they could easily be distinct hosts possibly
even in different geographic locations.

>>
>> 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 */

Yeah but when the mount is actually tried you'll most likely get a different
IP address which could easily be a different host entirely.

Just how file systems are replicated is (I believe) undefined so we have no
way of knowing if the host IP used will belong to the same host as the one
tried above.

Certainly, correct me if I'm wrong, ;)

>>  		} 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;
>>
> 

Re: [ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

On 21/12/17 19:06, Ian Kent wrote:
> On 21/12/17 09:09, NeilBrown wrote:
>> On Wed, Dec 20 2017, 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 cannot see why the round-robin behavior would get in the road.
>> It might be pointless to probe each IP address on a multi-homed host if
>> we are just going to mount by host name, but I don't see how it hurts.
> 
> I was wondering whether I had considered the configuration option
> when setting ->rr of the host struct. I haven't so it should be set
> if the name resolves to multiple addresses.
> 
>>
>> So this is what I'm thinking.  Some simple testing suggests that
>> it does the right things.
>>
>> If a host has addresses with different proximity they will still be
>> probed separately, but this won't affect the final choice.
> 
> Well no but ...
> 
> The prune_host_list() function is meant to put the list of hosts
> in proximity order with hosts ordered by response time within proximity.
> 
> It's also meant to remove hosts that don't respond from the list so that
> mount attempts are not made to them (hence the need to use address).
> 
> When use_hostname_for_mounts is set there will be multiple host structures
> with the same host name and each would be probed by name so all would be
> considered.
> 
> But the probe is not tied to IP address so each probe failure would remove
> one of the structures from the list resulting in a reduced list of hosts
> to try to mount.
> 
> So it's then quite possible a mount attempt will be made to a host that
> isn't responding with an associated long delay, around 3 minutes IIRC.
> That's not good for the interactive nature of autofs.
> 
> It's also possible that a number of the hosts in the list are not
> responding causing a portion of them to be removed making the list
> smaller and the likelihood that the mount will fail when one of the
> hosts actually would succeed but isn't tried because of the reduced
> list size. Worse, is the long delay for each host that's not responding.
> 
> What I was trying to get at in my previous reply is that when
> use_hostname_for_mounts is set and the hosts haven't been probed then
> an nfs_ping should be done before attempting the mount to avoid the long
> delay.
> 
> Before calling the prune function all the potential hosts are present on
> the list so I think pruning the list isn't going to be of any benefit and
> will cause problems of the type I described.
> 
> You have certainly identified a problem and I'd like to fix it but what is
> the problem your seeing and trying to resolve?

Oh wait, sorry, I didn't read far enough.

You concern is hosts with weighting ..... I'll need to look at that
and the patch you've offered, ;)

> 
> Maybe we are approaching this the wrong way.
> 
>>
>> Thanks,
>> NeilBrown
>>
>> --------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.

I don't understand your thinking wrt. interfaces.

I don't think the possibility a target machine offers the file system on
different interfaces can be considered as the only possibility in this
scenario.

The replicated server machines might offer the same file system
on multiple interfaces but they could easily be distinct hosts possibly
even in different geographic locations.

>>
>> 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 */

Yeah but when the mount is actually tried you'll most likely get a different
IP address which could easily be a different host entirely.

Just how file systems are replicated is (I believe) undefined so we have no
way of knowing if the host IP used will belong to the same host as the one
tried above.

Certainly, correct me if I'm wrong, ;)

>>  		} 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

Re: [ANNOUNCE] autofs 5.1.2 release

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 11507 bytes --]

On Thu, Dec 21 2017, Ian Kent wrote:

> On 21/12/17 19:06, Ian Kent wrote:
>> On 21/12/17 09:09, NeilBrown wrote:
>>> On Wed, Dec 20 2017, 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 cannot see why the round-robin behavior would get in the road.
>>> It might be pointless to probe each IP address on a multi-homed host if
>>> we are just going to mount by host name, but I don't see how it hurts.
>> 
>> I was wondering whether I had considered the configuration option
>> when setting ->rr of the host struct. I haven't so it should be set
>> if the name resolves to multiple addresses.
>> 
>>>
>>> So this is what I'm thinking.  Some simple testing suggests that
>>> it does the right things.
>>>
>>> If a host has addresses with different proximity they will still be
>>> probed separately, but this won't affect the final choice.
>> 
>> Well no but ...
>> 
>> The prune_host_list() function is meant to put the list of hosts
>> in proximity order with hosts ordered by response time within proximity.
>> 
>> It's also meant to remove hosts that don't respond from the list so that
>> mount attempts are not made to them (hence the need to use address).
>> 
>> When use_hostname_for_mounts is set there will be multiple host structures
>> with the same host name and each would be probed by name so all would be
>> considered.
>> 
>> But the probe is not tied to IP address so each probe failure would remove
>> one of the structures from the list resulting in a reduced list of hosts
>> to try to mount.
>> 
>> So it's then quite possible a mount attempt will be made to a host that
>> isn't responding with an associated long delay, around 3 minutes IIRC.
>> That's not good for the interactive nature of autofs.
>> 
>> It's also possible that a number of the hosts in the list are not
>> responding causing a portion of them to be removed making the list
>> smaller and the likelihood that the mount will fail when one of the
>> hosts actually would succeed but isn't tried because of the reduced
>> list size. Worse, is the long delay for each host that's not responding.
>> 
>> What I was trying to get at in my previous reply is that when
>> use_hostname_for_mounts is set and the hosts haven't been probed then
>> an nfs_ping should be done before attempting the mount to avoid the long
>> delay.
>> 
>> Before calling the prune function all the potential hosts are present on
>> the list so I think pruning the list isn't going to be of any benefit and
>> will cause problems of the type I described.
>> 
>> You have certainly identified a problem and I'd like to fix it but what is
>> the problem your seeing and trying to resolve?
>
> Oh wait, sorry, I didn't read far enough.
>
> You concern is hosts with weighting ..... I'll need to look at that
> and the patch you've offered, ;)
>
>> 
>> Maybe we are approaching this the wrong way.
>> 
>>>
>>> Thanks,
>>> NeilBrown
>>>
>>> --------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.
>
> I don't understand your thinking wrt. interfaces.

In my mind, "different interfaces on a host" and "different IP addresses
for a host name" are effectively the same thing.
If we mount by hostname then you cannot choose which IP address,
so you cannot choose which interface, so there is no point trying.

But maybe the two concepts are distinct, so I should have written

  This is little point, in this case, selecting between different IP
  address for the one hostname, but it is still worth selecting between
  distinct host names, particularly if different weights have been
  specified.
  

>
> I don't think the possibility a target machine offers the file system on
> different interfaces can be considered as the only possibility in this
> scenario.
>
> The replicated server machines might offer the same file system
> on multiple interfaces but they could easily be distinct hosts possibly
> even in different geographic locations.

Different hosts in different locations having the same hostname?  That
would be a circumstance where I think the best solution would be to
disabled use_hostname_for_mount.

When a hostname does always refer to a single host, and may have
multiple IP addresses only if it has multiple interfaces, then
use_hostname_for_mount is easily supported and should not prevent
selecting between different hosts which have been listed as replicas.

>
>>>
>>> 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 */
>
> Yeah but when the mount is actually tried you'll most likely get a different
> IP address which could easily be a different host entirely.

If you've set use_hostname_for_mount=yes, then you've explicitly said
that you don't care about that possibility.

>
> Just how file systems are replicated is (I believe) undefined so we have no
> way of knowing if the host IP used will belong to the same host as the one
> tried above.
>
> Certainly, correct me if I'm wrong, ;)

I don't think you are wrong.
I just think that while "use_hostname_for_mount=yes" might prevent
autofs from providing a perfect selecting of the best address to mount,
it shouldn't prevent autofs from doing the best it can given the
information it has.

Thanks,
NeilBrown


>
>>>  		} 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;
>>>
>> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

On 03/01/18 06:14, NeilBrown wrote:
> On Thu, Dec 21 2017, Ian Kent wrote:
> 
>> On 21/12/17 19:06, Ian Kent wrote:
>>> On 21/12/17 09:09, NeilBrown wrote:
>>>> On Wed, Dec 20 2017, 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 cannot see why the round-robin behavior would get in the road.
>>>> It might be pointless to probe each IP address on a multi-homed host if
>>>> we are just going to mount by host name, but I don't see how it hurts.
>>>
>>> I was wondering whether I had considered the configuration option
>>> when setting ->rr of the host struct. I haven't so it should be set
>>> if the name resolves to multiple addresses.
>>>
>>>>
>>>> So this is what I'm thinking.  Some simple testing suggests that
>>>> it does the right things.
>>>>
>>>> If a host has addresses with different proximity they will still be
>>>> probed separately, but this won't affect the final choice.
>>>
>>> Well no but ...
>>>
>>> The prune_host_list() function is meant to put the list of hosts
>>> in proximity order with hosts ordered by response time within proximity.
>>>
>>> It's also meant to remove hosts that don't respond from the list so that
>>> mount attempts are not made to them (hence the need to use address).
>>>
>>> When use_hostname_for_mounts is set there will be multiple host structures
>>> with the same host name and each would be probed by name so all would be
>>> considered.
>>>
>>> But the probe is not tied to IP address so each probe failure would remove
>>> one of the structures from the list resulting in a reduced list of hosts
>>> to try to mount.
>>>
>>> So it's then quite possible a mount attempt will be made to a host that
>>> isn't responding with an associated long delay, around 3 minutes IIRC.
>>> That's not good for the interactive nature of autofs.
>>>
>>> It's also possible that a number of the hosts in the list are not
>>> responding causing a portion of them to be removed making the list
>>> smaller and the likelihood that the mount will fail when one of the
>>> hosts actually would succeed but isn't tried because of the reduced
>>> list size. Worse, is the long delay for each host that's not responding.
>>>
>>> What I was trying to get at in my previous reply is that when
>>> use_hostname_for_mounts is set and the hosts haven't been probed then
>>> an nfs_ping should be done before attempting the mount to avoid the long
>>> delay.
>>>
>>> Before calling the prune function all the potential hosts are present on
>>> the list so I think pruning the list isn't going to be of any benefit and
>>> will cause problems of the type I described.
>>>
>>> You have certainly identified a problem and I'd like to fix it but what is
>>> the problem your seeing and trying to resolve?
>>
>> Oh wait, sorry, I didn't read far enough.
>>
>> You concern is hosts with weighting ..... I'll need to look at that
>> and the patch you've offered, ;)
>>
>>>
>>> Maybe we are approaching this the wrong way.
>>>
>>>>
>>>> Thanks,
>>>> NeilBrown
>>>>
>>>> --------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.
>>
>> I don't understand your thinking wrt. interfaces.
> 
> In my mind, "different interfaces on a host" and "different IP addresses
> for a host name" are effectively the same thing.

The whole reason we are having this difficulty in communicating and
the reason it ended up being done this way is that I don't believe
that, necessarily, a host name that resolves to multiple addresses
is a single host.

For example a DNS round robin setup might have:

www1.domain.com 192.168.1.200
www2.domain.com 192.168.1.201
www3.domain.com 192.168.1.202

all distinct devices and

www.domain.com www1,www2,www3

It isn't specified how a file system is replicated and originally
it was required replicated file systems be read-only (which Linux
autofs never enforced).

There are storage array products which can replicate file systems
so it's not a stretch to think that there are change managed file
systems that are replicated to multiple distinct hosts for read-only
use.

But ...

> If we mount by hostname then you cannot choose which IP address,
> so you cannot choose which interface, so there is no point trying.
> 
> But maybe the two concepts are distinct, so I should have written
> 
>   This is little point, in this case, selecting between different IP
>   address for the one hostname, but it is still worth selecting between
>   distinct host names, particularly if different weights have been
>   specified.

You are right, it "is" still worth selecting between different host
names and I understand and agree with with what you have been saying.

The difficulty is possible problems introduced by the case I'm claiming
exits and that case needs to be mitigated in some way.

Obviously time is not my friend lately so we'll need to think about
this and discus it as and when time permits.

>   
> 
>>
>> I don't think the possibility a target machine offers the file system on
>> different interfaces can be considered as the only possibility in this
>> scenario.
>>
>> The replicated server machines might offer the same file system
>> on multiple interfaces but they could easily be distinct hosts possibly
>> even in different geographic locations.
> 
> Different hosts in different locations having the same hostname?  That
> would be a circumstance where I think the best solution would be to
> disabled use_hostname_for_mount.
> 
> When a hostname does always refer to a single host, and may have
> multiple IP addresses only if it has multiple interfaces, then
> use_hostname_for_mount is easily supported and should not prevent
> selecting between different hosts which have been listed as replicas.
> 
>>
>>>>
>>>> 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 */
>>
>> Yeah but when the mount is actually tried you'll most likely get a different
>> IP address which could easily be a different host entirely.
> 
> If you've set use_hostname_for_mount=yes, then you've explicitly said
> that you don't care about that possibility.
> 
>>
>> Just how file systems are replicated is (I believe) undefined so we have no
>> way of knowing if the host IP used will belong to the same host as the one
>> tried above.
>>
>> Certainly, correct me if I'm wrong, ;)
> 
> I don't think you are wrong.
> I just think that while "use_hostname_for_mount=yes" might prevent
> autofs from providing a perfect selecting of the best address to mount,
> it shouldn't prevent autofs from doing the best it can given the
> information it has.
> 
> Thanks,
> NeilBrown
> 
> 
>>
>>>>  		} 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;
>>>>
>>>

Re: [ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

On 03/01/18 06:14, NeilBrown wrote:
> On Thu, Dec 21 2017, Ian Kent wrote:
> 
>> On 21/12/17 19:06, Ian Kent wrote:
>>> On 21/12/17 09:09, NeilBrown wrote:
>>>> On Wed, Dec 20 2017, 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 cannot see why the round-robin behavior would get in the road.
>>>> It might be pointless to probe each IP address on a multi-homed host if
>>>> we are just going to mount by host name, but I don't see how it hurts.
>>>
>>> I was wondering whether I had considered the configuration option
>>> when setting ->rr of the host struct. I haven't so it should be set
>>> if the name resolves to multiple addresses.
>>>
>>>>
>>>> So this is what I'm thinking.  Some simple testing suggests that
>>>> it does the right things.
>>>>
>>>> If a host has addresses with different proximity they will still be
>>>> probed separately, but this won't affect the final choice.
>>>
>>> Well no but ...
>>>
>>> The prune_host_list() function is meant to put the list of hosts
>>> in proximity order with hosts ordered by response time within proximity.
>>>
>>> It's also meant to remove hosts that don't respond from the list so that
>>> mount attempts are not made to them (hence the need to use address).
>>>
>>> When use_hostname_for_mounts is set there will be multiple host structures
>>> with the same host name and each would be probed by name so all would be
>>> considered.
>>>
>>> But the probe is not tied to IP address so each probe failure would remove
>>> one of the structures from the list resulting in a reduced list of hosts
>>> to try to mount.
>>>
>>> So it's then quite possible a mount attempt will be made to a host that
>>> isn't responding with an associated long delay, around 3 minutes IIRC.
>>> That's not good for the interactive nature of autofs.
>>>
>>> It's also possible that a number of the hosts in the list are not
>>> responding causing a portion of them to be removed making the list
>>> smaller and the likelihood that the mount will fail when one of the
>>> hosts actually would succeed but isn't tried because of the reduced
>>> list size. Worse, is the long delay for each host that's not responding.
>>>
>>> What I was trying to get at in my previous reply is that when
>>> use_hostname_for_mounts is set and the hosts haven't been probed then
>>> an nfs_ping should be done before attempting the mount to avoid the long
>>> delay.
>>>
>>> Before calling the prune function all the potential hosts are present on
>>> the list so I think pruning the list isn't going to be of any benefit and
>>> will cause problems of the type I described.
>>>
>>> You have certainly identified a problem and I'd like to fix it but what is
>>> the problem your seeing and trying to resolve?
>>
>> Oh wait, sorry, I didn't read far enough.
>>
>> You concern is hosts with weighting ..... I'll need to look at that
>> and the patch you've offered, ;)
>>
>>>
>>> Maybe we are approaching this the wrong way.
>>>
>>>>
>>>> Thanks,
>>>> NeilBrown
>>>>
>>>> --------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.
>>
>> I don't understand your thinking wrt. interfaces.
> 
> In my mind, "different interfaces on a host" and "different IP addresses
> for a host name" are effectively the same thing.

The whole reason we are having this difficulty in communicating and
the reason it ended up being done this way is that I don't believe
that, necessarily, a host name that resolves to multiple addresses
is a single host.

For example a DNS round robin setup might have:

www1.domain.com 192.168.1.200
www2.domain.com 192.168.1.201
www3.domain.com 192.168.1.202

all distinct devices and

www.domain.com www1,www2,www3

It isn't specified how a file system is replicated and originally
it was required replicated file systems be read-only (which Linux
autofs never enforced).

There are storage array products which can replicate file systems
so it's not a stretch to think that there are change managed file
systems that are replicated to multiple distinct hosts for read-only
use.

But ...

> If we mount by hostname then you cannot choose which IP address,
> so you cannot choose which interface, so there is no point trying.
> 
> But maybe the two concepts are distinct, so I should have written
> 
>   This is little point, in this case, selecting between different IP
>   address for the one hostname, but it is still worth selecting between
>   distinct host names, particularly if different weights have been
>   specified.

You are right, it "is" still worth selecting between different host
names and I understand and agree with with what you have been saying.

The difficulty is possible problems introduced by the case I'm claiming
exits and that case needs to be mitigated in some way.

Obviously time is not my friend lately so we'll need to think about
this and discus it as and when time permits.

>   
> 
>>
>> I don't think the possibility a target machine offers the file system on
>> different interfaces can be considered as the only possibility in this
>> scenario.
>>
>> The replicated server machines might offer the same file system
>> on multiple interfaces but they could easily be distinct hosts possibly
>> even in different geographic locations.
> 
> Different hosts in different locations having the same hostname?  That
> would be a circumstance where I think the best solution would be to
> disabled use_hostname_for_mount.
> 
> When a hostname does always refer to a single host, and may have
> multiple IP addresses only if it has multiple interfaces, then
> use_hostname_for_mount is easily supported and should not prevent
> selecting between different hosts which have been listed as replicas.
> 
>>
>>>>
>>>> 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 */
>>
>> Yeah but when the mount is actually tried you'll most likely get a different
>> IP address which could easily be a different host entirely.
> 
> If you've set use_hostname_for_mount=yes, then you've explicitly said
> that you don't care about that possibility.
> 
>>
>> Just how file systems are replicated is (I believe) undefined so we have no
>> way of knowing if the host IP used will belong to the same host as the one
>> tried above.
>>
>> Certainly, correct me if I'm wrong, ;)
> 
> I don't think you are wrong.
> I just think that while "use_hostname_for_mount=yes" might prevent
> autofs from providing a perfect selecting of the best address to mount,
> it shouldn't prevent autofs from doing the best it can given the
> information it has.
> 
> Thanks,
> NeilBrown
> 
> 
>>
>>>>  		} 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

Re: [ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

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;
> 

Re: [ANNOUNCE] autofs 5.1.2 release

[Ian Kent]

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