CTDB: Patch set re-based because of updates

Swen Schillig swen at vnet.ibm.com
Mon Mar 19 10:28:16 UTC 2018


Hey Martin

Attached is a patch-set which has received a RB+ from you already.
Not sure if you need to re-confirm.

I removed the patches which were in doubt or received a NAK from others
or just no RB+ from you.

Therefore all the "good" patches required a re-base which is done with
the attached patch-set.

I hope we can process these more easily now...and maybe get a second
eye-pair for the final RB.

Thanks again for your support...can't count the beers I owe you by now.

Cheers Swen

-------------- next part --------------
From 94de5f5712f53a71386de309ded80fe1514bdbf6 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Wed, 21 Feb 2018 13:34:28 +0100
Subject: [PATCH 1/8] ctdb: Use provided mem_ctx for newly allocated memory

ctdb_call_local is called with a mem_ctx parameter which should be used
for newly allocated memory.

This is safe because all allocations of this context are freed before
this function returns.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/client/ctdb_client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ctdb/client/ctdb_client.c b/ctdb/client/ctdb_client.c
index d399cb551f6..c3bb435a641 100644
--- a/ctdb/client/ctdb_client.c
+++ b/ctdb/client/ctdb_client.c
@@ -86,7 +86,7 @@ int ctdb_call_local(struct ctdb_db_context *ctdb_db, struct ctdb_call *call,
 	struct ctdb_registered_call *fn;
 	struct ctdb_context *ctdb = ctdb_db->ctdb;
 	
-	c = talloc(ctdb, struct ctdb_call_info);
+	c = talloc(mem_ctx, struct ctdb_call_info);
 	CTDB_NO_MEMORY(ctdb, c);
 
 	c->key = call->key;
-- 
2.14.3


From 9db856c7edc8c8c4bbc145fe7192c8819e74f4ff Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Wed, 21 Feb 2018 13:52:11 +0100
Subject: [PATCH 2/8] ctdb: Use talloc_zero instead of zeroing attributes

Zero entire structure with talloc_zero on memory allocation instead of
setting each attribute individually.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/client/ctdb_client.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/ctdb/client/ctdb_client.c b/ctdb/client/ctdb_client.c
index c3bb435a641..a7624b7e3c8 100644
--- a/ctdb/client/ctdb_client.c
+++ b/ctdb/client/ctdb_client.c
@@ -86,7 +86,7 @@ int ctdb_call_local(struct ctdb_db_context *ctdb_db, struct ctdb_call *call,
 	struct ctdb_registered_call *fn;
 	struct ctdb_context *ctdb = ctdb_db->ctdb;
 	
-	c = talloc(mem_ctx, struct ctdb_call_info);
+	c = talloc_zero(mem_ctx, struct ctdb_call_info);
 	CTDB_NO_MEMORY(ctdb, c);
 
 	c->key = call->key;
@@ -94,9 +94,6 @@ int ctdb_call_local(struct ctdb_db_context *ctdb_db, struct ctdb_call *call,
 	c->record_data.dptr = talloc_memdup(c, data->dptr, data->dsize);
 	c->record_data.dsize = data->dsize;
 	CTDB_NO_MEMORY(ctdb, c->record_data.dptr);
-	c->new_data = NULL;
-	c->reply_data = NULL;
-	c->status = 0;
 	c->header = header;
 
 	for (fn=ctdb_db->calls;fn;fn=fn->next) {
-- 
2.14.3


From d82dd31c19eeacbb3b5f07d753dd0936d14d65ae Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Thu, 8 Feb 2018 11:33:08 +0100
Subject: [PATCH 3/8] ctdb-server: Cleanup ctdb_daemon_call_send_remote

Minor code cleanup and adding a temporary variable to improve readabilty.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/server/ctdb_call.c | 48 ++++++++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/ctdb/server/ctdb_call.c b/ctdb/server/ctdb_call.c
index 17d6f98e37c..7487e0195b1 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1464,6 +1464,7 @@ struct ctdb_call_state *ctdb_daemon_call_send_remote(struct ctdb_db_context *ctd
 	uint32_t len;
 	struct ctdb_call_state *state;
 	struct ctdb_context *ctdb = ctdb_db->ctdb;
+	struct ctdb_req_call_old *c;
 
 	if (ctdb->methods == NULL) {
 		DEBUG(DEBUG_INFO,(__location__ " Failed send packet. Transport is down\n"));
@@ -1479,31 +1480,34 @@ struct ctdb_call_state *ctdb_daemon_call_send_remote(struct ctdb_db_context *ctd
 	state->ctdb_db = ctdb_db;
 	talloc_set_destructor(state, ctdb_call_destructor);
 
-	len = offsetof(struct ctdb_req_call_old, data) + call->key.dsize + call->call_data.dsize;
-	state->c = ctdb_transport_allocate(ctdb, state, CTDB_REQ_CALL, len, 
-					   struct ctdb_req_call_old);
-	CTDB_NO_MEMORY_NULL(ctdb, state->c);
-	state->c->hdr.destnode  = header->dmaster;
-
-	/* this limits us to 16k outstanding messages - not unreasonable */
-	state->c->hdr.reqid     = state->reqid;
-	state->c->hdr.generation = ctdb_db->generation;
-	state->c->flags         = call->flags;
-	state->c->db_id         = ctdb_db->db_id;
-	state->c->callid        = call->call_id;
-	state->c->hopcount      = 0;
-	state->c->keylen        = call->key.dsize;
-	state->c->calldatalen   = call->call_data.dsize;
-	memcpy(&state->c->data[0], call->key.dptr, call->key.dsize);
-	memcpy(&state->c->data[call->key.dsize], 
-	       call->call_data.dptr, call->call_data.dsize);
-	*(state->call)              = *call;
-	state->call->call_data.dptr = &state->c->data[call->key.dsize];
-	state->call->key.dptr       = &state->c->data[0];
-
 	state->state  = CTDB_CALL_WAIT;
 	state->generation = ctdb_db->generation;
 
+	len = offsetof(struct ctdb_req_call_old, data) + call->key.dsize +
+		       call->call_data.dsize;
+
+	c = ctdb_transport_allocate(ctdb, state, CTDB_REQ_CALL, len,
+				    struct ctdb_req_call_old);
+	CTDB_NO_MEMORY_NULL(ctdb, c);
+	state->c = c;
+
+	c->hdr.destnode  = header->dmaster;
+	c->hdr.reqid     = state->reqid;
+	c->hdr.generation = ctdb_db->generation;
+	c->flags         = call->flags;
+	c->db_id         = ctdb_db->db_id;
+	c->callid        = call->call_id;
+	c->hopcount      = 0;
+	c->keylen        = call->key.dsize;
+	c->calldatalen   = call->call_data.dsize;
+
+	memcpy(&c->data[0], call->key.dptr, call->key.dsize);
+	memcpy(&c->data[call->key.dsize], call->call_data.dptr,
+	       call->call_data.dsize);
+
+	state->call->call_data.dptr = &c->data[call->key.dsize];
+	state->call->key.dptr       = &c->data[0];
+
 	DLIST_ADD(ctdb_db->pending_calls, state);
 
 	ctdb_queue_packet(ctdb, &state->c->hdr);
-- 
2.14.3


From 24ccc64ecd825ca6ca7beda29ac3befb63dcea53 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Tue, 20 Feb 2018 17:08:59 +0100
Subject: [PATCH 4/8] ctdb-server: Only talloc_set_destructor when required

The destructor is only needed once the state got added to the DLIST.
Therefore, move the setting of the destructor to after the addition
of state to the DLIST.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/server/ctdb_call.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/ctdb/server/ctdb_call.c b/ctdb/server/ctdb_call.c
index 7487e0195b1..c4e197830eb 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1478,8 +1478,6 @@ struct ctdb_call_state *ctdb_daemon_call_send_remote(struct ctdb_db_context *ctd
 
 	state->reqid = reqid_new(ctdb->idr, state);
 	state->ctdb_db = ctdb_db;
-	talloc_set_destructor(state, ctdb_call_destructor);
-
 	state->state  = CTDB_CALL_WAIT;
 	state->generation = ctdb_db->generation;
 
@@ -1510,6 +1508,7 @@ struct ctdb_call_state *ctdb_daemon_call_send_remote(struct ctdb_db_context *ctd
 
 	DLIST_ADD(ctdb_db->pending_calls, state);
 
+	talloc_set_destructor(state, ctdb_call_destructor);
 	ctdb_queue_packet(ctdb, &state->c->hdr);
 
 	return state;
-- 
2.14.3


From eba268ac92522c01bb47df4c2b65c659529efc9c Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Wed, 31 Jan 2018 13:06:30 +0100
Subject: [PATCH 5/8] ctdb-server: Replace the variable rc by something
 meaningful

Replace the varibale name "rc" in ctdb_start_revoke_ro_record
to prevent a mix-up with the common meaning of rc (return code).

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/server/ctdb_call.c | 127 ++++++++++++++++++++++++------------------------
 1 file changed, 64 insertions(+), 63 deletions(-)

diff --git a/ctdb/server/ctdb_call.c b/ctdb/server/ctdb_call.c
index c4e197830eb..1e06ce87099 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1552,7 +1552,7 @@ struct revokechild_deferred_call {
 	struct ctdb_req_header *hdr;
 	deferred_requeue_fn fn;
 	void *ctx;
-	struct revokechild_handle *rc;
+	struct revokechild_handle *rev_hdl;
 };
 
 struct revokechild_handle {
@@ -1586,36 +1586,36 @@ static void deferred_call_requeue(struct tevent_context *ev,
 
 static int deferred_call_destructor(struct revokechild_deferred_call *dcall)
 {
-	struct revokechild_handle *rc = dcall->rc;
+	struct revokechild_handle *rev_hdl = dcall->rev_hdl;
 
-	DLIST_REMOVE(rc->deferred_call_list, dcall);
+	DLIST_REMOVE(rev_hdl->deferred_call_list, dcall);
 	return 0;
 }
 
-static int revokechild_destructor(struct revokechild_handle *rc)
+static int revokechild_destructor(struct revokechild_handle *rev_hdl)
 {
 	struct revokechild_deferred_call *now_list = NULL;
 	struct revokechild_deferred_call *delay_list = NULL;
 
-	if (rc->fde != NULL) {
-		talloc_free(rc->fde);
+	if (rev_hdl->fde != NULL) {
+		talloc_free(rev_hdl->fde);
 	}
 
-	if (rc->fd[0] != -1) {
-		close(rc->fd[0]);
+	if (rev_hdl->fd[0] != -1) {
+		close(rev_hdl->fd[0]);
 	}
-	if (rc->fd[1] != -1) {
-		close(rc->fd[1]);
+	if (rev_hdl->fd[1] != -1) {
+		close(rev_hdl->fd[1]);
 	}
-	ctdb_kill(rc->ctdb, rc->child, SIGKILL);
+	ctdb_kill(rev_hdl->ctdb, rev_hdl->child, SIGKILL);
 
-	DLIST_REMOVE(rc->ctdb_db->revokechild_active, rc);
+	DLIST_REMOVE(rev_hdl->ctdb_db->revokechild_active, rev_hdl);
 
-	while (rc->deferred_call_list != NULL) {
+	while (rev_hdl->deferred_call_list != NULL) {
 		struct revokechild_deferred_call *dcall;
 
-		dcall = rc->deferred_call_list;
-		DLIST_REMOVE(rc->deferred_call_list, dcall);
+		dcall = rev_hdl->deferred_call_list;
+		DLIST_REMOVE(rev_hdl->deferred_call_list, dcall);
 
 		/* If revoke is successful, then first process all the calls
 		 * that need write access, and delay readonly requests by 1
@@ -1626,7 +1626,7 @@ static int revokechild_destructor(struct revokechild_handle *rc)
 		 * be recovered.
 		 */
 
-		if (rc->status == 0) {
+		if (rev_hdl->status == 0) {
 			struct ctdb_req_call_old *c;
 
 			c = (struct ctdb_req_call_old *)dcall->hdr;
@@ -1641,13 +1641,13 @@ static int revokechild_destructor(struct revokechild_handle *rc)
 	}
 
 	if (now_list != NULL) {
-		tevent_add_timer(rc->ctdb->ev, rc->ctdb_db,
+		tevent_add_timer(rev_hdl->ctdb->ev, rev_hdl->ctdb_db,
 				 tevent_timeval_current_ofs(0, 0),
 				 deferred_call_requeue, now_list);
 	}
 
 	if (delay_list != NULL) {
-		tevent_add_timer(rc->ctdb->ev, rc->ctdb_db,
+		tevent_add_timer(rev_hdl->ctdb->ev, rev_hdl->ctdb_db,
 				 tevent_timeval_current_ofs(1, 0),
 				 deferred_call_requeue, delay_list);
 	}
@@ -1659,26 +1659,26 @@ static void revokechild_handler(struct tevent_context *ev,
 				struct tevent_fd *fde,
 				uint16_t flags, void *private_data)
 {
-	struct revokechild_handle *rc = talloc_get_type(private_data, 
+	struct revokechild_handle *rev_hdl = talloc_get_type(private_data,
 						     struct revokechild_handle);
 	int ret;
 	char c;
 
-	ret = sys_read(rc->fd[0], &c, 1);
+	ret = sys_read(rev_hdl->fd[0], &c, 1);
 	if (ret != 1) {
 		DEBUG(DEBUG_ERR,("Failed to read status from revokechild. errno:%d\n", errno));
-		rc->status = -1;
-		talloc_free(rc);
+		rev_hdl->status = -1;
+		talloc_free(rev_hdl);
 		return;
 	}
 	if (c != 0) {
 		DEBUG(DEBUG_ERR,("revokechild returned failure. status:%d\n", c));
-		rc->status = -1;
-		talloc_free(rc);
+		rev_hdl->status = -1;
+		talloc_free(rev_hdl);
 		return;
 	}
 
-	talloc_free(rc);
+	talloc_free(rev_hdl);
 }
 
 struct ctdb_revoke_state {
@@ -1817,7 +1817,7 @@ static int ctdb_revoke_all_delegations(struct ctdb_context *ctdb, struct ctdb_db
 int ctdb_start_revoke_ro_record(struct ctdb_context *ctdb, struct ctdb_db_context *ctdb_db, TDB_DATA key, struct ctdb_ltdb_header *header, TDB_DATA data)
 {
 	TDB_DATA tdata;
-	struct revokechild_handle *rc;
+	struct revokechild_handle *rev_hdl;
 	pid_t parent = getpid();
 	int ret;
 
@@ -1825,7 +1825,7 @@ int ctdb_start_revoke_ro_record(struct ctdb_context *ctdb, struct ctdb_db_contex
 	header->flags |= CTDB_REC_FLAG_MIGRATED_WITH_DATA;
 	header->rsn   -= 1;
 
-	if ((rc = talloc_zero(ctdb_db, struct revokechild_handle)) == NULL) {
+	if ((rev_hdl = talloc_zero(ctdb_db, struct revokechild_handle)) == NULL) {
 		DEBUG(DEBUG_ERR,("Failed to allocate revokechild_handle\n"));
 		return -1;
 	}
@@ -1835,44 +1835,44 @@ int ctdb_start_revoke_ro_record(struct ctdb_context *ctdb, struct ctdb_db_contex
 		uint8_t *tmp;
 
 		tmp = tdata.dptr;
-		tdata.dptr = talloc_memdup(rc, tdata.dptr, tdata.dsize);
+		tdata.dptr = talloc_memdup(rev_hdl, tdata.dptr, tdata.dsize);
 		free(tmp);
 	}
 
-	rc->status    = 0;
-	rc->ctdb      = ctdb;
-	rc->ctdb_db   = ctdb_db;
-	rc->fd[0]     = -1;
-	rc->fd[1]     = -1;
+	rev_hdl->status    = 0;
+	rev_hdl->ctdb      = ctdb;
+	rev_hdl->ctdb_db   = ctdb_db;
+	rev_hdl->fd[0]     = -1;
+	rev_hdl->fd[1]     = -1;
 
-	talloc_set_destructor(rc, revokechild_destructor);
+	talloc_set_destructor(rev_hdl, revokechild_destructor);
 
-	rc->key.dsize = key.dsize;
-	rc->key.dptr  = talloc_memdup(rc, key.dptr, key.dsize);
-	if (rc->key.dptr == NULL) {
+	rev_hdl->key.dsize = key.dsize;
+	rev_hdl->key.dptr  = talloc_memdup(rev_hdl, key.dptr, key.dsize);
+	if (rev_hdl->key.dptr == NULL) {
 		DEBUG(DEBUG_ERR,("Failed to allocate key for revokechild_handle\n"));
-		talloc_free(rc);
+		talloc_free(rev_hdl);
 		return -1;
 	}
 
-	ret = pipe(rc->fd);
+	ret = pipe(rev_hdl->fd);
 	if (ret != 0) {
 		DEBUG(DEBUG_ERR,("Failed to allocate key for revokechild_handle\n"));
-		talloc_free(rc);
+		talloc_free(rev_hdl);
 		return -1;
 	}
 
 
-	rc->child = ctdb_fork(ctdb);
-	if (rc->child == (pid_t)-1) {
+	rev_hdl->child = ctdb_fork(ctdb);
+	if (rev_hdl->child == (pid_t)-1) {
 		DEBUG(DEBUG_ERR,("Failed to fork child for revokechild\n"));
-		talloc_free(rc);
+		talloc_free(rev_hdl);
 		return -1;
 	}
 
-	if (rc->child == 0) {
+	if (rev_hdl->child == 0) {
 		char c = 0;
-		close(rc->fd[0]);
+		close(rev_hdl->fd[0]);
 
 		prctl_set_comment("ctdb_revokechild");
 		if (switch_from_server_to_client(ctdb) != 0) {
@@ -1884,47 +1884,48 @@ int ctdb_start_revoke_ro_record(struct ctdb_context *ctdb, struct ctdb_db_contex
 		c = ctdb_revoke_all_delegations(ctdb, ctdb_db, tdata, key, header, data);
 
 child_finished:
-		sys_write(rc->fd[1], &c, 1);
+		sys_write(rev_hdl->fd[1], &c, 1);
 		ctdb_wait_for_process_to_exit(parent);
 		_exit(0);
 	}
 
-	close(rc->fd[1]);
-	rc->fd[1] = -1;
-	set_close_on_exec(rc->fd[0]);
+	close(rev_hdl->fd[1]);
+	rev_hdl->fd[1] = -1;
+	set_close_on_exec(rev_hdl->fd[0]);
 
 	/* This is an active revokechild child process */
-	DLIST_ADD_END(ctdb_db->revokechild_active, rc);
+	DLIST_ADD_END(ctdb_db->revokechild_active, rev_hdl);
 
-	rc->fde = tevent_add_fd(ctdb->ev, rc, rc->fd[0], TEVENT_FD_READ,
-				revokechild_handler, (void *)rc);
-	if (rc->fde == NULL) {
+	rev_hdl->fde = tevent_add_fd(ctdb->ev, rev_hdl, rev_hdl->fd[0], TEVENT_FD_READ,
+				revokechild_handler, (void *)rev_hdl);
+	if (rev_hdl->fde == NULL) {
 		DEBUG(DEBUG_ERR,("Failed to set up fd event for revokechild process\n"));
-		talloc_free(rc);
+		talloc_free(rev_hdl);
 	}
-	tevent_fd_set_auto_close(rc->fde);
+	tevent_fd_set_auto_close(rev_hdl->fde);
 
 	return 0;
 }
 
 int ctdb_add_revoke_deferred_call(struct ctdb_context *ctdb, struct ctdb_db_context *ctdb_db, TDB_DATA key, struct ctdb_req_header *hdr, deferred_requeue_fn fn, void *call_context)
 {
-	struct revokechild_handle *rc;
+	struct revokechild_handle *rev_hdl;
 	struct revokechild_deferred_call *deferred_call;
 
-	for (rc = ctdb_db->revokechild_active; rc; rc = rc->next) {
-		if (rc->key.dsize == 0) {
+	for (rev_hdl = ctdb_db->revokechild_active; rev_hdl;
+	     rev_hdl = rev_hdl->next) {
+		if (rev_hdl->key.dsize == 0) {
 			continue;
 		}
-		if (rc->key.dsize != key.dsize) {
+		if (rev_hdl->key.dsize != key.dsize) {
 			continue;
 		}
-		if (!memcmp(rc->key.dptr, key.dptr, key.dsize)) {
+		if (!memcmp(rev_hdl->key.dptr, key.dptr, key.dsize)) {
 			break;
 		}
 	}
 
-	if (rc == NULL) {
+	if (rev_hdl == NULL) {
 		DEBUG(DEBUG_ERR,("Failed to add deferred call to revoke list. revoke structure not found\n"));
 		return -1;
 	}
@@ -1939,11 +1940,11 @@ int ctdb_add_revoke_deferred_call(struct ctdb_context *ctdb, struct ctdb_db_cont
 	deferred_call->hdr  = talloc_steal(deferred_call, hdr);
 	deferred_call->fn   = fn;
 	deferred_call->ctx  = call_context;
-	deferred_call->rc   = rc;
+	deferred_call->rev_hdl   = rev_hdl;
 
 	talloc_set_destructor(deferred_call, deferred_call_destructor);
 
-	DLIST_ADD(rc->deferred_call_list, deferred_call);
+	DLIST_ADD(rev_hdl->deferred_call_list, deferred_call);
 
 	return 0;
 }
-- 
2.14.3


From 48a15d1b2e82cb44bc4758ee220e86d95d90b08a Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Thu, 8 Feb 2018 11:57:23 +0100
Subject: [PATCH 6/8] ctdb-server: Minor code cleanup

Cleanup ctdb_start_revoke_ro_record to improve readability.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/server/ctdb_call.c | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/ctdb/server/ctdb_call.c b/ctdb/server/ctdb_call.c
index 1e06ce87099..ff0f078e1a7 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1814,19 +1814,25 @@ static int ctdb_revoke_all_delegations(struct ctdb_context *ctdb, struct ctdb_db
 }
 
 
-int ctdb_start_revoke_ro_record(struct ctdb_context *ctdb, struct ctdb_db_context *ctdb_db, TDB_DATA key, struct ctdb_ltdb_header *header, TDB_DATA data)
+int ctdb_start_revoke_ro_record(struct ctdb_context *ctdb,
+				struct ctdb_db_context *ctdb_db, TDB_DATA key,
+				struct ctdb_ltdb_header *header, TDB_DATA data)
 {
 	TDB_DATA tdata;
 	struct revokechild_handle *rev_hdl;
 	pid_t parent = getpid();
 	int ret;
 
-	header->flags &= ~(CTDB_REC_RO_REVOKING_READONLY|CTDB_REC_RO_HAVE_DELEGATIONS|CTDB_REC_RO_HAVE_READONLY);
+	header->flags &= ~(CTDB_REC_RO_REVOKING_READONLY |
+			   CTDB_REC_RO_HAVE_DELEGATIONS |
+			   CTDB_REC_RO_HAVE_READONLY);
+
 	header->flags |= CTDB_REC_FLAG_MIGRATED_WITH_DATA;
 	header->rsn   -= 1;
 
-	if ((rev_hdl = talloc_zero(ctdb_db, struct revokechild_handle)) == NULL) {
-		DEBUG(DEBUG_ERR,("Failed to allocate revokechild_handle\n"));
+	rev_hdl = talloc_zero(ctdb_db, struct revokechild_handle);
+	if (rev_hdl == NULL) {
+		D_ERR("Failed to allocate revokechild_handle\n");
 		return -1;
 	}
 
@@ -1850,14 +1856,14 @@ int ctdb_start_revoke_ro_record(struct ctdb_context *ctdb, struct ctdb_db_contex
 	rev_hdl->key.dsize = key.dsize;
 	rev_hdl->key.dptr  = talloc_memdup(rev_hdl, key.dptr, key.dsize);
 	if (rev_hdl->key.dptr == NULL) {
-		DEBUG(DEBUG_ERR,("Failed to allocate key for revokechild_handle\n"));
+		D_ERR("Failed to allocate key for revokechild_handle\n");
 		talloc_free(rev_hdl);
 		return -1;
 	}
 
 	ret = pipe(rev_hdl->fd);
 	if (ret != 0) {
-		DEBUG(DEBUG_ERR,("Failed to allocate key for revokechild_handle\n"));
+		D_ERR("Failed to allocate key for revokechild_handle\n");
 		talloc_free(rev_hdl);
 		return -1;
 	}
@@ -1865,7 +1871,7 @@ int ctdb_start_revoke_ro_record(struct ctdb_context *ctdb, struct ctdb_db_contex
 
 	rev_hdl->child = ctdb_fork(ctdb);
 	if (rev_hdl->child == (pid_t)-1) {
-		DEBUG(DEBUG_ERR,("Failed to fork child for revokechild\n"));
+		D_ERR("Failed to fork child for revokechild\n");
 		talloc_free(rev_hdl);
 		return -1;
 	}
@@ -1876,12 +1882,14 @@ int ctdb_start_revoke_ro_record(struct ctdb_context *ctdb, struct ctdb_db_contex
 
 		prctl_set_comment("ctdb_revokechild");
 		if (switch_from_server_to_client(ctdb) != 0) {
-			DEBUG(DEBUG_ERR,("Failed to switch from server to client for revokechild process\n"));
+			D_ERR("Failed to switch from server to client "
+			      "for revokechild process\n");
 			c = 1;
 			goto child_finished;
 		}
 
-		c = ctdb_revoke_all_delegations(ctdb, ctdb_db, tdata, key, header, data);
+		c = ctdb_revoke_all_delegations(ctdb, ctdb_db, tdata, key,
+						header, data);
 
 child_finished:
 		sys_write(rev_hdl->fd[1], &c, 1);
@@ -1896,10 +1904,11 @@ child_finished:
 	/* This is an active revokechild child process */
 	DLIST_ADD_END(ctdb_db->revokechild_active, rev_hdl);
 
-	rev_hdl->fde = tevent_add_fd(ctdb->ev, rev_hdl, rev_hdl->fd[0], TEVENT_FD_READ,
-				revokechild_handler, (void *)rev_hdl);
+	rev_hdl->fde = tevent_add_fd(ctdb->ev, rev_hdl, rev_hdl->fd[0],
+				     TEVENT_FD_READ, revokechild_handler,
+				     (void *)rev_hdl);
 	if (rev_hdl->fde == NULL) {
-		DEBUG(DEBUG_ERR,("Failed to set up fd event for revokechild process\n"));
+		D_ERR("Failed to set up fd event for revokechild process\n");
 		talloc_free(rev_hdl);
 	}
 	tevent_fd_set_auto_close(rev_hdl->fde);
-- 
2.14.3


From c8b132c4615205d32d79f566e7e83d4b8b51605e Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Thu, 8 Feb 2018 12:08:45 +0100
Subject: [PATCH 7/8] ctdb-server: Add goto tag avoiding code duplication

Introduced err_out goto tag to prevent code duplication.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/server/ctdb_call.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/ctdb/server/ctdb_call.c b/ctdb/server/ctdb_call.c
index ff0f078e1a7..35f287c5281 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1857,23 +1857,20 @@ int ctdb_start_revoke_ro_record(struct ctdb_context *ctdb,
 	rev_hdl->key.dptr  = talloc_memdup(rev_hdl, key.dptr, key.dsize);
 	if (rev_hdl->key.dptr == NULL) {
 		D_ERR("Failed to allocate key for revokechild_handle\n");
-		talloc_free(rev_hdl);
-		return -1;
+		goto err_out;
 	}
 
 	ret = pipe(rev_hdl->fd);
 	if (ret != 0) {
 		D_ERR("Failed to allocate key for revokechild_handle\n");
-		talloc_free(rev_hdl);
-		return -1;
+		goto err_out;
 	}
 
 
 	rev_hdl->child = ctdb_fork(ctdb);
 	if (rev_hdl->child == (pid_t)-1) {
 		D_ERR("Failed to fork child for revokechild\n");
-		talloc_free(rev_hdl);
-		return -1;
+		goto err_out;
 	}
 
 	if (rev_hdl->child == 0) {
@@ -1914,6 +1911,9 @@ child_finished:
 	tevent_fd_set_auto_close(rev_hdl->fde);
 
 	return 0;
+err_out:
+	talloc_free(rev_hdl);
+	return -1;
 }
 
 int ctdb_add_revoke_deferred_call(struct ctdb_context *ctdb, struct ctdb_db_context *ctdb_db, TDB_DATA key, struct ctdb_req_header *hdr, deferred_requeue_fn fn, void *call_context)
-- 
2.14.3


From 08efdaaea0ebe0ce367696697aa5180b5cedbbb5 Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at vnet.ibm.com>
Date: Thu, 8 Feb 2018 12:19:09 +0100
Subject: [PATCH 8/8] ctdb-server: Only set destructor if required

Set the detructor in ctdb_start_revoke_ro_record after the revokechild_handle
was added to the list.

Signed-off-by: Swen Schillig <swen at vnet.ibm.com>
---
 ctdb/server/ctdb_call.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/ctdb/server/ctdb_call.c b/ctdb/server/ctdb_call.c
index 35f287c5281..06ae1f1e2c1 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1851,8 +1851,6 @@ int ctdb_start_revoke_ro_record(struct ctdb_context *ctdb,
 	rev_hdl->fd[0]     = -1;
 	rev_hdl->fd[1]     = -1;
 
-	talloc_set_destructor(rev_hdl, revokechild_destructor);
-
 	rev_hdl->key.dsize = key.dsize;
 	rev_hdl->key.dptr  = talloc_memdup(rev_hdl, key.dptr, key.dsize);
 	if (rev_hdl->key.dptr == NULL) {
@@ -1898,9 +1896,6 @@ child_finished:
 	rev_hdl->fd[1] = -1;
 	set_close_on_exec(rev_hdl->fd[0]);
 
-	/* This is an active revokechild child process */
-	DLIST_ADD_END(ctdb_db->revokechild_active, rev_hdl);
-
 	rev_hdl->fde = tevent_add_fd(ctdb->ev, rev_hdl, rev_hdl->fd[0],
 				     TEVENT_FD_READ, revokechild_handler,
 				     (void *)rev_hdl);
@@ -1910,6 +1905,10 @@ child_finished:
 	}
 	tevent_fd_set_auto_close(rev_hdl->fde);
 
+	/* This is an active revokechild child process */
+	DLIST_ADD_END(ctdb_db->revokechild_active, rev_hdl);
+	talloc_set_destructor(rev_hdl, revokechild_destructor);
+
 	return 0;
 err_out:
 	talloc_free(rev_hdl);
-- 
2.14.3



More information about the samba-technical mailing list