[PATCH] Make server-side copy (copy-chunk) async
Ralph Böhme
slow at samba.org
Wed Mar 22 18:14:05 UTC 2017
Hi!
Attached is a patchset I've been working on that makes our server-side copy
asynchronous.
At the SMB layer the chunks are processed sequentially in order, but left to
right merged if copy ranges are adjecent. In the backend (vfs_default) I'm
essentially just using pread|pwrite_send|recv instead of the sync versions.
I'm also adding a small new feature to tevent: cancel forwarding. It allows the
implementation of a tevent request using subrequest to request forwarding of
cancels to be done automagically by the tevent library. This is useful for
forwarding cancels through a chain of subrequests.
Please review & comment but don't push yet.
Cheerio!
-slow
-------------- next part --------------
From 1674efe506c59c8fbff1b43e8b882c55f79d399c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 15 Mar 2017 16:16:49 +0100
Subject: [PATCH 01/16] lib/tevent: add struct private_cancel and move the
cancel function into it
The next commit will add more members to the new struct.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
lib/tevent/tevent_internal.h | 20 +++++++++++---------
lib/tevent/tevent_req.c | 8 ++++----
2 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/lib/tevent/tevent_internal.h b/lib/tevent/tevent_internal.h
index a5f1ebde..d60d354 100644
--- a/lib/tevent/tevent_internal.h
+++ b/lib/tevent/tevent_internal.h
@@ -65,15 +65,6 @@ struct tevent_req {
tevent_req_print_fn private_print;
/**
- * @brief A function to cancel the request
- *
- * The implementation might want to set a function
- * that is called when the tevent_req_cancel() function
- * was called.
- */
- tevent_req_cancel_fn private_cancel;
-
- /**
* @brief A function to cleanup the request
*
* The implementation might want to set a function
@@ -85,6 +76,17 @@ struct tevent_req {
enum tevent_req_state state;
} private_cleanup;
+ struct {
+ /**
+ * @brief A function to cancel the request
+ *
+ * The implementation might want to set a function
+ * that is called when the tevent_req_cancel() function
+ * was called.
+ */
+ tevent_req_cancel_fn fn;
+ } private_cancel;
+
/**
* @brief Internal state of the request
*
diff --git a/lib/tevent/tevent_req.c b/lib/tevent/tevent_req.c
index e309c3d..c066b7c 100644
--- a/lib/tevent/tevent_req.c
+++ b/lib/tevent/tevent_req.c
@@ -243,7 +243,7 @@ void tevent_req_received(struct tevent_req *req)
talloc_set_destructor(req, NULL);
req->private_print = NULL;
- req->private_cancel = NULL;
+ req->private_cancel.fn = NULL;
TALLOC_FREE(req->internal.trigger);
TALLOC_FREE(req->internal.timer);
@@ -341,16 +341,16 @@ void tevent_req_set_print_fn(struct tevent_req *req, tevent_req_print_fn fn)
void tevent_req_set_cancel_fn(struct tevent_req *req, tevent_req_cancel_fn fn)
{
- req->private_cancel = fn;
+ req->private_cancel.fn = fn;
}
bool _tevent_req_cancel(struct tevent_req *req, const char *location)
{
- if (req->private_cancel == NULL) {
+ if (req->private_cancel.fn == NULL) {
return false;
}
- return req->private_cancel(req);
+ return req->private_cancel.fn(req);
}
void tevent_req_set_cleanup_fn(struct tevent_req *req, tevent_req_cleanup_fn fn)
--
2.9.3
From 8e8cf4b77215ed05cf67861efae5bd50553ccdb8 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 13 Mar 2017 07:40:24 +0100
Subject: [PATCH 02/16] tevent: version 0.9.32: add
tevent_req_set_cancel_forwarding()
This function requests automatic fowarding of a cancel to a tevent
subrequest when tevent_req_cancel() is called on a request.
In an async computation with nested subrequests it is possible that a
subrequest is cancellable (as it called tevent_req_set_cancel_fn()), but
intermediate subrequests are not.
To avoid the necessity for all intermediate requests to install cancel
handlers with tevent_req_set_cancel_fn() and manually forward the request
from the cancel handler, a request that does a subrequest can call
tevent_req_set_cancel_forwarding(req, subreq) and then tevent takes care of
cancelling the subrequest when tevent_req_cancel() is called on the request.
Note that for a single request you can only either set a cancel function
with tevent_req_set_cancel_fn() or request cancel forwarding with
tevent_req_set_cancel_forwarding(), not both.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
lib/tevent/ABI/tevent-0.9.32.sigs | 100 ++++++++++++++++++++++++++++++++++++++
lib/tevent/tevent.h | 27 ++++++++++
lib/tevent/tevent_internal.h | 14 ++++++
lib/tevent/tevent_req.c | 44 +++++++++++++++--
lib/tevent/wscript | 2 +-
5 files changed, 183 insertions(+), 4 deletions(-)
create mode 100644 lib/tevent/ABI/tevent-0.9.32.sigs
diff --git a/lib/tevent/ABI/tevent-0.9.32.sigs b/lib/tevent/ABI/tevent-0.9.32.sigs
new file mode 100644
index 0000000..5ed37a8
--- /dev/null
+++ b/lib/tevent/ABI/tevent-0.9.32.sigs
@@ -0,0 +1,100 @@
+_tevent_add_fd: struct tevent_fd *(struct tevent_context *, TALLOC_CTX *, int, uint16_t, tevent_fd_handler_t, void *, const char *, const char *)
+_tevent_add_signal: struct tevent_signal *(struct tevent_context *, TALLOC_CTX *, int, int, tevent_signal_handler_t, void *, const char *, const char *)
+_tevent_add_timer: struct tevent_timer *(struct tevent_context *, TALLOC_CTX *, struct timeval, tevent_timer_handler_t, void *, const char *, const char *)
+_tevent_create_immediate: struct tevent_immediate *(TALLOC_CTX *, const char *)
+_tevent_loop_once: int (struct tevent_context *, const char *)
+_tevent_loop_until: int (struct tevent_context *, bool (*)(void *), void *, const char *)
+_tevent_loop_wait: int (struct tevent_context *, const char *)
+_tevent_queue_create: struct tevent_queue *(TALLOC_CTX *, const char *, const char *)
+_tevent_req_callback_data: void *(struct tevent_req *)
+_tevent_req_cancel: bool (struct tevent_req *, const char *)
+_tevent_req_create: struct tevent_req *(TALLOC_CTX *, void *, size_t, const char *, const char *)
+_tevent_req_data: void *(struct tevent_req *)
+_tevent_req_done: void (struct tevent_req *, const char *)
+_tevent_req_error: bool (struct tevent_req *, uint64_t, const char *)
+_tevent_req_nomem: bool (const void *, struct tevent_req *, const char *)
+_tevent_req_notify_callback: void (struct tevent_req *, const char *)
+_tevent_req_oom: void (struct tevent_req *, const char *)
+_tevent_schedule_immediate: void (struct tevent_immediate *, struct tevent_context *, tevent_immediate_handler_t, void *, const char *, const char *)
+_tevent_threaded_schedule_immediate: void (struct tevent_threaded_context *, struct tevent_immediate *, tevent_immediate_handler_t, void *, const char *, const char *)
+tevent_backend_list: const char **(TALLOC_CTX *)
+tevent_cleanup_pending_signal_handlers: void (struct tevent_signal *)
+tevent_common_add_fd: struct tevent_fd *(struct tevent_context *, TALLOC_CTX *, int, uint16_t, tevent_fd_handler_t, void *, const char *, const char *)
+tevent_common_add_signal: struct tevent_signal *(struct tevent_context *, TALLOC_CTX *, int, int, tevent_signal_handler_t, void *, const char *, const char *)
+tevent_common_add_timer: struct tevent_timer *(struct tevent_context *, TALLOC_CTX *, struct timeval, tevent_timer_handler_t, void *, const char *, const char *)
+tevent_common_add_timer_v2: struct tevent_timer *(struct tevent_context *, TALLOC_CTX *, struct timeval, tevent_timer_handler_t, void *, const char *, const char *)
+tevent_common_check_signal: int (struct tevent_context *)
+tevent_common_context_destructor: int (struct tevent_context *)
+tevent_common_fd_destructor: int (struct tevent_fd *)
+tevent_common_fd_get_flags: uint16_t (struct tevent_fd *)
+tevent_common_fd_set_close_fn: void (struct tevent_fd *, tevent_fd_close_fn_t)
+tevent_common_fd_set_flags: void (struct tevent_fd *, uint16_t)
+tevent_common_have_events: bool (struct tevent_context *)
+tevent_common_loop_immediate: bool (struct tevent_context *)
+tevent_common_loop_timer_delay: struct timeval (struct tevent_context *)
+tevent_common_loop_wait: int (struct tevent_context *, const char *)
+tevent_common_schedule_immediate: void (struct tevent_immediate *, struct tevent_context *, tevent_immediate_handler_t, void *, const char *, const char *)
+tevent_common_threaded_activate_immediate: void (struct tevent_context *)
+tevent_common_wakeup: int (struct tevent_context *)
+tevent_common_wakeup_fd: int (int)
+tevent_common_wakeup_init: int (struct tevent_context *)
+tevent_context_init: struct tevent_context *(TALLOC_CTX *)
+tevent_context_init_byname: struct tevent_context *(TALLOC_CTX *, const char *)
+tevent_context_init_ops: struct tevent_context *(TALLOC_CTX *, const struct tevent_ops *, void *)
+tevent_debug: void (struct tevent_context *, enum tevent_debug_level, const char *, ...)
+tevent_fd_get_flags: uint16_t (struct tevent_fd *)
+tevent_fd_set_auto_close: void (struct tevent_fd *)
+tevent_fd_set_close_fn: void (struct tevent_fd *, tevent_fd_close_fn_t)
+tevent_fd_set_flags: void (struct tevent_fd *, uint16_t)
+tevent_get_trace_callback: void (struct tevent_context *, tevent_trace_callback_t *, void *)
+tevent_loop_allow_nesting: void (struct tevent_context *)
+tevent_loop_set_nesting_hook: void (struct tevent_context *, tevent_nesting_hook, void *)
+tevent_num_signals: size_t (void)
+tevent_queue_add: bool (struct tevent_queue *, struct tevent_context *, struct tevent_req *, tevent_queue_trigger_fn_t, void *)
+tevent_queue_add_entry: struct tevent_queue_entry *(struct tevent_queue *, struct tevent_context *, struct tevent_req *, tevent_queue_trigger_fn_t, void *)
+tevent_queue_add_optimize_empty: struct tevent_queue_entry *(struct tevent_queue *, struct tevent_context *, struct tevent_req *, tevent_queue_trigger_fn_t, void *)
+tevent_queue_length: size_t (struct tevent_queue *)
+tevent_queue_running: bool (struct tevent_queue *)
+tevent_queue_start: void (struct tevent_queue *)
+tevent_queue_stop: void (struct tevent_queue *)
+tevent_queue_wait_recv: bool (struct tevent_req *)
+tevent_queue_wait_send: struct tevent_req *(TALLOC_CTX *, struct tevent_context *, struct tevent_queue *)
+tevent_re_initialise: int (struct tevent_context *)
+tevent_register_backend: bool (const char *, const struct tevent_ops *)
+tevent_req_default_print: char *(struct tevent_req *, TALLOC_CTX *)
+tevent_req_defer_callback: void (struct tevent_req *, struct tevent_context *)
+tevent_req_is_error: bool (struct tevent_req *, enum tevent_req_state *, uint64_t *)
+tevent_req_is_in_progress: bool (struct tevent_req *)
+tevent_req_poll: bool (struct tevent_req *, struct tevent_context *)
+tevent_req_post: struct tevent_req *(struct tevent_req *, struct tevent_context *)
+tevent_req_print: char *(TALLOC_CTX *, struct tevent_req *)
+tevent_req_received: void (struct tevent_req *)
+tevent_req_reset_endtime: void (struct tevent_req *)
+tevent_req_set_callback: void (struct tevent_req *, tevent_req_fn, void *)
+tevent_req_set_cancel_fn: void (struct tevent_req *, tevent_req_cancel_fn)
+tevent_req_set_cancel_forwarding: void (struct tevent_req *, struct tevent_req *)
+tevent_req_set_cleanup_fn: void (struct tevent_req *, tevent_req_cleanup_fn)
+tevent_req_set_endtime: bool (struct tevent_req *, struct tevent_context *, struct timeval)
+tevent_req_set_print_fn: void (struct tevent_req *, tevent_req_print_fn)
+tevent_sa_info_queue_count: size_t (void)
+tevent_set_abort_fn: void (void (*)(const char *))
+tevent_set_debug: int (struct tevent_context *, void (*)(void *, enum tevent_debug_level, const char *, va_list), void *)
+tevent_set_debug_stderr: int (struct tevent_context *)
+tevent_set_default_backend: void (const char *)
+tevent_set_trace_callback: void (struct tevent_context *, tevent_trace_callback_t, void *)
+tevent_signal_support: bool (struct tevent_context *)
+tevent_thread_proxy_create: struct tevent_thread_proxy *(struct tevent_context *)
+tevent_thread_proxy_schedule: void (struct tevent_thread_proxy *, struct tevent_immediate **, tevent_immediate_handler_t, void *)
+tevent_threaded_context_create: struct tevent_threaded_context *(TALLOC_CTX *, struct tevent_context *)
+tevent_timeval_add: struct timeval (const struct timeval *, uint32_t, uint32_t)
+tevent_timeval_compare: int (const struct timeval *, const struct timeval *)
+tevent_timeval_current: struct timeval (void)
+tevent_timeval_current_ofs: struct timeval (uint32_t, uint32_t)
+tevent_timeval_is_zero: bool (const struct timeval *)
+tevent_timeval_set: struct timeval (uint32_t, uint32_t)
+tevent_timeval_until: struct timeval (const struct timeval *, const struct timeval *)
+tevent_timeval_zero: struct timeval (void)
+tevent_trace_point_callback: void (struct tevent_context *, enum tevent_trace_point)
+tevent_update_timer: void (struct tevent_timer *, struct timeval)
+tevent_wakeup_recv: bool (struct tevent_req *)
+tevent_wakeup_send: struct tevent_req *(TALLOC_CTX *, struct tevent_context *, struct timeval)
diff --git a/lib/tevent/tevent.h b/lib/tevent/tevent.h
index ba4bb4d..57ac80c 100644
--- a/lib/tevent/tevent.h
+++ b/lib/tevent/tevent.h
@@ -986,6 +986,33 @@ typedef void (*tevent_req_cleanup_fn)(struct tevent_req *req,
*/
void tevent_req_set_cleanup_fn(struct tevent_req *req, tevent_req_cleanup_fn fn);
+/**
+ * @brief This function requests automatic fowarding of a cancel to a tevent
+ * subrequest when tevent_req_cancel() is called on a request.
+ *
+ * In an async computation with nested subrequests it is possible that a
+ * subrequest is cancellable (as it called tevent_req_set_cancel_fn()), but
+ * intermediate subrequests are not.
+ *
+ * To avoid the necessity for all intermediate requests to install cancel
+ * handlers with tevent_req_set_cancel_fn() and manually forward the request
+ * from the cancel handler, a request that does a subrequest can call
+ * tevent_req_set_cancel_forwarding(req, subreq) and then tevent takes care of
+ * cancelling the subrequest when tevent_req_cancel() is called on the request.
+ *
+ * Note that for a single request you can only either set a cancel function
+ * with tevent_req_set_cancel_fn() or request cancel forwarding with
+ * tevent_req_set_cancel_forwarding(), not both.
+ *
+ * @param[in] req The request that should forward the cancel request.
+ *
+ * @param[in] subreq The sub-request to which the cancel shall be forwarded. When
+ * passing NULL, the forwarding is removed
+ *
+ */
+void tevent_req_set_cancel_forwarding(struct tevent_req *req,
+ struct tevent_req *subreq);
+
#ifdef DOXYGEN
/**
* @brief Create an async tevent request.
diff --git a/lib/tevent/tevent_internal.h b/lib/tevent/tevent_internal.h
index d60d354..ec857d1 100644
--- a/lib/tevent/tevent_internal.h
+++ b/lib/tevent/tevent_internal.h
@@ -85,6 +85,20 @@ struct tevent_req {
* was called.
*/
tevent_req_cancel_fn fn;
+
+ struct {
+ /**
+ * The subreq to forward a cancel to. There can be only
+ * one subreq.
+ */
+ struct tevent_req *subreq;
+
+ /**
+ * A backpointer to req->subreq. Used by the subreq destructor
+ * to NULL out the forward pointer in the parent req.
+ */
+ struct tevent_req **_subreq;
+ } forwarding;
} private_cancel;
/**
diff --git a/lib/tevent/tevent_req.c b/lib/tevent/tevent_req.c
index c066b7c..2b543cc 100644
--- a/lib/tevent/tevent_req.c
+++ b/lib/tevent/tevent_req.c
@@ -244,6 +244,14 @@ void tevent_req_received(struct tevent_req *req)
req->private_print = NULL;
req->private_cancel.fn = NULL;
+ req->private_cancel.forwarding.subreq = NULL;
+
+ if (req->private_cancel.forwarding._subreq != NULL) {
+ if (*req->private_cancel.forwarding._subreq == req) {
+ *req->private_cancel.forwarding._subreq = NULL;
+ }
+ req->private_cancel.forwarding._subreq = NULL;
+ }
TALLOC_FREE(req->internal.trigger);
TALLOC_FREE(req->internal.timer);
@@ -341,16 +349,46 @@ void tevent_req_set_print_fn(struct tevent_req *req, tevent_req_print_fn fn)
void tevent_req_set_cancel_fn(struct tevent_req *req, tevent_req_cancel_fn fn)
{
+ if (req->private_cancel.forwarding.subreq != NULL) {
+ abort();
+ }
+
req->private_cancel.fn = fn;
}
+void tevent_req_set_cancel_forwarding(struct tevent_req *req,
+ struct tevent_req *subreq)
+{
+ if (req->private_cancel.fn) {
+ abort();
+ }
+
+ if (req->private_cancel.forwarding.subreq != NULL) {
+ /* Allow resetting the forwarding by passing subreq as NULL */
+ if (subreq == NULL) {
+ req->private_cancel.forwarding.subreq = NULL;
+ return;
+ }
+ abort();
+ }
+
+ req->private_cancel.forwarding.subreq = subreq;
+
+ subreq->private_cancel.forwarding._subreq =
+ &req->private_cancel.forwarding.subreq;
+}
+
bool _tevent_req_cancel(struct tevent_req *req, const char *location)
{
- if (req->private_cancel.fn == NULL) {
- return false;
+ if (req->private_cancel.fn != NULL) {
+ return req->private_cancel.fn(req);
+ }
+
+ if (req->private_cancel.forwarding.subreq != NULL) {
+ return tevent_req_cancel(req->private_cancel.forwarding.subreq);
}
- return req->private_cancel.fn(req);
+ return false;
}
void tevent_req_set_cleanup_fn(struct tevent_req *req, tevent_req_cleanup_fn fn)
diff --git a/lib/tevent/wscript b/lib/tevent/wscript
index 0c02f70..54f216d 100644
--- a/lib/tevent/wscript
+++ b/lib/tevent/wscript
@@ -1,7 +1,7 @@
#!/usr/bin/env python
APPNAME = 'tevent'
-VERSION = '0.9.31'
+VERSION = '0.9.32'
blddir = 'bin'
--
2.9.3
From 1dc0adedfc18fa4b73786637a3a3a16499b7adc0 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 8 Mar 2017 15:07:06 +0100
Subject: [PATCH 03/16] s3/smbd: move copychunk ioctl limits to IDL
This will be needed in the next commit in vfs_default.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
librpc/idl/ioctl.idl | 4 ++++
source3/smbd/smb2_ioctl_network_fs.c | 3 ---
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/librpc/idl/ioctl.idl b/librpc/idl/ioctl.idl
index eb2dc8a..72f6656 100644
--- a/librpc/idl/ioctl.idl
+++ b/librpc/idl/ioctl.idl
@@ -11,6 +11,10 @@ interface copychunk
uint8 context[4];
} req_resume_key_rsp;
+ const uint32 COPYCHUNK_MAX_CHUNKS = 256;
+ const uint32 COPYCHUNK_MAX_CHUNK_LEN = 1048576;
+ const uint32 COPYCHUNK_MAX_TOTAL_LEN = 16777216;
+
typedef struct {
hyper source_off;
hyper target_off;
diff --git a/source3/smbd/smb2_ioctl_network_fs.c b/source3/smbd/smb2_ioctl_network_fs.c
index c2b889b..c279182 100644
--- a/source3/smbd/smb2_ioctl_network_fs.c
+++ b/source3/smbd/smb2_ioctl_network_fs.c
@@ -31,9 +31,6 @@
#include "smb2_ioctl_private.h"
#include "../lib/tsocket/tsocket.h"
-#define COPYCHUNK_MAX_CHUNKS 256 /* 2k8r2 & win8 = 256 */
-#define COPYCHUNK_MAX_CHUNK_LEN 1048576 /* 2k8r2 & win8 = 1048576 */
-#define COPYCHUNK_MAX_TOTAL_LEN 16777216 /* 2k8r2 & win8 = 16777216 */
static void copychunk_pack_limits(struct srv_copychunk_rsp *cc_rsp)
{
cc_rsp->chunks_written = COPYCHUNK_MAX_CHUNKS;
--
2.9.3
From b45dbc02b3811a85c0f2ccc9a5cced168c016107 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 12 Mar 2017 17:18:39 +0100
Subject: [PATCH 04/16] s3/vfs_default: use const from IDL
This also increases the buffer size from 8 MB to the current value of
COPYCHUNK_MAX_TOTAL_LEN which is 16 MB.
For the typical case when vfswrap_copy_chunk_send is called from the SMB
layer for an copy_chunk ioctl() the parameter "num" is guaranteed to be
at most 1 MB though.
It will only be larger for special callers like vfs_fruit for their
special implementation of copyfile where num will be the size of a file
to copy.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_default.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index d4610f7..054ff64 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -33,6 +33,7 @@
#include "lib/util/tevent_ntstatus.h"
#include "lib/util/sys_rw.h"
#include "lib/pthreadpool/pthreadpool_tevent.h"
+#include "librpc/gen_ndr/ndr_ioctl.h"
#undef DBGC_CLASS
#define DBGC_CLASS DBGC_VFS
@@ -1617,7 +1618,7 @@ static struct tevent_req *vfswrap_copy_chunk_send(struct vfs_handle_struct *hand
}
vfs_cc_state->buf = talloc_array(vfs_cc_state, uint8_t,
- MIN(num, 8*1024*1024));
+ MIN(num, COPYCHUNK_MAX_TOTAL_LEN));
if (tevent_req_nomem(vfs_cc_state->buf, req)) {
return tevent_req_post(req, ev);
}
--
2.9.3
From 43d0ede7ee2cb55144874a8f9caf667841cec54d Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 21 Mar 2017 08:26:37 +0100
Subject: [PATCH 05/16] s3/smbd: move cc_copy into fsctl_srv_copychunk_state
No change, in behaviour, just preperational stuff to unroll the core
copy loop.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/smb2_ioctl_network_fs.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/source3/smbd/smb2_ioctl_network_fs.c b/source3/smbd/smb2_ioctl_network_fs.c
index c279182..efb6fe7 100644
--- a/source3/smbd/smb2_ioctl_network_fs.c
+++ b/source3/smbd/smb2_ioctl_network_fs.c
@@ -77,6 +77,7 @@ static NTSTATUS copychunk_check_limits(struct srv_copychunk_copy *cc_copy)
struct fsctl_srv_copychunk_state {
struct connection_struct *conn;
+ struct srv_copychunk_copy cc_copy;
uint32_t dispatch_count;
uint32_t recv_count;
uint32_t bad_recv_count;
@@ -164,7 +165,6 @@ static struct tevent_req *fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
struct smbd_smb2_request *smb2req)
{
struct tevent_req *req;
- struct srv_copychunk_copy cc_copy;
enum ndr_err_code ndr_ret;
uint64_t src_persistent_h;
uint64_t src_volatile_h;
@@ -192,7 +192,7 @@ static struct tevent_req *fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
return tevent_req_post(req, ev);
}
- ndr_ret = ndr_pull_struct_blob(in_input, mem_ctx, &cc_copy,
+ ndr_ret = ndr_pull_struct_blob(in_input, mem_ctx, &state->cc_copy,
(ndr_pull_flags_fn_t)ndr_pull_srv_copychunk_copy);
if (ndr_ret != NDR_ERR_SUCCESS) {
DEBUG(0, ("failed to unmarshall copy chunk req\n"));
@@ -202,8 +202,8 @@ static struct tevent_req *fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
}
/* persistent/volatile keys sent as the resume key */
- src_persistent_h = BVAL(cc_copy.source_key, 0);
- src_volatile_h = BVAL(cc_copy.source_key, 8);
+ src_persistent_h = BVAL(state->cc_copy.source_key, 0);
+ src_volatile_h = BVAL(state->cc_copy.source_key, 8);
state->src_fsp = file_fsp_get(smb2req, src_persistent_h, src_volatile_h);
if (state->src_fsp == NULL) {
DEBUG(3, ("invalid resume key in copy chunk req\n"));
@@ -223,7 +223,7 @@ static struct tevent_req *fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
return tevent_req_post(req, ev);
}
- state->status = copychunk_check_limits(&cc_copy);
+ state->status = copychunk_check_limits(&state->cc_copy);
if (!NT_STATUS_IS_OK(state->status)) {
DEBUG(3, ("copy chunk req exceeds limits\n"));
state->out_data = COPYCHUNK_OUT_LIMITS;
@@ -234,7 +234,7 @@ static struct tevent_req *fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
/* any errors from here onwards should carry copychunk response data */
state->out_data = COPYCHUNK_OUT_RSP;
- if (cc_copy.chunk_count == 0) {
+ if (state->cc_copy.chunk_count == 0) {
struct tevent_req *vfs_subreq;
/*
* Process as OS X copyfile request. This is currently
@@ -266,9 +266,9 @@ static struct tevent_req *fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
return req;
}
- for (i = 0; i < cc_copy.chunk_count; i++) {
+ for (i = 0; i < state->cc_copy.chunk_count; i++) {
struct tevent_req *vfs_subreq;
- chunk = &cc_copy.chunks[i];
+ chunk = &state->cc_copy.chunks[i];
vfs_subreq = SMB_VFS_COPY_CHUNK_SEND(dst_fsp->conn,
state, ev,
state->src_fsp,
--
2.9.3
From 677b46e7894145778e88e2d291da43260de30c54 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 21 Mar 2017 09:17:03 +0100
Subject: [PATCH 06/16] s3/smbd: implement a serializing async copy-chunk loop
Later commits will make the low level copy-chunk implementation async
using a thread pool. That means the individual chunks may be scheduled
and copied out-of-order at the low level.
According to conversation with MS Dochelp, a server implementation
must process individual chunks in order.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/smb2_ioctl_network_fs.c | 188 +++++++++++++++++++----------------
1 file changed, 102 insertions(+), 86 deletions(-)
diff --git a/source3/smbd/smb2_ioctl_network_fs.c b/source3/smbd/smb2_ioctl_network_fs.c
index efb6fe7..42d9857 100644
--- a/source3/smbd/smb2_ioctl_network_fs.c
+++ b/source3/smbd/smb2_ioctl_network_fs.c
@@ -76,11 +76,11 @@ static NTSTATUS copychunk_check_limits(struct srv_copychunk_copy *cc_copy)
}
struct fsctl_srv_copychunk_state {
+ struct tevent_context *ev;
struct connection_struct *conn;
struct srv_copychunk_copy cc_copy;
- uint32_t dispatch_count;
- uint32_t recv_count;
- uint32_t bad_recv_count;
+ uint32_t current_chunk;
+ uint32_t next_chunk;
NTSTATUS status;
off_t total_written;
struct files_struct *src_fsp;
@@ -156,6 +156,8 @@ static NTSTATUS copychunk_check_handles(uint32_t ctl_code,
return NT_STATUS_OK;
}
+static NTSTATUS fsctl_srv_copychunk_loop(struct tevent_req *req);
+
static struct tevent_req *fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
uint32_t ctl_code,
@@ -164,13 +166,12 @@ static struct tevent_req *fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
size_t in_max_output,
struct smbd_smb2_request *smb2req)
{
- struct tevent_req *req;
- enum ndr_err_code ndr_ret;
+ struct tevent_req *req = NULL;
+ struct fsctl_srv_copychunk_state *state = NULL;
uint64_t src_persistent_h;
uint64_t src_volatile_h;
- int i;
- struct srv_copychunk *chunk;
- struct fsctl_srv_copychunk_state *state;
+ enum ndr_err_code ndr_ret;
+ NTSTATUS status;
/* handler for both copy-chunk variants */
SMB_ASSERT((ctl_code == FSCTL_SRV_COPYCHUNK)
@@ -181,7 +182,10 @@ static struct tevent_req *fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
if (req == NULL) {
return NULL;
}
- state->conn = dst_fsp->conn;
+ *state = (struct fsctl_srv_copychunk_state) {
+ .conn = dst_fsp->conn,
+ .ev = ev,
+ };
if (in_max_output < sizeof(struct srv_copychunk_rsp)) {
DEBUG(3, ("max output %d not large enough to hold copy chunk "
@@ -234,109 +238,123 @@ static struct tevent_req *fsctl_srv_copychunk_send(TALLOC_CTX *mem_ctx,
/* any errors from here onwards should carry copychunk response data */
state->out_data = COPYCHUNK_OUT_RSP;
+ status = fsctl_srv_copychunk_loop(req);
+ if (tevent_req_nterror(req, status)) {
+ return tevent_req_post(req, ev);
+ }
+
+ return req;
+}
+
+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;
+
+ 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) {
- struct tevent_req *vfs_subreq;
/*
* Process as OS X copyfile request. This is currently
* the only copychunk request with a chunk count of 0
* we will process.
*/
- if (!state->src_fsp->aapl_copyfile_supported) {
- tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
- return tevent_req_post(req, ev);
- }
- if (!state->dst_fsp->aapl_copyfile_supported) {
- tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER);
- return tevent_req_post(req, ev);
+ if (!state->src_fsp->aapl_copyfile_supported ||
+ !state->dst_fsp->aapl_copyfile_supported)
+ {
+ /*
+ * This must not return an error but just a chunk count
+ * of 0 in the response.
+ */
+ tevent_req_done(req);
+ tevent_req_post(req, state->ev);
+ return NT_STATUS_OK;
}
state->aapl_copyfile = true;
- vfs_subreq = SMB_VFS_COPY_CHUNK_SEND(dst_fsp->conn,
- state, ev,
- state->src_fsp,
- 0,
- state->dst_fsp,
- 0,
- 0);
- if (tevent_req_nomem(vfs_subreq, req)) {
- return tevent_req_post(req, ev);
+
+ subreq = SMB_VFS_COPY_CHUNK_SEND(state->dst_fsp->conn,
+ state,
+ state->ev,
+ state->src_fsp,
+ 0,
+ state->dst_fsp,
+ 0,
+ 0);
+ if (subreq == NULL) {
+ return NT_STATUS_NO_MEMORY;
}
- tevent_req_set_callback(vfs_subreq,
+ tevent_req_set_callback(subreq,
fsctl_srv_copychunk_vfs_done, req);
- state->dispatch_count++;
- return req;
+ return NT_STATUS_OK;
}
- for (i = 0; i < state->cc_copy.chunk_count; i++) {
- struct tevent_req *vfs_subreq;
- chunk = &state->cc_copy.chunks[i];
- vfs_subreq = SMB_VFS_COPY_CHUNK_SEND(dst_fsp->conn,
- state, ev,
- state->src_fsp,
- chunk->source_off,
- state->dst_fsp,
- chunk->target_off,
- chunk->length);
- if (vfs_subreq == NULL) {
- DEBUG(0, ("VFS copy chunk send failed\n"));
- state->status = NT_STATUS_NO_MEMORY;
- if (state->dispatch_count == 0) {
- /* nothing dispatched, return immediately */
- tevent_req_nterror(req, state->status);
- return tevent_req_post(req, ev);
- } else {
- /*
- * wait for dispatched to complete before
- * returning error.
- */
- break;
- }
- }
- tevent_req_set_callback(vfs_subreq,
- fsctl_srv_copychunk_vfs_done, req);
- state->dispatch_count++;
+ chunk = &state->cc_copy.chunks[state->current_chunk];
+
+ /*
+ * Doing the increment and calculation for the next chunk here and not
+ * in the done function, as a later commit will make this a more
+ * sophisticated logic.
+ */
+ state->next_chunk++;
+
+ subreq = SMB_VFS_COPY_CHUNK_SEND(state->dst_fsp->conn,
+ state,
+ state->ev,
+ state->src_fsp,
+ chunk->source_off,
+ state->dst_fsp,
+ chunk->target_off,
+ chunk->length);
+ if (tevent_req_nomem(subreq, req)) {
+ return NT_STATUS_NO_MEMORY;
}
+ tevent_req_set_callback(subreq, fsctl_srv_copychunk_vfs_done, req);
- return req;
+ return NT_STATUS_OK;
}
static void fsctl_srv_copychunk_vfs_done(struct tevent_req *subreq)
{
struct tevent_req *req = tevent_req_callback_data(
subreq, struct tevent_req);
- struct fsctl_srv_copychunk_state *state = tevent_req_data(req,
- struct fsctl_srv_copychunk_state);
+ struct fsctl_srv_copychunk_state *state = tevent_req_data(
+ req, struct fsctl_srv_copychunk_state);
off_t chunk_nwritten;
NTSTATUS status;
- state->recv_count++;
status = SMB_VFS_COPY_CHUNK_RECV(state->conn, subreq,
&chunk_nwritten);
TALLOC_FREE(subreq);
- if (NT_STATUS_IS_OK(status)) {
- DEBUG(10, ("good copy chunk recv %u of %u\n",
- (unsigned int)state->recv_count,
- (unsigned int)state->dispatch_count));
- state->total_written += chunk_nwritten;
- } else {
- DEBUG(0, ("bad status in copy chunk recv %u of %u: %s\n",
- (unsigned int)state->recv_count,
- (unsigned int)state->dispatch_count,
- nt_errstr(status)));
- state->bad_recv_count++;
- /* may overwrite previous failed status */
- state->status = status;
- }
-
- if (state->recv_count != state->dispatch_count) {
- /*
- * Wait for all VFS copy_chunk requests to complete, even
- * if an error is received for a specific chunk.
- */
+ 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->cc_copy.chunk_count);
+ tevent_req_nterror(req, status);
return;
}
- if (!tevent_req_nterror(req, state->status)) {
+ DBG_DEBUG("good copy chunk [%u] of [%u]\n",
+ (unsigned int)state->next_chunk,
+ (unsigned int)state->cc_copy.chunk_count);
+ state->total_written += chunk_nwritten;
+
+ state->current_chunk = state->next_chunk;
+
+ if (state->next_chunk == state->cc_copy.chunk_count) {
tevent_req_done(req);
+ return;
+ }
+
+ status = fsctl_srv_copychunk_loop(req);
+ if (tevent_req_nterror(req, status)) {
+ return;
}
}
@@ -361,7 +379,7 @@ static NTSTATUS fsctl_srv_copychunk_recv(struct tevent_req *req,
if (state->aapl_copyfile == true) {
cc_rsp->chunks_written = 0;
} else {
- cc_rsp->chunks_written = state->recv_count - state->bad_recv_count;
+ cc_rsp->chunks_written = state->current_chunk;
}
cc_rsp->chunk_bytes_written = 0;
cc_rsp->total_bytes_written = state->total_written;
@@ -371,9 +389,7 @@ static NTSTATUS fsctl_srv_copychunk_recv(struct tevent_req *req,
assert(1);
break;
}
- status = state->status;
- tevent_req_received(req);
-
+ status = tevent_req_simple_recv_ntstatus(req);
return status;
}
--
2.9.3
From c8c4c1906ed03f31fe9cdd1c3a19703d9468bf8c Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Tue, 21 Mar 2017 18:34:22 +0100
Subject: [PATCH 07/16] s3/smbd: optimize copy-chunk by merging chunks if
possible
Merge chunks with adjacent ranges. This results in fewer IO requests for
the typical server-side file copy usecase: just one 16 MB copy instead
of sixteen 1 MB.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/smb2_ioctl_network_fs.c | 79 ++++++++++++++++++++++++++++++------
1 file changed, 67 insertions(+), 12 deletions(-)
diff --git a/source3/smbd/smb2_ioctl_network_fs.c b/source3/smbd/smb2_ioctl_network_fs.c
index 42d9857..5b869ec 100644
--- a/source3/smbd/smb2_ioctl_network_fs.c
+++ b/source3/smbd/smb2_ioctl_network_fs.c
@@ -246,12 +246,74 @@ 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",
@@ -269,8 +331,8 @@ static NTSTATUS fsctl_srv_copychunk_loop(struct tevent_req *req)
!state->dst_fsp->aapl_copyfile_supported)
{
/*
- * This must not return an error but just a chunk count
- * of 0 in the response.
+ * This must not produce an error but just return a
+ * chunk count of 0 in the response.
*/
tevent_req_done(req);
tevent_req_post(req, state->ev);
@@ -295,13 +357,7 @@ static NTSTATUS fsctl_srv_copychunk_loop(struct tevent_req *req)
}
chunk = &state->cc_copy.chunks[state->current_chunk];
-
- /*
- * Doing the increment and calculation for the next chunk here and not
- * in the done function, as a later commit will make this a more
- * sophisticated logic.
- */
- state->next_chunk++;
+ length = next_merged_io(state);
subreq = SMB_VFS_COPY_CHUNK_SEND(state->dst_fsp->conn,
state,
@@ -310,7 +366,7 @@ static NTSTATUS fsctl_srv_copychunk_loop(struct tevent_req *req)
chunk->source_off,
state->dst_fsp,
chunk->target_off,
- chunk->length);
+ length);
if (tevent_req_nomem(subreq, req)) {
return NT_STATUS_NO_MEMORY;
}
@@ -346,8 +402,7 @@ static void fsctl_srv_copychunk_vfs_done(struct tevent_req *subreq)
state->total_written += chunk_nwritten;
state->current_chunk = state->next_chunk;
-
- if (state->next_chunk == state->cc_copy.chunk_count) {
+ if (state->current_chunk == state->cc_copy.chunk_count) {
tevent_req_done(req);
return;
}
--
2.9.3
From 123a1862552f162759abc73f9a22c03b720eff60 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 12 Mar 2017 17:23:09 +0100
Subject: [PATCH 08/16] s3/vfs_default: move check for fsp->op validity
Move the check whether fsp->of is valid out of the copy loop in
vfswrap_copy_chunk_send().
It's sufficient to check src_fsp->op and dest_fsp->op once before the
copy loop. fsp->op can only be NULL for internal opens (cf file_new()),
it's not expected to become NULL behind our backs.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_default.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 054ff64..545e21a 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -1642,6 +1642,16 @@ static struct tevent_req *vfswrap_copy_chunk_send(struct vfs_handle_struct *hand
return tevent_req_post(req, ev);
}
+ if (src_fsp->op == NULL) {
+ tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
+ return tevent_req_post(req, ev);
+ }
+
+ if (dest_fsp->op == NULL) {
+ tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
+ return tevent_req_post(req, ev);
+ }
+
/* could use 2.6.33+ sendfile here to do this in kernel */
while (vfs_cc_state->copied < num) {
ssize_t ret;
@@ -1651,10 +1661,6 @@ static struct tevent_req *vfswrap_copy_chunk_send(struct vfs_handle_struct *hand
off_t this_num = MIN(talloc_array_length(vfs_cc_state->buf),
num - vfs_cc_state->copied);
- if (src_fsp->op == NULL) {
- tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
- return tevent_req_post(req, ev);
- }
init_strict_lock_struct(src_fsp,
src_fsp->op->global->open_persistent_id,
src_off,
@@ -1688,11 +1694,6 @@ static struct tevent_req *vfswrap_copy_chunk_send(struct vfs_handle_struct *hand
src_off += ret;
- if (dest_fsp->op == NULL) {
- tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
- return tevent_req_post(req, ev);
- }
-
init_strict_lock_struct(dest_fsp,
dest_fsp->op->global->open_persistent_id,
dest_off,
--
2.9.3
From cb894075fdae1af8537a183220cbf3792a5b2966 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Sun, 12 Mar 2017 18:13:48 +0100
Subject: [PATCH 09/16] s3/vfs_default: lock the whole byte-range before the
copy loop
For callers that pass the num argument with a value higher then the max
copy buffer we should check the byte-ranges before starting the copy
loop so the resulting locking semantics are coupled to the calling layer
sizes and not our buffer size of the day.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_default.c | 69 ++++++++++++++++++++++---------------------
1 file changed, 35 insertions(+), 34 deletions(-)
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 545e21a..e0945ff 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -1607,6 +1607,8 @@ static struct tevent_req *vfswrap_copy_chunk_send(struct vfs_handle_struct *hand
{
struct tevent_req *req;
struct vfs_cc_state *vfs_cc_state;
+ struct lock_struct write_lck;
+ struct lock_struct read_lck;
NTSTATUS status;
DEBUG(10, ("performing server side copy chunk of length %lu\n",
@@ -1652,77 +1654,72 @@ static struct tevent_req *vfswrap_copy_chunk_send(struct vfs_handle_struct *hand
return tevent_req_post(req, ev);
}
+ init_strict_lock_struct(src_fsp,
+ src_fsp->op->global->open_persistent_id,
+ src_off,
+ num,
+ READ_LOCK,
+ &read_lck);
+
+ if (!SMB_VFS_STRICT_LOCK(src_fsp->conn, src_fsp, &read_lck)) {
+ tevent_req_nterror(req, NT_STATUS_FILE_LOCK_CONFLICT);
+ return tevent_req_post(req, ev);
+ }
+
+ init_strict_lock_struct(dest_fsp,
+ dest_fsp->op->global->open_persistent_id,
+ dest_off,
+ num,
+ WRITE_LOCK,
+ &write_lck);
+
+ if (!SMB_VFS_STRICT_LOCK(dest_fsp->conn, dest_fsp, &write_lck)) {
+ SMB_VFS_STRICT_UNLOCK(dest_fsp->conn, dest_fsp, &read_lck);
+ tevent_req_nterror(req, NT_STATUS_FILE_LOCK_CONFLICT);
+ return tevent_req_post(req, ev);
+ }
+
/* could use 2.6.33+ sendfile here to do this in kernel */
while (vfs_cc_state->copied < num) {
ssize_t ret;
- struct lock_struct lck;
int saved_errno;
-
off_t this_num = MIN(talloc_array_length(vfs_cc_state->buf),
num - vfs_cc_state->copied);
- init_strict_lock_struct(src_fsp,
- src_fsp->op->global->open_persistent_id,
- src_off,
- this_num,
- READ_LOCK,
- &lck);
-
- if (!SMB_VFS_STRICT_LOCK(src_fsp->conn, src_fsp, &lck)) {
- tevent_req_nterror(req, NT_STATUS_FILE_LOCK_CONFLICT);
- return tevent_req_post(req, ev);
- }
-
ret = SMB_VFS_PREAD(src_fsp, vfs_cc_state->buf,
this_num, src_off);
if (ret == -1) {
saved_errno = errno;
}
- SMB_VFS_STRICT_UNLOCK(src_fsp->conn, src_fsp, &lck);
-
if (ret == -1) {
errno = saved_errno;
tevent_req_nterror(req, map_nt_error_from_unix(errno));
- return tevent_req_post(req, ev);
+ goto fail;
}
if (ret != this_num) {
/* zero tolerance for short reads */
tevent_req_nterror(req, NT_STATUS_IO_DEVICE_ERROR);
- return tevent_req_post(req, ev);
+ goto fail;
}
src_off += ret;
- init_strict_lock_struct(dest_fsp,
- dest_fsp->op->global->open_persistent_id,
- dest_off,
- this_num,
- WRITE_LOCK,
- &lck);
-
- if (!SMB_VFS_STRICT_LOCK(dest_fsp->conn, dest_fsp, &lck)) {
- tevent_req_nterror(req, NT_STATUS_FILE_LOCK_CONFLICT);
- return tevent_req_post(req, ev);
- }
-
ret = SMB_VFS_PWRITE(dest_fsp, vfs_cc_state->buf,
this_num, dest_off);
if (ret == -1) {
saved_errno = errno;
}
- SMB_VFS_STRICT_UNLOCK(dest_fsp->conn, dest_fsp, &lck);
-
if (ret == -1) {
errno = saved_errno;
tevent_req_nterror(req, map_nt_error_from_unix(errno));
- return tevent_req_post(req, ev);
+ goto fail;
}
if (ret != this_num) {
/* zero tolerance for short writes */
tevent_req_nterror(req, NT_STATUS_IO_DEVICE_ERROR);
- return tevent_req_post(req, ev);
+ goto fail;
}
dest_off += ret;
@@ -1730,6 +1727,10 @@ static struct tevent_req *vfswrap_copy_chunk_send(struct vfs_handle_struct *hand
}
tevent_req_done(req);
+
+fail:
+ SMB_VFS_STRICT_UNLOCK(src_fsp->conn, src_fsp, &read_lck);
+ SMB_VFS_STRICT_UNLOCK(dest_fsp->conn, dest_fsp, &write_lck);
return tevent_req_post(req, ev);
}
--
2.9.3
From 67cfeca7dbb2071ffe260c907244a67f1b39645f Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Mon, 13 Mar 2017 07:51:39 +0100
Subject: [PATCH 10/16] s3/vfs: forward tevent cancel request in copy_chunk
Use tevent_req_set_cancel_forwarding() to forward cancel requests. No
change in behaviour here, but a subsequent commit will make copy_chunk
async in vfs_default and allow it to be cancelled.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_btrfs.c | 2 ++
source3/modules/vfs_fruit.c | 1 +
source3/modules/vfs_time_audit.c | 1 +
source3/smbd/smb2_ioctl_network_fs.c | 9 +++++++++
4 files changed, 13 insertions(+)
diff --git a/source3/modules/vfs_btrfs.c b/source3/modules/vfs_btrfs.c
index 154fcc3..154ed79 100644
--- a/source3/modules/vfs_btrfs.c
+++ b/source3/modules/vfs_btrfs.c
@@ -126,6 +126,7 @@ static struct tevent_req *btrfs_copy_chunk_send(struct vfs_handle_struct *handle
tevent_req_set_callback(cc_state->subreq,
btrfs_copy_chunk_done,
req);
+ tevent_req_set_cancel_forwarding(req, cc_state->subreq);
return req;
}
@@ -204,6 +205,7 @@ static struct tevent_req *btrfs_copy_chunk_send(struct vfs_handle_struct *handle
tevent_req_set_callback(cc_state->subreq,
btrfs_copy_chunk_done,
req);
+ tevent_req_set_cancel_forwarding(req, cc_state->subreq);
return req;
}
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index fc80629..38ce697 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -5252,6 +5252,7 @@ static struct tevent_req *fruit_copy_chunk_send(struct vfs_handle_struct *handle
}
tevent_req_set_callback(subreq, fruit_copy_chunk_done, req);
+ tevent_req_set_cancel_forwarding(req, subreq);
return req;
}
diff --git a/source3/modules/vfs_time_audit.c b/source3/modules/vfs_time_audit.c
index 986fe79..b358f46 100644
--- a/source3/modules/vfs_time_audit.c
+++ b/source3/modules/vfs_time_audit.c
@@ -1907,6 +1907,7 @@ static struct tevent_req *smb_time_audit_copy_chunk_send(struct vfs_handle_struc
}
tevent_req_set_callback(subreq, smb_time_audit_copy_chunk_done, req);
+ tevent_req_set_cancel_forwarding(req, subreq);
return req;
}
diff --git a/source3/smbd/smb2_ioctl_network_fs.c b/source3/smbd/smb2_ioctl_network_fs.c
index 5b869ec..10d53e6 100644
--- a/source3/smbd/smb2_ioctl_network_fs.c
+++ b/source3/smbd/smb2_ioctl_network_fs.c
@@ -353,6 +353,7 @@ static NTSTATUS fsctl_srv_copychunk_loop(struct tevent_req *req)
}
tevent_req_set_callback(subreq,
fsctl_srv_copychunk_vfs_done, req);
+ tevent_req_set_cancel_forwarding(req, subreq);
return NT_STATUS_OK;
}
@@ -372,6 +373,13 @@ static NTSTATUS fsctl_srv_copychunk_loop(struct tevent_req *req)
}
tevent_req_set_callback(subreq, fsctl_srv_copychunk_vfs_done, req);
+ /*
+ * Request cancel forwarding. When the subreq is freed in
+ * fsctl_srv_copychunk_vfs_done() the cancel is also removed, so we can
+ * point the cancel to the next subreq.
+ */
+ tevent_req_set_cancel_forwarding(req, subreq);
+
return NT_STATUS_OK;
}
@@ -720,6 +728,7 @@ struct tevent_req *smb2_ioctl_network_fs(uint32_t ctl_code,
tevent_req_set_callback(subreq,
smb2_ioctl_network_fs_copychunk_done,
req);
+ tevent_req_set_cancel_forwarding(req, subreq);
return req;
break;
case FSCTL_QUERY_NETWORK_INTERFACE_INFO:
--
2.9.3
From 0a31c7041147f47787e9a516c5ce8fdc9ed282ed Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 15 Mar 2017 15:23:46 +0100
Subject: [PATCH 11/16] s3/smbd: decrease a debug log level in copy_chunk
The request could have been user cancelled, so let's just log errors at
here with level 10. The lower level implementation can better log this
for real IO errors, a subsequent commit will do just that.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/smb2_ioctl_network_fs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/smbd/smb2_ioctl_network_fs.c b/source3/smbd/smb2_ioctl_network_fs.c
index 10d53e6..80787e5 100644
--- a/source3/smbd/smb2_ioctl_network_fs.c
+++ b/source3/smbd/smb2_ioctl_network_fs.c
@@ -396,7 +396,7 @@ static void fsctl_srv_copychunk_vfs_done(struct tevent_req *subreq)
&chunk_nwritten);
TALLOC_FREE(subreq);
if (!NT_STATUS_IS_OK(status)) {
- DBG_ERR("copy chunk failed [%s] chunk [%u] of [%u]\n",
+ DBG_DEBUG("copy chunk failed [%s] chunk [%u] of [%u]\n",
nt_errstr(status),
(unsigned int)state->next_chunk,
(unsigned int)state->cc_copy.chunk_count);
--
2.9.3
From 97e865e4e480de2aa515f4f1b50620b43c589ddd Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 15 Mar 2017 15:25:54 +0100
Subject: [PATCH 12/16] vfs_fruit: decrease a debug log level in copy_chunk
The request could have been user cancelled, so let's just log errors at
here with level 10. The lower level implementation will error out anyway
in this failure case.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_fruit.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index 38ce697..23007c1 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -5366,8 +5366,7 @@ static NTSTATUS fruit_copy_chunk_recv(struct vfs_handle_struct *handle,
NTSTATUS status;
if (tevent_req_is_nterror(req, &status)) {
- DEBUG(1, ("server side copy chunk failed: %s\n",
- nt_errstr(status)));
+ DBG_DEBUG("copy chunk failed: %s\n", nt_errstr(status));
*copied = 0;
tevent_req_received(req);
return status;
--
2.9.3
From a631a3af1d84ccf2517e77e4095c99d02759f1be Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 22 Mar 2017 18:15:03 +0100
Subject: [PATCH 13/16] s3/smbd: let copy-chunk add an aio object to the fsp
This is needed for making the copy-chunk implementation async in
subsequent commits. For the current sync code it's an noop, the aio
object will immediately get removed again when the req is received.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/smbd/smb2_ioctl_network_fs.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/source3/smbd/smb2_ioctl_network_fs.c b/source3/smbd/smb2_ioctl_network_fs.c
index 80787e5..254f79e 100644
--- a/source3/smbd/smb2_ioctl_network_fs.c
+++ b/source3/smbd/smb2_ioctl_network_fs.c
@@ -705,6 +705,7 @@ struct tevent_req *smb2_ioctl_network_fs(uint32_t ctl_code,
{
struct tevent_req *subreq;
NTSTATUS status;
+ bool ok;
switch (ctl_code) {
/*
@@ -729,6 +730,14 @@ struct tevent_req *smb2_ioctl_network_fs(uint32_t ctl_code,
smb2_ioctl_network_fs_copychunk_done,
req);
tevent_req_set_cancel_forwarding(req, subreq);
+
+ /* Ensure we can't be closed in flight. */
+ ok = aio_add_req_to_fsp(state->fsp, req);
+ if (!ok) {
+ tevent_req_nterror(req, NT_STATUS_NO_MEMORY);
+ return tevent_req_post(req, ev);
+ }
+
return req;
break;
case FSCTL_QUERY_NETWORK_INTERFACE_INFO:
--
2.9.3
From cad8abbb65dbf0c2dad1e0c3defd9be6c09a9084 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 8 Mar 2017 15:07:58 +0100
Subject: [PATCH 14/16] s3/smbd: make copy chunk asynchronous
Just use SMB_VFS_PREAD_SEND/RECV and SMB_VFS_PWRITE_SEND/RECV in a
sensible loop.
Also install a cancel handler, so it's now possible to cancel it from
the SMB layer. This would be especially useful for the fruit:copyfile
behaviour where a single server-side copy request lets us copy the whole
file, unfortunately at present the macOS client doesn't support
cancelling the copy-chunk ioctl request.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source3/modules/vfs_default.c | 272 ++++++++++++++++++++++++++++++------------
1 file changed, 197 insertions(+), 75 deletions(-)
diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index e0945ff..9ced46b 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -1592,10 +1592,27 @@ static NTSTATUS vfswrap_fset_dos_attributes(struct vfs_handle_struct *handle,
}
struct vfs_cc_state {
- off_t copied;
+ struct tevent_context *ev;
+ bool cancelled;
uint8_t *buf;
+ bool read_lck_locked;
+ struct lock_struct read_lck;
+ bool write_lck_locked;
+ struct lock_struct write_lck;
+ struct files_struct *src_fsp;
+ off_t src_off;
+ struct files_struct *dst_fsp;
+ off_t dst_off;
+ off_t to_copy;
+ off_t remaining;
+ size_t next_io_size;
};
+static NTSTATUS copy_chunk_loop(TALLOC_CTX *mem_ctx,
+ struct tevent_req *req);
+
+static bool vfswrap_copy_chunk_cancel(struct tevent_req *req);
+
static struct tevent_req *vfswrap_copy_chunk_send(struct vfs_handle_struct *handle,
TALLOC_CTX *mem_ctx,
struct tevent_context *ev,
@@ -1603,25 +1620,32 @@ static struct tevent_req *vfswrap_copy_chunk_send(struct vfs_handle_struct *hand
off_t src_off,
struct files_struct *dest_fsp,
off_t dest_off,
- off_t num)
+ off_t to_copy)
{
struct tevent_req *req;
- struct vfs_cc_state *vfs_cc_state;
- struct lock_struct write_lck;
- struct lock_struct read_lck;
+ struct vfs_cc_state *state = NULL;
+ size_t num = MIN(to_copy, COPYCHUNK_MAX_TOTAL_LEN);
NTSTATUS status;
+ bool ok;
- DEBUG(10, ("performing server side copy chunk of length %lu\n",
- (unsigned long)num));
+ DBG_DEBUG("server side copy chunk of length %" PRIu64 "\n", to_copy);
- req = tevent_req_create(mem_ctx, &vfs_cc_state, struct vfs_cc_state);
+ req = tevent_req_create(mem_ctx, &state, struct vfs_cc_state);
if (req == NULL) {
return NULL;
}
- vfs_cc_state->buf = talloc_array(vfs_cc_state, uint8_t,
- MIN(num, COPYCHUNK_MAX_TOTAL_LEN));
- if (tevent_req_nomem(vfs_cc_state->buf, req)) {
+ *state = (struct vfs_cc_state) {
+ .ev = ev,
+ .src_fsp = src_fsp,
+ .src_off = src_off,
+ .dst_fsp = dest_fsp,
+ .dst_off = dest_off,
+ .to_copy = to_copy,
+ .remaining = to_copy,
+ };
+ state->buf = talloc_array(state, uint8_t, num);
+ if (tevent_req_nomem(state->buf, req)) {
return tevent_req_post(req, ev);
}
@@ -1654,105 +1678,203 @@ static struct tevent_req *vfswrap_copy_chunk_send(struct vfs_handle_struct *hand
return tevent_req_post(req, ev);
}
- init_strict_lock_struct(src_fsp,
- src_fsp->op->global->open_persistent_id,
- src_off,
- num,
+ init_strict_lock_struct(state->src_fsp,
+ state->src_fsp->op->global->open_persistent_id,
+ state->src_off,
+ to_copy,
READ_LOCK,
- &read_lck);
+ &state->read_lck);
- if (!SMB_VFS_STRICT_LOCK(src_fsp->conn, src_fsp, &read_lck)) {
+ ok = SMB_VFS_STRICT_LOCK(state->src_fsp->conn,
+ state->src_fsp,
+ &state->read_lck);
+ if (!ok) {
tevent_req_nterror(req, NT_STATUS_FILE_LOCK_CONFLICT);
return tevent_req_post(req, ev);
}
+ state->read_lck_locked = true;
- init_strict_lock_struct(dest_fsp,
- dest_fsp->op->global->open_persistent_id,
- dest_off,
- num,
+ init_strict_lock_struct(state->dst_fsp,
+ state->dst_fsp->op->global->open_persistent_id,
+ state->dst_off,
+ to_copy,
WRITE_LOCK,
- &write_lck);
+ &state->write_lck);
- if (!SMB_VFS_STRICT_LOCK(dest_fsp->conn, dest_fsp, &write_lck)) {
- SMB_VFS_STRICT_UNLOCK(dest_fsp->conn, dest_fsp, &read_lck);
+ ok = SMB_VFS_STRICT_LOCK(state->dst_fsp->conn,
+ state->dst_fsp,
+ &state->write_lck);
+ if (!ok) {
tevent_req_nterror(req, NT_STATUS_FILE_LOCK_CONFLICT);
return tevent_req_post(req, ev);
}
+ state->write_lck_locked = true;
- /* could use 2.6.33+ sendfile here to do this in kernel */
- while (vfs_cc_state->copied < num) {
- ssize_t ret;
- int saved_errno;
- off_t this_num = MIN(talloc_array_length(vfs_cc_state->buf),
- num - vfs_cc_state->copied);
+ status = copy_chunk_loop(state, req);
+ if (!NT_STATUS_IS_OK(status)) {
+ tevent_req_nterror(req, status);
+ return tevent_req_post(req, ev);
+ }
- ret = SMB_VFS_PREAD(src_fsp, vfs_cc_state->buf,
- this_num, src_off);
- if (ret == -1) {
- saved_errno = errno;
- }
+ tevent_req_set_cancel_fn(req, vfswrap_copy_chunk_cancel);
+ return req;
+}
- if (ret == -1) {
- errno = saved_errno;
- tevent_req_nterror(req, map_nt_error_from_unix(errno));
- goto fail;
- }
- if (ret != this_num) {
- /* zero tolerance for short reads */
- tevent_req_nterror(req, NT_STATUS_IO_DEVICE_ERROR);
- goto fail;
- }
+static bool vfswrap_copy_chunk_cancel(struct tevent_req *req)
+{
+ struct vfs_cc_state *state = tevent_req_data(req, struct vfs_cc_state);
- src_off += ret;
+ state->cancelled = true;
+ return true;
+}
- ret = SMB_VFS_PWRITE(dest_fsp, vfs_cc_state->buf,
- this_num, dest_off);
- if (ret == -1) {
- saved_errno = errno;
- }
+static void vfswrap_copy_chunk_read_done(struct tevent_req *subreq);
- if (ret == -1) {
- errno = saved_errno;
- tevent_req_nterror(req, map_nt_error_from_unix(errno));
- goto fail;
- }
- if (ret != this_num) {
- /* zero tolerance for short writes */
- tevent_req_nterror(req, NT_STATUS_IO_DEVICE_ERROR);
- goto fail;
- }
- dest_off += ret;
+static NTSTATUS copy_chunk_loop(TALLOC_CTX *mem_ctx,
+ struct tevent_req *req)
+{
+ struct vfs_cc_state *state = tevent_req_data(req, struct vfs_cc_state);
+ struct tevent_req *subreq = NULL;
- vfs_cc_state->copied += this_num;
+ state->next_io_size = MIN(state->remaining, talloc_array_length(state->buf));
+
+ subreq = SMB_VFS_PREAD_SEND(state,
+ state->src_fsp->conn->sconn->ev_ctx,
+ state->src_fsp,
+ state->buf,
+ state->next_io_size,
+ state->src_off);
+ if (subreq == NULL) {
+ return NT_STATUS_NO_MEMORY;
}
+ tevent_req_set_callback(subreq, vfswrap_copy_chunk_read_done, req);
- tevent_req_done(req);
+ return NT_STATUS_OK;
+}
+
+static void vfswrap_copy_chunk_write_done(struct tevent_req *subreq);
+
+static void vfswrap_copy_chunk_read_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(
+ subreq, struct tevent_req);
+ struct vfs_cc_state *state = tevent_req_data(req, struct vfs_cc_state);
+ struct vfs_aio_state aio_state;
+ ssize_t nread;
+
+ nread = SMB_VFS_PREAD_RECV(subreq, &aio_state);
+ TALLOC_FREE(subreq);
+ if (nread == -1) {
+ DBG_ERR("read failed: %s\n", strerror(errno));
+ tevent_req_nterror(req, map_nt_error_from_unix(aio_state.error));
+ return;
+ }
+ if (nread != state->next_io_size) {
+ DBG_ERR("Short read, only %zd of %zu\n",
+ nread, state->next_io_size);
+ tevent_req_nterror(req, NT_STATUS_IO_DEVICE_ERROR);
+ return;
+ }
-fail:
- SMB_VFS_STRICT_UNLOCK(src_fsp->conn, src_fsp, &read_lck);
- SMB_VFS_STRICT_UNLOCK(dest_fsp->conn, dest_fsp, &write_lck);
- return tevent_req_post(req, ev);
+ state->src_off += nread;
+
+ if (state->cancelled) {
+ DBG_DEBUG("Cancelled at offset %zd\n", state->src_off);
+ tevent_req_nterror(req, NT_STATUS_CANCELLED);
+ return;
+ }
+
+ subreq = SMB_VFS_PWRITE_SEND(state,
+ state->ev,
+ state->dst_fsp,
+ state->buf,
+ state->next_io_size,
+ state->dst_off);
+ if (subreq == NULL) {
+ tevent_req_nterror(req, NT_STATUS_NO_MEMORY);
+ return;
+ }
+ tevent_req_set_callback(subreq, vfswrap_copy_chunk_write_done, req);
+}
+
+static void vfswrap_copy_chunk_write_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(
+ subreq, struct tevent_req);
+ struct vfs_cc_state *state = tevent_req_data(req, struct vfs_cc_state);
+ struct vfs_aio_state aio_state;
+ ssize_t nwritten;
+ NTSTATUS status;
+
+ nwritten = SMB_VFS_PWRITE_RECV(subreq, &aio_state);
+ TALLOC_FREE(subreq);
+ if (nwritten == -1) {
+ DBG_ERR("write failed: %s\n", strerror(errno));
+ tevent_req_nterror(req, map_nt_error_from_unix(aio_state.error));
+ return;
+ }
+ if (nwritten != state->next_io_size) {
+ DBG_ERR("Short write, only %zd of %zu\n", nwritten, state->next_io_size);
+ tevent_req_nterror(req, NT_STATUS_IO_DEVICE_ERROR);
+ return;
+ }
+
+ state->dst_off += nwritten;
+
+ if (state->cancelled) {
+ DBG_DEBUG("Cancelled at offset %zd\n", state->dst_off);
+ tevent_req_nterror(req, NT_STATUS_CANCELLED);
+ return;
+ }
+
+ if ((state->remaining - nwritten) > state->remaining) {
+ /* Paranoia check */
+ tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR);
+ return;
+ }
+ state->remaining -= nwritten;
+ if (state->remaining == 0) {
+ tevent_req_done(req);
+ return;
+ }
+
+ status = copy_chunk_loop(state, req);
+ if (!NT_STATUS_IS_OK(status)) {
+ tevent_req_nterror(req, status);
+ return;
+ }
+
+ return;
}
static NTSTATUS vfswrap_copy_chunk_recv(struct vfs_handle_struct *handle,
struct tevent_req *req,
off_t *copied)
{
- struct vfs_cc_state *vfs_cc_state = tevent_req_data(req,
- struct vfs_cc_state);
+ struct vfs_cc_state *state = tevent_req_data(req, struct vfs_cc_state);
NTSTATUS status;
+ if (state->read_lck_locked) {
+ SMB_VFS_STRICT_UNLOCK(state->src_fsp->conn,
+ state->src_fsp,
+ &state->read_lck);
+ }
+
+ if (state->write_lck_locked) {
+ SMB_VFS_STRICT_UNLOCK(state->dst_fsp->conn,
+ state->dst_fsp,
+ &state->write_lck);
+ }
+
if (tevent_req_is_nterror(req, &status)) {
- DEBUG(2, ("server side copy chunk failed: %s\n",
- nt_errstr(status)));
+ DBG_DEBUG("copy chunk failed: %s\n", nt_errstr(status));
*copied = 0;
tevent_req_received(req);
return status;
}
- *copied = vfs_cc_state->copied;
- DEBUG(10, ("server side copy chunk copied %lu\n",
- (unsigned long)*copied));
+ *copied = state->to_copy;
+ DBG_DEBUG("copy chunk copied %lu\n", (unsigned long)*copied);
tevent_req_received(req);
return NT_STATUS_OK;
--
2.9.3
From dc61ed2f1e623b07b7931bc55935d4dc07ad1dd4 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 15 Mar 2017 13:47:39 +0100
Subject: [PATCH 15/16] torture: add two new assert macros
The two new macros are:
o torture_assert_ndr_err_equal_goto()
o torture_assert_ndr_success_goto()
Signed-off-by: Ralph Boehme <slow at samba.org>
---
lib/torture/torture.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/lib/torture/torture.h b/lib/torture/torture.h
index b6d1301..3e476c2 100644
--- a/lib/torture/torture.h
+++ b/lib/torture/torture.h
@@ -293,6 +293,15 @@ void torture_result(struct torture_context *test,
}\
} while(0)
+#define torture_assert_ndr_err_equal_goto(torture_ctx,got,expected,ret,label,cmt) \
+ do { enum ndr_err_code __got = got, __expected = expected; \
+ if (__got != __expected) { \
+ torture_result(torture_ctx, TORTURE_FAIL, __location__": "#got" was %d (%s), expected %d (%s): %s", __got, ndr_errstr(__got), __expected, __STRING(expected), cmt); \
+ ret = false; \
+ goto label; \
+ } \
+ } while(0)
+
#define torture_assert_hresult_equal(torture_ctx, got, expected, cmt) \
do { HRESULT __got = got, __expected = expected; \
if (!HRES_IS_EQUAL(__got, __expected)) { \
@@ -637,6 +646,9 @@ static inline void torture_dump_data_str_cb(const char *buf, void *private_data)
#define torture_assert_ndr_success(torture_ctx,expr,cmt) \
torture_assert_ndr_err_equal(torture_ctx,expr,NDR_ERR_SUCCESS,cmt)
+#define torture_assert_ndr_success_goto(torture_ctx,expr,ret,label,cmt) \
+ torture_assert_ndr_err_equal_goto(torture_ctx,expr,NDR_ERR_SUCCESS,ret,label,cmt)
+
#define torture_assert_hresult_ok(torture_ctx,expr,cmt) \
torture_assert_hresult_equal(torture_ctx,expr,HRES_ERROR(0), cmt)
--
2.9.3
From 102dd4c6375d061cf30ef38c1538b7873d22b7c9 Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Wed, 15 Mar 2017 13:49:29 +0100
Subject: [PATCH 16/16] s4/torture: add a test for async copy_chunk
cancellation
This tests async copy_chunk cancellecation of the request.
It is added to the vfs.fruit testsuite as leveraging the fruit:copyfile
semantics makes it possible to have a long running copy loop that runs
long enough so we can cancel it.
Signed-off-by: Ralph Boehme <slow at samba.org>
---
source4/torture/vfs/fruit.c | 64 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 63 insertions(+), 1 deletion(-)
diff --git a/source4/torture/vfs/fruit.c b/source4/torture/vfs/fruit.c
index d74a153..75fb619 100644
--- a/source4/torture/vfs/fruit.c
+++ b/source4/torture/vfs/fruit.c
@@ -2593,6 +2593,68 @@ done:
return true;
}
+static bool test_copy_chunk_cancel(struct torture_context *torture,
+ struct smb2_tree *tree)
+{
+ struct smb2_handle src_h = {{0}};
+ struct smb2_handle dest_h = {{0}};
+ NTSTATUS status;
+ union smb_ioctl ioctl;
+ struct smb2_request *req;
+ TALLOC_CTX *tmp_ctx = talloc_new(tree);
+ struct srv_copychunk_copy cc_copy;
+ enum ndr_err_code ndr_ret;
+ bool ok = true;
+
+ ok = neg_aapl_copyfile(torture, tree,
+ SMB2_CRTCTX_AAPL_SUPPORTS_OSX_COPYFILE);
+ torture_assert_goto(torture, ok == true, ok, done, "missing AAPL copyfile\n");
+
+ /*
+ * Create a BIG file so we have a good chance the copy is still in progress
+ * when the server gets our cancel.
+ */
+ ok = test_setup_copy_chunk(torture, tree, tmp_ctx,
+ 0, /* 0 chunks -> copyfile */
+ &src_h,
+ 512*1024*1024, /* 512 MB source file */
+ SEC_RIGHTS_FILE_ALL,
+ &dest_h,
+ 0, /* 0 byte dest file */
+ SEC_RIGHTS_FILE_ALL,
+ &cc_copy,
+ &ioctl);
+ torture_assert_goto(torture, ok == true, ok, done, "setup copy chunk error\n");
+
+ ndr_ret = ndr_push_struct_blob(
+ &ioctl.smb2.in.out, tmp_ctx, &cc_copy,
+ (ndr_push_flags_fn_t)ndr_push_srv_copychunk_copy);
+ torture_assert_ndr_success_goto(torture, ndr_ret, ok, done,
+ "ndr_push_srv_copychunk_copy\n");
+
+ req = smb2_ioctl_send(tree, &ioctl.smb2);
+ torture_assert_goto(torture, req != NULL, ok, done, "smb2_ioctl_send failed\n");
+
+ smb2_cancel(req);
+
+ status = smb2_ioctl_recv(req, tree, &ioctl.smb2);
+ torture_assert_ntstatus_equal_goto(torture, status, NT_STATUS_CANCELLED, ok, done,
+ "Cancel failed\n");
+
+done:
+ if (!smb2_util_handle_empty(src_h)) {
+ smb2_util_close(tree, src_h);
+ }
+ if (!smb2_util_handle_empty(dest_h)) {
+ smb2_util_close(tree, dest_h);
+ }
+ smb2_util_unlink(tree, FNAME_CC_SRC);
+ smb2_util_unlink(tree, FNAME_CC_DST);
+ talloc_free(tmp_ctx);
+
+ return ok;
+}
+
static bool check_stream_list(struct smb2_tree *tree,
struct torture_context *tctx,
const char *fname,
@@ -3950,8 +4012,8 @@ struct torture_suite *torture_vfs_fruit(void)
torture_suite_add_1smb2_test(suite, "delete", test_delete_file_with_rfork);
torture_suite_add_1smb2_test(suite, "read open rsrc after rename", test_rename_and_read_rsrc);
torture_suite_add_1smb2_test(suite, "readdir_attr with names with illegal ntfs characters", test_readdir_attr_illegal_ntfs);
-
torture_suite_add_2ns_smb2_test(suite, "invalid AFP_AfpInfo", test_invalid_afpinfo);
+ torture_suite_add_1smb2_test(suite, "copy_chunk_cancel", test_copy_chunk_cancel);
return suite;
}
--
2.9.3
More information about the samba-technical
mailing list