Discussion:
[PATCH v5 1/6] cifs: Extract DFS referral expansion logic to separate function
(too old to reply)
Sean Finney
2011-04-11 13:19:30 UTC
Permalink
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 <jlayton-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
Signed-off-by: Sean Finney <seanius-***@public.gmane.org>
---
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,
+ 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
--
1.7.4.1
Sean Finney
2011-04-11 13:19:31 UTC
Permalink
Windows 2008 CIFS servers do not always return PATH_NOT_COVERED when
attempting to access a DFS share. Therefore, when checking for remote
shares, unconditionally ask for a DFS referral for the UNC (w/out prepath)
before continuing with previous behavior of attempting to access the UNC +
prepath and checking for PATH_NOT_COVERED.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=31092

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
Signed-off-by: Sean Finney <seanius-***@public.gmane.org>
---
fs/cifs/connect.c | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index c89d871..4873bac 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2934,6 +2934,24 @@ try_mount_again:
(tcon->ses->server->maxBuf - MAX_CIFS_HDR_SIZE));

remote_path_check:
+#ifdef CONFIG_CIFS_DFS_UPCALL
+ /*
+ * Perform an unconditional check for whether there are DFS
+ * referrals for this path without prefix, to provide support
+ * for DFS referrals from w2k8 servers which don't seem to respond
+ * with PATH_NOT_COVERED to requests that include the prefix.
+ * Chase the referral if found, otherwise continue normally.
+ */
+ if (referral_walks_count == 0) {
+ int refrc = expand_dfs_referral(xid, pSesInfo, volume_info,
+ cifs_sb, &mount_data, false);
+ if (!refrc) {
+ referral_walks_count++;
+ goto try_mount_again;
+ }
+ }
+#endif
+
/* check if a whole path (including prepath) is not remote */
if (!rc && tcon) {
/* build_path_to_root works only when we have a valid tcon */
--
1.7.4.1
Sean Finney
2011-04-11 13:19:34 UTC
Permalink
A relatively minor nit, but also clarified the "consensus" from the
preceding comments that it is in fact better to try for the kstrdup
early and cleanup while cleaning up is still a simple thing to do.

Reviewed-By: Steve French <smfrench-***@public.gmane.org>
Signed-off-by: Sean Finney <seanius-***@public.gmane.org>
---
fs/cifs/cifsfs.c | 17 ++++++-----------
1 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index fb6a2ad..2f646cb 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -134,24 +134,19 @@ cifs_read_super(struct super_block *sb, void *data,
cifs_sb->bdi.ra_pages = default_backing_dev_info.ra_pages;

#ifdef CONFIG_CIFS_DFS_UPCALL
- /* copy mount params to sb for use in submounts */
- /* BB: should we move this after the mount so we
- * do not have to do the copy on failed mounts?
- * BB: May be it is better to do simple copy before
- * complex operation (mount), and in case of fail
- * just exit instead of doing mount and attempting
- * undo it if this copy fails?*/
+ /*
+ * Copy mount params to sb for use in submounts. Better to do
+ * the copy here and deal with the error before cleanup gets
+ * complicated post-mount.
+ */
if (data) {
- int len = strlen(data);
- cifs_sb->mountdata = kzalloc(len + 1, GFP_KERNEL);
+ cifs_sb->mountdata = kstrndup(data, PAGE_SIZE, GFP_KERNEL);
if (cifs_sb->mountdata == NULL) {
bdi_destroy(&cifs_sb->bdi);
kfree(sb->s_fs_info);
sb->s_fs_info = NULL;
return -ENOMEM;
}
- strncpy(cifs_sb->mountdata, data, len + 1);
- cifs_sb->mountdata[len] = '\0';
}
#endif
--
1.7.4.1
Sean Finney
2011-04-11 13:19:32 UTC
Permalink
To keep strings passed to cifs_parse_mount_options re-usable (which is
needed to clean up the DFS referral handling), tokenize a copy of the
mount options instead. If values are needed from this tokenized string,
they too must be duplicated (previously, some options were copied and
others duplicated).

Since we are not on the critical path and any cleanup is relatively easy,
the extra memory usage shouldn't be a problem (and it is a bit simpler
than trying to implement something smarter).

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
Signed-off-by: Sean Finney <seanius-***@public.gmane.org>
---
fs/cifs/connect.c | 109 ++++++++++++++++++++++++++++++++++++-----------------
1 files changed, 74 insertions(+), 35 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 4873bac..232121e 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -807,11 +807,12 @@ extract_hostname(const char *unc)
}

