Re: Who needs these silly statfs changes...

On Fri, 14 Nov 2003, Peter Edwards wrote:

> Bruce Evans wrote:
>
> > On Fri, 14 Nov 2003, Peter Edwards wrote:

> >> The NFS protocols have unsigned fields where statfs has signed
> >> equivalents: NFS can't represent negative available disk space ( Without
> >> the knowledge of the underlying filesystem on the server, negative free
> >> space is a little nonsensical anyway, I suppose)
> >>
> >> The attached patch stops the NFS server assigning negative values to
> >> unsigned fields in the statfs response, and works against my local
> >> solaris box. Seem reasonable?
> >
> > The client attampts to fix this by pretending that the unsigned fields
> > are signed. -current tries to do more to support file system sizes larger
> > that 1TB, but the code for this is not even wrong except it may be wrong
> > enough to break the negative values. See my reply to one of the PRs
> > for more details.
> >
> > I just got around to testing the patch in that reply:
> > ...
>
> Your patch to nfs_vfsops won't apply to my Solaris kernel :-)
> The protocol says "abytes" is unsigned, so the server shouldn't be lying
> by sending a huge positive value for available space on a full
> filesystem. No?

Possibly not, but the protocol is broken if it actually requires that.
The "free" fields are signed in struct statfs so that they can be negative.
However, this is broken in POSIX's struct statvfs (all count fields have
type fsblkcnt_t or fsfilcnt_t and these are specified to be unsigned).
Is Solaris bug for bug compatible with that?

Anyway, my patch is mainly supposed to fix the scaling.  The main bug
in the initial scaling patch was that the huge positive values were
scaled before they were interpreted as negative values, so they became
not so huge but still preposterous values that could not be interpreted
as negative values.

The type pun to negative values is in most versions of BSD:

RELENG_4:
	u_quad_t tquad;
	...
	if (v3) {
		sbp->f_bsize = NFS_FABLKSIZE;
		tquad = fxdr_hyper(&sfp->sf_tbytes);
		sbp->f_blocks = (long)(tquad / ((u_quad_t)NFS_FABLKSIZE));
		tquad = fxdr_hyper(&sfp->sf_fbytes);
		sbp->f_bfree = (long)(tquad / ((u_quad_t)NFS_FABLKSIZE));
		tquad = fxdr_hyper(&sfp->sf_abytes);
		sbp->f_bavail = (long)(tquad / ((u_quad_t)NFS_FABLKSIZE));
		sbp->f_files = (fxdr_unsigned(int32_t,
		    sfp->sf_tfiles.nfsuquad[1]) & 0x7fffffff);
		sbp->f_ffree = (fxdr_unsigned(int32_t,
		    sfp->sf_ffiles.nfsuquad[1]) & 0x7fffffff);
	} else {
		sbp->f_bsize = fxdr_unsigned(int32_t, sfp->sf_bsize);
		sbp->f_blocks = fxdr_unsigned(int32_t, sfp->sf_blocks);
		sbp->f_bfree = fxdr_unsigned(int32_t, sfp->sf_bfree);
		sbp->f_bavail = fxdr_unsigned(int32_t, sfp->sf_bavail);
		sbp->f_files = 0;
		sbp->f_ffree = 0;
	}

Oops, this has the cast to long perfectly misplaced so that negative
sizes are not converted like I want.  It just prevents warnings.
Overflow has occurred long before, on the server when negative block
counts were converted to hug positive sizes.

NetBSD (nfs_vfsops.c 1.132):
	u_quad_t tquad;
	...
	...
	if (v3) {
		sbp->f_bsize = NFS_FABLKSIZE;
		tquad = fxdr_hyper(&sfp->sf_tbytes);
		sbp->f_blocks = (long)((quad_t)tquad / (quad_t)NFS_FABLKSIZE);
		tquad = fxdr_hyper(&sfp->sf_fbytes);
		sbp->f_bfree = (long)((quad_t)tquad / (quad_t)NFS_FABLKSIZE);
		tquad = fxdr_hyper(&sfp->sf_abytes);
		sbp->f_bavail = (long)((quad_t)tquad / (quad_t)NFS_FABLKSIZE);
		tquad = fxdr_hyper(&sfp->sf_tfiles);
		sbp->f_files = (long)tquad;
		tquad = fxdr_hyper(&sfp->sf_ffiles);
		sbp->f_ffree = (long)tquad;
	} else {
		sbp->f_bsize = fxdr_unsigned(int32_t, sfp->sf_bsize);
		sbp->f_blocks = fxdr_unsigned(int32_t, sfp->sf_blocks);
		sbp->f_bfree = fxdr_unsigned(int32_t, sfp->sf_bfree);
		sbp->f_bavail = fxdr_unsigned(int32_t, sfp->sf_bavail);
		sbp->f_files = 0;
		sbp->f_ffree = 0;
	}

This converts tquad to quad_t so that the divisions work like I want.  These
conversions were added in rev.1.82 in 1999.

More changes are needed here to catch up with the recent changes to struct
statfs in FreeBSD.  The casts to long are now just wrong since the block
count fields don't have type long.

Bruce