Subject: RE: spec update?

From: "Noveck, Dave" <[email-masked]>
Date: Wed, 2 May 2001 09:59:06 -0700

This response is mainly regarding the interoperability issues that
Kendrick raises.  I think it is important not to lose the inter-
operability we already have, just because a few spec issues
are a bit unsettled.

I will comment on the substance of some of the issues in passing.

I think the rule we should be adhering to right now is  that if 
something is clear in rfc3010, we should code to that, and not try to 
anticipate changes, even if there is general agreement that they will 
be made.  On the other hand, in cases where the spec does not say 
anything either way, we should look to what everyone has agreed is 
the correct interpretation (and will presumably be reflected in the
updated spec).

> Toward the end of Connectathon, it was agreed informally that a number of
> changes needed to be made to the spec.  As I see it, the main issues are:

>   o  A precise specification of the conditions under which a lockowner's
> seqid should be incremented on error.  On the one hand, if the seqid is
> never incremented on error, which is what we're doing now, we are open to
> some inherent race conditions (as pointed out by Neil Brown on the list
> shortly before Connectathon).  On the other hand, for some errors, such
> as ERR_BAD_STATEID, the server can't identify which lockowner the client
> is trying to specify, so the client can't assume that the seqid has been
> incremented on the server (although it's not clear that anything would be
> lost if the seqids got desynchronized in this case; if the client is
> emitting bad stateids, there's probably not much hope of the client
> recovering in any case).

This is a case in which the spec doesn't really say anything one way
or the other, but we know that what we have done previously is not right,
and we should change.

While I agree that the spec should have a precise description of when
to increment, I don't believe we should wait for that to stop doing what
is clearly wrong, which is never incrementing the sequence number on 
any error.  Neil Brown proved that won't work, so let's not do that.

The only reason we didn't make this change for Connectathon is that 
there was no time.  I believe we should be doing the right thing 
henceforth.  This is one of the most complicated areas of the protocol
so if there are problems with what we now believe to be correct let's
not wait to find them.

>    o  A precise specification of what types of byterange locking servers
> are required to support without returning ERR_LOCK_RANGE.  Can a write
> lock be atomically downgraded to a read lock, for example? 

UNIX can do this and all the Windows people I talked to said Windows/NT can 
as well, so I don't see any problem requiring servers to support this.

> Should there
> be a way for a client to ask the server what types of locking operations
> it supports, such as a new file attribute in GETATTR?

I think the spec does need to be a lot more explicit about what locking
operations the server must support and which are optional.  An attribute 
might be part of resolving that.  I think we need a proposal that addresses
all those pesky details.  Volunteers?

>    o  Replacing 64-bit stateids with 128-bit stateids.  (Everyone seemed
> to think that this was a good idea.)

As I recall everybody thought 64-bit stateids was too tight but there
was no general agreement about exactly what to do.  On occasion, I've
suggested 96 bits would be OK with me, too.

There are two other ways to deal with the 64-bit stateid constraint that
occur to me:

     Explicitly say that a stateid is only valid in the context of a 
     given filehandle.  This would involve requiring filehandles be set
     in a very few ops that it isn't now (RENEW is one but that may be 
     dealt with in another way).  If this were done, then I could multiply
     the stateid space many-fold without actually expanidng them.  There
     might be problems with this approach in the world of volatile file
     handles, however.

     Keep stateid's 64-bit but reieve them of the responsibility of
     detecting server reboot (i.e. don't need to devote a lot of it to
     be boot time, for example).  Then require that everywhere you 
     now pass a stateid to the server, you pass a clientid-stateid
     pair (as a state-value structure, maybe), but where the server 
     returns a stateid to you, the 64 bits would suffice.   

> Here are two more issues which I seem to recall most people agreeing on,
> although it was less clear whether these were definitely going into the
> spec or not:
> 
>    o  Modifying the RENEW operation to take a clientid as its argument
> instead of a stateid.

I didn't hear anybody object to this, but I'm certainly have no plans
to change my code until it as it least in an (at least) semi-official draft.
 
>    o  Adding a 'type' field, of type nfs_lock_type4, to the LOCK4denied
> structure, to support correct implementation of the POSIX system call
> fcntl(F_GETLK,...).

I thought it was clear that this would be done, but I haven't changed
my code and don't plan to until it is in some sort of draft.  

> I was wondering whether these changes are still going to be made, and if
> so, when they will be made available, even in "unofficial" form pending
> inclusion into the spec. 

I'm guessing that most of the changes you mention will be made in some form, 
and I think there are some others that might be included (e.g. some way
of resolving the open upgrade/downgrade issue).

I'm not hung up on the officialness of the spec, but I would like to 
have something that was at least in the form of an updated spec that 
we could all point to as defining the protocol for interoperability 
testing as of any given time.

> Our implementation is becoming increasingly
> polluted with conditionals such as #if STATEID_128_BITS (I don't know if
> the other developers are doing something similar), and it's getting to be
> quite awkward. 

I haven't tried to anticipate changes, but I do think we should be moving
faster to resolve these issues. 

> It would be nice to put these issues to rest soon,
> especially since the next bakeoff is on the horizon..