From bd38426958fb27879bc1a06cf7104e94c7789796 Mon Sep 17 00:00:00 2001 From: Joseph Fernandes Date: Wed, 12 Feb 2014 13:03:30 +0530 Subject: 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 Signed-off-by: Joseph Fernandes Signed-off-by: Rajesh Joseph Reviewed-on: http://review.gluster.org/6981 Reviewed-by: Raghavendra Bhat --- tests/bugs/bug-1049834.t | 40 +++++++ tests/snapshot.rc | 71 ++++++++++++ xlators/mgmt/glusterd/src/glusterd-snapshot.c | 154 ++++++++++++++++---------- 3 files changed, 206 insertions(+), 59 deletions(-) create mode 100755 tests/bugs/bug-1049834.t 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 : +#Arg2 : +#Arg3 : +#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 : +#Arg2 : +#Arg3 : +#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 : +#Arg2 : +#Arg3 : +#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; } -- cgit