make posix return errors when gfid2path key is absent

Change-Id: I3a8d452d00560dac5e0b7ff0b1835d1f20a59f91
updates: bz#1570962
Signed-off-by: Raghavendra Bhat <raghavendra@redhat.com>

3 files changed, 42 insertions, 23 deletions

diff --git a/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c b/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c
index 24aa9aa5ff4..fff7a7eedf2 100644
--- a/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c
+++ b/xlators/features/bit-rot/src/stub/bit-rot-stub-helpers.c
@@ -722,35 +722,21 @@ br_stub_get_path_of_gfid (xlator_t *this, inode_t *parent, inode_t *inode,
         GF_VALIDATE_OR_GOTO (this->name, parent, out);
         GF_VALIDATE_OR_GOTO (this->name, path, out);
 
-        /* No need to validate the @inode for hard resolution. Because inode
-         * can be NULL and if it is NULL, then syncop_gfid_to_path_hard will
-         * allocate a new inode and proceed. So no need to bother about
+        /* Above, No need to validate the @inode for hard resolution. Because
+         * inode can be NULL and if it is NULL, then syncop_gfid_to_path_hard
+         * will allocate a new inode and proceed. So no need to bother about
          * @inode. Because we need it only to send a syncop_getxattr call
          * from inside syncop_gfid_to_path_hard. And getxattr fetches the
          * path from the backend.
          */
+
         ret = syncop_gfid_to_path_hard (parent->table, FIRST_CHILD (this), gfid,
                                         inode, path, _gf_true);
-
-        /*
-         * This is to handle those corrupted files which does not contain
-         * the gfid2path xattr in the backend (because they were created
-         * when the option was OFF OR it was upgraded from a version before
-         * gfid2path was brought in.
-         * Ideally posix should be returning ret < 0 i.e. error if the
-         * gfid2path xattr is not present. But for some reason it is
-         * returning success and path as "". THis is causing problems.
-         * For now handling it by adding extra checks. But the better way
-         * is to make posix return error if gfid2path xattr is absent.
-         * When that is done remove below if block and also this entire
-         * comment.
-         */
-        if (ret >= 0 && !strlen (*path)) {
+        if (ret < 0)
                 gf_msg (this->name, GF_LOG_WARNING, 0, BRS_MSG_PATH_GET_FAILED,
-                        "path for the object %s is %s. Going for in memory path",
-                        uuid_utoa_r (gfid, gfid_str), *path);
-                ret = -1;
-        }
+                        "failed to get the path xattr from disk for the "
+                        " gfid %s. Trying to get path from the memory",
+                        uuid_utoa_r (gfid, gfid_str));
 
         /*
          * Try with soft resolution of path if hard resolve fails. Because
@@ -767,10 +753,16 @@ br_stub_get_path_of_gfid (xlator_t *this, inode_t *parent, inode_t *inode,
          * found in the inode table and better not to do inode_path() on the
          * inode which has not been linked.
          */
-        if (ret < 0 && inode)
+        if (ret < 0 && inode) {
                 ret = syncop_gfid_to_path_hard (parent->table,
                                                 FIRST_CHILD (this), gfid, inode,
                                                 path, _gf_false);
+                if (ret < 0)
+                        gf_msg (this->name, GF_LOG_WARNING, 0,
+                                BRS_MSG_PATH_GET_FAILED,
+                                "failed to get the path from the memory for gfid %s",
+                                uuid_utoa_r (gfid, gfid_str));
+        }
 
 out:
         return ret;
diff --git a/xlators/storage/posix/src/posix-gfid-path.c b/xlators/storage/posix/src/posix-gfid-path.c
index 2834c2fc4fd..9873e7e5a84 100644
--- a/xlators/storage/posix/src/posix-gfid-path.c
+++ b/xlators/storage/posix/src/posix-gfid-path.c
@@ -123,6 +123,7 @@ posix_get_gfid2path (xlator_t *this, inode_t *inode, const char *real_path,
         gf_boolean_t           have_val                        = _gf_false;
         struct posix_private  *priv                            = NULL;
         char                   pargfid_str[UUID_CANONICAL_FORM_LEN + 1] = {0,};
+        gf_boolean_t           found                           = _gf_false;
 
         priv = this->private;
 
@@ -141,6 +142,7 @@ posix_get_gfid2path (xlator_t *this, inode_t *inode, const char *real_path,
                                 "value for key (%s)", GFID2PATH_VIRT_XATTR_KEY);
                         goto err;
                 }
+                found = _gf_true;
         } else {
                 have_val = _gf_false;
                 memset (value_buf, '\0', sizeof(value_buf));
@@ -202,6 +204,7 @@ posix_get_gfid2path (xlator_t *this, inode_t *inode, const char *real_path,
                                 goto ignore;
                         }
 
+                        found = _gf_true;
                         memset (value_buf, '\0', sizeof(value_buf));
                         size = sys_lgetxattr (real_path, keybuffer, value_buf,
                                               sizeof (value_buf) - 1);
@@ -231,6 +234,29 @@ ignore:
                         list_offset += strlen (keybuffer) + 1;
                 } /* while (remaining_size > 0) */
 
+                /* gfid2path xattr is absent in the list of xattrs */
+                if (!found) {
+                        ret = -1;
+                        /*
+                         * ENODATA because xattr is not present in the
+                         * list of xattrs. Thus the consumer should
+                         * face error instead of a success and a empty
+                         * string in the dict for the key.
+                         */
+                        *op_errno = ENODATA;
+                        goto err;
+                }
+
+                /*
+                 * gfid2path xattr is found in list of xattrs, but getxattr
+                 * on the 1st gfid2path xattr itself failed and the while
+                 * loop above broke. So there is nothing in the value. So
+                 * it would be better not to send "" as the value for any
+                 * key, as it is not true.
+                 */
+                if (found && !i)
+                        goto err; /* both errno and ret are set before beak */
+
                 /* Calculate memory to be allocated */
                 for (j = 0; j < i; j++) {
                         bytes += strlen(paths[j]);
diff --git a/xlators/storage/posix/src/posix-gfid-path.h b/xlators/storage/posix/src/posix-gfid-path.h
index ac3d03e14c3..59799125bbb 100644
--- a/xlators/storage/posix/src/posix-gfid-path.h
+++ b/xlators/storage/posix/src/posix-gfid-path.h
@@ -13,6 +13,7 @@
 
 #include "xlator.h"
 #include "common-utils.h"
+#include "compat-errno.h"
 
 #define MAX_GFID2PATH_LINK_SUP 500