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

From: "Noveck, Dave" <[email-masked]>
Date: Mon, 22 Oct 2007 12:45:46 -0400

Can anybody shed light on this comment from the locking review:

	Kustarz - General comment - It should be mentioned somewhere
that when doing IO the order of checking stateids should be
delegation/lock/open/special.

I'm unclear about what situation is being referred to "when doing IO".
Is the checking of stateids alluded to being done by the client or the
server.  Not clear where this proposed order is derived from from, or
how a different order would affect the result.  For example a stateid
will be one of special, lock, delegation, or open, and if that is is so,
the order that you check for thos is irrelevant.  Not matter what order
you check, you will at some point deal with the class of object the
stateid designates.  What am I missing here?

	-----Original Message-----
	From: Khan, Saadia 
	Sent: Wednesday, August 29, 2007 8:00 PM
	To: [email-masked]
	Subject: [nfsv4] Review comments for Locking and Directory
DelegationsReview# 2

	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

---

From: Trond Myklebust <[email-masked]>
Date: Mon, 22 Oct 2007 13:33:30 -0400

On Mon, 2007-10-22 at 12:45 -0400, Noveck, Dave wrote:
> will be one of special, lock, delegation, or open, and if that is is
> so, the order that you check for thos is irrelevant.  Not matter what
> order you check, you will at some point deal with the class of object
> the stateid designates.  What am I missing here?

Wasn't that meant as advice for clients when selecting which stateid
they should present to the server? In other words, if a client holds a
delegation, then it should choose to present that to the server. Next it
should check whether or not it holds a lock stateid, followed by an open
stateid, etc...

I seem to remember, though, we chose to change that ordering in the pNFS
review. I believe that Mike Eisler asked that we deprecate use of the
lock stateid in all cases except the one where the server is enforcing
mandatory locks. Mike?

Cheers,
  Trond

>         The second review meeting for locking and directory
>         delegations was held on August 27, 2007.
>          
>         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.
>          
>         persistent session where the state is lost and the client is
>         asked to reclaim its state.
>          
>         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.
>          
>         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.
>          
>         referred to as byte range locks or record locks in all places,
>         currently this is not consistent. Same for octet locks.
>          
>         Kustarz - General Comment - Clarify all references to owners
>         to be specifically lock owner/open owner or state owner.
>          
>         Gordon - Comment for this section - replace setattr for set
>         size with write.
>          
>         Eisler - L 9254 - This is not true, it should be clarified
>         what the new behavior is.
>          
>         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
>          
>         Khan - L 9581 - Clarify that nfs4err_delay could be returned
>         in this case as well.
>          
>         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.
>

---

From: eric kustarz <[email-masked]>
Date: Mon, 22 Oct 2007 10:35:20 -0700

On Oct 22, 2007, at 9:45 AM, Noveck, Dave wrote:

> Kustarz - General comment - It should be mentioned somewhere that  
> when doing IO the order of checking stateids should be delegation/ 
> lock/open/special.
> I'm unclear about what situation is being referred to "when doing  
> IO".  Is the checking of stateids alluded to being done by the  
> client or the server.  Not clear where this proposed order is  
> derived from from, or how a different order would affect the  
> result.  For example a stateid will be one of special, lock,  
> delegation, or open, and if that is is so, the order that you check  
> for thos is irrelevant.  Not matter what order you check, you will  
> at some point deal with the class of object the stateid  
> designates.  What am I missing here?

This is referring to the client side.  If the client is doing a read/ 
write, we should be clear about which stateid it should send OTW.

In Solaris (and i believe everyone else is doing the same), the  
client will send over the first stateid that it has in the order:  
delegation, then lock, then open, then special.

Check out at nfs4_get_stateid():
http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/ 
common/fs/nfs/nfs4_client_state.c

eric

