From fe5cf303121215be6f00b4da0796fbf83922ec36 Mon Sep 17 00:00:00 2001 From: Raghavendra G Date: Fri, 5 Sep 2014 12:05:15 +0530 Subject: 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 Reviewed-on: http://review.gluster.org/8327 Reviewed-by: Pranith Kumar Karampuri Tested-by: Gluster Build System Reviewed-by: Vijay Bellur Reviewed-on: http://review.gluster.org/8675 Reviewed-by: Kaleb KEITHLEY --- xlators/storage/posix/src/posix-handle.c | 2 +- xlators/storage/posix/src/posix-helpers.c | 18 +++ xlators/storage/posix/src/posix.c | 194 +++++++++++++++++------------- xlators/storage/posix/src/posix.h | 4 + 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 */ -- cgit