From 2d01558ad3d9452c8c64a30e5bf143b4bdc3e207 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Sat, 10 Jun 2017 09:05:55 +0200 Subject: [PATCH] s3/smbd: remove unneeded flags argument from SMB_VFS_OFFLOAD_WRITE_SEND ...and instead use the fsctl to infer required behaviour in the VFS backends. Note that this removes the check from vfs_default because there we only handle FSCTL_SRV_COPYCHUNK(_WRITE) and must always perform the lock checks. Signed-off-by: Ralph Boehme Reviewed-by: Stefan Metzmacher --- examples/VFS/skel_opaque.c | 3 +- examples/VFS/skel_transparent.c | 5 +- source3/include/vfs.h | 20 +----- source3/include/vfs_macros.h | 8 +-- source3/modules/vfs_btrfs.c | 21 +++---- source3/modules/vfs_default.c | 91 +++++++++++----------------- source3/modules/vfs_fruit.c | 6 +- source3/modules/vfs_full_audit.c | 6 +- source3/modules/vfs_time_audit.c | 5 +- source3/smbd/smb2_ioctl_filesys.c | 4 +- source3/smbd/smb2_ioctl_network_fs.c | 3 +- source3/smbd/vfs.c | 6 +- 12 files changed, 66 insertions(+), 112 deletions(-) diff --git a/examples/VFS/skel_opaque.c b/examples/VFS/skel_opaque.c index 831d0f14fce5..fed9d2f3ddb3 100644 --- a/examples/VFS/skel_opaque.c +++ b/examples/VFS/skel_opaque.c @@ -590,8 +590,7 @@ static struct tevent_req *skel_offload_write_send(struct vfs_handle_struct *hand off_t transfer_offset, struct files_struct *dest_fsp, off_t dest_off, - off_t num, - uint32_t flags) + off_t num) { struct tevent_req *req; struct skel_cc_state *cc_state; diff --git a/examples/VFS/skel_transparent.c b/examples/VFS/skel_transparent.c index f813926141bf..d9123e091c14 100644 --- a/examples/VFS/skel_transparent.c +++ b/examples/VFS/skel_transparent.c @@ -724,8 +724,7 @@ static struct tevent_req *skel_offload_write_send(struct vfs_handle_struct *hand off_t transfer_offset, struct files_struct *dest_fsp, off_t dest_off, - off_t num, - uint32_t flags) + off_t num) { struct tevent_req *req; struct tevent_req *subreq; @@ -739,7 +738,7 @@ static struct tevent_req *skel_offload_write_send(struct vfs_handle_struct *hand state->handle = handle; subreq = SMB_VFS_NEXT_OFFLOAD_WRITE_SEND(handle, state, ev, fsctl, token, transfer_offset, - dest_fsp, dest_off, num, flags); + dest_fsp, dest_off, num); if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } diff --git a/source3/include/vfs.h b/source3/include/vfs.h index b651baef0e23..7b9a6f8f0857 100644 --- a/source3/include/vfs.h +++ b/source3/include/vfs.h @@ -592,20 +592,6 @@ enum vfs_fallocate_flags { VFS_FALLOCATE_FL_PUNCH_HOLE = 0x0002, }; -/* - * @VFS_OFFLOAD_WRITE_FL_MUST_CLONE: indicates that offload_write_send_fn() copy must - * be handled as a COW clone, AKA reflink. - * @VFS_OFFLOAD_WRITE_FL_MASK_ALL: all valid flags. - */ -enum vfs_offload_write_flags { - VFS_OFFLOAD_WRITE_FL_MUST_CLONE = 0x0001, - VFS_OFFLOAD_WRITE_FL_IGNORE_LOCKS = 0x0002, - - VFS_OFFLOAD_WRITE_FL_MASK_ALL = - (VFS_OFFLOAD_WRITE_FL_MUST_CLONE - | VFS_OFFLOAD_WRITE_FL_IGNORE_LOCKS), -}; - struct vfs_aio_state { int error; uint64_t duration; @@ -803,8 +789,7 @@ struct vfs_fn_pointers { off_t transfer_offset, struct files_struct *dest_fsp, off_t dest_off, - off_t to_copy, - uint32_t flags); + off_t to_copy); NTSTATUS (*offload_write_recv_fn)(struct vfs_handle_struct *handle, struct tevent_req *req, off_t *copied); @@ -1384,8 +1369,7 @@ struct tevent_req *smb_vfs_call_offload_write_send(struct vfs_handle_struct *han off_t transfer_offset, struct files_struct *dest_fsp, off_t dest_off, - off_t num, - uint32_t flags); + off_t num); NTSTATUS smb_vfs_call_offload_write_recv(struct vfs_handle_struct *handle, struct tevent_req *req, off_t *copied); diff --git a/source3/include/vfs_macros.h b/source3/include/vfs_macros.h index 77a2a8d37a22..69fa85b297b7 100644 --- a/source3/include/vfs_macros.h +++ b/source3/include/vfs_macros.h @@ -425,10 +425,10 @@ #define SMB_VFS_NEXT_OFFLOAD_READ_RECV(req, handle, mem_ctx, token_blob) \ smb_vfs_call_offload_read_recv((req), (handle)->next, (mem_ctx), (token_blob)) -#define SMB_VFS_OFFLOAD_WRITE_SEND(conn, mem_ctx, ev, fsctl, token, transfer_offset, dest_fsp, dest_off, num, flags) \ - smb_vfs_call_offload_write_send((conn)->vfs_handles, (mem_ctx), (ev), (fsctl), (token), (transfer_offset), (dest_fsp), (dest_off), (num), (flags)) -#define SMB_VFS_NEXT_OFFLOAD_WRITE_SEND(handle, mem_ctx, ev, fsctl, token, transfer_offset, dest_fsp, dest_off, num, flags) \ - smb_vfs_call_offload_write_send((handle)->next, (mem_ctx), (ev), (fsctl), (token), (transfer_offset), (dest_fsp), (dest_off), (num), (flags)) +#define SMB_VFS_OFFLOAD_WRITE_SEND(conn, mem_ctx, ev, fsctl, token, transfer_offset, dest_fsp, dest_off, num) \ + smb_vfs_call_offload_write_send((conn)->vfs_handles, (mem_ctx), (ev), (fsctl), (token), (transfer_offset), (dest_fsp), (dest_off), (num)) +#define SMB_VFS_NEXT_OFFLOAD_WRITE_SEND(handle, mem_ctx, ev, fsctl, token, transfer_offset, dest_fsp, dest_off, num) \ + smb_vfs_call_offload_write_send((handle)->next, (mem_ctx), (ev), (fsctl), (token), (transfer_offset), (dest_fsp), (dest_off), (num)) #define SMB_VFS_OFFLOAD_WRITE_RECV(conn, req, copied) \ smb_vfs_call_offload_write_recv((conn)->vfs_handles, (req), (copied)) diff --git a/source3/modules/vfs_btrfs.c b/source3/modules/vfs_btrfs.c index f310a3dfd658..0fd4c6cb0d86 100644 --- a/source3/modules/vfs_btrfs.c +++ b/source3/modules/vfs_btrfs.c @@ -211,8 +211,7 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han off_t transfer_offset, struct files_struct *dest_fsp, off_t dest_off, - off_t num, - uint32_t flags) + off_t num) { struct tevent_req *req; struct btrfs_cc_state *cc_state; @@ -223,6 +222,7 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han files_struct *src_fsp = NULL; int ret; bool handle_offload_write = true; + bool do_locking = false; NTSTATUS status; req = tevent_req_create(mem_ctx, &cc_state, struct btrfs_cc_state); @@ -230,11 +230,6 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han return NULL; } - if (flags & ~VFS_OFFLOAD_WRITE_FL_MASK_ALL) { - tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); - return tevent_req_post(req, ev); - } - cc_state->handle = handle; status = vfs_offload_token_db_fetch_fsp(btrfs_offload_ctx, @@ -246,7 +241,11 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han switch (fsctl) { case FSCTL_SRV_COPYCHUNK: case FSCTL_SRV_COPYCHUNK_WRITE: + do_locking = true; + break; + case FSCTL_DUP_EXTENTS_TO_FILE: + /* dup extents does not use locking */ break; default: @@ -271,7 +270,7 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han transfer_offset, dest_fsp, dest_off, - num, flags); + num); if (tevent_req_nomem(cc_state->subreq, req)) { return tevent_req_post(req, ev); } @@ -304,7 +303,7 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han return tevent_req_post(req, ev); } - if (!(flags & VFS_OFFLOAD_WRITE_FL_IGNORE_LOCKS)) { + if (do_locking) { init_strict_lock_struct(src_fsp, src_fsp->op->global->open_persistent_id, src_off, @@ -336,7 +335,7 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han cr_args.src_length = (uint64_t)num; ret = ioctl(dest_fsp->fh->fd, BTRFS_IOC_CLONE_RANGE, &cr_args); - if (!(flags & VFS_OFFLOAD_WRITE_FL_IGNORE_LOCKS)) { + if (do_locking) { SMB_VFS_STRICT_UNLOCK(dest_fsp->conn, dest_fsp, &dest_lck); SMB_VFS_STRICT_UNLOCK(src_fsp->conn, src_fsp, &src_lck); } @@ -361,7 +360,7 @@ static struct tevent_req *btrfs_offload_write_send(struct vfs_handle_struct *han transfer_offset, dest_fsp, dest_off, - num, flags); + num); if (tevent_req_nomem(cc_state->subreq, req)) { return tevent_req_post(req, ev); } diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c index 9add9155f12a..111a0c3aa712 100644 --- a/source3/modules/vfs_default.c +++ b/source3/modules/vfs_default.c @@ -1697,7 +1697,6 @@ struct vfswrap_offload_write_state { off_t to_copy; off_t remaining; size_t next_io_size; - uint32_t flags; }; static NTSTATUS vfswrap_offload_write_loop(struct tevent_req *req); @@ -1711,8 +1710,7 @@ static struct tevent_req *vfswrap_offload_write_send( off_t transfer_offset, struct files_struct *dest_fsp, off_t dest_off, - off_t to_copy, - uint32_t flags) + off_t to_copy) { struct tevent_req *req; struct vfswrap_offload_write_state *state = NULL; @@ -1726,17 +1724,6 @@ static struct tevent_req *vfswrap_offload_write_send( return NULL; } - if (flags & ~VFS_OFFLOAD_WRITE_FL_MASK_ALL) { - tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); - return tevent_req_post(req, ev); - } - - if (flags & VFS_OFFLOAD_WRITE_FL_MUST_CLONE) { - DEBUG(10, ("COW clones not supported by vfs_default\n")); - tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); - return tevent_req_post(req, ev); - } - *state = (struct vfswrap_offload_write_state) { .ev = ev, .token = token, @@ -1745,7 +1732,6 @@ static struct tevent_req *vfswrap_offload_write_send( .dst_off = dest_off, .to_copy = to_copy, .remaining = to_copy, - .flags = flags, }; switch (fsctl) { @@ -1757,6 +1743,11 @@ static struct tevent_req *vfswrap_offload_write_send( tevent_req_nterror(req, NT_STATUS_NOT_IMPLEMENTED); return tevent_req_post(req, ev); + case FSCTL_DUP_EXTENTS_TO_FILE: + DBG_DEBUG("COW clones not supported by vfs_default\n"); + tevent_req_nterror(req, NT_STATUS_INVALID_PARAMETER); + return tevent_req_post(req, ev); + default: tevent_req_nterror(req, NT_STATUS_INTERNAL_ERROR); return tevent_req_post(req, ev); @@ -1840,20 +1831,18 @@ static NTSTATUS vfswrap_offload_write_loop(struct tevent_req *req) state->next_io_size = MIN(state->remaining, talloc_array_length(state->buf)); - if (!(state->flags & VFS_OFFLOAD_WRITE_FL_IGNORE_LOCKS)) { - init_strict_lock_struct(state->src_fsp, + init_strict_lock_struct(state->src_fsp, state->src_fsp->op->global->open_persistent_id, - state->src_off, - state->next_io_size, - READ_LOCK, - &state->read_lck); - - ok = SMB_VFS_STRICT_LOCK(state->src_fsp->conn, - state->src_fsp, - &state->read_lck); - if (!ok) { - return NT_STATUS_FILE_LOCK_CONFLICT; - } + state->src_off, + state->next_io_size, + READ_LOCK, + &state->read_lck); + + ok = SMB_VFS_STRICT_LOCK(state->src_fsp->conn, + state->src_fsp, + &state->read_lck); + if (!ok) { + return NT_STATUS_FILE_LOCK_CONFLICT; } subreq = SMB_VFS_PREAD_SEND(state, @@ -1882,12 +1871,10 @@ static void vfswrap_offload_write_read_done(struct tevent_req *subreq) ssize_t nread; bool ok; - if (!(state->flags & VFS_OFFLOAD_WRITE_FL_IGNORE_LOCKS)) { - SMB_VFS_STRICT_UNLOCK(state->src_fsp->conn, - state->src_fsp, - &state->read_lck); - ZERO_STRUCT(state->read_lck); - } + SMB_VFS_STRICT_UNLOCK(state->src_fsp->conn, + state->src_fsp, + &state->read_lck); + ZERO_STRUCT(state->read_lck); nread = SMB_VFS_PREAD_RECV(subreq, &aio_state); TALLOC_FREE(subreq); @@ -1905,21 +1892,19 @@ static void vfswrap_offload_write_read_done(struct tevent_req *subreq) state->src_off += nread; - if (!(state->flags & VFS_OFFLOAD_WRITE_FL_IGNORE_LOCKS)) { - init_strict_lock_struct(state->dst_fsp, + init_strict_lock_struct(state->dst_fsp, state->dst_fsp->op->global->open_persistent_id, - state->dst_off, - state->next_io_size, - WRITE_LOCK, - &state->write_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; - } + state->dst_off, + state->next_io_size, + WRITE_LOCK, + &state->write_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; } subreq = SMB_VFS_PWRITE_SEND(state, @@ -1945,12 +1930,10 @@ static void vfswrap_offload_write_write_done(struct tevent_req *subreq) ssize_t nwritten; NTSTATUS status; - if (!(state->flags & VFS_OFFLOAD_WRITE_FL_IGNORE_LOCKS)) { - SMB_VFS_STRICT_UNLOCK(state->dst_fsp->conn, - state->dst_fsp, - &state->write_lck); - ZERO_STRUCT(state->write_lck); - } + SMB_VFS_STRICT_UNLOCK(state->dst_fsp->conn, + state->dst_fsp, + &state->write_lck); + ZERO_STRUCT(state->write_lck); nwritten = SMB_VFS_PWRITE_RECV(subreq, &aio_state); TALLOC_FREE(subreq); diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c index 36552c1fb94b..b49ee1cca12d 100644 --- a/source3/modules/vfs_fruit.c +++ b/source3/modules/vfs_fruit.c @@ -5479,8 +5479,7 @@ static struct tevent_req *fruit_offload_write_send(struct vfs_handle_struct *han off_t transfer_offset, struct files_struct *dest_fsp, off_t dest_off, - off_t num, - uint32_t flags) + off_t num) { struct tevent_req *req, *subreq; struct fruit_offload_write_state *state; @@ -5546,8 +5545,7 @@ static struct tevent_req *fruit_offload_write_send(struct vfs_handle_struct *han transfer_offset, dest_fsp, dest_off, - to_copy, - flags); + to_copy); if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } diff --git a/source3/modules/vfs_full_audit.c b/source3/modules/vfs_full_audit.c index 892cf5d73786..bd31fadec786 100644 --- a/source3/modules/vfs_full_audit.c +++ b/source3/modules/vfs_full_audit.c @@ -1949,15 +1949,13 @@ static struct tevent_req *smb_full_audit_offload_write_send(struct vfs_handle_st off_t transfer_offset, struct files_struct *dest_fsp, off_t dest_off, - off_t num, - uint32_t flags) + off_t num) { struct tevent_req *req; req = SMB_VFS_NEXT_OFFLOAD_WRITE_SEND(handle, mem_ctx, ev, fsctl, token, transfer_offset, - dest_fsp, dest_off, num, - flags); + dest_fsp, dest_off, num); do_log(SMB_VFS_OP_OFFLOAD_WRITE_SEND, req, handle, ""); diff --git a/source3/modules/vfs_time_audit.c b/source3/modules/vfs_time_audit.c index cc21a4762451..164f56da7ac9 100644 --- a/source3/modules/vfs_time_audit.c +++ b/source3/modules/vfs_time_audit.c @@ -2010,8 +2010,7 @@ static struct tevent_req *smb_time_audit_offload_write_send(struct vfs_handle_st off_t transfer_offset, struct files_struct *dest_fsp, off_t dest_off, - off_t num, - uint32_t flags) + off_t num) { struct tevent_req *req; struct tevent_req *subreq; @@ -2027,7 +2026,7 @@ static struct tevent_req *smb_time_audit_offload_write_send(struct vfs_handle_st clock_gettime_mono(&state->ts_send); subreq = SMB_VFS_NEXT_OFFLOAD_WRITE_SEND(handle, state, ev, fsctl, token, transfer_offset, - dest_fsp, dest_off, num, flags); + dest_fsp, dest_off, num); if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } diff --git a/source3/smbd/smb2_ioctl_filesys.c b/source3/smbd/smb2_ioctl_filesys.c index fa1cb93c4397..732e3ab96fcd 100644 --- a/source3/smbd/smb2_ioctl_filesys.c +++ b/source3/smbd/smb2_ioctl_filesys.c @@ -295,9 +295,7 @@ static void fsctl_dup_extents_offload_read_done(struct tevent_req *subreq) state->dup_extents.source_off, state->dst_fsp, state->dup_extents.target_off, - state->dup_extents.byte_count, - VFS_OFFLOAD_WRITE_FL_MUST_CLONE - | VFS_OFFLOAD_WRITE_FL_IGNORE_LOCKS); + state->dup_extents.byte_count); if (tevent_req_nomem(subreq, req)) { return; } diff --git a/source3/smbd/smb2_ioctl_network_fs.c b/source3/smbd/smb2_ioctl_network_fs.c index 993f6f8d6194..4006ccf3162e 100644 --- a/source3/smbd/smb2_ioctl_network_fs.c +++ b/source3/smbd/smb2_ioctl_network_fs.c @@ -200,8 +200,7 @@ static NTSTATUS fsctl_srv_copychunk_loop(struct tevent_req *req) source_off, state->dst_fsp, target_off, - length, - 0); + length); if (tevent_req_nomem(subreq, req)) { return NT_STATUS_NO_MEMORY; } diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c index c82496b94a45..67b79ef621b0 100644 --- a/source3/smbd/vfs.c +++ b/source3/smbd/vfs.c @@ -2382,14 +2382,12 @@ struct tevent_req *smb_vfs_call_offload_write_send(struct vfs_handle_struct *han off_t transfer_offset, struct files_struct *dest_fsp, off_t dest_off, - off_t num, - uint32_t flags) + off_t num) { VFS_FIND(offload_write_send); return handle->fns->offload_write_send_fn(handle, mem_ctx, ev, fsctl, token, transfer_offset, - dest_fsp, dest_off, num, - flags); + dest_fsp, dest_off, num); } NTSTATUS smb_vfs_call_offload_write_recv(struct vfs_handle_struct *handle, -- 2.34.1