Subject: [nfsv4] Review comments for Locking and Directory Delegations Review # 1

From: "Khan, Saadia" <[email-masked]>
Date: Fri, 24 Aug 2007 10:44:50 -0700

The first review meeting for locking and directory delegations sections
was held on August 20, 2007. 

The roles in this meeting were:

- moderator - Eisler

- reader - Eisler

- scribe - Khan

- author - Noveck

- inspectors: Erasani, Gordon, Kustarz

In this meeting, we covered lines 7907 - 8749 from draft 13.

I am listing the review comments received during the review by name,
line number and comment.
 
Eisler - L 7936 - clarify what 'largest guage information'  means, might
be better to replace it with something that talks about hierarchy
 
Gordon - General comment - There should be a distinction between locks
and state and it should be clearly defined what state means.
 
Noveck - General comment - How is a lease different in v4.1 vs. v4.0?
and what exactly is tied to the lease. This should be clarified.
 
Ersani - General comment - Layout vs. device maps need to be
differentiated.
 
Eisler - L 7982 - L 7984 - It should also be mentioned here that this
can apply to locks held by the same client on different files as well.
 
Eisler - L 7991 - Section 8.2.1 should mention delegations can be handed
out on non-regular files as well.
 
Khan - L 8045 - s/the/them
 
Eisler - L 8048 - It should be noted that the next valid value for seqid
should be 1 and not 0 after a wraparound.
 
Khan - L 8141 - It should be clarified that if a special stateid
designating the current stateid is used when being in the context of a
different fh from the one the current stateid is associated with, an
error should be returned, probably BAD_STATEID.
 
Eisler - L 8150 - A differentiation needs to be made between the stateid
becoming invalid vs. the lock.
 
Eisler - L 8185 - Generation no. being mentioned twice here is
confusing.
 
Kustarz - L 8210 - Special stateid section no. should be mentioned here.
 
Noveck - L 8185 - The table entry should also have information regarding
whether the state was revoked and if so, why.
 
Noveck - L 8259 - 8265 - These lines should be moved after L 8249.
 
Eisler - L 8267 - Clarify that each clientid is associated with only one
lease, there cant be multiple leases for a clientid.
 
Eisler - L 8269 - Reinforce the fact that the purpose of a lease is to
remove stale lock state, not stale stateids.
 
Eisler - L 8299, 8322 - Multiple leases again.
 
Eisler - L 8322 - Reference the Lease Expiration section here.
 
Eisler - L 8334 - Reference the CREATE_SESSION section regarding
confirmation of clientid.
 
Khan - L 8355 - It should also be mentioned that all stateids should
also be freed at this time.
 
Khan/Noveck - L 8382 - Sections 1 and 2 should be combined together
since section 2 is a subsection of 1.
 
Khan - L 8409 - The two scenarios here need to be clarified. ie when is
lock recovery needed and when not.
 
Ersani - General comment - Maybe the relationship between the lease
period and reclaim period should be defined and explained in the Crash
Recovery section.
 
Khan - L 8440 -Clarify that ops required to create clientids/sessions
are allowed in reclaim period, its only lock state operations that are
not allowed.
 
Khan - L 8461 - It should state here that the client MUST do a
RECLAIM_COMPLETE after its done with reclaim or if it doesn;t have
anything to reclaim.
 
Eisler - L 8470 - change paran to comma. 
 
Eisler - L 8537 - 8543 - combine these lines with the paragraph above. 
 
Saadia/Pranoop - General comment - Should the server somehow indicate to
the client that the grace period is over, because it it now possible for
the client to have done a RECLAIM_COMPLETE but still get a GRACE error
back if the server is not out of the grace period. That means the client
has to be prepared to receive this error even after its done reclaiming
all state and has no way of knowing when the server will get out of
grace.
 
Eisler - L 8573 - It should also mention that the lease time might even
change if the clientid persists across reboots and so clients should
always refetch the lease time attribute upon server reboot. The impact
of changing lease time attribute should be mentioned in the attributes
section and this section should be referenced there.
 
Eisler - L 8589 - 'leases' again. 
 
Eisler - L 8583 - 8590 - It should say server MUST remove lock state for
conflicting requests.
 
Khan - L 8598 - should be replaced by 'even if the conflict is with only
a subset of locks'.
 
Eisler - L 8602 - It should be clarified that if the clientid does not
have any valid locks, the server may remove the clientid and all the
stateid or it may choose not to. The client will need to do different
actions based on what the server chooses to do.
 
Khan - L 8619 - The flag that will be returned in this case should be
mentioned here.
 
Kustarz - L 8632 - s/them/then
 
Eisler - L 8727 - client id string should be replaced by client_owner.id
field.
 
thanks.
Saadia

_______________________________________________
nfsv4 mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/nfsv4