From 0d399dd85ac2df01ddec5812e67e5bc60fbc2edb Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Mon, 8 Aug 2016 07:09:38 +1000 Subject: [PATCH 1/5] ctdb-daemon: Try to release IP address even if interface is unknown The "releaseip" event in 10.interface will determine the interface and do the right thing. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12158 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit 509491a868ed01bfc5a970bd36eea4b01130853a) --- ctdb/server/ctdb_takeover.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c index 6d182de..0d30f43 100644 --- a/ctdb/server/ctdb_takeover.c +++ b/ctdb/server/ctdb_takeover.c @@ -2126,9 +2126,6 @@ void ctdb_release_all_ips(struct ctdb_context *ctdb) ctdb_vnn_unassign_iface(ctdb, vnn); continue; } - if (!vnn->iface) { - continue; - } /* Don't allow multiple releases at once. Some code, * particularly ctdb_tickle_sentenced_connections() is -- 2.8.1 From 153c7bb02bf9714a9c6217bc2b90a41ad11a289e Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Sat, 30 Jul 2016 11:12:19 +1000 Subject: [PATCH 2/5] ctdb-daemon: Do not update the VNN state on RELEASE_IP failure If RELEASE_IP fails then updating the VNN makes it inconsistent with reality. Instead, log the failure and move on to the next IP address. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12158 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit ca22373231918dab4e94cf1bab03253aadd61993) --- ctdb/server/ctdb_takeover.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c index 0d30f43..3040642 100644 --- a/ctdb/server/ctdb_takeover.c +++ b/ctdb/server/ctdb_takeover.c @@ -2147,9 +2147,21 @@ void ctdb_release_all_ips(struct ctdb_context *ctdb) ctdb_vnn_iface_string(vnn))); ctdb_event_script_args(ctdb, CTDB_EVENT_RELEASE_IP, "%s %s %u", - ctdb_vnn_iface_string(vnn), - ctdb_addr_to_str(&vnn->public_address), - vnn->public_netmask_bits); + ctdb_vnn_iface_string(vnn), + ctdb_addr_to_str(&vnn->public_address), + vnn->public_netmask_bits); + /* releaseip timeouts are converted to success, so to + * detect failures just check if the IP address is + * still there... + */ + if (ctdb_sys_have_ip(&vnn->public_address)) { + DEBUG(DEBUG_ERR, + (__location__ + " IP address %s not released\n", + ctdb_addr_to_str(&vnn->public_address))); + vnn->update_in_flight = false; + continue; + } data.dptr = (uint8_t *)talloc_strdup( vnn, ctdb_addr_to_str(&vnn->public_address)); -- 2.8.1 From 50c6cc4ed8f188fa7ca8833fcfef1efb9a64b234 Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 11 Aug 2016 13:41:12 +1000 Subject: [PATCH 3/5] ctdb-daemon: Do not copy address for RELEASE_IP message If there's an allocation failure then the implicit early return in CTDB_NO_MEMORY_VOID() means that no reply is sent to the control. ctdb_daemon_send_message() makes a copy of the data, so don't copy it here and remove an unnecessary chance of failure. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12158 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit e653c8bb4a7bd712351a4ead3997c61b22c46f8d) --- ctdb/server/ctdb_takeover.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c index 3040642..4e0b7f5 100644 --- a/ctdb/server/ctdb_takeover.c +++ b/ctdb/server/ctdb_takeover.c @@ -879,8 +879,7 @@ static void release_ip_callback(struct ctdb_context *ctdb, int status, /* send a message to all clients of this node telling them that the cluster has been reconfigured and they should release any sockets on this IP */ - data.dptr = (uint8_t *)talloc_strdup(state, ctdb_addr_to_str(state->addr)); - CTDB_NO_MEMORY_VOID(ctdb, data.dptr); + data.dptr = (uint8_t *) ctdb_addr_to_str(state->addr); data.dsize = strlen((char *)data.dptr)+1; DEBUG(DEBUG_INFO,(__location__ " sending RELEASE_IP for '%s'\n", data.dptr)); @@ -2163,14 +2162,10 @@ void ctdb_release_all_ips(struct ctdb_context *ctdb) continue; } - data.dptr = (uint8_t *)talloc_strdup( - vnn, ctdb_addr_to_str(&vnn->public_address)); - if (data.dptr != NULL) { - data.dsize = strlen((char *)data.dptr) + 1; - ctdb_daemon_send_message(ctdb, ctdb->pnn, - CTDB_SRVID_RELEASE_IP, data); - talloc_free(data.dptr); - } + data.dptr = (uint8_t *)ctdb_addr_to_str(&vnn->public_address); + data.dsize = strlen((char *)data.dptr) + 1; + ctdb_daemon_send_message(ctdb, ctdb->pnn, + CTDB_SRVID_RELEASE_IP, data); ctdb_vnn_unassign_iface(ctdb, vnn); vnn->update_in_flight = false; -- 2.8.1 From 8b4ec69204dea592646d131aff14d7ad38f26a7d Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 11 Aug 2016 13:57:43 +1000 Subject: [PATCH 4/5] ctdb-daemon: Factor out new function release_ip_post() This contains the cleanup that needs to be done after an IP address is released from an interface. state->vnn is set to the return value from release_ip_post(), which is either the original VNN, or NULL if it was deleted. This allows correct handling of the in-flight flag in the destructor for state. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12158 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit 46c5136e4e4bd291cbb96395374c9b133f5d8ad8) --- ctdb/server/ctdb_takeover.c | 45 +++++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c index 4e0b7f5..f96cd0a 100644 --- a/ctdb/server/ctdb_takeover.c +++ b/ctdb/server/ctdb_takeover.c @@ -850,6 +850,32 @@ static void do_delete_ip(struct ctdb_context *ctdb, struct ctdb_vnn *vnn) talloc_free(vnn); } +static struct ctdb_vnn *release_ip_post(struct ctdb_context *ctdb, + struct ctdb_vnn *vnn, + ctdb_sock_addr *addr) +{ + TDB_DATA data; + + /* Send a message to all clients of this node telling them + * that the cluster has been reconfigured and they should + * close any connections on this IP address + */ + data.dptr = (uint8_t *)ctdb_addr_to_str(addr); + data.dsize = strlen((char *)data.dptr)+1; + DEBUG(DEBUG_INFO, ("Sending RELEASE_IP message for %s\n", data.dptr)); + ctdb_daemon_send_message(ctdb, ctdb->pnn, CTDB_SRVID_RELEASE_IP, data); + + ctdb_vnn_unassign_iface(ctdb, vnn); + + /* Process the IP if it has been marked for deletion */ + if (vnn->delete_pending) { + do_delete_ip(ctdb, vnn); + return NULL; + } + + return vnn; +} + /* called when releaseip event finishes */ @@ -858,7 +884,6 @@ static void release_ip_callback(struct ctdb_context *ctdb, int status, { struct takeover_callback_state *state = talloc_get_type(private_data, struct takeover_callback_state); - TDB_DATA data; if (status == -ETIME) { ctdb_ban_self(ctdb); @@ -876,23 +901,7 @@ static void release_ip_callback(struct ctdb_context *ctdb, int status, } } - /* send a message to all clients of this node telling them - that the cluster has been reconfigured and they should - release any sockets on this IP */ - data.dptr = (uint8_t *) ctdb_addr_to_str(state->addr); - data.dsize = strlen((char *)data.dptr)+1; - - DEBUG(DEBUG_INFO,(__location__ " sending RELEASE_IP for '%s'\n", data.dptr)); - - ctdb_daemon_send_message(ctdb, ctdb->pnn, CTDB_SRVID_RELEASE_IP, data); - - ctdb_vnn_unassign_iface(ctdb, state->vnn); - - /* Process the IP if it has been marked for deletion */ - if (state->vnn->delete_pending) { - do_delete_ip(ctdb, state->vnn); - state->vnn = NULL; - } + state->vnn = release_ip_post(ctdb, state->vnn, state->addr); /* the control succeeded */ ctdb_request_control_reply(ctdb, state->c, NULL, 0, NULL); -- 2.8.1 From 2908579b377e97c75896e62ce782ed1208c1872a Mon Sep 17 00:00:00 2001 From: Martin Schwenke Date: Thu, 11 Aug 2016 14:07:44 +1000 Subject: [PATCH 5/5] ctdb-daemon: Use release_ip_post() when releasing all IP addresses This has the advantage of using common code. Also, if there was previously a failed attempt to release the IP address as part of a delete, then this will finish processing the delete. Extra care needs to be taken when a VNN is actually deleted. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12158 Signed-off-by: Martin Schwenke Reviewed-by: Amitay Isaacs (cherry picked from commit d2a91394f55a2e0152bf470dac2608618db13b1f) --- ctdb/server/ctdb_takeover.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c index f96cd0a..b369d03 100644 --- a/ctdb/server/ctdb_takeover.c +++ b/ctdb/server/ctdb_takeover.c @@ -2121,15 +2121,17 @@ void ctdb_takeover_client_destructor_hook(struct ctdb_client *client) void ctdb_release_all_ips(struct ctdb_context *ctdb) { - struct ctdb_vnn *vnn; + struct ctdb_vnn *vnn, *next; int count = 0; - TDB_DATA data; if (ctdb->tunable.disable_ip_failover == 1) { return; } - for (vnn=ctdb->vnn;vnn;vnn=vnn->next) { + for (vnn = ctdb->vnn; vnn != NULL; vnn = next) { + /* vnn can be freed below in release_ip_post() */ + next = vnn->next; + if (!ctdb_sys_have_ip(&vnn->public_address)) { ctdb_vnn_unassign_iface(ctdb, vnn); continue; @@ -2171,13 +2173,10 @@ void ctdb_release_all_ips(struct ctdb_context *ctdb) continue; } - data.dptr = (uint8_t *)ctdb_addr_to_str(&vnn->public_address); - data.dsize = strlen((char *)data.dptr) + 1; - ctdb_daemon_send_message(ctdb, ctdb->pnn, - CTDB_SRVID_RELEASE_IP, data); - - ctdb_vnn_unassign_iface(ctdb, vnn); - vnn->update_in_flight = false; + vnn = release_ip_post(ctdb, vnn, &vnn->public_address); + if (vnn != NULL) { + vnn->update_in_flight = false; + } count++; } -- 2.8.1