mgmt/Glusterd : Reply to CLI error: graph construction

Signed-off-by: Kaushik BV <kaushikbv@gluster.com>
Signed-off-by: Vijay Bellur <vijay@dev.gluster.com>

BUG: 1159 ()
URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=1159

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);
 
@@ -978,6 +985,58 @@ out:
 }
 
 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, ...)
 {
         xlator_t *old_THIS = NULL;
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)
 {