Re: review 5, was Re: projected date for mount.cifs to support DFS junction points

["Q (Igor Mammedov)" <qwerty0987654321@mail.ru>]

[-- Attachment #1: Type: text/plain, Size: 1546 bytes --]

Sorry guys, but I have a lot of work for the last 3 weeks,
so I couldn't spare much time for a hobby and react quickly.

Here is what I've done the last weekend.
Attached:
 fixed patch [5/5] (0001-DFS-patch-that-connects-inode-with-dfs-handling-ops.patch).
 fixed mixed case in struct member (0002-Fixed-mixed-case-name-in-structure-dfs_info3_param.patch)


Also I noticed that patch 2/5 is not completely applied yet. I'll send Steve
interim patch I've made to make thing compiled and working.

Christoph Hellwig wrote:
> +#ifdef CONFIG_CIFS_DFS_UPCALL
> +			if (is_remote) {
> +				inode->i_op =
> +					&cifs_dfs_referral_inode_operations;
> +				inode->i_fop = NULL;
> 
> i_fop should never be set to NULL.  Just leave it alone so it stays
> at &empty_fops.
> 
> +#ifdef CONFIG_CIFS_DFS_UPCALL
> +			if (is_remote) {
> +				inode->i_op =
> +					&cifs_dfs_referral_inode_operations;
> +				inode->i_fop = NULL;
> +			} else {
> +				inode->i_op = &cifs_dir_inode_ops;
> +				inode->i_fop = &cifs_dir_ops;
> +			}
> +#else
>  			inode->i_op = &cifs_dir_inode_ops;
>  			inode->i_fop = &cifs_dir_ops;
> +#endif
> 
> This code and everything surrounding it is duplicated in two functions.
> Please refactor it into a common helper before adding new code to it.
> 
> _______________________________________________
> linux-cifs-client mailing list
> linux-cifs-client@lists.samba.org
> https://lists.samba.org/mailman/listinfo/linux-cifs-client
> 
> .
> 


-- 

Best regards,

-------------------------
Igor Mammedov,
niallain "at" gmail.com





[-- Attachment #2: 0001-DFS-patch-that-connects-inode-with-dfs-handling-ops.patch --]
[-- Type: text/x-patch, Size: 7131 bytes --]

>From 490ed5e40713206bf3011880b03cd9f5766b5467 Mon Sep 17 00:00:00 2001
From: Igor Mammedov <niallain@gmail.com>
Date: Fri, 15 Feb 2008 19:31:42 +0300
Subject: [PATCH] DFS patch that connects inode with dfs handling ops
 if it is DFS junction point.

DFS junction point is detected by EREMOTE error from
CIFSSMBQPathInfo.Then we need to request server again,
this time with full path name so we could get correct
info for this inode.
It is final DFS patch that gets all patchset working
and it depends on all previous DFS patches.

Signed-off-by: Igor Mammedov <niallain@gmail.com>
---
 fs/cifs/cifs_dfs_ref.c |   10 +++++
 fs/cifs/cifsfs.h       |   11 ++++++
 fs/cifs/inode.c        |   93 +++++++++++++++++++++---------------------------
 3 files changed, 62 insertions(+), 52 deletions(-)

diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c
index 413ee23..3161986 100644
--- a/fs/cifs/cifs_dfs_ref.c
+++ b/fs/cifs/cifs_dfs_ref.c
@@ -29,6 +29,16 @@ LIST_HEAD(cifs_dfs_automount_list);
  * DFS functions
 */
 
+void cifs_dfs_ref_inode_op_fixup(int is_remote, struct inode *inode)
+{
+	if (is_remote) {
+		inode->i_op = &cifs_dfs_referral_inode_operations;
+	} else {
+		inode->i_op = &cifs_dir_inode_ops;
+		inode->i_fop = &cifs_dir_ops;
+	}
+}
+
 void dfs_shrink_umount_helper(struct vfsmount *vfsmnt)
 {
 	mark_mounts_for_expiry(&cifs_dfs_automount_list);
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index 195b14d..d693e2d 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -91,6 +91,17 @@ extern int cifs_dir_notify(struct file *, unsigned long arg);
 extern struct dentry_operations cifs_dentry_ops;
 extern struct dentry_operations cifs_ci_dentry_ops;
 
+#ifdef CONFIG_CIFS_DFS_UPCALL
+extern void cifs_dfs_ref_inode_op_fixup(int is_remote, struct inode *inode);
+#else
+static inline void cifs_dfs_ref_inode_op_fixup(int is_remote,
+			struct inode *inode)
+{
+	inode->i_op = &cifs_dir_inode_ops;
+	inode->i_fop = &cifs_dir_ops;
+}
+#endif /* DFS_UPCALL */
+
 /* Functions related to symlinks */
 extern void *cifs_follow_link(struct dentry *direntry, struct nameidata *nd);
 extern void cifs_put_link(struct dentry *direntry,
diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c
index 6020add..79e61c5 100644
--- a/fs/cifs/inode.c
+++ b/fs/cifs/inode.c
@@ -37,7 +37,7 @@ int cifs_get_inode_info_unix(struct inode **pinode,
 	struct cifsTconInfo *pTcon;
 	struct inode *inode;
 	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
-	char *tmp_path;
+	int is_remote = 0;
 
 	pTcon = cifs_sb->tcon;
 	cFYI(1, ("Getting info on %s", search_path));
@@ -48,30 +48,10 @@ int cifs_get_inode_info_unix(struct inode **pinode,
 /*	dump_mem("\nUnixQPathInfo return data", &findData,
 		 sizeof(findData)); */
 	if (rc) {
-		if (rc == -EREMOTE) {
-			tmp_path =
-			    kmalloc(strnlen(pTcon->treeName,
-					    MAX_TREE_SIZE + 1) +
-				    strnlen(search_path, MAX_PATHCONF) + 1,
-				    GFP_KERNEL);
-			if (tmp_path == NULL)
-				return -ENOMEM;
-
-			/* have to skip first of the double backslash of
-			   UNC name */
-			strncpy(tmp_path, pTcon->treeName, MAX_TREE_SIZE);
-			strncat(tmp_path, search_path, MAX_PATHCONF);
-			rc = connect_to_dfs_path(xid, pTcon->ses,
-						 /* treename + */ tmp_path,
-						 cifs_sb->local_nls,
-						 cifs_sb->mnt_cifs_flags &
-						    CIFS_MOUNT_MAP_SPECIAL_CHR);
-			kfree(tmp_path);
-
-			/* BB fix up inode etc. */
-		} else if (rc) {
-			return rc;
-		}
+		/* BB: we need to revise code here
+		 * and do it as in cifs_get_inode_info
+		 * now it will return error -EREMOTE in case of dfs*/
+		return rc;
 	} else {
 		struct cifsInodeInfo *cifsInfo;
 		__u32 type = le32_to_cpu(findData.Type);
@@ -200,8 +180,7 @@ int cifs_get_inode_info_unix(struct inode **pinode,
 				inode->i_data.a_ops = &cifs_addr_ops;
 		} else if (S_ISDIR(inode->i_mode)) {
 			cFYI(1, ("Directory inode"));
-			inode->i_op = &cifs_dir_inode_ops;
-			inode->i_fop = &cifs_dir_ops;
+			cifs_dfs_ref_inode_op_fixup(is_remote, inode);
 		} else if (S_ISLNK(inode->i_mode)) {
 			cFYI(1, ("Symbolic Link inode"));
 			inode->i_op = &cifs_symlink_inode_ops;
@@ -326,8 +305,9 @@ int cifs_get_inode_info(struct inode **pinode,
 	struct cifsTconInfo *pTcon;
 	struct inode *inode;
 	struct cifs_sb_info *cifs_sb = CIFS_SB(sb);
-	char *tmp_path;
+	char *tmp_path = NULL;
 	char *buf = NULL;
+	int is_remote = 0;
 	int adjustTZ = FALSE;
 
 	pTcon = cifs_sb->tcon;
@@ -342,12 +322,38 @@ int cifs_get_inode_info(struct inode **pinode,
 
 	/* if file info not passed in then get it from server */
 	if (pfindData == NULL) {
+		int l_max_len;
+		const char *full_path;
+try_again_CIFSSMBQPathInfo:
+
 		buf = kmalloc(sizeof(FILE_ALL_INFO), GFP_KERNEL);
 		if (buf == NULL)
 			return -ENOMEM;
 		pfindData = (FILE_ALL_INFO *)buf;
+
+		cFYI(1, ("%s: pTcon->Flags: 0x%x", __FUNCTION__, pTcon->Flags));
+		if ((!is_remote) && (pTcon->Flags & 0x2)) {
+			/* use full path name for working with DFS */
+			l_max_len = strnlen(pTcon->treeName, MAX_TREE_SIZE + 1)
+				+ strnlen(search_path, MAX_PATHCONF) + 1;
+			kfree(tmp_path);
+			tmp_path = kmalloc(l_max_len, GFP_KERNEL);
+			if (tmp_path == NULL) {
+				kfree(buf);
+				return -ENOMEM;
+			}
+			strncpy(tmp_path, pTcon->treeName, l_max_len);
+			strcat(tmp_path, search_path);
+			tmp_path[l_max_len-1] = 0;
+			full_path = tmp_path;
+		} else {
+			full_path = search_path;
+		}
+
+		cFYI(1, ("%s: query server with full path: '%s'",
+					__FUNCTION__, full_path));
 		/* could do find first instead but this returns more info */
-		rc = CIFSSMBQPathInfo(xid, pTcon, search_path, pfindData,
+		rc = CIFSSMBQPathInfo(xid, pTcon, full_path, pfindData,
 			      0 /* not legacy */,
 			      cifs_sb->local_nls, cifs_sb->mnt_cifs_flags &
 				CIFS_MOUNT_MAP_SPECIAL_CHR);
@@ -364,27 +370,11 @@ int cifs_get_inode_info(struct inode **pinode,
 	}
 	/* dump_mem("\nQPathInfo return data",&findData, sizeof(findData)); */
 	if (rc) {
-		if (rc == -EREMOTE) {
-			tmp_path =
-			    kmalloc(strnlen
-				    (pTcon->treeName,
-				     MAX_TREE_SIZE + 1) +
-				    strnlen(search_path, MAX_PATHCONF) + 1,
-				    GFP_KERNEL);
-			if (tmp_path == NULL) {
-				kfree(buf);
-				return -ENOMEM;
-			}
-
-			strncpy(tmp_path, pTcon->treeName, MAX_TREE_SIZE);
-			strncat(tmp_path, search_path, MAX_PATHCONF);
-			rc = connect_to_dfs_path(xid, pTcon->ses,
-						 /* treename + */ tmp_path,
-						 cifs_sb->local_nls,
-						 cifs_sb->mnt_cifs_flags &
-						   CIFS_MOUNT_MAP_SPECIAL_CHR);
-			kfree(tmp_path);
-			/* BB fix up inode etc. */
+		if ((rc == -EREMOTE) && (!is_remote)) {
+			is_remote = 1;
+			kfree(buf);
+			buf = NULL;
+			goto try_again_CIFSSMBQPathInfo;
 		} else if (rc) {
 			kfree(buf);
 			return rc;
@@ -567,8 +557,7 @@ int cifs_get_inode_info(struct inode **pinode,
 				inode->i_data.a_ops = &cifs_addr_ops;
 		} else if (S_ISDIR(inode->i_mode)) {
 			cFYI(1, ("Directory inode"));
-			inode->i_op = &cifs_dir_inode_ops;
-			inode->i_fop = &cifs_dir_ops;
+			cifs_dfs_ref_inode_op_fixup(is_remote, inode);
 		} else if (S_ISLNK(inode->i_mode)) {
 			cFYI(1, ("Symbolic Link inode"));
 			inode->i_op = &cifs_symlink_inode_ops;
-- 
1.5.3.7


[-- Attachment #3: 0002-Fixed-mixed-case-name-in-structure-dfs_info3_param.patch --]
[-- Type: text/x-patch, Size: 1255 bytes --]

>From b15ae2e7e315abc7a3aad572b84d4604a4ab491b Mon Sep 17 00:00:00 2001
From: Igor Mammedov <niallain@gmail.com>
Date: Fri, 15 Feb 2008 18:26:34 +0300
Subject: [PATCH]  Fixed mixed case name in structure dfs_info3_param

Signed-off-by: Igor Mammedov <niallain@gmail.com>
---
 fs/cifs/cifs_dfs_ref.c |    2 +-
 fs/cifs/cifsglob.h     |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifs_dfs_ref.c b/fs/cifs/cifs_dfs_ref.c
index 3161986..39a0108 100644
--- a/fs/cifs/cifs_dfs_ref.c
+++ b/fs/cifs/cifs_dfs_ref.c
@@ -296,7 +296,7 @@ static void dump_referral(const struct dfs_info3_param *ref)
 	cFYI(1, ("DFS: node path: %s", ref->node_name));
 	cFYI(1, ("DFS: fl: %hd, srv_type: %hd", ref->flags, ref->server_type));
 	cFYI(1, ("DFS: ref_flags: %hd, path_consumed: %hd", ref->ref_flag,
-				ref->PathConsumed));
+				ref->path_consumed));
 }
 
 
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 5d32d8d..69a2e19 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -454,7 +454,7 @@ struct dir_notify_req {
 
 struct dfs_info3_param {
 	int flags; /* DFSREF_REFERRAL_SERVER, DFSREF_STORAGE_SERVER*/
-	int PathConsumed;
+	int path_consumed;
 	int server_type;
 	int ref_flag;
 	char *path_name;
-- 
1.5.3.7


[-- Attachment #4: Type: text/plain, Size: 172 bytes --]

_______________________________________________
linux-cifs-client mailing list
linux-cifs-client@lists.samba.org
https://lists.samba.org/mailman/listinfo/linux-cifs-client