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 03:06 PM, Al Viro wrote:
> On Thu, Mar 25, 2010 at 02:18:56PM +0200, Benny Halevy wrote:
> 
>> Indeed this error is coming from the server:
>>
>> nfsd_dispatch: vers 4 proc 1                                                                            
>> nfsv4 compound op #1/7: 22 (OP_PUTFH)                                                                   
>> nfsd: fh_verify(16: 01010001 00000000 000e6592 345b9f25 00000000 00000000)                              
>> nfsv4 compound op ffff880076734078 opcnt 7 #1: 22: status 0                                             
>> nfsv4 compound op #2/7: 32 (OP_SAVEFH)                                                                  
>> nfsv4 compound op ffff880076734078 opcnt 7 #2: 32: status 0                                             
>> nfsv4 compound op #3/7: 18 (OP_OPEN)                                                                    
>> NFSD: nfsd4_open filename pack op_stateowner (null)                                                     
>> renewing client (clientid 4bab503e/00000002)                                                            
>> nfsd: nfsd_lookup(fh 16: 01010001 00000000 000e6592 345b9f25 00000000 00000000, pack)                   
>> nfsd: fh_verify(16: 01010001 00000000 000e6592 345b9f25 00000000 00000000)                              
>> nfsd: fh_compose(exp 08:05/106497 objects/pack, ino=943508)                                             
>> nfsd: fh_verify(16: 01010001 00000000 000e6594 345b9f26 00000000 00000000)                              
>> nfsv4 compound op ffff880076734078 opcnt 7 #3: 18: status 21                                            
>> nfsv4 compound returned 21                                                                              
> 
> Ho-hum...  So it hits the "let's try to open it atomically" path and
> gets told to FOAD by server (as it should, of course).
> 
> And if we see different behaviour after ls -l, presumably that's a
> difference between ->lookup() and ->d_revalidate() paths on client...
> 
> OK, I think I see what's going on in this case.  However, it doesn't
> explain everything; my current theory is that we used to get LOOKUP_DIRECTORY
> on the last components in O_DIRECTORY opens and we don't do that now.
> That used to derail the is_atomic_open(), now it's hit and there we go.
> 
> It's not hard to verify (and it might take care of this testcase), but
> I still have questions about the way this code used to work *without*
> O_DIRECTORY.
> 
> Let's try this: before do_lookup() call there add
> 	if (*want_dir)
> 		nd->flags |= LOOKUP_DIRECTORY;

Yes this fixes it!!
2.6.34-rc2 plus above, now works, horay. (diff attached)

> and see how does it behave.
> 
> However, even if it does help, it doesn't explain everything.  Normal
> open() on a directory without O_DIRECTORY if flags shouldn't fail with
> -EISDIR.  How did that manage to avoid it all along?

---
diff --git a/fs/namei.c b/fs/namei.c
index 1c0fca6..434ad2a 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1647,6 +1647,8 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 
 	/* just plain open? */
 	if (!(open_flag & O_CREAT)) {
+		if (*want_dir)
+			nd->flags |= LOOKUP_DIRECTORY;
 		error = do_lookup(nd, &nd->last, path);
 		if (error)
 			goto exit;