On Feb 2, 2012, at 2:00 PM, Bryan Schumaker wrote: > On 02/02/12 12:03, Adamson, Dros wrote: > >> >> On Feb 2, 2012, at 8:51 AM, Bryan Schumaker wrote: >> >>> On 02/01/12 18:07, Adamson, Dros wrote: >>> >>>> On Feb 1, 2012, at 5:44 PM, Boaz Harrosh wrote: >>>> >>>>> On 02/01/2012 09:06 PM, Weston Andros Adamson wrote: >>>>>> Don't allow invalid 'vers' and 'minorversion' combinations in mount options, >>>>>> such as "vers=3,minorversion=1". >>>>>> >>>>> >>>>> Just my $0.017 I don't see the point in this. >>>>> >>>>> If vers==3 then minorversion is ignored, just like today. >>>>> What kind of extra protection does it buy us? >>>> >>>> No, minorversion is not ignored when vers=3. >>> >>> >>> But after mounting, does setting vers=3, minorversion=1 cause any change in NFS v3 behavior? >>> >> >> No it doesn't. Past the parsing of options, minorversion is ignored for versions other than 4. >> >> I just don't understand how anyone can have problem with this patch. Why would we want to validate minorversion in some cases, but not all cases? How would this patch be a bad thing? >> > > I don't have a problem with the patch, it makes sense that we shouldn't confuse developers or users. I was just curious if there was a spot where we had "if minor_version == 1: do_something()" without checking for major_version == 4. > Ah, I misunderstood… Versions != 4 pass the nfs_parsed_mount_data struct to nfs_create_server(), which completely ignores the minorversion member. Version == 4 passes the nfs_parsed_mount_data struct to nfs4_create_server(), which (through nfs4_init_server()) uses the minorversion member. So, having a set minorversion when mounting vers != 4 has no effect on how the NFS module operates. This is Boaz's argument for why the patch isn't needed. I understand that reasoning, but this is a user experience enhancement and I think they are important too. This patch only addresses an inconsistency in mount option validation. This doesn't change anything at the protocol level. I should have done a better job explaining this in the original post! -dros > - Bryan > >> It's about usability -- if this can confuse NFS developers, how are end users going to handle it? >> >> -dros > >
Attachment:
smime.p7s
Description: S/MIME cryptographic signature
