s3/smbd: remove copy-chunk chunk merging optimisation
authorRalph Boehme <slow@samba.org>
Fri, 9 Jun 2017 14:35:39 +0000 (16:35 +0200)
committerRalph Boehme <slow@samba.org>
Mon, 3 Jul 2017 17:59:08 +0000 (19:59 +0200)
As we won't have the source fsp around with the coming token based
offload read/write based code, we can't merge chunks as that requires
checking against the source file size.

We could still merge chunks without checking, but getting the error
handling correct would require comlicated logic for the SMB2 ioctl
copy-chunk error reporting.

Signed-off-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
source3/smbd/smb2_ioctl_network_fs.c

index fa3e42951ee1010206e6fdd66b004b22dac5716d..98355e2d98835ca56b254d44de070d9f83b2b511 100644 (file)
@@ -80,7 +80,6 @@ struct fsctl_srv_copychunk_state {
        struct connection_struct *conn;
        struct srv_copychunk_copy cc_copy;
        uint32_t current_chunk;
-       uint32_t next_chunk;
        NTSTATUS status;
        off_t total_written;
        struct files_struct *src_fsp;
@@ -244,80 +243,12 @@ static struct tevent_req *fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
        return req;
 }
 
-/*
- * Work out the next IO request from the remaining chunks starting with
- * state->current_chunk. Chunks with left to right adjacent ranges can be merged
- * into a single IO request.
- */
-static uint32_t next_merged_io(struct fsctl_srv_copychunk_state *state)
-{
-       struct srv_copychunk *chunk = NULL;
-       uint32_t length;
-       off_t next_src_offset;
-       off_t next_dst_offset;
-
-       /*
-        * We're expected to process at least one chunk and return it's length,
-        * so let's do this first.
-        */
-
-       chunk = &state->cc_copy.chunks[state->current_chunk];
-       length = chunk->length;
-       state->next_chunk++;
-
-       /*
-        * Now process subsequent chunks merging chunks as long as ranges are
-        * adjacent and the source file size is not exceeded (this is needed
-        * for error reporting).
-        */
-
-       next_src_offset = chunk->source_off + chunk->length;
-       next_dst_offset = chunk->target_off + chunk->length;
-
-       while (state->next_chunk < state->cc_copy.chunk_count) {
-               chunk = &state->cc_copy.chunks[state->next_chunk];
-
-               if ((chunk->source_off != next_src_offset) ||
-                   (chunk->target_off != next_dst_offset))
-               {
-                       /* Not adjacent, stop merging */
-                       break;
-               }
-
-               next_src_offset += chunk->length;
-               next_dst_offset += chunk->length;
-
-               if (next_src_offset > state->src_fsp->fsp_name->st.st_ex_size) {
-                       /* Source filesize exceeded, stop merging */
-                       break;
-               }
-
-               /*
-                * Found a mergable chunk, merge it and continue searching.
-                * Note: this can't wrap, limits were already checked in
-                * copychunk_check_limits().
-                */
-               length += chunk->length;
-
-               state->next_chunk++;
-       }
-
-       return length;
-}
-
 static NTSTATUS fsctl_srv_copychunk_loop(struct tevent_req *req)
 {
        struct fsctl_srv_copychunk_state *state = tevent_req_data(
                req, struct fsctl_srv_copychunk_state);
        struct tevent_req *subreq = NULL;
        struct srv_copychunk *chunk = NULL;
-       uint32_t length;
-
-       if (state->next_chunk > state->cc_copy.chunk_count) {
-               DBG_ERR("Copy-chunk loop next_chunk [%d] chunk_count [%d]\n",
-                       state->next_chunk, state->cc_copy.chunk_count);
-               return NT_STATUS_INTERNAL_ERROR;
-       }
 
        if (state->cc_copy.chunk_count == 0) {
                /*
@@ -356,7 +287,6 @@ static NTSTATUS fsctl_srv_copychunk_loop(struct tevent_req *req)
        }
 
        chunk = &state->cc_copy.chunks[state->current_chunk];
-       length = next_merged_io(state);
 
        subreq = SMB_VFS_OFFLOAD_WRITE_SEND(state->dst_fsp->conn,
                                         state,
@@ -365,7 +295,7 @@ static NTSTATUS fsctl_srv_copychunk_loop(struct tevent_req *req)
                                         chunk->source_off,
                                         state->dst_fsp,
                                         chunk->target_off,
-                                        length,
+                                        chunk->length,
                                         0);
        if (tevent_req_nomem(subreq, req)) {
                return NT_STATUS_NO_MEMORY;
@@ -390,18 +320,18 @@ static void fsctl_srv_copychunk_vfs_done(struct tevent_req *subreq)
        if (!NT_STATUS_IS_OK(status)) {
                DBG_ERR("copy chunk failed [%s] chunk [%u] of [%u]\n",
                        nt_errstr(status),
-                       (unsigned int)state->next_chunk,
+                       (unsigned int)state->current_chunk,
                        (unsigned int)state->cc_copy.chunk_count);
                tevent_req_nterror(req, status);
                return;
        }
 
        DBG_DEBUG("good copy chunk [%u] of [%u]\n",
-                 (unsigned int)state->next_chunk,
+                 (unsigned int)state->current_chunk,
                  (unsigned int)state->cc_copy.chunk_count);
        state->total_written += chunk_nwritten;
 
-       state->current_chunk = state->next_chunk;
+       state->current_chunk++;
        if (state->current_chunk == state->cc_copy.chunk_count) {
                tevent_req_done(req);
                return;