autofs-5.0.1 - dont umount existing direct mount on master re-read

From: Ian Kent <raven@themaw.net>

Since direct mounts can have multiple entries in the master map they each
have an instance associated with them. If one entry changes, such as the
mount options, the instance comparison test fails and a new instance is
added. This causes autofs to get confused because there are now two
entries that contain the same mount information in different internal
caches. There are several consequences of this, most of which are just
noise in the log, but it also causes confuion for the expiration of mounts
since, for an active mount, the old cache entry can't be pruned until it's
umounted. Also, the map caches were not being properly pruned.
---

 daemon/lookup.c     |  162 ++++++++++++++++++++++++++++------------------------
 daemon/state.c      |   90 +++++++++++++++++++++-------
 include/automount.h |    1 
 3 files changed, 156 insertions(+), 97 deletions(-)


--- autofs-5.0.1.orig/daemon/lookup.c
+++ autofs-5.0.1/daemon/lookup.c
@@ -1013,96 +1013,114 @@ static char *make_fullpath(const char *r
 	return path;
 }
 
-int lookup_prune_cache(struct autofs_point *ap, time_t age)
+void lookup_prune_one_cache(struct autofs_point *ap, struct mapent_cache *mc, time_t age)
 {
-	struct master_mapent *entry = ap->entry;
-	struct map_source *map;
-	struct mapent_cache *mc;
 	struct mapent *me, *this;
 	char *path;
 	int status = CHE_FAIL;
 
-	pthread_cleanup_push(master_source_lock_cleanup, entry);
-	master_source_readlock(entry);
+	me = cache_enumerate(mc, NULL);
+	while (me) {
+		struct mapent *valid;
+		char *key = NULL, *next_key = NULL;
 
-	map = entry->maps;
-	while (map) {
-		/* Is the map stale */
-		if (!map->stale) {
-			map = map->next;
+		if (me->age >= age) {
+			me = cache_enumerate(mc, me);
 			continue;
 		}
-		mc = map->mc;
-		pthread_cleanup_push(cache_lock_cleanup, mc);
-		cache_readlock(mc);
-		me = cache_enumerate(mc, NULL);
-		while (me) {
-			char *key = NULL, *next_key = NULL;
-
-			if (me->age >= age) {
-				me = cache_enumerate(mc, me);
-				continue;
-			}
 
-			key = strdup(me->key);
-			me = cache_enumerate(mc, me);
-			if (!key || *key == '*') {
-				if (key)
-					free(key);
-				continue; 
-			}
-
-			path = make_fullpath(ap->path, key);
-			if (!path) {
-				warn(ap->logopt,
-				     "can't malloc storage for path");
+		key = strdup(me->key);
+		me = cache_enumerate(mc, me);
+		if (!key || *key == '*') {
+			if (key)
 				free(key);
-				continue;
-			}
+			continue;
+		}
 
-			if (is_mounted(_PATH_MOUNTED, path, MNTS_REAL)) {
-				debug(ap->logopt,
-				      "prune check posponed, %s mounted", path);
-				free(key);
-				free(path);
-				continue;
-			}
+		path = make_fullpath(ap->path, key);
+		if (!path) {
+			warn(ap->logopt, "can't malloc storage for path");
+			free(key);
+			continue;
+		}
 
-			if (me)
-				next_key = strdup(me->key);
+		/*
+		 * If this key has another valid entry we want to prune it,
+		 * even if it's a mount, as the valid entry will take the
+		 * mount if it is a direct mount or it's just a stale indirect
+		 * cache entry.
+		 */
+		valid = lookup_source_valid_mapent(ap, key, LKP_DISTINCT);
+		if (!valid &&
+		    is_mounted(_PATH_MOUNTED, path, MNTS_REAL)) {
+			debug(ap->logopt,
+			      "prune check posponed, %s mounted", path);
+			free(key);
+			free(path);
+			continue;
+		}
+		if (valid)
+			cache_unlock(valid->mc);
 
-			cache_unlock(mc);
+		if (me)
+			next_key = strdup(me->key);
 
-			cache_writelock(mc);
-			this = cache_lookup_distinct(mc, key);
-			if (!this) {
-				cache_unlock(mc);
-				goto next;
-			}
+		cache_unlock(mc);
 
-			if (!is_mounted(_PROC_MOUNTS, path, MNTS_AUTOFS)) {
-				status = CHE_FAIL;
-				if (this->ioctlfd == -1)
-					status = cache_delete(mc, key);
-				if (status != CHE_FAIL) {
-					if (ap->type == LKP_INDIRECT) {
-						if (ap->flags & MOUNT_FLAG_GHOST)
-							rmdir_path(ap, path, ap->dev);
-					} else
-						rmdir_path(ap, path, this->dev);
-				}
-			}
+		cache_writelock(mc);
+		this = cache_lookup_distinct(mc, key);
+		if (!this) {
 			cache_unlock(mc);
+			goto next;
+		}
 
-next:
-			cache_readlock(mc);
-			if (next_key) {
-				me = cache_lookup_distinct(mc, next_key);
-				free(next_key);
+		if (valid)
+			cache_delete(mc, key);
+		else if (!is_mounted(_PROC_MOUNTS, path, MNTS_AUTOFS)) {
+			status = CHE_FAIL;
+			if (this->ioctlfd == -1)
+				status = cache_delete(mc, key);
+			if (status != CHE_FAIL) {
+				if (ap->type == LKP_INDIRECT) {
+					if (ap->flags & MOUNT_FLAG_GHOST)
+						rmdir_path(ap, path, ap->dev);
+				} else
+					rmdir_path(ap, path, this->dev);
 			}
-			free(key);
-			free(path);
 		}
+		cache_unlock(mc);
+
+next:
+		cache_readlock(mc);
+		if (next_key) {
+			me = cache_lookup_distinct(mc, next_key);
+			free(next_key);
+		}
+		free(key);
+		free(path);
+	}
+
+	return;
+}
+
+int lookup_prune_cache(struct autofs_point *ap, time_t age)
+{
+	struct master_mapent *entry = ap->entry;
+	struct map_source *map;
+
+	pthread_cleanup_push(master_source_lock_cleanup, entry);
+	master_source_readlock(entry);
+
+	map = entry->maps;
+	while (map) {
+		/* Is the map stale */
+		if (!map->stale) {
+			map = map->next;
+			continue;
+		}
+		pthread_cleanup_push(cache_lock_cleanup, map->mc);
+		cache_readlock(map->mc);
+		lookup_prune_one_cache(ap, map->mc, age);
 		pthread_cleanup_pop(1);
 		map->stale = 0;
 		map = map->next;
@@ -1121,7 +1139,6 @@ struct mapent *lookup_source_valid_mapen
 	struct mapent_cache *mc;
 	struct mapent *me = NULL;
 
-	master_source_readlock(entry);
 	map = entry->maps;
 	while (map) {
 		/*
@@ -1144,7 +1161,6 @@ struct mapent *lookup_source_valid_mapen
 		cache_unlock(mc);
 		map = map->next;
 	}
-	master_source_unlock(entry);
 
 	return me;
 }
--- autofs-5.0.1.orig/daemon/state.c
+++ autofs-5.0.1/daemon/state.c
@@ -352,6 +352,68 @@ static void tree_mnts_cleanup(void *arg)
 	return;
 }
 
+static void do_readmap_mount(struct autofs_point *ap, struct mnt_list *mnts,
+			     struct map_source *map, struct mapent *me, time_t now)
+{
+	struct mapent_cache *nc;
+	struct mapent *ne, *nested, *valid;
+
+	nc = ap->entry->master->nc;
+
+	ne = cache_lookup_distinct(nc, me->key);
+	if (!ne) {
+		nested = cache_partial_match(nc, me->key);
+		if (nested) {
+			error(ap->logopt,
+			      "removing invalid nested null entry %s",
+			      nested->key);
+			nested = cache_partial_match(nc, me->key);
+			if (nested)
+				cache_delete(nc, nested->key);
+		}
+	}
+
+	if (me->age < now || (ne && map->master_line > ne->age)) {
+		/*
+		 * The map instance may have changed, such as the map name or
+		 * the mount options, but the direct map entry may still exist
+		 * in one of the other maps. If so then update the new cache
+		 * entry device and inode so we can find it at lookup. Later,
+		 * the mount for the new cache entry will just update the
+		 * timeout.
+		 *
+		 * TODO: how do we recognise these orphaned map instances. We
+		 * can't just delete these instances when the cache becomes
+		 * empty because that is a valid state for a master map entry.
+		 * This is becuase of the requirement to continue running with
+		 * an empty cache awaiting a map re-load.
+		 */
+		valid = lookup_source_valid_mapent(ap, me->key, LKP_DISTINCT);
+		if (valid) {
+			struct mapent_cache *vmc = valid->mc;
+			cache_unlock(vmc);
+			debug(ap->logopt,
+			     "updating cache entry for valid direct trigger %s",
+			     me->key);
+			cache_writelock(vmc);
+			valid = cache_lookup_distinct(vmc, me->key);
+			/* Take over the mount if there is one */
+			valid->ioctlfd = me->ioctlfd;
+			me->ioctlfd = -1;
+			/* Set device and inode number of the new mapent */
+			cache_set_ino_index(vmc, me->key, me->dev, me->ino);
+			cache_unlock(vmc);
+		} else if (!tree_is_mounted(mnts, me->key, MNTS_REAL))
+			do_umount_autofs_direct(ap, mnts, me);
+		else
+			debug(ap->logopt,
+			      "%s is mounted", me->key);
+	} else
+		do_mount_autofs_direct(ap, mnts, me);
+
+	return;
+}
+
 static void *do_readmap(void *arg)
 {
 	struct autofs_point *ap;
@@ -398,7 +460,8 @@ static void *do_readmap(void *arg)
 		lookup_prune_cache(ap, now);
 		status = lookup_ghost(ap, ap->path);
 	} else {
-		struct mapent *me, *ne, *nested;
+		struct mapent *me;
+
 		mnts = tree_make_mnt_tree(_PROC_MOUNTS, "/");
 		pthread_cleanup_push(tree_mnts_cleanup, mnts);
 		pthread_cleanup_push(master_source_lock_cleanup, ap->entry);
@@ -418,31 +481,10 @@ static void *do_readmap(void *arg)
 			cache_readlock(mc);
 			me = cache_enumerate(mc, NULL);
 			while (me) {
-				ne = cache_lookup_distinct(nc, me->key);
-				if (!ne) {
-					nested = cache_partial_match(nc, me->key);
-					if (nested) {
-						error(ap->logopt,
-						"removing invalid nested null entry %s",
-						nested->key);
-						nested = cache_partial_match(nc, me->key);
-						if (nested)
-							cache_delete(nc, nested->key);
-					}
-				}
-
-				/* TODO: check return of do_... */
-				if (me->age < now || (ne && map->master_line > ne->age)) {
-					if (!tree_is_mounted(mnts, me->key, MNTS_REAL))
-						do_umount_autofs_direct(ap, mnts, me);
-					else
-                                		debug(ap->logopt,
-						      "%s is mounted", me->key);
-				} else
-					do_mount_autofs_direct(ap, mnts, me);
-
+				do_readmap_mount(ap, mnts, map, me, now);
 				me = cache_enumerate(mc, me);
 			}
+			lookup_prune_one_cache(ap, map->mc, now);
 			pthread_cleanup_pop(1);
 			map->stale = 0;
 			map = map->next;
--- autofs-5.0.1.orig/include/automount.h
+++ autofs-5.0.1/include/automount.h
@@ -236,6 +236,7 @@ int lookup_enumerate(struct autofs_point
 int lookup_ghost(struct autofs_point *ap, const char *root);
 int lookup_nss_mount(struct autofs_point *ap, struct map_source *source, const char *name, int name_len);
 void lookup_close_lookup(struct autofs_point *ap);
+void lookup_prune_one_cache(struct autofs_point *ap, struct mapent_cache *mc, time_t age);
 int lookup_prune_cache(struct autofs_point *ap, time_t age);
 struct mapent *lookup_source_valid_mapent(struct autofs_point *ap, const char *key, unsigned int type);
 struct mapent *lookup_source_mapent(struct autofs_point *ap, const char *key, unsigned int type);