Patchwork Can't mount smb shares using mount.cifs with 2.6.31 kernel

login
register
mail settings
Submitter Jeff Layton
Date Oct. 16, 2009, 8:47 p.m.
Message ID <20091016164759.5bf12da8@tlielax.poochiereds.net>
Download mbox | patch
Permalink /patch/36275/
State New
Headers show

Comments

Jeff Layton - Oct. 16, 2009, 8:47 p.m.
On Fri, 16 Oct 2009 16:12:43 -0400
Jeff Layton <jlayton@redhat.com> wrote:

> On Fri, 16 Oct 2009 14:49:56 -0400
> Jeff Layton <jlayton@redhat.com> wrote:
> 
> > On Fri, 16 Oct 2009 12:06:59 -0400
> > Timothy Normand Miller <theosib@gmail.com> wrote:
> > 
> > > I'm sending you the captures privately.  The first one is just a
> > > mount.  The second one is a mount with an ls.
> > > 
> > > On Fri, Oct 16, 2009 at 11:43 AM, Jeff Layton <jlayton@redhat.com> wrote:
> > > > On Fri, 16 Oct 2009 11:35:07 -0400
> > > > Timothy Normand Miller <theosib@gmail.com> wrote:
> > > >
> > > >> On Fri, Oct 16, 2009 at 10:50 AM, Jeff Layton <jlayton@redhat.com> wrote:
> > > >> > On Fri, 16 Oct 2009 10:32:39 -0400
> > > >> >
> > > >> > A little. Can you try mounting again with "-o noserverino"? Perhaps
> > > >> > something is wrong with the inode numbers being generated by the server
> > > >> > here...
> > > >>
> > > >> That fixed the problem.  It mounts properly now.
> > > >>
> > > >> > If that doesn't help, then maybe also capture some debugging info of
> > > >> > you doing an "ls" in the top-level dir after doing the mount.
> > > >>
> > > >> I can't tell where one set of debug messages end and where the next
> > > >> set starts.  Is there something I can do to insert a dummy message
> > > >> into dmesg so I can tell where to start copying?
> > > >>
> > > >> > I doubt this is anything you've done. There were a number of changes in
> > > >> > how inodes are handled under CIFS between 2.6.30 and 2.6.31. It's quite
> > > >> > possible that one of those changes is causing problems with this server.
> > > >>
> > > >> Thanks for the help.  Is there anything else I can do to diagnose the
> > > >> cause so it can be fixed permanently?
> > > >>
> > > >>
> > > >
> > > > Yes, could you send me a wire capture of a mount on the 2.6.31 kernel
> > > > without the "-o noserverino" option? I need to have a look at why the
> > > > GetSrvInum call isn't doing the right thing here. There are
> > > > instructions on the troubleshooting page for doing this.
> > > >
> > > > Thanks,
> > > > --
> > > > Jeff Layton <jlayton@redhat.com>
> > > >
> > > 
> > > 
> > > 
> > 
> > Thanks for the info. The mount seems to be successful. The ls makes
> > CIFS issue a "FindFile First" which is how SMB does a READDIR on the
> > wire. The server sends the response and then after that there is no
> > further traffic. I assume that CIFS doesn't like that response for some
> > reason, but wireshark unfortunately does not yet implement a dissector
> > for the FFF infolevel in use here.
> > 
> > I'm going to see if I can fix that in wireshark and then may have a
> > little more info. What may help in the meantime is the output from
> > cifsFYI debugging turned up while doing an "ls" after a fresh mount
> > without "-o noserverino".
> > 
> 
> Yay. Figured out how to fix the wireshark dissector for this FindFile
> infolevel. Wireshark patch attached that allows me to dissect these
> packets correctly.  I'll plan to send that to the wireshark devs as
> soon as I figure out where to send it. The good news is that I think I
> see the problem. The bad news is that I'm not quite sure how to fix it
> yet and it may even be a server bug.
> 
> Prior to 2.6.30, the default was to generate inode numbers out of the
> air (noserverino). With 2.6.31, the default is "serverino" which makes
> it so that we query the server for inode numbers. When mounting, we do
> a QueryPathInfo against the root inode. With serverino enabled, we also
> do a FileInternalInfo query against the file for the
> "IndexNumber" (which I assumed was also the equivalent of the UniqueId
> but maybe isn't?). In any case, that first call succeeds against this
> server.
> 
> The problem comes in with the FindFile call. There we do a
> FIND_FILE_ID_FULL_DIRECTORY_INFO infolevel query. That also succeeds,
> but the inode number values in there (FileId's) are zeroed out. That's
> technically within the letter of the spec for that call. When the
> underlying filesystem doesn't support unique ID's, then it's supposed
> to return 0. The problem is that it doesn't make much sense for the
> server to claim that it does support unique ID's for one call but not
> for others. 
> 
> Ideally, there'll be a way to deal with this automatically, but I'll
> probably need to ponder this a bit. In any case, thanks for the problem
> report. I'll let you know once I come up with something.
> 
> Cheers,

Timothy,

I think this will probably fix it. Can you test this patch out and let
me know if it does?

Thanks,

Patch

From 87f412fa02c9a0ba3eedf964f08efaa67f61edaa Mon Sep 17 00:00:00 2001
From: Jeff Layton <jlayton@redhat.com>
Date: Fri, 16 Oct 2009 16:42:23 -0400
Subject: [PATCH] cifs: fix server returning zeroed out FileID's in SMB_FIND_FILE_ID_FULL_DIR_INFO

It's possible that a server will return a valid FileID when we query the
FILE_INTERNAL_INFO for the root inode, but then zeroed out inode numbers
when we do a FindFile with an infolevel of SMB_FIND_FILE_ID_FULL_DIR_INFO.

In this situation turn off querying for server inode numbers, and just
generate an inode number using iunique.

Reported-by: Timothy Normand Miller <theosib@gmail.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/cifs/readdir.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c
index 1f098ca..bafef8b 100644
--- a/fs/cifs/readdir.c
+++ b/fs/cifs/readdir.c
@@ -727,11 +727,13 @@  static int cifs_filldir(char *pfindEntry, struct file *file, filldir_t filldir,
 		cifs_dir_info_to_fattr(&fattr, (FILE_DIRECTORY_INFO *)
 					pfindEntry, cifs_sb);
 
-	/* FIXME: make _to_fattr functions fill this out */
-	if (pCifsF->srch_inf.info_level == SMB_FIND_FILE_ID_FULL_DIR_INFO)
+	if (inum) {
 		fattr.cf_uniqueid = inum;
-	else
+	} else {
 		fattr.cf_uniqueid = iunique(sb, ROOT_I);
+		if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_SERVER_INUM)
+			cifs_sb->mnt_cifs_flags &= ~CIFS_MOUNT_SERVER_INUM;
+	}
 
 	ino = cifs_uniqueid_to_ino_t(fattr.cf_uniqueid);
 	tmp_dentry = cifs_readdir_lookup(file->f_dentry, &qstring, &fattr);
-- 
1.6.0.6