CTDB: Patch set re-based because of updates
Swen Schillig
swen at vnet.ibm.com
Tue Mar 20 07:00:34 UTC 2018
Hi Martin
On Tue, 2018-03-20 at 16:48 +1100, Martin Schwenke via samba-technical
wrote:
> Hi Sven,
>
> On Mon, 19 Mar 2018 17:46:26 +0100, Swen Schillig <swen at vnet.ibm.com>
> wrote:
>
> > On Mon, 2018-03-19 at 11:28 +0100, Swen Schillig wrote:
> > > 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.
> > Updated patch-set...found one-bug.
>
> OK, my apologies for this, given that I've already given a RB+ but...
No problem, that's why we're doing those review's, I'd just wish we
could get slightly less iterative :-)
>
> One of the threads about one of your other patches prompted me to
> read
> the section in README.Coding about function
> calls/declarations/definitions (including the recent change). I
> can't remember if I read it before or after the previous RB+.
>
> Some of the cleanups don't meet the all on 1 line or 1 per line
> thing... that was new to me too.
>
> Similarly, there's a for loop that's end up like this:
>
> + for (rev_hdl = ctdb_db->revokechild_active; rev_hdl;
> + rev_hdl = rev_hdl->next) {
> I think that would be clearer if it were 1-per-line.
Well, that "new" thing came in after we started this patch-series,
maybe that's why.
>
> Sorry for the churn...
No problem at all !!
Here's the updated one.
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 b85e14017c5662a79dec68bc0a01ecb77b4f4367 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 | 54 +++++++++++++++++++++++++++++--------------------
1 file changed, 32 insertions(+), 22 deletions(-)
diff --git a/ctdb/server/ctdb_call.c b/ctdb/server/ctdb_call.c
index 17d6f98e37c..836d615d3c2 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,40 @@ 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 ff25398b2d6c68338cecba60ed3ac534f1a3d852 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 836d615d3c2..ee16a075a69 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;
@@ -1516,6 +1514,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 be429001a4a82c50e80909db3f48c47338d915ec 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 | 143 ++++++++++++++++++++++++++----------------------
1 file changed, 77 insertions(+), 66 deletions(-)
diff --git a/ctdb/server/ctdb_call.c b/ctdb/server/ctdb_call.c
index ee16a075a69..0fd35c90b78 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1558,7 +1558,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 {
@@ -1592,36 +1592,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
@@ -1632,7 +1632,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;
@@ -1647,15 +1647,19 @@ 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);
+ 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);
+ deferred_call_requeue,
+ delay_list);
}
return 0;
@@ -1665,26 +1669,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);
+ 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 {
@@ -1823,7 +1827,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;
@@ -1831,7 +1835,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;
}
@@ -1841,44 +1845,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) {
@@ -1890,47 +1894,54 @@ 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;
}
@@ -1945,11 +1956,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 dc06df7fd79a01846c57902dafc53e1556b4fc98 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 | 28 ++++++++++++++++++----------
1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/ctdb/server/ctdb_call.c b/ctdb/server/ctdb_call.c
index 0fd35c90b78..5816bb38028 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1824,19 +1824,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;
}
@@ -1860,14 +1866,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;
}
@@ -1875,7 +1881,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;
}
@@ -1886,12 +1892,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);
@@ -1914,7 +1922,7 @@ child_finished:
(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 d3d4fe4f980c6487744bad16305cc4e2feb869dc 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 5816bb38028..b0cae65e4c2 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1867,23 +1867,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) {
@@ -1928,6 +1925,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 af77b044eea6c8252bc75bb153d6054714e28621 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 | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/ctdb/server/ctdb_call.c b/ctdb/server/ctdb_call.c
index b0cae65e4c2..ffc0ec81292 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1861,8 +1861,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) {
@@ -1924,6 +1922,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