On Fri, 1 Nov 2013 17:57:39 +0400 Pavel Shilovsky <[email protected]> wrote:
> From: Pavel Shilovsky <[email protected]> > > Now we treat any reparse point as a symbolic link and map it to a Unix > one that is not true in a common case due to many reparse point types > supported by SMB servers. > > Distinguish reparse point types into two groups: > 1) that can be accessed directly through a reparse point > (junctions, deduplicated files, NFS symlinks); > 2) that need to be processed manually (Windows symbolic links, DFS); > > and map only Windows symbolic links to Unix ones. > > Signed-off-by: Pavel Shilovsky <[email protected]> > --- > fs/cifs/cifsglob.h | 2 +- > fs/cifs/inode.c | 23 +++++++++++++---------- > fs/cifs/readdir.c | 40 ++++++++-------------------------------- > fs/cifs/smb1ops.c | 21 ++++++++++++++++++++- > fs/cifs/smb2inode.c | 16 ++++++++++++---- > fs/cifs/smb2proto.h | 2 +- > 6 files changed, 55 insertions(+), 49 deletions(-) > > diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h > index a67cf12..b688f2b 100644 > --- a/fs/cifs/cifsglob.h > +++ b/fs/cifs/cifsglob.h > @@ -261,7 +261,7 @@ struct smb_version_operations { > /* query path data from the server */ > int (*query_path_info)(const unsigned int, struct cifs_tcon *, > struct cifs_sb_info *, const char *, > - FILE_ALL_INFO *, bool *); > + FILE_ALL_INFO *, bool *, bool *); This looks much better for performance, but I think it really shines a light on what an abortion the query_path_info and query_file_info operations are. They work with a FILE_ALL_INFO struct which is SMB1 specific, so for SMB2/3 we have to convert data to something that doesn't even match the protocol. I think it would be best to base this fix on top of a patchset to fix that properly. Those functions should be changed to pass data around in a cifs_fattr. With that, you could just add a new cifs_fattr->flags value and you wouldn't need to add this extra bool * argument. > /* query file data from the server */ > int (*query_file_info)(const unsigned int, struct cifs_tcon *, > struct cifs_fid *, FILE_ALL_INFO *); > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c > index 867b7cd..36f9ebb 100644 > --- a/fs/cifs/inode.c > +++ b/fs/cifs/inode.c > @@ -542,7 +542,8 @@ static int cifs_sfu_mode(struct cifs_fattr *fattr, const > unsigned char *path, > /* Fill a cifs_fattr struct with info from FILE_ALL_INFO */ > static void > cifs_all_info_to_fattr(struct cifs_fattr *fattr, FILE_ALL_INFO *info, > - struct cifs_sb_info *cifs_sb, bool adjust_tz) > + struct cifs_sb_info *cifs_sb, bool adjust_tz, > + bool symlink) > { > struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb); > > @@ -569,7 +570,11 @@ cifs_all_info_to_fattr(struct cifs_fattr *fattr, > FILE_ALL_INFO *info, > fattr->cf_createtime = le64_to_cpu(info->CreationTime); > > fattr->cf_nlink = le32_to_cpu(info->NumberOfLinks); > - if (fattr->cf_cifsattrs & ATTR_DIRECTORY) { > + > + if (symlink) { > + fattr->cf_mode = S_IFLNK; > + fattr->cf_dtype = DT_LNK; > + } else if (fattr->cf_cifsattrs & ATTR_DIRECTORY) { > fattr->cf_mode = S_IFDIR | cifs_sb->mnt_dir_mode; > fattr->cf_dtype = DT_DIR; > /* > @@ -578,10 +583,6 @@ cifs_all_info_to_fattr(struct cifs_fattr *fattr, > FILE_ALL_INFO *info, > */ > if (!tcon->unix_ext) > fattr->cf_flags |= CIFS_FATTR_UNKNOWN_NLINK; > - } else if (fattr->cf_cifsattrs & ATTR_REPARSE) { > - fattr->cf_mode = S_IFLNK; > - fattr->cf_dtype = DT_LNK; > - fattr->cf_nlink = le32_to_cpu(info->NumberOfLinks); > } else { > fattr->cf_mode = S_IFREG | cifs_sb->mnt_file_mode; > fattr->cf_dtype = DT_REG; > @@ -626,7 +627,8 @@ cifs_get_file_info(struct file *filp) > rc = server->ops->query_file_info(xid, tcon, &cfile->fid, &find_data); > switch (rc) { > case 0: > - cifs_all_info_to_fattr(&fattr, &find_data, cifs_sb, false); > + cifs_all_info_to_fattr(&fattr, &find_data, cifs_sb, false, > + false); > break; > case -EREMOTE: > cifs_create_dfs_fattr(&fattr, inode->i_sb); > @@ -673,6 +675,7 @@ cifs_get_inode_info(struct inode **inode, const char > *full_path, > bool adjust_tz = false; > struct cifs_fattr fattr; > struct cifs_search_info *srchinf = NULL; > + bool symlink = false; > > tlink = cifs_sb_tlink(cifs_sb); > if (IS_ERR(tlink)) > @@ -702,12 +705,12 @@ cifs_get_inode_info(struct inode **inode, const char > *full_path, > } > data = (FILE_ALL_INFO *)buf; > rc = server->ops->query_path_info(xid, tcon, cifs_sb, full_path, > - data, &adjust_tz); > + data, &adjust_tz, &symlink); > } > > if (!rc) { > - cifs_all_info_to_fattr(&fattr, (FILE_ALL_INFO *)data, cifs_sb, > - adjust_tz); > + cifs_all_info_to_fattr(&fattr, data, cifs_sb, adjust_tz, > + symlink); > } else if (rc == -EREMOTE) { > cifs_create_dfs_fattr(&fattr, sb); > rc = 0; > diff --git a/fs/cifs/readdir.c b/fs/cifs/readdir.c > index 53a75f3..5940eca 100644 > --- a/fs/cifs/readdir.c > +++ b/fs/cifs/readdir.c > @@ -134,22 +134,6 @@ out: > dput(dentry); > } > > -/* > - * Is it possible that this directory might turn out to be a DFS referral > - * once we go to try and use it? > - */ > -static bool > -cifs_dfs_is_possible(struct cifs_sb_info *cifs_sb) > -{ > -#ifdef CONFIG_CIFS_DFS_UPCALL > - struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb); > - > - if (tcon->Flags & SMB_SHARE_IS_IN_DFS) > - return true; > -#endif > - return false; > -} > - > static void > cifs_fill_common_info(struct cifs_fattr *fattr, struct cifs_sb_info *cifs_sb) > { > @@ -159,27 +143,19 @@ cifs_fill_common_info(struct cifs_fattr *fattr, struct > cifs_sb_info *cifs_sb) > if (fattr->cf_cifsattrs & ATTR_DIRECTORY) { > fattr->cf_mode = S_IFDIR | cifs_sb->mnt_dir_mode; > fattr->cf_dtype = DT_DIR; > - /* > - * Windows CIFS servers generally make DFS referrals look > - * like directories in FIND_* responses with the reparse > - * attribute flag also set (since DFS junctions are > - * reparse points). We must revalidate at least these > - * directory inodes before trying to use them (if > - * they are DFS we will get PATH_NOT_COVERED back > - * when queried directly and can then try to connect > - * to the DFS target) > - */ > - if (cifs_dfs_is_possible(cifs_sb) && > - (fattr->cf_cifsattrs & ATTR_REPARSE)) > - fattr->cf_flags |= CIFS_FATTR_NEED_REVAL; > - } else if (fattr->cf_cifsattrs & ATTR_REPARSE) { > - fattr->cf_mode = S_IFLNK; > - fattr->cf_dtype = DT_LNK; > } else { > fattr->cf_mode = S_IFREG | cifs_sb->mnt_file_mode; > fattr->cf_dtype = DT_REG; > } > > + /* > + * We need to revalidate it further to make a decision about whether it > + * is a symbolic link, DFS referral or a reparse point with a direct > + * access like junctions, deduplicated files, NFS symlinks. > + */ > + if (fattr->cf_cifsattrs & ATTR_REPARSE) > + fattr->cf_flags |= CIFS_FATTR_NEED_REVAL; > + > /* non-unix readdir doesn't provide nlink */ > fattr->cf_flags |= CIFS_FATTR_UNKNOWN_NLINK; > > diff --git a/fs/cifs/smb1ops.c b/fs/cifs/smb1ops.c > index ea99efe..47ab035 100644 > --- a/fs/cifs/smb1ops.c > +++ b/fs/cifs/smb1ops.c > @@ -534,10 +534,12 @@ cifs_is_path_accessible(const unsigned int xid, struct > cifs_tcon *tcon, > static int > cifs_query_path_info(const unsigned int xid, struct cifs_tcon *tcon, > struct cifs_sb_info *cifs_sb, const char *full_path, > - FILE_ALL_INFO *data, bool *adjustTZ) > + FILE_ALL_INFO *data, bool *adjustTZ, bool *symlink) > { > int rc; > > + *symlink = false; > + > /* could do find first instead but this returns more info */ > rc = CIFSSMBQPathInfo(xid, tcon, full_path, data, 0 /* not legacy */, > cifs_sb->local_nls, cifs_sb->mnt_cifs_flags & > @@ -554,6 +556,23 @@ cifs_query_path_info(const unsigned int xid, struct > cifs_tcon *tcon, > CIFS_MOUNT_MAP_SPECIAL_CHR); > *adjustTZ = true; > } > + > + if (!rc && (le32_to_cpu(data->Attributes) & ATTR_REPARSE)) { > + int tmprc; > + int oplock = 0; > + __u16 netfid; > + > + /* Need to check if this is a symbolic link or not */ > + tmprc = CIFSSMBOpen(xid, tcon, full_path, FILE_OPEN, > + FILE_READ_ATTRIBUTES, 0, &netfid, &oplock, > + NULL, cifs_sb->local_nls, > + cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR); > + if (tmprc == -EOPNOTSUPP) > + *symlink = true; > + else > + CIFSSMBClose(xid, tcon, netfid); > + } > + > return rc; > } > > diff --git a/fs/cifs/smb2inode.c b/fs/cifs/smb2inode.c > index 78ff88c..84c012a 100644 > --- a/fs/cifs/smb2inode.c > +++ b/fs/cifs/smb2inode.c > @@ -123,12 +123,13 @@ move_smb2_info_to_cifs(FILE_ALL_INFO *dst, struct > smb2_file_all_info *src) > int > smb2_query_path_info(const unsigned int xid, struct cifs_tcon *tcon, > struct cifs_sb_info *cifs_sb, const char *full_path, > - FILE_ALL_INFO *data, bool *adjust_tz) > + FILE_ALL_INFO *data, bool *adjust_tz, bool *symlink) > { > int rc; > struct smb2_file_all_info *smb2_data; > > *adjust_tz = false; > + *symlink = false; > > smb2_data = kzalloc(sizeof(struct smb2_file_all_info) + MAX_NAME * 2, > GFP_KERNEL); > @@ -136,9 +137,16 @@ smb2_query_path_info(const unsigned int xid, struct > cifs_tcon *tcon, > return -ENOMEM; > > rc = smb2_open_op_close(xid, tcon, cifs_sb, full_path, > - FILE_READ_ATTRIBUTES, FILE_OPEN, > - OPEN_REPARSE_POINT, smb2_data, > - SMB2_OP_QUERY_INFO); > + FILE_READ_ATTRIBUTES, FILE_OPEN, 0, > + smb2_data, SMB2_OP_QUERY_INFO); > + if (rc == -EOPNOTSUPP) { > + *symlink = true; > + /* Failed on a symbolic link - query a reparse point info */ > + rc = smb2_open_op_close(xid, tcon, cifs_sb, full_path, > + FILE_READ_ATTRIBUTES, FILE_OPEN, > + OPEN_REPARSE_POINT, smb2_data, > + SMB2_OP_QUERY_INFO); > + } > if (rc) > goto out; > > diff --git a/fs/cifs/smb2proto.h b/fs/cifs/smb2proto.h > index 313813e..b4eea10 100644 > --- a/fs/cifs/smb2proto.h > +++ b/fs/cifs/smb2proto.h > @@ -61,7 +61,7 @@ extern void move_smb2_info_to_cifs(FILE_ALL_INFO *dst, > extern int smb2_query_path_info(const unsigned int xid, struct cifs_tcon > *tcon, > struct cifs_sb_info *cifs_sb, > const char *full_path, FILE_ALL_INFO *data, > - bool *adjust_tz); > + bool *adjust_tz, bool *symlink); > extern int smb2_set_path_size(const unsigned int xid, struct cifs_tcon *tcon, > const char *full_path, __u64 size, > struct cifs_sb_info *cifs_sb, bool set_alloc); -- Jeff Layton <[email protected]> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
