From 45fbf99cb669e891a84a8228cef27973f5e774bf Mon Sep 17 00:00:00 2001 From: Raghavendra G Date: Thu, 17 Jul 2014 21:59:26 +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: 1117851 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 --- xlators/storage/posix/src/posix-handle.c | 2 +- xlators/storage/posix/src/posix-helpers.c | 18 +++++ xlators/storage/posix/src/posix.c | 108 +++++++++++++++++++----------- xlators/storage/posix/src/posix.h | 4 ++ 4 files changed, 91 insertions(+), 41 deletions(-) (limited to 'xlators/storage/posix/src') diff --git a/xlators/storage/posix/src/posix-handle.c b/xlators/storage/posix/src/posix-handle.c index b6cfbd081ea..48ca77d9e01 100644 --- a/xlators/storage/posix/src/posix-handle.c +++ b/xlators/storage/posix/src/posix-handle.c @@ -801,7 +801,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 9ba95bc4941..5f8984cc8a7 100644 --- a/xlators/storage/posix/src/posix-helpers.c +++ b/xlators/storage/posix/src/posix-helpers.c @@ -654,6 +654,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 63b65edcda5..16188a23694 100644 --- a/xlators/storage/posix/src/posix.c +++ b/xlators/storage/posix/src/posix.c @@ -1042,7 +1042,6 @@ posix_mknod (call_frame_t *frame, xlator_t *this, 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,}; @@ -1050,6 +1049,7 @@ posix_mknod (call_frame_t *frame, xlator_t *this, void * uuid_req = NULL; int32_t nlink_samepgfid = 0; char *pgfid_xattr_key = NULL; + gf_boolean_t entry_created = _gf_false, gfid_set = _gf_false; DECLARE_OLD_FS_ID_VAR; @@ -1127,10 +1127,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 @@ -1196,28 +1200,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; @@ -1245,9 +1256,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); @@ -1274,10 +1282,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 @@ -1331,8 +1343,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; @@ -1613,11 +1629,11 @@ posix_symlink (call_frame_t *frame, xlator_t *this, 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,}; char *pgfid_xattr_key = NULL; int32_t nlink_samepgfid = 0; + gf_boolean_t entry_created = _gf_false, gfid_set = _gf_false; DECLARE_OLD_FS_ID_VAR; @@ -1631,10 +1647,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; @@ -1662,10 +1674,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 @@ -1727,8 +1743,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; @@ -1932,10 +1952,6 @@ out: &preoldparent, &postoldparent, &prenewparent, &postnewparent, NULL); - if ((op_ret == -1) && !was_present) { - unlink (real_newpath); - } - return 0; } @@ -1951,11 +1967,11 @@ posix_link (call_frame_t *frame, xlator_t *this, 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 nlink_samepgfid = 0; char *pgfid_xattr_key = NULL; + gf_boolean_t entry_created = _gf_false; DECLARE_OLD_FS_ID_VAR; @@ -1971,9 +1987,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) { @@ -1994,6 +2007,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; @@ -2041,8 +2056,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; @@ -2130,6 +2146,7 @@ posix_create (call_frame_t *frame, xlator_t *this, int nlink_samepgfid = 0; char * pgfid_xattr_key = NULL; + gf_boolean_t entry_created = _gf_false, gfid_set = _gf_false; DECLARE_OLD_FS_ID_VAR; @@ -2187,6 +2204,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; @@ -2194,6 +2216,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 @@ -2274,16 +2298,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 aa4a66c3051..efd52f00b18 100644 --- a/xlators/storage/posix/src/posix.h +++ b/xlators/storage/posix/src/posix.h @@ -228,4 +228,8 @@ int posix_get_ancestry (xlator_t *this, inode_t *leaf_inode, gf_dirent_t *head, char **path, int type, int32_t *op_errno, dict_t *xdata); + +void +posix_gfid_unset (xlator_t *this, dict_t *xdata); + #endif /* _POSIX_H */ -- cgit