Attachment 12378 Details for Bug 12158 – Patch for 4.5rc

[patch] Patch for 4.5rc

v4-5-BZ12158.patch (text/plain), 10.09 KB, created by Martin Schwenke on 2016-08-18 06:15:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Schwenke

Created: 2016-08-18 06:15:10 UTC

Size: 10.09 KB

patch

obsolete

>From 0d399dd85ac2df01ddec5812e67e5bc60fbc2edb Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>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 <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(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 <martin@meltin.net>
>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 <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(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 <martin@meltin.net>
>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 <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(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 <martin@meltin.net>
>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 <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(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 <martin@meltin.net>
>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 <martin@meltin.net>
>Reviewed-by: Amitay Isaacs <amitay@gmail.com>
>(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
>

Flags:

amitay: review+

Actions: View

Attachments on bug 12158: 12378 | 12379 | 12391 | 12392