From 9380b1d1fb74cd73d306a1a501e4b7b982c1a76e Mon Sep 17 00:00:00 2001 From: vmallika Date: Mon, 2 Nov 2015 15:39:46 +0530 Subject: marker: do remove xattr only for last link This is a backport of http://review.gluster.org/#/c/12033/ With unlink, rename, rmdir, contribution xattrs are removed. If the file is a last link then remove_xattr will fail with ENOENT. So it better to perform remove_xattr only if there are more links to the file > Change-Id: Ifc1e7fda4d310fd87f6f28a635c9ea78b8f3929d > BUG: 1257694 > Signed-off-by: vmallika Change-Id: Icf5fdd86bbb8eef0adeb9518e89e5b612e9e0705 BUG: 1279331 Signed-off-by: vmallika Reviewed-on: http://review.gluster.org/12549 Tested-by: NetBSD Build System Tested-by: Gluster Build System Reviewed-by: Raghavendra G --- .../changetimerecorder/src/changetimerecorder.c | 16 ++--- xlators/features/marker/src/marker-quota.c | 24 +++++-- xlators/features/marker/src/marker-quota.h | 4 +- xlators/features/marker/src/marker.c | 48 +++++++++++--- xlators/features/marker/src/marker.h | 2 +- xlators/features/trash/src/trash.c | 14 ++--- xlators/storage/posix/src/posix-helpers.c | 4 +- xlators/storage/posix/src/posix.c | 73 +++++++++------------- 8 files changed, 111 insertions(+), 74 deletions(-) (limited to 'xlators') diff --git a/xlators/features/changetimerecorder/src/changetimerecorder.c b/xlators/features/changetimerecorder/src/changetimerecorder.c index c4634c16ee3..9602fd70649 100644 --- a/xlators/features/changetimerecorder/src/changetimerecorder.c +++ b/xlators/features/changetimerecorder/src/changetimerecorder.c @@ -814,15 +814,15 @@ ctr_unlink_cbk (call_frame_t *frame, void *cookie, xlator_t *this, /* * - * Extracting CTR_RESPONSE_LINK_COUNT_XDATA from POSIX Xlator + * Extracting GF_RESPONSE_LINK_COUNT_XDATA from POSIX Xlator * * */ - ret = dict_get_uint32 (xdata , CTR_RESPONSE_LINK_COUNT_XDATA, + ret = dict_get_uint32 (xdata , GF_RESPONSE_LINK_COUNT_XDATA, &remaining_links); if (ret) { gf_msg (this->name, GF_LOG_ERROR, 0, CTR_MSG_GET_CTR_RESPONSE_LINK_COUNT_XDATA_FAILED, - "Failed to getting CTR_RESPONSE_LINK_COUNT_XDATA"); + "Failed to getting GF_RESPONSE_LINK_COUNT_XDATA"); remaining_links = -1; } @@ -851,7 +851,7 @@ ctr_unlink_cbk (call_frame_t *frame, void *cookie, xlator_t *this, out: STACK_UNWIND_STRICT (unlink, frame, op_ret, op_errno, preparent, - postparent, NULL); + postparent, xdata); return 0; } @@ -908,7 +908,7 @@ ctr_unlink (call_frame_t *frame, xlator_t *this, /* * - * Sending CTR_REQUEST_LINK_COUNT_XDATA + * Sending GF_REQUEST_LINK_COUNT_XDATA * to POSIX Xlator to send link count in unwind path * * */ @@ -920,15 +920,15 @@ ctr_unlink (call_frame_t *frame, xlator_t *this, if (!xdata) { gf_msg (this->name, GF_LOG_ERROR, 0, CTR_MSG_XDATA_NULL, "xdata is NULL :Cannot send " - "CTR_REQUEST_LINK_COUNT_XDATA to posix"); + "GF_REQUEST_LINK_COUNT_XDATA to posix"); goto out; } - ret = dict_set_int32 (xdata, CTR_REQUEST_LINK_COUNT_XDATA, 1); + ret = dict_set_int32 (xdata, GF_REQUEST_LINK_COUNT_XDATA, 1); if (ret) { gf_msg (this->name, GF_LOG_ERROR, 0, CTR_MSG_SET_CTR_RESPONSE_LINK_COUNT_XDATA_FAILED, - "Failed setting CTR_REQUEST_LINK_COUNT_XDATA"); + "Failed setting GF_REQUEST_LINK_COUNT_XDATA"); if (is_xdata_created) { dict_unref (xdata); } diff --git a/xlators/features/marker/src/marker-quota.c b/xlators/features/marker/src/marker-quota.c index bb65c8787b2..65973edcd00 100644 --- a/xlators/features/marker/src/marker-quota.c +++ b/xlators/features/marker/src/marker-quota.c @@ -760,11 +760,18 @@ out: int32_t mq_remove_contri (xlator_t *this, loc_t *loc, quota_inode_ctx_t *ctx, - inode_contribution_t *contri, quota_meta_t *delta) + inode_contribution_t *contri, quota_meta_t *delta, + uint32_t nlink) { int32_t ret = -1; char contri_key[QUOTA_KEY_MAX] = {0, }; + if (nlink == 1) { + /*File was a last link and has been deleted */ + ret = 0; + goto done; + } + GET_CONTRI_KEY (this, contri_key, contri->gfid, ret); if (ret < 0) { gf_log (this->name, GF_LOG_ERROR, "get contri_key " @@ -791,6 +798,7 @@ mq_remove_contri (xlator_t *this, loc_t *loc, quota_inode_ctx_t *ctx, } } +done: LOCK (&contri->lock); { contri->contribution += delta->size; @@ -948,7 +956,7 @@ mq_synctask_cleanup (int ret, call_frame_t *frame, void *opaque) int mq_synctask1 (xlator_t *this, synctask_fn_t task, gf_boolean_t spawn, - loc_t *loc, quota_meta_t *contri) + loc_t *loc, quota_meta_t *contri, uint32_t nlink) { int32_t ret = -1; quota_synctask_t *args = NULL; @@ -964,6 +972,7 @@ mq_synctask1 (xlator_t *this, synctask_fn_t task, gf_boolean_t spawn, args->this = this; loc_copy (&args->loc, loc); + args->ia_nlink = nlink; if (contri) { args->contri = *contri; @@ -993,7 +1002,7 @@ out: int mq_synctask (xlator_t *this, synctask_fn_t task, gf_boolean_t spawn, loc_t *loc) { - return mq_synctask1 (this, task, spawn, loc, NULL); + return mq_synctask1 (this, task, spawn, loc, NULL, -1); } int32_t @@ -1201,12 +1210,14 @@ mq_reduce_parent_size_task (void *opaque) xlator_t *this = NULL; loc_t *loc = NULL; gf_boolean_t remove_xattr = _gf_true; + uint32_t nlink = 0; GF_ASSERT (opaque); args = (quota_synctask_t *) opaque; loc = &args->loc; contri = args->contri; + nlink = args->ia_nlink; this = args->this; THIS = this; @@ -1266,7 +1277,8 @@ mq_reduce_parent_size_task (void *opaque) mq_sub_meta (&delta, NULL); if (remove_xattr) { - ret = mq_remove_contri (this, loc, ctx, contribution, &delta); + ret = mq_remove_contri (this, loc, ctx, contribution, &delta, + nlink); if (ret < 0) goto out; } @@ -1312,7 +1324,7 @@ out: int32_t mq_reduce_parent_size_txn (xlator_t *this, loc_t *origin_loc, - quota_meta_t *contri) + quota_meta_t *contri, uint32_t nlink) { int32_t ret = -1; loc_t loc = {0, }; @@ -1330,7 +1342,7 @@ mq_reduce_parent_size_txn (xlator_t *this, loc_t *origin_loc, } ret = mq_synctask1 (this, mq_reduce_parent_size_task, _gf_true, &loc, - contri); + contri, nlink); out: loc_wipe (&loc); return ret; diff --git a/xlators/features/marker/src/marker-quota.h b/xlators/features/marker/src/marker-quota.h index d655f125435..387db4e4553 100644 --- a/xlators/features/marker/src/marker-quota.h +++ b/xlators/features/marker/src/marker-quota.h @@ -120,6 +120,7 @@ struct quota_synctask { loc_t loc; quota_meta_t contri; gf_boolean_t is_static; + uint32_t ia_nlink; }; typedef struct quota_synctask quota_synctask_t; @@ -150,7 +151,8 @@ int mq_create_xattrs_txn (xlator_t *this, loc_t *loc, struct iatt *buf); int32_t -mq_reduce_parent_size_txn (xlator_t *, loc_t *, quota_meta_t *); +mq_reduce_parent_size_txn (xlator_t *, loc_t *, quota_meta_t *, + uint32_t nlink); int32_t mq_forget (xlator_t *, quota_inode_ctx_t *); diff --git a/xlators/features/marker/src/marker.c b/xlators/features/marker/src/marker.c index 41c3e1dc74d..1fd7a45d2a4 100644 --- a/xlators/features/marker/src/marker.c +++ b/xlators/features/marker/src/marker.c @@ -943,7 +943,7 @@ marker_rmdir_cbk (call_frame_t *frame, void *cookie, xlator_t *this, priv = this->private; if (priv->feature_enabled & GF_QUOTA) - mq_reduce_parent_size_txn (this, &local->loc, NULL); + mq_reduce_parent_size_txn (this, &local->loc, NULL, 1); if (priv->feature_enabled & GF_XTIME) marker_xtime_update_marks (this, local); @@ -992,6 +992,8 @@ marker_unlink_cbk (call_frame_t *frame, void *cookie, xlator_t *this, { marker_conf_t *priv = NULL; marker_local_t *local = NULL; + uint32_t nlink = -1; + int32_t ret = 0; if (op_ret == -1) { gf_log (this->name, GF_LOG_TRACE, @@ -1011,8 +1013,14 @@ marker_unlink_cbk (call_frame_t *frame, void *cookie, xlator_t *this, priv = this->private; if (priv->feature_enabled & GF_QUOTA) { - if (!local->skip_txn) - mq_reduce_parent_size_txn (this, &local->loc, NULL); + if (!local->skip_txn) { + if (xdata) + ret = dict_get_uint32 (xdata, + GF_RESPONSE_LINK_COUNT_XDATA, &nlink); + + mq_reduce_parent_size_txn (this, &local->loc, NULL, + nlink); + } } if (priv->feature_enabled & GF_XTIME) @@ -1031,6 +1039,7 @@ marker_unlink (call_frame_t *frame, xlator_t *this, loc_t *loc, int xflag, int32_t ret = 0; marker_local_t *local = NULL; marker_conf_t *priv = NULL; + gf_boolean_t dict_free = _gf_false; priv = this->private; @@ -1053,12 +1062,26 @@ marker_unlink (call_frame_t *frame, xlator_t *this, loc_t *loc, int xflag, goto unlink_wind; } + if (xdata == NULL) { + xdata = dict_new (); + dict_free = _gf_true; + } + + ret = dict_set_int32 (xdata, GF_REQUEST_LINK_COUNT_XDATA, 1); + if (ret < 0) + goto err; + unlink_wind: STACK_WIND (frame, marker_unlink_cbk, FIRST_CHILD(this), FIRST_CHILD(this)->fops->unlink, loc, xflag, xdata); - return 0; + goto out; + err: MARKER_STACK_UNWIND (unlink, frame, -1, ENOMEM, NULL, NULL, NULL); + +out: + if (dict_free) + dict_unref (xdata); return 0; } @@ -1165,13 +1188,15 @@ marker_rename_done (call_frame_t *frame, void *cookie, xlator_t *this, if (local->err != 0) goto err; - mq_reduce_parent_size_txn (this, &oplocal->loc, &oplocal->contribution); + mq_reduce_parent_size_txn (this, &oplocal->loc, &oplocal->contribution, + -1); if (local->loc.inode != NULL) { /* If destination file exits before rename, it would have * been unlinked while renaming a file */ - mq_reduce_parent_size_txn (this, &local->loc, NULL); + mq_reduce_parent_size_txn (this, &local->loc, NULL, + local->ia_nlink); } newloc.inode = inode_ref (oplocal->loc.inode); @@ -1329,6 +1354,12 @@ marker_rename_cbk (call_frame_t *frame, void *cookie, xlator_t *this, goto quota_err; } + local->ia_nlink = 0; + if (xdata) + ret = dict_get_uint32 (xdata, + GF_RESPONSE_LINK_COUNT_XDATA, + &local->ia_nlink); + local->buf = *buf; stub = fop_rename_cbk_stub (frame, default_rename_cbk, op_ret, op_errno, buf, preoldparent, @@ -1638,7 +1669,10 @@ marker_rename (call_frame_t *frame, xlator_t *this, loc_t *oldloc, lock.l_type = F_WRLCK; lock.l_whence = SEEK_SET; - local->xdata = dict_ref (xdata); + local->xdata = xdata ? dict_ref (xdata) : dict_new (); + ret = dict_set_int32 (local->xdata, GF_REQUEST_LINK_COUNT_XDATA, 1); + if (ret < 0) + goto err; local->frame = frame; local->lk_frame = create_frame (this, this->ctx->pool); diff --git a/xlators/features/marker/src/marker.h b/xlators/features/marker/src/marker.h index 19f65474453..c9bc2e369c7 100644 --- a/xlators/features/marker/src/marker.h +++ b/xlators/features/marker/src/marker.h @@ -100,7 +100,7 @@ struct marker_local{ uid_t uid; gid_t gid; int32_t ref; - int32_t ia_nlink; + uint32_t ia_nlink; struct iatt buf; gf_lock_t lock; mode_t mode; diff --git a/xlators/features/trash/src/trash.c b/xlators/features/trash/src/trash.c index ab008651e1a..c2d36bd37d7 100644 --- a/xlators/features/trash/src/trash.c +++ b/xlators/features/trash/src/trash.c @@ -840,7 +840,7 @@ trash_unlink_rename_cbk (call_frame_t *frame, void *cookie, xlator_t *this, * CTR Xlator. And trash translator only handles the unlink for * the last hardlink. * - * Check if there is a CTR_REQUEST_LINK_COUNT_XDATA from CTR Xlator + * Check if there is a GF_REQUEST_LINK_COUNT_XDATA from CTR Xlator * */ @@ -848,16 +848,16 @@ trash_unlink_rename_cbk (call_frame_t *frame, void *cookie, xlator_t *this, /* Sending back inode link count to ctr_unlink * (changetimerecoder xlator) via - * "CTR_RESPONSE_LINK_COUNT_XDATA" key using xdata. + * "GF_RESPONSE_LINK_COUNT_XDATA" key using xdata. * */ if (xdata) { ret = dict_set_uint32 (xdata, - CTR_RESPONSE_LINK_COUNT_XDATA, + GF_RESPONSE_LINK_COUNT_XDATA, 1); if (ret == -1) { gf_log (this->name, GF_LOG_WARNING, "Failed to set" - " CTR_RESPONSE_LINK_COUNT_XDATA"); + " GF_RESPONSE_LINK_COUNT_XDATA"); } } else { new_xdata = dict_new (); @@ -868,12 +868,12 @@ trash_unlink_rename_cbk (call_frame_t *frame, void *cookie, xlator_t *this, goto ctr_out; } ret = dict_set_uint32 (new_xdata, - CTR_RESPONSE_LINK_COUNT_XDATA, + GF_RESPONSE_LINK_COUNT_XDATA, 1); if (ret == -1) { gf_log (this->name, GF_LOG_WARNING, "Failed to set" - " CTR_RESPONSE_LINK_COUNT_XDATA"); + " GF_RESPONSE_LINK_COUNT_XDATA"); } ctr_out: TRASH_STACK_UNWIND (unlink, frame, 0, op_errno, @@ -1091,7 +1091,7 @@ trash_unlink (call_frame_t *frame, xlator_t *this, loc_t *loc, int xflags, } /* To know whether CTR xlator requested for the link count */ - ret = dict_get_int32 (xdata, CTR_REQUEST_LINK_COUNT_XDATA, + ret = dict_get_int32 (xdata, GF_REQUEST_LINK_COUNT_XDATA, &ctr_link_req); if (ret) { local->ctr_link_count_req = _gf_false; diff --git a/xlators/storage/posix/src/posix-helpers.c b/xlators/storage/posix/src/posix-helpers.c index ec6075a8dfd..b6f3bf6686b 100644 --- a/xlators/storage/posix/src/posix-helpers.c +++ b/xlators/storage/posix/src/posix-helpers.c @@ -479,9 +479,9 @@ _posix_xattr_get_set (dict_t *xattr_req, char *key, data_t *data, } else if (fnmatch (marker_contri_key, key, 0) == 0) { ret = _posix_get_marker_quota_contributions (filler, key); - } else if (strcmp(key, CTR_REQUEST_LINK_COUNT_XDATA) == 0) { + } else if (strcmp(key, GF_REQUEST_LINK_COUNT_XDATA) == 0) { ret = dict_set (filler->xattr, - CTR_REQUEST_LINK_COUNT_XDATA, data); + GF_REQUEST_LINK_COUNT_XDATA, data); } else { ret = _posix_xattr_get_set_from_backend (filler, key); } diff --git a/xlators/storage/posix/src/posix.c b/xlators/storage/posix/src/posix.c index 4a01e9f036f..790b61ad2da 100644 --- a/xlators/storage/posix/src/posix.c +++ b/xlators/storage/posix/src/posix.c @@ -80,6 +80,28 @@ extern char *marker_xattrs[]; #define SET_TO_OLD_FS_ID() #endif + +dict_t* +posix_dict_set_nlink (dict_t *req, dict_t *res, int32_t nlink) +{ + int ret = -1; + + if (req == NULL || !dict_get (req, GF_REQUEST_LINK_COUNT_XDATA)) + goto out; + + if (res == NULL) + res = dict_new (); + if (res == NULL) + goto out; + + ret = dict_set_uint32 (res, GF_RESPONSE_LINK_COUNT_XDATA, nlink); + if (ret == -1) + gf_msg ("posix", GF_LOG_WARNING, 0, P_MSG_SET_XDATA_FAIL, + "Failed to set GF_RESPONSE_LINK_COUNT_XDATA"); +out: + return res; +} + int posix_forget (xlator_t *this, inode_t *inode) { @@ -1485,7 +1507,7 @@ posix_unlink (call_frame_t *frame, xlator_t *this, int32_t ctr_link_req = 0; ssize_t xattr_size = -1; int32_t is_dht_linkto_file = 0; - dict_t *unwind_dict = NULL; + dict_t *unwind_dict = NULL; DECLARE_OLD_FS_ID_VAR; @@ -1620,46 +1642,8 @@ posix_unlink (call_frame_t *frame, xlator_t *this, goto out; } - /* - * - * Check if there is a CTR_REQUEST_LINK_COUNT_XDATA from CTR Xlator - * - * */ - op_ret = dict_get_int32 (xdata, CTR_REQUEST_LINK_COUNT_XDATA, - &ctr_link_req); - if (op_ret) { - /*Since no request no response*/ - op_ret = 0; - goto out; - } - - /* Sending back inode link count to ctr_unlink(changetimerecoder xlator) - * via "CTR_RESPONSE_LINK_COUNT_XDATA" key using unwind_dict. - * CTR Xlator will clear all the records if the link count has become 1 - * i.e this was the last hard link. - * */ - unwind_dict = dict_new (); - /* Even if unwind_dict fails to alloc memory we will not mark the FOP - * unsuccessful - * because this dict is only used by CTR Xlator to clear - * all records if link count == 0*/ - if (!unwind_dict) { - op_ret = 0; - goto out; - } - /* Even if unwind_dict fails to set CTR_RESPONSE_LINK_COUNT_XDATA we - * will not mark the FOP unsuccessful - * because this dict is only used by CTR Xlator to clear - * all records if link count == 0*/ - op_ret = dict_set_uint32 (unwind_dict, CTR_RESPONSE_LINK_COUNT_XDATA, - stbuf.ia_nlink); - if (op_ret == -1) { - gf_msg (this->name, GF_LOG_WARNING, 0, P_MSG_SET_XDATA_FAIL, - "Failed to set CTR_RESPONSE_LINK_COUNT_XDATA"); - } - + unwind_dict = posix_dict_set_nlink (xdata, NULL, stbuf.ia_nlink); op_ret = 0; - out: SET_TO_OLD_FS_ID (); @@ -1958,6 +1942,7 @@ posix_rename (call_frame_t *frame, xlator_t *this, int nlink = 0; char *pgfid_xattr_key = NULL; int32_t nlink_samepgfid = 0; + dict_t *unwind_dict = NULL; DECLARE_OLD_FS_ID_VAR; @@ -2137,15 +2122,19 @@ unlock: goto out; } + if (was_present) + unwind_dict = posix_dict_set_nlink (xdata, NULL, nlink); op_ret = 0; - out: SET_TO_OLD_FS_ID (); STACK_UNWIND_STRICT (rename, frame, op_ret, op_errno, &stbuf, &preoldparent, &postoldparent, - &prenewparent, &postnewparent, NULL); + &prenewparent, &postnewparent, unwind_dict); + + if (unwind_dict) + dict_unref (unwind_dict); return 0; } -- cgit