[linux-cifs-client] mkstemp fails on cifs linux-2.6.31-rc1 /
samba-3.3.6
Shirish Pargaonkar
shirishpargaonkar at gmail.com
Wed Jul 1 14:31:11 GMT 2009
On Wed, Jul 1, 2009 at 8:59 AM, Jeff Layton<jlayton at redhat.com> wrote:
> On Wed, 2009-07-01 at 08:17 -0500, Shirish Pargaonkar wrote:
>> On Wed, Jul 1, 2009 at 7:13 AM, Jeff Layton<jlayton at redhat.com> wrote:
>> > On Wed, 2009-07-01 at 06:59 -0500, Shirish Pargaonkar wrote:
>> >> On Wed, Jul 1, 2009 at 6:21 AM, Jeff Layton<jlayton at redhat.com> wrote:
>> >> > On Wed, 2009-07-01 at 06:01 -0500, Shirish Pargaonkar wrote:
>> >> >>
>> >> >> I think the problem is, why EEXIST since file does not exist. The
>> >> >> looping is I guess
>> >> >> because mktemp then attempts to create another file and receives EEXIST and it
>> >> >> goes on forever.
>> >> >> So the basic issues is why EEXIST since mktemp is attempting a file
>> >> >> which does not
>> >> >> exist on the server.
>> >> >
>> >> > The problem is that the lookup is returning a positive dentry since it
>> >> > just created the file. This makes the VFS turn around and return -EEXIST
>> >> > (see the second O_EXCL check in do_filp_open). So we do have to skip the
>> >> > create on lookup for O_EXCL or it won't work.
>> >> >
>> >> > I think what's needed is something like this patch. It seems to fix the
>> >> > testcase for me. That said, this is not adequately regression tested and
>> >> > should not be committed until it is.
>> >> >
>> >> > On that note, I'd like to reiterate my earlier sentiment that all of
>> >> > this create-on-lookup code was committed prematurely and should be
>> >> > backed out until it's more fully baked.
>> >> >
>> >> > Just my USD$.02...
>> >> >
>> >> > --
>> >> > Jeff Layton <jlayton at redhat.com>
>> >> >
>> >>
>> >> meant to send this to everybody but ended up sending only to Jeff,
>> >> so here it is again...
>> >>
>> >> I think that is why it is not possible to exclusive create within
>> >> lookup intent code.
>> >> So, IMHO, we should put the check back in the cifs_lookup.
>> >> That is what NFS does, and cifs should do the same
>> >
>> > Right, but do we really need to do the lookup in that case? It seems
>> > like the old check didn't optimize it away (but maybe I'm remembering
>> > incorrectly).
>> >
>> > --
>> > Jeff Layton <jlayton at redhat.com>
>> >
>> >
>>
>> With the check, we are back to what was before lookup intent for
>> exclusive create. I think for exclusive create, a lookup can result in
>> not create if the file exists, so for a command like mktemp, it
>> probably does not help, there will be a lookup and there will be a create.
>
> I'm sorry, I don't understand what you're saying here. Can you rephrase
> it? Or better yet, maybe post the patch that you're proposing to fix
> this?
>
> --
> Jeff Layton <jlayton at redhat.com>
>
>
This is the patch, this is what we had it before in dir.c in cifs_lookup()
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 7dc6b74..29d5642 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -673,22 +673,26 @@ cifs_lookup(struct inode *parent_dir_inode,
struct dentry *direntry,
if (!(nd->flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY)) &&
(nd->flags & LOOKUP_OPEN) && !pTcon->broken_posix_open &&
(nd->intent.open.flags & O_CREAT)) {
- rc = cifs_posix_open(full_path, &newInode,
+ /* skip posix open if exclusive create */
+ if (!((nd->intent.open.flags & O_CREAT) &&
+ (nd->intent.open.flags & O_EXCL))) {
+ rc = cifs_posix_open(full_path, &newInode,
parent_dir_inode->i_sb,
nd->intent.open.create_mode,
nd->intent.open.flags, &oplock,
&fileHandle, xid);
- /*
- * The check below works around a bug in POSIX
- * open in samba versions 3.3.1 and earlier where
- * open could incorrectly fail with invalid parameter.
- * If either that or op not supported returned, follow
- * the normal lookup.
- */
- if ((rc == 0) || (rc == -ENOENT))
- posix_open = true;
- else if ((rc == -EINVAL) || (rc != -EOPNOTSUPP))
- pTcon->broken_posix_open = true;
+ /*
+ * The check below works around a bug in POSIX
+ * open in samba versions 3.3.1 and earlier
+ * where open could incorrectly fail with
+ * invalid parameter. If either that or op not
+ * supported returned, follow the normal lookup.
+ */
+ if ((rc == 0) || (rc == -ENOENT))
+ posix_open = true;
+ else if ((rc == -EINVAL) || (rc != -EOPNOTSUPP))
+ pTcon->broken_posix_open = true;
+ }
}
if (!posix_open)
rc = cifs_get_inode_info_unix(&newInode, full_path,
-------------- next part --------------
A non-text attachment was scrubbed...
Name: excl_create.patch
Type: application/octet-stream
Size: 1665 bytes
Desc: not available
Url : http://lists.samba.org/archive/linux-cifs-client/attachments/20090701/9d8a11a4/excl_create-0001.obj
More information about the linux-cifs-client
mailing list