On Oct 13, 2012, at 5:46 PM, Wolfram Gloger <wmglo@xxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> Hi,
>
>>>>> result = nfs_sys_mount(mi, options);
>>>
>>> No, nfs_sys_mount() does not use mi->extra_opts at all, only the
>>> binary options.
>>
>> This is the text-based code, which I wrote. nfs_sys_mount() passes an options string (NUL-terminated C string) to the kernel, not a binary object. That string contains all the FS-specific mount options specified by the user.
>>
>> But your patch makes that string empty, by my reading. I think this is incorrect.
>
> Ok, I'm happy to go through this line-by-line.
>
> static int nfs_sys_mount(struct nfsmount_info *mi, struct mount_options *opts)
> {
> char *options = NULL;
> int result;
>
> if (mi->fake)
> return 1;
>
> if (po_join(opts, &options) == PO_FAILED) { // HERE
> errno = EIO;
> return 0;
> }
>
> result = mount(mi->spec, mi->node, mi->type,
> mi->flags & ~(MS_USER|MS_USERS), options);
> ...
> }
>
> nfs_sys_mount() constructs the text options _itself_ purely from the
> opts (2nd) argument HERE -- po_join has opts as input and options as
> output.
>
> My patch only changes the first argument (mi). So, no functional change
> within nfs_sys_mount() at all.
Agreed.
> The functional change is that with my patch, mi->extra_opts is kept
> unchanged unless the system call is successful. mi->extra_opts is
> actually reused later throughout the mount program, because
> nfsmount_string() has extra_opts as an input _and_ output argument,
> and propagates mi->extra_opts into the extra_opts variable in main.c.
>
> I have actually tested this patch extensively and it fixes the
> problem.
Unfortunately there are a rather unimaginable number of use cases for mount.nfs, which is why it is so complicated. "Extensively" could mean you've tested it for a long time but with just a few use cases.
In any event:
Reviewed-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
