- Devel - libvirt List Archives

Re: [libvirt PATCH 00/28] native support for nftables in virtual network driver
by Roman Bogorodskiy 14 Jun '24

14 Jun '24

Laine Stump wrote: > On 6/10/24 2:54 PM, Roman Bogorodskiy wrote: > > Laine Stump wrote: > > > > > This patch series enables libvirt to use nftables rules rather than > > > iptables *when setting up virtual networks* (it does *not* add > > > nftables support to the nwfilter driver). It accomplishes this by > > > abstracting several iptables functions (from viriptables.[ch] called > > > by the virtual network driver into a rudimentary "virNetfilter API" > > > (in virnetfilter.[ch], having the virtual network driver call the > > > virNetFilter API rather than calling the existing iptables functions > > > directly, and then finally adding an equivalent virNftables backend > > > that can be used instead of iptables (selected manually via a > > > network.conf setting, or automatically if iptables isn't found on the > > > host). > > > > Hi, > > > > Apparently, I'm late to the discussion. > > > > I noticed that now I cannot use the bridge driver on FreeBSD as it's > > failing to initialize both iptables and nftables backends (which is > > expect). > > Yeah, previously we wouldn't check if iptables was available until someone > tried to start a network that would need to use it, and would then log an > error (and just fail starting that network, but the network driver would > remain running). But now we figure out which firewall backend to use > immediately when the driver is loaded, and if we fail to fin a workable > backend we fail the entire driver init.r > > How did you use the network driver before? With a <forward mode='open'/> > network? Truthfully I hadn't ever considered the case of someone using it > with only network types that didn't need firewall rules. I wonder if there > are other platforms we support that have a usable network driver for > <forward mode='open'/> (MacOS?) I'm using it with the following network configuration: virsh # net-dumpxml default <network> <name>default</name> <uuid>2a1415c9-325b-41e4-82c6-e805162d8934</uuid> <forward mode='nat'/> <bridge name='virbr0' stp='on' delay='0'/> <mac address='52:54:00:24:fa:43'/> <ip address='192.168.122.1' netmask='255.255.255.0'> <dhcp> <range start='192.168.122.2' end='192.168.122.254'/> </dhcp> </ip> </network> So basically all the mechanics like creating tap devices, bridges, serving dhcp, etc, all these work for me. On top of that I had a few iterations of manual firewall configurations (with both ipfw and pf) to implement NAT on guests. Unfortunately, I don't have access to that setup anymore and I haven't re-created it yet. IIRC, it could probably show some warnings about missing iptables, but it didn't affect anything for me. > > > > What would be a good way to address that? I see at least two options: > > > > 1. Add a Noop firewall driver > > 2. Implement a "real" FreeBSD driver based either on pf or ipfw (that's > > been on my TODO list forever, but I somehow got stuck on the very first > > step on choosing between pf and ipfw). > > Why not both? :-) > > > This obviously will take much > > more time. > > > > Maybe there are other options I'm missing. > > Obviously (2) would be nicest, but I guess in the short term some variation > of (1) would be quicker. > > Another possibility could be to restore the old behavior of saving the error > and only reporting it when a network requiring a firewall is loaded, but I > think I remember a discussion about this during review of an earlier > revision of the patches, and we agreed that it made the problem easier to > find if it was reported immediately and the driver load failed. > > I suppose in the long run the build-time option firewall_backend_priority > should be used to control which backends are included in the build (rather > than just which ones are checked at runtime), so that FreeBSD could > completely skip all the iptables and nftables code (and firewalld when > that's done), and Linux platforms could skip pf and ipfw. > > > > > What do you think? > > I'm about to be offline for 3 weeks, but in the meantime if you'd like to > try making a NULL backend that is only an option if it's listed in > firewall_backend_priority (you'll need to remove the compile-time check that > all possible backends are accounted for - I think that is the first of the > two G_STATIC_ASSERTS at the top of virNetworkLoadDriverConfig()), always > initializes successfully in bridge_driver_conf.c if it is listed in the > options, and then in networkAddFirewallRules add a check to log an error and > fail if backend == NULL (something about attempting to start a network type > that would require firewall rules, but the system not having any of the > supported types of firewallbackend or something - it's too late now and my > brain is too fried and sleepy to think of good wording :-)). As long as it > isn't a valid selection on Linux builds that are done with > firewall_backend_priority=nftables,iptables, but *is* a valid selection if > the setting is "firewall_backend_priority=null" that shouldn't be *too* > controversial. Ok, I think I can try making the NULL backend. > Later we can talk about pf and ipfw backends :-) Yeah, that sounds good. My main problem with the choice is that ipfw is the most actively supported firewall, but it relies quite heavily on the rule numbering, which makes it a little hard to integrate with user-specific rules (i.e. defined outside of libvirt). The "pf" seem to be better in this regard (at least to my taste), but it's not "native" FreeBSD firewall and is not as active (at least, to my impression). Roman

4 13

[PATCH 0/3] Couple of memleak fixes
by Michal Privoznik 14 Jun '24

14 Jun '24

The third patch MIGHT fix the following issue: https://issues.redhat.com/browse/RHEL-22574 but at this point it's still unclear. I'll append appropriate 'Resolves:' line when I learn more. Michal Prívozník (3): virfirewall: Fir a memleak in virFirewallParseXML() virnetworkobj: Free fwRemoval before setting another one in virNetworkObjSetFwRemoval() remote_daemon_dispatch: Unref sasl session when closing client connection src/conf/virnetworkobj.c | 1 + src/remote/remote_daemon_dispatch.c | 4 ++++ src/util/virfirewall.c | 2 +- 3 files changed, 6 insertions(+), 1 deletion(-) -- 2.44.2

2 6

[PATCH] network: introduce a "none" firewall backend type
by Daniel P. Berrangé 13 Jun '24

13 Jun '24

There are two scenarios identified after the recent firewall backend selection was introduced, which result in libvirtd failing to startup due to an inability to find either iptables/nftables - On Linux if running unprivileged with $PATH lacking the dir containing iptables/nftables - On non-Linux where iptables/nftables never existed In the former case, it is preferrable to restore the behaviour whereby the driver starts successfully. Users will get an error reported when attempting to start any virtual network, due to the lack of permissions needed to create bridge devices. This makes the missing firewall backend irrelevant. In the latter case, the network driver calls the 'nop' platform implementation which does not attempt to implement any firewall logic, just allowing the network to start without firewall rules. To solve this are number of changes are required * Introduce VIR_FIREWALL_BACKEND_NONE, which does nothing except report a fatal error from virFirewallApply(). This code path is unreachable, since we'll never create a virFirewall object with with VIR_FIREWALL_BACKEND_NONE, so the error reporting is just a sanity check. * Ignore the compile time backend defaults and assume use of the 'none' backend if running unprivileged. This fixes the first regression, avoiding the failure to start libvirtd on Linux in unprivileged context, instead allowing use of the driver and expecting a permission denied when creating a bridge. * Reject the use of compile time backend defaults no non-Linux and hardcode the 'none' backend. The non-Linux platforms have no firewall implementation at all currently, so there's no reason to permit the use of 'firewall_backend_priority' meson option. This fixes the second regression, avoiding the failure to start libvirtd on non-Linux hosts due to non-existant Linux binaries. * Change the Linux platform backend to raise an error if the firewall backend is 'none'. Again this code path is unreachable by default since we'll fail to create the bridge before getting here, but if someone modified network.conf to request the 'none' backend, this will stop further progress. * Change the nop platform backend to raise an error if the firewall backend is 'iptables' or 'nftables'. Again this code path is unreachable, since we should already have failed to find the iptables/nftables binaries on non-Linux hosts, so this is just a sanity check. * 'none' is not permited as a value in 'firewall_backend_priority' meson option, since it is conceptually meaningless to ask for that on Linux. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- meson.build | 26 +++++++++++++++++++------- meson_options.txt | 2 +- src/network/bridge_driver_conf.c | 19 ++++++++++++++----- src/network/bridge_driver_linux.c | 10 ++++++++++ src/network/bridge_driver_nop.c | 13 ++++++++++++- src/util/virfirewall.c | 6 ++++++ src/util/virfirewall.h | 1 + 7 files changed, 63 insertions(+), 14 deletions(-) diff --git a/meson.build b/meson.build index 5c7cd7ec2e..2e8b87280d 100644 --- a/meson.build +++ b/meson.build @@ -1647,15 +1647,27 @@ if not get_option('driver_network').disabled() and conf.has('WITH_LIBVIRTD') conf.set('WITH_NETWORK', 1) firewall_backend_priority = get_option('firewall_backend_priority') - if (not firewall_backend_priority.contains('nftables') or - not firewall_backend_priority.contains('iptables') or - firewall_backend_priority.length() != 2) - error('invalid value for firewall_backend_priority option') + if firewall_backend_priority.length() == 0 + if host_machine.system() == 'linux' + firewall_backend_priority = ['nftables', 'iptables'] + else + # No firewall impl on non-Linux so far, so force 'none' + # as placeholder + firewall_backend_priority = ['none'] + endif + else + if host_machine.system() != 'linux' + error('firewall backend priority only supported on linux hosts') + endif endif - conf.set('FIREWALL_BACKEND_PRIORITY_0', 'VIR_FIREWALL_BACKEND_' + firewall_backend_priority[0].to_upper()) - conf.set('FIREWALL_BACKEND_PRIORITY_1', 'VIR_FIREWALL_BACKEND_' + firewall_backend_priority[1].to_upper()) - conf.set('FIREWALL_BACKEND_PRIORITY_NUM', firewall_backend_priority.length()) + backends = [] + foreach backend: firewall_backend_priority + backend = 'VIR_FIREWALL_BACKEND_' + backend.to_upper() + backends += backend + endforeach + + conf.set('FIREWALL_BACKENDS', ', '.join(backends)) elif get_option('driver_network').enabled() error('libvirtd must be enabled to build the network driver') endif diff --git a/meson_options.txt b/meson_options.txt index 50d71427cb..2d440c63d8 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -117,7 +117,7 @@ option('dtrace', type: 'feature', value: 'auto', description: 'use dtrace for st option('firewalld', type: 'feature', value: 'auto', description: 'firewalld support') # dep:firewalld option('firewalld_zone', type: 'feature', value: 'auto', description: 'whether to install firewalld libvirt zone') -option('firewall_backend_priority', type: 'array', choices: ['nftables', 'iptables'], description: 'order in which to try firewall backends') +option('firewall_backend_priority', type: 'array', choices: ['nftables', 'iptables'], value: [], description: 'order in which to try firewall backends') option('host_validate', type: 'feature', value: 'auto', description: 'build virt-host-validate') option('init_script', type: 'combo', choices: ['systemd', 'openrc', 'check', 'none'], value: 'check', description: 'Style of init script to install') option('loader_nvram', type: 'string', value: '', description: 'Pass list of pairs of <loader>:<nvram> paths. Both pairs and list items are separated by a colon.') diff --git a/src/network/bridge_driver_conf.c b/src/network/bridge_driver_conf.c index e2f3613a41..9da5e790b7 100644 --- a/src/network/bridge_driver_conf.c +++ b/src/network/bridge_driver_conf.c @@ -61,6 +61,7 @@ networkGetDnsmasqCaps(virNetworkDriverState *driver) static int virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, + bool privileged, const char *filename) { g_autoptr(virConf) conf = NULL; @@ -68,13 +69,17 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, bool fwBackendSelected = false; size_t i; int fwBackends[] = { - FIREWALL_BACKEND_PRIORITY_0, - FIREWALL_BACKEND_PRIORITY_1, + FIREWALL_BACKENDS }; - G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == VIR_FIREWALL_BACKEND_LAST); - G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) == FIREWALL_BACKEND_PRIORITY_NUM); + G_STATIC_ASSERT(G_N_ELEMENTS(fwBackends) > 0 && + G_N_ELEMENTS(fwBackends) <= VIR_FIREWALL_BACKEND_LAST); int nFwBackends = G_N_ELEMENTS(fwBackends); + if (!privileged) { + fwBackends[0] = VIR_FIREWALL_BACKEND_NONE; + nFwBackends = 1; + } + if (access(filename, R_OK) == 0) { conf = virConfReadFile(filename, 0); @@ -104,6 +109,10 @@ virNetworkLoadDriverConfig(virNetworkDriverConfig *cfg G_GNUC_UNUSED, for (i = 0; i < nFwBackends && !fwBackendSelected; i++) { switch ((virFirewallBackend)fwBackends[i]) { + case VIR_FIREWALL_BACKEND_NONE: + fwBackendSelected = true; + break; + case VIR_FIREWALL_BACKEND_IPTABLES: { g_autofree char *iptablesInPath = virFindFileInPath(IPTABLES); @@ -187,7 +196,7 @@ virNetworkDriverConfigNew(bool privileged) configfile = g_strconcat(configdir, "/network.conf", NULL); - if (virNetworkLoadDriverConfig(cfg, configfile) < 0) + if (virNetworkLoadDriverConfig(cfg, privileged, configfile) < 0) return NULL; if (g_mkdir_with_parents(cfg->stateDir, 0777) < 0) { diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c index 35e6bd1154..fe7c6e193c 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -47,6 +47,11 @@ networkFirewallSetupPrivateChains(virFirewallBackend backend, virFirewallLayer layer) { switch (backend) { + case VIR_FIREWALL_BACKEND_NONE: + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("No firewall backend is available")); + return -1; + case VIR_FIREWALL_BACKEND_IPTABLES: return iptablesSetupPrivateChains(layer); @@ -417,6 +422,11 @@ networkAddFirewallRules(virNetworkDef *def, } switch (firewallBackend) { + case VIR_FIREWALL_BACKEND_NONE: + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("No firewall backend is available")); + return -1; + case VIR_FIREWALL_BACKEND_IPTABLES: return iptablesAddFirewallRules(def, fwRemoval); diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c index 537b9234f8..7797be1ba8 100644 --- a/src/network/bridge_driver_nop.c +++ b/src/network/bridge_driver_nop.c @@ -37,9 +37,20 @@ int networkCheckRouteCollision(virNetworkDef *def G_GNUC_UNUSED) } int networkAddFirewallRules(virNetworkDef *def G_GNUC_UNUSED, - virFirewallBackend firewallBackend G_GNUC_UNUSED, + virFirewallBackend firewallBackend, virFirewall **fwRemoval G_GNUC_UNUSED) { + /* + * Shouldn't be possible, since virNetworkLoadDriverConfig + * ought to fail to find the required binaries when loading, + * so this is just a sanity check + */ + if (firewallBackend != VIR_FIREWALL_NONE) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("Firewall backend '%s' not available on this platform"), + virFirewallBackendTypeToSTring(firewallBackend)); + return -1; + } return 0; } diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 2219506b18..d374f54b64 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -37,6 +37,7 @@ VIR_LOG_INIT("util.firewall"); VIR_ENUM_IMPL(virFirewallBackend, VIR_FIREWALL_BACKEND_LAST, + "none", "iptables", "nftables"); @@ -815,6 +816,11 @@ virFirewallApplyCmd(virFirewall *firewall, } switch (virFirewallGetBackend(firewall)) { + case VIR_FIREWALL_BACKEND_NONE: + virReportError(VIR_ERR_NO_SUPPORT, + _("Firewall backend is not implemented")); + return -1; + case VIR_FIREWALL_BACKEND_IPTABLES: if (virFirewallCmdIptablesApply(firewall, fwCmd, &output) < 0) return -1; diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h index 302a6a4e5b..bce51259d2 100644 --- a/src/util/virfirewall.h +++ b/src/util/virfirewall.h @@ -44,6 +44,7 @@ typedef enum { } virFirewallLayer; typedef enum { + VIR_FIREWALL_BACKEND_NONE, /* Always fails */ VIR_FIREWALL_BACKEND_IPTABLES, VIR_FIREWALL_BACKEND_NFTABLES, -- 2.45.1

2 1

[PATCH 0/2] network: fix network driver to gracefully skip startup
by Daniel P. Berrangé 13 Jun '24

13 Jun '24

We should gracefully skip startup when: * No network.conf firewall_backend is explicitly set, and neither iptables/nftables are present * Running unprivileged The former fixes libvirtd startup on non-Linux, or minimal linux installs without firewall tools. The latter skips pointless initialization that creates a driver that cannot do anything useful Daniel P. Berrangé (2): network: skip network driver init if no firewall backend is present network: don't attempt to initialize if non-privileged src/network/bridge_driver.c | 14 +++++++++++++- src/network/bridge_driver_conf.c | 8 ++++---- 2 files changed, 17 insertions(+), 5 deletions(-) -- 2.45.1

3 6

[PATCH] tools: fix paths in PKI validation error messages
by Daniel P. Berrangé 13 Jun '24

13 Jun '24

A couple of paths passed in the error messages, didnt match the paths that were actually being tested. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- tools/virt-pki-validate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virt-pki-validate.c b/tools/virt-pki-validate.c index 656f29fdc5..e693ffaed6 100644 --- a/tools/virt-pki-validate.c +++ b/tools/virt-pki-validate.c @@ -184,7 +184,7 @@ virPKIValidateIdentity(bool isServer, bool system, const char *path) _("Checking system cert dir access"), 0, 0, 0755, _("The system cert dir %1$s must be accessible to all users. As root, run: chown root.root; chmod 0755 %2$s"), - LIBVIRT_PKI_DIR, LIBVIRT_PKI_DIR); + LIBVIRT_CERT_DIR, LIBVIRT_CERT_DIR); FILE_REQUIRE_EXISTS(scope, LIBVIRT_KEY_DIR, @@ -197,7 +197,7 @@ virPKIValidateIdentity(bool isServer, bool system, const char *path) _("Checking system key dir access"), 0, 0, 0755, _("The system key dir %1$s must be accessible to all users. As root, run: chown root.root; chmod 0755 %2$s"), - LIBVIRT_KEY_DIR, LIBVIRT_PKI_DIR); + LIBVIRT_KEY_DIR, LIBVIRT_KEY_DIR); } else if (path) { virNetTLSConfigCustomTrust(path, &cacert, -- 2.45.1

2 1

[libvirt PATCH 00/28] native support for nftables in virtual network driver
by Laine Stump 12 Jun '24

12 Jun '24

This patch series enables libvirt to use nftables rules rather than iptables *when setting up virtual networks* (it does *not* add nftables support to the nwfilter driver). It accomplishes this by abstracting several iptables functions (from viriptables.[ch] called by the virtual network driver into a rudimentary "virNetfilter API" (in virnetfilter.[ch], having the virtual network driver call the virNetFilter API rather than calling the existing iptables functions directly, and then finally adding an equivalent virNftables backend that can be used instead of iptables (selected manually via a network.conf setting, or automatically if iptables isn't found on the host). A first look at the result may have you thinking that it's filled with a lot of bad decisions. While I would agree with that in many cases, I think that overall they are the "least bad" decisions, or at least "bad within acceptable limits / no worse than something else", and point out that it's been done in a way that minimizes (actually eliminates) the need for immediate changes to nwfilter (the other consumer of iptables, which *also* needs to be updated to use native nftables), and makes it much easier to change our mind about the details in the future. When I first started on this (long, protracted, repeatedly interrupted for extended periods - many of these patches are > a year old) task, I considered doing an all-at-once complete replacement of iptables with nftables, since all the Linux distros we support have had nftables for several years, and I'm pretty sure nobody has it disabled (not even sure if it's possible to disable nftables while still enabling iptables, since they both use xtables in the kernel). But due to libvirt's use of "-t mangle -j CHECKSUM --checksum-fill" (see commit fd5b15ff all the way back in July 2010 for details) which has no equivalent in nftables rules (and we don't *want* it to!!), and the desire to be able to easily switch back to iptables in case of an unforeseen regression, we decided that both iptables and nftables need to be supported (for now), with the default (for now) remaining as iptables. Just allowing for dual backends complicated matters, since it means that we have to have a config file, a setting, detection of which backends are available, and of course some sort of concept of an abstracted frontend that can use either backend based on the config setting (and/or auto-detection). Combining that with the fact that it would just be "too big" of a project to switch over nwfilter's iptables usage at the same time means that we have to keep around a lot of existing code for compatibility's sake rather than just wiping it all away and starting over. So, what I've ended up with is: 1) a network.conf file (didn't exist before) with a single setting "firewall_backend". If unset, the network driver tries to use iptables on the backend, and if that's missing, then tries to use nftables. 2) a new (internal-only, so transient!) virNetFilterXXX API that is used by the network driver in place of the iptablesXXX API, and calls either iptablesXXX or: 3) a virNftablesXXX API that exactly replicates the filtering rules of the existing iptablesXXX API (except in the custom "libvirt" base table rather than the system "filter" and "nat" tables). This means that: 4) when the nftables backend is used, the rules added are *exactly the same* (functionally speaking) as we currently add for iptables (except they are in the "libvirt" table). We had spent some time in IRC discussing different ways of using new functionality available in nftables to make a more efficient/performant implemention of the desired filtering, and there are some really great possibilities that need to be explored, but in the end there were too many details up in the air, and I decided that it would be more "accomplishable" (coined a new word there!) to first replicate existing behavior with nftables, but do it inside a framework that makes it easy to modify the details in the future (in particular making it painless to switch back and forth between builds with differing filter models at runtime) - this way we'll be able to separate the infrastructure work from the details of the rules (which we can then more easily work on and experiment with). (This implies that the main objective right now is "get rid of iptables dependencies", not "make the filtering faster and more efficient"). Notable features of this patchset: * allows switching between iptables/nftables backends without rebooting or restarting networks/guests. Because the commands required to remove a network's filter rules are now saved in the network status XML, each time libvirtd (or virtnetworkd) is restarted, it will execute exactly the commands needed to remove the filter rules that had been added by the previous libvirtd/virtnetworkd (rather than just making a guess, as we've always done up until now), and then add new rules using the current backend+binary's set of rules (while also saving the info needed for future removal of these new rules back into the network's status XML). * firewall_backend can be explicitly set in (new) /etc/libvirt/network.conf, but if it's not explicitly set, libvirt will default to the iptables backend if the iptables binary is found, and otherwise fall back to nftables as long as the nft binary is found; otherwise the first attempt to start a network will fail with an appropriate error. Things that seem ugly / that I would like to clean up / that I think are just fine as they are: * virFirewall does *not* provide a backend-agnostic interface [this is fine] * We need to maintain a backward-compatible API for virFirewall so that we don't have to touch nwfilter code. Trying to make its API backend-agnostic would require individually considering/changing every nwfilter use of virFirewall. * instead virFirewall objects are just a way to build a collection of commands to execute to build a firewall, then execute them while collecting info for and building a collection of commands that will tear down that firewall in the future. Do I want to "fix" this in the future by making virFirewall a higher level interface that accepts tokens describing the type of rule to add (rather than backend-specific arguments to a backend-specific command)? No. I think I like the way virFirewall works (as described in that previous bullet-point), instead I'm thinking that it is just slightly mis-named - I've lately been thinking of it as a "virNetFilterCmdList". Similarly, the virFirewallRules that it has a list of aren't really "rules", they are better described as commands or actions, so maybe they should be renamed to virNetfilterCmd or virNetfilterAction. But that is just cosmetic, so I didn't want to get into it in these patches (especially in case someone disagrees, or has a better idea for naming). * Speaking of renaming - I should probably rename all the "iptablesXXX" functions to "virIptablesXXX" to be consistent with so much of our other code. I lost the ambition to deal with it right now though, so I'm leaving that for later cleanup (or I could do it now if it really makes someone's day :-). * I could have chosen a higher place in the callchain to make the virNetfilter abstraction, e.g. at the level of "networkAddXXXFirewallRules()" rather than at the lower level of iptablesXXX(). That is actually probably what will happen in the future (since it will be necessary in order for an nftables-based firewall to be significantly different in structure from an iptables-based firewall). But that's the beauty of an API being private - we can freely add/remove things as needed. the important thing is that we now have the basic structure there. For now, the split is just above the existing iptablesXXX API (util/viriptables.[ch], which seems like a "narrow" enough place. Most iptablesXXX functions are written in terms of just 10 *other* iptablesXXX functions that add iptables-specific commands - I've just moved those functions into virnetfilter.[ch] (appropriately renamed), and changed them to call the 10 virNetfilterXXX functions that will in-turn call those 10 iptablesXXX (or equivalent virNftablesXXX) functions. * Some people may dislike that the 10 virNetfilterXXX functions are each written with a switch statement that has cases to directly call each backend, rather than each backend driver having a table of pointers to API functions, with the virNetfilter API function calling backends[fwBackend]->XXX() (ie the pattern for so many drivers in libvirt). But for just 2 backends, that really seemed like overkill and unnecessary obfuscation. * As implemented here, I am storing a "<fwRemoval>" element in the network status XML - it contains a serialized virFirewall object that directly contains the commands necessary to remove the firewall. I could instead just store "<firewall>", which would include all the commands that were used to *create* the firewall in addition to the commands needed to remove the firewall. The way it's done currently takes up less space; switching to storing the full firewall *might* be more informative to somebody, but on the other hand would make the network status XML *very* long. If anybody has an opinion about this, now is the time to bring it up - do you think it's worth having a separate list of all the commands that were used to create a network's firewall (keeping in mind that there is no public API to access it)? Or is it enough to just store what's needed to remove the firewall? * Several months ago Eric Garver posted patches for a pure firewalld backend, and I requested that they not be pushed because I wanted that to be integrated with my nftables backend support. Due to the fact that the firewalld backend is almost entirely implemented by putting the bridge into a new firewalld "zone", with no individual rules added, that won't happen as just another backend driver file in parallel to iptables and nftables; it will instead work by checking firewall_backend at a higher level in the network driver, thus avoiding the calls to virNetfilterXXX() entirely. I have locally merged Eric's patches over the top of these patches, and there are surprisingly few conflicts, but since his patches didn't account for a user-settable config (but instead just always used the firewalld backend if firewalld was active), some of the patches are going to require a bit of rework, which I'll take care of after getting these patches in. Laine Stump (28): util: add -w/--concurrent when applying the rule rather than when building it util: new virFirewallRuleGet*() APIs util: determine ignoreErrors value when creating rule, not when applying util: rename iptables helpers that will become the frontend for ip&nftables util: move backend-agnostic virNetfilter*() functions to their own file util: make netfilter action a proper typedefed (virFirewall) enum util: #define the names used for private packet filter chains util: move/rename virFirewallApplyRuleDirect to virIptablesApplyFirewallRule util/network: reintroduce virFirewallBackend, but different network: add (empty) network.conf file to distribution files network: allow setting firewallBackend from network.conf network: do not add DHCP checksum mangle rule unless using iptables network: call backend agnostic function to init private filter chains util: setup functions in virnetfilter which will call appropriate backend build: add nft to the list of binaries we attempt to locate util: add nftables backend to virnetfilter API used by network driver tests: test cases for nftables backend util: new functions to support adding individual rollback rules util: check for 0 args when applying iptables rule util: implement rollback rule autosave for iptables backend util: implement rollback rule autosave for nftables backend network: turn on auto-rollback for the rules added for virtual networks util: new function virFirewallNewFromRollback() util: new functions virFirewallParseXML() and virFirewallFormat() conf: add a virFirewall object to virNetworkObj network: use previously saved list of firewall rules when removing network: save network status when firewall rules are reloaded network: improve log message when reloading virtual network firewall rules libvirt.spec.in | 5 + meson.build | 1 + po/POTFILES | 2 + src/conf/virnetworkobj.c | 40 + src/conf/virnetworkobj.h | 11 + src/libvirt_private.syms | 68 +- src/network/bridge_driver.c | 40 +- src/network/bridge_driver_conf.c | 44 + src/network/bridge_driver_conf.h | 3 + src/network/bridge_driver_linux.c | 241 +++-- src/network/bridge_driver_nop.c | 6 +- src/network/bridge_driver_platform.h | 6 +- src/network/libvirtd_network.aug | 39 + src/network/meson.build | 11 + src/network/network.conf | 24 + src/network/test_libvirtd_network.aug.in | 5 + src/nwfilter/nwfilter_ebiptables_driver.c | 16 +- src/util/meson.build | 2 + src/util/virebtables.c | 4 +- src/util/virfirewall.c | 490 ++++++++-- src/util/virfirewall.h | 51 +- src/util/viriptables.c | 762 ++++----------- src/util/viriptables.h | 222 ++--- src/util/virnetfilter.c | 892 ++++++++++++++++++ src/util/virnetfilter.h | 159 ++++ src/util/virnftables.c | 698 ++++++++++++++ src/util/virnftables.h | 118 +++ .../{base.args => base.iptables} | 0 tests/networkxml2firewalldata/base.nftables | 256 +++++ ...-linux.args => nat-default-linux.iptables} | 0 .../nat-default-linux.nftables | 248 +++++ ...pv6-linux.args => nat-ipv6-linux.iptables} | 0 .../nat-ipv6-linux.nftables | 384 ++++++++ ...rgs => nat-ipv6-masquerade-linux.iptables} | 0 .../nat-ipv6-masquerade-linux.nftables | 456 +++++++++ ...linux.args => nat-many-ips-linux.iptables} | 0 .../nat-many-ips-linux.nftables | 472 +++++++++ ...-linux.args => nat-no-dhcp-linux.iptables} | 0 .../nat-no-dhcp-linux.nftables | 384 ++++++++ ...ftp-linux.args => nat-tftp-linux.iptables} | 0 .../nat-tftp-linux.nftables | 274 ++++++ ...inux.args => route-default-linux.iptables} | 0 .../route-default-linux.nftables | 162 ++++ tests/networkxml2firewalltest.c | 56 +- tests/virfirewalltest.c | 20 +- 45 files changed, 5718 insertions(+), 954 deletions(-) create mode 100644 src/network/libvirtd_network.aug create mode 100644 src/network/network.conf create mode 100644 src/network/test_libvirtd_network.aug.in create mode 100644 src/util/virnetfilter.c create mode 100644 src/util/virnetfilter.h create mode 100644 src/util/virnftables.c create mode 100644 src/util/virnftables.h rename tests/networkxml2firewalldata/{base.args => base.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/base.nftables rename tests/networkxml2firewalldata/{nat-default-linux.args => nat-default-linux.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/nat-default-linux.nftables rename tests/networkxml2firewalldata/{nat-ipv6-linux.args => nat-ipv6-linux.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/nat-ipv6-linux.nftables rename tests/networkxml2firewalldata/{nat-ipv6-masquerade-linux.args => nat-ipv6-masquerade-linux.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables rename tests/networkxml2firewalldata/{nat-many-ips-linux.args => nat-many-ips-linux.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/nat-many-ips-linux.nftables rename tests/networkxml2firewalldata/{nat-no-dhcp-linux.args => nat-no-dhcp-linux.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/nat-no-dhcp-linux.nftables rename tests/networkxml2firewalldata/{nat-tftp-linux.args => nat-tftp-linux.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/nat-tftp-linux.nftables rename tests/networkxml2firewalldata/{route-default-linux.args => route-default-linux.iptables} (100%) create mode 100644 tests/networkxml2firewalldata/route-default-linux.nftables -- 2.39.2

7 78

New libvirt-rust-xml project
by Daniel P. Berrangé 12 Jun '24

12 Jun '24

There has been interest in creating an equivalent to the libvirt-go-xml-module project, but for Rust. Thus I've created the following new project: https://gitlab.com/libvirt/libvirt-rust-xml Don't expect to see any actual code there yet. It is just the absolute bare minimum Rust boilerplate setup and CI integration to allow us to kick off the concept. Anyone interested in this should open issues/merge requests there in the usual way. 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 :|

1 0

[PATCH RFC v2 00/12] Support throttle block filters
by wucf＠linux.ibm.com 12 Jun '24

12 Jun '24

From: Chun Feng Wu <wucf(a)linux.ibm.com> Hi, I am thinking to leverage "throttle block filter" in QEMU to support more flexible I/O limits(e.g. tiered I/O groups), one sample provided by QEMU doc is: https://github.com/qemu/qemu/blob/master/docs/throttle.txt "For example, let's say that we have three different drives and we want to set I/O limits for each one of them and an additional set of limits for the combined I/O of all three drives." The implementation idea is to - Define throttle groups(limit) in domain - Define throttle filter to reference throttle group within disk - Within domain disk, throttle filters references multiple throttle groups to form filter chain to apply multiple limits in QEMU like above sample - Add new virsh cmds for throttle group management: throttlegroupset Add or update a throttling group. throttlegroupdel Delete a throttling group. throttlegroupinfo Get a throttling group. throttlegrouplist list all domain throttlegroups - Update "attach-disk" to add one more option "--throttle-groups" to apply throttle filters e.g. "virsh attach-disk $VM_ID ${DISK_PATH}/vm1_disk_2.qcow2 vdd --driver qemu --subdriver qcow2 --targetbus virtio --throttle-groups limit2,limit012" - I chose above semantics as I felt they're appropriate, if there are better ones please kindly suggest. This patchset includes: - Throttle group and throttle filter definition in patch 1 - New QMP processing to update and get throttle group in patch 2 - New API definition and implementation in patch 3 - QEMU driver implementation in patch 4 - Hotplug and qemuProcessLaunch flow implemenation in patch 5,6 - Domain XML schema and doc(formatdomain.rst) change in patch 7 - Tests in patch 8,9 - Virsh cmd implementation in patch 10 - Other verification implementation in patch 11, 12 v2 changes: - fix failure caused by "check-remote_protocol" between patch 3 and patch 9 in v1 - make sure coding style is consistent about function declaration - patch 3 in v1 is splitted into two commits: "remote: New APIs for ThrottleGroup lifecycle management" and "qemu: Implement qemu driver for throttle API" - replace "// comment" with "/* comment */" - avoid empty 'cleanup' labels - use "virJSONValueObjectAdd(&limits, "P:name", value)" to avoid introducing extra helper, and check return value - preserve spacing between blocks of code which are not related - remove "VSH_OT_ALIAS" section for new cmds in virsh_domain.c From QMP perspective, the sample flow works this way: - Throttle group creation: virsh qemu-monitor-command 1 '{"execute":"object-add", "arguments":{"qom-type":"throttle-group","id":"limit0","limits":{"iops-total":200,"iops-read":0,"iops-total-max":200,"iops-total-max-length":1}}}' virsh qemu-monitor-command 1 '{"execute":"object-add", "arguments":{"qom-type":"throttle-group","id":"limit1","limits":{"iops-total":250,"iops-read":0,"iops-total-max":250,"iops-total-max-length":1}}}' virsh qemu-monitor-command 1 '{"execute":"object-add", "arguments":{"qom-type":"throttle-group","id":"limit2","limits":{"iops-total":300,"iops-read":0,"iops-total-max":300,"iops-total-max-length":1}}}' virsh qemu-monitor-command 1 '{"execute":"object-add", "arguments":{"qom-type":"throttle-group","id":"limit012","limits":{"iops-total":400,"iops-read":0,"iops-total-max":400,"iops-total-max-length":1}}}' - Chain up filters during attaching disk to apply two filters(limit0 and limit012): virsh qemu-monitor-command 1 '{"execute":"blockdev-add", "arguments": {"driver":"file","filename":"/virt/disks/vm1_disk_1.qcow2","node-name":"test-3-storage","auto-read-only":true,"discard":"unmap"}}' virsh qemu-monitor-command 1 '{"execute":"blockdev-add", "arguments":{"node-name":"test-3-format","read-only":false,"driver":"qcow2","file":"test-3-storage","backing":null}}' virsh qemu-monitor-command 1 '{"execute":"blockdev-add", "arguments":{"driver":"throttle","node-name":"libvirt-5-filter","throttle-group": "limit0","file":"test-3-format"}}' virsh qemu-monitor-command 1 '{"execute":"blockdev-add", "arguments": {"driver":"throttle","node-name":"libvirt-6-filter","throttle-group":"limit012","file":"libvirt-5-filter"}}' virsh qemu-monitor-command 1 '{"execute": "device_add", "arguments": {"driver":"virtio-blk-pci","scsi":false,"bus":"pci.0","addr":"0x5","drive":"libvirt-6-filter","id":"virtio-disk1"}}' Any comments/suggestions will be appriciated! Chun Feng Wu (10): config: Introduce ThrottleGroup and ThrottleFilter qemu: monitor: Add support for ThrottleGroup operations remote: New APIs for ThrottleGroup lifecycle management qemu: Implement qemu driver for throttle API qemu: hotplug: Support hot attach block disk along with throttle filters qemu: command: Support throttle groups and filters during qemuProcessLaunch schema: Add new domain elements to support multiple throttle filters test: Test throttle group lifecycle APIs tests: Test qemuMonitorJSONGetThrottleGroup and qemuMonitorJSONUpdateThrottleGroup virsh: Add support for throttle group operations Hao Ning Xin (1): config: validate: Verify throttle group fields Yan Xiu Wu (1): config: validate: Use "iotune" and "throttlefilters" exclusively for specific disk docs/formatdomain.rst | 48 ++ include/libvirt/libvirt-domain.h | 29 + src/conf/domain_conf.c | 386 +++++++++++++ src/conf/domain_conf.h | 54 ++ src/conf/domain_validate.c | 120 ++-- src/conf/schemas/domaincommon.rng | 164 +++++- src/conf/virconftypes.h | 4 + src/driver-hypervisor.h | 22 + src/libvirt-domain.c | 190 +++++++ src/libvirt_private.syms | 9 + src/libvirt_public.syms | 7 + src/qemu/qemu_block.c | 131 +++++ src/qemu/qemu_block.h | 53 ++ src/qemu/qemu_command.c | 158 ++++++ src/qemu/qemu_command.h | 9 + src/qemu/qemu_domain.c | 85 +++ src/qemu/qemu_domain.h | 15 + src/qemu/qemu_driver.c | 529 ++++++++++++++++++ src/qemu/qemu_hotplug.c | 24 + src/qemu/qemu_monitor.c | 34 ++ src/qemu/qemu_monitor.h | 14 + src/qemu/qemu_monitor_json.c | 151 +++++ src/qemu/qemu_monitor_json.h | 14 + src/remote/remote_daemon_dispatch.c | 61 ++ src/remote/remote_driver.c | 49 ++ src/remote/remote_protocol.x | 50 +- src/remote_protocol-structs | 30 + src/test/test_driver.c | 382 +++++++++++++ tests/qemumonitorjsontest.c | 88 +++ .../qemustatusxml2xmldata/backup-pull-in.xml | 1 + .../blockjob-blockdev-in.xml | 1 + .../blockjob-mirror-in.xml | 1 + .../migration-in-params-in.xml | 1 + .../migration-out-nbd-bitmaps-in.xml | 1 + .../migration-out-nbd-out.xml | 1 + .../migration-out-nbd-tls-out.xml | 1 + .../migration-out-params-in.xml | 1 + tests/qemustatusxml2xmldata/modern-in.xml | 1 + tests/qemustatusxml2xmldata/upgrade-out.xml | 1 + .../qemustatusxml2xmldata/vcpus-multi-in.xml | 1 + tools/virsh-completer-domain.c | 64 +++ tools/virsh-completer-domain.h | 10 + tools/virsh-domain.c | 471 ++++++++++++++++ 43 files changed, 3430 insertions(+), 36 deletions(-) -- 2.34.1

3 39

[PATCH] news: document new virt-host-validate impl
by Daniel P. Berrangé 12 Jun '24

12 Jun '24

Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- NEWS.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index f3ca29443a..bc8acc62d4 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -19,6 +19,13 @@ v10.5.0 (unreleased) * **Improvements** + * tools: virt-pki-validate has been rewritten in C + + The ``virt-pki-validate`` shell script has been rewritten as a C program, + providing an output format that matches ``virt-host-validate``, removing + the dependency on ``certtool`` and providing more comprehensive checks + of the certificate properties. + * **Bug fixes** -- 2.45.1

2 1

[PATCH 0/3] qemu: Fix missing validation of 'scsi' property support for 'virtio-blk' and update capabilites dump for qemu-9.1
by Peter Krempa 11 Jun '24

11 Jun '24

Peter Krempa (3): qemuValidateDomainDeviceDefDiskFrontend: Refactor validation of <disk type='lun'> qemu_validate: Validate support for SCSI emulation support in 'virtio-blk' devices qemucapabilitiestest: Update test data for qemu 9.1 dev cycle src/qemu/qemu_validate.c | 38 +- .../caps_9.1.0_x86_64.replies | 3210 +++++++++-------- .../caps_9.1.0_x86_64.xml | 16 +- ...test.args => virtio-lun.x86_64-9.0.0.args} | 2 +- ...latest.xml => virtio-lun.x86_64-9.0.0.xml} | 2 +- .../virtio-lun.x86_64-latest.err | 1 + tests/qemuxmlconftest.c | 3 +- 7 files changed, 1716 insertions(+), 1556 deletions(-) rename tests/qemuxmlconfdata/{virtio-lun.x86_64-latest.args => virtio-lun.x86_64-9.0.0.args} (96%) rename tests/qemuxmlconfdata/{virtio-lun.x86_64-latest.xml => virtio-lun.x86_64-9.0.0.xml} (97%) create mode 100644 tests/qemuxmlconfdata/virtio-lun.x86_64-latest.err -- 2.45.2

2 5