On 2006-2-10 Mike Ashmore <mike@motomike.net> wrote:
> On Feb 9, 2006, at 7:32 AM, Malcolm Rowe wrote:
>
> > On Wed, Feb 08, 2006 at 04:35:35PM -0500, Mike Ashmore wrote:
> >> Unfortunately, Malcolm's patch still doesn't fix STOP. Using strace,
> >> it looks like the problem occurs when you open("<repository>/db/
> >> revprops", O_RDONLY). The fsync immediately after that operation
> >> fails.
> >>
> >
> > Okay. I'm not entirely surprised, though it seems odd that
> > STOP provides a close-enough Linux ABI to define __linux__,
> > but not one that supports the well-known mechanism for
> > flushing directory modifications to disk.
> >
> > I presume that prior to this patch, the fsync that failed was the one
> > sync-ing the revision file (/db/revs/0)? If so, I'll commit the patch
> > I provided anyway - it does make sense by itself.
> >
> > What error does fsync() return in the failing case?
> > (syncing the directory) I suppose we could ignore EROFS or
> > EINVAL, or possibly even EBADF, since those errors don't
> > really indicate a failure to sync the directory per se
> > (that would be EIO, I think), more an inability to sync a
> > directory in the first place, which isn't something we
> > should fail on.
>
> As to STOP's oddness, there's no denying that. The fsync
> implementation is probably somehow related to the fact that there are
> a ridiculous number of security checks on all filesystem operations
> to ensure information remains properly compartmentalized.
>
> At any rate. The failing fsync, it turns out, is the same with or
> without your patch: <repository>/db/revprops. It's apparently synced
> even before /db/revs/0, and of course as soon as it fails the whole
> process bails out. Note that I'm running 1.3.0, as opposed to the
> trunk release. Oh, and I compiled without BDB support (I trust fsfs
> more anyway), and threading disabled (another STOP thing, pthreads
> aren't allowed). The return code I get for the fsync is EBADF (-1).
> Tricky.
Ideally STOP would set EISDIR or EINVAL in that case and the
code could check for that. EBADF is typically used when a fd
has been closed or is otherwise non-opened.
What about a configure test to see if fsyncing a dir actually
works, and the ifdef in svn_fs_fs__move_into_place can then be:
#if defined(__linux__) && defined(SVN_CAN_FSYNC_DIR)
(it only has #ifdef __linux__ currently). Subversion-With-Space
could use something similar.
I don't think a run-time check on EBADF is superior since it
could mask genuine bugs where a dir fd was closed on the way to
svn_io_file_flush_to_disk. Presumably STOP needs its own native
build anyway-- it (apparently) can't just use the redhat or
fedora build.
-- bart
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Feb 10 23:10:56 2006