Subject: [nfsv4] RFC 3530 question: LOCKT and expired/stale clientid

From: Frank S Filz <[email-masked]>
Date: Tue, 6 Nov 2012 09:20:23 -0800

Resending after joining the mailing list...

I'm working on the Ganesha user space NFS server and we are trying to be
more correct with expired/stale errors.

In looking at the RFC (3530 bis-17), I see LOCKT does not define
NFS4ERR_EXPIRED as a possible return code, leaving only
NFS4ERR_STALE_CLIENTID, yet, it does include a clientid with the lock/open
owner.

I also see that the RFC doesn't indicate LOCKT as a lease renewing
operation. Is that actually expected? Since it does reference a clientid,
easy enough to renew lease.

Frank

---

From: "J. Bruce Fields" <[email-masked]>
Date: Tue, 6 Nov 2012 13:24:13 -0500

> NFS4ERR_STALE_CLIENTID, yet, it does include a clientid with the lock/open

That sounds like a clear spec bug to me, unless there's some subtlety
I'm missing.  (In which case it would be worth documenting.)

> easy enough to renew lease.

And that must be a bug too.

--b.

---

From: Frank S Filz <[email-masked]>
Date: Tue, 6 Nov 2012 10:32:00 -0800

"J. Bruce Fields" <[email-masked]> wrote on 11/06/2012 10:24:13 AM:
> Re: [nfsv4] RFC 3530 question: LOCKT and expired/stale clientid
>

RELEASE_LOCKOWNER also seems to be missing from the list, missing from both
these places.

Frank

---

From: "J. Bruce Fields" <[email-masked]>
Date: Tue, 6 Nov 2012 13:37:56 -0500

> RELEASE_LOCKOWNER also seems to be missing from the list, missing from both

Might also be worth checking the tables in rfc 5661 to make sure it
doesn't have the same mistakes.

--b.

---

From: Frank S Filz <[email-masked]>
Date: Tue, 6 Nov 2012 10:58:48 -0800

"J. Bruce Fields" <[email-masked]> wrote on 11/06/2012 10:37:56 AM:
> > > > I'm working on the Ganesha user space NFS server and we are trying
to
> > be

NFS v4.1 doesn't have RELEASE_LOCKOWNER, so that's clear.

It does exclude NFS4ERR_EXPIRED from LOCKT, but that may be less of an
issue since the SEQUENCE op should handle lease renewal and discovery of
expired leases.

Frank

---

From: "Noveck, David" <[email-masked]>
Date: Tue, 6 Nov 2012 15:26:26 -0500

> RELEASE_LOCKOWNER also seems to be missing from the list, missing from both these places.

In -20 at least, RELEASE_LOCKOWNER is listed as returning NFS4ERR_EXPIRED, although it
isn't on the automatic renewal list.

I agree that both LOCKT and RELEASE_LOCKOWNER:

  - should be able to return NFS4ERR_EXPIRED.

  - should cause lease renewal.

One other likely problem I just noted is that RENEW and RELEASE_LOCKOWNER are listed as 
allowing NFS4ERR_ADMIN_REVOKED and I think they shouldn't be.  This error is defined as only
applying when a specific stateid has a problem.

From: [email-masked] [mailto:[email-masked]] On Behalf Of Frank S Filz
Sent: Tuesday, November 06, 2012 1:32 PM
To: J. Bruce Fields
Cc: [email-masked]; [email-masked]
Subject: Re: [nfsv4] RFC 3530 question: LOCKT and expired/stale clientid

> And that must be a bug too.

RELEASE_LOCKOWNER also seems to be missing from the list, missing from both these places.

Frank

---

From: Frank S Filz <[email-masked]>
Date: Tue, 6 Nov 2012 14:18:37 -0800

> isn't on the automatic renewal list.

Oops, it's actually so in -17. I guess I missed it and believed a comment
in
the code explicitly stating that it wasn't an allowed result.

>   - should cause lease renewal.

I've changed our code to conform to this (it actually was already renewing
leases).

Thanks

Frank

---

From: "Haynes, Tom" <[email-masked]>
Date: Wed, 7 Nov 2012 19:20:04 +0000

Do you have a suggested diff in mind?

---

From: "Noveck, David" <[email-masked]>
Date: Thu, 8 Nov 2012 13:23:28 -0500

(1): Replace

   The client can implicitly provide a a positive indication that it is
   still active and that the associated state held at the server, for
   the client, is still valid.  Any operation made with a valid clientid
   (DELEGPURGE, RENEW, OPEN, LOCK) or a valid stateid (CLOSE,
   DELEGRETURN, LOCK, LOCKU, OPEN, OPEN_CONFIRM, OPEN_DOWNGRADE, READ,
   SETATTR, or WRITE) informs the server to renew all of the leases for
   that client (i.e., all those sharing a given client ID).  In the
   latter case, the stateid must not be one of the special stateids
   consisting of all bits 0 or all bits 1.

With

   The client can implicitly provide a positive indication that it is
   still active and that the associated state held at the server, for
   the client, is still valid.  Any operation made with a valid clientid
   (DELEGPURGE, LOCK, LOCKT, OPEN, RELEASE_LOCKOWNER, or RENEW) or a 
   valid stateid (CLOSE, DELEGRETURN, LOCK, LOCKU, OPEN, OPEN_CONFIRM, 
   OPEN_DOWNGRADE, READ, SETATTR, or WRITE) informs the server to renew 
   all of the leases for that client (i.e., all those sharing a given 
   client ID).  In the latter case, the stateid must not be one of the 
   special stateids consisting of all bits 0 or all bits 1.

