Re: Who needs these silly statfs changes...

peter.edwards_at_openet-telecom.com wrote:
> >I disagree.
> >
> >The intent of the negative number from df is to subtract the amount
> >used from the total amount available, in order to get the amount
> >remaining.
> 
> I just don't see how you can possibly infer from the NFS spec that
> "abytes" is anything other than an unsigned quantity. I just think
> assuming the client will interpret a massive value as probably
> negative is a bit of a leap of faith.

I'm not.  I'm saying that the value needs to be zero on the server,
if it's zero.

In reality, the negative value that's being stuffed is wrong; it's
possible to be "root" over NFS to a server that supports the idea
of a free reserve.  If you represent the free reserve over the wire,
then you have the problem; if you don't represent the free reserve
over the wire, then you don't.

The problem here is that you are taking the reported value for the
available bytes *after* the local free reserve (if any) is taken
into account, and exporting it over the wire.


> >This is an artifact of implementation on the server, and should
> >not be second-guessed by the client.
> 
> If the server tells the client that there are 2^64 - 1 bytes
> remaining on the server, it's not second guessing anything by
> presenting that to the user.

The server can't tell the client that, because that's outside the
range that's representable witin the NFS protocol.  Now if you
were willing to limit yourself to sizeof(abytes), then you have a
valid argument.

> >The problem in this case is on the client, not the server, in not
> >doing the conversion as an unsigned operation.  The place for the
> >subtraction to occur is in the "df" program.  In other words, the
> >statfs->f_bavail should be recalculated locally from the values
> >of statfs->f_blocks and statfs->f_bfree, not used directly out of
> >the (unsigned) NFS values... or the values should be converted to
> >signed values coming out of NFS prior to their sign extension to
> >the size type.
> 
> (Note that NFS also gives a "fbytes", indicating the number of free
> bytes, as opposed to "available to a particular user")
> 
> "bavail" can really only be worked out by the server. The server
> is reserving a percentage for non-root user. The client can't work
> out what that reserve percentage is.

Sure, fine, work it out.  And report 0 if the value is negative
on the server after you work it out.

In theory, it's not possible for this value to be less than 0 on
the server, unlss you are doing your calculations wrong, since
you can't assume the credential to be used by the client user, the
state of the -maproot option globally, for all mounts, or that the
client is familiar with the concept of a free reserve.

So the only server problem, if there is one, is that the server
should report the number of unallocated bytes, ignoring the
free reserve.


> >On a slightly related note, the standards mandated interfaces say
> >that the values should be fsblkcnt_t, which must be an unsigned
> >integer type.  This coordinates well with my point of the sign
> >conversion on legacy interface needing to happen at presentation
> >time.
> 
> Maybe we're talking across each other. That's my main point: the
> server shouldn't put huge values in an unsigned field and
> expect the client to interpret them in a way that the spec sets
> no precedent for.

It depends; are these values "huge" because they're "huge", or are
they "huge" because that's what you get when you convert a negative
signed value to an unsigned value as bits, without clamping negative
values to 0?

I think Bruce Evans' post on type conversion is relevent here.

The largest value you should ever stuff in it is MAX_UINT (or
whatever sized type maps to sizeof(abytes), and the smallest is
0.

If you do that, the value is absolutely correct when going over
the wire, and whether or not remote(MAX_UNIT) > signed local
representation is a problem for the client.


> >Also, if you read the ISO C99 standard, you'll see that on an
> >ILP32 system, there is no way to legitimately define an integer
> >type in excess of 32 bits, unless long is larger than 32 bits
> >(see section 3.6), so defining these things as 64 bits without
> >compiler changes is wrong anyway.
> 
> As far as implementing NFS is concerned, that's probably not
> relevant: It doesn't have to be implemented in ISO C. The FreeBSD
> compiler provides a 64-bit integer type that its implementation
> is free to use :-)

You misunderstand: statfs/fstatfs is not a standard interface.  It
may be time to deprecate it in favor of statvfs/fstatvfs.  If we
did that, the values become unsigned, and we could (maybe) ignore
the problem (everywhere except the traditional reporting of negative
blocks available in "df", which could be handled by an unsigned
compare and a "%c" that got ' ' or '-').

Alternately, we ned to acknowledge that the interface that's being
tried for is outside the scope of standards entirely.  Doing this
is a real pain, though, since idiots will use it to "get more
resolution", and it will proagate.

Probably the best thing would be to implement statvfs64/fstatvfs64
as an extension, and deprecate statfs/fstatfs.

Any way you look at it, it's a lot more things that need to be done
than the proposed patch accomplishes, and it's too close to 5.2 for
it to go in.

I'd be happy if the change was backed out until all the issues were
properly addressed.  As it is, the change effectively deprecates
the standards-mandated statvfs/fstatvfs interfaces in favor of
perpetuating the non-standard statfs/fstatfs interfaces, and the
proposed patch sanctifies it by making it more useful.  8-(.

-- Terry