darcs record fails on sshfs filesystems because sshfs does not (and
cannot, due to limitations of the underlying protocol, SFTP) implement
the atomic_create operation.  The attached transcript demonstrates the
failure.  For reference, I have also included analogous transcripts of
mercurial (hg) and git; the former works, the latter fails without
much explanation.

If it can be done easily and simply, I would like darcs to fall back
to whatever method hg uses (assuming it's safe) when atomic_create
isn't available.

Attachments

• typescript

I'm in no position to comment on this bug-report.  I do note that on issue554,
David says:

> You could try that, but sshfs is a pretty cruddy piece of work, and may not
> work very well.  It doesn't implement POSIX filesystem semantics.

Which makes think that this is wont-fix.

Juliusz: if you are around, perhaps you might have something to say about it? 
Otherwise, just ignore and I'll un-assign in you in a week

Please note that Darcs issues no warnings when using DARCS_SLOPPY_LOCKS=Yes 
over sshfs, and, from limited testing, seems to correctly block other instances 
from mutating the repository concurrently.  It would be convenient if darcs 
fell back to the sloppy locking mechanism in the presence of ENOSYS, as it 
already does in the presence of EOPNOTSUPP or EPERM.  In fact, I've attached a 
patch to do exactly that.  Thoughts?

Attachments

• darcs-sshfs-sloppy-locks.dpatch

> Please note that Darcs issues no warnings when using
> DARCS_SLOPPY_LOCKS=Yes over sshfs, and, from limited testing, seems
> to correctly block other instances from mutating the repository
> concurrently.

There's a race condition.  You won't be able to demonstrate it while
testing, but it wil trigger at some point.

If you run darcs over sshfs, your repository will get corrupted, your
computer will explode, your son will become a Wall Street trader and
your daughter will go work for Microsoft.

This warranty is void in case of nuclear war, whether caused by Darcs
or otherwise.

> It would be convenient if darcs fell back to the sloppy locking
> mechanism in the presence of ENOSYS, as it already does in the
> presence of EOPNOTSUPP or EPERM.  In fact, I've attached a patch to
> do exactly that.

I strongly oppose doing that.

                                        Juliusz

> There's a race condition.  You won't be able to demonstrate it while
> testing, but it wil trigger at some point.

Can you please be more verbose?  darcs already falls back to sloppy locks 
under several other circumstances, so what is intrinsically wrong with sshfs 
that induces a race where these other fallbacks don't?

>> There's a race condition.  You won't be able to demonstrate it while
>> testing, but it wil trigger at some point.

> Can you please be more verbose?  darcs already falls back to sloppy locks 
> under several other circumstances, so what is intrinsically wrong with sshfs 
> that induces a race where these other fallbacks don't?

open(O_CREAT|O_EXCL) is not atomic.

                                        Juliusz

I'm afraid I don't understand...  Looking at "man 2 open", I see:

> O_EXCL Ensure that this call creates the file: if this flag is specified in
> conjunction with O_CREAT, and pathname already exists, then open() will
> fail.

Moreover, The Open Group agrees 
(http://www.opengroup.org/onlinepubs/000095399/functions/open.html; IEEE 
1003.1, 2004 edition):

> O_EXCL
>  If O_CREAT and O_EXCL are set, open() shall fail if the file exists. The
>  check for the existence of the file and the creation of the file if it does
>  not exist shall be atomic with respect to other threads executing open()
>  naming the same filename in the same directory with O_EXCL and O_CREAT set.

There are some warnings about NFS in the man page, but this isn't NFS, so 
let's venture onwards; perhaps the SFTP definition is inadequate... Reading 
the SFTP spec (http://www.openssh.org/txt/draft-ietf-secsh-filexfer-02.txt), I 
see

>    SSH_FXF_EXCL
>      Causes the request to fail if the named file already exists.

OK, that looks fine.  How about sshfs?  If this isn't behaving correctly, then 
it's a bug in sshfs, given the above definition from the protocol.  Looking 
inside sshfs's code (v2.1), I see, inside sshfs.c:/^sshfs_open_common

> 2147         if (fi->flags & O_EXCL)
> 2148                 pflags |= SSH_FXF_EXCL;

Also looks fine... so perhaps the problem is in the server, but if so this 
surely would be a server bug.  Inside OpenSSH's code (CVS rev 1.84 of 
sftp-server.c), in :/^flags_from_portable

> 125  	if (pflags & SSH2_FXF_EXCL)
> 126        flags |= O_EXCL;

AFAICT, sshfs and openssh understand open(O_CREAT|O_EXCL) just fine and carry 
the O_EXCL bit across the SFTP transport faithfully; further, open(O_CREAT|
O_EXCL) is inherently an atomic operation, or you've found a bug in the host 
OS, not sshfs.  If there is a problem or race here, I believe you have it 
misidentified.  In the interim, please reconsider your strong disapproval?

Thanks.

> AFAICT, sshfs and openssh understand open(O_CREAT|O_EXCL) just fine and carry 
> the O_EXCL bit across the SFTP transport faithfully; further, open(O_CREAT|
> O_EXCL) is inherently an atomic operation, or you've found a bug in the host 
> OS, not sshfs.

This was not the case when I looked at sshfs, which was over a year
ago.  I'm glad to hear that sshfs has improved since then.

> If there is a problem or race here, I believe you have it 
> misidentified.  In the interim, please reconsider your strong disapproval?

Only if I get a confirmation from the sshfs developers themselves.
(Search the list archives for ``AFS semantics'' for an example.)

                                        Juliusz

Hello fuse-sshfs.  I'm attempting to get a patch included into darcs to make
it fall back to open(O_CREAT|O_EXCL) locking mechanisms when link() returns
ENOSYS as it does on sshfs.  While it certainly appears that this operation
is safe, the darcs developers would like confirmation that this is the case.

Thanks much.
--nwf;

On Sat, Jul 12, 2008 at 07:28:16PM +0200, Juliusz Chroboczek wrote:
> > AFAICT, sshfs and openssh understand open(O_CREAT|O_EXCL) just fine and carry 
> > the O_EXCL bit across the SFTP transport faithfully; further, open(O_CREAT|
> > O_EXCL) is inherently an atomic operation, or you've found a bug in the host 
> > OS, not sshfs.
> 
> This was not the case when I looked at sshfs, which was over a year
> ago.  I'm glad to hear that sshfs has improved since then.
> 
> > If there is a problem or race here, I believe you have it 
> > misidentified.  In the interim, please reconsider your strong disapproval?
> 
> Only if I get a confirmation from the sshfs developers themselves.
> (Search the list archives for ``AFS semantics'' for an example.)
> 
>                                         Juliusz

The strict requirements of roundup's Subject header field parser are annoying.

----- Forwarded message from Miklos Szeredi <miklos@szeredi.hu> -----

To: trentbuck@gmail.com
CC: fuse-sshfs@lists.sourceforge.net
Subject: Re: [sshfs] [issue904] record fails on sshfs (sshfs has no
	atomic_create)
From: Miklos Szeredi <miklos@szeredi.hu>
Date: Sun, 25 Jan 2009 20:32:00 +0100

On Sun, 25 Jan 2009, trentbuck@gmail.com (Trent W. Buck wrote:
> Nathaniel W Filardo <nwf@cs.jhu.edu> writes:
> >>> open(O_CREAT| O_EXCL) is inherently an atomic operation [...]
> >>> AFAICT, sshfs understands open(O_CREAT|O_EXCL) just fine and carries
> >>> the O_EXCL bit across the SFTP transport faithfully[...]
> 
> Would a developer here please confirm the above?

It has been true for a while now (since about 2006).  The requrements
for atomic create+open are:

  linux kernel >= 2.6.15
  libfuse >= 2.5
  sshfs >= 1.3

> 
> Such confirmation is necessary before Darcs developers will remove a
> paranoia setting that prevents Darcs from working over sshfs:
> 
> > Hello fuse-sshfs.  I'm attempting to get a patch included into
> > darcs to make it fall back to open(O_CREAT|O_EXCL) locking
> > mechanisms when link() returns ENOSYS as it does on sshfs.  While
> > it certainly appears that this operation is safe, the darcs
> > developers would like confirmation that this is the case.
> 
> It seems Nathaniel didn't receive a reply.  Perhaps this list is for
> users, and the developer list is elsewhere?

It seems the previous posting didn't make it to the list.  It may have
been discarded accidentally while moderating for spam.

Thanks,
Miklos

----- End forwarded message -----

Hi!

>It has been true for a while now (since about 2006).  The requrements
>for atomic create+open are:

>  linux kernel >= 2.6.15
>  libfuse >= 2.5
>  sshfs >= 1.3

>> Such confirmation is necessary before Darcs developers will remove a
>> paranoia setting that prevents Darcs from working over sshfs:

Any news regarding the above?


Sincerely,
Gour
Sincerely,
Gour

Hi Juliusz,

Any chance you could have a second look at this ticket?  

All I understand about this thread is that Nathaniel (nwf) submitted a patch
which you disapproved of pending some confirmation from sshfs developers.

And that this reply from Miklos Szeredi <miklos@szeredi.hu> on the sshfs list
makes me think we have that confirmation:

> Nathaniel W Filardo <nwf@cs.jhu.edu> writes:
> >>> open(O_CREAT| O_EXCL) is inherently an atomic operation [...]
> >>> AFAICT, sshfs understands open(O_CREAT|O_EXCL) just fine and carries
> >>> the O_EXCL bit across the SFTP transport faithfully[...]

>It has been true for a while now (since about 2006).  The requrements
>for atomic create+open are:

>  linux kernel >= 2.6.15
>  libfuse >= 2.5
>  sshfs >= 1.3

>> Such confirmation is necessary before Darcs developers will remove a
>> paranoia setting that prevents Darcs from working over sshfs:

Thanks!

Sent Nathaniel's patch as http://bugs.darcs.net/patch1022 and will accept 
it.

The following patch sent by Nathaniel Filardo <nwf@cs.jhu.edu> updated issue issue904 with
status=resolved;resolvedin=2.10.0 HEAD

* Resolve issue904: Fix record on Linux/FUSE/sshfs (fall back to sloppy locks automatically) 
Ignore-this: acdc2639207ccb4f73042397b549dc4b