ping_manager() to prevent 'add' before 'remove' completes

It is currently possible to remove a device and re-add it without the
manager noticing, i.e. without detecting a mdstat->devcnt
container->devcnt mismatch.  Introduce ping_manager() to arrange for
mdmon to run manage_container() prior to mdadm dropping the exclusive
open() on the container.  Despite these precautions sysfs_read() may
still fail.  If this happens invalidate container->devcnt to ensure
manage_container() runs at the next event.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>

diff --git a/Manage.c b/Manage.c
index 76447edacd2be231229cee95ff7d061d1af769dd..5853515db91f52552de918f6cdda527340ec27aa 100644 (file)
--- a/Manage.c
+++ b/Manage.c
@@ -712,6 +712,23 @@ int Manage_subdevs(char *devname, int fd,
                                        close(lfd);
                                return 1;
                        }
+                       if (tst->ss->external) {
+                               /*
+                                * Before dropping our exclusive open we make an
+                                * attempt at preventing mdmon from seeing an
+                                * 'add' event before reconciling this 'remove'
+                                * event.
+                                */
+                               char *name = devnum2devname(fd2devnum(fd));
+
+                               if (!name) {
+                                       fprintf(stderr, Name ": unable to get container name\n");
+                                       return 1;
+                               }
+
+                               ping_manager(name);
+                               free(name);
+                       }
                        close(lfd);
                        if (verbose >= 0)
                                fprintf(stderr, Name ": hot removed %s\n",

diff --git a/managemon.c b/managemon.c
index fc34d7789320cbcad13fef223d45b59826ba7c67..fc5da713feffc5f61d23980cb63157fb0aebf4ad 100644 (file)
--- a/managemon.c
+++ b/managemon.c
@@ -265,8 +265,11 @@ static void manage_container(struct mdstat_ent *mdstat,
                 * These need to be remove from, or added to, the array
                 */
                mdi = sysfs_read(-1, mdstat->devnum, GET_DEVS);
-               if (!mdi)
+               if (!mdi) {
+                       /* invalidate the current count so we can try again */
+                       container->devcnt = -1;
                        return;
+               }
 
                /* check for removals */
                for (cdp = &container->devs; *cdp; ) {
@@ -525,14 +528,15 @@ static void handle_message(struct supertype *container, struct metadata_update *
 
        struct metadata_update *mu;
 
-       if (msg->len == 0) {
-               int cnt;
-               
+       if (msg->len <= 0)
                while (update_queue_pending || update_queue) {
                        check_update_queue(container);
                        usleep(15*1000);
                }
 
+       if (msg->len == 0) { /* ping_monitor */
+               int cnt;
+               
                cnt = monitor_loop_cnt;
                if (cnt & 1)
                        cnt += 2; /* wait until next pselect */
@@ -542,6 +546,11 @@ static void handle_message(struct supertype *container, struct metadata_update *
 
                while (monitor_loop_cnt - cnt < 0)
                        usleep(10 * 1000);
+       } else if (msg->len == -1) { /* ping_manager */
+               struct mdstat_ent *mdstat = mdstat_read(1, 0);
+
+               manage(mdstat, container);
+               free_mdstat(mdstat);
        } else {
                mu = malloc(sizeof(*mu));
                mu->len = msg->len;

diff --git a/msg.c b/msg.c
index 013bcb99e71fa7e4c2a6bb21f012444507478517..5a4839fad7bf40e6fadd745b7228e15b3df1204e 100644 (file)
--- a/msg.c
+++ b/msg.c
@@ -81,12 +81,12 @@ static int recv_buf(int fd, void* buf, int len, int tmo)
 
 int send_message(int fd, struct metadata_update *msg, int tmo)
 {
-       __u32 len = msg->len;
+       __s32 len = msg->len;
        int rv;
 
        rv = send_buf(fd, &start_magic, 4, tmo);
        rv = rv ?: send_buf(fd, &len, 4, tmo);
-       if (len)
+       if (len > 0)
                rv = rv ?: send_buf(fd, msg->buf, msg->len, tmo);
        rv = send_buf(fd, &end_magic, 4, tmo);
 
@@ -96,7 +96,7 @@ int send_message(int fd, struct metadata_update *msg, int tmo)
 int receive_message(int fd, struct metadata_update *msg, int tmo)
 {
        __u32 magic;
-       __u32 len;
+       __s32 len;
        int rv;
 
        rv = recv_buf(fd, &magic, 4, tmo);
@@ -105,7 +105,7 @@ int receive_message(int fd, struct metadata_update *msg, int tmo)
        rv = recv_buf(fd, &len, 4, tmo);
        if (rv < 0 || len > MSG_MAX_LEN)
                return -1;
-       if (len) {
+       if (len > 0) {
                msg->buf = malloc(len);
                if (msg->buf == NULL)
                        return -1;
@@ -177,6 +177,7 @@ int connect_monitor(char *devname)
        return sfd;
 }
 
+/* give the monitor a chance to update the metadata */
 int ping_monitor(char *devname)
 {
        int sfd = connect_monitor(devname);
@@ -196,3 +197,27 @@ int ping_monitor(char *devname)
        close(sfd);
        return err;
 }
+
+/* give the manager a chance to view the updated container state.  This
+ * would naturally happen due to the manager noticing a change in
+ * /proc/mdstat; however, pinging encourages this detection to happen
+ * while an exclusive open() on the container is active
+ */
+int ping_manager(char *devname)
+{
+       int sfd = connect_monitor(devname);
+       struct metadata_update msg = { .len = -1 };
+       int err = 0;
+
+       if (sfd < 0)
+               return sfd;
+
+       err = send_message(sfd, &msg, 20);
+
+       /* check the reply */
+       if (!err && wait_reply(sfd, 20) != 0)
+               err = -1;
+
+       close(sfd);
+       return err;
+}

diff --git a/msg.h b/msg.h
index 4dc805e5233e8bb4dca9405893efc39b80702430..b9bd205d66387bcb5d523cd997a6968cbeb9b7af 100644 (file)
--- a/msg.h
+++ b/msg.h
@@ -27,5 +27,6 @@ extern int ack(int fd, int tmo);
 extern int wait_reply(int fd, int tmo);
 extern int connect_monitor(char *devname);
 extern int ping_monitor(char *devname);
+extern int ping_manager(char *devname);
 
 #define MSG_MAX_LEN (4*1024*1024)