diff options
| author | Ravishankar N <ravishankar@redhat.com> | 2017-07-31 23:38:20 +0530 | 
|---|---|---|
| committer | Shyamsundar Ranganathan <srangana@redhat.com> | 2017-08-08 12:24:02 +0000 | 
| commit | 25ebb4d36fddb75f19c78e015aa8e9374964a5bb (patch) | |
| tree | 12dd888d7f5c52a372b620b98e978bf67f985a21 | |
| parent | aad7c1c196e0cf60b821ef7db72bf5279253d4b8 (diff) | |
posix: add sanity checks for removing the gfid symlink for directories
...during mkdir and rmdir. Otherwise, during entry self-heal, the
directory could be left out without a .glusterfs symlink causing fops like
opendir, readdir to fail.  The only chance the missing symlink will be
created is when a fresh lookup comes on it.
> Reviewed-on: https://review.gluster.org/17945
> Smoke: Gluster Build System <jenkins@build.gluster.org>
> CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
> Reviewed-by: Raghavendra Bhat <raghavendra@redhat.com>
(cherry picked from commit 42c057a6d1a03dd2825a278393acb15d52220c8d)
Change-Id: I2e1cf1bce8962ea80187edd8f6d73e0a09cf9f8e
BUG: 1479118
Signed-off-by: Ravishankar N <ravishankar@redhat.com>
Reviewed-on: https://review.gluster.org/17991
Smoke: Gluster Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Shyamsundar Ranganathan <srangana@redhat.com>
| -rw-r--r-- | tests/bugs/replicate/bug-1477169-entry-selfheal-rename.t | 47 | ||||
| -rw-r--r-- | xlators/storage/posix/src/posix.c | 70 | 
2 files changed, 103 insertions, 14 deletions
diff --git a/tests/bugs/replicate/bug-1477169-entry-selfheal-rename.t b/tests/bugs/replicate/bug-1477169-entry-selfheal-rename.t new file mode 100644 index 00000000000..465800b19da --- /dev/null +++ b/tests/bugs/replicate/bug-1477169-entry-selfheal-rename.t @@ -0,0 +1,47 @@ +#!/bin/bash +. $(dirname $0)/../../include.rc +. $(dirname $0)/../../volume.rc +cleanup; + +TEST glusterd +TEST pidof glusterd +TEST $CLI volume create $V0 replica 3 $H0:$B0/${V0}{0,1,2} +TEST $CLI volume start $V0 + +TEST $GFS --volfile-id=$V0 --volfile-server=$H0 --attribute-timeout=0 --entry-timeout=0 $M0; +EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status $V0 0 +EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status $V0 1 +EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status $V0 2 + +TEST mkdir -p $M0/d1/dir $M0/d2 +gfid_d1=$(gf_get_gfid_xattr $B0/${V0}0/d1) +gfid_d2=$(gf_get_gfid_xattr $B0/${V0}0/d2) +gfid_dir=$(gf_get_gfid_xattr $B0/${V0}0/d1/dir) + +gfid_str_d1=$(gf_gfid_xattr_to_str $gfid_d1) +gfid_str_d2=$(gf_gfid_xattr_to_str $gfid_d2) +gfid_str_d3=$(gf_gfid_xattr_to_str $gfid_dir) + +# Kill 3rd brick and rename the dir from mount. +TEST kill_brick $V0 $H0 $B0/${V0}2 +TEST mv $M0/d1/dir $M0/d2 + +# Bring it back and trigger heal. +TEST $CLI volume start $V0 force + +EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status $V0 2 +EXPECT_WITHIN $PROCESS_UP_TIMEOUT "Y" glustershd_up_status +EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 0 +EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 1 +EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 2 + +TEST $CLI volume heal $V0 +EXPECT_WITHIN $HEAL_TIMEOUT "^0$" get_pending_heal_count $V0 + +# Check that .glusterfs symlink for dir exists and points to d2/dir +TEST linkname=$(readlink $B0/${V0}2/.glusterfs/${gfid_str_d3:0:2}/${gfid_str_d3:2:2}/$gfid_str_d3) +EXPECT "dir" basename $linkname +TEST parent_dir_gfid_str=$(echo $linkname|cut -d / -f5) +EXPECT $gfid_str_d2 echo $parent_dir_gfid_str + +cleanup; diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix.c index 6d7fc24b2da..1e139e5c32b 100644 --- a/xlators/storage/posix/src/posix.c +++ b/xlators/storage/posix/src/posix.c @@ -109,6 +109,37 @@ static char *disallow_removexattrs[] = {          NULL  }; +gf_boolean_t +posix_symlinks_match (xlator_t *this, loc_t *loc, uuid_t gfid) +{ +        struct posix_private *priv = NULL; +        char linkname_actual[PATH_MAX] = {0,}; +        char linkname_expected[PATH_MAX] = {0}; +        char *dir_handle = NULL; +        size_t len = 0; +        size_t handle_size = 0; +        gf_boolean_t ret = _gf_false; + +        priv = this->private; +        handle_size = POSIX_GFID_HANDLE_SIZE(priv->base_path_length); +        dir_handle = alloca0 (handle_size); + +        snprintf (linkname_expected, handle_size, "../../%02x/%02x/%s/%s", +                  loc->pargfid[0], loc->pargfid[1], uuid_utoa (loc->pargfid), +                  loc->name); + +        MAKE_HANDLE_GFID_PATH (dir_handle, this, gfid, NULL); +        len = sys_readlink (dir_handle, linkname_actual, PATH_MAX); +        if (len < 0) +                goto out; +        linkname_actual[len] = '\0'; + +        if (!strncmp (linkname_actual, linkname_expected, handle_size)) +                ret = _gf_true; + +out: +        return ret; +}  dict_t*  posix_dict_set_nlink (dict_t *req, dict_t *res, int32_t nlink) @@ -1573,19 +1604,29 @@ posix_mkdir (call_frame_t *frame, xlator_t *this,                                  posix_handle_path (this, uuid_req, NULL,                                                     gfid_path, size); -                        gf_msg (this->name, GF_LOG_WARNING, 0, -                                P_MSG_DIR_OF_SAME_ID, "mkdir (%s): gfid (%s) " -                                "is already associated with directory (%s). " -                                "Hence, both directories will share same gfid " -                                "and this can lead to inconsistencies.", -                                loc->path, uuid_utoa (uuid_req), -                                gfid_path ? gfid_path : "<NULL>"); - -                        gf_event (EVENT_POSIX_SAME_GFID, "gfid=%s;path=%s;" -                                  "newpath=%s;brick=%s:%s", -                                  uuid_utoa (uuid_req), -                                  gfid_path ? gfid_path : "<NULL>", loc->path, -                                  priv->hostname, priv->base_path); +                        if (frame->root->pid != GF_CLIENT_PID_SELF_HEALD) { +                                gf_msg (this->name, GF_LOG_WARNING, 0, +                                        P_MSG_DIR_OF_SAME_ID, "mkdir (%s): " +                                        "gfid (%s) is already associated with " +                                        "directory (%s). Hence, both " +                                        "directories will share same gfid and " +                                        "this can lead to inconsistencies.", +                                        loc->path, uuid_utoa (uuid_req), +                                        gfid_path ? gfid_path : "<NULL>"); + +                                gf_event (EVENT_POSIX_SAME_GFID, "gfid=%s;" +                                          "path=%s;newpath=%s;brick=%s:%s", +                                          uuid_utoa (uuid_req), +                                          gfid_path ? gfid_path : "<NULL>", +                                          loc->path, priv->hostname, +                                          priv->base_path); +                        } +                        if (!posix_symlinks_match (this, loc, uuid_req)) +                                /* For afr selfheal of dir renames, we need to +                                 * remove the old symlink in order for +                                 * posix_gfid_set to set the symlink to the +                                 * new dir.*/ +                                posix_handle_unset (this, stbuf.ia_gfid, NULL);                  }          } else if (!uuid_req && frame->root->pid != GF_SERVER_PID_TRASH) {                  op_ret = -1; @@ -2276,7 +2317,8 @@ posix_rmdir (call_frame_t *frame, xlator_t *this,          op_errno = errno;          if (op_ret == 0) { -                posix_handle_unset (this, stbuf.ia_gfid, NULL); +                if (posix_symlinks_match (this, loc, stbuf.ia_gfid)) +                        posix_handle_unset (this, stbuf.ia_gfid, NULL);          }          if (op_errno == EEXIST)  | 
