Re: [PATCH] cifs: use standard token parser for mount options

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Sachin,
I am a little worried about merge conflicts - am about 1/3 of the way
through merging (and reviewing) Pavel's and similarly have started to
merge Jeff's patches, so make sure you rebase them on what is in
cifs-2.6.git tree on samba.org

On Wed, Mar 21, 2012 at 3:30 PM, Jeff Layton <jlayton@xxxxxxxxxx> wrote:
> On Wed, 21 Mar 2012 18:33:07 +0000
> Sachin Prabhu <sprabhu@xxxxxxxxxx> wrote:
>
>> On Wed, 2012-03-21 at 13:38 -0400, Jeff Layton wrote:
>> > On Fri, 09 Mar 2012 15:33:17 +0000
>> > Sachin Prabhu <sprabhu@xxxxxxxxxx> wrote:
>> >
>> > > Use the standard token parser instead of the long if confidtion to parse
>> > > cifs mount options.
>> > >
>> > > This was first proposed by Scott Lovenberg
>> > > http://lists.samba.org/archive/linux-cifs-client/2010-May/006079.html
>> > >
>> > > Mount options have been grouped together in terms of their input types.
>> > > Aliases for username, password, domain and credentials have been added.
>> > > The password parser has been modified to make it easier to read.
>> > >
>> > > Signed-off-by: Sachin Prabhu <sprabhu@xxxxxxxxxx>
>> > >
>> >
>> >
>> > In testing this today, I found a problem. I tried to mount up a share
>> > with sec=none as the options. With that, I got this error:
>> >
>> >     CIFS: Unknown mount option pass=
>> >
>> > The problem is that the mount.cifs helper will pass a blank "pass="
>> > option to the kernel. It seems like the standard option parser doesn't
>> > have a way to allow you to specify an optional argument. That may need
>> > to be added in order for this to work.
>>
>>
>> We could use it in this manner.
>>
>> ----
>> Allow mount option pass to be a blank value.
>>
>> When using sec=none, the mount.cifs helper will pass a blank 'pass='
>> mount option. This should be handled by the parser.
>>
>> Signed-off-by: Sachin Prabhu <sprabhu@xxxxxxxxxx>
>> ---
>>  fs/cifs/connect.c |    7 +++++++
>>  1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index 8f83ea0..831f242 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -107,6 +107,9 @@ enum {
>>       /* Mount options to be ignored */
>>       Opt_ignore,
>>
>> +     /* Options which could be blank */
>> +     Opt_blank_pass,
>> +
>>       Opt_err
>>  };
>>
>> @@ -180,6 +183,7 @@ static const match_table_t cifs_mount_option_tokens = {
>>
>>       { Opt_user, "user=%s" },
>>       { Opt_user, "username=%s" },
>> +     { Opt_blank_pass, "pass=" },
>>       { Opt_pass, "pass=%s" },
>>       { Opt_pass, "password=%s" },
>>       { Opt_ip, "ip=%s" },
>> @@ -1546,6 +1550,9 @@ cifs_parse_mount_options(const char *mountdata, const char *devname,
>>                       }
>>                       vol->nullauth = 0;
>>                       break;
>> +             case Opt_blank_pass:
>> +                     vol->password = NULL;
>> +                     break;
>>               case Opt_pass:
>>                       /* passwords have to be handled differently
>>                        * to allow the character used for deliminator
>
> ACK.
>
> Since Steve hasn't committed this yet, care to send a respin of the
> original? I don't think there are any others that require the ability
> to handle a blank option, but we can always add that later if needed.
>
> --
> Jeff Layton <jlayton@xxxxxxxxxx>



-- 
Thanks,

Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux