On Fri, May 05, 2023 at 02:04:01AM -0700, Andrea Bolognani wrote:
On Thu, May 04, 2023 at 02:21:57PM -0400, Laine Stump wrote:
On 5/4/23 4:33 AM, Andrea Bolognani wrote:
I don't think we need the BuildRequires, or the build time detection, at all. Just
#define NFT "nft"
in the relevant file and be done with it. We'll locate the binary at runtime, same as we're doing with most of them already.
Are we? What's the huge list of "optional programs" in meson.build then?
Leftovers, that I intend to clean up At Some Point™ :)
I don't have any problem with doing all binary-location at runtime, as long as we don't think there's any potential security problem / bug that could arise from having a different binary with the same name added in some place earlier in $PATH
If some malicious actor can alter root's $PATH, or inject binaries into it, it's pretty much game over already.
(is that why we started canonicalizing binary paths during the build?)
I think it was done more for feature detection purposes, e.g. only enable the network driver if ifconfig is present or something.
But that gets in the way of packagers, who usually want to explicitly enable/disable features anyway and to build in a minimal environment. It also assumes same-host deployment, and locks the configuration too early (what if I install ifconfig after building libvirt?).
Runtime detection has some drawbacks too, but overall is more flexible and we've been moving in that direction.
Maybe we also want to turn the iptables dependency into a Recommends? That way you will be able to uninstall it for a pure nft-based setup.
I was being ultra-conservative about the change, making it opt-in for the distros for now at least. But I'm also fine with making it opt-out
I believe Dan argued for the nft backend to be made the default where possible. I generally agree that we should adopt forward-looking defaults whenever that can be done without breaking existing users.
Anyway, regardless of which one of the backends ends up being the default one, maybe *both* nft and iptables should be Recommends? That way you'll get both installed by default, but you'll be able to drop the one that you're not using if you're aiming for a minimal deployment.
Fedora has used nft kmod since at least Fedora 32 IIRC. While you could potentially unload it and load the iptbles kmods I expect the users doing that are minimal if any. Even if someone is doing that, I see no reason why we can't exclusively have Requires: nft, and ignore iptables as far as deps are concerned. The only "downside" is that someone who has done the edge case of revertnig to iptables will have a redundant 'nft' userspace package installed. I think that's totally acceptable for such a niche edge case. Same for RHEL >= 9. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|