[PATCH] mount.cifs: remove smb2 multicall binary code

[PATCH] mount.cifs: remove smb2 multicall binary code

[Jeff Layton]

This was added when it was thought that smb2 would be a different fstype
altogether. Now that we are not adding a separate fstype, this code is
no longer needed since nothing will ever call /sbin/mount.smb2.

Cc: Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
---
 mount.cifs.c |   61 +++------------------------------------------------------
 1 files changed, 4 insertions(+), 57 deletions(-)

diff --git a/mount.cifs.c b/mount.cifs.c
index 3041987..2d1d842 100644
--- a/mount.cifs.c
+++ b/mount.cifs.c
@@ -181,7 +181,6 @@ struct parsed_mount_info {
 
 const char *thisprogram;
 const char *cifs_fstype = "cifs";
-const char *smb2_fstype = "smb2";
 
 static int parse_unc(const char *unc_name, struct parsed_mount_info *parsed_info);
 
@@ -250,7 +249,7 @@ check_fstab(const char *progname, const char *mountpoint, const char *devname,
 
 BB end finish BB */
 
-static int mount_cifs_usage(FILE * stream)
+static int mount_usage(FILE * stream)
 {
 	fprintf(stream, "\nUsage:  %s <remotetarget> <dir> -o <options>\n",
 		thisprogram);
@@ -290,52 +289,6 @@ static int mount_cifs_usage(FILE * stream)
 	return 0;
 }
 
-static int mount_smb2_usage(FILE *stream)
-{
-	fprintf(stream, "\nUsage:  %s <remotetarget> <dir> -o <options>\n",
-		thisprogram);
-	fprintf(stream, "\nMount the remote target, specified as a UNC name,");
-	fprintf(stream, " to a local directory.\n\nOptions:\n");
-	fprintf(stream, "\tuser=<arg>\n\tpass=<arg>\n\tdom=<arg>\n");
-	fprintf(stream, "\nLess commonly used options:");
-	fprintf(stream,
-		"\n\tcredentials=<filename>,guest,perm,noperm,rw,ro,");
-	fprintf(stream,
-		"\n\tsep=<char>,iocharset=<codepage>,exec,noexec");
-	fprintf(stream,
-		"\n\tnolock,directio,sec=<authentication mechanism>,sign");
-	fprintf(stream,
-		"\n\tuid=<uid>,gid=<gid>,dir_mode=<mode>,file_mode=<mode>");
-	fprintf(stream, "\n\nRarely used options:");
-	fprintf(stream,
-		"\n\tport=<tcpport>,rsize=<size>,wsize=<size>,unc=<unc_name>,ip=<ip_address>,");
-	fprintf(stream,
-		"\n\tdev,nodev,hard,soft,intr,");
-	fprintf(stream,
-		"\n\tnointr,ignorecase,noacl,prefixpath=<path>,nobrl");
-	fprintf(stream,
-		"\n\nOptions are described in more detail in the manual page");
-	fprintf(stream, "\n\tman 8 mount.smb2\n");
-	fprintf(stream, "\nTo display the version number of the mount helper:");
-	fprintf(stream, "\n\tmount.smb2 -V\n");
-
-	if (stream == stderr)
-		return EX_USAGE;
-	return 0;
-}
-
-static int mount_usage(FILE *stream)
-{
-	int rc;
-
-	if (strcmp(thisprogram, "mount.smb2") == 0)
-		rc = mount_smb2_usage(stream);
-	else
-		rc = mount_cifs_usage(stream);
-
-	return rc;
-}
-
 /*
  * CIFS has to "escape" commas in the password field so that they don't
  * end up getting confused for option delimiters. Copy password into pw
@@ -1928,7 +1881,6 @@ int main(int argc, char **argv)
 	size_t options_size = MAX_OPTIONS_LEN;
 	struct parsed_mount_info *parsed_info = NULL;
 	pid_t pid;
-	const char *fstype;
 
 	rc = check_setuid();
 	if (rc)
@@ -2109,14 +2061,9 @@ mount_retry:
 	if (rc)
 		goto mount_exit;
 
-	if (strcmp(thisprogram, "mount.smb2") == 0)
-		fstype = smb2_fstype;
-	else
-		fstype = cifs_fstype;
-
 	if (!parsed_info->fakemnt) {
 		toggle_dac_capability(0, 1);
-		rc = mount(orig_dev, ".", fstype, parsed_info->flags, options);
+		rc = mount(orig_dev, ".", cifs_fstype, parsed_info->flags, options);
 		toggle_dac_capability(0, 0);
 		if (rc == 0)
 			goto do_mtab;
@@ -2133,7 +2080,7 @@ mount_retry:
 			goto mount_retry;
 		case ENODEV:
 			fprintf(stderr,
-				"mount error: %s filesystem not supported by the system\n", fstype);
+				"mount error: %s filesystem not supported by the system\n", cifs_fstype);
 			break;
 		case ENXIO:
 			if (!already_uppercased &&
@@ -2163,7 +2110,7 @@ do_mtab:
 				goto mount_exit;
 		}
 
-		rc = add_mtab(orig_dev, mountpoint, parsed_info->flags, fstype);
+		rc = add_mtab(orig_dev, mountpoint, parsed_info->flags, cifs_fstype);
 	}
 
 mount_exit:
-- 
1.7.7.6

Re: [PATCH] mount.cifs: remove smb2 multicall binary code

[Steve French]

This may be fine but we need to ask the obvious question ...

Are we sure that we don't ever want a distinct fstype (cifs vs. smb2)
in the long run?

It does seem strange that smb2 and cifs would be the same fstype.  For a while
fstab supported nfs4 as an fstype (as distinct from nfs (nfs3)) cifs
and smb2 are
quite a bit different.

Certainly calling smb3 or smb2 as cifs should work - but I have the feeling
that smb3 will in the long run be better known as a name (than cifs) so
we will have to provide synonyms (mount.smb3 could basically would
be mount.cifs but pass vers=3)

On Fri, May 11, 2012 at 8:00 PM, Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> This was added when it was thought that smb2 would be a different fstype
> altogether. Now that we are not adding a separate fstype, this code is
> no longer needed since nothing will ever call /sbin/mount.smb2.
>
> Cc: Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  mount.cifs.c |   61 +++------------------------------------------------------
>  1 files changed, 4 insertions(+), 57 deletions(-)
>
> diff --git a/mount.cifs.c b/mount.cifs.c
> index 3041987..2d1d842 100644
> --- a/mount.cifs.c
> +++ b/mount.cifs.c
> @@ -181,7 +181,6 @@ struct parsed_mount_info {
>
>  const char *thisprogram;
>  const char *cifs_fstype = "cifs";
> -const char *smb2_fstype = "smb2";
>
>  static int parse_unc(const char *unc_name, struct parsed_mount_info *parsed_info);
>
> @@ -250,7 +249,7 @@ check_fstab(const char *progname, const char *mountpoint, const char *devname,
>
>  BB end finish BB */
>
> -static int mount_cifs_usage(FILE * stream)
> +static int mount_usage(FILE * stream)
>  {
>        fprintf(stream, "\nUsage:  %s <remotetarget> <dir> -o <options>\n",
>                thisprogram);
> @@ -290,52 +289,6 @@ static int mount_cifs_usage(FILE * stream)
>        return 0;
>  }
>
> -static int mount_smb2_usage(FILE *stream)
> -{
> -       fprintf(stream, "\nUsage:  %s <remotetarget> <dir> -o <options>\n",
> -               thisprogram);
> -       fprintf(stream, "\nMount the remote target, specified as a UNC name,");
> -       fprintf(stream, " to a local directory.\n\nOptions:\n");
> -       fprintf(stream, "\tuser=<arg>\n\tpass=<arg>\n\tdom=<arg>\n");
> -       fprintf(stream, "\nLess commonly used options:");
> -       fprintf(stream,
> -               "\n\tcredentials=<filename>,guest,perm,noperm,rw,ro,");
> -       fprintf(stream,
> -               "\n\tsep=<char>,iocharset=<codepage>,exec,noexec");
> -       fprintf(stream,
> -               "\n\tnolock,directio,sec=<authentication mechanism>,sign");
> -       fprintf(stream,
> -               "\n\tuid=<uid>,gid=<gid>,dir_mode=<mode>,file_mode=<mode>");
> -       fprintf(stream, "\n\nRarely used options:");
> -       fprintf(stream,
> -               "\n\tport=<tcpport>,rsize=<size>,wsize=<size>,unc=<unc_name>,ip=<ip_address>,");
> -       fprintf(stream,
> -               "\n\tdev,nodev,hard,soft,intr,");
> -       fprintf(stream,
> -               "\n\tnointr,ignorecase,noacl,prefixpath=<path>,nobrl");
> -       fprintf(stream,
> -               "\n\nOptions are described in more detail in the manual page");
> -       fprintf(stream, "\n\tman 8 mount.smb2\n");
> -       fprintf(stream, "\nTo display the version number of the mount helper:");
> -       fprintf(stream, "\n\tmount.smb2 -V\n");
> -
> -       if (stream == stderr)
> -               return EX_USAGE;
> -       return 0;
> -}
> -
> -static int mount_usage(FILE *stream)
> -{
> -       int rc;
> -
> -       if (strcmp(thisprogram, "mount.smb2") == 0)
> -               rc = mount_smb2_usage(stream);
> -       else
> -               rc = mount_cifs_usage(stream);
> -
> -       return rc;
> -}
> -
>  /*
>  * CIFS has to "escape" commas in the password field so that they don't
>  * end up getting confused for option delimiters. Copy password into pw
> @@ -1928,7 +1881,6 @@ int main(int argc, char **argv)
>        size_t options_size = MAX_OPTIONS_LEN;
>        struct parsed_mount_info *parsed_info = NULL;
>        pid_t pid;
> -       const char *fstype;
>
>        rc = check_setuid();
>        if (rc)
> @@ -2109,14 +2061,9 @@ mount_retry:
>        if (rc)
>                goto mount_exit;
>
> -       if (strcmp(thisprogram, "mount.smb2") == 0)
> -               fstype = smb2_fstype;
> -       else
> -               fstype = cifs_fstype;
> -
>        if (!parsed_info->fakemnt) {
>                toggle_dac_capability(0, 1);
> -               rc = mount(orig_dev, ".", fstype, parsed_info->flags, options);
> +               rc = mount(orig_dev, ".", cifs_fstype, parsed_info->flags, options);
>                toggle_dac_capability(0, 0);
>                if (rc == 0)
>                        goto do_mtab;
> @@ -2133,7 +2080,7 @@ mount_retry:
>                        goto mount_retry;
>                case ENODEV:
>                        fprintf(stderr,
> -                               "mount error: %s filesystem not supported by the system\n", fstype);
> +                               "mount error: %s filesystem not supported by the system\n", cifs_fstype);
>                        break;
>                case ENXIO:
>                        if (!already_uppercased &&
> @@ -2163,7 +2110,7 @@ do_mtab:
>                                goto mount_exit;
>                }
>
> -               rc = add_mtab(orig_dev, mountpoint, parsed_info->flags, fstype);
> +               rc = add_mtab(orig_dev, mountpoint, parsed_info->flags, cifs_fstype);
>        }
>
>  mount_exit:
> --
> 1.7.7.6
>



-- 
Thanks,

Steve

Re: [PATCH] mount.cifs: remove smb2 multicall binary code

[Jeff Layton]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Fri, 11 May 2012 21:43:57 -0500
Steve French <smfrench@gmail.com> wrote:

> This may be fine but we need to ask the obvious question ...
> 
> Are we sure that we don't ever want a distinct fstype (cifs vs. smb2)
> in the long run?
> 
> It does seem strange that smb2 and cifs would be the same fstype.  For a while
> fstab supported nfs4 as an fstype (as distinct from nfs (nfs3)) cifs
> and smb2 are
> quite a bit different.
> 
> Certainly calling smb3 or smb2 as cifs should work - but I have the feeling
> that smb3 will in the long run be better known as a name (than cifs) so
> we will have to provide synonyms (mount.smb3 could basically would
> be mount.cifs but pass vers=3)
> 

I see no value in a separate fstype here. It just adds confusion and
extra code that we'll need to maintain and also doesn't add anything
from a user interface perspective -- quite the contrary, really...

/bin/mount already knows to choose a "cifs" fstype whenever it sees a
device string that looks like a UNC. A different fstype breaks that
heuristic. So, the only way /sbin/mount.smb2 or /sbin/mount.smb3 would
ever be called is if you were to mount with a specific command like:

    # mount -t smb2 //server/share...

...and I don't consider that any easier from a user perspective than
using "-o vers="

Also, if you ever want to do autonegotiation, then a separate fstype is
really a non-starter. Once you've called mount() then your fstype is
decided and that's that...

The fact that nfs4 was a separate fstype from nfs is pretty well
considered a mistake now. We're moving as quickly as we can to
deprecate the use of '-t nfs4' in the mainline implementation, fwiw.

In short, just say no to a new fstype unless it's unavoidable... ;)

- -- 
Jeff Layton <jlayton@samba.org>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (GNU/Linux)

iQIcBAEBAgAGBQJPrsREAAoJEAAOaEEZVoIVSG4QAIluoTeSjdgOSfVsXKqjETuF
E/ZtaPLTkRJK3xy1iKRBvF6Mk6I6EjzM2DZ+bt/xpYxczYnnT+UuFGWTL+N/9cjL
CGiZKSDYtDrDxo+JZW7sEXp9Ds19jU1ZSODlXlu2maRoXJ1VbkZoEvExaBHAo+00
GwHz1De9SO7uSSCnLDSvf6g/z6NrfUe5rFCyhNSuOsVlY2tn/h2wiPnY+2j9ZbEM
h9ZgoEtiZrW6JgRliCZ1HiMfrnR+llhsGGbR/hCUx6ZebHBYsS6tMTI1N6ZBs0Pj
5gwc9bo5sdzYHG9FF1xP5MMPWvnx3D+38BP0f4BV4R/0mGkqYbk9z5DP/Looqfeh
rHfjs8GRIukmarduQskBkYz+3AoSf8zdZCpXbIBsFv50SYOLvj1abVGEvWlplmSJ
XJwHjdPaiKj9iKUXQ6w2xSEcTd4pwi52gW9HTbct/xwytNZLkUkY38N4ToLgXzh0
uRXITWR4JGMveWqjWwcEOhNWfiZtNODBZqgRaiAxNraf1MeD8MoaCnvRB2BEm6qt
gu1eNbG1LzXg7AfZxl6Zfz1GMoccHwu2LFCMYUmbCHzzuprSKagrzQAhnfMEWsje
77wTxnlg3DwKwC5cdo/S0sGzU0v8jE5evfswX0Qt40lqOXNKvpcfTLjdDDGQr376
4YJdb37f/YOfoSGWx4Ci
=y4SA
-----END PGP SIGNATURE-----

Re: [PATCH] mount.cifs: remove smb2 multicall binary code

[Jeff Layton]

On Fri, 11 May 2012 21:00:36 -0400
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:

> This was added when it was thought that smb2 would be a different fstype
> altogether. Now that we are not adding a separate fstype, this code is
> no longer needed since nothing will ever call /sbin/mount.smb2.
> 
> Cc: Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> ---
>  mount.cifs.c |   61 +++------------------------------------------------------
>  1 files changed, 4 insertions(+), 57 deletions(-)
> 
> diff --git a/mount.cifs.c b/mount.cifs.c
> index 3041987..2d1d842 100644
> --- a/mount.cifs.c
> +++ b/mount.cifs.c
> @@ -181,7 +181,6 @@ struct parsed_mount_info {
>  
>  const char *thisprogram;
>  const char *cifs_fstype = "cifs";
> -const char *smb2_fstype = "smb2";
>  
>  static int parse_unc(const char *unc_name, struct parsed_mount_info *parsed_info);
>  
> @@ -250,7 +249,7 @@ check_fstab(const char *progname, const char *mountpoint, const char *devname,
>  
>  BB end finish BB */
>  
> -static int mount_cifs_usage(FILE * stream)
> +static int mount_usage(FILE * stream)
>  {
>  	fprintf(stream, "\nUsage:  %s <remotetarget> <dir> -o <options>\n",
>  		thisprogram);
> @@ -290,52 +289,6 @@ static int mount_cifs_usage(FILE * stream)
>  	return 0;
>  }
>  
> -static int mount_smb2_usage(FILE *stream)
> -{
> -	fprintf(stream, "\nUsage:  %s <remotetarget> <dir> -o <options>\n",
> -		thisprogram);
> -	fprintf(stream, "\nMount the remote target, specified as a UNC name,");
> -	fprintf(stream, " to a local directory.\n\nOptions:\n");
> -	fprintf(stream, "\tuser=<arg>\n\tpass=<arg>\n\tdom=<arg>\n");
> -	fprintf(stream, "\nLess commonly used options:");
> -	fprintf(stream,
> -		"\n\tcredentials=<filename>,guest,perm,noperm,rw,ro,");
> -	fprintf(stream,
> -		"\n\tsep=<char>,iocharset=<codepage>,exec,noexec");
> -	fprintf(stream,
> -		"\n\tnolock,directio,sec=<authentication mechanism>,sign");
> -	fprintf(stream,
> -		"\n\tuid=<uid>,gid=<gid>,dir_mode=<mode>,file_mode=<mode>");
> -	fprintf(stream, "\n\nRarely used options:");
> -	fprintf(stream,
> -		"\n\tport=<tcpport>,rsize=<size>,wsize=<size>,unc=<unc_name>,ip=<ip_address>,");
> -	fprintf(stream,
> -		"\n\tdev,nodev,hard,soft,intr,");
> -	fprintf(stream,
> -		"\n\tnointr,ignorecase,noacl,prefixpath=<path>,nobrl");
> -	fprintf(stream,
> -		"\n\nOptions are described in more detail in the manual page");
> -	fprintf(stream, "\n\tman 8 mount.smb2\n");
> -	fprintf(stream, "\nTo display the version number of the mount helper:");
> -	fprintf(stream, "\n\tmount.smb2 -V\n");
> -
> -	if (stream == stderr)
> -		return EX_USAGE;
> -	return 0;
> -}
> -
> -static int mount_usage(FILE *stream)
> -{
> -	int rc;
> -
> -	if (strcmp(thisprogram, "mount.smb2") == 0)
> -		rc = mount_smb2_usage(stream);
> -	else
> -		rc = mount_cifs_usage(stream);
> -
> -	return rc;
> -}
> -
>  /*
>   * CIFS has to "escape" commas in the password field so that they don't
>   * end up getting confused for option delimiters. Copy password into pw
> @@ -1928,7 +1881,6 @@ int main(int argc, char **argv)
>  	size_t options_size = MAX_OPTIONS_LEN;
>  	struct parsed_mount_info *parsed_info = NULL;
>  	pid_t pid;
> -	const char *fstype;
>  
>  	rc = check_setuid();
>  	if (rc)
> @@ -2109,14 +2061,9 @@ mount_retry:
>  	if (rc)
>  		goto mount_exit;
>  
> -	if (strcmp(thisprogram, "mount.smb2") == 0)
> -		fstype = smb2_fstype;
> -	else
> -		fstype = cifs_fstype;
> -
>  	if (!parsed_info->fakemnt) {
>  		toggle_dac_capability(0, 1);
> -		rc = mount(orig_dev, ".", fstype, parsed_info->flags, options);
> +		rc = mount(orig_dev, ".", cifs_fstype, parsed_info->flags, options);
>  		toggle_dac_capability(0, 0);
>  		if (rc == 0)
>  			goto do_mtab;
> @@ -2133,7 +2080,7 @@ mount_retry:
>  			goto mount_retry;
>  		case ENODEV:
>  			fprintf(stderr,
> -				"mount error: %s filesystem not supported by the system\n", fstype);
> +				"mount error: %s filesystem not supported by the system\n", cifs_fstype);
>  			break;
>  		case ENXIO:
>  			if (!already_uppercased &&
> @@ -2163,7 +2110,7 @@ do_mtab:
>  				goto mount_exit;
>  		}
>  
> -		rc = add_mtab(orig_dev, mountpoint, parsed_info->flags, fstype);
> +		rc = add_mtab(orig_dev, mountpoint, parsed_info->flags, cifs_fstype);
>  	}
>  
>  mount_exit:

Committed...
-- 
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>