storage/posix: removing deleting entries in case of creation failures

The code is not atomic enough to not to delete a dentry created by a
prallel dentry creation operation.

Change-Id: I9bd6d2aa9e7a1c0688c0a937b02a4b4f56d7aa3d
BUG: 1139988
Signed-off-by: Raghavendra G <rgowdapp@redhat.com>
Reviewed-on: http://review.gluster.org/8327
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>
Reviewed-on: http://review.gluster.org/8675
Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com>

4 files changed, 134 insertions, 84 deletions

diff --git a/xlators/storage/posix/src/posix-handle.c b/xlators/storage/posix/src/posix-handle.c
index a19b3ff00f5..abdae282c51 100644
--- a/xlators/storage/posix/src/posix-handle.c
+++ b/xlators/storage/posix/src/posix-handle.c
@@ -660,7 +660,7 @@ posix_handle_soft (xlator_t *this, const char *real_path, loc_t *loc,
 }
 
 
-static int
+int
 posix_handle_unset_gfid (xlator_t *this, uuid_t gfid)
 {
         char        *path = NULL;
diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c
index 545956690a7..9f781026f39 100644
--- a/xlators/storage/posix/src/posix-helpers.c
+++ b/xlators/storage/posix/src/posix-helpers.c
@@ -449,6 +449,24 @@ out:
         return xattr;
 }
 
+void
+posix_gfid_unset (xlator_t *this, dict_t *xdata)
+{
+        uuid_t uuid = {0, };
+        int    ret  = 0;
+
+        if (xdata == NULL)
+                goto out;
+
+        ret = dict_get_ptr (xdata, "gfid-req", (void **)&uuid);
+        if (ret) {
+                goto out;
+        }
+
+        posix_handle_unset (this, uuid, NULL);
+out:
+        return;
+}
 
 int
 posix_gfid_set (xlator_t *this, const char *path, loc_t *loc, dict_t *xattr_req)
diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix.c
index bf5c188e5ca..c0dc3862006 100644
--- a/xlators/storage/posix/src/posix.c
+++ b/xlators/storage/posix/src/posix.c
@@ -728,18 +728,18 @@ int
 posix_mknod (call_frame_t *frame, xlator_t *this,
              loc_t *loc, mode_t mode, dev_t dev, mode_t umask, dict_t *xdata)
 {
-        int                   tmp_fd      = 0;
-        int32_t               op_ret      = -1;
-        int32_t               op_errno    = 0;
-        char                 *real_path   = 0;
-        char                 *par_path    = 0;
-        struct iatt           stbuf       = { 0, };
-        char                  was_present = 1;
-        struct posix_private *priv        = NULL;
-        gid_t                 gid         = 0;
-        struct iatt           preparent = {0,};
-        struct iatt           postparent = {0,};
-        void *                uuid_req  = NULL;
+        int                   tmp_fd          = 0;
+        int32_t               op_ret          = -1;
+        int32_t               op_errno        = 0;
+        char                 *real_path       = 0;
+        char                 *par_path        = 0;
+        struct iatt           stbuf           = { 0, };
+        struct posix_private *priv            = NULL;
+        gid_t                 gid             = 0;
+        struct iatt           preparent       = {0,};
+        struct iatt           postparent      = {0,};
+        void *                uuid_req        = NULL;
+        gf_boolean_t          entry_created   = _gf_false, gfid_set = _gf_false;
 
         DECLARE_OLD_FS_ID_VAR;
 
@@ -817,10 +817,14 @@ real_op:
                 }
         }
 
+        entry_created = _gf_true;
+
         op_ret = posix_gfid_set (this, real_path, loc, xdata);
         if (op_ret) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "setting gfid on %s failed", real_path);
+        } else {
+                gfid_set = _gf_true;
         }
 
 #ifndef HAVE_SET_FSID
@@ -876,28 +880,35 @@ out:
                              (loc)?loc->inode:NULL, &stbuf, &preparent,
                              &postparent, NULL);
 
-        if ((op_ret == -1) && (!was_present)) {
-                unlink (real_path);
+        if (op_ret < 0) {
+                if (entry_created) {
+                        if (S_ISREG (mode))
+                                sys_unlink (real_path);
+                        else
+                                sys_rmdir (real_path);
+                }
+
+                if (gfid_set)
+                        posix_gfid_unset (this, xdata);
         }
 
         return 0;
 }
 
-
 int
 posix_mkdir (call_frame_t *frame, xlator_t *this,
              loc_t *loc, mode_t mode, mode_t umask, dict_t *xdata)
 {
-        int32_t               op_ret      = -1;
-        int32_t               op_errno    = 0;
-        char                 *real_path   = NULL;
-        char                 *par_path   = NULL;
-        struct iatt           stbuf       = {0, };
-        char                  was_present = 1;
-        struct posix_private *priv        = NULL;
-        gid_t                 gid         = 0;
-        struct iatt           preparent = {0,};
-        struct iatt           postparent = {0,};
+        int32_t               op_ret        = -1;
+        int32_t               op_errno      = 0;
+        char                 *real_path     = NULL;
+        char                 *par_path      = NULL;
+        struct iatt           stbuf         = {0, };
+        struct posix_private *priv          = NULL;
+        gid_t                 gid           = 0;
+        struct iatt           preparent     = {0,};
+        struct iatt           postparent    = {0,};
+        gf_boolean_t          entry_created = _gf_false, gfid_set = _gf_false;
 
         DECLARE_OLD_FS_ID_VAR;
 
@@ -925,9 +936,6 @@ posix_mkdir (call_frame_t *frame, xlator_t *this,
         gid = frame->root->gid;
 
         op_ret = posix_pstat (this, NULL, real_path, &stbuf);
-        if ((op_ret == -1) && (errno == ENOENT)) {
-                was_present = 0;
-        }
 
         SET_FS_ID (frame->root->uid, gid);
 
@@ -954,10 +962,14 @@ posix_mkdir (call_frame_t *frame, xlator_t *this,
                 goto out;
         }
 
+        entry_created = _gf_true;
+
         op_ret = posix_gfid_set (this, real_path, loc, xdata);
         if (op_ret) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "setting gfid on %s failed", real_path);
+        } else {
+                gfid_set = _gf_true;
         }
 
 #ifndef HAVE_SET_FSID
@@ -1012,8 +1024,12 @@ out:
                              (loc)?loc->inode:NULL, &stbuf, &preparent,
                              &postparent, NULL);
 
-        if ((op_ret == -1) && (!was_present)) {
-                unlink (real_path);
+        if (op_ret < 0) {
+                if (entry_created)
+                        sys_rmdir (real_path);
+
+                if (gfid_set)
+                        posix_gfid_unset (this, xdata);
         }
 
         return 0;
@@ -1300,16 +1316,16 @@ int
 posix_symlink (call_frame_t *frame, xlator_t *this,
                const char *linkname, loc_t *loc, mode_t umask, dict_t *xdata)
 {
-        int32_t               op_ret      = -1;
-        int32_t               op_errno    = 0;
-        char *                real_path   = 0;
-        char *                par_path   = 0;
-        struct iatt           stbuf       = { 0, };
-        struct posix_private *priv        = NULL;
-        gid_t                 gid         = 0;
-        char                  was_present = 1;
-        struct iatt           preparent = {0,};
-        struct iatt           postparent = {0,};
+        int32_t               op_ret          = -1;
+        int32_t               op_errno        = 0;
+        char *                real_path       = 0;
+        char *                par_path        = 0;
+        struct iatt           stbuf           = { 0, };
+        struct posix_private *priv            = NULL;
+        gid_t                 gid             = 0;
+        struct iatt           preparent       = {0,};
+        struct iatt           postparent      = {0,};
+        gf_boolean_t          entry_created   = _gf_false, gfid_set = _gf_false;
 
         DECLARE_OLD_FS_ID_VAR;
 
@@ -1323,10 +1339,6 @@ posix_symlink (call_frame_t *frame, xlator_t *this,
 
         MAKE_ENTRY_HANDLE (real_path, par_path, this, loc, &stbuf);
 
-        if ((op_ret == -1) && (errno == ENOENT)){
-                was_present = 0;
-        }
-
         SET_FS_ID (frame->root->uid, gid);
 
         gid = frame->root->gid;
@@ -1354,10 +1366,14 @@ posix_symlink (call_frame_t *frame, xlator_t *this,
                 goto out;
         }
 
+        entry_created = _gf_true;
+
         op_ret = posix_gfid_set (this, real_path, loc, xdata);
         if (op_ret) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "setting gfid on %s failed", real_path);
+        } else {
+                gfid_set = _gf_true;
         }
 
 #ifndef HAVE_SET_FSID
@@ -1412,8 +1428,12 @@ out:
                              (loc)?loc->inode:NULL, &stbuf, &preparent,
                              &postparent, NULL);
 
-        if ((op_ret == -1) && (!was_present)) {
-                unlink (real_path);
+        if (op_ret < 0) {
+                if (entry_created)
+                        sys_unlink (real_path);
+
+                if (gfid_set)
+                        posix_gfid_unset (this, xdata);
         }
 
         return 0;
@@ -1568,10 +1588,6 @@ out:
                              &preoldparent, &postoldparent,
                              &prenewparent, &postnewparent, NULL);
 
-        if ((op_ret == -1) && !was_present) {
-                unlink (real_newpath);
-        }
-
         return 0;
 }
 
