From 2eb9861cbc0387b1054bfeb7864c255a42b475f5 Mon Sep 17 00:00:00 2001 From: Kaushik BV Date: Sat, 9 Oct 2010 06:58:00 +0000 Subject: mgmt/Glusterd: Memory leak fixes, minor CLI changes Signed-off-by: Kaushik BV Signed-off-by: Vijay Bellur BUG: 1852 (Usage message of volume set printed twice) URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=1852 --- cli/src/cli-cmd-parser.c | 8 ++-- libglusterfs/src/xlator.c | 8 ++++ xlators/mgmt/glusterd/src/glusterd-handler.c | 29 ++++++++------ xlators/mgmt/glusterd/src/glusterd-op-sm.c | 59 +++++++++++++++------------- xlators/mgmt/glusterd/src/glusterd-volgen.c | 3 +- 5 files changed, 63 insertions(+), 44 deletions(-) diff --git a/cli/src/cli-cmd-parser.c b/cli/src/cli-cmd-parser.c index 7d3fa84d87c..1381e0fecef 100644 --- a/cli/src/cli-cmd-parser.c +++ b/cli/src/cli-cmd-parser.c @@ -355,7 +355,7 @@ cli_cmd_volume_set_parse (const char **words, int wordcount, dict_t **options) if (!dict) goto out; - if (wordcount < 3) + if (wordcount < 4) goto out; volname = (char *)words[2]; @@ -367,11 +367,12 @@ cli_cmd_volume_set_parse (const char **words, int wordcount, dict_t **options) if (ret) goto out; + for (i = 3; i < wordcount; i+=2) { key = (char *) words[i]; value = (char *) words[i+1]; - + if ( key && !value ) { if ( !strcmp (key, "history")) { ret = dict_set_str (dict, key, "history"); @@ -384,10 +385,9 @@ cli_cmd_volume_set_parse (const char **words, int wordcount, dict_t **options) goto out; } } - + if ( !key || !value) { ret = -1; - cli_out ("Usage: volume set "); goto out; } diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c index ff1dc5efa26..b0f3eba4a54 100644 --- a/libglusterfs/src/xlator.c +++ b/libglusterfs/src/xlator.c @@ -1205,6 +1205,9 @@ xlator_list_destroy (xlator_list_t *list) int xlator_destroy (xlator_t *xl) { + volume_opt_list_t *vol_opt = NULL; + volume_opt_list_t *tmp = NULL; + if (!xl) return 0; @@ -1221,6 +1224,11 @@ xlator_destroy (xlator_t *xl) xlator_list_destroy (xl->parents); + list_for_each_entry_safe (vol_opt, tmp, &xl->volume_options, list) { + list_del_init (&vol_opt->list); + GF_FREE (vol_opt); + } + GF_FREE (xl); return 0; diff --git a/xlators/mgmt/glusterd/src/glusterd-handler.c b/xlators/mgmt/glusterd/src/glusterd-handler.c index 8254569658c..49281b956d0 100644 --- a/xlators/mgmt/glusterd/src/glusterd-handler.c +++ b/xlators/mgmt/glusterd/src/glusterd-handler.c @@ -2786,40 +2786,45 @@ _print (dict_t *unused, char *key, data_t *value, void *newdict) int glusterd_set_volume_history (rpcsvc_request_t *req,dict_t *dict) { -// dict_t *reply_dict = NULL; glusterd_volinfo_t *volinfo = NULL; gf1_cli_set_vol_rsp rsp = {0, }; int ret = -1; char *volname = NULL; - - gf_log ("", GF_LOG_DEBUG, "'volume set history' command"); - + char vol[256] = {0, }; + ret = dict_get_str (dict, "volname", &volname); if (ret) { - gf_log ("", GF_LOG_ERROR, "Unable to get volume name"); + gf_log ("glusterd", GF_LOG_ERROR, "Unable to get volume name"); goto out; } ret = glusterd_volinfo_find (volname, &volinfo); if (ret) { - gf_log ("", GF_LOG_ERROR, "Unable to allocate memory"); + gf_log ("glusterd", GF_LOG_ERROR, + "'volume set' Volume %s not found", volname); + snprintf (vol, 256, "Volume %s not present", volname); + + rsp.op_errstr = gf_strdup (vol); + if (!rsp.op_errstr) { + rsp.op_errstr = ""; + gf_log ("glusterd", GF_LOG_ERROR, "Out of memory"); + } goto out; } - - dict_foreach (volinfo->dict, _print, volinfo->dict); - + ret = dict_allocate_and_serialize (volinfo->dict, &rsp.dict.dict_val, (size_t *)&rsp.dict.dict_len); - + if (ret) { gf_log ("", GF_LOG_DEBUG, "FAILED: allocatea n serialize dict"); goto out; } +out: if (!ret) rsp.op_ret = 1; else @@ -2827,12 +2832,12 @@ glusterd_set_volume_history (rpcsvc_request_t *req,dict_t *dict) if (!rsp.volname) rsp.volname = ""; if (!rsp.op_errstr) - rsp.op_errstr = ""; + rsp.op_errstr = "Error, Validation failed"; ret = glusterd_submit_reply (req, &rsp, NULL, 0, NULL, gf_xdr_serialize_cli_set_vol_rsp); -out: + gf_log ("", GF_LOG_DEBUG, "Returning %d", ret); return ret; } diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c index 34caacd5ea8..75605e51ec9 100644 --- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c +++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c @@ -1191,13 +1191,12 @@ glusterd_op_stage_set_volume (gd1_mgmt_stage_op_req *req, char **op_errstr) dict = dict_new (); if (!dict) goto out; - + val_dict = dict_new(); if (!val_dict) goto out; ret = dict_unserialize (req->buf.buf_val, req->buf.buf_len, &dict); - ret = dict_unserialize (req->buf.buf_val, req->buf.buf_len, &dict); if (ret) { gf_log ("", GF_LOG_ERROR, "Unable to unserialize dict"); @@ -1217,46 +1216,48 @@ glusterd_op_stage_set_volume (gd1_mgmt_stage_op_req *req, char **op_errstr) gf_log ("", GF_LOG_ERROR, "Volume with name: %s " "does not exist", volname); snprintf (errstr, 2048, "Volume : %s does not exist", - key); + volname); *op_errstr = gf_strdup (errstr); ret = -1; goto out; } - + ret = glusterd_volinfo_find (volname, &volinfo); if (ret) { gf_log ("", GF_LOG_ERROR, "Unable to allocate memory"); goto out; } - + ret = dict_get_int32 (dict, "count", &dict_count); - + if (ret) { gf_log ("", GF_LOG_ERROR, "Count(dict),not set in Volume-Set"); goto out; } - + if ( dict_count == 1 ) { if (dict_get (dict, "history" )) { ret = 0; goto out; } - + gf_log ("", GF_LOG_ERROR, "No options received "); + *op_errstr = gf_strdup ("Options not specified"); ret = -1; goto out; } - - for ( count = 1; ret != -1 ; count++ ) { + + for ( count = 1; ret != 1 ; count++ ) { sprintf (str, "key%d", count); ret = dict_get_str (dict, str, &key); if (ret) - break; + break; + exists = glusterd_check_option_exists (key, NULL); @@ -1270,7 +1271,7 @@ glusterd_op_stage_set_volume (gd1_mgmt_stage_op_req *req, char **op_errstr) ret = -1; goto out; } - + sprintf (str, "value%d", count); ret = dict_get_str (dict, str, &value); @@ -1280,9 +1281,9 @@ glusterd_op_stage_set_volume (gd1_mgmt_stage_op_req *req, char **op_errstr) ret = -1; goto out; } - + ret = dict_set_str (val_dict, key, value); - + if (ret) { gf_log ("", GF_LOG_ERROR, "Unable to set the options" "in 'volume set'"); @@ -1306,20 +1307,24 @@ glusterd_op_stage_set_volume (gd1_mgmt_stage_op_req *req, char **op_errstr) ret = 0; out: - if (dict) + if (dict) dict_unref (dict); - if (ret) { - if (!(*op_errstr)) { - *op_errstr = gf_strdup ("Error, Validation Failed"); - gf_log ("glsuterd", GF_LOG_DEBUG, - "Error, Cannot Validate option :%s", - *op_errstr); - } - else - gf_log ("glsuterd", GF_LOG_DEBUG, - "Error, Cannot Validate option"); - } - return ret; + + if (val_dict) + dict_unref (val_dict); + + if (ret) { + if (!(*op_errstr)) { + *op_errstr = gf_strdup ("Error, Validation Failed"); + gf_log ("glsuterd", GF_LOG_DEBUG, + "Error, Cannot Validate option :%s", + *op_errstr); + } + else + gf_log ("glsuterd", GF_LOG_DEBUG, + "Error, Cannot Validate option"); + } +return ret; } static int diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.c b/xlators/mgmt/glusterd/src/glusterd-volgen.c index 93e276dc9fb..df4818df82f 100644 --- a/xlators/mgmt/glusterd/src/glusterd-volgen.c +++ b/xlators/mgmt/glusterd/src/glusterd-volgen.c @@ -125,7 +125,7 @@ static struct volopt_map_entry glusterd_volopt_map[] = { {"auth.allow", "protocol/server", "!server-auth", "*"}, {"auth.reject", "protocol/server", "!server-auth",}, - + {"transport.keepalive", "protocol/server", "transport.socket.keepalive",}, {"performance.write-behind", "performance/write-behind", "!perf", "on"}, /* NODOC */ @@ -186,6 +186,7 @@ xlator_instantiate_va (const char *type, const char *format, va_list arg) if (!xl->options) goto error; xl->name = volname; + INIT_LIST_HEAD (&xl->volume_options); return xl; -- cgit