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

[Ralph Böhme]

Hi David,

On Mon, Apr 30, 2018 at 11:12:58AM +0200, David Disseldorp wrote:
> On Sat, 28 Apr 2018 16:12:07 +0200, Ralph Böhme wrote:
> 
> > 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.
> ...
> > > +	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);
> >         }
> > 
> > ?
> 
> We still need to retain the errno negation, so that'd be
> tevent_req_error(req, -ret)...

yep, but the -ret in tevent_req_error() looks somewhat strange, maybe this is
better:

        if (ret != 0) {
                /* ceph_fsync returns -errno on error. */
                tevent_req_error(req, -ret);
                return tevent_req_post(req, ev);
        }

> > Then this should be:
> > 
> >         if (tevent_req_is_unix_error(req, &vfs_aio_state->error)) {
> >                 return -1;
> 
> This would mean that we don't propagate up the call duration on failure.
> I'm not so familiar with the async VFS API, but is that to be expected?

vfs_ceph doesn't implement profiling so it doesn't really matter, cf the
SMBPROFILE_BYTES_ASYNC_* macros in vfs_default how it would look like.

So I guess you can either remove the PROFILE_TIMESTAMP macro invocations or
implement the profiling stuff.

-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