@@ -1580,16 +1596,16 @@ int
 posix_link (call_frame_t *frame, xlator_t *this,
             loc_t *oldloc, loc_t *newloc, dict_t *xdata)
 {
-        int32_t               op_ret       = -1;
-        int32_t               op_errno     = 0;
-        char                 *real_oldpath = 0;
-        char                 *real_newpath = 0;
-        char                 *par_newpath = 0;
-        struct iatt           stbuf        = {0, };
-        struct posix_private *priv         = NULL;
-        char                  was_present  = 1;
-        struct iatt           preparent = {0,};
-        struct iatt           postparent = {0,};
+        int32_t               op_ret          = -1;
+        int32_t               op_errno        = 0;
+        char                 *real_oldpath    = 0;
+        char                 *real_newpath    = 0;
+        char                 *par_newpath     = 0;
+        struct iatt           stbuf           = {0, };
+        struct posix_private *priv            = NULL;
+        struct iatt           preparent       = {0,};
+        struct iatt           postparent      = {0,};
+        gf_boolean_t          entry_created   = _gf_false;
 
         DECLARE_OLD_FS_ID_VAR;
 
@@ -1605,9 +1621,6 @@ posix_link (call_frame_t *frame, xlator_t *this,
         MAKE_INODE_HANDLE (real_oldpath, this, oldloc, &stbuf);
 
         MAKE_ENTRY_HANDLE (real_newpath, par_newpath, this, newloc, &stbuf);
-        if ((op_ret == -1) && (errno == ENOENT)) {
-                was_present = 0;
-        }
 
         op_ret = posix_pstat (this, newloc->pargfid, par_newpath, &preparent);
         if (op_ret == -1) {
@@ -1637,6 +1650,8 @@ posix_link (call_frame_t *frame, xlator_t *this,
                 goto out;
         }
 
+        entry_created = _gf_true;
+
         op_ret = posix_pstat (this, NULL, real_newpath, &stbuf);
         if (op_ret == -1) {
                 op_errno = errno;
@@ -1663,8 +1678,9 @@ out:
                              (oldloc)?oldloc->inode:NULL, &stbuf, &preparent,
                              &postparent, NULL);
 
-        if ((op_ret == -1) && (!was_present)) {
-                unlink (real_newpath);
+        if (op_ret < 0) {
+                if (entry_created)
+                        sys_unlink (real_newpath);
         }
 
         return 0;
@@ -1736,20 +1752,21 @@ posix_create (call_frame_t *frame, xlator_t *this,
               loc_t *loc, int32_t flags, mode_t mode,
               mode_t umask, fd_t *fd, dict_t *xdata)
 {
-        int32_t                op_ret      = -1;
-        int32_t                op_errno    = 0;
-        int32_t                _fd         = -1;
-        int                    _flags      = 0;
-        char *                 real_path   = NULL;
-        char *                 par_path   = NULL;
-        struct iatt            stbuf       = {0, };
-        struct posix_fd *      pfd         = NULL;
-        struct posix_private * priv        = NULL;
-        char                   was_present = 1;
-
-        gid_t                  gid         = 0;
-        struct iatt            preparent = {0,};
-        struct iatt            postparent = {0,};
+        int32_t                op_ret          = -1;
+        int32_t                op_errno        = 0;
+        int32_t                _fd             = -1;
+        int                    _flags          = 0;
+        char *                 real_path       = NULL;
+        char *                 par_path        = NULL;
+        struct iatt            stbuf           = {0, };
+        struct posix_fd *      pfd             = NULL;
+        struct posix_private * priv            = NULL;
+        char                   was_present     = 1;
+
+        gid_t                  gid             = 0;
+        struct iatt            preparent       = {0,};
+        struct iatt            postparent      = {0,};
+        gf_boolean_t           entry_created   = _gf_false, gfid_set = _gf_false;
 
         DECLARE_OLD_FS_ID_VAR;
 
@@ -1807,6 +1824,11 @@ posix_create (call_frame_t *frame, xlator_t *this,
                 goto out;
         }
 
+        if ((_flags & O_CREAT) && (_flags & O_EXCL)) {
+                entry_created = _gf_true;
+        }
+
+
         if (was_present)
                 goto fill_stat;
 
@@ -1814,6 +1836,8 @@ posix_create (call_frame_t *frame, xlator_t *this,
         if (op_ret) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "setting gfid on %s failed", real_path);
+        } else {
+                gfid_set = _gf_true;
         }
 
 #ifndef HAVE_SET_FSID
@@ -1887,16 +1911,20 @@ out:
 
         if ((-1 == op_ret) && (_fd != -1)) {
                 close (_fd);
-
-                if (!was_present) {
-                        unlink (real_path);
-                }
         }
 
         STACK_UNWIND_STRICT (create, frame, op_ret, op_errno,
                              fd, (loc)?loc->inode:NULL, &stbuf, &preparent,
                              &postparent, xdata);
 
+        if (op_ret < 0) {
+                if (entry_created)
+                        sys_unlink (real_path);
+
+                if (gfid_set)
+                        posix_gfid_unset (this, xdata);
+        }
+
         return 0;
 }
 
diff --git a/xlators/storage/posix/src/posix.h b/xlators/storage/posix/src/posix.h
index 80121c08c8f..e109e58d60a 100644
--- a/xlators/storage/posix/src/posix.h
+++ b/xlators/storage/posix/src/posix.h
@@ -184,4 +184,8 @@ gf_boolean_t posix_special_xattr (char **pattern, char *key);
 void
 __posix_fd_set_odirect (fd_t *fd, struct posix_fd *pfd, int opflags,
 			off_t offset, size_t size);
+
+void
+posix_gfid_unset (xlator_t *this, dict_t *xdata);
+
 #endif /* _POSIX_H */