Re: [pnfs] [GIT BISECT] first bad commit: 1f36f774 Switch !O_CREAT case to use of do_last()

[Boaz Harrosh <bharrosh@panasas.com>]

On 03/25/2010 05:25 PM, Al Viro wrote:
> On Thu, Mar 25, 2010 at 04:27:13PM +0200, Boaz Harrosh wrote:
>  
>> And about the NFS thing, let misbehaving applications byte there own bullets.
>> But a good app like git, does put an O_DIRECTORY on the directory open, and
>> therefore will receive consistent results, right?
> 
> What the hell do you mean, "misbehaving"?  O_DIRECTORY means one thing:
> "refuse to open if it turns out to be a non-directory".  If open() succeed
> with it and fails without, it's a definite bug.  Application has every
> right to call the damn thing without that flag.
> 

Ha, OK, sorry I thought exactly the opposite. I assumed it meant
"don't allow a directory to be opened, unless set". As an application
hint of saying I'm going to do file things to this handle, don't want
any dirs.

So yes definitely it looks like an NFS bug masked by the fact that everyone
is using opendir() form libc for this.

> It *can* be used as a hint by fs code, and it's a useful hint, so yes,
> we need to restore LOOKUP_DIRECTORY (until we get rid of the entire
> d_revalidate/lookup+open mess and pass struct file + open flags directly
> to fs method for the last step of pathname resolution; basically, most
> of the do_last() guts would become that method, killing the intents
> crap for good; new method would simply check for O_DIRECTORY presence
> instead of looking at LOOKUP_DIRECTORY).
> 
> However, NFS behaviour in cases when O_DIRECTORY is absent is a separate
> (and quite real) bug.  If nfs_atomic_lookup() sees -EISDIR from attempt
> to open on server, it should either issue plain lookup or try to use the
> information it got in response to open attempt (if there's enough of it
> there).  Passing -EISDIR to ->lookup() caller is simply wrong.
> 
> I'm going to send a fix for O_DIRECTORY case (restoring the behaviour
> we had in 2.6.33) today, but NFS side of things also needs to be dealt
> with.

Thank you Al for fixing this, I hope some capable NFS person will take
that issue to heart.

Boaz