2008-10-17 15:14:35

by Steve French

[permalink] [raw]
Subject: unlink behavior when file is open by other process

Even when a file is open by another process, posix allows the file to
be deleted (the file is removed from the namespace and eventually the
data is removed from disk). Unfortunately due to problems in some
NAS filers/servers this can be hard to implement, and I am not sure
what the "best" behavior is in that case. Currently when unlink
fails with the cifs network status codes equivalent to ETXTBUSY, cifs
retries unlink by first renaming the file (ala nfs's "silly rename")
by file handle and then marking the file attribute as "delete on
close" (which will cause the server to unlink the file when the last
opener closes the file). This is similar to the behavior required by
posix (although, like in nfs, the silly renamed file is temporarily
visible in the namespace, can't be reopened by anyone else).

Jeff Layton included a behavior change within a patch to fix another
problem with NTCreateX flags
(http://git.samba.org/?p=jlayton/cifs.git;a=commitdiff;h=f0c39587b7111deb13e56e5a521c5f3d8278cf5e)
that I just merged that will break this (delete of open files) to at
least one popular filer because that filer does not support rename by
handle (rename of open file is one of the SMB transact2 levels, and
one that most servers support). His patch would give up in
cifs_unlink if we can't "silly-rename" the file. I have mixed
feelings about this since with current code we can delete the file
(mark the file delete on close) but we can't rename it (we could hide
it in the namespace but it obviously can't be completely transparent
because you can't create a file of the same name).

Is it better to fail unlink if the file can't be removed from the
namespace immediately or better to allow unlink (but then some
applications will get an access denied on open if they try to create a
file of the same name before the original opener closes the file)?

--
Thanks,

Steve


2008-10-17 15:24:43

by Steve French

[permalink] [raw]
Subject: Re: unlink behavior when file is open by other process

On Fri, Oct 17, 2008 at 10:09 AM, Steve French <[email protected]> wrote:
> Even when a file is open by another process, posix allows the file to
> be deleted (the file is removed from the namespace and eventually the
> data is removed from disk). Unfortunately due to problems in some
> NAS filers/servers this can be hard to implement, and I am not sure
> what the "best" behavior is in that case. Currently when unlink
> fails with the cifs network status codes equivalent to ETXTBUSY, cifs
> retries unlink by first renaming the file (ala nfs's "silly rename")
> by file handle and then marking the file attribute as "delete on
> close" (which will cause the server to unlink the file when the last
> opener closes the file). This is similar to the behavior required by
> posix (although, like in nfs, the silly renamed file is temporarily
> visible in the namespace, can't be reopened by anyone else).
>
> Jeff Layton included a behavior change within a patch to fix another
> problem with NTCreateX flags
> (http://git.samba.org/?p=jlayton/cifs.git;a=commitdiff;h=f0c39587b7111deb13e56e5a521c5f3d8278cf5e)
> that I just merged that will break this (delete of open files) to at
> least one popular filer because that filer does not support rename by
> handle (rename of open file is one of the SMB transact2 levels, and
> one that most servers support). His patch would give up in
> cifs_unlink if we can't "silly-rename" the file. I have mixed
> feelings about this since with current code we can delete the file
> (mark the file delete on close) but we can't rename it (we could hide
> it in the namespace but it obviously can't be completely transparent
> because you can't create a file of the same name).
>
> Is it better to fail unlink if the file can't be removed from the
> namespace immediately or better to allow unlink (but then some
> applications will get an access denied on open if they try to create a
> file of the same name before the original opener closes the file)?

The two particular examples:
1) An application that does:
open, unlink, close, create
used to always work but now would fail unless the server/filer has
rename-by-handle support

2) An application that does:
open, unlink, create
used to fail (with access denied on create) when the server did not have
rename-by-handle support but now (with Jeff's patch sideeffect) will
fail on unlink.


--
Thanks,

Steve

2008-10-17 17:27:31

by Jeff Layton

[permalink] [raw]
Subject: Re: [linux-cifs-client] Re: unlink behavior when file is open by other process

On Fri, 17 Oct 2008 10:24:29 -0500
"Steve French" <[email protected]> wrote:

> On Fri, Oct 17, 2008 at 10:09 AM, Steve French <[email protected]> wrote:
> > Even when a file is open by another process, posix allows the file to
> > be deleted (the file is removed from the namespace and eventually the
> > data is removed from disk). Unfortunately due to problems in some
> > NAS filers/servers this can be hard to implement, and I am not sure
> > what the "best" behavior is in that case. Currently when unlink
> > fails with the cifs network status codes equivalent to ETXTBUSY, cifs
> > retries unlink by first renaming the file (ala nfs's "silly rename")
> > by file handle and then marking the file attribute as "delete on
> > close" (which will cause the server to unlink the file when the last
> > opener closes the file). This is similar to the behavior required by
> > posix (although, like in nfs, the silly renamed file is temporarily
> > visible in the namespace, can't be reopened by anyone else).
> >
> > Jeff Layton included a behavior change within a patch to fix another
> > problem with NTCreateX flags
> > (http://git.samba.org/?p=jlayton/cifs.git;a=commitdiff;h=f0c39587b7111deb13e56e5a521c5f3d8278cf5e)
> > that I just merged that will break this (delete of open files) to at
> > least one popular filer because that filer does not support rename by
> > handle (rename of open file is one of the SMB transact2 levels, and
> > one that most servers support). His patch would give up in
> > cifs_unlink if we can't "silly-rename" the file. I have mixed
> > feelings about this since with current code we can delete the file
> > (mark the file delete on close) but we can't rename it (we could hide
> > it in the namespace but it obviously can't be completely transparent
> > because you can't create a file of the same name).
> >
> > Is it better to fail unlink if the file can't be removed from the
> > namespace immediately or better to allow unlink (but then some
> > applications will get an access denied on open if they try to create a
> > file of the same name before the original opener closes the file)?
>
> The two particular examples:
> 1) An application that does:
> open, unlink, close, create
> used to always work but now would fail unless the server/filer has
> rename-by-handle support
>
> 2) An application that does:
> open, unlink, create
> used to fail (with access denied on create) when the server did not have
> rename-by-handle support but now (with Jeff's patch sideeffect) will
> fail on unlink.
>
>

My argument is that the primary function of unlink() is to remove a
filename from the filesystem. If we return success from such a call
without actually having freed up that filename, then that's a bug. It's
unfortunate that some servers don't support the calls we need to make
this work all the time.

We can't however make assumptions about what applications want. We
could, in principle, fix up the situation where a server does
open->unlink->create by truncating the old file and pretending that
it's a new one. An application may be unlinking the file for some other
reason -- what if it wants to create a directory there?

All we can reasonably do is try to have each syscall give us the
expected end result or an error if it can't be done.

Down any other path lies madness ;)

--
Jeff Layton <[email protected]>

2008-10-17 17:41:21

by Steve French

[permalink] [raw]
Subject: Re: [linux-cifs-client] Re: unlink behavior when file is open by other process

On Fri, Oct 17, 2008 at 12:27 PM, Jeff Layton <[email protected]> wrote:
> On Fri, 17 Oct 2008 10:24:29 -0500
> "Steve French" <[email protected]> wrote:
>
>> On Fri, Oct 17, 2008 at 10:09 AM, Steve French <[email protected]> wrote:
>> > Even when a file is open by another process, posix allows the file to
>> > be deleted (the file is removed from the namespace and eventually the
>> > data is removed from disk). Unfortunately due to problems in some
>> > NAS filers/servers this can be hard to implement, and I am not sure
>> > what the "best" behavior is in that case.
>
> My argument is that the primary function of unlink() is to remove a
> filename from the filesystem. If we return success from such a call
> without actually having freed up that filename, then that's a bug. It's
> unfortunate that some servers don't support the calls we need to make
> this work all the time.
The filename will be freed - and the trade off is which breaks fewer apps.
Both open and unlink man pages list plausible return codes but I
am worried that the sequence of file operations open/unlink/close
(I think we see both dbench and connectathon do this IIRC)
is as common a a sequence as open/unlink/create/close
thus we could break more apps your way than leaving it as is.

> We can't however make assumptions about what applications want. We
> could, in principle, fix up the situation where a server does
> open->unlink->create by truncating the old file and pretending that
> it's a new one.
That could corrupt data - the original opener may need that data up
to the moment they close that handle.

> All we can reasonably do is try to have each syscall give us the
> expected end result or an error if it can't be done.
The open syscall is allowed to fail with ETXTBUSY (or even access
denied among other).
Although this type of situation is not common on open, it more common in
open (and create) than on unlink and thus A likely that an app could deal with
an open error than on the unlink that preceeded it. The other argument
here is that whether or not we allow unlink (when it can be marked for deletion
but not silly renamed) - we have apps that will get the same error on open due
to Windows, MacOS and other non-Linux clients setting the flag (ie open/create
failures for a filename that was marked delete-on-close can still
happen even if we aren't the
ones who set the flag on the file since Windows and various other OS can and do
set this file on the server or remotely)


--
Thanks,

Steve

2008-10-17 18:10:31

by Jeff Layton

[permalink] [raw]
Subject: Re: [linux-cifs-client] Re: unlink behavior when file is open by other process

On Fri, 17 Oct 2008 12:41:07 -0500
"Steve French" <[email protected]> wrote:

> On Fri, Oct 17, 2008 at 12:27 PM, Jeff Layton <[email protected]> wrote:
> > On Fri, 17 Oct 2008 10:24:29 -0500
> > "Steve French" <[email protected]> wrote:
> >
> >> On Fri, Oct 17, 2008 at 10:09 AM, Steve French <[email protected]> wrote:
> >> > Even when a file is open by another process, posix allows the file to
> >> > be deleted (the file is removed from the namespace and eventually the
> >> > data is removed from disk). Unfortunately due to problems in some
> >> > NAS filers/servers this can be hard to implement, and I am not sure
> >> > what the "best" behavior is in that case.
> >
> > My argument is that the primary function of unlink() is to remove a
> > filename from the filesystem. If we return success from such a call
> > without actually having freed up that filename, then that's a bug. It's
> > unfortunate that some servers don't support the calls we need to make
> > this work all the time.
> The filename will be freed - and the trade off is which breaks fewer apps.
> Both open and unlink man pages list plausible return codes but I
> am worried that the sequence of file operations open/unlink/close
> (I think we see both dbench and connectathon do this IIRC)
> is as common a a sequence as open/unlink/create/close
> thus we could break more apps your way than leaving it as is.
>

I think we have to shoot for mimicking POSIX as closely as we can. If
we can't do it, I don't think we have any option other than to return
error. We also have to consider that the existing behavior is racy and
unreliable. Suppose we have 2 processes:

Process 1 Process 2
======================================
creates file
opens file
unlinks file
closes file
closes file
creates file

So this will work against a server that doesn't support rename by
filehandle, but what happens if the second create comes in before
process 2 closes the file? The create will fail. The problem is that we
can't predict this. It all depends on the timing.

Someone may QA their application and have it work great, then all of a
sudden when they move to a big load, this sort of thing starts failing
because the timing has changed.

I would *much* prefer to have an application fail reliably than work 90%
of the time, and fail unpredictably the other 10%. Consistency is key.

> > We can't however make assumptions about what applications want. We
> > could, in principle, fix up the situation where a server does
> > open->unlink->create by truncating the old file and pretending that
> > it's a new one.
> That could corrupt data - the original opener may need that data up
> to the moment they close that handle.
>

Good point, all the more reason not do this.

> > All we can reasonably do is try to have each syscall give us the
> > expected end result or an error if it can't be done.
> The open syscall is allowed to fail with ETXTBUSY (or even access
> denied among other).
> Although this type of situation is not common on open, it more common in
> open (and create) than on unlink and thus A likely that an app could deal with
> an open error than on the unlink that preceeded it. The other argument
> here is that whether or not we allow unlink (when it can be marked for deletion
> but not silly renamed) - we have apps that will get the same error on open due
> to Windows, MacOS and other non-Linux clients setting the flag (ie open/create
> failures for a filename that was marked delete-on-close can still
> happen even if we aren't the
> ones who set the flag on the file since Windows and various other OS can and do
> set this file on the server or remotely)
>
>

Sure, I'm not disputing whether returning an error on open is right or
wrong. The problem is that it's not expected. We've just unlinked the
filename and returned success -- there is *no* reason that the create
should fail here. An application programmer will (rightfully) consider
this a bug.

Again, I think we have to try and mimic POSIX to the best of our
ability and just return error on anything else.
--
Jeff Layton <[email protected]>

2008-10-17 19:53:11

by jim owens

[permalink] [raw]
Subject: Re: [linux-cifs-client] Re: unlink behavior when file is open by other process

Jeff Layton wrote:

> Sure, I'm not disputing whether returning an error on open is right or
> wrong. The problem is that it's not expected. We've just unlinked the
> filename and returned success -- there is *no* reason that the create
> should fail here. An application programmer will (rightfully) consider
> this a bug.

I agree that failing the unlink if you can not do it is
"the right thing to do"... but unless you have some magic
to prevent anyone else from creating the file between that
unlink and the create then in fact there is a reason the
application can see the create fail after unlink succeeds :)

2008-10-17 20:18:14

by Jeff Layton

[permalink] [raw]
Subject: Re: [linux-cifs-client] Re: unlink behavior when file is open by other process

On Fri, 17 Oct 2008 15:52:46 -0400
jim owens <[email protected]> wrote:

> Jeff Layton wrote:
>
> > Sure, I'm not disputing whether returning an error on open is right or
> > wrong. The problem is that it's not expected. We've just unlinked the
> > filename and returned success -- there is *no* reason that the create
> > should fail here. An application programmer will (rightfully) consider
> > this a bug.
>
> I agree that failing the unlink if you can not do it is
> "the right thing to do"... but unless you have some magic
> to prevent anyone else from creating the file between that
> unlink and the create then in fact there is a reason the
> application can see the create fail after unlink succeeds :)

It's all about expectations. If you have your environment set up in
such a way that you allow other processes or clients to race in and
create a file or directory here, then you should be expecting that
the create can fail, when it occurs :)

I just think that we have to strive for _consistent_ behavior from the
kernel. If we allow unlink to return without actually removing the
link, then it may "just work" in most cases. The problem is that it
won't work in some cases and it'll be very hard to predict when that
will be. IMO, that's far worse than just failing the unlink outright.

--
Jeff Layton <[email protected]>