cvs commit: src/sys/fs/nullfs null_vfsops.c src/sys/fs/nwfs nwfs_vfsops.c src/sys/fs/smbfs smbfs_vfsops.c src/sys/ufs/ufs quota.h ufs_quota.c ufs_vfsops.c src/sys/kern vfs_default.c vfs_vnops.c vnode_if.src src/sys/sys mount.h vnode.h

[Robert Watson]

On Tue, 26 Feb 2008, Dag-Erling Smørgrav wrote:

> Robert Watson <rwatson at FreeBSD.org> writes:
>> Dag-Erling Smørgrav <des at des.no> writes:
>>> No, it changes neither the API nor the ABI.  It replaces caddr_t (which is 
>>> typedef'd to char *) with void *, and those two are compatible types.
>> I'm sorry, but I disagree.  The case you failed to test involves a function 
>> pointer typedef.
>
> Umm, OK, I didn't spot that.  It's unfortunate that David didn't change our 
> own file system code to match (or even build LINT, which would have shown 
> him the problem), so that RELENG_6 current doesn't build.
>
> Still, I would argue for fixing the code rather than reverting the change.
>
>> Here's the test I had to add to Arla to detect the change with autoconf; 
>> without this autoconf mess and changed prototypes in the Arla nnpfs code, I 
>> can't build nnpfs on -CURRENT, and presumably now also on our -STABLE 
>> branches:
>
> Arguably, Arla has been broken for more than two years and you only just 
> woke up and noticed.  If you had tried to build Arla on -CURRENT at any 
> point since 2005/12/14, you would have noticed and presumably fixed it.

You're missing the point.  This isn't about Arla being broken on 7.x and 8.x 
-- we knew that already, and hence my already having autoconf code to detect 
and adapt to the difference in the 7.x and 8.x KPIs.  What this is about is 
the fact that Arla *did* work on 6.x before this change, and now it's broken. 
This is because the KPI has been broken, as our KPI definition is pretty 
pragmatic (undocumented): third party kernel modules that implement the 
quotactl vfsop may well no longer compile.  I couldn't tell you which those 
are off-hand, but it wouldn't surprise me to hear that there are other third 
party kernel modules that declare quotactl implementations.

I'm fine with us discussing how committed we are to VFS being a KPI we don't 
break in -STABLE branches, but I'm not fine with the claim that the KPI wasn't 
broken by this change.  If pushed on the breaking the KPI point, I would come 
down on the side that says 6.x is a pretty mature branch at this point, and 
that breaking the KPI for purely cosmetic reasons (i.e., caddr_t -> void *) 
doesn't cut it.  I would be more swayed by an argument involving a necessary 
KPI change to address a critical bugfix, such as a race condition that results 
from a poor existing interface.

Robert N M Watson
Computer Laboratory
University of Cambridge