diff options
| -rw-r--r-- | tests/bugs/glusterd/bug-1245045-remove-brick-validation.t | 54 | ||||
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-brick-ops.c | 119 | ||||
| -rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-utils.c | 1 | 
3 files changed, 143 insertions, 31 deletions
diff --git a/tests/bugs/glusterd/bug-1245045-remove-brick-validation.t b/tests/bugs/glusterd/bug-1245045-remove-brick-validation.t new file mode 100644 index 00000000000..22a8d557d28 --- /dev/null +++ b/tests/bugs/glusterd/bug-1245045-remove-brick-validation.t @@ -0,0 +1,54 @@ +#!/bin/bash + +. $(dirname $0)/../../include.rc +. $(dirname $0)/../../cluster.rc + +cleanup + +TEST launch_cluster 3; +TEST $CLI_1 peer probe $H2; +TEST $CLI_1 peer probe $H3; +EXPECT_WITHIN $PROBE_TIMEOUT 2 peer_count + +TEST $CLI_1 volume create $V0 $H1:$B1/$V0 $H2:$B2/$V0 +TEST $CLI_1 volume start $V0 + +kill_glusterd 2 + +#remove-brick should fail as the peer hosting the brick is down +TEST ! $CLI_1 volume remove-brick $V0 $H2:$B2/${V0} start + +TEST start_glusterd 2 + +EXPECT_WITHIN $PROBE_TIMEOUT 2 peer_count + +#volume status should work +TEST $CLI_2 volume status + + +TEST $CLI_1 volume remove-brick $V0 $H2:$B2/${V0} start +kill_glusterd 2 + +#remove-brick commit should fail as the peer hosting the brick is down +TEST ! $CLI_1 volume remove-brick $V0 $H2:$B2/${V0} commit + +TEST start_glusterd 2 + +EXPECT_WITHIN $PROBE_TIMEOUT 2 peer_count + +#volume status should work +TEST $CLI_2 volume status + +TEST $CLI_1 volume remove-brick $V0 $H2:$B2/${V0} stop + +kill_glusterd 3 +EXPECT_WITHIN $PROBE_TIMEOUT 1 peer_count + +TEST $CLI_1 volume remove-brick $V0 $H2:$B2/${V0} start + +TEST start_glusterd 3 +EXPECT_WITHIN $PROBE_TIMEOUT 2 peer_count + +TEST $CLI_3 volume status + +cleanup diff --git a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c index 76a05b2ef93..9e6c73d88a1 100644 --- a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c @@ -1656,6 +1656,83 @@ out:          return ret;  } +static int +glusterd_remove_brick_validate_bricks (gf1_op_commands cmd, int32_t brick_count, +                                       dict_t *dict, +                                       glusterd_volinfo_t *volinfo, +                                       char **errstr) +{ +        char                   *brick       = NULL; +        char                    msg[2048]   = {0,}; +        char                    key[256]    = {0,}; +        glusterd_brickinfo_t   *brickinfo   = NULL; +        glusterd_peerinfo_t    *peerinfo    = NULL; +        int                     i           = 0; +        int                     ret         = -1; + +        /* Check whether all the nodes of the bricks to be removed are +        * up, if not fail the operation */ +        for (i = 1; i <= brick_count; i++) { +                snprintf (key, sizeof (key), "brick%d", i); +                ret = dict_get_str (dict, key, &brick); +                if (ret) { +                        snprintf (msg, sizeof (msg), +                                  "Unable to get %s", key); +                        *errstr = gf_strdup (msg); +                        goto out; +                } + +                ret = +                glusterd_volume_brickinfo_get_by_brick(brick, volinfo, +                                                       &brickinfo); +                if (ret) { +                        snprintf (msg, sizeof (msg), "Incorrect brick " +                                  "%s for volume %s", brick, volinfo->volname); +                        *errstr = gf_strdup (msg); +                        goto out; +                } +                /* Do not allow commit if the bricks are not decommissioned +                 * if its a remove brick commit +                 */ +                if (cmd == GF_OP_CMD_COMMIT && !brickinfo->decommissioned) { +                        snprintf (msg, sizeof (msg), "Brick %s " +                                  "is not decommissioned. " +                                  "Use start or force option", +                                  brick); +                        *errstr = gf_strdup (msg); +                        ret = -1; +                        goto out; +                } + +                if (glusterd_is_local_brick (THIS, volinfo, brickinfo)) +                        continue; + +                rcu_read_lock (); +                peerinfo = glusterd_peerinfo_find_by_uuid +                                                (brickinfo->uuid); +                if (!peerinfo) { +                        snprintf (msg, sizeof(msg), "Host node of the " +                                  "brick %s is not in cluster", brick); +                        *errstr = gf_strdup (msg); +                        ret = -1; +                        rcu_read_unlock (); +                        goto out; +                } +                if (!peerinfo->connected) { +                        snprintf (msg, sizeof(msg), "Host node of the " +                                  "brick %s is down", brick); +                        *errstr = gf_strdup (msg); +                        ret = -1; +                        rcu_read_unlock (); +                        goto out; +                } +                rcu_read_unlock (); +        } + +out: +        return ret; +} +  int  glusterd_op_stage_remove_brick (dict_t *dict, char **op_errstr)  { @@ -1674,6 +1751,7 @@ glusterd_op_stage_remove_brick (dict_t *dict, char **op_errstr)          char                   *brick       = NULL;          glusterd_brickinfo_t   *brickinfo   = NULL;          gsync_status_param_t    param       = {0,}; +        glusterd_peerinfo_t    *peerinfo    = NULL;          this = THIS;          GF_ASSERT (this); @@ -1812,6 +1890,12 @@ glusterd_op_stage_remove_brick (dict_t *dict, char **op_errstr)                          goto out;                  } +                ret = glusterd_remove_brick_validate_bricks (cmd, brick_count, +                                                             dict, volinfo, +                                                             &errstr); +                if (ret) +                        goto out; +                  if (is_origin_glusterd (dict)) {                          ret = glusterd_generate_and_set_task_id                                  (dict, GF_REMOVE_BRICK_TID_KEY); @@ -1856,36 +1940,11 @@ glusterd_op_stage_remove_brick (dict_t *dict, char **op_errstr)                          goto out;                  } -                /* Do not allow commit if the bricks are not decommissioned */ -                for ( i = 1; i <= brick_count; i++ ) { -                        snprintf (key, sizeof (key), "brick%d", i); -                        ret = dict_get_str (dict, key, &brick); -                        if (ret) { -                                snprintf (msg, sizeof (msg), -                                          "Unable to get %s", key); -                                errstr = gf_strdup (msg); -                                goto out; -                        } - -                        ret = -                        glusterd_volume_brickinfo_get_by_brick(brick, volinfo, -                                                               &brickinfo); -                        if (ret) { -                                snprintf (msg, sizeof (msg), "Incorrect brick " -                                          "%s for volume %s", brick, volname); -                                errstr = gf_strdup (msg); -                                goto out; -                        } -                        if ( !brickinfo->decommissioned ) { -                                snprintf (msg, sizeof (msg), "Brick %s " -                                          "is not decommissioned. " -                                          "Use start or force option", -                                          brick); -                                errstr = gf_strdup (msg); -                                ret = -1; -                                goto out; -                        } -                } +                ret = glusterd_remove_brick_validate_bricks (cmd, brick_count, +                                                             dict, volinfo, +                                                             &errstr); +                if (ret) +                        goto out;                  /* If geo-rep is configured, for this volume, it should be                   * stopped. diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c index ed9e70b9caa..26a7fface29 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-utils.c @@ -3735,7 +3735,6 @@ gd_check_and_update_rebalance_info (glusterd_volinfo_t *old_volinfo,          new->skipped_files      = old->skipped_files;          new->rebalance_failures = old->rebalance_failures;          new->rebalance_time     = old->rebalance_time; -        new->dict               = (old->dict ? dict_ref (old->dict) : NULL);          /* glusterd_rebalance_t.{op, id, defrag_cmd} are copied during volume           * import  | 
