Re: mkstemp fails on cifs linux-2.6.31-rc1 / samba-3.3.6

Am Mittwoch, 1. Juli 2009 13:08:14 schrieb Shirish Pargaonkar:
> On Tue, Jun 30, 2009 at 1:36 PM, Shirish
>
> Pargaonkar<[email protected]> wrote:
> > On Tue, Jun 30, 2009 at 8:40 AM, Jeff Layton<[email protected]> wrote:
> >> On Tue, 2009-06-30 at 08:27 -0500, Shirish Pargaonkar wrote:
> >>> On Tue, Jun 30, 2009 at 8:01 AM, Jeff Layton<[email protected]> wrote:
> >>> > On Tue, 2009-06-30 at 13:00 +0200, Wilhelm Meier wrote:
> >>> >> Hi,
> >>> >>
> >>> >> I made a simple test with cifs in linux-2.6.31-rc1 to see if the so
> >>> >> called kmail-problem (cifs-user-homes are totally unusable for
> >>> >> kmail-mail-cache) still remains. Than I ran into a strange problem
> >>> >> using "sed -i <command> <file- on-cifs>". "sed" uses mkstemp
> >>> >> libc-funktion and fails with EEXIST, writing therefore tons of files
> >>> >> onto the cifs-share.
> >>> >>
> >>> >> You can reproduce it with:
> >>> >>
> >>> >> strace mktemp -p . abcXXXXXX 2>&1 | more
> >>> >>
> >>> >> giving
> >>> >>
> >>> >> stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0
> >>> >> open("./abcUJcKWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File
> >>> >> exists) stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0
> >>> >> open("./abcUGJaiA", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File
> >>> >> exists) stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0
> >>> >> open("./abcWfKacs", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File
> >>> >> exists) stat64(".", {st_mode=S_IFDIR|0700, st_size=0, ...}) = 0
> >>> >> open("./abcBaGjWM", O_RDWR|O_CREAT|O_EXCL, 0600) = -1 EEXIST (File
> >>> >> exists)
> >>> >>
> >>> >> This is the same on 2.6.31-rc1-git6.
> >>> >>
> >>> >> This is not the case in 2.6.26-2-vserver-686 (debian).
> >>> >>
> >>> >> The Samba-Server is lenny with samba-enterprise:
> >>> >> kmux-fs:/# dpkg -l | grep samba
> >>> >> ii  sernet-samba                    3.3.6-24                 a
> >>> >> LanManager-like file and printer server fo
> >>> >> ii  sernet-samba-common             3.3.6-24                 Samba
> >>> >> common files used by both the server a
> >>> >> ii  sernet-samba-keyring            1.1                      GnuPG
> >>> >> archive keys of the SerNet Samba archi
> >>> >>
> >>> >> If I change Samba to the lenny-version
> >>> >>
> >>> >> ii  samba                           2:3.2.5-4lenny6          a
> >>> >> LanManager-like file and printer server fo
> >>> >> ii  samba-common                    2:3.2.5-4lenny6          Samba
> >>> >> common files used by both the server a
> >>> >>
> >>> >> the described test-case is fine.
> >>> >
> >>> > I can reproduce this too.
> >>> >
> >>> > Looks like another regression from the create-on-lookup patches. I
> >>> > commented the relevant chunk out of cifs_lookup and that fixes the
> >>> > issue.
> >>> >
> >>> > Shirish, thoughts?
> >>> >
> >>> > --
> >>> > Jeff Layton <[email protected]>
> >>>
> >>> Jeff, Wilhelm,
> >>>
> >>> I will spend some time later in the day on this but one thought came to
> >>> mind, Jeff, we took out a line where if there was exclusive create, we
> >>> would let vfs handle it, not sure if putting it back would help!
> >>
> >> Probably...it does seem to be related to O_EXCL. Rather than just
> >> blindly putting that back though, I'd like to see some understanding
> >> about why this is failing, and a consistent idea about how this is code
> >> is intended to work.
> >>
> >> Skipping create on lookup for the O_EXCL case seems ass-backwards to me.
> >> Doing it on lookup should help atomicity, and that's just what you want
> >> for O_EXCL.
> >>
> >> --
> >> Jeff Layton <[email protected]>
> >
> > So adding this back helps, this is what I had initially.  The reason I
> > did put this was because I
> > was not sure how to handle exclusive create then when I added lookup
> > intent code to cifs.
> > It sounds correct that a Samba server would handle the case of
> > exclusive create for cifs
> > and return an appropriate code (EEXISTS) in the response, may be cifs
> > is not handling it.
> >
> > This is what I get with this patch in
> >
> > # mktemp -p . abcXXXXXX
> > ./abcWpeApL
> >
> > I am debugging further to identify what the exact problem is.
> >
> >
> > diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> > index 7dc6b74..d6f9ecd 100644
> > --- a/fs/cifs/dir.c
> > +++ b/fs/cifs/dir.c
> > @@ -673,6 +673,8 @@ 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)) {
> > +                       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,
> > @@ -689,6 +691,7 @@ cifs_lookup(struct inode *parent_dir_inode, struct
> > dentry *direntry,
> >                                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,
>
> This is a problem at vfs level/interaction.  I see posix open
> returning without any error and wireshark
> trace says the same, set_path_info with info level SMB_POSIX_OPEN succeeds.

Did you get any progress in this respect? And if so, were the patches merged 
into kernel-git?

Thx,
Wilhelm

> _______________________________________________
> linux-cifs-client mailing list
> [email protected]
> https://lists.samba.org/mailman/listinfo/linux-cifs-client

-- 
Wilhelm