Attachment 12379 Details for Bug 12158 – Patch for 4.4

[patch] Patch for 4.4

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

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Schwenke

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

Size: 14.47 KB

patch

obsolete

>From f088423089f49d40375a10bf2f900a07364c8939 Mon Sep 17 00:00:00 2001
>From: Amitay Isaacs <amitay@gmail.com>
>Date: Fri, 4 Mar 2016 15:04:13 +1100
>Subject: [PATCH 1/7] ctdb-takeover: Do not kill smbd processes on releasing IP
>
>CTDB already notifies Samba with RELEASE_IP message.  Samba can take
>appropriate action based on that.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12158
>
>Signed-off-by: Amitay Isaacs <amitay@gmail.com>
>Reviewed-by: Martin Schwenke <martin@meltin.net>
>(cherry picked from commit b8272d835d6e5186568237cd8b7a2105884c0515)
>---
> ctdb/server/ctdb_takeover.c | 42 ------------------------------------------
> 1 file changed, 42 deletions(-)
>
>diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c
>index a613aa0..b32772f 100644
>--- a/ctdb/server/ctdb_takeover.c
>+++ b/ctdb/server/ctdb_takeover.c
>@@ -815,44 +815,6 @@ int32_t ctdb_control_takeover_ip(struct ctdb_context *ctdb,
> 	return 0;
> }
> 
>-/*
>-  kill any clients that are registered with a IP that is being released
>- */
>-static void release_kill_clients(struct ctdb_context *ctdb, ctdb_sock_addr *addr)
>-{
>-	struct ctdb_client_ip *ip;
>-
>-	DEBUG(DEBUG_INFO,("release_kill_clients for ip %s\n",
>-		ctdb_addr_to_str(addr)));
>-
>-	for (ip=ctdb->client_ip_list; ip; ip=ip->next) {
>-		ctdb_sock_addr tmp_addr;
>-
>-		tmp_addr = ip->addr;
>-		DEBUG(DEBUG_INFO,("checking for client %u with IP %s\n", 
>-			ip->client_id,
>-			ctdb_addr_to_str(&ip->addr)));
>-
>-		if (ctdb_same_ip(&tmp_addr, addr)) {
>-			struct ctdb_client *client = reqid_find(ctdb->idr,
>-								ip->client_id,
>-								struct ctdb_client);
>-			DEBUG(DEBUG_INFO,("matched client %u with IP %s and pid %u\n", 
>-				ip->client_id,
>-				ctdb_addr_to_str(&ip->addr),
>-				client->pid));
>-
>-			if (client->pid != 0) {
>-				DEBUG(DEBUG_INFO,(__location__ " Killing client pid %u for IP %s on client_id %u\n",
>-					(unsigned)client->pid,
>-					ctdb_addr_to_str(addr),
>-					ip->client_id));
>-				kill(client->pid, SIGKILL);
>-			}
>-		}
>-	}
>-}
>-
> static void do_delete_ip(struct ctdb_context *ctdb, struct ctdb_vnn *vnn)
> {
> 	DLIST_REMOVE(ctdb->vnn, vnn);
>@@ -898,9 +860,6 @@ static void release_ip_callback(struct ctdb_context *ctdb, int status,
> 
> 	ctdb_daemon_send_message(ctdb, ctdb->pnn, CTDB_SRVID_RELEASE_IP, data);
> 
>-	/* kill clients that have registered with this IP */
>-	release_kill_clients(ctdb, state->addr);
>-
> 	ctdb_vnn_unassign_iface(ctdb, state->vnn);
> 
> 	/* Process the IP if it has been marked for deletion */
>@@ -2415,7 +2374,6 @@ void ctdb_release_all_ips(struct ctdb_context *ctdb)
> 				  ctdb_vnn_iface_string(vnn),
> 				  ctdb_addr_to_str(&vnn->public_address),
> 				  vnn->public_netmask_bits);
>-		release_kill_clients(ctdb, &vnn->public_address);
> 		ctdb_vnn_unassign_iface(ctdb, vnn);
> 		vnn->update_in_flight = false;
> 		count++;
>-- 
>2.8.1
>
>
>From 22138a8a41e65ff2a76d1a0fa9dc8f2db8c12e5f Mon Sep 17 00:00:00 2001
>From: Amitay Isaacs <amitay@gmail.com>
>Date: Sat, 5 Mar 2016 14:05:21 +1100
>Subject: [PATCH 2/7] ctdb-takeover: Inform clients when dropping all IP
> addresses
>
>CTDB releases all IPs in following cases: starting up, shutting down,
>node gets banned, node does not come out of recovery for a long time.
>Always inform samba when CTDB releases IP addresses.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=12158
>
>Signed-off-by: Amitay Isaacs <amitay@gmail.com>
>Reviewed-by: Martin Schwenke <martin@meltin.net>
>(cherry picked from commit 00b9e76904bb1108e0f06d0dba4df89394d58252)
>---
> ctdb/server/ctdb_takeover.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
>diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c
>index b32772f..916f82c 100644
>--- a/ctdb/server/ctdb_takeover.c
>+++ b/ctdb/server/ctdb_takeover.c
>@@ -2337,6 +2337,7 @@ void ctdb_release_all_ips(struct ctdb_context *ctdb)
> {
> 	struct ctdb_vnn *vnn;
> 	int count = 0;
>+	TDB_DATA data;
> 
> 	if (ctdb->tunable.disable_ip_failover == 1) {
> 		return;
>@@ -2374,6 +2375,16 @@ void ctdb_release_all_ips(struct ctdb_context *ctdb)
> 				  ctdb_vnn_iface_string(vnn),
> 				  ctdb_addr_to_str(&vnn->public_address),
> 				  vnn->public_netmask_bits);
>+
>+		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);
>+		}
>+
> 		ctdb_vnn_unassign_iface(ctdb, vnn);
> 		vnn->update_in_flight = false;
> 		count++;
>-- 
>2.8.1
>
>
>From 3888ad6f59a102f3475af968a53d0b17fe4155b7 Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Mon, 8 Aug 2016 07:09:38 +1000
>Subject: [PATCH 3/7] 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 916f82c..e32dc1a 100644
>--- a/ctdb/server/ctdb_takeover.c
>+++ b/ctdb/server/ctdb_takeover.c
>@@ -2348,9 +2348,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 bd8c60e8d109bf5e8ef2feceee62cf9a41f92d1b Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Sat, 30 Jul 2016 11:12:19 +1000
>Subject: [PATCH 4/7] 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 e32dc1a..e37eb7a 100644
>--- a/ctdb/server/ctdb_takeover.c
>+++ b/ctdb/server/ctdb_takeover.c
>@@ -2369,9 +2369,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 ce7ba497c887573d21ab6b8395b95792c8839b85 Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Thu, 11 Aug 2016 13:41:12 +1000
>Subject: [PATCH 5/7] 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 e37eb7a..d9353df 100644
>--- a/ctdb/server/ctdb_takeover.c
>+++ b/ctdb/server/ctdb_takeover.c
>@@ -852,8 +852,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));
>@@ -2385,14 +2384,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 5919ca2b959a4338f86eaf84b5a6f565ca42685b Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Thu, 11 Aug 2016 13:57:43 +1000
>Subject: [PATCH 6/7] 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 d9353df..5966e36 100644
>--- a/ctdb/server/ctdb_takeover.c
>+++ b/ctdb/server/ctdb_takeover.c
>@@ -823,6 +823,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
>  */
>@@ -831,7 +857,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);
>@@ -849,23 +874,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 2fdf777a8d9e668f74954ef4e24011ab45b101ec Mon Sep 17 00:00:00 2001
>From: Martin Schwenke <martin@meltin.net>
>Date: Thu, 11 Aug 2016 14:07:44 +1000
>Subject: [PATCH 7/7] 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 5966e36..8cf4405 100644
>--- a/ctdb/server/ctdb_takeover.c
>+++ b/ctdb/server/ctdb_takeover.c
>@@ -2343,15 +2343,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;
>@@ -2393,13 +2395,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