CTDB: Patch set re-based because of updates
Swen Schillig
swen at vnet.ibm.com
Wed Mar 21 11:52:10 UTC 2018
On Wed, 2018-03-21 at 22:30 +1100, Martin Schwenke wrote:
> On Wed, 21 Mar 2018 08:46:36 +0100, Swen Schillig <swen at vnet.ibm.com>
> wrote:
>
> > On Wed, 2018-03-21 at 16:54 +1100, Martin Schwenke wrote:
> > > On Tue, 20 Mar 2018 08:00:34 +0100, Swen Schillig <swen at vnet.ibm.
> > > com>
> > > wrote:
> > >
> > > Oops. The last commit doesn't move the DLIST_ADD_END() but it
> > > inserts
> > > an additional use of that macro. I think this was correct in an
> > > earlier
> > > version but must have got lost in the rebase. :-(
> > >
> >
> > Good catch !
> >
> > Updated.
>
> This will eventually kill us...
I totally agree !
>
> In commit 6, the definition of ctdb_start_revoke_ro_record() has been
> wrapped but had more than 1 parameter per line. Also the call to
> ctdb_revoke_all_delegations() also has more then 1 parameter per
> line.
>
> With those 2 changes:
>
> Reviewed-by: Martin Schwenke <martin at meltin.net>
>
> I'm happy for a 2nd reviewer to jump in if you resend with those
> changes.
Updates as requested.
Second reviewer, please ...
Cheers Swen.
-------------- next part --------------
From 2f5649a0064662a36f053b0a4959619fb546c392 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 ff3c683b2fd66811e12304df76c7a8b964ecb806 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 7c1ee19063eea8a0d532fede46c281608b5c6851 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 c0ebc37106a54e28ca38fb0bc63c27e672f997a2 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 5bae2bc94490044ba5e65d34a50dd91aac82309d 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 edb4296cdbe50ade26d3440e4c2cf37ea9fdb143 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 | 34 ++++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/ctdb/server/ctdb_call.c b/ctdb/server/ctdb_call.c
index 0fd35c90b78..d357f9407a2 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1824,19 +1824,27 @@ 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 +1868,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 +1883,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 +1894,18 @@ 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 +1928,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 8cc0bd9fdb878f3f69fef68be29b47ed47caf14d 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 d357f9407a2..beaae42fb59 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1869,23 +1869,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) {
@@ -1934,6 +1931,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 19c69c4b7f5a81faa2d21a968884391eb06ffb7a 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 beaae42fb59..03059206026 100644
--- a/ctdb/server/ctdb_call.c
+++ b/ctdb/server/ctdb_call.c
@@ -1863,8 +1863,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) {
@@ -1914,9 +1912,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],
@@ -1930,6 +1925,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