(2): Replace

   | LOCKT               | NFS4ERR_ACCESS, NFS4ERR_BADHANDLE,          |
   |                     | NFS4ERR_BAD_RANGE, NFS4ERR_BADXDR,          |
   |                     | NFS4ERR_DELAY, NFS4ERR_DENIED,              |
   |                     | NFS4ERR_FHEXPIRED, NFS4ERR_GRACE,           |
   |                     | NFS4ERR_INVAL, NFS4ERR_ISDIR,               |
   |                     | NFS4ERR_LEASE_MOVED, NFS4ERR_LOCK_RANGE,    |
   |                     | NFS4ERR_MOVED, NFS4ERR_NOFILEHANDLE,        |
   |                     | NFS4ERR_RESOURCE, NFS4ERR_SERVERFAULT,      |
   |                     | NFS4ERR_STALE, NFS4ERR_STALE_CLIENTID       |

With

   | LOCKT               | NFS4ERR_ACCESS, NFS4ERR_BADHANDLE,          |
   |                     | NFS4ERR_BAD_RANGE, NFS4ERR_BADXDR,          |
   |                     | NFS4ERR_DELAY, NFS4ERR_DENIED,              |
   |                     | NFS4ERR_EXPIRED, NFS4ERR_FHEXPIRED,         |
   |                     | NFS4ERR_GRACE, NFS4ERR_INVAL, NFS4ERR_ISDIR |
   |                     | NFS4ERR_LEASE_MOVED, NFS4ERR_LOCK_RANGE,    |
   |                     | NFS4ERR_MOVED, NFS4ERR_NOFILEHANDLE,        |
   |                     | NFS4ERR_RESOURCE, NFS4ERR_SERVERFAULT,      |
   |                     | NFS4ERR_STALE, NFS4ERR_STALE_CLIENTID       |

(3): Replace

   | RELEASE_LOCKOWNER   | NFS4ERR_ADMIN_REVOKED, NFS4ERR_BADXDR,      |
   |                     | NFS4ERR_EXPIRED, NFS4ERR_LEASE_MOVED,       |
   |                     | NFS4ERR_LOCKS_HELD, NFS4ERR_RESOURCE,       |
   |                     | NFS4ERR_SERVERFAULT, NFS4ERR_STALE_CLIENTID |

With

   | RELEASE_LOCKOWNER   | NFS4ERR_BADXDR, NFS4ERR_EXPIRED,            |
   |                     | NFS4ERR_LEASE_MOVED,  NFS4ERR_LOCKS_HELD,   |
   |                     | NFS4ERR_RESOURCE,  NFS4ERR_SERVERFAULT,     |
   |                     | NFS4ERR_STALE_CLIENTID                      |

(4): Replace 

   | RENEW               | NFS4ERR_ACCESS, NFS4ERR_ADMIN_REVOKED,      |
   |                     | NFS4ERR_BADXDR, NFS4ERR_CB_PATH_DOWN,       |
   |                     | NFS4ERR_EXPIRED, NFS4ERR_LEASE_MOVED,       |
   |                     | NFS4ERR_RESOURCE, NFS4ERR_SERVERFAULT       |

With

   | RENEW               | NFS4ERR_ACCESS, NFS4ERR_BADXDR,             |
   |                     | NFS4ERR_CB_PATH_DOWN, NFS4ERR_EXPIRED,      |
   |                     | NFS4ERR_LEASE_MOVED, NFS4ERR_RESOURCE,      |
   |                     | NFS4ERR_SERVERFAULT                         |

(5): Replace

   | NFS4ERR_ADMIN_REVOKED    | CLOSE, DELEGRETURN, LOCK, LOCKU, OPEN, |
   |                          | OPEN_CONFIRM, OPEN_DOWNGRADE, READ,    |
   |                          | RELEASE_LOCKOWNER, RENEW, SETATTR,     |
   |                          | WRITE                                  |   

With

   | NFS4ERR_ADMIN_REVOKED    | CLOSE, DELEGRETURN, LOCK, LOCKU, OPEN, |
   |                          | OPEN_CONFIRM, OPEN_DOWNGRADE, READ,    |
   |                          | SETATTR, WRITE                         |
 
(6): Replace

   | NFS4ERR_EXPIRED          | CLOSE, DELEGRETURN, LOCK, LOCKU, OPEN, |
   |                          | OPEN_CONFIRM, OPEN_DOWNGRADE, READ,    |
   |                          | RELEASE_LOCKOWNER, RENEW, SETATTR,     |
   |                          | WRITE                                  |

With

   | NFS4ERR_EXPIRED          | CLOSE, DELEGRETURN, LOCK, LOCKU, OPEN, |
   |                          | OPEN_CONFIRM, OPEN_DOWNGRADE, LOCKT,   |
   |                          | READ, RELEASE_LOCKOWNER, RENEW,        |
   |                          | SETATTR, WRITE                         |

---

From: Frank S Filz <[email-masked]>
Date: Thu, 8 Nov 2012 10:31:50 -0800

Looks good to me.

"Noveck, David" <[email-masked]> wrote on 11/08/2012 10:23:28 AM:

> "Noveck, David" <[email-masked]>
> 11/08/2012 10:23 AM
>
> To
>
> "Haynes, Tom" <[email-masked]>,
>
> cc
>
> Frank S Filz/Beaverton/IBM@IBMUS, "J. Bruce Fields"
> <[email-masked]>, "[email-masked]" <[email-masked]>
>
> Subject
>
> RE: [nfsv4] RFC 3530 question: LOCKT and expired/stale clientid
>
> (1): Replace
>

> >>> NFS4ERR_STALE_CLIENTID, yet, it does include a clientid with
thelock/open

---

From: "Haynes, Tom" <[email-masked]>
Date: Thu, 8 Nov 2012 21:22:15 +0000

Thanks, will be in the next version.