glusterd/volgen: refactor some things

- use macros for exported volume tunables
- introduce glusterd_volinfo_get_boolean as retrieval of boolean
  optuions was a recurring pattern and became somewhat cumbersome

Also fixed illegal access to volinfo dict.
@pranith: use always the glusterd_volinfo_get* API for that purpose
so that defaults are taken to consideration.

Signed-off-by: Csaba Henk <csaba@gluster.com>
Signed-off-by: Anand Avati <avati@gluster.com>

BUG: 2757 (refactory gsync/gsyncd/syncdaemon/whatever to geo-replication)
URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=2757

3 files changed, 76 insertions, 61 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
index b446f79d0..6e358821b 100644
--- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c
+++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
@@ -2578,21 +2578,15 @@ static gf_boolean_t
 glusterd_is_profile_on (glusterd_volinfo_t *volinfo)
 {
         int                                     ret = -1;
-        char                                    *latency_key = NULL;
-        char                                    *fd_stats_key = NULL;
         gf_boolean_t                            is_latency_on = _gf_false;
         gf_boolean_t                            is_fd_stats_on = _gf_false;
 
         GF_ASSERT (volinfo);
-        latency_key = "diagnostics.latency-measurement";
-        fd_stats_key = "diagnostics.count-fop-hits";
 
-        ret = dict_get_str_boolean (volinfo->dict, fd_stats_key,
-                                    _gf_false);
+        ret = glusterd_volinfo_get_boolean (volinfo, VKEY_DIAG_CNT_FOP_HITS);
         if (ret != -1)
                 is_fd_stats_on = ret;
-        ret = dict_get_str_boolean (volinfo->dict, latency_key,
-                                    _gf_false);
+        ret = glusterd_volinfo_get_boolean (volinfo, VKEY_DIAG_LAT_MEASUREMENT);
         if (ret != -1)
                 is_latency_on = ret;
         if ((_gf_true == is_latency_on) &&
@@ -4548,8 +4542,7 @@ glusterd_set_marker_gsync (char *master)
         char                    *volname = NULL;
         glusterd_volinfo_t      *volinfo = NULL;
         int                      ret     = -1;
-        char                    *marker_value   = NULL;
-        gf_boolean_t             marker_set = _gf_false;
+        int                      marker_set = _gf_false;
         char                    *gsync_status = NULL;
         glusterd_conf_t         *priv = NULL;
 
@@ -4566,17 +4559,13 @@ glusterd_set_marker_gsync (char *master)
                 ret = -1;
                 goto out;
         }
-        ret = glusterd_volinfo_get (volinfo, "features.marker-gsync", &marker_value);
-        if (ret) {
+        marker_set = glusterd_volinfo_get_boolean (volinfo, VKEY_MARKER_XTIME);
+        if (marker_set == -1) {
                 gf_log ("", GF_LOG_ERROR, "failed to get the marker status");
                 ret = -1;
                 goto out;
         }
 
-        ret = gf_string2boolean (marker_value, &marker_set);
-        if (ret != 0)
-                goto out;
-
         if (marker_set == _gf_false) {
                 gsync_status = gf_strdup ("on");
                 if (gsync_status == NULL) {
@@ -4585,7 +4574,7 @@ glusterd_set_marker_gsync (char *master)
                 }
 
                 ret = glusterd_gsync_volinfo_dict_set (volinfo,
-                                                      "features.marker-gsync", gsync_status);
+                                                       VKEY_MARKER_XTIME, gsync_status);
                 if (ret < 0)
                         goto out;
 
@@ -4847,20 +4836,15 @@ int32_t
 glusterd_check_if_quota_trans_enabled (glusterd_volinfo_t *volinfo)
 {
         int32_t  ret           = 0;
-        char    *quota_status  = NULL;
-        gf_boolean_t flag     = _gf_false;
+        int      flag          = _gf_false;
 
-        ret = glusterd_volinfo_get (volinfo, "features.quota", &quota_status);
-        if (ret) {
+        flag = glusterd_volinfo_get_boolean (volinfo, VKEY_FEATURES_QUOTA);
+        if (flag == -1) {
                 gf_log ("", GF_LOG_ERROR, "failed to get the quota status");
                 ret = -1;
                 goto out;
         }
 
-        ret = gf_string2boolean (quota_status, &flag);
-        if (ret != 0)
-                goto out;
-
         if (flag == _gf_false) {
                 gf_log ("", GF_LOG_ERROR, "first enable the quota translator");
                 ret = -1;
@@ -5113,7 +5097,7 @@ _glusterd_quota_get_limit_usages (glusterd_volinfo_t *volinfo,
         if (volinfo == NULL)
                 return NULL;
 
-        ret = glusterd_volinfo_get (volinfo, "features.limit-usage",
+        ret = glusterd_volinfo_get (volinfo, VKEY_FEATURES_LIMIT_USAGE,
                                     &quota_limits);
         if (ret)
                 return NULL;
@@ -5198,7 +5182,7 @@ glusterd_quota_enable (glusterd_volinfo_t *volinfo, char **op_errstr,
                 goto out;
         }
 
-        ret = dict_set_dynstr (volinfo->dict, "features.quota", quota_status);
+        ret = dict_set_dynstr (volinfo->dict, VKEY_FEATURES_QUOTA, quota_status);
         if (ret) {
                 gf_log ("", GF_LOG_ERROR, "dict set failed");
                 *op_errstr = gf_strdup ("Enabling quota has been unsuccessful");
@@ -5242,7 +5226,7 @@ glusterd_quota_disable (glusterd_volinfo_t *volinfo, char **op_errstr)
                 goto out;
         }
 
-        ret = dict_set_dynstr (volinfo->dict, "features.quota", quota_status);
+        ret = dict_set_dynstr (volinfo->dict, VKEY_FEATURES_QUOTA, quota_status);
         if (ret) {
                 gf_log ("", GF_LOG_ERROR, "dict set failed");
                 *op_errstr = gf_strdup ("Disabling quota has been unsuccessful");
@@ -5251,7 +5235,7 @@ glusterd_quota_disable (glusterd_volinfo_t *volinfo, char **op_errstr)
 
         *op_errstr = gf_strdup ("Disabling quota has been successful");
 
-        dict_del (volinfo->dict, "features.limit-usage");
+        dict_del (volinfo->dict, VKEY_FEATURES_LIMIT_USAGE);
 
         quota_status = gf_strdup ("off");
         if (quota_status == NULL) {
@@ -5284,7 +5268,7 @@ glusterd_quota_limit_usage (glusterd_volinfo_t *volinfo, dict_t *dict, char **op
                 goto out;
         }
 
-        ret = glusterd_volinfo_get (volinfo, "features.limit-usage",
+        ret = glusterd_volinfo_get (volinfo, VKEY_FEATURES_LIMIT_USAGE,
                                     &quota_limits);
         if (ret) {
                 gf_log ("", GF_LOG_ERROR, "failed to get the quota limits");
@@ -5336,7 +5320,7 @@ glusterd_quota_limit_usage (glusterd_volinfo_t *volinfo, dict_t *dict, char **op
 
         quota_limits = value;
 
-        ret = dict_set_str (volinfo->dict, "features.limit-usage",
+        ret = dict_set_str (volinfo->dict, VKEY_FEATURES_LIMIT_USAGE,
                             quota_limits);
         if (ret) {
                 gf_log ("", GF_LOG_ERROR, "Unable to set quota limits" );
@@ -5367,7 +5351,7 @@ glusterd_quota_remove_limits (glusterd_volinfo_t *volinfo, dict_t *dict, char **
         if (ret == -1)
                 goto out;
 
-        ret = glusterd_volinfo_get (volinfo, "features.limit-usage",
+        ret = glusterd_volinfo_get (volinfo, VKEY_FEATURES_LIMIT_USAGE,
                                     &quota_limits);
         if (ret) {
                 gf_log ("", GF_LOG_ERROR, "failed to get the quota limits");
@@ -5391,14 +5375,14 @@ glusterd_quota_remove_limits (glusterd_volinfo_t *volinfo, dict_t *dict, char **
         }
 
         if (quota_limits) {
-                ret = dict_set_str (volinfo->dict, "features.limit-usage",
+                ret = dict_set_str (volinfo->dict, VKEY_FEATURES_LIMIT_USAGE,
                                     quota_limits);
                 if (ret) {
                         gf_log ("", GF_LOG_ERROR, "Unable to set quota limits" );
                         goto out;
                 }
         } else {
-               dict_del (volinfo->dict, "features.limit-usage");
+               dict_del (volinfo->dict, VKEY_FEATURES_LIMIT_USAGE);
         }
 
         ret = 0;
@@ -6244,8 +6228,8 @@ glusterd_add_profile_volume_options (glusterd_volinfo_t *volinfo)
 
         GF_ASSERT (volinfo);
 
-        latency_key = "diagnostics.latency-measurement";
-        fd_stats_key = "diagnostics.count-fop-hits";
+        latency_key = VKEY_DIAG_LAT_MEASUREMENT;
+        fd_stats_key = VKEY_DIAG_CNT_FOP_HITS;
 
         ret = dict_set_str (volinfo->dict, latency_key, "on");
         if (ret) {
@@ -6275,8 +6259,8 @@ glusterd_remove_profile_volume_options (glusterd_volinfo_t *volinfo)
 
         GF_ASSERT (volinfo);
 
-        latency_key = "diagnostics.latency-measurement";
-        fd_stats_key = "diagnostics.count-fop-hits";
+        latency_key = VKEY_DIAG_LAT_MEASUREMENT;
+        fd_stats_key = VKEY_DIAG_CNT_FOP_HITS;
         dict_del (volinfo->dict, latency_key);
         dict_del (volinfo->dict, fd_stats_key);
 }
diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.c b/xlators/mgmt/glusterd/src/glusterd-volgen.c
index 54221d7d9..6b5653be4 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volgen.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volgen.c
@@ -74,6 +74,17 @@
  *
  * "NODOC" entries are not part of the public interface and are subject
  * to change at any time.
+ *
+ * Another kind of grouping for options, according to visibility:
+ *
+ * - Exported: one which is used in the code. These are characterized by
+ *   being used a macro as <key> (of the format VKEY_..., defined in
+ *   glusterd-volgen.h
+ *
+ * - Non-exported: the rest; these have string literal <keys>.
+ *
+ * Adhering to this policy, option name changes shall be one-liners.
+ *
  */
 typedef enum  { DOC, NO_DOC, GLOBAL_DOC, GLOBAL_NO_DOC } option_type_t;
 
@@ -106,9 +117,9 @@ static struct volopt_map_entry glusterd_volopt_map[] = {
 
         {"cluster.stripe-block-size",            "cluster/stripe",            "block-size", NULL, DOC, 0},
 
-        {"diagnostics.latency-measurement",      "debug/io-stats",     NULL, NULL, NO_DOC, 0      },
+        {VKEY_DIAG_LAT_MEASUREMENT,              "debug/io-stats",     "latency-measurement", "off", NO_DOC, 0      },
         {"diagnostics.dump-fd-stats",            "debug/io-stats",     NULL, NULL, NO_DOC, 0     },
-        {"diagnostics.count-fop-hits",           "debug/io-stats",     NULL, NULL, NO_DOC, 0     },
+        {VKEY_DIAG_CNT_FOP_HITS,                 "debug/io-stats",     "count-fop-hits", "off", NO_DOC, 0     },
         {"diagnostics.brick-log-level",          "debug/io-stats",            "!log-level", NULL, DOC, 0},
         {"diagnostics.client-log-level",         "debug/io-stats",            "!log-level", NULL, DOC, 0},
 
@@ -141,9 +152,9 @@ static struct volopt_map_entry glusterd_volopt_map[] = {
         {"performance.read-ahead",               "performance/read-ahead",    "!perf", "on", NO_DOC, 0},
         {"performance.io-cache",                 "performance/io-cache",      "!perf", "on", NO_DOC, 0},
         {"performance.quick-read",               "performance/quick-read",    "!perf", "on", NO_DOC, 0},
-        {"performance.stat-prefetch",            "performance/stat-prefetch", "!perf", "on", NO_DOC, 0},
+        {VKEY_PERF_STAT_PREFETCH,                "performance/stat-prefetch", "!perf", "on", NO_DOC, 0},
 
-        {"features.marker-gsync",                "features/marker",           "gsync", "off", NO_DOC, OPT_FLAG_FORCE},
+        {VKEY_MARKER_XTIME,                      "features/marker",           "gsync", "off", NO_DOC, OPT_FLAG_FORCE},
 
         {"nfs.enable-ino32",                     "nfs/server",                "nfs.enable-ino32", NULL, GLOBAL_DOC, 0},
         {"nfs.mem-factor",                       "nfs/server",                "nfs.mem-factor", NULL, GLOBAL_DOC, 0},
@@ -166,9 +177,8 @@ static struct volopt_map_entry glusterd_volopt_map[] = {
         {"nfs.export-dir",                       "nfs/server",                "!nfs-export-dir", NULL, DOC, 0},
         {"nfs.disable",                          "nfs/server",                "!nfs-disable", NULL, DOC, 0},
 
-        {"features.quota",                       "features/quota",            "!quota", "off", NO_DOC, OPT_FLAG_FORCE},
-        {"features.quota",                       "features/marker",           "quota", "off", NO_DOC, OPT_FLAG_FORCE},
-        {"features.limit-usage",                 "features/quota",            "limit-set", NULL, NO_DOC, 0},
+        {VKEY_FEATURES_QUOTA,                    "features/marker",           "quota", "off", NO_DOC, OPT_FLAG_FORCE},
+        {VKEY_FEATURES_LIMIT_USAGE,              "features/quota",            "limit-set", NULL, NO_DOC, 0},
         {"features.quota-timeout",               "features/quota",            "timeout", "0", NO_DOC, 0},
         {NULL,                                                                }
 };
@@ -749,6 +759,28 @@ glusterd_volinfo_get (glusterd_volinfo_t *volinfo, char *key, char **value)
         return volgen_dict_get (volinfo->dict, key, value);
 }
 
+int
+glusterd_volinfo_get_boolean (glusterd_volinfo_t *volinfo, char *key)
+{
+        char *val = NULL;
+        gf_boolean_t  boo = _gf_false;
+        int ret = 0;
+
+        ret = glusterd_volinfo_get (volinfo, key, &val);
+        if (ret)
+                return -1;
+
+        if (val)
+                ret = gf_string2boolean (val, &boo);
+        if (ret) {
+                gf_log ("", GF_LOG_ERROR, "value for %s option is not valid", key);
+
+                return -1;
+        }
+
+        return boo;
+}
+
 gf_boolean_t
 glusterd_check_voloption_flags (char *key, int32_t flags)
 {
@@ -1175,7 +1207,7 @@ glusterd_gsync_option_set (glusterd_volinfo_t *volinfo,
         GF_VALIDATE_OR_GOTO ("glusterd", xl, out);
         GF_VALIDATE_OR_GOTO ("glusterd", set_dict, out);
 
-        ret = volgen_dict_get (set_dict, "features.marker-gsync",
+        ret = volgen_dict_get (set_dict, VKEY_MARKER_XTIME,
                                &gsync_val);
         if (ret)
                 return -1;
@@ -1370,8 +1402,6 @@ client_graph_builder (glusterfs_graph_t *graph, glusterd_volinfo_t *volinfo,
         xlator_t                *xl                 = NULL;
         xlator_t                *txl                = NULL;
         xlator_t                *trav               = NULL;
-        char                    *quota_val          = NULL;
-        gf_boolean_t             quota              = _gf_false;
 
         volname = volinfo->volname;
         dict    = volinfo->dict;
@@ -1486,19 +1516,10 @@ client_graph_builder (glusterfs_graph_t *graph, glusterd_volinfo_t *volinfo,
                 }
         }
 
-        ret = glusterd_volinfo_get (volinfo, "features.quota", &quota_val);
-        if (ret)
+        ret = glusterd_volinfo_get_boolean (volinfo, VKEY_FEATURES_QUOTA);
+        if (ret == -1)
                 return -1;
-
-        if (quota_val)
-                ret = gf_string2boolean (quota_val, &quota);
         if (ret) {
-                gf_log ("", GF_LOG_ERROR, "value for quota option is not valid");
-
-                return -1;
-        }
-
-        if (quota) {
                 xl = volgen_graph_add (graph, "features/quota", volname);
 
                 if (!xl)
@@ -1735,8 +1756,7 @@ build_nfs_graph (glusterfs_graph_t *graph, dict_t *mod_dict)
                 return -1;
         }
 
-        ret = dict_set_str (set_dict, "performance.stat-prefetch",
-                            "off");
+        ret = dict_set_str (set_dict, VKEY_PERF_STAT_PREFETCH, "off");
         if (ret)
                 goto out;
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.h b/xlators/mgmt/glusterd/src/glusterd-volgen.h
index 7028ea7b6..9e90b4130 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volgen.h
+++ b/xlators/mgmt/glusterd/src/glusterd-volgen.h
@@ -27,10 +27,20 @@
 
 #include "glusterd.h"
 
+/* volopt map key name definitions */
+
+#define VKEY_DIAG_CNT_FOP_HITS    "diagnostics.count-fop-hits"
+#define VKEY_DIAG_LAT_MEASUREMENT "diagnostics.latency-measurement"
+#define VKEY_FEATURES_LIMIT_USAGE "features.limit-usage"
+#define VKEY_MARKER_XTIME         "features.marker-gsync"
+#define VKEY_FEATURES_QUOTA       "features.quota"
+#define VKEY_PERF_STAT_PREFETCH   "performance.stat-prefetch"
+
 typedef enum gd_volopt_flags_ {
         OPT_FLAG_NONE,
         OPT_FLAG_FORCE = 1,
 } gd_volopt_flags_t;
+
 int glusterd_create_rb_volfiles (glusterd_volinfo_t *volinfo,
                                  glusterd_brickinfo_t *brickinfo);
 
@@ -44,6 +54,7 @@ int glusterd_delete_volfile (glusterd_volinfo_t *volinfo,
                              glusterd_brickinfo_t *brickinfo);
 
 int glusterd_volinfo_get (glusterd_volinfo_t *volinfo, char *key, char **value);
+int glusterd_volinfo_get_boolean (glusterd_volinfo_t *volinfo, char *key);
 
 int glusterd_validate_globalopts (glusterd_volinfo_t *volinfo, dict_t *val_dict, char **op_errstr);