From mboxrd@z Thu Jan  1 00:00:00 1970
From: Jeff Layton <jlayton@redhat.com>
Subject: Re: [patch] utilize lookup intents to open in
	lookup
Date: Thu, 9 Apr 2009 10:00:19 -0400
Message-ID: <20090409100019.35ca2e35@barsoom.rdu.redhat.com>
References: <4a4634330903310957j25bbefcibb28e09a3eaf0c10@mail.gmail.com>
	<4a4634330904011233q5423fb2bydafd445134fc0717@mail.gmail.com>
	<20090409085554.1648fed8@barsoom.rdu.redhat.com>
	<4a4634330904090646v32f469f3g2303f64a75ff7389@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Steve French <smfrench@gmail.com>,
	"linux-cifs-client@lists.samba.org" <linux-cifs-client@lists.samba.org>,
	viro@zeniv.linux.org.uk, Trond Myklebust <trond.myklebust@fys.uio.no>
To: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
Return-path: <linux-cifs-client-bounces+glfc-linux-cifs-client=gmane.org@lists.samba.org>
In-Reply-To: <4a4634330904090646v32f469f3g2303f64a75ff7389@mail.gmail.com>
List-Unsubscribe: <https://lists.samba.org/mailman/listinfo/linux-cifs-client>,
	<mailto:linux-cifs-client-request@lists.samba.org?subject=unsubscribe>
List-Archive: <http://lists.samba.org/archive/linux-cifs-client>
List-Post: <mailto:linux-cifs-client@lists.samba.org>
List-Help: <mailto:linux-cifs-client-request@lists.samba.org?subject=help>
List-Subscribe: <https://lists.samba.org/mailman/listinfo/linux-cifs-client>,
	<mailto:linux-cifs-client-request@lists.samba.org?subject=subscribe>
Sender: linux-cifs-client-bounces+glfc-linux-cifs-client=gmane.org@lists.samba.org
Errors-To: linux-cifs-client-bounces+glfc-linux-cifs-client=gmane.org@lists.samba.org
List-Id: linux-fsdevel.vger.kernel.org

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, stru=
ct dentry *direntry,
> >> =C2=A0 =C2=A0 =C2=A0 }
> >> =C2=A0 =C2=A0 =C2=A0 cFYI(1, ("Full path: %s inode =3D 0x%p", full_pat=
h, direntry->d_inode));
> >>
> >> - =C2=A0 =C2=A0 if (pTcon->unix_ext)
> >> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 rc =3D cifs_get_inode_info=
_unix(&newInode, full_path,
> >> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 parent_dir_inode->i_sb, xid);
> >> - =C2=A0 =C2=A0 else
> >> + =C2=A0 =C2=A0 if (pTcon->unix_ext) {
> >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!(nd->flags & (LOOKUP_=
PARENT | LOOKUP_DIRECTORY)) &&
> >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (nd->flags & LOOKUP_OPEN)) {
> >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 if (!((nd->intent.open.flags & O_CREAT) &&
> >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 (nd->intent.ope=
n.flags & O_EXCL))) {
> >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 mode =3D nd->intent.open.create_mode &
> >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 ~current->fs->umask;
> >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 rc =3D cifs_posix_open(full_path, &newInode,
> >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 parent_dir_inod=
e->i_sb, mode,
> >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 nd->intent.open=
.flags, &oplock,
> >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 &fileHandle, xi=
d);
> >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if ((rc !=3D -EINVAL) && (rc !=3D -EOPNOTSU=
PP))
> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0^^^^^^=
^^^^^^^^^^^^^^^^^^^^^^
> > This looks wrong to me. =C2=A0It'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.
>=20
> Jeff,
>=20
> Thanks, will handle the identation/formatting errors in a subsequent patch
> for sure.
>=20
> 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),.
>=20

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

--=20
Jeff Layton <jlayton@redhat.com>