tools/ctdb: Fix racy ipreallocate code

This code tried to find the recovery master and send an ipreallocate
request to that node.  When a node is stopped, this code asked the
stopped node for recovery master.  Stopped node does not have up-to-date
information on the current recovery master.  So ipreallocate requests
were sent to the wrong node and ignored by that node which is not the
recovery master.

Send ipreallocate request to all active nodes.  That way we guarantee
that the current recovery master will see it and respond to it.

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Pair-Programmed-With: Martin Schwenke <martin@meltin.net>

(cherry picked from commit 0577ce3c68e4febf49a1ef5093e918db9d5ec636)

Conflicts:
	tools/ctdb.c

diff --git a/tools/ctdb.c b/tools/ctdb.c
index b7b5e124e19a604e1fe65bec82a291d9626a4b28..a59791a72878e89b810e276c40c7623086d4d84c 100644 (file)
--- a/tools/ctdb.c
+++ b/tools/ctdb.c
@@ -1616,7 +1616,7 @@ control_get_all_public_ips(struct ctdb_context *ctdb, TALLOC_CTX *tmp_ctx, struc
 }
 
 
-static uint32_t ipreallocate_finished;
+static bool ipreallocate_finished;
 
 /*
   handler for receiving the response to ipreallocate
@@ -1624,7 +1624,7 @@ static uint32_t ipreallocate_finished;
 static void ip_reallocate_handler(struct ctdb_context *ctdb, uint64_t srvid, 
                             TDB_DATA data, void *private_data)
 {
-       ipreallocate_finished = 1;
+       ipreallocate_finished = true;
 }
 
 static void ctdb_every_second(struct event_context *ev, struct timed_event *te, struct timeval t, void *p)
@@ -1644,9 +1644,8 @@ static int control_ipreallocate(struct ctdb_context *ctdb, int argc, const char
        int i, ret;
        TDB_DATA data;
        struct takeover_run_reply rd;
-       uint32_t recmaster;
        struct ctdb_node_map *nodemap=NULL;
-       int retries=0;
+       int count;
        struct timeval tv = timeval_current();
 
        /* we need some events to trigger so we can timeout and restart
@@ -1672,82 +1671,41 @@ static int control_ipreallocate(struct ctdb_context *ctdb, int argc, const char
        data.dsize = sizeof(rd);
 
 again:
-       /* check that there are valid nodes available */
+       /* get the number of nodes and node flags */
        if (ctdb_ctrl_getnodemap(ctdb, TIMELIMIT(), options.pnn, ctdb, &nodemap) != 0) {
                DEBUG(DEBUG_ERR, ("Unable to get nodemap from local node\n"));
                return -1;
        }
-       for (i=0; i<nodemap->num;i++) {
-               if ((nodemap->nodes[i].flags & (NODE_FLAGS_DELETED|NODE_FLAGS_BANNED|NODE_FLAGS_STOPPED)) == 0) {
-                       break;
-               }
-       }
-       if (i==nodemap->num) {
-               DEBUG(DEBUG_ERR,("No recmaster available, no need to wait for cluster convergence\n"));
-               return 0;
-       }
-
-
-       ret = ctdb_ctrl_getrecmaster(ctdb, ctdb, TIMELIMIT(), options.pnn, &recmaster);
-       if (ret != 0) {
-               DEBUG(DEBUG_ERR, ("Unable to get recmaster from node %u\n", options.pnn));
-               return ret;
-       }
-
-       /* verify the node exists */
-       if (ctdb_ctrl_getnodemap(ctdb, TIMELIMIT(), recmaster, ctdb, &nodemap) != 0) {
-               DEBUG(DEBUG_ERR, ("Unable to get nodemap from local node\n"));
-               return -1;
-       }
-
 
-       /* check tha there are nodes available that can act as a recmaster */
-       for (i=0; i<nodemap->num; i++) {
-               if (nodemap->nodes[i].flags & (NODE_FLAGS_DELETED|NODE_FLAGS_BANNED|NODE_FLAGS_STOPPED)) {
+       ipreallocate_finished = false;
+       count = 0;
+       for (i=0; i<nodemap->num;i++) {
+               if (nodemap->nodes[i].flags & NODE_FLAGS_INACTIVE) {
                        continue;
+               } else {
+                       /* Send to all active nodes. Only recmaster will reply. */
+                       ret = ctdb_client_send_message(ctdb, i, CTDB_SRVID_TAKEOVER_RUN, data);
+                       if (ret != 0) {
+                               DEBUG(DEBUG_ERR,("Failed to send ip takeover run request message to %u\n", options.pnn));
+                               return -1;
+                       }
+                       count++;
                }
-               break;
        }
-       if (i == nodemap->num) {
-               DEBUG(DEBUG_ERR,("No possible nodes to host addresses.\n"));
+       if (count == 0) {
+               DEBUG(DEBUG_ERR,("No recmaster available, no need to wait for cluster convergence\n"));
                return 0;
        }
 
-       /* verify the recovery master is not STOPPED, nor BANNED */
-       if (nodemap->nodes[recmaster].flags & (NODE_FLAGS_DELETED|NODE_FLAGS_BANNED|NODE_FLAGS_STOPPED)) {
-               DEBUG(DEBUG_ERR,("No suitable recmaster found. Try again\n"));
-               retries++;
-               sleep(1);
-               goto again;
-       } 
-       
-       /* verify the recovery master is not STOPPED, nor BANNED */
-       if (nodemap->nodes[recmaster].flags & (NODE_FLAGS_DELETED|NODE_FLAGS_BANNED|NODE_FLAGS_STOPPED)) {
-               DEBUG(DEBUG_ERR,("No suitable recmaster found. Try again\n"));
-               retries++;
-               sleep(1);
-               goto again;
-       } 
-
-       ipreallocate_finished = 0;
-       ret = ctdb_client_send_message(ctdb, recmaster, CTDB_SRVID_TAKEOVER_RUN, data);
-       if (ret != 0) {
-               DEBUG(DEBUG_ERR,("Failed to send ip takeover run request message to %u\n", options.pnn));
-               return -1;
-       }
-
        tv = timeval_current();
        /* this loop will terminate when we have received the reply */
-       while (timeval_elapsed(&tv) < 5.0 && ipreallocate_finished == 0) {
+       while (timeval_elapsed(&tv) < 5.0 && !ipreallocate_finished) {
                event_loop_once(ctdb->ev);
        }
-       if (ipreallocate_finished == 1) {
-               return 0;
-       }
 
-       retries++;
-       sleep(1);
-       goto again;
+       if (!ipreallocate_finished) {
+               goto again;
+       }
 
        return 0;
 }