Re: [patch] utilize lookup intents to open in lookup

[Jeff Layton <jlayton@redhat.com>]

On Thu, 9 Apr 2009 08:46:07 -0500
Shirish Pargaonkar <shirishpargaonkar@gmail.com> wrote:
> >> @@ -632,12 +655,27 @@ cifs_lookup(struct inode *parent_dir_inode, struct dentry *direntry,
> >>       }
> >>       cFYI(1, ("Full path: %s inode = 0x%p", full_path, direntry->d_inode));
> >>
> >> -     if (pTcon->unix_ext)
> >> -             rc = cifs_get_inode_info_unix(&newInode, full_path,
> >> -                                           parent_dir_inode->i_sb, xid);
> >> -     else
> >> +     if (pTcon->unix_ext) {
> >> +             if (!(nd->flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY)) &&
> >> +                             (nd->flags & LOOKUP_OPEN)) {
> >> +                     if (!((nd->intent.open.flags & O_CREAT) &&
> >> +                                     (nd->intent.open.flags & O_EXCL))) {
> >> +                             mode = nd->intent.open.create_mode &
> >> +                                             ~current->fs->umask;
> >> +                             rc = cifs_posix_open(full_path, &newInode,
> >> +                                     parent_dir_inode->i_sb, mode,
> >> +                                     nd->intent.open.flags, &oplock,
> >> +                                     &fileHandle, xid);
> >> +                             if ((rc != -EINVAL) && (rc != -EOPNOTSUPP))
> >                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > This looks wrong to me.  It's possible that you'd get a different
> > return code here on an unsuccessful open call? Why would we want to
> > instantiate the filp on any error from cifs_posix_open? For instance,
> > that function can return -ENOMEM. I don't think we want to instantiate
> > the filp in that case.
> 
> Jeff,
> 
> Thanks, will handle the identation/formatting errors in a subsequent patch
> for sure.
> 
> If there is any other error code returned besides these two error codes,
> it is handled as an error, so file pointer will not be instantiated
> (as posix_open
> will be true).
> These two errors are there to handle the bug in samba posix open and
> if either of these error codes are returned, cifs_lookup does what it
> had been doing (as posix_open will be false),.
> 

Ok, thanks for clarifying that. Maybe a comment in there to explain
that would be helpful too?

-- 
Jeff Layton <jlayton@redhat.com>