CTDB: Patch set re-based because of updates
Swen Schillig
swen at vnet.ibm.com
Mon Mar 19 16:46:26 UTC 2018
On Mon, 2018-03-19 at 11:28 +0100, Swen Schillig wrote:
> 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
>
Updated patch-set...found one-bug.
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 89da3163fe08f5c01506e3c851e17d9b7c3eb777 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 | 49 +++++++++++++++++++++++++++----------------------
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/ctdb/server/ctdb_call.c b/ctdb/server/ctdb_call.c
index 17d6f98e37c..ac4f90bb152 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,35 @@ 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;
+ 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 29fc315ec286243154f652e748c82dcbfef36224 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 ac4f90bb152..6903a3c6916 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;
@@ -1511,6 +1509,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 b1bddb76707704a1da29442432861d86be36f2bc 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 6903a3c6916..6a83b63cc31 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1553,7 +1553,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 {
@@ -1587,36 +1587,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
@@ -1627,7 +1627,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;
@@ -1642,13 +1642,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);
}
@@ -1660,26 +1660,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 {
@@ -1818,7 +1818,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;
@@ -1826,7 +1826,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;
}
@@ -1836,44 +1836,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) {
@@ -1885,47 +1885,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;
}
@@ -1940,11 +1941,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 81ec13631e6366da4c6040f23f9037d4bcbf4661 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 6a83b63cc31..92e89566d40 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1815,19 +1815,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;
}
@@ -1851,14 +1857,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;
}
@@ -1866,7 +1872,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;
}
@@ -1877,12 +1883,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);
@@ -1897,10 +1905,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 42a8b1dc1b01bb2681fed0a3fc22c727140af851 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 92e89566d40..c9aba6880c3 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1858,23 +1858,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) {
@@ -1915,6 +1912,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 e78ca564ce60615a1d5bb9f9cf37d02ccb081b0f 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 c9aba6880c3..10b16a37f67 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1852,8 +1852,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) {
@@ -1899,9 +1897,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);
@@ -1911,6 +1906,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