Subject: LOCK,LOCKT,LOCKU

From: "Noveck, Dave" <[email-masked]>
Date: Tue, 14 Dec 1999 08:01:30 -0800

I'm actually starting to implement the locking stuff and
so I've been looking at the LOCK* ops in the spec more
carefully.  I think they need some work.

LOCK, LOCKT, and LOCKU supposedly all take a LOCK4args
but LOCKT and LOCKU's description of this is different
from that for LOCK and what is in the .x file.  Also,
I don't understand some of the result definitions.
(this is based on the version-02 of the spec.)

I think the idea of trying to share LOCK4ARGS among the
different ops is a mistake and just adds to confusion.

As far as args,

     LOCKT and LOCKU have a lockowner instead of a
     stateid.  You could get the owner from the stateid
     if you actually have some lock on the file.  Also,
     having LOCKU take a lockowner rather than a stateid
     would (I think) allow you to unlock locks in a case
     some leases had expired, since without a stateid,
     you won't get a EXPIRED error on the stateid.  I
     would vote to keep LOCKT and LOCKU as having a 
     lockowner, as described in the spec, rather than
     as they currently are in the .x file.

     Right now all of these have a reclaim parameter
     which is presumably either ignored or checked and
     required to be false (which?) in the LOCKU and
     LOCKT cases.  Why not just pull these out of 
     LOCKT and LOCKU?

As far as results,

     What is the access field in the result for LOCK?  What 
     form does it have and why do we need it?  If you get the
     lock, you have the access you asked for.  Don't you?

     What is the purpose of the stateid_oth in the default
     case of LOCKU?  Why does the client need to have a
     stateid when unlock fails?  Presumably, it is unchanged
     in this case and the client knows what it is.  Also,
     if there is some error case in which it is required,
     then that could be handled but trying to return it in
     all error cases (e.g. FHEXPIRED, STALE, SERVERFAULT) 
     doesn't seem right.

---

From: Spencer Shepler <[email-masked]>
Date: Tue, 14 Dec 1999 12:40:16 -0600

> different ops is a mistake and just adds to confusion.

Agreed.

>      as they currently are in the .x file.

If the lease has expired, the lock will no longer exist
and the client won't have a need to do a LOCKU, right?
I have changed LOCKT to pass nfs_lockowner but not LOCKU
(still stateid).

>      LOCKT cases.  Why not just pull these out of

Removed.

>      lock, you have the access you asked for.  Don't you?

Removed.

>      all error cases (e.g. FHEXPIRED, STALE, SERVERFAULT)

I don't have anything in my notes about it so I have 
removed the stateid return in the case of failure.

Here is the updated (and dependent) portions of the .x definition:
-----
typedef uint32_t	seqid4;
typedef uint64_t	stateid4;
typedef uint64_t	offset4;
typedef	uint64_t	length4;

enum nfs_lock_type4 {
	READ_LT 	= 1,
	WRITE_LT 	= 2,
	READW_LT 	= 3,	/* blocking read */
	WRITEW_LT 	= 4	/* blocking write */
};

struct nfs_lockowner4 {
	clientid4	clientid;
	opaque		owner<>;
};

/*
 * LOCK/LOCKT/LOCKU: Record lock management
 */
struct LOCK4args {
	/* CURRENT_FH: file */
	nfs_lock_type4	type;
	seqid4		seqid;
	bool		reclaim;
	stateid4	stateid;
	offset4		offset;
	length4		length;
};

union LOCK4res switch (nfsstat4 status) {
 case NFS4_OK:
	 stateid4	stateid;
 default:
	 void;
};

struct LOCKT4args {
	/* CURRENT_FH: file */
	nfs_lock_type4	type;
        nfs_lockowner   owner;
	offset4		offset;
	length4		length;
};

union LOCKT4res switch (nfsstat4 status) {
 case NFS4ERR_DENIED:
	 nfs_lockowner4	owner;
 case NFS4_OK:
	 void;
 default:
	 void;
};

struct LOCKU4args {
	/* CURRENT_FH: file */
	nfs_lock_type4	type;
	seqid4		seqid;
	stateid4	stateid;
	offset4		offset;
	length4		length;
};

union LOCKU4res switch (nfsstat4 status) {
 case	NFS4_OK:
	 stateid4	stateid_ok;
 default:
	 void;
};