[PATCH] Add missing async fsync_send()/recv() code to ceph VFS.

[Ralph Böhme]

Hi Jeremy,

On Fri, Apr 27, 2018 at 04:07:43PM -0700, Jeremy Allison wrote:
> Patch that implements fsync_send()/recv() by cheating
> and using ceph_fsync() synchronously under the covers
> attached.
> 
> Please review and push if happy ! I need this as a
> prerequisite for my other work (ceph is also missing
> async pread/pwrite but that's a patch for another day :-).

few nitpicks:

> diff --git a/source3/modules/vfs_ceph.c b/source3/modules/vfs_ceph.c
> index d61213110e0..cbe5fac1f89 100644
> --- a/source3/modules/vfs_ceph.c
> +++ b/source3/modules/vfs_ceph.c
> @@ -569,6 +569,62 @@ static int cephwrap_fsync(struct vfs_handle_struct *handle, files_struct *fsp)
>  	WRAP_RETURN(result);
>  }
>  
> +/*
> + * Fake up an async ceph fsync by calling the sychronous API.
> + */
> +
> +static struct tevent_req *cephwrap_fsync_send(struct vfs_handle_struct *handle,
> +					TALLOC_CTX *mem_ctx,
> +					struct tevent_context *ev,
> +					files_struct *fsp)
> +{
> +	struct tevent_req *req = NULL;
> +	struct vfs_aio_state *state = NULL;
> +	struct timespec start_time = { 0 };
> +	struct timespec end_time = { 0 };
> +	int ret = -1;
> +
> +	DBG_DEBUG("[CEPH] cephwrap_fsync_send\n");
> +
> +	req = tevent_req_create(mem_ctx, &state, struct vfs_aio_state);
> +	if (req == NULL) {
> +		return NULL;
> +	}
> +
> +	PROFILE_TIMESTAMP(&start_time);
> +
> +	/* Make sync call. */
> +	ret = ceph_fsync(handle->data, fsp->fh->fd, false);
> +
> +	if (ret != 0) {
> +		/* ceph_fsync returns -errno on error. */
> +		state->error = -ret;
> +	}

Shouldn't this be:

        if (tevent_req_error(req, ret)) {
                return tevent_req_post(req, ev);
        }

?

> +
> +	PROFILE_TIMESTAMP(&end_time);
> +	state->duration = nsec_time_diff(&end_time, &start_time);
> +
> +	/* Mark it as done. */
> +	tevent_req_done(req);
> +	/* Return and schedule the completion of the call. */
> +	return tevent_req_post(req, ev);
> +}
> +
> +static int cephwrap_fsync_recv(struct tevent_req *req,
> +				struct vfs_aio_state *vfs_aio_state)
> +{
> +	struct vfs_aio_state *state =
> +		tevent_req_data(req, struct vfs_aio_state);
> +
> +	DBG_DEBUG("[CEPH] cephwrap_fsync_recv\n");
> +
> +	*vfs_aio_state = *state;
> +	if (state->error != 0) {
> +		return -1;
> +	}

Then this should be:

        if (tevent_req_is_unix_error(req, &vfs_aio_state->error)) {
                return -1;
        }
        *vfs_aio_state = *state;

> +	return 0;
> +}
> +
>  #ifdef HAVE_CEPH_STATX
>  #define SAMBA_STATX_ATTR_MASK	(CEPH_STATX_BASIC_STATS|CEPH_STATX_BTIME)
>  
> @@ -1440,6 +1496,8 @@ static struct vfs_fn_pointers ceph_fns = {
>  	.recvfile_fn = cephwrap_recvfile,
>  	.rename_fn = cephwrap_rename,
>  	.fsync_fn = cephwrap_fsync,
> +        .fsync_send_fn = cephwrap_fsync_send,
> +        .fsync_recv_fn = cephwrap_fsync_recv,

This uses spaces for indentation.

Otherwise looks good.

-slow

-- 
Ralph Boehme, Samba Team       https://samba.org/
Samba Developer, SerNet GmbH   https://sernet.de/en/samba/
GPG Key Fingerprint:           FAE2 C608 8A24 2520 51C5
                               59E4 AA1E 9B71 2639 9E46