Red Hat Bugzilla – Bug 1233069
Direct map does not expire if map is initially empty
Last modified: 2016-05-18 07:57:49 EDT
+++ This bug was initially created as a clone of Bug #1233057 +++ Description of problem: If an empty direct map is present at startup the expire alarm can't be set because the expire run frequency isn't known. If the map is re-read and is no longer empty the expire alarm isn't then set either. Version-Release number of selected component (if applicable): autofs.x86_64 1:5.0.5-109.el6 This problem was seen when working on bug 1227496 so a variation of the steps to reproduce the bug of 1227496 can be used to reproduce it. Steps to Reproduce: 1. Set autofs to produce debug log output to syslog. # grep debug /etc/autofs.conf # logging - set default log level "none", "verbose" or "debug" logging = debug and ensure rsyslog is logging facility daemon output # grep daemon /etc/rsyslog.conf daemon.* /var/log/debug 2. Add a direct mount map to auto.master. # grep direct /etc/auto.master /- /etc/auto.direct 3. Add a couple of entries to the direct mount map but leave them commented out. # cat /etc/auto.direct #/test/s1 bilbo:/Public #/test/s2 bilbo:/autofs 4. Start the autofs service # service autofs start Observe that there are no expire runs seen in the debug logging. The expire run frequency is timeout/4 so if we don't see an expire run within timeout time we know expres aren't happening. But, since there are no mounts in the map the expire run frequency isn't known so this is expected. 4. Modify /etc/auto.direct and uncomment the entries # cat /etc/auto.direct /test/s1 bilbo:/Public /test/s2 bilbo:/autofs 5. Reload the autofs maps # service autofs reload 6. Access one of the direct mount map entries causing it to be mounted. # ls /test/s1 7. Again observe that there are no expire runs seen in the debug log and the above mount is never expired. Waiting timeout time is sufficient to confirm this. Actual results: Direct mount is not expired. Expected results: Direct mount is expired and umounted roughly after timeout time (although this could be as much as (timeout + timeout/4).
Created attachment 1042096 [details] Patch - fix direct map expire not set for initial empty map
Hi, Ian, Thanks for discovering and reporting this bug, and I think I understand the root cause and the purpose of your patch. Based on more code reading, I think maybe it's better for us to put the supplement of "alarm_add" as following: --- ../autofs-5.0.7/daemon/state.c 2015-01-23 17:10:12.740443598 +0800 +++ daemon/state.c 2015-07-01 15:48:26.235034830 +0800 @@ -488,6 +486,7 @@ status = lookup_ghost(ap, ap->path); } else { struct mapent *me; + int append_alarm = !ap->exp_runfreq; mnts = tree_make_mnt_tree(_PROC_MOUNTS, "/"); pthread_cleanup_push(tree_mnts_cleanup, mnts); @@ -516,6 +515,13 @@ map->stale = 0; map = map->next; } + + /* If the direct mount map was empty at startup no expire alarm will + * have been added. So add it here if there are now map entries. + */ + if (append_alarm && ap->exp_runfreq) + alarm_add(ap, ap->exp_runfreq + rand() % ap->exp_runfreq); + pthread_cleanup_pop(1); pthread_cleanup_pop(1); pthread_cleanup_pop(1); Because in this way, we use the final ap->exp_runfreq as the timeout. And this behavor for alarm_add is consistent with the handle_mounts function. I am not sure, just proposed it, you decide it. Another question, what's the supposed behavor for expire if the map files(direct and indirect) become empty and reload called? Thanks.
This problem is introduced by the following patch, so it can only be reproduced since autofs-5.0.7+ commit f3dedc7bf56a8b226ee9bb7bade8d58954c0a7de Author: Leonardo Chiquitto <leonardo.lists@gmail.com> Date: Tue Jan 15 09:26:11 2013 +0800 autofs-5.0.7 - don't schedule new alarms after readmap
(In reply to XuWang from comment #2) > Hi, Ian, > > Thanks for discovering and reporting this bug, and I think I understand the > root cause and the purpose of your patch. > > Based on more code reading, I think maybe it's better for us to put the > supplement of "alarm_add" as following: > > --- ../autofs-5.0.7/daemon/state.c 2015-01-23 17:10:12.740443598 +0800 > +++ daemon/state.c 2015-07-01 15:48:26.235034830 +0800 > @@ -488,6 +486,7 @@ > status = lookup_ghost(ap, ap->path); > } else { > struct mapent *me; > + int append_alarm = !ap->exp_runfreq; > > mnts = tree_make_mnt_tree(_PROC_MOUNTS, "/"); > pthread_cleanup_push(tree_mnts_cleanup, mnts); > @@ -516,6 +515,13 @@ > map->stale = 0; > map = map->next; > } > + > + /* If the direct mount map was empty at startup no expire alarm will > + * have been added. So add it here if there are now map entries. > + */ > + if (append_alarm && ap->exp_runfreq) > + alarm_add(ap, ap->exp_runfreq + rand() % ap->exp_runfreq); > + > pthread_cleanup_pop(1); > pthread_cleanup_pop(1); > pthread_cleanup_pop(1); > > Because in this way, we use the final ap->exp_runfreq as the timeout. And > this behavor for alarm_add is consistent with the handle_mounts function. > I am not sure, just proposed it, you decide it. I like this patch better than mine, I'll use it. It has lower overhead because my change would cause an additional comparison to be done for every direct mount map entry whereas this will do a single comparison at the end. > > > Another question, what's the supposed behavor for expire if the map > files(direct and indirect) become empty and reload called? The alarm will continue to trigger. That was the expected behaviour before commit f3dedc7b. An improvement could be made to remove the alarm when the map becomes empty but isn't a high priority since maps usually aren't empty and the alarm overhead is small. Ian
(In reply to XuWang from comment #3) > This problem is introduced by the following patch, so it can only be > reproduced since autofs-5.0.7+ > > commit f3dedc7bf56a8b226ee9bb7bade8d58954c0a7de > Author: Leonardo Chiquitto <leonardo.lists@gmail.com> > Date: Tue Jan 15 09:26:11 2013 +0800 > > autofs-5.0.7 - don't schedule new alarms after readmap Yes, it looks like that at first sight but alarm_add(ap, 0) won't add an alarm (have a look at lib/alarm.c:alarm_add()). This patch isn't in RHEL-6 autofs but the problem is still present because of it. In fact that's were I first saw it. Ian
Created attachment 1045341 [details] Patch - fix direct map expire not set for initial empty map (updated)
run test case /autofs/sanity/reload for autofs-5.0.7-53.el7, seems good. autofs previous version: :: [ BEGIN ] :: Running 'tests/expire.sh' posix on Redirecting to /bin/systemctl stop autofs.service Redirecting to /bin/systemctl restart autofs.service :: [ BEGIN ] :: Running 'check_indirect_reload' Redirecting to /bin/systemctl reload autofs.service :: [ PASS ] :: Command 'check_indirect_reload' (Expected 0, got 0) :: [ BEGIN ] :: Running 'check_direct_reload' Redirecting to /bin/systemctl reload autofs.service :: [ PASS ] :: Command 'check_direct_reload' (Expected 0, got 0) :: [ BEGIN ] :: Running 'check_already_umount /indirect/plus' :: [ PASS ] :: Command 'check_already_umount /indirect/plus' (Expected 0, got 0) :: [ BEGIN ] :: Running 'check_already_umount /direct/plus' :: [ FAIL ] :: Command 'check_already_umount /direct/plus' (Expected 0, got 1) Redirecting to /bin/systemctl stop autofs.service :: [ PASS ] :: Command 'tests/expire.sh' (Expected 0, got 0) autofs-5.0.7-53.el7: [04:22:07 root@ ~~]# tests/expire.sh posix on Redirecting to /bin/systemctl stop autofs.service Redirecting to /bin/systemctl restart autofs.service :: [ BEGIN ] :: Running 'check_indirect_reload' Redirecting to /bin/systemctl reload autofs.service :: [ PASS ] :: Command 'check_indirect_reload' (Expected 0, got 0) :: [ BEGIN ] :: Running 'check_direct_reload' Redirecting to /bin/systemctl reload autofs.service :: [ PASS ] :: Command 'check_direct_reload' (Expected 0, got 0) :: [ BEGIN ] :: Running 'check_already_umount /indirect/plus' :: [ PASS ] :: Command 'check_already_umount /indirect/plus' (Expected 0, got 0) :: [ BEGIN ] :: Running 'check_already_umount /direct/plus' :: [ PASS ] :: Command 'check_already_umount /direct/plus' (Expected 0, got 0) Redirecting to /bin/systemctl stop autofs.service :: [ PASS ] :: Running 'tests/expire.sh' (Expected 0, got 0) ------------------------------------------------------------------------------- also run regression, connectathon, stress, bugzillas, sanity test cases for autofs-5.0.7-53.el7, covers x86_64, s390x, ppc64, ppc64le, no new issues. so I will change this bug status to be verified.
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://rhn.redhat.com/errata/RHSA-2015-2417.html