[PATCH 04/12] s3-vfs: add copy_chunk vfs hooks

David Disseldorp ddiss at suse.de
Thu Nov 8 16:19:14 MST 2012


Hi Metze,

On Thu, 08 Nov 2012 15:21:50 +0100
"Stefan (metze) Metzmacher" <metze at samba.org> wrote:

> > +static struct tevent_req *skel_copy_chunk_send(struct vfs_handle_struct *handle,
> > +					       TALLOC_CTX *mem_ctx,
> > +					       struct tevent_context *ev,
> > +					       struct files_struct *src_fsp,
> > +					       off_t src_off,
> > +					       struct files_struct *dest_fsp,
> > +					       off_t dest_off,
> > +					       off_t num)
> > +{
> > +	return NULL;
> > +}
> > +
> > +static NTSTATUS skel_copy_chunk_recv(struct vfs_handle_struct *handle,
> > +				     struct tevent_req *req,
> > +				     off_t *copied)
> > +{
> > +	return NT_STATUS_NOT_IMPLEMENTED;
> > +}
> > +
> 
> Can you implement this as complete tevent_req function please?
> The _send() function should only return NULL if tevent_req_create() fails.

Will do.

> 
> >  static NTSTATUS skel_streaminfo(struct vfs_handle_struct *handle,
> >  				struct files_struct *fsp,
> >  				const char *fname,
> > @@ -825,6 +844,8 @@ struct vfs_fn_pointers skel_opaque_fns = {
> >  	.notify_watch_fn = skel_notify_watch,
> >  	.chflags_fn = skel_chflags,
> >  	.file_id_create_fn = skel_file_id_create,
> > +	.copy_chunk_send_fn = skel_copy_chunk_send,
> > +	.copy_chunk_recv_fn = skel_copy_chunk_recv,
> >  
> >  	.streaminfo_fn = skel_streaminfo,
> >  	.get_real_filename_fn = skel_get_real_filename,
> > diff --git a/examples/VFS/skel_transparent.c b/examples/VFS/skel_transparent.c
> > index 02e8184..4bd659f 100644
> > --- a/examples/VFS/skel_transparent.c
> > +++ b/examples/VFS/skel_transparent.c
> > @@ -572,6 +572,26 @@ static struct file_id skel_file_id_create(vfs_handle_struct *handle,
> >  	return SMB_VFS_NEXT_FILE_ID_CREATE(handle, sbuf);
> >  }
> >  
> > +static struct tevent_req *skel_copy_chunk_send(struct vfs_handle_struct *handle,
> > +					       TALLOC_CTX *mem_ctx,
> > +					       struct tevent_context *ev,
> > +					       struct files_struct *src_fsp,
> > +					       off_t src_off,
> > +					       struct files_struct *dest_fsp,
> > +					       off_t dest_off,
> > +					       off_t num)
> > +{
> > +	return SMB_VFS_NEXT_COPY_CHUNK_SEND(handle, mem_ctx, ev, src_fsp, src_off,
> > +					    dest_fsp, dest_off, num);
> > +}
> > +
> > +static NTSTATUS skel_copy_chunk_recv(struct vfs_handle_struct *handle,
> > +				     struct tevent_req *req,
> > +				     off_t *copied)
> > +{
> > +	return SMB_VFS_NEXT_COPY_CHUNK_RECV(handle, req, copied);
> > +}
> 
> We should also implement a correct tevent_req layering with a subreq here.

Would seem to be over-abstraction for example code, but I'll add this
nevertheless.

> 
> >  static NTSTATUS skel_streaminfo(struct vfs_handle_struct *handle,
> >  				struct files_struct *fsp,
> >  				const char *fname,
> > @@ -898,6 +918,8 @@ struct vfs_fn_pointers skel_transparent_fns = {
> >  	.notify_watch_fn = skel_notify_watch,
> >  	.chflags_fn = skel_chflags,
> >  	.file_id_create_fn = skel_file_id_create,
> > +	.copy_chunk_send_fn = skel_copy_chunk_send,
> > +	.copy_chunk_recv_fn = skel_copy_chunk_recv,
> >  
> >  	.streaminfo_fn = skel_streaminfo,
> >  	.get_real_filename_fn = skel_get_real_filename,
> > diff --git a/source3/include/vfs.h b/source3/include/vfs.h
> > index 2992c1d..2805c1e 100644
> > --- a/source3/include/vfs.h
> > +++ b/source3/include/vfs.h
> > @@ -147,6 +147,7 @@
> >  /* Bump to version 30 - Samba 4.0.0 will ship with interface version 30 */
> >  /* Leave at 30 - not yet released. Added conn->cwd to save vfs_GetWd() calls. */
> >  /* Leave at 30 - not yet released. Changed sys_acl_blob_get_file interface to remove type */
> > +/* Leave at 30 - not yet released. add SMB_VFS_COPY_CHUNK() */
> >  #define SMB_VFS_INTERFACE_VERSION 30
> >  
> >  /*
> > @@ -609,6 +610,17 @@ struct vfs_fn_pointers {
> >  	int (*chflags_fn)(struct vfs_handle_struct *handle, const char *path, unsigned int flags);
> >  	struct file_id (*file_id_create_fn)(struct vfs_handle_struct *handle,
> >  					    const SMB_STRUCT_STAT *sbuf);
> > +	struct tevent_req *(*copy_chunk_send_fn)(struct vfs_handle_struct *handle,
> > +						 TALLOC_CTX *mem_ctx,
> > +						 struct tevent_context *ev,
> > +						 struct files_struct *src_fsp,
> > +						 off_t src_off,
> > +						 struct files_struct *dest_fsp,
> > +						 off_t dest_off,
> > +						 off_t num);
> > +	NTSTATUS (*copy_chunk_recv_fn)(struct vfs_handle_struct *handle,
> > +				       struct tevent_req *req,
> > +				       off_t *copied);
> >  
> >  	NTSTATUS (*streaminfo_fn)(struct vfs_handle_struct *handle,
> >  				  struct files_struct *fsp,
> > @@ -1080,7 +1092,18 @@ NTSTATUS smb_vfs_call_fsctl(struct vfs_handle_struct *handle,
> >  			    uint32_t in_len,
> >  			    uint8_t **_out_data,
> >  			    uint32_t max_out_len,
> > -			    uint32_t *out_len); 
> > +			    uint32_t *out_len);
> > +struct tevent_req *smb_vfs_call_copy_chunk_send(struct vfs_handle_struct *handle,
> > +						TALLOC_CTX *mem_ctx,
> > +						struct tevent_context *ev,
> > +						struct files_struct *src_fsp,
> > +						off_t src_off,
> > +						struct files_struct *dest_fsp,
> > +						off_t dest_off,
> > +						off_t num);
> > +NTSTATUS smb_vfs_call_copy_chunk_recv(struct vfs_handle_struct *handle,
> > +				      struct tevent_req *req,
> > +				      off_t *copied);
> >  NTSTATUS smb_vfs_call_fget_nt_acl(struct vfs_handle_struct *handle,
> >  				  struct files_struct *fsp,
> >  				  uint32 security_info,
> > diff --git a/source3/include/vfs_macros.h b/source3/include/vfs_macros.h
> > index 331fe00..364a4ca 100644
> > --- a/source3/include/vfs_macros.h
> > +++ b/source3/include/vfs_macros.h
> > @@ -399,6 +399,16 @@
> >  #define SMB_VFS_NEXT_FSCTL(handle, fsp, ctx, function, req_flags, in_data, in_len, out_data, max_out_len, out_len) \
> >  	smb_vfs_call_fsctl((handle)->next, (fsp), (ctx), (function), (req_flags), (in_data), (in_len), (out_data), (max_out_len), (out_len))
> >  
> > +#define SMB_VFS_COPY_CHUNK_SEND(conn, mem_ctx, ev, src_fsp, src_off, dest_fsp, dest_off, num) \
> > +	smb_vfs_call_copy_chunk_send((conn)->vfs_handles, (mem_ctx), (ev), (src_fsp), (src_off), (dest_fsp), (dest_off), (num))
> > +#define SMB_VFS_NEXT_COPY_CHUNK_SEND(handle, mem_ctx, ev, src_fsp, src_off, dest_fsp, dest_off, num) \
> > +	smb_vfs_call_copy_chunk_send((handle)->next, (mem_ctx), (ev), (src_fsp), (src_off), (dest_fsp), (dest_off), (num))
> > +
> > +#define SMB_VFS_COPY_CHUNK_RECV(conn, req, copied) \
> > +	smb_vfs_call_copy_chunk_recv((conn)->vfs_handles, (req), (copied))
> > +#define SMB_VFS_NEXT_COPY_CHUNK_RECV(handle, req, copied) \
> > +	smb_vfs_call_copy_chunk_recv((handle)->next, (req), (copied))
> > +
> >  #define SMB_VFS_FGET_NT_ACL(fsp, security_info, mem_ctx, ppdesc)		\
> >  		smb_vfs_call_fget_nt_acl((fsp)->conn->vfs_handles, (fsp), (security_info), (mem_ctx), (ppdesc))
> >  #define SMB_VFS_NEXT_FGET_NT_ACL(handle, fsp, security_info, mem_ctx, ppdesc) \
> > diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
> > index 0f651dc..2bac8f2 100644
> > --- a/source3/modules/vfs_default.c
> > +++ b/source3/modules/vfs_default.c
> > @@ -31,6 +31,7 @@
> >  #include "librpc/gen_ndr/ndr_dfsblobs.h"
> >  #include "lib/util/tevent_unix.h"
> >  #include "lib/asys/asys.h"
> > +#include "lib/util/tevent_ntstatus.h"
> >  
> >  #undef DBGC_CLASS
> >  #define DBGC_CLASS DBGC_VFS
> > @@ -1323,6 +1324,74 @@ static NTSTATUS vfswrap_fsctl(struct vfs_handle_struct *handle,
> >  	return NT_STATUS_NOT_SUPPORTED;
> >  }
> >  
> > +struct vfs_cc_state {
> > +	off_t copied;
> > +};
> > +
> > +static struct tevent_req *vfswrap_copy_chunk_send(struct vfs_handle_struct *handle,
> > +						  TALLOC_CTX *mem_ctx,
> > +						  struct tevent_context *ev,
> > +						  struct files_struct *src_fsp,
> > +						  off_t src_off,
> > +						  struct files_struct *dest_fsp,
> > +						  off_t dest_off,
> > +						  off_t num)
> > +{
> > +	struct tevent_req *req;
> > +	struct vfs_cc_state *vfs_cc_state;
> > +
> > +	DEBUG(10, ("performing server side copy chunk of length %lu\n",
> > +		   (unsigned long)num));
> > +
> > +	req = tevent_req_create(mem_ctx, &vfs_cc_state, struct vfs_cc_state);
> > +	if (req == NULL) {
> > +		return NULL;
> > +	}
> > +
> > +	/* everything is done synchronously for now */
> > +	if (SMB_VFS_LSEEK(src_fsp, src_off, SEEK_SET) != src_off) {
> > +		tevent_req_nterror(req, map_nt_error_from_unix(errno));
> > +		return tevent_req_post(req, ev);
> > +	}
> > +	if (SMB_VFS_LSEEK(dest_fsp, dest_off, SEEK_SET) != dest_off) {
> > +		tevent_req_nterror(req, map_nt_error_from_unix(errno));
> > +		return tevent_req_post(req, ev);
> > +	}
> > +
> > +	vfs_cc_state->copied = vfs_transfer_file(src_fsp, dest_fsp, num);
> > +	if (vfs_cc_state->copied == -1) {
> > +		tevent_req_nterror(req, NT_STATUS_IO_DEVICE_ERROR);
> > +		return tevent_req_post(req, ev);
> > +	}
> > +
> > +	tevent_req_done(req);
> > +	return tevent_req_post(req, ev);
> > +}
> > +
> > +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);
> > +	NTSTATUS status;
> > +
> > +	if (tevent_req_is_nterror(req, &status)) {
> > +		DEBUG(2, ("server side 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));
> > +	tevent_req_received(req);
> > +
> > +	return NT_STATUS_OK;
> > +}
> > +
> >  /********************************************************************
> >   Given a stat buffer return the allocated size on disk, taking into
> >   account sparse files.
> > @@ -2367,6 +2436,8 @@ static struct vfs_fn_pointers vfs_default_fns = {
> >  	.strict_unlock_fn = vfswrap_strict_unlock,
> >  	.translate_name_fn = vfswrap_translate_name,
> >  	.fsctl_fn = vfswrap_fsctl,
> > +	.copy_chunk_send_fn = vfswrap_copy_chunk_send,
> > +	.copy_chunk_recv_fn = vfswrap_copy_chunk_recv,
> >  
> >  	/* NT ACL operations. */
> >  
> > diff --git a/source3/modules/vfs_full_audit.c b/source3/modules/vfs_full_audit.c
> > index b1fb090..549f55e 100644
> > --- a/source3/modules/vfs_full_audit.c
> > +++ b/source3/modules/vfs_full_audit.c
> > @@ -161,6 +161,8 @@ typedef enum _vfs_op_type {
> >  	SMB_VFS_OP_STRICT_LOCK,
> >  	SMB_VFS_OP_STRICT_UNLOCK,
> >  	SMB_VFS_OP_TRANSLATE_NAME,
> > +	SMB_VFS_OP_COPY_CHUNK_SEND,
> > +	SMB_VFS_OP_COPY_CHUNK_RECV,
> >  
> >  	/* NT ACL operations. */
> >  
> > @@ -281,6 +283,8 @@ static struct {
> >  	{ SMB_VFS_OP_STRICT_LOCK, "strict_lock" },
> >  	{ SMB_VFS_OP_STRICT_UNLOCK, "strict_unlock" },
> >  	{ SMB_VFS_OP_TRANSLATE_NAME,	"translate_name" },
> > +	{ SMB_VFS_OP_COPY_CHUNK_SEND,	"copy_chunk_send" },
> > +	{ SMB_VFS_OP_COPY_CHUNK_RECV,	"copy_chunk_recv" },
> >  	{ SMB_VFS_OP_FGET_NT_ACL,	"fget_nt_acl" },
> >  	{ SMB_VFS_OP_GET_NT_ACL,	"get_nt_acl" },
> >  	{ SMB_VFS_OP_FSET_NT_ACL,	"fset_nt_acl" },
> > @@ -1732,6 +1736,38 @@ static NTSTATUS smb_full_audit_translate_name(struct vfs_handle_struct *handle,
> >  	return result;
> >  }
> >  
> > +static struct tevent_req *smb_full_audit_copy_chunk_send(struct vfs_handle_struct *handle,
> > +							 TALLOC_CTX *mem_ctx,
> > +							 struct tevent_context *ev,
> > +							 struct files_struct *src_fsp,
> > +							 off_t src_off,
> > +							 struct files_struct *dest_fsp,
> > +							 off_t dest_off,
> > +							 off_t num)
> > +{
> > +	struct tevent_req *req;
> > +
> > +	req = SMB_VFS_NEXT_COPY_CHUNK_SEND(handle, mem_ctx, ev, src_fsp,
> > +					   src_off, dest_fsp, dest_off, num);
> > +
> > +	do_log(SMB_VFS_OP_COPY_CHUNK_SEND, req, handle, "");
> > +
> > +	return req;
> > +}
> > +
> > +static NTSTATUS smb_full_audit_copy_chunk_recv(struct vfs_handle_struct *handle,
> > +					       struct tevent_req *req,
> > +					       off_t *copied)
> > +{
> > +	NTSTATUS result;
> > +
> > +	result = SMB_VFS_NEXT_COPY_CHUNK_RECV(handle, req, copied);
> > +
> > +	do_log(SMB_VFS_OP_COPY_CHUNK_RECV, NT_STATUS_IS_OK(result), handle, "");
> > +
> > +	return result;
> > +}
> > +
> >  static NTSTATUS smb_full_audit_fget_nt_acl(vfs_handle_struct *handle, files_struct *fsp,
> >  					   uint32 security_info,
> >  					   TALLOC_CTX *mem_ctx,
> > @@ -2131,6 +2167,8 @@ static struct vfs_fn_pointers vfs_full_audit_fns = {
> >  	.strict_lock_fn = smb_full_audit_strict_lock,
> >  	.strict_unlock_fn = smb_full_audit_strict_unlock,
> >  	.translate_name_fn = smb_full_audit_translate_name,
> > +	.copy_chunk_send_fn = smb_full_audit_copy_chunk_send,
> > +	.copy_chunk_recv_fn = smb_full_audit_copy_chunk_recv,
> >  	.fget_nt_acl_fn = smb_full_audit_fget_nt_acl,
> >  	.get_nt_acl_fn = smb_full_audit_get_nt_acl,
> >  	.fset_nt_acl_fn = smb_full_audit_fset_nt_acl,
> > diff --git a/source3/modules/vfs_time_audit.c b/source3/modules/vfs_time_audit.c
> > index 95b4148..73ee59c 100644
> > --- a/source3/modules/vfs_time_audit.c
> > +++ b/source3/modules/vfs_time_audit.c
> > @@ -29,6 +29,7 @@
> >  #include "smbd/smbd.h"
> >  #include "ntioctl.h"
> >  #include "lib/util/tevent_unix.h"
> > +#include "lib/util/tevent_ntstatus.h"
> >  
> >  #undef DBGC_CLASS
> >  #define DBGC_CLASS DBGC_VFS
> > @@ -1668,6 +1669,90 @@ static NTSTATUS smb_time_audit_translate_name(struct vfs_handle_struct *handle,
> >  	return result;
> >  }
> >  
> > +struct time_audit_cc_state {
> > +	struct timespec ts_send;
> > +	struct vfs_handle_struct *handle;
> > +	off_t copied;
> > +	struct tevent_req *subreq;
> > +};
> 
> We don't need to remember subreq here.

Indeed.

> 
> > +static void smb_time_audit_copy_chunk_done(struct tevent_req *subreq);
> > +
> > +static struct tevent_req *smb_time_audit_copy_chunk_send(struct vfs_handle_struct *handle,
> > +							 TALLOC_CTX *mem_ctx,
> > +							 struct tevent_context *ev,
> > +							 struct files_struct *src_fsp,
> > +							 off_t src_off,
> > +							 struct files_struct *dest_fsp,
> > +							 off_t dest_off,
> > +							 off_t num)
> > +{
> > +	struct tevent_req *req;
> > +	struct time_audit_cc_state *cc_state;
> > +
> > +	req = tevent_req_create(mem_ctx, &cc_state, struct time_audit_cc_state);
> > +	if (req == NULL) {
> > +		return NULL;
> > +	}
> > +
> > +	cc_state->handle = handle;
> > +	clock_gettime_mono(&cc_state->ts_send);
> > +	cc_state->subreq = SMB_VFS_NEXT_COPY_CHUNK_SEND(handle, cc_state, ev,
> > +							src_fsp, src_off,
> > +							dest_fsp, dest_off,
> > +							num);
> > +	if (tevent_req_nomem(cc_state->subreq, req)) {
> > +		return tevent_req_post(req, ev);
> > +	}
> > +
> > +	tevent_req_set_callback(cc_state->subreq,
> > +				smb_time_audit_copy_chunk_done,
> > +				req);
> > +	return req;
> > +}
> > +
> > +static void smb_time_audit_copy_chunk_done(struct tevent_req *subreq)
> > +{
> > +	struct tevent_req *req = tevent_req_callback_data(
> > +		subreq, struct tevent_req);
> > +	struct time_audit_cc_state *cc_state
> > +			= tevent_req_data(req, struct time_audit_cc_state);
> > +	NTSTATUS status;
> > +
> > +	status = SMB_VFS_NEXT_COPY_CHUNK_RECV(cc_state->handle,
> > +					      cc_state->subreq,
> > +					      &cc_state->copied);
> 
> Here we can use 'subreq' as it's the same as cc_state->subreq.
> Also TALLOC_FREE(subreq); should be typically called after
> the _recv() function.

I'll change this over, thanks for the feedback Metze!

Cheers, David


More information about the samba-technical mailing list