Snapshot: snap-max-limit implemented

Added the check on snap-max-limit in snapshot_create_prevalidate. Now snapshot
creation will fail if snap count reaches max limit.

Change-Id: I5b1cf8441f02c32085d2f6f5fd8902cb61031af5
BUG: 1049834
Author:    Joseph Fernandes <josferna@redhat.com>
Signed-off-by: Joseph Fernandes <josferna@redhat.com>
Signed-off-by: Rajesh Joseph <rjoseph@redhat.com>
Reviewed-on: http://review.gluster.org/6981
Reviewed-by: Raghavendra Bhat <raghavendra@redhat.com>

3 files changed, 206 insertions, 59 deletions

diff --git a/tests/bugs/bug-1049834.t b/tests/bugs/bug-1049834.t
new file mode 100755
index 000000000..ea4b3c85d
--- /dev/null
+++ b/tests/bugs/bug-1049834.t
@@ -0,0 +1,40 @@
+#!/bin/bash
+
+. $(dirname $0)/../include.rc
+. $(dirname $0)/../cluster.rc
+. $(dirname $0)/../snapshot.rc
+. $(dirname $0)/../volume.rc
+
+cleanup;
+
+TEST launch_cluster 2
+TEST setup_lvm 2
+
+TEST $CLI_1 peer probe $H2
+EXPECT_WITHIN 20 1 peer_count
+
+TEST $CLI_1 volume create $V0 $H1:$L1 $H2:$L2
+EXPECT 'Created' volinfo_field $V0 'Status'
+
+TEST $CLI_1 volume start $V0
+EXPECT 'Started' volinfo_field $V0 'Status'
+
+#Setting the snap-max-hard-limit to 4
+TEST $CLI_1 snapshot config $V0 snap-max-hard-limit 4
+PID_1=$!
+wait $PID_1
+
+#Creating 4 snapshots on the volume
+TEST create_n_snapshots $V0 4 $V0_snap
+TEST snapshot_n_exists $V0 4 $V0_snap
+
+#Creating the 5th snapshots on the volume and expecting it not to be created.
+TEST ! $CLI_1 snapshot create $V0 -n ${V0}_snap5
+TEST ! snapshot_exists ${V0}_snap5
+TEST ! $CLI_1 snapshot delete $V0 -s ${V0}_snap5
+
+#Deleting the 4 snaps
+TEST delete_n_snapshots $V0 4 $V0_snap
+TEST ! snapshot_n_exists $V0 4 $V0_snap
+
+cleanup;
diff --git a/tests/snapshot.rc b/tests/snapshot.rc
index 0be98dc06..230f62a68 100755
--- a/tests/snapshot.rc
+++ b/tests/snapshot.rc
@@ -175,3 +175,74 @@ function snapshot_exists() {
         $CLI snapshot list | egrep -q "\s$snapname\b"
         return $?
 }
