Blob Blame History Raw
From 3c0f27fd697a8c977873d44fbdf3aa63c1065645 Mon Sep 17 00:00:00 2001
From: Csaba Henk <csaba@redhat.com>
Date: Thu, 6 Dec 2018 16:13:46 +0100
Subject: [PATCH 528/529] locks: handle "clear locks" xattr in fgetxattr too

The lock clearing procedure was kicked in only in
getxattr context. We need it to work the same way
if it's triggered via fgetxattr (as is the case
with interrupt handling).

Also cleaned up the instrumentation a bit (more logs,
proper management of allocated data).

Upstream: https://review.gluster.org/21820
> updates: #465
> Change-Id: Icfca26ee181da3b8e15ca3fcf61cd5702e2730c8
> Signed-off-by: Csaba Henk <csaba@redhat.com>

Change-Id: Ia15108fd6d92ea2bdb73cea5fb04126785b19663
BUG: 1595246
Signed-off-by: Csaba Henk <csaba@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/162551
Tested-by: RHGS Build Bot <nigelb@redhat.com>
Reviewed-by: Amar Tumballi Suryanarayan <amarts@redhat.com>
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
---
 xlators/features/locks/src/clear.c |   6 +++
 xlators/features/locks/src/clear.h |   2 +
 xlators/features/locks/src/posix.c | 107 ++++++++++++++++++++++++-------------
 3 files changed, 77 insertions(+), 38 deletions(-)

diff --git a/xlators/features/locks/src/clear.c b/xlators/features/locks/src/clear.c
index 22c03b5..c3d5dd2 100644
--- a/xlators/features/locks/src/clear.c
+++ b/xlators/features/locks/src/clear.c
@@ -24,6 +24,12 @@
 #include "statedump.h"
 #include "clear.h"
 
