recoverd: Clean up log messages in remote IP verification

The log messages in verify_remote_ip_allocation() are confusing
because they don't include the PNN of the problem node, because it is
not known in this function.

Add the PNN of the node being verified as a function argument and then
shuffle the log messages around to make them clearer.

Also fold 3 nested if statements into just one.

Signed-off-by: Martin Schwenke <martin@meltin.net>

diff --git a/include/ctdb_private.h b/include/ctdb_private.h
index 1939f07ccebb3658dbc8ecd91653b07de83b0fc6..17b893355388174662027432e6529c6937def667 100644 (file)
--- a/include/ctdb_private.h
+++ b/include/ctdb_private.h
@@ -1494,7 +1494,8 @@ void ctdb_run_notification_script(struct ctdb_context *ctdb, const char *event);
 void ctdb_fault_setup(void);
 
 int verify_remote_ip_allocation(struct ctdb_context *ctdb, 
-                               struct ctdb_all_public_ips *ips);
+                               struct ctdb_all_public_ips *ips,
+                               uint32_t pnn);
 int update_ip_assignment_tree(struct ctdb_context *ctdb,
                                struct ctdb_public_ip *ip);
 

diff --git a/server/ctdb_recoverd.c b/server/ctdb_recoverd.c
index bdf978e6da1a52b30f70ac375903bdd07f722de7..cc51562b1cb89c65e62b281d9b6b66246cedea8d 100644 (file)
--- a/server/ctdb_recoverd.c
+++ b/server/ctdb_recoverd.c
@@ -1463,13 +1463,13 @@ static int ctdb_reload_remote_public_ips(struct ctdb_context *ctdb,
                        return -1;
                }
 
-               if (ctdb->do_checkpublicip) {
-                       if (rec->ip_check_disable_ctx == NULL) {
-                               if (verify_remote_ip_allocation(ctdb, ctdb->nodes[j]->known_public_ips)) {
-                                       DEBUG(DEBUG_ERR,("Node %d has inconsistent public ip allocation and needs update.\n", ctdb->nodes[j]->pnn));
-                                       rec->need_takeover_run = true;
-                               }
-                       }
+               if (ctdb->do_checkpublicip &&
+                   (rec->ip_check_disable_ctx == NULL) &&
+                   verify_remote_ip_allocation(ctdb,
+                                                ctdb->nodes[j]->known_public_ips,
+                                                ctdb->nodes[j]->pnn)) {
+                       DEBUG(DEBUG_ERR,("Trigger IP reallocation\n"));
+                       rec->need_takeover_run = true;
                }
 
                /* grab a new shiny list of public ips from the node */

diff --git a/server/ctdb_takeover.c b/server/ctdb_takeover.c
index a071861a82f71354602e48bef4796283ccdf69f8..fcc1b683d0b321fb877dc9a0eeea319b854b1047 100644 (file)
--- a/server/ctdb_takeover.c
+++ b/server/ctdb_takeover.c
@@ -4257,7 +4257,9 @@ int32_t ctdb_control_ipreallocated(struct ctdb_context *ctdb,
    node has the expected ip allocation.
    This is verified against ctdb->ip_tree
 */
-int verify_remote_ip_allocation(struct ctdb_context *ctdb, struct ctdb_all_public_ips *ips)
+int verify_remote_ip_allocation(struct ctdb_context *ctdb,
+                               struct ctdb_all_public_ips *ips,
+                               uint32_t pnn)
 {
        struct ctdb_public_ip_list *tmp_ip; 
        int i;
@@ -4275,7 +4277,7 @@ int verify_remote_ip_allocation(struct ctdb_context *ctdb, struct ctdb_all_publi
        for (i=0; i<ips->num; i++) {
                tmp_ip = trbt_lookuparray32(ctdb->ip_tree, IP_KEYLEN, ip_key(&ips->ips[i].addr));
                if (tmp_ip == NULL) {
-                       DEBUG(DEBUG_ERR,(__location__ " Could not find host for address %s, reassign ips\n", ctdb_addr_to_str(&ips->ips[i].addr)));
+                       DEBUG(DEBUG_ERR,("Node %u has new or unknown public IP %s\n", pnn, ctdb_addr_to_str(&ips->ips[i].addr)));
                        return -1;
                }
 
@@ -4284,7 +4286,11 @@ int verify_remote_ip_allocation(struct ctdb_context *ctdb, struct ctdb_all_publi
                }
 
                if (tmp_ip->pnn != ips->ips[i].pnn) {
-                       DEBUG(DEBUG_ERR,("Inconsistent ip allocation. Trigger reallocation. Thinks %s is held by node %u while it is held by node %u\n", ctdb_addr_to_str(&ips->ips[i].addr), ips->ips[i].pnn, tmp_ip->pnn));
+                       DEBUG(DEBUG_ERR,
+                             ("Inconsistent IP allocation - node %u thinks %s is held by node %u while it is assigned to node %u\n",
+                              pnn,
+                              ctdb_addr_to_str(&ips->ips[i].addr),
+                              ips->ips[i].pnn, tmp_ip->pnn));
                        return -1;
                }
        }