Re: [nfsv4] Review of 3530bis "Multi-Server Namespace" Chapter

[Thomas Haynes <[e-mail]>]

Rob and Dave,

There is a question for each of you below.


On Sep 22, 2010, at 6:31 PM, James Lentini wrote:

> 
> At the last 3530bis meeting, I volunteered to review the "Muli-Server 
> Namespace" chapter. Below are my comments on the version in 
> draft-ietf-nfsv4-rfc3530bis-04.
> 
> RFC 5661 added a significant amount of text in this area, and I think 
> the strategy of pulling this text back into 3530bis is the right one. 
> Overall, it looks pretty good. Here are my detailed comments on the 
> text:
> 
> One reference to v4.1's change_policy attribute slipped into Chapter 
> 7. Since change_policy isn't supported in v4, the reference to it 
> should be removed from Section 7.6 "Additional Client-side 
> Considerations" on page 83.
> 
> I have a few comments related to the file system equivalence classes 
> in 7.7.
> 
> Section 7.7.5 introduces the concept a change equivalence class. 
> Later, in Section 7.9.1, the text states that file system instances 
> should always be considered to be of different change classes for v4. 
> I'd consider removing section 7.7.5 since it isn't possible to 
> communicate that file system instances are in an equivalent change 
> class.


Agreed


> 
> For the write-verifier class in Section 7.7.7, Section 7.9.1 
> recommends assuming file system instances are of a different class in 
> the case of a failover. Is there a recommendation for the 
> NFS4ERR_MOVED case? I realize that the recommendations in 7.9.1 are 
> taken almost verbatim from RFC 5661, but it seems that there is a case 
> missing here.
> 


Rob, any comment here?



> No mention is made of the readdir class after Section 7.7.8. I 
> recommend that either (A) Section 7.9.1 should suggest a way to infer 
> the readdir class (it would be safest just to assume file system 
> instances were not in the same readdir class) or (B) remove section 
> 7.7.8 and the readdir equivalence class concept (just as RFC 5661's 
> "Simultaneous Use and Transparent Transitions" section describing the 
> simultaneous-use class was removed in the 3530bis version).


I'd go with A.

Would you have the same comment for 5661?


> 
> One minor note on the typographical representation of the equivalence 
> class identifiers. Throughout Chapter 7 equivalence class foo is 
> represented as "_foo_" where the '_' characters are meant to denote an 
> emphasis. Early drafts of v4.1 used this notation, but it was dropped 
> in the published version of RFC 5661. I'd suggest removing the XML 
> emphasis tags ("<spanx style='emph'>foo</spanx>") to match RFC 5661.


Yes, done.



> 
> In Section 7.9's definition of a struct fs_location4, the server<> 
> field's type was changed from utf8str_cis to utf8val_must. It seems 
> that the original versions case-insensitive behavior is no longer 
> being recorded. I suggest noting the case-insensitivity in the text 
> description of an fs_location4 or creating a new type 
> (utf8cis_val_must?).


I think this is because of Dave's i18n changes, i.e., it crept in between
bis-03 and bis-04.

Dave, can you comment here?


> 
> Throughout Chapter 7, I noticed a few minor typos that I remembered 
> sending corrections for in the run up to RFC 5661. Comparing a diff of 
> Chapter 11 in RFC 5661 and Chapter 7 in the 3530bis draft leads me to 
> conclude that the text in 3530bis was not taken from the final version 
> of RFC 5661. As a results, it looks like some fixes from the WG and 
> RFC Editor are missing in the 3530bis version. In the diff below, I've 
> pulled the missing corrections into the 3530bis git repo's XML source 
> file for Chapter 7. These changes are mostly just punctuation, missing 
> words, etc.
> 


James,

I can't stress how awed I feel at the amount of work you had to have 
done here. <bow />




> 
> @@ -1176,19 +1187,19 @@
>           since we have the fsid of the latter and it is that of the
>           pseudo-fs, which presumably cannot move.  However, in other
>           examples, we might not have this kind of information to rely
> -          on (e.g. /this/is/the might be a non-pseudo file system
> +          on (e.g., /this/is/the might be a non-pseudo file system
>           separate from /this/is/the/path), so we need to have
> -          another reliable source information on the boundary of the file system
> -          which is moved.  If, for example, the file system "/this/is"
> -          had moved we would have a case of migration rather than
> -          referral and once the boundaries of the migrated file system
> +          other reliable source information on the boundary of the file system
> +          that is moved.  If, for example, the file system /this/is
> +          had moved, we would have a case of migration rather than
> +          referral, and once the boundaries of the migrated file system
>           was clear we could fetch fs_locations.
>         </t>
>         <t hangText='- '>
>           We are fetching fs_locations because the fact that we got an
> -          NFS4ERR_MOVED at this point means that it most likely that
> +          NFS4ERR_MOVED at this point means that it is most likely that
>           this is a referral and we need the destination.  Even if it is
> -          the case that "/this/is/the" is a file system which has
> +          the case that /this/is/the is a file system thathatt has


that instead of thathatt ?


>           migrated, we will still need the location information for that
>           file system.
>         </t>
> 





> @@ -1242,14 +1253,14 @@
>         <t>
>           LOOKUP "the"
>         </t>
> -        <t>
> +        <t
>           READDIR (fsid, size, time_modify, mounted_on_fileid)


I did not understand the above.


> 
>      Note that: there is no requirement that the number
>      of components in each rootpath be the same; there
>      is no relation between the number of components in
> -     rootpath or fs_root; and the none of the components
> +     rootpath or fs_root, and none of the components
>      in each rootpath and fs_root have to be the same. In
>      the above example, we could have had a third element
> -     in the locations array, with server equal to "servC",
> +     in the locations array, with server equal to "servC"
>      and rootpath equal to "/I/II", and a fourth element in
> -     locations with server equal to "servD", and rootpath
> +     locations with server equal to "servD" and rootpath
>      equal to "/aleph/beth/gimel/daleth/he".
>    </t>


I found it odd that there were still some double-quoted
servers and paths. I understand they were variables,
but still.


> 
>     <section anchor="fs_locations_trans" title="Inferring Transition Modes">
> @@ -1537,7 +1548,7 @@
>        </t> 
>        <t>
>          All listed file system instances should be considered as of the 
> -         same <spanx style='emph'>fileid</spanx> class, if and only if, the 
> +         same <spanx style='emph'>fileid</spanx> class if and only if the 
>          fh_expire_type attribute indicates persistent filehandles and 
>          does not include the FH4_VOL_MIGRATION
>          bit.  Note that in the case of referral, fileid issues do


In 5661, this is page 259 and we have the fix in one paragraph and not
the other:

o  All listed file system instances should be considered as of the
same handle class, if and only if, the current fh_expire_type
attribute does not include the FH4_VOL_MIGRATION bit.  Note that
in the case of referral, filehandle issues do not apply since there
can be no filehandles known within the current file system, nor is
there any access to the fh_expire_type attribute on the referring
(absent) file system.

o  All listed file system instances should be considered as of the
same fileid class if and only if the fh_expire_type attribute
indicates persistent filehandles and does not include the
FH4_VOL_MIGRATION bit.  Note that in the case of referral, fileid
issues do not apply since there can be no fileids known within the
referring (absent) file system, nor is there any access to the
fh_expire_type attribute.

I've made it consistent.