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>

diff --git a/Assemble.c b/Assemble.c
index 77f2b50e0ad92e967a2b5fbaac5cba2890777d24..a2bb7b6465d99a7dba902fb777831b61919d0fc7 100644 (file)
--- 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 83db071214ee57ba507e5cfed73f85494cb9950e..abc7721b51dc5bc88c10b12b9f691a94efc80b36 100644 (file)
--- 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 f0304e1e4d3d7edbb0e34c18ca79bf424c1ce3cf..241de05520d62ab929f506fe5ffbdf5672a485f8 100644 (file)
--- 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 26c53e13e1aa1c7ff74d1913947bb3448809537b..cf14fbb3ae7139472964babd967fd5a0cb50b78a 100644 (file)
--- 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 6ea905f36b003db57cd2d30ccfeac231fad41306..5411a4804ac37720fa36e6480b378744212ff02d 100644 (file)
--- 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 ea9837acd0c121cce82bbdcf5bce5f1096b54818..632cf5e89f39b5993781f29adbedd9a203c78678 100644 (file)
--- 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 22d5e8f49bc2de5d8277f1188cd923b3c7cb7781..5c3a9836404cb79c09b309af05422dd8b7288a74 100644 (file)
--- 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 5fdb5cdb5a495eb5fa59329394857e703f34601c..95e350f4ab00d9d13fb1bdadf540f0b68861d153 100644 (file)
--- 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 b3d72ac37b67f186c87ab1849b7693b6b924a936..110cbef29205222df0ea7834358d960b9565049f 100644 (file)
--- 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 e233f094c4805ebca8ba53d0297b8fdeb5f37410..cbbace3d73aaca6e14003f75afa13b012f26c49f 100644 (file)
--- 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 713bfccfeed343b0a09bb3e4a336477832f4af0a..c215b91021073feb779dc85bafc0ef332dda4ff9 100644 (file)
--- 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 908f84306ed2f543b68f9eda8107fdfcb40179d4..83d428333d50a40139d1332c8f8effcf8db891b4 100644 (file)
--- 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);