Subject: [nfsv4] Review comments for Locking and Directory DelegationsReview # 2

From: "Khan, Saadia" <[email-masked]>
Date: Wed, 29 Aug 2007 16:59:41 -0700

The second review meeting for locking and directory delegations was held
on August 27, 2007.
 
The roles in this meeting were:

- moderator - Shepler

- reader - Eisler

- scribe - Khan

- author - Noveck

- inspectors: Erasani, Gordon, Kustarz

In this meeting, we covered lines 8751 - 9446, 9530 - 9728, 10476 -
10519 from draft 13.

I am listing the review comments received during the review by name,
line number and comment.
 
lines 8751 - 9446
 
Khan - L 8775 - It should be emphasized that the server must not allow
reclaims to happen if it is not storing the lock information on stable
storage in order to be spec compliant.
 
Kustarz - L 8781 - Clarify what 'false positives' means here.
 
Noveck - L 8826 - Another case missing here is the one for a persistent
session where the state is lost and the client is asked to reclaim its
state.
 
Kustarz - General Comment - All state related errors should be
consolidated and explained together, maybe in a separate section.
 
Eisler - L 8834 - s/as discussed above/as discussed in section blah.
 
Eisler - L 8838 - These lines are not correct and need to clarify what
some of the SEQUENCE flags do.
 
Noveck - General comment - The two edge conditions mentioned in 8.4.3
are only in terms of lease expiration and not in the context of server
revocation of locks. That needs to be addressed either in this section
or this section needs to be somehow part of 8.4.3.
 
Ersani/Eisler - General comment - In the desription of test_stateid, it
should be mentioned that the client may do this op whenever it wants,
but it should definitely do this when any locks have been revoked and
the server should allow the client to issue this op multiple times.
 
Eisler - L 8899, 8917 - s/lock/lease
 
Eisler  - L 8978 - server should ignore these fields and not return an
error.
 
Noveck - L 8971 - Mention here that the cleintid in CREATE_SESSION and
DESTROY_SESSION is not ignored.
 
Eisler - L 8984 - OPEN and CREATE should be in lower case.
 
Eisler - L 8994 - s/lock/byte range lock
 
Khan/Kustarz - L 9004 - byte range locks should either be referred to as
byte range locks or record locks in all places, currently this is not
consistent. Same for octet locks.
 
Noveck - L 9029 - This should be mentioned in the previous chapter.
 
Kustarz - General Comment - Clarify all references to owners to be
specifically lock owner/open owner or state owner.
 
Eisler - L 9051 - special stateid section should be referenced here.
 
Noveck - L 9132 - s/OPEN/stateid
 
Gordon - Comment for this section - replace setattr for set size with
write.
 
Eisler - L 9176 - remove the line.
 
Eisler - L 9157 - There is no difference between using special stateids
or regular stateids for this scenario.
 
Eisler - L 9254 - This is not true, it should be clarified what the new
behavior is.
 
Eisler  - L 9318 - Needs clarification.
 
Eisler - L 9381 - Section on stateid creation should be referenced and
explained what happens to the old stateid in this case.
 
Eisler  - L 9403 - s/open/OPENs
 
Kustarz - L 9403 - reference to the section that discusses how parallel
opens should be handled and implemented.
 
Eisler - L 9413 - s/ay/may
 
Eisler - L 9417 - inadvertently (typo)
 
Noveck - L 9435 - This needs to be discussed on the mailing list. There
could be issues here with part of the request being processed by one
server instance and part of it by another one.
 
lines 9530 - 9728
 
Eisler - L 9554 - s/callback/back channel
 
Khan - L 9581 - Clarify that nfs4err_delay could be returned in this
case as well.
 
Eisler - L 9620 - s/leases/lease
 
Khan - L 9651 - s/but/and
 
Eisler - L 9693 - s/reclaim a/reclaim via a
 
Eisler - L 9701 - remove quotes.
 
Gordon - L 9648 - CLAIM_DELEGATE_PREV_FH should be mentioned here as
well.
 
Eisler - L 9705 - s/freeing/revocation
 
Eisler  - L 9723 - Needs to be clarified if the state needs to be in
stable storage, the proper section should be referenced here.
 
lines 10476 - 10519 
 
Noveck -  L 10510 - Needs to be clarified.
 
Khan - L 10516 - s/leases/lease
 
Eisler - General comment - It should be mentioned somewhere that the
client needs to look at the status flags returned from SEQUENCE because
it might need to do some actions based on what it got.
 
Gordon - L 10504 - s/a reasonable time/atleast one lease period
 
Khan - L 10534 - It should be added here that any open/lock state
subsumed by the delegation does not get revoked when the delegation gets
revoked.
 
Kustarz - General comment - It should be mentioned somewhere that when
doing IO the order of checking stateids should be
delegation/lock/open/special.

thanks.
Saadia

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