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 <vmallika@redhat.com>

Change-Id: Icf5fdd86bbb8eef0adeb9518e89e5b612e9e0705
BUG: 1279331
Signed-off-by: vmallika <vmallika@redhat.com>
Reviewed-on: http://review.gluster.org/12549
Tested-by: NetBSD Build System <jenkins@build.gluster.org>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Raghavendra G <rgowdapp@redhat.com>

9 files changed, 114 insertions, 77 deletions

diff --git a/libglusterfs/src/glusterfs.h b/libglusterfs/src/glusterfs.h
index 67677bff156..a3028522aaa 100644
--- a/libglusterfs/src/glusterfs.h
+++ b/libglusterfs/src/glusterfs.h
@@ -241,9 +241,9 @@
 #define DHT_SKIP_OPEN_FD_UNLINK     "dont-unlink-for-open-fd"
 #define DHT_IATT_IN_XDATA_KEY       "dht-get-iatt-in-xattr"
 
-/*CTR requires inode dentry link count from posix*/
-#define CTR_RESPONSE_LINK_COUNT_XDATA "ctr_response_link_count"
-#define CTR_REQUEST_LINK_COUNT_XDATA  "ctr_request_link_count"
+/*CTR and Marker requires inode dentry link count from posix*/
+#define GF_RESPONSE_LINK_COUNT_XDATA "gf_response_link_count"
+#define GF_REQUEST_LINK_COUNT_XDATA  "gf_request_link_count"
 
 #define CTR_ATTACH_TIER_LOOKUP    "ctr_attach_tier_lookup"
 
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;
 }