> The second review meeting for locking and directory delegations was  
> held on August 27, 2007.
>
> In this meeting, we covered lines 8751 - 9446, 9530 - 9728, 10476 -  
> 10519 from draft 13.
>
> allow reclaims to happen if it is not storing the lock information  
> on stable storage in order to be spec compliant.
>
> persistent session where the state is lost and the client is asked  
> to reclaim its state.
>
> 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.
>
> 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.
>
> 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 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.
>
> Khan/Kustarz - L 9004 - byte range locks should either be referred  
> to as byte range locks or record locks in all places, currently  
> 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.
>
> Eisler - L 9254 - This is not true, it should be clarified what the  
> new behavior is.
>
> Eisler - L 9381 - Section on stateid creation should be referenced  
> and explained what happens to the old stateid in this case.
>
> 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
>
> Gordon - L 9648 - CLAIM_DELEGATE_PREV_FH should be mentioned here  
> as well.
>
> 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
>
> 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
>
>

---

From: Mike Eisler <[email-masked]>
Date: Mon, 22 Oct 2007 10:57:03 -0700 (PDT)

> lock stateid in all cases except the one where the server is

On the theory that the data server to metadata server handshake will be
simpler (and less chatty)
if the data server is not cracking octet range record locks,
yes, the spec should advise clients to not use record lock stateids
when accessing data servers, unless mandatory locking is in force.

---

From: "Noveck, Dave" <[email-masked]>
Date: Mon, 22 Oct 2007 19:03:19 -0400

How does the client know whether mandatory locking is in force.  The
spec says you can find out when you get NFS4ERR_LOCKED that mandatory
locking is in effect, but that doesn't give you any way to find out that
mandatory locking is not in effect.  So how is a client going to find
out that he should not put record lock stateids into reads and writes?

-----Original Message-----
From: Mike Eisler [mailto:[email-masked]] 
Sent: Monday, October 22, 2007 1:57 PM
To: Trond Myklebust; Noveck, Dave
Cc: Khan, Saadia; [email-masked]
Subject: RE: [nfsv4] Review comments for Locking and Directory
DelegationsReview# 2

--- Trond Myklebust <[email-masked]> wrote:

> I seem to remember, though, we chose to change that ordering in the 
> pNFS review. I believe that Mike Eisler asked that we deprecate use of

> the lock stateid in all cases except the one where the server is 
> enforcing mandatory locks. Mike?

On the theory that the data server to metadata server handshake will be
simpler (and less chatty) if the data server is not cracking octet range
record locks, yes, the spec should advise clients to not use record lock
stateids when accessing data servers, unless mandatory locking is in
force.

---

From: Mike Eisler <[email-masked]>
Date: Mon, 22 Oct 2007 17:37:40 -0700 (PDT)

> out that he should not put record lock stateids into reads and

The data servers should know whether a file requires mandatory locking
or not. Whether a file requires mandatory locking or not is
a property of the operating environment of the NFS server (which
might be dependent on the presence of a file attribute).

If a file requires mandatory locking, and the client uses an
OPEN stateid, then the data server -- whether the file has a 
conflicting lock or not -- can return NFS4ERR_LOCKED to READ or WRITE.
The client then issues LOCK to the MDS, gets a record lock stateid,
and uses that stateid for the READ or WRITE.

---

From: "Noveck, Dave" <[email-masked]>
Date: Tue, 23 Oct 2007 14:12:37 -0400

So what we want here is:

     If you are doing IO to the metadata server, then you use a lock 
     stateid if the current lockowner has one, and only use an open
     stateid if the there is no current lock stateid for the current 
     lockowner.

     If you are doing IO to the data server (pnfs files), you don't 
     use a lock stateid unless you get back NFS4ERR_LOCKED back from
     the data server.

I this correct?

-----Original Message-----
From: Mike Eisler [mailto:[email-masked]] 
Sent: Monday, October 22, 2007 1:57 PM
To: Trond Myklebust; Noveck, Dave
Cc: Khan, Saadia; [email-masked]
Subject: RE: [nfsv4] Review comments for Locking and Directory
DelegationsReview# 2

--- Trond Myklebust <[email-masked]> wrote:

> pNFS

---

From: Mike Eisler <[email-masked]>
Date: Tue, 23 Oct 2007 11:27:06 -0700 (PDT)

Looks good.

---

From: Trond Myklebust <[email-masked]>
Date: Tue, 23 Oct 2007 14:30:37 -0400

>      stateid if the there is no current lock stateid for the current 

Yes, but you should prefer the delegation over the lock stateid if you
have one.

>      use a lock stateid unless you get back NFS4ERR_LOCKED back from

Yes, although again I assume that the delegation is to be preferred over
the open stateid.

Cheers
  Trond