summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPranith Kumar K <pkarampu@redhat.com>2014-08-01 18:30:32 +0530
committerVijay Bellur <vbellur@redhat.com>2014-09-12 02:17:42 -0700
commita82cfba2795bb15c74b4a8d28bd53ad73a7997c1 (patch)
treea453f5bd55bb15f06e164c65182e0af059a0dc2e
parent533b2ea61b6b43f2cd7b600b635c4c59ca2fa0ba (diff)
mount/fuse: Handle fd resolution failures
Backport of http://review.gluster.org/8402 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 BUG: 1136827 Change-Id: I4010b7fccd7d8caf0ce4e7629e81b605102d8fb4 Signed-off-by: Pranith Kumar K <pkarampu@redhat.com> Reviewed-on: http://review.gluster.org/8592 Tested-by: Gluster Build System <jenkins@build.gluster.com> Reviewed-by: Raghavendra G <rgowdapp@redhat.com> Reviewed-by: Vijay Bellur <vbellur@redhat.com>
-rw-r--r--tests/bugs/bug-1126048.c37
-rwxr-xr-xtests/bugs/bug-1126048.t30
-rw-r--r--xlators/mount/fuse/src/fuse-bridge.c79
-rw-r--r--xlators/mount/fuse/src/fuse-bridge.h178
-rw-r--r--xlators/mount/fuse/src/fuse-helpers.c2
-rw-r--r--xlators/mount/fuse/src/fuse-resolve.c10
6 files changed, 149 insertions, 187 deletions
diff --git a/tests/bugs/bug-1126048.c b/tests/bugs/bug-1126048.c
new file mode 100644
index 00000000000..2282bb2025e
--- /dev/null
+++ b/tests/bugs/bug-1126048.c
@@ -0,0 +1,37 @@
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+
+/*
+ * This function opens a file and to trigger migration failure, unlinks the
+ * file and performs graph switch (cmd passed in argv). If everything goes fine,
+ * fsync should fail without crashing the mount process.
+ */
+int
+main (int argc, char **argv)
+{
+ int ret = 0;
+ int fd = 0;
+ char *cmd = argv[1];
+
+ printf ("cmd is: %s\n", cmd);
+ fd = open("a.txt", O_CREAT|O_RDWR);
+ if (fd < 0)
+ printf ("open failed: %s\n", strerror(errno));
+
+ ret = unlink("a.txt");
+ if (ret < 0)
+ printf ("unlink failed: %s\n", strerror(errno));
+ if (write (fd, "abc", 3) < 0)
+ printf ("Not able to print %s\n", strerror (errno));
+ system(cmd);
+ sleep(1); //No way to confirm graph switch so sleep 1
+ ret = fsync(fd);
+ if (ret < 0)
+ printf ("Not able to fsync %s\n", strerror (errno));
+ return 0;
+}
diff --git a/tests/bugs/bug-1126048.t b/tests/bugs/bug-1126048.t
new file mode 100755
index 00000000000..53c8ced4f96
--- /dev/null
+++ b/tests/bugs/bug-1126048.t
@@ -0,0 +1,30 @@
+#!/bin/bash
+
+. $(dirname $0)/../include.rc
+
+cleanup;
+
+function grep_for_ebadf {
+ $M0/bug-1126048 "gluster --mode=script --wignore volume add-brick $V0 $H0:$B0/brick2" | grep -i "Bad file descriptor"
+}
+TEST glusterd
+TEST pidof glusterd
+TEST $CLI volume info;
+
+TEST $CLI volume create $V0 $H0:$B0/brick1;
+EXPECT 'Created' volinfo_field $V0 'Status';
+
+TEST $CLI volume start $V0;
+EXPECT 'Started' volinfo_field $V0 'Status';
+
+TEST glusterfs -s $H0 --volfile-id=$V0 $M0 --direct-io-mode=yes
+
+build_tester $(dirname $0)/bug-1126048.c
+
+TEST cp $(dirname $0)/bug-1126048 $M0
+cd $M0
+TEST grep_for_ebadf
+TEST ls -l $M0
+cd -
+TEST rm -f $(dirname $0)/bug-1126048
+cleanup;
diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c
index 81697593b7e..dd51f5a6608 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)
{