On Sat, 27 Aug 2011 14:41:36 +0400
Pavel Shilovsky <piastry@xxxxxxxxxxx> wrote:
> 2011/8/27 Jeff Layton <jlayton@xxxxxxxxxxxxxxx>:
> > On Fri, 26 Aug 2011 22:03:03 +0400
> > Pavel Shilovsky <piastry@xxxxxxxxxxx> wrote:
> >
> >> Both these options are started with "rw" - that's why the first one
> >> isn't switched on even if it is specified. Rename it to piforwardio
> >> to avoid the wrong matching.
> >>
> >> Signed-off-by: Pavel Shilovsky <piastry@xxxxxxxxxxx>
> >
> >
> > This seems like the wrong way to fix this. Isn't this a problem in
> > userspace code? It seems like fixing mount.cifs to handle this properly
> > would be a better fix, and we wouldn't need to change a mount option
> > (without warning) that's already in kernels that are in the field.
> >
> > NAK from my point of view...
> >
>
> In kernel code we have the section :
>
> cifs_parse_mount_options:
> ...
> } else if (strnicmp(data, "rw", 2) == 0) {
> /* ignore */
> } else if (strnicmp(data, "ro", 2) == 0) {
> ...
> } else if (strnicmp(data, "rwpidforward", 4) == 0) {
> vol->rwpidforward = 1;
> } else
> ...
>
> So, it is not only the userspace problem. Even if we fix it kernel
> code doesn't handle it right too. So, the easier way to fix this is to
> rename the option, I think.
>
Except that this option is already in existing kernels that are in the
field, as well as the documentation (the mount.cifs manpage). Changing
user-visible interfaces like this without any warning is not good
practice.
As a side note...how was rwpidforward originally tested if the option
was broken and could never be turned on?
--
Jeff Layton <jlayton@xxxxxxxxx>
--
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
