summaryrefslogtreecommitdiffstats
path: root/xlators/mgmt
diff options
context:
space:
mode:
authorKrutika Dhananjay <kdhananj@redhat.com>2012-10-15 17:04:29 +0530
committerVijay Bellur <vbellur@redhat.com>2012-11-12 07:08:35 -0800
commit009c728b401287ef9aa9ee4a7fb509ccc5baa156 (patch)
tree4b947e8a93e9c66a13b4feaf938a4c5476a3b8cd /xlators/mgmt
parent7997d36ce3d37b0b3ac33c1529f03969442b6595 (diff)
glusterd: Fixed glusterd crash in volume add-brick
PROBLEMS: a. glusterd crashes when add-brick operation fails on the machine other than the originator, owing to double free done on op_errstr: once in glusterd_op_begin_synctask and once through a dict unref on req_dict in gd_sync_task_begin. b. In gd_sync_task_begin, there's no need to place the error string in the dictionary, when it is never retrieved and used elsewhere. c. Command execution status is not logged into .cmd_log_history. FIX: For (a) and (b): Knocked off code that places the error string in req_dict. That way, both the problems are solved. For (b), passed op_ctx to glusterd_op_send_cli_response as it is needed to extract the command string before logging. Change-Id: I549a07ba5e31332b691a8cacd1ab32c2673810ba BUG: 862834 Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com> Reviewed-on: http://review.gluster.org/4102 Reviewed-by: Krishnan Parthasarathi <kparthas@redhat.com> Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com> Tested-by: Gluster Build System <jenkins@build.gluster.com>
Diffstat (limited to 'xlators/mgmt')
-rw-r--r--xlators/mgmt/glusterd/src/glusterd-syncop.c53
1 files changed, 20 insertions, 33 deletions
diff --git a/xlators/mgmt/glusterd/src/glusterd-syncop.c b/xlators/mgmt/glusterd/src/glusterd-syncop.c
index 6c7bccf9539..d7947281ea8 100644
--- a/xlators/mgmt/glusterd/src/glusterd-syncop.c
+++ b/xlators/mgmt/glusterd/src/glusterd-syncop.c
@@ -470,8 +470,8 @@ out:
return ret;
}
-int
-gd_sync_task_begin (dict_t *op_ctx, char **op_errstr)
+void
+gd_sync_task_begin (dict_t *op_ctx, rpcsvc_request_t * req)
{
int ret = -1;
dict_t *req_dict = NULL;
@@ -482,6 +482,7 @@ gd_sync_task_begin (dict_t *op_ctx, char **op_errstr)
glusterd_op_t op = 0;
int32_t tmp_op = 0;
gf_boolean_t local_locked = _gf_false;
+ char *op_errstr = NULL;
conf = THIS->private;
@@ -511,12 +512,12 @@ gd_sync_task_begin (dict_t *op_ctx, char **op_errstr)
/* TODO: Only on lock successful nodes it should unlock */
}
- ret = glusterd_op_build_payload (&req_dict, op_errstr, op_ctx);
+ ret = glusterd_op_build_payload (&req_dict, &op_errstr, op_ctx);
if (ret)
goto out;
/* stage op */
- ret = glusterd_op_stage_validate (op, req_dict, op_errstr, rsp_dict);
+ ret = glusterd_op_stage_validate (op, req_dict, &op_errstr, rsp_dict);
if (ret)
goto out;
@@ -524,27 +525,21 @@ gd_sync_task_begin (dict_t *op_ctx, char **op_errstr)
ret = gd_syncop_mgmt_stage_op (peerinfo->rpc,
MY_UUID, tmp_uuid,
op, req_dict, &rsp_dict,
- op_errstr);
- if (ret) {
- if (*op_errstr)
- ret = dict_set_dynstr (req_dict, "error",
- *op_errstr);
-
- ret = -1;
+ &op_errstr);
+ if (ret)
goto out;
- }
if (op == GD_OP_REPLACE_BRICK)
(void) glusterd_syncop_aggr_rsp_dict (op, op_ctx,
rsp_dict,
- *op_errstr);
+ op_errstr);
if (rsp_dict)
dict_unref (rsp_dict);
}
/* commit op */
- ret = glusterd_op_commit_perform (op, req_dict, op_errstr, rsp_dict);
+ ret = glusterd_op_commit_perform (op, req_dict, &op_errstr, rsp_dict);
if (ret)
goto out;
@@ -552,17 +547,11 @@ gd_sync_task_begin (dict_t *op_ctx, char **op_errstr)
ret = gd_syncop_mgmt_commit_op (peerinfo->rpc,
MY_UUID, tmp_uuid,
op, req_dict, &rsp_dict,
- op_errstr);
- if (ret) {
- if (*op_errstr)
- ret = dict_set_dynstr (req_dict, "error",
- *op_errstr);
-
- ret = -1;
+ &op_errstr);
+ if (ret)
goto out;
- }
(void) glusterd_syncop_aggr_rsp_dict (op, op_ctx, rsp_dict,
- *op_errstr);
+ op_errstr);
if (rsp_dict)
dict_unref (rsp_dict);
}
@@ -585,13 +574,18 @@ out:
glusterd_unlock (MY_UUID);
}
+ glusterd_op_send_cli_response (op, ret, 0, req, op_ctx, op_errstr);
+
if (req_dict)
dict_unref (req_dict);
if (rsp_dict)
dict_unref (rsp_dict);
- return ret;
+ if (op_errstr)
+ GF_FREE (op_errstr);
+
+ return;
}
int32_t
@@ -599,8 +593,6 @@ glusterd_op_begin_synctask (rpcsvc_request_t *req, glusterd_op_t op,
void *dict)
{
int ret = 0;
- int op_ret = 0;
- char *op_errstr = NULL;
ret = dict_set_int32 (dict, GD_SYNC_OPCODE_KEY, op);
if (ret) {
@@ -609,15 +601,10 @@ glusterd_op_begin_synctask (rpcsvc_request_t *req, glusterd_op_t op,
goto out;
}
- op_ret = gd_sync_task_begin (dict, &op_errstr);
- glusterd_op_send_cli_response (op, op_ret, 0, req, NULL,
- op_errstr);
- ret = 0;
+ gd_sync_task_begin (dict, req);
out:
if (dict)
- dict_unref (dict);
- if (op_errstr)
- GF_FREE (op_errstr);
+ dict_unref (dict);
return ret;
}