static int
-cifs_parse_mount_options(char *options, const char *devname,
+cifs_parse_mount_options(const char *mountdata, const char *devname,
struct smb_vol *vol)
{
char *value;
char *data;
+ char *mountdata_copy, *options;
unsigned int temp_len, i, j;
char separator[2];
short int override_uid = -1;
@@ -851,9 +852,14 @@ cifs_parse_mount_options(char *options, const char *devname,

vol->actimeo = CIFS_DEF_ACTIMEO;

- if (!options)
- return 1;
+ if (!mountdata)
+ goto cifs_parse_mount_err;
+
+ mountdata_copy = kstrndup(mountdata, PAGE_SIZE, GFP_KERNEL);
+ if (!mountdata_copy)
+ goto cifs_parse_mount_err;

+ options = mountdata_copy;
if (strncmp(options, "sep=", 4) == 0) {
if (options[4] != 0) {
separator[0] = options[4];
@@ -878,17 +884,22 @@ cifs_parse_mount_options(char *options, const char *devname,
if (!value) {
printk(KERN_WARNING
"CIFS: invalid or missing username\n");
- return 1; /* needs_arg; */
+ goto cifs_parse_mount_err;
} else if (!*value) {
/* null user, ie anonymous, authentication */
vol->nullauth = 1;
}
if (strnlen(value, MAX_USERNAME_SIZE) <
MAX_USERNAME_SIZE) {
- vol->username = value;
+ vol->username = kstrdup(value, GFP_KERNEL);
+ if (!vol->username) {
+ printk(KERN_WARNING "CIFS: no memory "
+ "for username\n");
+ goto cifs_parse_mount_err;
+ }
} else {
printk(KERN_WARNING "CIFS: username too long\n");
- return 1;
+ goto cifs_parse_mount_err;
}
} else if (strnicmp(data, "pass", 4) == 0) {
if (!value) {
@@ -951,7 +962,7 @@ cifs_parse_mount_options(char *options, const char *devname,
if (vol->password == NULL) {
printk(KERN_WARNING "CIFS: no memory "
"for password\n");
- return 1;
+ goto cifs_parse_mount_err;
}
for (i = 0, j = 0; i < temp_len; i++, j++) {
vol->password[j] = value[i];
@@ -967,7 +978,7 @@ cifs_parse_mount_options(char *options, const char *devname,
if (vol->password == NULL) {
printk(KERN_WARNING "CIFS: no memory "
"for password\n");
- return 1;
+ goto cifs_parse_mount_err;
}
strcpy(vol->password, value);
}
@@ -977,11 +988,16 @@ cifs_parse_mount_options(char *options, const char *devname,
vol->UNCip = NULL;
} else if (strnlen(value, INET6_ADDRSTRLEN) <
INET6_ADDRSTRLEN) {
- vol->UNCip = value;
+ vol->UNCip = kstrdup(value, GFP_KERNEL);
+ if (!vol->UNCip) {
+ printk(KERN_WARNING "CIFS: no memory "
+ "for UNC IP\n");
+ goto cifs_parse_mount_err;
+ }
} else {
printk(KERN_WARNING "CIFS: ip address "
"too long\n");
- return 1;
+ goto cifs_parse_mount_err;
}
} else if (strnicmp(data, "sec", 3) == 0) {
if (!value || !*value) {
@@ -994,7 +1010,7 @@ cifs_parse_mount_options(char *options, const char *devname,
/* vol->secFlg |= CIFSSEC_MUST_SEAL |
CIFSSEC_MAY_KRB5; */
cERROR(1, "Krb5 cifs privacy not supported");
- return 1;
+ goto cifs_parse_mount_err;
} else if (strnicmp(value, "krb5", 4) == 0) {
vol->secFlg |= CIFSSEC_MAY_KRB5;
} else if (strnicmp(value, "ntlmsspi", 8) == 0) {
@@ -1024,7 +1040,7 @@ cifs_parse_mount_options(char *options, const char *devname,
vol->nullauth = 1;
} else {
cERROR(1, "bad security option: %s", value);
- return 1;
+ goto cifs_parse_mount_err;
}
} else if (strnicmp(data, "vers", 3) == 0) {
if (!value || !*value) {
@@ -1048,12 +1064,12 @@ cifs_parse_mount_options(char *options, const char *devname,
if (!value || !*value) {
printk(KERN_WARNING "CIFS: invalid path to "
"network resource\n");
- return 1; /* needs_arg; */
+ goto cifs_parse_mount_err;
}
if ((temp_len = strnlen(value, 300)) < 300) {
vol->UNC = kmalloc(temp_len+1, GFP_KERNEL);
if (vol->UNC == NULL)
- return 1;
+ goto cifs_parse_mount_err;
strcpy(vol->UNC, value);
if (strncmp(vol->UNC, "//", 2) == 0) {
vol->UNC[0] = '\\';
@@ -1062,27 +1078,32 @@ cifs_parse_mount_options(char *options, const char *devname,
printk(KERN_WARNING
"CIFS: UNC Path does not begin "
"with // or \\\\ \n");
- return 1;
+ goto cifs_parse_mount_err;
}
} else {
printk(KERN_WARNING "CIFS: UNC name too long\n");
- return 1;
+ goto cifs_parse_mount_err;
}
} else if ((strnicmp(data, "domain", 3) == 0)
|| (strnicmp(data, "workgroup", 5) == 0)) {
if (!value || !*value) {
printk(KERN_WARNING "CIFS: invalid domain name\n");
- return 1; /* needs_arg; */
+ goto cifs_parse_mount_err;
}
/* BB are there cases in which a comma can be valid in
a domain name and need special handling? */
if (strnlen(value, 256) < 256) {
- vol->domainname = value;
+ vol->domainname = kstrdup(value, GFP_KERNEL);
+ if (!vol->domainname) {
+ printk(KERN_WARNING "CIFS: no memory "
+ "for domainname\n");
+ goto cifs_parse_mount_err;
+ }
cFYI(1, "Domain name set");
} else {
printk(KERN_WARNING "CIFS: domain name too "
"long\n");
- return 1;
+ goto cifs_parse_mount_err;
}
} else if (strnicmp(data, "srcaddr", 7) == 0) {
vol->srcaddr.ss_family = AF_UNSPEC;
@@ -1090,7 +1111,7 @@ cifs_parse_mount_options(char *options, const char *devname,
if (!value || !*value) {
printk(KERN_WARNING "CIFS: srcaddr value"
" not specified.\n");
- return 1; /* needs_arg; */
+ goto cifs_parse_mount_err;
}
i = cifs_convert_address((struct sockaddr *)&vol->srcaddr,
value, strlen(value));
@@ -1098,20 +1119,20 @@ cifs_parse_mount_options(char *options, const char *devname,
printk(KERN_WARNING "CIFS: Could not parse"
" srcaddr: %s\n",
value);
- return 1;
+ goto cifs_parse_mount_err;
}
} else if (strnicmp(data, "prefixpath", 10) == 0) {
if (!value || !*value) {
printk(KERN_WARNING
"CIFS: invalid path prefix\n");
- return 1; /* needs_argument */
+ goto cifs_parse_mount_err;
}
if ((temp_len = strnlen(value, 1024)) < 1024) {
if (value[0] != '/')
temp_len++; /* missing leading slash */
vol->prepath = kmalloc(temp_len+1, GFP_KERNEL);
if (vol->prepath == NULL)
- return 1;
+ goto cifs_parse_mount_err;
if (value[0] != '/') {
vol->prepath[0] = '/';
strcpy(vol->prepath+1, value);
@@ -1120,24 +1141,33 @@ cifs_parse_mount_options(char *options, const char *devname,
cFYI(1, "prefix path %s", vol->prepath);
} else {
printk(KERN_WARNING "CIFS: prefix too long\n");
- return 1;
+ goto cifs_parse_mount_err;
}
} else if (strnicmp(data, "iocharset", 9) == 0) {
if (!value || !*value) {
printk(KERN_WARNING "CIFS: invalid iocharset "
"specified\n");
- return 1; /* needs_arg; */
+ goto cifs_parse_mount_err;
}
if (strnlen(value, 65) < 65) {
- if (strnicmp(value, "default", 7))
- vol->iocharset = value;
+ if (strnicmp(value, "default", 7)) {
+ vol->iocharset = kstrdup(value,
+ GFP_KERNEL);
+
+ if (!vol->iocharset) {
+ printk(KERN_WARNING "CIFS: no "
+ "memory for"
+ "charset\n");
+ goto cifs_parse_mount_err;
+ }
+ }
/* if iocharset not set then load_nls_default
is used by caller */
cFYI(1, "iocharset set to %s", value);
} else {
printk(KERN_WARNING "CIFS: iocharset name "
"too long.\n");
- return 1;
+ goto cifs_parse_mount_err;
}
} else if (!strnicmp(data, "uid", 3) && value && *value) {
vol->linux_uid = simple_strtoul(value, &value, 0);
@@ -1250,7 +1280,7 @@ cifs_parse_mount_options(char *options, const char *devname,
if (vol->actimeo > CIFS_MAX_ACTIMEO) {
cERROR(1, "CIFS: attribute cache"
"timeout too large");
- return 1;
+ goto cifs_parse_mount_err;
}
}
} else if (strnicmp(data, "credentials", 4) == 0) {
@@ -1394,7 +1424,7 @@ cifs_parse_mount_options(char *options, const char *devname,
#ifndef CONFIG_CIFS_FSCACHE
cERROR(1, "FS-Cache support needs CONFIG_CIFS_FSCACHE"
"kernel config option set");
- return 1;
+ goto cifs_parse_mount_err;
#endif
vol->fsc = true;
} else if (strnicmp(data, "mfsymlinks", 10) == 0) {
@@ -1409,12 +1439,12 @@ cifs_parse_mount_options(char *options, const char *devname,
if (devname == NULL) {
printk(KERN_WARNING "CIFS: Missing UNC name for mount "
"target\n");
- return 1;
+ goto cifs_parse_mount_err;
}
if ((temp_len = strnlen(devname, 300)) < 300) {
vol->UNC = kmalloc(temp_len+1, GFP_KERNEL);
if (vol->UNC == NULL)
- return 1;
+ goto cifs_parse_mount_err;
strcpy(vol->UNC, devname);
if (strncmp(vol->UNC, "//", 2) == 0) {
vol->UNC[0] = '\\';
@@ -1422,21 +1452,21 @@ cifs_parse_mount_options(char *options, const char *devname,
} else if (strncmp(vol->UNC, "\\\\", 2) != 0) {
printk(KERN_WARNING "CIFS: UNC Path does not "
"begin with // or \\\\ \n");
- return 1;
+ goto cifs_parse_mount_err;
}
value = strpbrk(vol->UNC+2, "/\\");
if (value)
*value = '\\';
} else {
printk(KERN_WARNING "CIFS: UNC name too long\n");
- return 1;
+ goto cifs_parse_mount_err;
}
}

if (vol->multiuser && !(vol->secFlg & CIFSSEC_MAY_KRB5)) {
cERROR(1, "Multiuser mounts currently require krb5 "
"authentication!");
- return 1;
+ goto cifs_parse_mount_err;
}

if (vol->UNCip == NULL)
@@ -1454,7 +1484,12 @@ cifs_parse_mount_options(char *options, const char *devname,
printk(KERN_NOTICE "CIFS: ignoring forcegid mount option "
"specified with no gid= option.\n");

+ kfree(mountdata_copy);
return 0;
+
+cifs_parse_mount_err:
+ kfree(mountdata_copy);
+ return 1;
}

/** Returns true if srcaddr isn't specified and rhs isn't
@@ -2704,8 +2739,12 @@ cleanup_volume_info(struct smb_vol **pvolume_info)
return;

volume_info = *pvolume_info;
+ kfree(volume_info->username);
kzfree(volume_info->password);
kfree(volume_info->UNC);
+ kfree(volume_info->UNCip);
+ kfree(volume_info->domainname);
+ kfree(volume_info->iocharset);
kfree(volume_info->prepath);
kfree(volume_info);
*pvolume_info = NULL;
--
1.7.4.1
Sean Finney
2011-04-11 13:19:33 UTC
Permalink
With CONFIG_DFS_UPCALL enabled, maintain the submount options in
cifs_sb->mountdata, simplifying the code just a bit as well as making
corner-case allocation problems less likely.

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
Signed-off-by: Sean Finney <seanius-***@public.gmane.org>
---
fs/cifs/connect.c | 24 +++++++++++-------------
1 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 232121e..08185e5 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2785,9 +2785,9 @@ build_unc_path_to_root(const struct smb_vol *volume_info,
/*
* 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.
+ * If a referral is found, cifs_sb->mountdata will be (re-)allocated
+ * to a 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.
@@ -2795,7 +2795,7 @@ build_unc_path_to_root(const struct smb_vol *volume_info,
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 check_prefix)
{
int rc;
unsigned int num_referrals = 0;
@@ -2823,11 +2823,14 @@ expand_dfs_referral(int xid, struct cifs_ses *pSesInfo,
free_dfs_info_array(referrals, num_referrals);
kfree(fake_devname);

+ if (cifs_sb->mountdata != NULL)
+ kfree(cifs_sb->mountdata);
+
if (IS_ERR(mdata)) {
rc = PTR_ERR(mdata);
mdata = NULL;
}
- *mount_data = mdata;
+ cifs_sb->mountdata = mdata;
}
kfree(full_path);
return rc;
@@ -2850,6 +2853,7 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
#ifdef CONFIG_CIFS_DFS_UPCALL
int referral_walks_count = 0;
try_mount_again:
+ mount_data = cifs_sb->mountdata;

/* cleanup activities if we're chasing a referral */
if (referral_walks_count) {
@@ -2983,7 +2987,7 @@ remote_path_check:
*/
if (referral_walks_count == 0) {
int refrc = expand_dfs_referral(xid, pSesInfo, volume_info,
- cifs_sb, &mount_data, false);
+ cifs_sb, false);
if (!refrc) {
referral_walks_count++;
goto try_mount_again;
@@ -3025,17 +3029,13 @@ remote_path_check:
convert_delimiter(cifs_sb->prepath,
CIFS_DIR_SEP(cifs_sb));

- if (mount_data != mount_data_global)
- kfree(mount_data);
-
rc = expand_dfs_referral(xid, pSesInfo, volume_info, cifs_sb,
- &mount_data, true);
+ true);

if (!rc) {
referral_walks_count++;
goto try_mount_again;
}
- mount_data = NULL;
goto mount_fail_check;
#else /* No DFS support, return error on mount */
rc = -EOPNOTSUPP;
@@ -3069,8 +3069,6 @@ remote_path_check:
mount_fail_check:
/* on error free sesinfo and tcon struct if needed */
if (rc) {
- if (mount_data != mount_data_global)
- kfree(mount_data);
/* If find_unc succeeded then rc == 0 so we can not end */
/* up accidently freeing someone elses tcon struct */
if (tcon)
--
1.7.4.1
Sean Finney
2011-04-11 13:19:35 UTC
Permalink
Previously mount options were copied and updated in the cifs_sb_info
struct only when CONFIG_CIFS_DFS_UPCALL was enabled. Making this
information generally available allows us to remove a number of ifdefs,
extra function params, and temporary variables.

Reviewed-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
Signed-off-by: Sean Finney <seanius-***@public.gmane.org>
---
fs/cifs/cifs_fs_sb.h | 4 +---
fs/cifs/cifsfs.c | 8 +-------
fs/cifs/cifsproto.h | 2 +-
fs/cifs/connect.c | 8 +++-----
4 files changed, 6 insertions(+), 16 deletions(-)

diff --git a/fs/cifs/cifs_fs_sb.h b/fs/cifs/cifs_fs_sb.h
index 1ef54ab..adecd36 100644
--- a/fs/cifs/cifs_fs_sb.h
+++ b/fs/cifs/cifs_fs_sb.h
@@ -60,9 +60,7 @@ struct cifs_sb_info {
unsigned int mnt_cifs_flags;
int prepathlen;
char *prepath; /* relative path under the share to mount to */
-#ifdef CONFIG_CIFS_DFS_UPCALL
- char *mountdata; /* mount options received at mount time */
-#endif
+ char *mountdata; /* options received at mount time or via DFS refs */
struct backing_dev_info bdi;
struct delayed_work prune_tlinks;
};
diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 2f646cb..9cfbfd2 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -133,7 +133,6 @@ cifs_read_super(struct super_block *sb, void *data,
}
cifs_sb->bdi.ra_pages = default_backing_dev_info.ra_pages;

-#ifdef CONFIG_CIFS_DFS_UPCALL
/*
* Copy mount params to sb for use in submounts. Better to do
* the copy here and deal with the error before cleanup gets
@@ -148,9 +147,8 @@ cifs_read_super(struct super_block *sb, void *data,
return -ENOMEM;
}
}
-#endif

- rc = cifs_mount(sb, cifs_sb, data, devname);
+ rc = cifs_mount(sb, cifs_sb, devname);

if (rc) {
if (!silent)
@@ -202,12 +200,10 @@ out_no_root:

out_mount_failed:
if (cifs_sb) {
-#ifdef CONFIG_CIFS_DFS_UPCALL
if (cifs_sb->mountdata) {
kfree(cifs_sb->mountdata);
cifs_sb->mountdata = NULL;
}
-#endif
unload_nls(cifs_sb->local_nls);
bdi_destroy(&cifs_sb->bdi);
kfree(cifs_sb);
@@ -231,12 +227,10 @@ cifs_put_super(struct super_block *sb)
rc = cifs_umount(sb, cifs_sb);
if (rc)
cERROR(1, "cifs_umount failed with return code %d", rc);
-#ifdef CONFIG_CIFS_DFS_UPCALL
if (cifs_sb->mountdata) {
kfree(cifs_sb->mountdata);
cifs_sb->mountdata = NULL;
}
-#endif

unload_nls(cifs_sb->local_nls);
bdi_destroy(&cifs_sb->bdi);
diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h
index 76c4dc7f..9985f99 100644
--- a/fs/cifs/cifsproto.h
+++ b/fs/cifs/cifsproto.h
@@ -146,7 +146,7 @@ extern struct cifs_ntsd *get_cifs_acl(struct cifs_sb_info *, struct inode *,
extern int set_cifs_acl(struct cifs_ntsd *, __u32, struct inode *,
const char *);

-extern int cifs_mount(struct super_block *, struct cifs_sb_info *, char *,
+extern int cifs_mount(struct super_block *, struct cifs_sb_info *,
const char *);
extern int cifs_umount(struct super_block *, struct cifs_sb_info *);
extern void cifs_dfs_release_automount_timer(void);
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 08185e5..7742de7 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2839,7 +2839,7 @@ expand_dfs_referral(int xid, struct cifs_ses *pSesInfo,

int
cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
- char *mount_data_global, const char *devname)
+ const char *devname)
{
int rc;
int xid;
@@ -2848,13 +2848,10 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
struct cifs_tcon *tcon;
struct TCP_Server_Info *srvTcp;
char *full_path;
- char *mount_data = mount_data_global;
struct tcon_link *tlink;
#ifdef CONFIG_CIFS_DFS_UPCALL
int referral_walks_count = 0;
try_mount_again:
- mount_data = cifs_sb->mountdata;
-
/* cleanup activities if we're chasing a referral */
if (referral_walks_count) {
if (tcon)
@@ -2881,7 +2878,8 @@ try_mount_again:
goto out;
}

- if (cifs_parse_mount_options(mount_data, devname, volume_info)) {
+ if (cifs_parse_mount_options(cifs_sb->mountdata, devname,
+ volume_info)) {
rc = -EINVAL;
goto out;
}
--
1.7.4.1
Sean Finney
2011-04-18 14:06:40 UTC
Permalink
Hi Steve,

Just a friendly ping to ask on the status of inclusion for the following
patch set for supporing w2k8 DFS shares. Only the first two are strictly
necessary for adding the support, but the remainder were the results
of some code review and "could you clean that up while you're at it"
suggestions from Jeff and I don't think are entirely controversial.

I also realized that I sent the original patchset to the git committer
address, which is not the same email address that you have been using
on-list (oops), sorry about that!


Sean
Steve French
2011-04-18 14:17:27 UTC
Permalink
I will be merging additional patches (probably including this one) to
a for-next branch of cifs-2.6.git
Post by Sean Finney
Hi Steve,
Just a friendly ping to ask on the status of inclusion for the follow=
ing
Post by Sean Finney
patch set for supporing w2k8 DFS shares. =A0Only the first two are st=
rictly
Post by Sean Finney
necessary for adding the support, but the remainder were the results
of some code review and "could you clean that up while you're at it"
suggestions from Jeff and I don't think are entirely controversial.
I also realized that I sent the original patchset to the git committe=
r
Post by Sean Finney
address, which is not the same email address that you have been using
on-list (oops), sorry about that!
=A0 =A0 =A0 =A0Sean
--=20
Thanks,

Steve

Loading...