Details
- Reviewers
will jfv - Commits
- rS279437: Allow Illumos code to co-exist with nv(9)
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
The primary testing that I did was to ensure that a kernel with "device zfs" would still link. However, I suppose that doesn't exclude the possibility of their being an zfs source file that is built wrong and doesn't include this header. Is there a zfs test suite that I could run?
Not yet, unfortunately. It's in the zfsd project branch, but needs some work so it can be pushed into head.
It'd probably be sufficient to load zfs and create pools/filesystems, and a few other basic tests. Are the userland bits covered (and tested) with this change? This is a <sys/nvpair.h> overload (effectively), which may be pulled into the userland zfs builds.
Ok, I did a full buildworld/buildkernel (with device zfs in my kernel config) and installed it into a couple of zfs. I tested creating a zpool out of a mfs disk, creating filesystems, setting some properties, doing a git clone into a file system, and then doing a zfs send of the result from one vm to the problem. No problems cropped up.
This has already been committed, so I've missed the train, but I still would like to comment. And I hope that my comment won't be just a rant.
So, I don't like this change for several reasons.
For one, given that nv(9) is a new thing while libnvpair already existed it would have been wise to select non-conflicting names for the new interfaces. And I think that it's still not too late to do that.
Then, it seems that this change has solved a theoretical problem as I don't think that currently there is any executable that links to both libraries.Perhaps I am wrong.
Also, libnvpair.so was treated like a "private" library: its ABI was wildly changed but its version hasn't been bumped.
Finally, from ABI point of view libnvpair now appears very inconsistent: many of its interfaces are prefixed with "illumos_" while quite a few don't have that prefix.
Now, why am I interested in libnvpair?
At work we have a Python module that interfaces libzfs_core and by necessity libnvpair through CFFI.
The module used to work perfectly well across FreeBSD, illumos and Linux because the library interfaces on ABI level are the same (from CFFI's point of view) across platforms.
After this change FreeBSD is an odd platform. I have to add a workaround to keep the module working. And even the workaround is not trivial because of the mix of prefixed and non-prefixed names.
Hope you sympathize.
Another thought.
Or was the intention only to make nv(9) and hrm.. nvpair compatible only within the kernel? Section 9 seems like a big hint here, but I am not sure.
If the intention was such, then my previous comment is to inform that that change affected the userland as well.