Date: Tue, 26 Feb 2008 13:29:57 +0000 (GMT) From: Robert Watson <rwatson@FreeBSD.org> To: =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= <des@des.no> Cc: cvs-src@FreeBSD.org, Kris Kennaway <kris@FreeBSD.org>, src-committers@FreeBSD.org, cvs-all@FreeBSD.org, "David E. O'Brien" <obrien@FreeBSD.org> Subject: Re: 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 Message-ID: <20080226132155.W90776@fledge.watson.org> In-Reply-To: <86r6ezswnu.fsf@ds4.des.no> References: <200802250855.m1P8t3w6052042@repoman.freebsd.org> <47C29A07.2070908@FreeBSD.org> <86d4qli8a1.fsf@ds4.des.no> <20080226123107.N90776@fledge.watson.org> <86r6ezswnu.fsf@ds4.des.no>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 26 Feb 2008, Dag-Erling Smørgrav wrote: > Robert Watson <rwatson@FreeBSD.org> writes: >> Dag-Erling Smørgrav <des@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080226132155.W90776>