mdstat: Rework mdstat external arrays handling

To avoid repeating mdstat_read() in IncrementalRemove(), new function
mdstat_find_by_member_name() has been proposed. With that,
IncrementalRemove() handles own copy of mdstat content and there is no
need to repeat reading for external stop.

Additionally, It proposed few helper to avoid repeating
mdstat_ent->metadata_version checks across code.

Signed-off-by: Mariusz Tkaczyk <mariusz.tkaczyk@linux.intel.com>

12 files changed, 167 insertions, 124 deletions

diff --git a/Assemble.c b/Assemble.c
index 77f2b50e..a2bb7b64 100644
--- a/Assemble.c
+++ b/Assemble.c
@@ -114,14 +114,11 @@ static int is_member_busy(char *metadata_version)
 	int busy = 0;
 
 	for (ent = mdstat; ent; ent = ent->next) {
-		if (ent->metadata_version == NULL)
-			continue;
-		if (strncmp(ent->metadata_version, "external:", 9) != 0)
-			continue;
-		if (!is_subarray(&ent->metadata_version[9]))
+		if (!is_mdstat_ent_subarray(ent))
 			continue;
+
 		/* Skip first char - it can be '/' or '-' */
-		if (strcmp(&ent->metadata_version[10], metadata_version+1) == 0) {
+		if (strcmp(&ent->metadata_version[10], metadata_version + 1) == 0) {
 			busy = 1;
 			break;
 		}
diff --git a/Incremental.c b/Incremental.c
index 83db0712..abc7721b 100644
--- a/Incremental.c
+++ b/Incremental.c
@@ -1686,12 +1686,13 @@ static void remove_from_member_array(struct mdstat_ent *memb,
  */
 int IncrementalRemove(char *devname, char *id_path, int verbose)
 {
-	int mdfd;
-	int rv = 0;
-	struct mdstat_ent *ent;
+	struct mdstat_ent *ent = NULL;
+	char buf[SYSFS_MAX_BUF_SIZE];
+	struct mdstat_ent *mdstat;
 	struct mddev_dev devlist;
 	struct mdinfo mdi;
-	char buf[SYSFS_MAX_BUF_SIZE];
+	int rv = 1;
+	int mdfd;
 
 	if (!id_path)
 		dprintf("incremental removal without --path <id_path> lacks the possibility to re-add new device in this port\n");
@@ -1700,16 +1701,25 @@ int IncrementalRemove(char *devname, char *id_path, int verbose)
 		pr_err("incremental removal requires a kernel device name, not a file: %s\n", devname);
 		return 1;
 	}
-	ent = mdstat_by_component(devname);
+
+	mdstat = mdstat_read(0, 0);
+	if (!mdstat) {
+		pr_err("Cannot read /proc/mdstat file, aborting\n");
+		return 1;
+	}
+
+	ent = mdstat_find_by_member_name(mdstat, devname);
 	if (!ent) {
 		if (verbose >= 0)
 			pr_err("%s does not appear to be a component of any array\n", devname);
-		return 1;
+		goto out;
 	}
+
 	if (sysfs_init(&mdi, -1, ent->devnm)) {
 		pr_err("unable to initialize sysfs for: %s\n", devname);
-		return 1;
+		goto out;
 	}
+
 	mdfd = open_dev_excl(ent->devnm);
 	if (is_fd_valid(mdfd)) {
 		close_fd(&mdfd);
@@ -1725,8 +1735,7 @@ int IncrementalRemove(char *devname, char *id_path, int verbose)
 	if (mdfd < 0) {
 		if (verbose >= 0)
 			pr_err("Cannot open array %s!!\n", ent->devnm);
-		free_mdstat(ent);
-		return 1;
+		goto out;
 	}
 
 	if (id_path) {
@@ -1741,16 +1750,13 @@ int IncrementalRemove(char *devname, char *id_path, int verbose)
 	devlist.devname = devname;
 	devlist.disposition = 'I';
 	/* for a container, we must fail each member array */
-	if (ent->metadata_version &&
-	    strncmp(ent->metadata_version, "external:", 9) == 0) {
-		struct mdstat_ent *mdstat = mdstat_read(0, 0);
+	if (is_mdstat_ent_external(ent)) {
 		struct mdstat_ent *memb;
 		for (memb = mdstat ; memb ; memb = memb->next) {
 			if (is_container_member(memb, ent->devnm))
 				remove_from_member_array(memb,
 					&devlist, verbose);
 		}
-		free_mdstat(mdstat);
 	} else {
 		/*
 		 * This 'I' incremental remove is a try-best effort,
@@ -1765,7 +1771,8 @@ int IncrementalRemove(char *devname, char *id_path, int verbose)
 	rv = Manage_subdevs(ent->devnm, mdfd, &devlist,
 			    verbose, 0, UOPT_UNDEFINED, 0);
 
-	close(mdfd);
-	free_mdstat(ent);
+	close_fd(&mdfd);
+out:
+	free_mdstat(mdstat);
 	return rv;
 }
diff --git a/Manage.c b/Manage.c
index f0304e1e..241de055 100644
--- a/Manage.c
+++ b/Manage.c
@@ -276,10 +276,8 @@ int Manage_stop(char *devname, int fd, int verbose, int will_retry)
 		 */
 		mds = mdstat_read(0, 0);
 		for (m = mds; m; m = m->next)
-			if (m->metadata_version &&
-			    strncmp(m->metadata_version, "external:", 9)==0 &&
-			    metadata_container_matches(m->metadata_version+9,
-						       devnm)) {
+			if (is_mdstat_ent_external(m) &&
+			    metadata_container_matches(m->metadata_version + 9, devnm)) {
 				if (verbose >= 0)
 					pr_err("Cannot stop container %s: member %s still active\n",
 					       devname, m->devnm);
diff --git a/Monitor.c b/Monitor.c
index 26c53e13..cf14fbb3 100644
--- a/Monitor.c
+++ b/Monitor.c
@@ -879,9 +879,7 @@ static int check_array(struct state *st, struct mdstat_ent *mdstat,
 	}
 	last_disk = i;
 
-	if (mse->metadata_version &&
-	    strncmp(mse->metadata_version, "external:", 9) == 0 &&
-	    is_subarray(mse->metadata_version+9)) {
+	if (is_mdstat_ent_subarray(mse)) {
 		char *sl;
 		snprintf(st->parent_devnm, MD_NAME_MAX, "%s", mse->metadata_version + 10);
 		sl = strchr(st->parent_devnm, '/');
@@ -991,13 +989,12 @@ static int add_new_arrays(struct mdstat_ent *mdstat, struct state **statelist)
 			snprintf(st->devnm, MD_NAME_MAX, "%s", mse->devnm);
 			st->percent = RESYNC_UNKNOWN;
 			st->expected_spares = -1;
-			if (mse->metadata_version &&
-			    strncmp(mse->metadata_version,
-				    "external:", 9) == 0 &&
-			    is_subarray(mse->metadata_version+9)) {
+
+			if (is_mdstat_ent_subarray(mse)) {
 				char *sl;
-				snprintf(st->parent_devnm, MD_NAME_MAX,
-					 "%s", mse->metadata_version + 10);
+
+				snprintf(st->parent_devnm, MD_NAME_MAX, "%s",
+					 mse->metadata_version + 10);
 				sl = strchr(st->parent_devnm, '/');
 				if (sl)
 					*sl = 0;
@@ -1297,8 +1294,7 @@ int Wait(char *dev)
 			}
 		}
 		if (!e || e->percent == RESYNC_NONE) {
-			if (e && e->metadata_version &&
-			    strncmp(e->metadata_version, "external:", 9) == 0) {
+			if (e && is_mdstat_ent_external(e)) {
 				if (is_subarray(&e->metadata_version[9]))
 					ping_monitor(&e->metadata_version[9]);
 				else
diff --git a/config.c b/config.c
index 6ea905f3..5411a480 100644
--- a/config.c
+++ b/config.c
@@ -360,35 +360,38 @@ struct mddev_dev *load_partitions(void)
 struct mddev_dev *load_containers(void)
 {
 	struct mdstat_ent *mdstat = mdstat_read(0, 0);
+	struct mddev_dev *dev_list = NULL;
+	struct map_ent *map_list = NULL;
 	struct mdstat_ent *ent;
-	struct mddev_dev *d;
-	struct mddev_dev *rv = NULL;
-	struct map_ent *map = NULL, *me;
 
-	if (!mdstat)
-		return NULL;
+	for (ent = mdstat; ent; ent = ent->next) {
+		struct mddev_dev *d;
+		struct map_ent *map;
 
-	for (ent = mdstat; ent; ent = ent->next)
-		if (ent->metadata_version &&
-		    strncmp(ent->metadata_version, "external:", 9) == 0 &&
-		    !is_subarray(&ent->metadata_version[9])) {
-			d = xcalloc(1, sizeof(*d));
-			me = map_by_devnm(&map, ent->devnm);
-			if (me)
-				d->devname = xstrdup(me->path);
-			else if (asprintf(&d->devname, "/dev/%s", ent->devnm) < 0) {
-				free(d);
-				continue;
-			}
-			d->next = rv;
-			rv = d;
-			map_free(map);
-			map = NULL;
+		if (!is_mdstat_ent_external(ent))
+			continue;
+
+		if (is_mdstat_ent_subarray(ent))
+			continue;
+
+		d = xcalloc(1, sizeof(*d));
+
+		map = map_by_devnm(&map_list, ent->devnm);
+		if (map) {
+			d->devname = xstrdup(map->path);
+		} else if (asprintf(&d->devname, "/dev/%s", ent->devnm) < 0) {
+			free(d);
+			continue;
 		}
+
+		d->next = dev_list;
+		dev_list = d;
+	}
+
 	free_mdstat(mdstat);
-	map_free(map);
+	map_free(map_list);
 
-	return rv;
+	return dev_list;
 }
 
 struct createinfo createinfo = {
diff --git a/mapfile.c b/mapfile.c
index ea9837ac..632cf5e8 100644
--- a/mapfile.c
+++ b/mapfile.c
@@ -339,18 +339,14 @@ struct map_ent *map_by_name(struct map_ent **map, char *name)
  */
 static char *get_member_info(struct mdstat_ent *ent)
 {
+	char *subarray;
 
-	if (ent->metadata_version == NULL ||
-	    strncmp(ent->metadata_version, "external:", 9) != 0)
+	if (!is_mdstat_ent_subarray(ent))
 		return NULL;
 
-	if (is_subarray(&ent->metadata_version[9])) {
-		char *subarray;
+	subarray = strrchr(ent->metadata_version, '/');
 
-		subarray = strrchr(ent->metadata_version, '/');
-		return subarray + 1;
-	}
-	return NULL;
+	return subarray + 1;
 }
 
 void RebuildMap(void)
diff --git a/mdadm.h b/mdadm.h
index 22d5e8f4..5c3a9836 100644
--- a/mdadm.h
+++ b/mdadm.h
@@ -743,8 +743,12 @@ extern int mdstat_wait(int seconds);
 extern void mdstat_wait_fd(int fd, const sigset_t *sigmask);
 extern int mddev_busy(char *devnm);
 extern struct mdstat_ent *mdstat_by_component(char *name);
+extern struct mdstat_ent *mdstat_find_by_member_name(struct mdstat_ent *mdstat, char *member_devnm);
 extern struct mdstat_ent *mdstat_by_subdev(char *subdev, char *container);
 
+extern bool is_mdstat_ent_external(struct mdstat_ent *ent);
+extern bool is_mdstat_ent_subarray(struct mdstat_ent *ent);
+
 struct map_ent {
 	struct map_ent *next;
 	char	devnm[32];
@@ -1771,7 +1775,7 @@ extern int is_mddev(char *dev);
 extern int open_container(int fd);
 extern int metadata_container_matches(char *metadata, char *devnm);
 extern int metadata_subdev_matches(char *metadata, char *devnm);
-extern int is_container_member(struct mdstat_ent *ent, char *devname);
+extern bool is_container_member(struct mdstat_ent *ent, char *devname);
 extern int is_subarray_active(char *subarray, char *devname);
 extern int open_subarray(char *dev, char *subarray, struct supertype *st, int quiet);
 extern struct superswitch *version_to_superswitch(char *vers);
diff --git a/mdmon.c b/mdmon.c
index 5fdb5cdb..95e350f4 100644
--- a/mdmon.c
+++ b/mdmon.c
@@ -394,9 +394,7 @@ int main(int argc, char *argv[])
 		/* launch an mdmon instance for each container found */
 		mdstat = mdstat_read(0, 0);
 		for (e = mdstat; e; e = e->next) {
-			if (e->metadata_version &&
-			    strncmp(e->metadata_version, "external:", 9) == 0 &&
-			    !is_subarray(&e->metadata_version[9])) {
+			if (is_mdstat_ent_external(e) && !is_mdstat_ent_subarray(e)) {
 				/* update cmdline so this mdmon instance can be
 				 * distinguished from others in a call to ps(1)
 				 */
diff --git a/mdmon.h b/mdmon.h
index b3d72ac3..110cbef2 100644
--- a/mdmon.h
+++ b/mdmon.h
@@ -78,7 +78,7 @@ void do_manager(struct supertype *container);
 extern int sigterm;
 
 int read_dev_state(int fd);
-int is_container_member(struct mdstat_ent *mdstat, char *container);
+bool is_container_member(struct mdstat_ent *mdstat, char *container);
 
 struct mdstat_ent *mdstat_read(int hold, int start);
 
diff --git a/mdstat.c b/mdstat.c
index e233f094..cbbace3d 100644
--- a/mdstat.c
+++ b/mdstat.c
@@ -110,6 +110,28 @@ static int add_member_devname(struct dev_member **m, char *name)
 	return 1;
 }
 
+/* Detach element from the list, it may modify list_head */
+static void mdstat_ent_list_detach_element(struct mdstat_ent **list_head, struct mdstat_ent *el)
+{
+	struct mdstat_ent *ent = *list_head;
+
+	if (ent == el) {
+		*list_head = ent->next;
+	} else {
+		while (ent) {
+			if (ent->next == el) {
+				ent->next = el->next;
+				break;
+			}
+		}
+
+		ent = ent->next;
+	}
+
+	assert(ent);
+	ent->next = NULL;
+}
+
 void free_mdstat(struct mdstat_ent *ms)
 {
 	while (ms) {
@@ -124,6 +146,32 @@ void free_mdstat(struct mdstat_ent *ms)
 	}
 }
 
+bool is_mdstat_ent_external(struct mdstat_ent *ent)
+{
+	if (!ent->metadata_version)
+		return false;
+
+	if (strncmp(ent->metadata_version, "external:", 9) == 0)
+		return true;
+	return false;
+}
+
+bool is_mdstat_ent_subarray(struct mdstat_ent *ent)
+{
+	if (is_mdstat_ent_external(ent) && is_subarray(ent->metadata_version + 9))
+		return true;
+	return false;
+}
+
+bool is_container_member(struct mdstat_ent *mdstat, char *container)
+{
+	if (is_mdstat_ent_external(mdstat) &&
+	    metadata_container_matches(mdstat->metadata_version + 9, container))
+		return true;
+
+	return false;
+}
+
 static int mdstat_fd = -1;
 struct mdstat_ent *mdstat_read(int hold, int start)
 {
@@ -382,61 +430,70 @@ int mddev_busy(char *devnm)
 	return me != NULL;
 }
 
-struct mdstat_ent *mdstat_by_component(char *name)
+/**
+ * mdstat_find_by_member_devnm()- Return first array or external container with member device.
+ * @mdstat: Preloaded mdstat to iterate over.
+ * @member_devnm: devnm of the device to find.
+ *
+ * External subarrays are skipped.
+ */
+struct mdstat_ent *mdstat_find_by_member_name(struct mdstat_ent *mdstat, char *member_devnm)
 {
-	struct mdstat_ent *mdstat = mdstat_read(0, 0);
+	struct mdstat_ent *ent;
 
-	while (mdstat) {
-		struct dev_member *m;
-		struct mdstat_ent *ent;
-		if (mdstat->metadata_version &&
-		    strncmp(mdstat->metadata_version, "external:", 9) == 0 &&
-		    is_subarray(mdstat->metadata_version+9))
-			/* don't return subarrays, only containers */
-			;
-		else for (m = mdstat->members; m; m = m->next) {
-				if (strcmp(m->name, name) == 0) {
-					free_mdstat(mdstat->next);
-					mdstat->next = NULL;
-					return mdstat;
-				}
-			}
-		ent = mdstat;
-		mdstat = mdstat->next;
-		ent->next = NULL;
-		free_mdstat(ent);
+	for (ent = mdstat; ent; ent = ent->next) {
+		struct dev_member *member;
+
+		if (is_mdstat_ent_subarray(ent))
+			continue;
+
+		for (member = ent->members; member; member = member->next)
+			if (strcmp(member->name, member_devnm) == 0)
+				return ent;
 	}
+
 	return NULL;
 }
 
+
+struct mdstat_ent *mdstat_by_component(char *name)
+{
+	struct mdstat_ent *mdstat = mdstat_read(0, 0);
+	struct mdstat_ent *ent = mdstat_find_by_member_name(mdstat, name);
+
+	if (ent)
+		mdstat_ent_list_detach_element(&mdstat, ent);
+
+	free_mdstat(mdstat);
+
+	return ent;
+}
+
 struct mdstat_ent *mdstat_by_subdev(char *subdev, char *container)
 {
 	struct mdstat_ent *mdstat = mdstat_read(0, 0);
 	struct mdstat_ent *ent = NULL;
 
-	while (mdstat) {
+	for (ent = mdstat; ent; ent = ent->next) {
 		/* metadata version must match:
 		 *   external:[/-]%s/%s
 		 * where first %s is 'container' and second %s is 'subdev'
 		 */
-		if (ent)
-			free_mdstat(ent);
-		ent = mdstat;
-		mdstat = mdstat->next;
-		ent->next = NULL;
 
-		if (ent->metadata_version == NULL ||
-		    strncmp(ent->metadata_version, "external:", 9) != 0)
+		if (!is_mdstat_ent_external(ent))
 			continue;
 
-		if (!metadata_container_matches(ent->metadata_version+9,
-					       container) ||
-		    !metadata_subdev_matches(ent->metadata_version+9,
-					     subdev))
+		if (!metadata_container_matches(ent->metadata_version + 9, container))
+			continue;
+		if (!metadata_subdev_matches(ent->metadata_version + 9, subdev))
 			continue;
 
-		free_mdstat(mdstat);
-		return ent;
+		break;
 	}
-	return NULL;
+
+	if (ent)
+		mdstat_ent_list_detach_element(&mdstat, ent);
+
+	free_mdstat(mdstat);
+	return ent;
 }
diff --git a/super-intel.c b/super-intel.c
index 713bfccf..c215b910 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -6974,13 +6974,11 @@ active_arrays_by_format(char *name, char* hba, struct md_list **devlist,
 	int found;
 
 	for (memb = mdstat ; memb ; memb = memb->next) {
-		if (memb->metadata_version &&
-		    (strncmp(memb->metadata_version, "external:", 9) == 0) &&
-		    (strcmp(&memb->metadata_version[9], name) == 0) &&
-		    !is_subarray(memb->metadata_version+9) &&
-		    memb->members) {
+		if (is_mdstat_ent_external(memb) && !is_subarray(memb->metadata_version + 9) &&
+		    strcmp(&memb->metadata_version[9], name) == 0 && memb->members) {
 			struct dev_member *dev = memb->members;
 			int fd = -1;
+
 			while (dev && !is_fd_valid(fd)) {
 				char *path = xmalloc(strlen(dev->name) + strlen("/dev/") + 1);
 				num = snprintf(path, PATH_MAX, "%s%s", "/dev/", dev->name);
@@ -6998,7 +6996,6 @@ active_arrays_by_format(char *name, char* hba, struct md_list **devlist,
 				struct mdstat_ent *vol;
 				for (vol = mdstat ; vol ; vol = vol->next) {
 					if (vol->active > 0 &&
-					    vol->metadata_version &&
 					    is_container_member(vol, memb->devnm)) {
 						found++;
 						count++;
diff --git a/util.c b/util.c
index 908f8430..83d42833 100644
--- a/util.c
+++ b/util.c
@@ -1671,16 +1671,6 @@ int metadata_subdev_matches(char *metadata, char *devnm)
 	return 0;
 }
 
-int is_container_member(struct mdstat_ent *mdstat, char *container)
-{
-	if (mdstat->metadata_version == NULL ||
-	    strncmp(mdstat->metadata_version, "external:", 9) != 0 ||
-	    !metadata_container_matches(mdstat->metadata_version+9, container))
-		return 0;
-
-	return 1;
-}
-
 int is_subarray_active(char *subarray, char *container)
 {
 	struct mdstat_ent *mdstat = mdstat_read(0, 0);