vfs_glusterfs: Fix AIO crash on smb.conf reload.
authorIra Cooper <ira@samba.org>
Wed, 18 Nov 2015 16:09:06 +0000 (11:09 -0500)
committerJeremy Allison <jra@samba.org>
Fri, 11 Dec 2015 20:56:12 +0000 (21:56 +0100)
This fixes an issue where we couldn't handle cancellation properly
so when smb.conf was reloaded we crashed.

Signed-off-by: Ira Cooper <ira@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
source3/modules/vfs_glusterfs.c

index cf8066eee101d078560f9115712c90098e734187..7f60d8034ee87f6e66d63d5f2697e069058caef5 100644 (file)
@@ -487,11 +487,28 @@ static ssize_t vfs_gluster_pread(struct vfs_handle_struct *handle,
        return glfs_pread(*(glfs_fd_t **)VFS_FETCH_FSP_EXTENSION(handle, fsp), data, n, offset, 0);
 }
 
+struct glusterfs_aio_state;
+
+struct glusterfs_aio_wrapper {
+       struct glusterfs_aio_state *state;
+};
+
 struct glusterfs_aio_state {
        ssize_t ret;
        int err;
+       struct tevent_req *req;
+       bool cancelled;
 };
 
+static int aio_wrapper_destructor(void *ptr)
+{
+       struct glusterfs_aio_wrapper *wrap = (struct glusterfs_aio_wrapper *)ptr;
+
+       wrap->state->cancelled = true;
+
+       return 0;
+}
+
 /*
  * This function is the callback that will be called on glusterfs
  * threads once the async IO submitted is complete. To notify
@@ -499,12 +516,10 @@ struct glusterfs_aio_state {
  */
 static void aio_glusterfs_done(glfs_fd_t *fd, ssize_t ret, void *data)
 {
-       struct tevent_req *req = NULL;
        struct glusterfs_aio_state *state = NULL;
        int sts = 0;
 
-       req = talloc_get_type_abort(data, struct tevent_req);
-       state = tevent_req_data(req, struct glusterfs_aio_state);
+       state = (struct glusterfs_aio_state *)data;
 
        if (ret < 0) {
                state->ret = -1;
@@ -515,10 +530,10 @@ static void aio_glusterfs_done(glfs_fd_t *fd, ssize_t ret, void *data)
        }
 
        /*
-        * Write the pointer to each req that needs to be completed
-        * by calling tevent_req_done(). tevent_req_done() cannot
-        * be called here, as it is not designed to be executed
-        * in the multithread environment, tevent_req_done() must be
+        * Write the state pointer to glusterfs_aio_state to the
+        * pipe, so we can call tevent_req_done() from the main thread,
+        * because tevent_req_done() is not designed to be executed in
+        * the multithread environment, so tevent_req_done() must be
         * executed from the smbd main thread.
         *
         * write(2) on pipes with sizes under _POSIX_PIPE_BUF
@@ -529,7 +544,7 @@ static void aio_glusterfs_done(glfs_fd_t *fd, ssize_t ret, void *data)
         * that we can trust it here.
         */
 
-       sts = sys_write(write_fd, &req, sizeof(struct tevent_req *));
+       sts = sys_write(write_fd, &state, sizeof(struct glusterfs_aio_state *));
        if (sts < 0) {
                DEBUG(0,("\nWrite to pipe failed (%s)", strerror(errno)));
        }
@@ -545,6 +560,7 @@ static void aio_tevent_fd_done(struct tevent_context *event_ctx,
                                uint16_t flags, void *data)
 {
        struct tevent_req *req = NULL;
+       struct glusterfs_aio_state *state = NULL;
        int sts = 0;
 
        /*
@@ -557,11 +573,24 @@ static void aio_tevent_fd_done(struct tevent_context *event_ctx,
         * can trust it here.
         */
 
-       sts = sys_read(read_fd, &req, sizeof(struct tevent_req *));
+       sts = sys_read(read_fd, &state, sizeof(struct glusterfs_aio_state *));
+
        if (sts < 0) {
                DEBUG(0,("\nRead from pipe failed (%s)", strerror(errno)));
        }
 
+       if (state->cancelled) {
+               return;
+       }
+
+       req = state->req;
+
+       /* if we've cancelled the op, there is no req, so just clean up. */
+       if (state->cancelled == true) {
+               TALLOC_FREE(state);
+               return;
+       }
+
        if (req) {
                tevent_req_done(req);
        }
@@ -610,28 +639,62 @@ fail:
        return false;
 }
 
-static struct tevent_req *vfs_gluster_pread_send(struct vfs_handle_struct
-                                                *handle, TALLOC_CTX *mem_ctx,
-                                                struct tevent_context *ev,
-                                                files_struct *fsp, void *data,
-                                                size_t n, off_t offset)
+static struct glusterfs_aio_state *aio_state_create(TALLOC_CTX *mem_ctx)
 {
        struct tevent_req *req = NULL;
        struct glusterfs_aio_state *state = NULL;
-       int ret = 0;
+       struct glusterfs_aio_wrapper *wrapper = NULL;
+
+       req = tevent_req_create(mem_ctx, &wrapper, struct glusterfs_aio_wrapper);
 
-       req = tevent_req_create(mem_ctx, &state, struct glusterfs_aio_state);
        if (req == NULL) {
                return NULL;
        }
 
+       state = talloc(NULL, struct glusterfs_aio_state);
+
+       if (state == NULL) {
+               TALLOC_FREE(req);
+               return NULL;
+       }
+
+       state->cancelled = false;
+       state->ret = 0;
+       state->err = 0;
+       state->req = req;
+
+       wrapper->state = state;
+
+       return state;
+}
+
+static struct tevent_req *vfs_gluster_pread_send(struct vfs_handle_struct
+                                                 *handle, TALLOC_CTX *mem_ctx,
+                                                 struct tevent_context *ev,
+                                                 files_struct *fsp,
+                                                 void *data, size_t n,
+                                                 off_t offset)
+{
+       struct glusterfs_aio_state *state = NULL;
+       struct tevent_req *req = NULL;
+       int ret = 0;
+
+       state = aio_state_create(mem_ctx);
+
+       if (state == NULL) {
+               return NULL;
+       }
+
+       req = state->req;
+
        if (!init_gluster_aio(handle)) {
                tevent_req_error(req, EIO);
                return tevent_req_post(req, ev);
        }
        ret = glfs_pread_async(*(glfs_fd_t **)VFS_FETCH_FSP_EXTENSION(handle,
                                fsp), data, n, offset, 0, aio_glusterfs_done,
-                               req);
+                               state);
+
        if (ret < 0) {
                tevent_req_error(req, -ret);
                return tevent_req_post(req, ev);
@@ -660,21 +723,25 @@ static struct tevent_req *vfs_gluster_pwrite_send(struct vfs_handle_struct
                                                  const void *data, size_t n,
                                                  off_t offset)
 {
-       struct tevent_req *req = NULL;
        struct glusterfs_aio_state *state = NULL;
+       struct tevent_req *req = NULL;
        int ret = 0;
 
-       req = tevent_req_create(mem_ctx, &state, struct glusterfs_aio_state);
-       if (req == NULL) {
+       state = aio_state_create(mem_ctx);
+
+       if (state == NULL) {
                return NULL;
        }
+
+       req = state->req;
+
        if (!init_gluster_aio(handle)) {
                tevent_req_error(req, EIO);
                return tevent_req_post(req, ev);
        }
        ret = glfs_pwrite_async(*(glfs_fd_t **)VFS_FETCH_FSP_EXTENSION(handle,
                                fsp), data, n, offset, 0, aio_glusterfs_done,
-                               req);
+                               state);
        if (ret < 0) {
                tevent_req_error(req, -ret);
                return tevent_req_post(req, ev);
@@ -685,8 +752,17 @@ static struct tevent_req *vfs_gluster_pwrite_send(struct vfs_handle_struct
 static ssize_t vfs_gluster_recv(struct tevent_req *req, int *err)
 {
        struct glusterfs_aio_state *state = NULL;
+       struct glusterfs_aio_wrapper *wrapper = NULL;
+       int ret = 0;
+
+       wrapper = tevent_req_data(req, struct glusterfs_aio_wrapper);
+
+       if (wrapper == NULL) {
+               return -1;
+       }
+
+       state = wrapper->state;
 
-       state = tevent_req_data(req, struct glusterfs_aio_state);
        if (state == NULL) {
                return -1;
        }
@@ -697,7 +773,14 @@ static ssize_t vfs_gluster_recv(struct tevent_req *req, int *err)
        if (state->ret == -1) {
                *err = state->err;
        }
-       return state->ret;
+
+       ret = state->ret;
+
+       /* Clean up the state, it is in a NULL context. */
+
+       TALLOC_FREE(state);
+
+       return ret;
 }
 
 static off_t vfs_gluster_lseek(struct vfs_handle_struct *handle,
@@ -746,10 +829,14 @@ static struct tevent_req *vfs_gluster_fsync_send(struct vfs_handle_struct
        struct glusterfs_aio_state *state = NULL;
        int ret = 0;
 
-       req = tevent_req_create(mem_ctx, &state, struct glusterfs_aio_state);
-       if (req == NULL) {
+       state = aio_state_create(mem_ctx);
+
+       if (state == NULL) {
                return NULL;
        }
+
+       req = state->req;
+
        if (!init_gluster_aio(handle)) {
                tevent_req_error(req, EIO);
                return tevent_req_post(req, ev);