On Sat, 27 Aug 2011 14:41:36 +0400 Pavel Shilovsky <[email protected]> wrote:
> 2011/8/27 Jeff Layton <[email protected]>: > > On Fri, 26 Aug 2011 22:03:03 +0400 > > Pavel Shilovsky <[email protected]> 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 <[email protected]> > > > > > > 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 <[email protected]> -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
