[v4,1/6] cifs: Extract DFS referral expansion logic to separate function

Commit Message

Sean Finney April 6, 2011, 2:26 p.m. UTC

The logic behind the expansion of DFS referrals is now extracted from
cifs_mount into a new static function, expand_dfs_referral.  This will
reduce duplicate code in upcoming commits.

Reviewed-by: Jeff Layton <[e-mail]>
Signed-off-by: Sean Finney <[e-mail]>
---
 fs/cifs/connect.c |  105 +++++++++++++++++++++++++++++++++++------------------
 1 files changed, 69 insertions(+), 36 deletions(-)

Comments

Suresh Jayaraman April 26, 2011, 5:43 a.m. UTC | #1

On 04/06/2011 07:56 PM, Sean Finney wrote:
> The logic behind the expansion of DFS referrals is now extracted from
> cifs_mount into a new static function, expand_dfs_referral.  This will
> reduce duplicate code in upcoming commits.
>
> Reviewed-by: Jeff Layton<[e-mail]>
> Signed-off-by: Sean Finney<[e-mail]>
> ---
>   fs/cifs/connect.c |  105 +++++++++++++++++++++++++++++++++++------------------
>   1 files changed, 69 insertions(+), 36 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 94e60c5..c89d871 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2742,6 +2742,57 @@ build_unc_path_to_root(const struct smb_vol *volume_info,
>   	full_path[unc_len + cifs_sb->prepathlen] = 0; /* add trailing null */
>   	return full_path;
>   }
> +
> +/*
> + * Perform a dfs referral query for a share and (optionally) prefix
> + *
> + * If a referral is found, mount_data will be set to point at a newly
> + * allocated string containing updated options for the submount.
> + * Otherwise it will be left untouched.
> + *
> + * Returns the rc from get_dfs_path to the caller, which can be used to
> + * determine whether there were referrals.
> + */
> +static int
> +expand_dfs_referral(int xid, struct cifs_ses *pSesInfo,
				^^^ Did you mean struct cifsSesInfo ?

I'm not seeing any other patch renaming cifsSesInfo to cifs_ses. Or Am I 
missing something. Sorry about pitching in so late..
			
The compiler gives a warning, but nevertheless it compiles... looks odd.

Sean Finney April 26, 2011, 11:40 a.m. UTC | #2

Hi Suresh,

On Tue, Apr 26, 2011 at 11:13:16AM +0530, Suresh Jayaraman wrote:
>> +static int
>> +expand_dfs_referral(int xid, struct cifs_ses *pSesInfo,
> 				^^^ Did you mean struct cifsSesInfo ?
>
> I'm not seeing any other patch renaming cifsSesInfo to cifs_ses. Or Am I  
> missing something. Sorry about pitching in so late..
> 			
> The compiler gives a warning, but nevertheless it compiles... looks odd.

this is based on steve's master/for-next branch, which among other things
has commit 1c1c95b7b97ee797832c0f973419983faa4761ed, which renames
cifsSesInfo to cifs_ses.  IIRC that was the only thing different between
the two branches, so if you'd like to try it against the mainline kernel
you can probably just re-rename the struct back to struct cifsSesInfo.


	sean
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 94e60c5..c89d871 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2742,6 +2742,57 @@  build_unc_path_to_root(const struct smb_vol *volume_info,
 	full_path[unc_len + cifs_sb->prepathlen] = 0; /* add trailing null */
 	return full_path;
 }
+
+/*
+ * Perform a dfs referral query for a share and (optionally) prefix
+ *
+ * If a referral is found, mount_data will be set to point at a newly
+ * allocated string containing updated options for the submount.
+ * Otherwise it will be left untouched.
+ *
+ * Returns the rc from get_dfs_path to the caller, which can be used to
+ * determine whether there were referrals.
+ */
+static int
+expand_dfs_referral(int xid, struct cifs_ses *pSesInfo,
+		    struct smb_vol *volume_info, struct cifs_sb_info *cifs_sb,
+		    char **mount_data, int check_prefix)
+{
+	int rc;
+	unsigned int num_referrals = 0;
+	struct dfs_info3_param *referrals = NULL;
+	char *full_path = NULL, *ref_path = NULL, *mdata = NULL;
+
+	full_path = build_unc_path_to_root(volume_info, cifs_sb);
+	if (IS_ERR(full_path))
+		return PTR_ERR(full_path);
+
+	/* For DFS paths, skip the first '\' of the UNC */
+	ref_path = check_prefix ? full_path + 1 : volume_info->UNC + 1;
+
+	rc = get_dfs_path(xid, pSesInfo , ref_path, cifs_sb->local_nls,
+			  &num_referrals, &referrals,
+			  cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
+
+	if (!rc && num_referrals > 0) {
+		char *fake_devname = NULL;
+
+		mdata = cifs_compose_mount_options(cifs_sb->mountdata,
+						   full_path + 1, referrals,
+						   &fake_devname);
+
+		free_dfs_info_array(referrals, num_referrals);
+		kfree(fake_devname);
+
+		if (IS_ERR(mdata)) {
+			rc = PTR_ERR(mdata);
+			mdata = NULL;
+		}
+		*mount_data = mdata;
+	}
+	kfree(full_path);
+	return rc;
+}
 #endif
 
 int
@@ -2758,10 +2809,19 @@  cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
 	char *mount_data = mount_data_global;
 	struct tcon_link *tlink;
 #ifdef CONFIG_CIFS_DFS_UPCALL
-	struct dfs_info3_param *referrals = NULL;
-	unsigned int num_referrals = 0;
 	int referral_walks_count = 0;
 try_mount_again:
+
+	/* cleanup activities if we're chasing a referral */
+	if (referral_walks_count) {
+		if (tcon)
+			cifs_put_tcon(tcon);
+		else if (pSesInfo)
+			cifs_put_smb_ses(pSesInfo);
+
+		cleanup_volume_info(&volume_info);
+		FreeXid(xid);
+	}
 #endif
 	rc = 0;
 	tcon = NULL;
@@ -2907,46 +2967,19 @@  remote_path_check:
 		if ((cifs_sb->mnt_cifs_flags & CIFS_MOUNT_POSIX_PATHS) == 0)
 			convert_delimiter(cifs_sb->prepath,
 					CIFS_DIR_SEP(cifs_sb));
-		full_path = build_unc_path_to_root(volume_info, cifs_sb);
-		if (IS_ERR(full_path)) {
-			rc = PTR_ERR(full_path);
-			goto mount_fail_check;
-		}
 
-		cFYI(1, "Getting referral for: %s", full_path);
-		rc = get_dfs_path(xid, pSesInfo , full_path + 1,
-			cifs_sb->local_nls, &num_referrals, &referrals,
-			cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MAP_SPECIAL_CHR);
-		if (!rc && num_referrals > 0) {
-			char *fake_devname = NULL;
-
-			if (mount_data != mount_data_global)
-				kfree(mount_data);
-
-			mount_data = cifs_compose_mount_options(
-					cifs_sb->mountdata, full_path + 1,
-					referrals, &fake_devname);
-
-			free_dfs_info_array(referrals, num_referrals);
-			kfree(fake_devname);
-			kfree(full_path);
-
-			if (IS_ERR(mount_data)) {
-				rc = PTR_ERR(mount_data);
-				mount_data = NULL;
-				goto mount_fail_check;
-			}
+		if (mount_data != mount_data_global)
+			kfree(mount_data);
 
-			if (tcon)
-				cifs_put_tcon(tcon);
-			else if (pSesInfo)
-				cifs_put_smb_ses(pSesInfo);
+		rc = expand_dfs_referral(xid, pSesInfo, volume_info, cifs_sb,
+					 &mount_data, true);
 
-			cleanup_volume_info(&volume_info);
+		if (!rc) {
 			referral_walks_count++;
-			FreeXid(xid);
 			goto try_mount_again;
 		}
+		mount_data = NULL;
+		goto mount_fail_check;
 #else /* No DFS support, return error on mount */
 		rc = -EOPNOTSUPP;
 #endif