mount/fuse: Handle fd resolution failures

Problem:
Even when the fd resolution failed, the fop is continuing on the
new graph which may not have valid inode. This lead to NULL layout
subvols in dht which lead to crash in fsync after graph migration.

Fix:
- Remove resolution error handling in FUSE_FOP as it was only added
  to handle fd migration failures.
- check in fuse_resolve_done for fd resolution failures and fail the
  fop right away.
- loc resolution failures are already handled in the corresponding
  fops.
- Return errno from state->resolve.op_errno in resume functions.
- Send error to fuse on frame allocation failures.
- Removed unused variable state->resolved
- Removed unused macro FUSE_FOP_COOKIE

Change-Id: I479d6e1ff2ca626ad8c8fcb6f293022149474992
BUG: 1126048
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Reviewed-on: http://review.gluster.org/8402
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>

4 files changed, 82 insertions, 187 deletions

diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c
index 0aadb649fbd..a50f0c8387f 100644
--- a/xlators/mount/fuse/src/fuse-bridge.c
+++ b/xlators/mount/fuse/src/fuse-bridge.c
@@ -550,12 +550,32 @@ fuse_lookup_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
 }
 
 void
+fuse_fop_resume (fuse_state_t *state)
+{
+        fuse_resume_fn_t fn = NULL;
+
+        /*
+         * Fail fd resolution failures right away.
+         */
+        if (state->resolve.fd && state->resolve.op_ret < 0) {
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
+                free_fuse_state (state);
+                return;
+        }
+
+        fn = state->resume_fn;
+        fn (state);
+}
+
+void
 fuse_lookup_resume (fuse_state_t *state)
 {
         if (!state->loc.parent && !state->loc.inode) {
                 gf_log ("fuse", GF_LOG_ERROR, "failed to resolve path %s",
                         state->loc.path);
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -786,7 +806,8 @@ fuse_getattr_resume (fuse_state_t *state)
                         "%"PRIu64": GETATTR %"PRIu64" (%s) resolution failed",
                         state->finh->unique, state->finh->nodeid,
                         uuid_utoa (state->resolve.gfid));
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -1109,7 +1130,8 @@ fuse_setattr_resume (fuse_state_t *state)
                         "%"PRIu64": SETATTR %"PRIu64" (%s) resolution failed",
                         state->finh->unique, state->finh->nodeid,
                         uuid_utoa (state->resolve.gfid));
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -1316,7 +1338,8 @@ fuse_access_resume (fuse_state_t *state)
                         "%"PRIu64": ACCESS %"PRIu64" (%s) resolution failed",
                         state->finh->unique, state->finh->nodeid,
                         uuid_utoa (state->resolve.gfid));
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -1390,7 +1413,8 @@ fuse_readlink_resume (fuse_state_t *state)
                 gf_log ("glusterfs-fuse", GF_LOG_ERROR,
                         "READLINK %"PRIu64" (%s) resolution failed",
                         state->finh->unique, uuid_utoa (state->resolve.gfid));
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -1425,7 +1449,8 @@ fuse_mknod_resume (fuse_state_t *state)
                         "MKNOD %"PRIu64"/%s (%s/%s) resolution failed",
                         state->finh->nodeid, state->resolve.bname,
                         uuid_utoa (state->resolve.gfid), state->resolve.bname);
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -1495,7 +1520,8 @@ fuse_mkdir_resume (fuse_state_t *state)
                         "MKDIR %"PRIu64" (%s/%s) resolution failed",
                         state->finh->nodeid, uuid_utoa (state->resolve.gfid),
                         state->resolve.bname);
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -1559,7 +1585,8 @@ fuse_unlink_resume (fuse_state_t *state)
                         "UNLINK %"PRIu64" (%s/%s) resolution failed",
                         state->finh->nodeid, uuid_utoa (state->resolve.gfid),
                         state->resolve.bname);
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -1595,7 +1622,8 @@ fuse_rmdir_resume (fuse_state_t *state)
                         "RMDIR %"PRIu64" (%s/%s) resolution failed",
                         state->finh->nodeid, uuid_utoa (state->resolve.gfid),
                         state->resolve.bname);
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -1631,7 +1659,8 @@ fuse_symlink_resume (fuse_state_t *state)
                         "SYMLINK %"PRIu64" (%s/%s) -> %s resolution failed",
                         state->finh->nodeid, uuid_utoa (state->resolve.gfid),
                         state->resolve.bname, state->name);
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -1747,7 +1776,8 @@ fuse_rename_resume (fuse_state_t *state)
                         uuid_utoa_r (state->resolve2.gfid, loc2_uuid),
                         state->resolve2.bname);
 
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -1804,7 +1834,8 @@ fuse_link_resume (fuse_state_t *state)
                 gf_log ("glusterfs-fuse", GF_LOG_WARNING,
                         "fuse_loc_fill() failed %"PRIu64": LINK %s %s",
                         state->finh->unique, state->loc2.path, state->loc.path);
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -1956,7 +1987,8 @@ fuse_create_resume (fuse_state_t *state)
                         "%"PRIu64" CREATE %s/%s resolution failed",
                         state->finh->unique, uuid_utoa (state->resolve.gfid),
                         state->resolve.bname);
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -2062,7 +2094,8 @@ fuse_open_resume (fuse_state_t *state)
                         "%"PRIu64": OPEN %s resolution failed",
                         state->finh->unique, uuid_utoa (state->resolve.gfid));
 
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -2460,7 +2493,8 @@ fuse_opendir_resume (fuse_state_t *state)
                 gf_log ("glusterfs-fuse", GF_LOG_WARNING,
                         "%"PRIu64": OPENDIR (%s) resolution failed",
                         state->finh->unique, uuid_utoa (state->resolve.gfid));
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -2992,7 +3026,8 @@ fuse_statfs_resume (fuse_state_t *state)
                         "%"PRIu64": STATFS (%s) resolution fail",
                         state->finh->unique, uuid_utoa (state->resolve.gfid));
 
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -3027,7 +3062,8 @@ fuse_setxattr_resume (fuse_state_t *state)
                         "resolution failed",
                         state->finh->unique, uuid_utoa (state->resolve.gfid),
                         state->finh->nodeid, state->name);
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -3326,7 +3362,8 @@ fuse_getxattr_resume (fuse_state_t *state)
                         uuid_utoa (state->resolve.gfid),
                         state->finh->nodeid, state->name);
 
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -3471,7 +3508,8 @@ fuse_listxattr_resume (fuse_state_t *state)
                         "resolution failed", state->finh->unique,
                         uuid_utoa (state->resolve.gfid), state->finh->nodeid);
 
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
@@ -3526,7 +3564,8 @@ fuse_removexattr_resume (fuse_state_t *state)
                         state->finh->unique, uuid_utoa (state->resolve.gfid),
                         state->finh->nodeid, state->name);
 