+const char *clrlk_type_names[CLRLK_TYPE_MAX] = {
+        [CLRLK_INODE] = "inode",
+        [CLRLK_ENTRY] = "entry",
+        [CLRLK_POSIX] = "posix",
+};
+
 int
 clrlk_get_kind (char *kind)
 {
diff --git a/xlators/features/locks/src/clear.h b/xlators/features/locks/src/clear.h
index 78fc5ae..1542953 100644
--- a/xlators/features/locks/src/clear.h
+++ b/xlators/features/locks/src/clear.h
@@ -22,6 +22,8 @@ typedef enum {
         CLRLK_TYPE_MAX
 } clrlk_type;
 
+extern const char *clrlk_type_names[];
+
 typedef enum {
         CLRLK_BLOCKED = 1,
         CLRLK_GRANTED,
diff --git a/xlators/features/locks/src/posix.c b/xlators/features/locks/src/posix.c
index 2cc2837..142a5cc 100644
--- a/xlators/features/locks/src/posix.c
+++ b/xlators/features/locks/src/posix.c
@@ -1028,41 +1028,35 @@ pl_getxattr_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
 
 }
 
-int32_t
-pl_getxattr (call_frame_t *frame, xlator_t *this, loc_t *loc,
-             const char *name, dict_t *xdata)
+static int32_t
+pl_getxattr_clrlk (xlator_t *this, const char *name, inode_t *inode,
+                   dict_t **dict, int32_t *op_errno)
 {
-        int32_t                 op_errno        = EINVAL;
-        int                     op_ret          = -1;
         int32_t                 bcount          = 0;
         int32_t                 gcount          = 0;
-        char                    key[PATH_MAX]   = {0, };
+        char *key = NULL;
         char                    *lk_summary     = NULL;
         pl_inode_t              *pl_inode       = NULL;
-        dict_t                  *dict           = NULL;
         clrlk_args              args            = {0,};
         char                    *brickname      = NULL;
+        int32_t op_ret = -1;
 
-        if (!name)
-                goto usual;
-
-        if (strncmp (name, GF_XATTR_CLRLK_CMD, strlen (GF_XATTR_CLRLK_CMD)))
-                goto usual;
+        *op_errno = EINVAL;
 
         if (clrlk_parse_args (name, &args)) {
-                op_errno = EINVAL;
+                *op_errno = EINVAL;
                 goto out;
         }
 
-        dict = dict_new ();
-        if (!dict) {
-                op_errno = ENOMEM;
+        *dict = dict_new ();
+        if (!*dict) {
+                *op_errno = ENOMEM;
                 goto out;
         }
 
-        pl_inode = pl_inode_get (this, loc->inode);
+        pl_inode = pl_inode_get (this, inode);
         if (!pl_inode) {
-                op_errno = ENOMEM;
+                *op_errno = ENOMEM;
                 goto out;
         }
 
@@ -1072,23 +1066,31 @@ pl_getxattr (call_frame_t *frame, xlator_t *this, loc_t *loc,
                         op_ret = clrlk_clear_lks_in_all_domains (this, pl_inode,
                                                                  &args, &bcount,
                                                                  &gcount,
-                                                                 &op_errno);
-                        if (op_ret)
-                                goto out;
+                                                                 op_errno);
                         break;
                 case CLRLK_POSIX:
                         op_ret = clrlk_clear_posixlk (this, pl_inode, &args,
                                                       &bcount, &gcount,
-                                                      &op_errno);
-                        if (op_ret)
-                                goto out;
+                                                      op_errno);
                         break;
-                case CLRLK_TYPE_MAX:
-                        op_errno = EINVAL;
-                        goto out;
+                default:
+                        op_ret = -1;
+                        *op_errno = EINVAL;
+        }
+        if (op_ret) {
+                if (args.type >= CLRLK_TYPE_MAX) {
+                        gf_log (this->name, GF_LOG_ERROR,
+                                   "clear locks: invalid lock type %d", args.type);
+                } else {
+                        gf_log (this->name, GF_LOG_ERROR,
+                                   "clear locks of type %s failed: %s",
+                                   clrlk_type_names[args.type], strerror (*op_errno));
+                }
+
+                goto out;
         }
 
-        op_ret = fetch_pathinfo (this, loc->inode, &op_errno, &brickname);
+        op_ret = fetch_pathinfo (this, inode, op_errno, &brickname);
         if (op_ret) {
                 gf_log (this->name, GF_LOG_WARNING,
                         "Couldn't get brickname");
@@ -1105,39 +1107,62 @@ pl_getxattr (call_frame_t *frame, xlator_t *this, loc_t *loc,
         if (!gcount && !bcount) {
                 if (gf_asprintf (&lk_summary, "No locks cleared.") == -1) {
                         op_ret = -1;
-                        op_errno = ENOMEM;
+                        *op_errno = ENOMEM;
                         goto out;
                 }
         } else if (gf_asprintf (&lk_summary, "%s: %s blocked locks=%d "
                                 "granted locks=%d",
                                 (brickname == NULL)? this->name : brickname,
-                                (args.type == CLRLK_INODE)? "inode":
-                                (args.type == CLRLK_ENTRY)? "entry":
-                                (args.type == CLRLK_POSIX)? "posix": " ",
+                                clrlk_type_names[args.type],
                                 bcount, gcount) == -1) {
                 op_ret = -1;
-                op_errno = ENOMEM;
+                *op_errno = ENOMEM;
                 goto out;
         }
+        gf_log (this->name, GF_LOG_DEBUG, "%s", lk_summary);
 
-        if (snprintf(key, sizeof(key), "%s", name) >= sizeof(key)) {
+        key = gf_strdup (name);
+        if (!key) {
                 op_ret = -1;
                 goto out;
         }
-        if (dict_set_dynstr (dict, key, lk_summary)) {
+        if (dict_set_dynstr (*dict, key, lk_summary)) {
                 op_ret = -1;
-                op_errno = ENOMEM;
+                *op_errno = ENOMEM;
                 goto out;
         }
 
         op_ret = 0;
 out:
         GF_FREE(brickname);
-        STACK_UNWIND_STRICT (getxattr, frame, op_ret, op_errno, dict, xdata);
 
         GF_FREE (args.opts);
-        if (op_ret && lk_summary)
+        if (op_ret) {
                 GF_FREE (lk_summary);
+                GF_FREE (key);
+        }
+
+        return op_ret;
+}
+
+int32_t
+pl_getxattr (call_frame_t *frame, xlator_t *this, loc_t *loc, const char *name,
+                        dict_t *xdata)
+{
+        int32_t op_errno = EINVAL;
+        int32_t op_ret = -1;
+        dict_t *dict = NULL;
+
+        if (!name)
+                goto usual;
+
+        if (strncmp (name, GF_XATTR_CLRLK_CMD, strlen (GF_XATTR_CLRLK_CMD)))
+                goto usual;
+
+        op_ret = pl_getxattr_clrlk (this, name, loc->inode, &dict, &op_errno);
+
+        STACK_UNWIND_STRICT (getxattr, frame, op_ret, op_errno, dict, xdata);
+
         if (dict)
                 dict_unref (dict);
         return 0;
@@ -1415,6 +1440,12 @@ pl_fgetxattr (call_frame_t *frame, xlator_t *this, fd_t *fd,
                 }
 
                 goto unwind;
+        } else if (strncmp (name, GF_XATTR_CLRLK_CMD,
+                            strlen (GF_XATTR_CLRLK_CMD)) == 0) {
+                op_ret = pl_getxattr_clrlk (this, name, fd->inode, &dict,
+                                            &op_errno);
+
+                goto unwind;
         } else {
                 goto usual;
         }
-- 
1.8.3.1