From 7aec50bdad06665243a16f1ce83a03037bcd5d56 Mon Sep 17 00:00:00 2001 From: Kaushik BV Date: Tue, 5 Oct 2010 07:01:52 +0000 Subject: mgmt/Glusterd : Reply to CLI error: graph construction Signed-off-by: Kaushik BV Signed-off-by: Vijay Bellur BUG: 1159 () URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=1159 --- cli/src/cli-cmd-parser.c | 2 +- cli/src/cli3_1-cops.c | 3 + libglusterfs/src/xlator.c | 59 +++++++++++ libglusterfs/src/xlator.h | 4 + rpc/xdr/src/cli1-xdr.c | 2 + rpc/xdr/src/cli1-xdr.h | 1 + rpc/xdr/src/cli1.x | 1 + xlators/cluster/afr/src/afr.c | 1 + xlators/mgmt/glusterd/src/glusterd-handler.c | 2 + xlators/mgmt/glusterd/src/glusterd-op-sm.c | 129 +++++++++++++++++++----- xlators/mgmt/glusterd/src/glusterd-volgen.c | 97 ++++++++++++++++++ xlators/mgmt/glusterd/src/glusterd.h | 3 + xlators/performance/io-threads/src/io-threads.c | 1 + xlators/protocol/server/src/server.c | 1 + 14 files changed, 279 insertions(+), 27 deletions(-) diff --git a/cli/src/cli-cmd-parser.c b/cli/src/cli-cmd-parser.c index d01e4f71196..7d3fa84d87c 100644 --- a/cli/src/cli-cmd-parser.c +++ b/cli/src/cli-cmd-parser.c @@ -405,7 +405,7 @@ cli_cmd_volume_set_parse (const char **words, int wordcount, dict_t **options) goto out; } - ret = dict_set_int32 (dict, "count", count); + ret = dict_set_int32 (dict, "count", wordcount-3); if (ret) goto out; diff --git a/cli/src/cli3_1-cops.c b/cli/src/cli3_1-cops.c index a4a3304cd58..bc98d2c942e 100644 --- a/cli/src/cli3_1-cops.c +++ b/cli/src/cli3_1-cops.c @@ -864,6 +864,9 @@ gf_cli3_1_set_volume_cbk (struct rpc_req *req, struct iovec *iov, gf_log ("cli", GF_LOG_NORMAL, "Received resp to set"); cli_out ("Set volume %s", (rsp.op_ret) ? "unsuccessful": "successful"); + + if (rsp.op_ret && rsp.op_errstr) + cli_out ("%s", rsp.op_errstr); ret = rsp.op_ret; diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c index b9819584eee..ff1dc5efa26 100644 --- a/libglusterfs/src/xlator.c +++ b/libglusterfs/src/xlator.c @@ -763,6 +763,13 @@ xlator_dynload (xlator_t *xl) "dlsym(reconfigure) on %s -- neglecting", dlerror()); } + + if (!(xl->validate_options = dlsym (handle, "validate_options"))) { + gf_log ("xlator", GF_LOG_DEBUG, + "dlsym(validate_options) on %s -- neglecting", + dlerror()); + } + INIT_LIST_HEAD (&xl->volume_options); @@ -977,6 +984,58 @@ out: return ret; } +int +xlator_validate_rec (xlator_t *xlator, char **op_errstr) +{ + xlator_list_t *trav = NULL; + + if (xlator == NULL ) { + gf_log ("xlator", GF_LOG_DEBUG, "invalid argument"); + return -1; + } + + trav = xlator->children; + + while (trav) { + if (xlator_validate_rec (trav->xlator, op_errstr) ) + return -1; + + trav = trav->next; + } + + if (xlator_dynload (xlator)) + gf_log ("", GF_LOG_DEBUG, "Did not load the symbols"); + + if (xlator->validate_options) { + if (xlator->validate_options (xlator, xlator->options, + op_errstr)) { + gf_log ("", GF_LOG_DEBUG, *op_errstr); + return -1; + } + gf_log (xlator->name, GF_LOG_DEBUG, "Validated option"); + + } + + gf_log (xlator->name, GF_LOG_DEBUG, "No validate_options() found"); + + return 0; +} + +int +graph_reconf_validateopt (glusterfs_graph_t *graph, + char **op_errstr) +{ + xlator_t *xlator = NULL; + int ret = -1; + + GF_ASSERT (graph); + + xlator = graph->first; + + ret = xlator_validate_rec (xlator, op_errstr); + + return ret; +} int xlator_notify (xlator_t *xl, int event, void *data, ...) { diff --git a/libglusterfs/src/xlator.h b/libglusterfs/src/xlator.h index 7c71f8e40b5..b4200976e4c 100644 --- a/libglusterfs/src/xlator.h +++ b/libglusterfs/src/xlator.h @@ -807,6 +807,8 @@ struct _xlator { int32_t (*init) (xlator_t *this); int32_t (*reconfigure) (xlator_t *this, dict_t *options); int32_t (*mem_acct_init) (xlator_t *this); + int32_t (*validate_options) (xlator_t *this, dict_t *options, + char **op_errstr); event_notify_fn_t notify; gf_loglevel_t loglevel; /* Log level for translator */ @@ -860,6 +862,8 @@ void loc_wipe (loc_t *loc); int xlator_mem_acct_init (xlator_t *xl, int num_types); int xlator_tree_reconfigure (xlator_t *old_xl, xlator_t *new_xl); int is_gf_log_command (xlator_t *trans, const char *name, char *value); +int xlator_validate_rec (xlator_t *xlator, char **op_errstr); +int graph_reconf_validateopt (glusterfs_graph_t *graph, char **op_errstr); #define GF_STAT_PRINT_FMT_STR "%"PRIx64",%"PRIx64",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx64",%"PRIx64",%"PRIx32",%"PRIx64",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32",%"PRIx32"\n" diff --git a/rpc/xdr/src/cli1-xdr.c b/rpc/xdr/src/cli1-xdr.c index 334cd26de14..807392d1e8b 100644 --- a/rpc/xdr/src/cli1-xdr.c +++ b/rpc/xdr/src/cli1-xdr.c @@ -510,6 +510,8 @@ xdr_gf1_cli_set_vol_rsp (XDR *xdrs, gf1_cli_set_vol_rsp *objp) return FALSE; if (!xdr_string (xdrs, &objp->volname, ~0)) return FALSE; + if (!xdr_string (xdrs, &objp->op_errstr, ~0)) + return FALSE; if (!xdr_bytes (xdrs, (char **)&objp->dict.dict_val, (u_int *) &objp->dict.dict_len, ~0)) return FALSE; return TRUE; diff --git a/rpc/xdr/src/cli1-xdr.h b/rpc/xdr/src/cli1-xdr.h index f18e2a75068..f1f5bc05002 100644 --- a/rpc/xdr/src/cli1-xdr.h +++ b/rpc/xdr/src/cli1-xdr.h @@ -313,6 +313,7 @@ struct gf1_cli_set_vol_rsp { int op_ret; int op_errno; char *volname; + char *op_errstr; struct { u_int dict_len; char *dict_val; diff --git a/rpc/xdr/src/cli1.x b/rpc/xdr/src/cli1.x index b43bc46d201..bce605ede5e 100644 --- a/rpc/xdr/src/cli1.x +++ b/rpc/xdr/src/cli1.x @@ -219,6 +219,7 @@ struct gf1_cli_set_vol_req { int op_ret; int op_errno; string volname<>; + string op_errstr<>; opaque dict<>; } ; diff --git a/xlators/cluster/afr/src/afr.c b/xlators/cluster/afr/src/afr.c index 8f9052cae8c..813fbb24904 100644 --- a/xlators/cluster/afr/src/afr.c +++ b/xlators/cluster/afr/src/afr.c @@ -61,6 +61,7 @@ mem_acct_init (xlator_t *this) } + int reconfigure (xlator_t *this, dict_t *options) { diff --git a/xlators/mgmt/glusterd/src/glusterd-handler.c b/xlators/mgmt/glusterd/src/glusterd-handler.c index ee547639e0b..c337655857f 100644 --- a/xlators/mgmt/glusterd/src/glusterd-handler.c +++ b/xlators/mgmt/glusterd/src/glusterd-handler.c @@ -2760,6 +2760,8 @@ glusterd_set_volume_history (rpcsvc_request_t *req,dict_t *dict) rsp.op_ret = ret; if (!rsp.volname) rsp.volname = ""; + if (!rsp.op_errstr) + rsp.op_errstr = ""; ret = glusterd_submit_reply (req, &rsp, NULL, 0, NULL, gf_xdr_serialize_cli_set_vol_rsp); diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c index 26e23e9fc62..7eb83a2d338 100644 --- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c +++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c @@ -1042,22 +1042,32 @@ out: } static int -glusterd_op_stage_set_volume (gd1_mgmt_stage_op_req *req) +glusterd_op_stage_set_volume (gd1_mgmt_stage_op_req *req, char **op_errstr) { - int ret = 0; - dict_t *dict = NULL; - char *volname = NULL; - int exists = 0; - char *key = NULL; - char str[100] = {0, }; - int count = 0; - - GF_ASSERT (req); - - dict = dict_new (); - if (!dict) + int ret = 0; + dict_t *dict = NULL; + char *volname = NULL; + gf_boolean_t exists = _gf_false; + char *key = NULL; + char *value = NULL; + char str[100] = {0, }; + int count = 0; + int dict_count = 0; + char errstr[2048] = {0, }; + glusterd_volinfo_t *volinfo = NULL; + dict_t *val_dict = NULL; + + GF_ASSERT (req); + + 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) { @@ -1076,11 +1086,38 @@ glusterd_op_stage_set_volume (gd1_mgmt_stage_op_req *req) if (!exists) { gf_log ("", GF_LOG_ERROR, "Volume with name: %s " - "does not exist", - volname); + "does not exist", volname); + snprintf (errstr, 2048, "Volume : %s does not exist", + key); + *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 "); + ret = -1; + goto out; + } + for ( count = 1; ret != -1 ; count++ ) { @@ -1097,27 +1134,63 @@ glusterd_op_stage_set_volume (gd1_mgmt_stage_op_req *req) if (exists != 1) { gf_log ("", GF_LOG_ERROR, "Option with name: %s " "does not exist", key); + snprintf (errstr, 2048, "option : %s does not exist", + key); + *op_errstr = gf_strdup (errstr); + ret = -1; goto out; } + + sprintf (str, "value%d", count); + ret = dict_get_str (dict, str, &value); + + if (ret) { + gf_log ("", GF_LOG_ERROR, "invalid key,value pair" + "in 'volume set'"); + 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'"); + ret = -1; + goto out; + } } - if ( count == 1 ) { - gf_log ("", GF_LOG_ERROR, "No options received "); - ret = -1; - goto out; - } + *op_errstr = NULL; + ret = glusterd_validate_reconfopts (volinfo, val_dict, op_errstr); + if (ret) { + gf_log ("glsuterd", GF_LOG_DEBUG, + "Could not create temp volfile, some option failed: %s", + *op_errstr); + goto out; + } + ret = 0; out: - if (dict) + if (dict) dict_unref (dict); - gf_log ("", GF_LOG_DEBUG, "Returning %d", ret); - - return ret; + 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 @@ -4228,6 +4301,10 @@ glusterd_op_send_cli_response (int32_t op, int32_t op_ret, rsp.op_ret = op_ret; rsp.op_errno = op_errno; rsp.volname = ""; + if (op_errstr) + rsp.op_errstr = op_errstr; + else + rsp.op_errstr = ""; cli_rsp = &rsp; sfunc = gf_xdr_serialize_cli_set_vol_rsp; break; @@ -4528,7 +4605,7 @@ glusterd_op_stage_validate (gd1_mgmt_stage_op_req *req, char **op_errstr) break; case GD_OP_SET_VOLUME: - ret = glusterd_op_stage_set_volume (req); + ret = glusterd_op_stage_set_volume (req, op_errstr); break; case GD_OP_RESET_VOLUME: diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.c b/xlators/mgmt/glusterd/src/glusterd-volgen.c index 1b01e10eb0d..9d3ceaf5659 100644 --- a/xlators/mgmt/glusterd/src/glusterd-volgen.c +++ b/xlators/mgmt/glusterd/src/glusterd-volgen.c @@ -1254,3 +1254,100 @@ glusterd_delete_volfile (glusterd_volinfo_t *volinfo, get_brick_filepath (filename, volinfo, brickinfo); return unlink (filename); } + +int +validate_clientopts (glusterd_volinfo_t *volinfo, + dict_t *val_dict, + char **op_errstr) +{ + glusterfs_graph_t graph = {{0,},}; + int ret = -1; + + GF_ASSERT (volinfo); + + + ret = build_client_graph (&graph, volinfo, val_dict); + if (!ret) + ret = graph_reconf_validateopt (&graph, op_errstr); + + volgen_graph_free (&graph); + + gf_log ("", GF_LOG_DEBUG, "Returning %d", ret); + return ret; +} + +int +validate_brickopts (glusterd_volinfo_t *volinfo, + char *brickinfo_path, + dict_t *val_dict, + char **op_errstr) +{ + glusterfs_graph_t graph = {{0,},}; + int ret = -1; + + GF_ASSERT (volinfo); + + + + ret = build_server_graph (&graph, volinfo, val_dict, brickinfo_path); + if (!ret) + ret = graph_reconf_validateopt (&graph, op_errstr); + + volgen_graph_free (&graph); + + gf_log ("", GF_LOG_DEBUG, "Returning %d", ret); + return ret; +} + +int +glusterd_validate_brickreconf (glusterd_volinfo_t *volinfo, + dict_t *val_dict, + char **op_errstr) +{ + glusterd_brickinfo_t *brickinfo = NULL; + int ret = -1; + + list_for_each_entry (brickinfo, &volinfo->bricks, brick_list) { + gf_log ("", GF_LOG_DEBUG, + "Validating %s", brickinfo->hostname); + + ret = validate_brickopts (volinfo, brickinfo->path, val_dict, + op_errstr); + if (ret) + goto out; + } + + ret = 0; +out: + + return ret; +} + +int +glusterd_validate_reconfopts (glusterd_volinfo_t *volinfo, dict_t *val_dict, + char **op_errstr) +{ + int ret = -1; + + gf_log ("", GF_LOG_DEBUG, "Inside Validate reconfigure options"); + + ret = glusterd_validate_brickreconf (volinfo, val_dict, op_errstr); + + if (ret) { + gf_log ("", GF_LOG_DEBUG, + "Could not Validate bricks"); + goto out; + } + + ret = validate_clientopts (volinfo, val_dict, op_errstr); + if (ret) { + gf_log ("", GF_LOG_DEBUG, + "Could not Validate client"); + goto out; + } + + +out: + gf_log ("", GF_LOG_DEBUG, "Returning %d", ret); + return ret; +} \ No newline at end of file diff --git a/xlators/mgmt/glusterd/src/glusterd.h b/xlators/mgmt/glusterd/src/glusterd.h index e1cbc3583d1..ac983760130 100644 --- a/xlators/mgmt/glusterd/src/glusterd.h +++ b/xlators/mgmt/glusterd/src/glusterd.h @@ -446,4 +446,7 @@ glusterd_restart_bricks(glusterd_conf_t *conf); int32_t glusterd_volume_txn (rpcsvc_request_t *req, char *volname, int flags, glusterd_op_t op); + +int +glusterd_validate_reconfopts (glusterd_volinfo_t *volinfo, dict_t *val_dict, char **op_errstr); #endif diff --git a/xlators/performance/io-threads/src/io-threads.c b/xlators/performance/io-threads/src/io-threads.c index 1748dff58ca..b593e90e449 100644 --- a/xlators/performance/io-threads/src/io-threads.c +++ b/xlators/performance/io-threads/src/io-threads.c @@ -2084,6 +2084,7 @@ mem_acct_init (xlator_t *this) return ret; } + int reconfigure ( xlator_t *this, dict_t *options) { diff --git a/xlators/protocol/server/src/server.c b/xlators/protocol/server/src/server.c index 6274644b72b..1594b8d0a01 100644 --- a/xlators/protocol/server/src/server.c +++ b/xlators/protocol/server/src/server.c @@ -456,6 +456,7 @@ mem_acct_init (xlator_t *this) return ret; } + int reconfigure (xlator_t *this, dict_t *options) { -- cgit