-                send_fuse_err (state->this, state->finh, ENOENT);
+                send_fuse_err (state->this, state->finh,
+                               state->resolve.op_errno);
                 free_fuse_state (state);
                 return;
         }
diff --git a/xlators/mount/fuse/src/fuse-bridge.h b/xlators/mount/fuse/src/fuse-bridge.h
index c08d55daf4b..d0d4120026b 100644
--- a/xlators/mount/fuse/src/fuse-bridge.h
+++ b/xlators/mount/fuse/src/fuse-bridge.h
@@ -151,10 +151,17 @@ typedef struct fuse_graph_switch_args fuse_graph_switch_args_t;
 
 #define FUSE_FOP(state, ret, op_num, fop, args ...)                     \
         do {                                                            \
-                call_frame_t   *frame   = NULL;                         \
-                xlator_t       *xl      = NULL;                         \
-                int32_t         op_ret  = 0, op_errno = 0;              \
-                fuse_resolve_t *resolve = NULL;                         \
+                xlator_t     *xl     = NULL;                            \
+                call_frame_t *frame  = NULL;                            \
+                                                                        \
+                xl = state->active_subvol;                              \
+                if (!xl) {                                              \
+                        gf_log_callingfn (state->this->name, GF_LOG_ERROR, \
+                                          "No active subvolume");       \
+                        send_fuse_err (state->this, state->finh, ENOENT); \
+                        free_fuse_state (state);                        \
+                        break;                                          \
+                }                                                       \
                                                                         \
                 frame = get_call_frame_for_req (state);                 \
                 if (!frame) {                                           \
@@ -164,13 +171,7 @@ typedef struct fuse_graph_switch_args fuse_graph_switch_args_t;
                           * better than trying to go on with a NULL     \
                           * frame ...                                   \
                           */                                            \
-                        gf_log_callingfn ("glusterfs-fuse",             \
-                                GF_LOG_ERROR,                           \
-                                "FUSE message"                          \
-                                " unique %"PRIu64" opcode %d:"          \
-                                " frame allocation failed",             \
-                                state->finh->unique,                    \
-                                state->finh->opcode);                   \
+                        send_fuse_err (state->this, state->finh, ENOMEM); \
                         free_fuse_state (state);                        \
                         /* ideally, need to 'return', but let the */    \
                         /* calling function take care of it */          \
@@ -181,150 +182,15 @@ typedef struct fuse_graph_switch_args fuse_graph_switch_args_t;
                 frame->root->op    = op_num;                            \
                 frame->op          = op_num;                            \
                                                                         \
-                if ( state->resolve_now ) {                             \
-                        resolve = state->resolve_now;                   \
-                } else {                                                \
-                        resolve = &(state->resolve);                    \
-                }                                                       \
-                                                                        \
-                xl = state->active_subvol;				\
-                if (!xl) {                                              \
-                        gf_log_callingfn ("glusterfs-fuse", GF_LOG_ERROR, \
-                                          "xl is NULL");                \
-                        op_errno = ENOENT;                              \
-                        op_ret = -1;                                    \
-                } else if (resolve->op_ret < 0) {                       \
-                        op_errno = resolve->op_errno;                   \
-                        op_ret = -1;                                    \
-                        if (op_num == GF_FOP_LOOKUP) {                  \
-                                gf_log ("glusterfs-fuse",               \
-                                        (op_errno == ENOENT ? GF_LOG_TRACE \
-                                         : GF_LOG_WARNING),             \
-                                        "%"PRIu64": %s() %s => -1 (%s)", \
-                                        frame->root->unique,            \
-                                        gf_fop_list[frame->root->op],   \
-                                        resolve->resolve_loc.path,      \
-                                        strerror (op_errno));           \
-                        } else {                                        \
-                                gf_log ("glusterfs-fuse",               \
-                                        GF_LOG_WARNING,                 \
-                                        "%"PRIu64": %s() inode "        \
-                                        "migration of %s failed (%s)",  \
-                                        frame->root->unique,            \
-                                        gf_fop_list[frame->root->op],   \
-                                        resolve->resolve_loc.path,      \
-                                        strerror (op_errno));           \
-                        }                                               \
-                } else if (state->resolve2.op_ret < 0) {                \
-                        op_errno = state->resolve2.op_errno;            \
-                        op_ret = -1;                                    \
-                        gf_log ("glusterfs-fuse",                       \
-                                GF_LOG_WARNING,                         \
-                                "%"PRIu64": %s() inode "                \
-                                "migration of %s failed (%s)",          \
-                                frame->root->unique,                    \
-                                gf_fop_list[frame->root->op],           \
-                                state->resolve2.resolve_loc.path,       \
-                                strerror (op_errno));                   \
-                }                                                       \
-                                                                        \
-                if (op_ret < 0) {                                       \
-                        send_fuse_err (state->this, state->finh, op_errno); \
-                        free_fuse_state (state);                        \
-                        STACK_DESTROY (frame->root);                    \
-                } else {                                                \
-                        if (state->this->history)                       \
-                                gf_log_eh ("%"PRIu64", %s, path: (%s), gfid: " \
-                                           "(%s)", frame->root->unique, \
-                                           gf_fop_list[frame->root->op], \
-                                           state->loc.path,             \
-                                           (state->fd == NULL)?         \
-                                           uuid_utoa (state->loc.gfid): \
-                                           uuid_utoa (state->fd->inode->gfid));\
-                        STACK_WIND (frame, ret, xl, xl->fops->fop, args); \
-                }                                                       \
-                                                                        \
-        } while (0)
-
-
-#define FUSE_FOP_COOKIE(state, xl, ret, cky, op_num, fop, args ...)     \
-        do {                                                            \
-                call_frame_t *frame  = NULL;                            \
-                xlator_t     *xl     = NULL;                            \
-                int32_t       op_ret = 0, op_errno = 0;                 \
-                                                                        \
-                frame = get_call_frame_for_req (state);                 \
-                if (!frame) {                                           \
-                        gf_log ("glusterfs-fuse",                       \
-                                GF_LOG_ERROR,                           \
-                                "FUSE message"                          \
-                                " unique %"PRIu64" opcode %d:"          \
-                                " frame allocation failed",             \
-                                state->finh->unique,                    \
-                                state->finh->opcode);                   \
-                        free_fuse_state (state);                        \
-                        return 0;                                       \
-                }                                                       \
-                                                                        \
-                frame->root->state = state;                             \
-                frame->root->op    = op_num;                            \
-		frame->op          = op_num;				\
-                                                                        \
-                xl = state->active_subvol;				\
-                if (!xl) {                                              \
-                        gf_log_callingfn ("glusterfs-fuse", GF_LOG_ERROR, \
-                                          "xl is NULL");                \
-                        op_errno = ENOENT;                              \
-                        op_ret = -1;                                    \
-                } else if (state->resolve.op_ret < 0) {                 \
-                        op_errno = state->resolve.op_errno;             \
-                        op_ret = -1;                                    \
-                        if (op_num == GF_FOP_LOOKUP) {                  \
-                                gf_log ("glusterfs-fuse",               \
-                                        (op_errno == ENOENT ? GF_LOG_TRACE \
-                                         : GF_LOG_WARNING),             \
-                                        "%"PRIu64": %s() %s => -1 (%s)", \
-                                        frame->root->unique,            \
-                                        gf_fop_list[frame->root->op],   \
-                                        state->resolve.resolve_loc.path, \
-                                        strerror (op_errno));           \
-                        } else {                                        \
-                                gf_log ("glusterfs-fuse",               \
-                                        GF_LOG_WARNING,                 \
-                                        "%"PRIu64": %s() inode "        \
-                                        "migration of %s failed (%s)",  \
-                                        frame->root->unique,            \
-                                        gf_fop_list[frame->root->op],   \
-                                        state->resolve.resolve_loc.path, \
-                                        strerror (op_errno));           \
-                        }                                               \
-                } else if (state->resolve2.op_ret < 0) {                \
-                        op_errno = state->resolve2.op_errno;            \
-                        op_ret = -1;                                    \
-                        gf_log ("glusterfs-fuse",                       \
-                                GF_LOG_WARNING,                         \
-                                "%"PRIu64": %s() inode "                \
-                                "migration of %s failed (%s)",          \
-                                frame->root->unique,                    \
-                                gf_fop_list[frame->root->op],           \
-                                state->resolve2.resolve_loc.path,       \
-                                strerror (op_errno));                   \
-                }                                                       \
-                                                                        \
-                if (op_ret < 0) {                                       \
-                        send_fuse_err (state->this, state->finh, op_errno); \
-                        free_fuse_state (state);                        \
-                        STACK_DESTROY (frame->root);                    \
-                } else {                                                \
-                        if (xl->history)                                \
-                                gf_log_eh ("%"PRIu64", %s, path: (%s), gfid: " \
-                                           "(%s)", frame->root->unique, \
-                                           gf_fop_list[frame->root->op], \
-                                           state->loc.path,             \
-                                           uuid_utoa (state->loc.gfid)); \
-                        STACK_WIND_COOKIE (frame, ret, cky, xl, xl->fops->fop, \
-                                           args);                       \
-                }                                                       \
+                if (state->this->history)                               \
+                        gf_log_eh ("%"PRIu64", %s, path: (%s), gfid: "  \
+                                   "(%s)", frame->root->unique,         \
+                                   gf_fop_list[frame->root->op],        \
+                                   state->loc.path,                     \
+                                   (state->fd == NULL)?                 \
+                                   uuid_utoa (state->loc.gfid):         \
+                                   uuid_utoa (state->fd->inode->gfid)); \
+                STACK_WIND (frame, ret, xl, xl->fops->fop, args);       \
         } while (0)
 
 #define GF_SELECT_LOG_LEVEL(_errno)                     \
@@ -452,7 +318,6 @@ typedef struct {
 	inode_t               *hint;
         u_char                 pargfid[16];
 	inode_t               *parhint;
-	char                  *resolved;
         int                    op_ret;
         int                    op_errno;
         loc_t                  resolve_loc;
@@ -539,5 +404,6 @@ int fuse_resolve_entry_init (fuse_state_t *state, fuse_resolve_t *resolve,
 int fuse_resolve_fd_init (fuse_state_t *state, fuse_resolve_t *resolve,
 			  fd_t *fd);
 int fuse_ignore_xattr_set (fuse_private_t *priv, char *key);
+void fuse_fop_resume (fuse_state_t *state);
 int dump_history_fuse (circular_buffer_t *cb, void *data);
 #endif /* _GF_FUSE_BRIDGE_H_ */
diff --git a/xlators/mount/fuse/src/fuse-helpers.c b/xlators/mount/fuse/src/fuse-helpers.c
index ef9bcd4e43e..d30f2e8a282 100644
--- a/xlators/mount/fuse/src/fuse-helpers.c
+++ b/xlators/mount/fuse/src/fuse-helpers.c
@@ -30,8 +30,6 @@ fuse_resolve_wipe (fuse_resolve_t *resolve)
 
         GF_FREE ((void *)resolve->bname);
 
-        GF_FREE ((void *)resolve->resolved);
-
         if (resolve->fd)
                 fd_unref (resolve->fd);
 
diff --git a/xlators/mount/fuse/src/fuse-resolve.c b/xlators/mount/fuse/src/fuse-resolve.c
index 76b1d9a72cc..25abe162e6c 100644
--- a/xlators/mount/fuse/src/fuse-resolve.c
+++ b/xlators/mount/fuse/src/fuse-resolve.c
@@ -654,20 +654,13 @@ fuse_resolve (fuse_state_t *state)
         return 0;
 }
 
-
 static int
 fuse_resolve_done (fuse_state_t *state)
 {
-        fuse_resume_fn_t fn = NULL;
-
-        fn = state->resume_fn;
-
-	fn (state);
-
+        fuse_fop_resume (state);
         return 0;
 }
 
-
 /*
  * This function is called multiple times, once per resolving one location/fd.
  * state->resolve_now is used to decide which location/fd is to be resolved now
@@ -712,7 +705,6 @@ fuse_resolve_continue (fuse_state_t *state)
         return 0;
 }
 
-
 int
 fuse_resolve_and_resume (fuse_state_t *state, fuse_resume_fn_t fn)
 {