+
+#Create N number of snaps in a given volume
+#Arg1 : <Volume Name>
+#Arg2 : <Count of snaps to be created>
+#Arg3 : <Snap Name Pattern>
+#Return: Returns 0 if all snaps are created ,
+#        if not will return exit code of last failed
+#        snap create command.
+function create_n_snapshots() {
+        local vol=$1
+        local snap_count=$2
+        local snap_name=$3
+        local ret=0
+        for i in `seq 1 $snap_count`; do
+                $CLI_1 snapshot create ${vol} -n $snap_name$i &
+                PID_1=$!
+                wait $PID_1
+                ret=$?
+                if [ "$ret" != "0" ]; then
+                        break
+                fi
+         done
+         return $ret
+}
+
+
+#Delete N number of snaps in a given volume
+#Arg1 : <Volume Name>
+#Arg2 : <Count of snaps to be deleted>
+#Arg3 : <Snap Name Pattern>
+#Return: Returns 0 if all snaps are Delete,
+#        if not will return exit code of last failed
+#        snap delete command.
+function delete_n_snapshots() {
+        local vol=$1
+        local snap_count=$2
+        local snap_name=$3
+        local ret=0
+        for i in `seq 1 $snap_count`; do
+                $CLI_1 snapshot delete ${vol} -s $snap_name$i &
+                PID_1=$!
+                wait $PID_1
+                temp=$?
+                if [ "$temp" != "0" ]; then
+                        ret=$temp
+                fi
+         done
+         return $ret
+}
+
+#Check for the existance of N number of snaps in a given volume
+#Arg1 : <Volume Name>
+#Arg2 : <Count of snaps to be checked>
+#Arg3 : <Snap Name Pattern>
+#Return: Returns 0 if all snaps exists,
+#        if not will return exit code of last failed
+#        snapshot_exists().
+function snapshot_n_exists() {
+        local vol=$1
+        local snap_count=$2
+        local snap_name=$3
+        local ret=0
+        for i in `seq 1 $snap_count`; do
+                snapshot_exists $snap_name$i
+                ret=$?
+                if [ "$ret" != "0" ]; then
+                        break
+                fi
+         done
+         return $ret
+}
diff --git a/xlators/mgmt/glusterd/src/glusterd-snapshot.c b/xlators/mgmt/glusterd/src/glusterd-snapshot.c
index e886f4b55..4093cda1c 100644
--- a/xlators/mgmt/glusterd/src/glusterd-snapshot.c
+++ b/xlators/mgmt/glusterd/src/glusterd-snapshot.c
@@ -359,15 +359,16 @@ out:
 int
 glusterd_snapshot_config_prevalidate (dict_t *dict, char **op_errstr)
 {
-        char               *volname              = NULL;
-        char               *key                  = NULL;
-        glusterd_volinfo_t *volinfo              = NULL;
-        xlator_t           *this                 = NULL;
-        int                 ret                  = -1;
-        int                 config_command       = 0;
-        char                err_str[PATH_MAX]    = {0,};
-        glusterd_conf_t    *conf                 = NULL;
-        uint64_t            value                = 0;
+        char               *volname             = NULL;
+        char               *key                 = NULL;
+        glusterd_volinfo_t *volinfo             = NULL;
+        xlator_t           *this                = NULL;
+        int                 ret                 = -1;
+        int                 config_command      = 0;
+        char                err_str[PATH_MAX]   = {0,};
+        glusterd_conf_t    *conf                = NULL;
+        uint64_t            value               = 0;
+        gf_loglevel_t       loglevel            = GF_LOG_ERROR;
 
         this = THIS;
 
@@ -381,8 +382,8 @@ glusterd_snapshot_config_prevalidate (dict_t *dict, char **op_errstr)
 
         ret = dict_get_int32 (dict, "config-command", &config_command);
         if (ret) {
-                gf_log (this->name, GF_LOG_ERROR,
-                        "failed to get config-command type");
+                snprintf (err_str, sizeof (err_str),
+                          "failed to get config-command type");
                 goto out;
         }
 
@@ -390,30 +391,25 @@ glusterd_snapshot_config_prevalidate (dict_t *dict, char **op_errstr)
         if (!ret) {
                 ret = dict_get_uint64 (dict, key, &value);
                 if (ret) {
-                        snprintf (err_str, PATH_MAX,"Failed to get the"
+                        snprintf (err_str, sizeof (err_str), "Failed to get the"
                                   " value for %s", key);
-                        *op_errstr = gf_strdup (err_str);
-                        gf_log (this->name, GF_LOG_ERROR, "%s", err_str);
+
                         goto out;
                 }
         }
 
         ret = dict_get_str (dict, "volname", &volname);
         if (ret) {
-                snprintf (err_str, PATH_MAX,"Failed to get the"
+                snprintf (err_str, sizeof (err_str), "Failed to get the"
                           " volume name");
-                *op_errstr = gf_strdup (err_str);
-                gf_log (this->name, GF_LOG_ERROR, "%s", err_str);
                 goto out;
         }
 
         if (strncmp (volname, "all", strlen(volname))) {
                 ret = glusterd_volinfo_find (volname, &volinfo);
                 if (ret) {
-                        snprintf (err_str, PATH_MAX,"Volume %s does not exist.",
-                                  volname);
-                        *op_errstr = gf_strdup (err_str);
-                        gf_log (this->name, GF_LOG_ERROR, "%s", err_str);
+                        snprintf (err_str, sizeof (err_str),
+                                  "Volume %s does not exist.", volname);
                         goto out;
                 }
         }
@@ -437,6 +433,12 @@ glusterd_snapshot_config_prevalidate (dict_t *dict, char **op_errstr)
                 break;
         }
 out:
+
+         if (ret && err_str[0] != '\0') {
+                 gf_log (this->name, loglevel, "%s", err_str);
+                 *op_errstr = gf_strdup (err_str);
+         }
+
         return ret;
 }
 
@@ -586,6 +588,7 @@ glusterd_snapshot_create_prevalidate (dict_t *dict, char **op_errstr,
         char                   snapbrckcnt[PATH_MAX]     = "";
         char                   snapbrckord[PATH_MAX]     = "";
         char                   snap_volname[64]          = "";
+        char                   err_str[PATH_MAX]         = "";
         int                    ret                       = -1;
         int64_t                i                         = 0;
         int64_t                volume_count              = 0;
@@ -596,13 +599,15 @@ glusterd_snapshot_create_prevalidate (dict_t *dict, char **op_errstr,
         gf_boolean_t           is_cg                     = _gf_false;
         xlator_t              *this                      = NULL;
         uuid_t                *snap_volid                = NULL;
+        gf_loglevel_t          loglevel                  = GF_LOG_ERROR;
 
         this = THIS;
+        GF_ASSERT (op_errstr);
 
         ret = dict_get_int64 (dict, "volcount", &volume_count);
         if (ret) {
-                gf_log (this->name, GF_LOG_ERROR, "failed to "
-                        "get the volume count");
+                snprintf (err_str, sizeof (err_str), "failed to "
+                          "get the volume count");
                 goto out;
         }
         GF_ASSERT (volume_count);
@@ -611,15 +616,15 @@ glusterd_snapshot_create_prevalidate (dict_t *dict, char **op_errstr,
                 is_cg = _gf_true;
                 ret = dict_get_str (dict, "cg-name", &name);
                 if (ret) {
-                        gf_log (this->name, GF_LOG_ERROR,
-                                "failed to get cg-name");
+                        snprintf (err_str, sizeof (err_str),
+                                  "failed to get cg-name");
                         goto out;
                 }
         } else {
                 ret = dict_get_str (dict, "snap-name", &name);
                 if (ret) {
-                        gf_log (this->name, GF_LOG_ERROR,
-                                "failed to get snap-name");
+                        snprintf (err_str, sizeof (err_str),
+                                  "failed to get snap-name");
                         goto out;
                 }
         }
@@ -629,29 +634,46 @@ glusterd_snapshot_create_prevalidate (dict_t *dict, char **op_errstr,
                 snprintf (key, sizeof (key), "volname%ld", i+1);
                 ret = dict_get_str (dict, key, &volname);
                 if (ret) {
-                        gf_log (this->name, GF_LOG_ERROR,
-                                "failed to get volume name");
+                        snprintf (err_str, sizeof (err_str),
+                                  "failed to get volume name");
                         goto out;
                 }
                 ret = glusterd_volinfo_find (volname, &volinfo);
                 if (ret) {
-                        gf_log (this->name, GF_LOG_ERROR,
-                                "failed to get the volinfo for "
-                                "the volume %s", volname);
+                        snprintf (err_str, sizeof (err_str),
+                                  "failed to get the volinfo for the volume "
+                                  "%s", volname);
                         goto out;
                 }
                 if (!glusterd_is_volume_started (volinfo)) {
-                        gf_log (this->name, GF_LOG_WARNING,
-                                "volume %s is not started",
-                                volinfo->volname);
                         ret = -1;
+                        snprintf (err_str, sizeof (err_str),
+                                  "volume %s is not started",volinfo->volname);
+                        loglevel = GF_LOG_WARNING;
                         goto out;
                 }
                 if (glusterd_is_defrag_on (volinfo)) {
                         ret = -1;
-                        gf_log (this->name, GF_LOG_WARNING,
-                                "rebalance process is running "
-                                "for the volume %s", volname);
+                        snprintf (err_str, sizeof (err_str),
+                                  "rebalance process is running for the "
+                                  "volume %s", volname);
+                        loglevel = GF_LOG_WARNING;
+                        goto out;
+                }
+                if (volinfo->is_snap_volume == _gf_true) {
+                        ret = -1;
+                        snprintf (err_str, sizeof (err_str),
+                                  "Volume %s is a snap volume", volname);
+                        loglevel = GF_LOG_WARNING;
+                        goto out;
+                }
+                if (volinfo->snap_count >= volinfo->snap_max_hard_limit) {
+                       ret = -1;
+                       snprintf (err_str, sizeof (err_str),
+                                 "The number of existing snaps has reached the "
+                                 "maximum limit of %"PRIu64" ,for the volume %s",
+                                 volinfo->snap_max_hard_limit, volname);
+                        loglevel = GF_LOG_WARNING;
                         goto out;
                 }
 
@@ -670,8 +692,10 @@ glusterd_snapshot_create_prevalidate (dict_t *dict, char **op_errstr,
 
                 if (is_cg) {
                         memset(snapname, '\0', sizeof (snapname));
-                        snprintf (tmp, sizeof (tmp), "%s_snap", volinfo->volname);
-                        snprintf (snapname, sizeof (snapname), "%s_%s", name, tmp);
+                        snprintf (tmp, sizeof (tmp), "%s_snap",
+                                  volinfo->volname);
+                        snprintf (snapname, sizeof (snapname), "%s_%s",
+                                  name, tmp);
                         strcpy(snap_volname, snapname);
                 }
 
@@ -692,9 +716,10 @@ glusterd_snapshot_create_prevalidate (dict_t *dict, char **op_errstr,
 
                         device = glusterd_get_brick_mount_details (brickinfo);
                         if (!device) {
-                                gf_log (this->name, GF_LOG_ERROR, "getting device name for "
-                                        "the brick %s:%s failed", brickinfo->hostname,
-                                        brickinfo->path);
+                                snprintf (err_str, sizeof (err_str),
+                                          "getting device name for the brick "
+                                          "%s:%s failed", brickinfo->hostname,
+                                          brickinfo->path);
                                 ret = -1;
                                 goto out;
                         }
@@ -702,9 +727,11 @@ glusterd_snapshot_create_prevalidate (dict_t *dict, char **op_errstr,
                         device = glusterd_build_snap_device_path (device,
                                                                   snap_volname);
                         if (!device) {
-                                gf_log (this->name, GF_LOG_WARNING, "cannot copy the "
-                                        "snapshot device name (volname: %s, "
-                                        "snapname: %s)", volinfo->volname, snapname);
+                                snprintf (err_str, sizeof (err_str),
+                                          "cannot copy the snapshot device "
+                                          "name (volname: %s, snapname: %s)",
+                                          volinfo->volname, snapname);
+                                loglevel = GF_LOG_WARNING;
                                 ret = -1;
                                 goto out;
                         }
@@ -721,14 +748,17 @@ glusterd_snapshot_create_prevalidate (dict_t *dict, char **op_errstr,
 
                         ret = glusterd_get_brick_root (brickinfo->path, &mnt_pt);
                         if (ret) {
-                                gf_log (this->name, GF_LOG_WARNING, "could not get the root of"
-                                        "the brick path %s", brickinfo->path);
+                                snprintf (err_str, sizeof (err_str),
+                                "could not get the root of the brick path %s",
+                                brickinfo->path);
+                                loglevel = GF_LOG_WARNING;
                                 goto out;
                         }
-
                         if (strncmp (brickinfo->path, mnt_pt, strlen (mnt_pt))) {
-                                gf_log (this->name, GF_LOG_WARNING, "brick: %s brick mount: %s",
-                                        brickinfo->path, mnt_pt);
+                                snprintf (err_str, sizeof (err_str),
+                                "brick: %s brick mount: %s",
+                                 brickinfo->path, mnt_pt);
+                                loglevel = GF_LOG_WARNING;
                                 goto out;
                         }
 
@@ -736,8 +766,8 @@ glusterd_snapshot_create_prevalidate (dict_t *dict, char **op_errstr,
                         if (snap_brick_dir)
                                 snap_brick_dir++;
 
-                        snprintf (snap_brick_path, sizeof (snap_brick_path), "%s/%s",
-                                  snap_mount, snap_brick_dir);
+                        snprintf (snap_brick_path, sizeof (snap_brick_path),
+                                  "%s/%s", snap_mount, snap_brick_dir);
 
                         ret = snprintf (snapmntname, sizeof(snapmntname) - 1,
                                         "vol%ld.brick%ld", i+1, brick_count);
@@ -745,26 +775,27 @@ glusterd_snapshot_create_prevalidate (dict_t *dict, char **op_errstr,
 
                         tmpstr = gf_strdup (snap_brick_path);
                         if (!tmpstr) {
-                                gf_log ("", GF_LOG_ERROR, "Out Of Memory");
                                 ret = -1;
                                 goto out;
                         }
                         ret = dict_set_dynstr (rsp_dict, snapmntname, tmpstr);
                         if (ret) {
-                                gf_log ("", GF_LOG_ERROR, "Failed to set %s",
-                                        snap_mount);
+                                gf_log (this->name, GF_LOG_ERROR,
+                                        "Failed to set %s", snap_mount);
                                 GF_FREE (tmpstr);
                                 goto out;
                         }
                         tmpstr = NULL;
 
                         ret = snprintf (snapbrckord, sizeof(snapbrckord) - 1,
-                                        "vol%ld.brick%ld.order", i+1, brick_count);
+                                        "vol%ld.brick%ld.order",
+                                        i+1, brick_count);
                         snapbrckord[ret] = '\0';
 
                         ret = dict_set_int64 (rsp_dict, snapbrckord, brick_order);
                         if (ret) {
-                                gf_log ("", GF_LOG_ERROR, "Failed to set brick order");
+                                gf_log (this->name, GF_LOG_ERROR,
+                                        "Failed to set brick order");
                                 goto out;
                         }
 
@@ -794,7 +825,12 @@ out:
         if (ret)
                 GF_FREE (tmpstr);
 
-        gf_log ("", GF_LOG_TRACE, "Returning %d", ret);
+         if (ret && err_str[0] != '\0') {
+                 gf_log (this->name, loglevel, "%s", err_str);
+                 *op_errstr = gf_strdup (err_str);
+         }
+
+        gf_log (this->name, GF_LOG_TRACE, "Returning %d", ret);
         return ret;
 }