[PATCH v4 24/30] network: add an nftables backend for network driver's firewall construction
Support using nftables to setup the firewall for each virtual network, rather than iptables. The initial implementation of the nftables backend creates (almost) exactly the same ruleset as the iptables backend, determined by running the following commands on a host that has an active virtual network: iptables-save >iptables.txt iptables-restore-translate -f iptables.txt (and the similar ip6tables-save/ip6tables-restore-translate for an IPv6 network). Correctness of the new backend was checked by comparing the output of: nft list ruleset when the backend is set to iptables and when it is set to nftables. This page was used as a guide: https://wiki.nftables.org/wiki-nftables/index.php/Moving_from_iptables_to_nftables The only differences between the rules created by the nftables backed vs. the iptables backend (aside from a few inconsequential changes in display order of some chains/options) are: 1) When we add nftables rules, rather than adding them in the system-created "filter" and "nat" tables, we add them in a private table (ie only we should be using it) created by us called "libvirt" (the system-created "filter" and "nat" tables can't be used because adding any rules to those tables directly with nft will cause failure of any legacy application attempting to use iptables when it tries to list the iptables rules (e.g. "iptables -S"). (NB: in nftables only a single table is required for both nat and filter rules - the chains for each are differentiated by specifying different "hook" locations for the toplevel chain of each) 2) nftables doesn't support the "checksum mangle" rule (or any equivalent functionality) that we have historically added to our iptables rules, so the nftables rules we add have nothing related to checksum mangling. (NB: The result of (2) is that if you a) have a very old guest (RHEL5 era or earlier) and b) that guest is using a virtio-net network device, and c) the virtio-net device is using vhost packet processing (the default) then DHCP on the guest will fail. You can work around this by adding to the XML for the guest). There are certainly much better nftables rulesets that could be used instead of those implemented here, and everything is in place to make future changes to the rules that are used simple and free of surprises (e.g. the rules that are added have coresponding "removal" commands added to the network status so that we will always remove exactly the rules that were previously added rather than trying to remove the rules that "this build of libvirt would have added" (which will be incorrect the first time we run a libvirt with a newly modified ruleset). For this initial implementation though, I wanted the nftables rules to be as identical to the iptables rules as possible, just to make it easier to verify that everything is working. The backend can be manually chosen using the firewall_backend setting in /etc/libvirt/network.conf. libvirtd/virtnetworkd will read this setting when it starts; if there is no explicit setting, it will look for iptables commands on the host and, if they are found, will select the iptables backend (this is the default for the sake of 100% backward compatibility), but if iptables commands aren't found, then it will use the nftables backend. (Since libvirt will automatically remove all its previous filter rules and re-add rules using the current backend setting for all active networks when it starts up, and the only noticeable change in behavior between the iptables and nftables backends is that noted in item (2) above, we could instead decide to make nftables the default backend rather than iptables - it all depends on how important it is to work properly on 15 year old guest OSes using DHCP with virtio-net interfaces). Signed-off-by: Laine Stump --- meson.build | 2 + po/POTFILES | 1 + src/network/bridge_driver_conf.c | 12 + src/network/bridge_driver_linux.c | 17 +- src/network/meson.build | 1 + src/network/network.conf.in | 21 +- src/network/network_nftables.c| 940 ++ src/network/network_nftables.h| 28 + src/util/virfirewall.c| 167 +- src/util/virfirewall.h| 2 + 10 files changed, 1186 insertions(+), 5 deletions(-) create mode 100644 src/network/network_nftables.c create mode 100644 src/network/network_nftables.h diff --git a/meson.build b/meson.build index b9379d6ce7..4e125048b3 100644 --- a/meson.build +++ b/meson.build @@ -1619,6 +1619,8 @@ if not get_option('driver_network').disabled() and conf.has('WITH_LIBVIRTD') conf.set_quoted('FIREWALL_BACKEND', firewall_backend) if firewall_backend == 'iptables' conf.set('FIREWALL_BACKEND_DEFAULT_IPTABLES', 1) + elif firewall_backend == 'nftables' +conf.set('FIREWALL_BACKEND_DEFAULT_NFTABLES', 1) endif elif get_option('driver_network').enabled() error('libvirtd must be enabled to build the network
[PATCH v4 25/30] tests: test cases for nftables backend
Run all the networkxml2firewall tests twice - once with iptables backend, and once with the nftables backend. The results files for the existing iptables tests were previously named *.args. That has been changed to *.iptables, and the results files for the new nftables tests are named *.nftables. Signed-off-by: Laine Stump --- .../{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 ++- 17 files changed, 2678 insertions(+), 14 deletions(-) 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 diff --git a/tests/networkxml2firewalldata/base.args b/tests/networkxml2firewalldata/base.iptables similarity index 100% rename from tests/networkxml2firewalldata/base.args rename to tests/networkxml2firewalldata/base.iptables diff --git a/tests/networkxml2firewalldata/base.nftables b/tests/networkxml2firewalldata/base.nftables new file mode 100644 index 00..4f1f475a85 --- /dev/null +++ b/tests/networkxml2firewalldata/base.nftables @@ -0,0 +1,256 @@ +nft \ +list \ +table \ +ip \ +libvirt +nft \ +add \ +table \ +ip \ +libvirt +nft \ +add \ +chain \ +ip \ +libvirt \ +INPUT \ +'{ type filter hook input priority 0; policy accept; }' +nft \ +add \ +chain \ +ip \ +libvirt \ +FORWARD \ +'{ type filter hook forward priority 0; policy accept; }' +nft \ +add \ +chain \ +ip \ +libvirt \ +OUTPUT \ +'{ type filter hook output priority 0; policy accept; }' +nft \ +add \ +chain \ +ip \ +libvirt \ +LIBVIRT_INP +nft \ +insert \ +rule \ +ip \ +libvirt \ +INPUT \ +counter \ +jump \ +LIBVIRT_INP +nft \ +add \ +chain \ +ip \ +libvirt \ +LIBVIRT_OUT +nft \ +insert \ +rule \ +ip \ +libvirt \ +OUTPUT \ +counter \ +jump \ +LIBVIRT_OUT +nft \ +add \ +chain \ +ip \ +libvirt \ +LIBVIRT_FWO +nft \ +insert \ +rule \ +ip \ +libvirt \ +FORWARD \ +counter \ +jump \ +LIBVIRT_FWO +nft \ +add \ +chain \ +ip \ +libvirt \ +LIBVIRT_FWI +nft \ +insert \ +rule \ +ip \ +libvirt \ +FORWARD \ +counter \ +jump \ +LIBVIRT_FWI +nft \ +add \ +chain \ +ip \ +libvirt \ +LIBVIRT_FWX +nft \ +insert \ +rule \ +ip \ +libvirt \ +FORWARD \ +counter \ +jump \ +LIBVIRT_FWX +nft \ +add \ +chain \ +ip \ +libvirt \ +POSTROUTING \ +'{ type nat hook postrouting priority 100; policy accept; }' +nft \ +add \ +chain \ +ip \ +libvirt \ +LIBVIRT_PRT +nft \ +insert \ +rule \ +ip \ +libvirt \ +POSTROUTING \ +counter \ +jump \ +LIBVIRT_PRT +nft \ +list \ +table \ +ip6 \ +libvirt +nft \ +add \ +table \ +ip6 \ +libvirt +nft \ +add \ +chain \ +ip6 \ +libvirt \ +INPUT \ +'{ type filter hook input priority 0; policy accept; }' +nft \ +add \ +chain \ +ip6 \ +libvirt \ +FORWARD \ +'{ type filter hook forward priority 0; policy accept; }' +nft \ +add \ +chain \ +ip6 \ +libvirt \ +OUTPUT \ +'{ type filter hook output priority 0;
[PATCH v4 28/30] network: name the nftables table "libvirt_network" rather than "libvirt"
This way when we implement nftables for the nwfilter driver, we can create a separate table called "libvirt_nwfilter" and everything will look all symmetrical and stuff. Signed-off-by: Laine Stump --- src/network/network_nftables.c| 2 +- .../nat-default-linux.nftables| 36 +- .../nat-ipv6-linux.nftables | 58 .../nat-ipv6-masquerade-linux.nftables| 66 +-- .../nat-many-ips-linux.nftables | 64 +- .../nat-no-dhcp-linux.nftables| 58 .../nat-tftp-linux.nftables | 40 +-- .../route-default-linux.nftables | 26 8 files changed, 175 insertions(+), 175 deletions(-) diff --git a/src/network/network_nftables.c b/src/network/network_nftables.c index c8cee98df5..ec9194a8b8 100644 --- a/src/network/network_nftables.c +++ b/src/network/network_nftables.c @@ -53,7 +53,7 @@ VIR_LOG_INIT("network.nftables"); * the iptables-compat */ -#define VIR_NFTABLES_PRIVATE_TABLE "libvirt" +#define VIR_NFTABLES_PRIVATE_TABLE "libvirt_network" /* nftables backend uses the same binary (nft) for all layers, but * IPv4 and IPv6 have their rules in separate classes of tables, diff --git a/tests/networkxml2firewalldata/nat-default-linux.nftables b/tests/networkxml2firewalldata/nat-default-linux.nftables index 2dafe078a1..92b3dd7fc0 100644 --- a/tests/networkxml2firewalldata/nat-default-linux.nftables +++ b/tests/networkxml2firewalldata/nat-default-linux.nftables @@ -2,7 +2,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_INP \ iifname \ virbr0 \ @@ -15,7 +15,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_INP \ iifname \ virbr0 \ @@ -28,7 +28,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_OUT \ oifname \ virbr0 \ @@ -41,7 +41,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_OUT \ oifname \ virbr0 \ @@ -54,7 +54,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_INP \ iifname \ virbr0 \ @@ -67,7 +67,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_INP \ iifname \ virbr0 \ @@ -80,7 +80,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_OUT \ oifname \ virbr0 \ @@ -93,7 +93,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_OUT \ oifname \ virbr0 \ @@ -106,7 +106,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_FWO \ iifname \ virbr0 \ @@ -116,7 +116,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_FWI \ oifname \ virbr0 \ @@ -126,7 +126,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_FWX \ iifname \ virbr0 \ @@ -138,7 +138,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_FWO \ ip \ saddr \ @@ -151,7 +151,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_FWI \ oifname \ virbr0 \ @@ -167,7 +167,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_PRT \ ip \ saddr \ @@ -182,7 +182,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_PRT \ meta \ l4proto \ @@ -202,7 +202,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_PRT \ meta \ l4proto \ @@ -222,7 +222,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_PRT \ ip \ saddr \ @@ -236,7 +236,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_PRT \ ip \ saddr \ diff --git a/tests/networkxml2firewalldata/nat-ipv6-linux.nftables b/tests/networkxml2firewalldata/nat-ipv6-linux.nftables index 63d4d8e2a5..f8317415cf 100644 --- a/tests/networkxml2firewalldata/nat-ipv6-linux.nftables +++ b/tests/networkxml2firewalldata/nat-ipv6-linux.nftables @@ -2,7 +2,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_INP \ iifname \ virbr0 \ @@ -15,7 +15,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_INP \ iifname \ virbr0 \ @@ -28,7 +28,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_OUT \ oifname \ virbr0 \ @@ -41,7 +41,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_OUT \ oifname \ virbr0 \ @@ -54,7 +54,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_INP \ iifname \ virbr0 \ @@ -67,7 +67,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_INP \ iifname \ virbr0 \ @@ -80,7 +80,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_OUT \ oifname \ virbr0 \ @@ -93,7 +93,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network \ LIBVIRT_OUT \ oifname \ virbr0 \ @@ -106,7 +106,7 @@ nft \ -ae insert \ rule \ ip \ -libvirt \ +libvirt_network
[PATCH v4 30/30] network: eliminate pointless host input/output rules from nftables backend
The iptables backend (which was used as the model for the nftables backend) used the same "filter" and "nat" tables used by other services on the system (e.g. firewalld or any other host firewall management application), so it was possible that one of those other services would be blocking DNS, DHCP, or TFTP from guests to the host; we added our own rules at the beginning of the chain to allow this traffic no matter if someone else rejected it later. But with nftables, each service uses their own table, and all traffic must be acepted by all tables no matter what - it's not possible for us to just insert a higher priority/earlier rule that will override some reject rule put in by, e.g., firewalld. Instead the firewalld (or other) table must be setup by that service to allow the traffic. That, along with the fact that our table is already "accept by default", makes it possible to eliminate the individual accept rules for DHCP, DNS, and TFTP. And once those rules are eliminated, there is no longer any need for the guest_to_host or host_to_guest tables. Signed-off-by: Laine Stump --- I've just #ifdef'ed out the code that adds these rules so that it remains there as an example if someone wants to add in some different guest<->host rules in the future. I could instead completely remove all the now-uncompiled code, and just leave a comment referencing the upstream commit ID of the last commit that still contained all of that code. I'm fine either way. src/network/network_nftables.c| 36 +++- .../nat-default-linux.nftables| 104 -- .../nat-ipv6-linux.nftables | 182 -- .../nat-ipv6-masquerade-linux.nftables| 182 -- .../nat-many-ips-linux.nftables | 104 -- .../nat-no-dhcp-linux.nftables| 182 -- .../nat-tftp-linux.nftables | 130 - .../route-default-linux.nftables | 104 -- 8 files changed, 33 insertions(+), 991 deletions(-) diff --git a/src/network/network_nftables.c b/src/network/network_nftables.c index fd0d0f82dc..5bdde822f1 100644 --- a/src/network/network_nftables.c +++ b/src/network/network_nftables.c @@ -40,8 +40,12 @@ VIR_LOG_INIT("network.nftables"); #define VIR_FROM_THIS VIR_FROM_NONE -#define VIR_NFTABLES_INPUT_CHAIN "guest_to_host" -#define VIR_NFTABLES_OUTPUT_CHAIN "host_to_guest" +#ifdef VIR_NFTABLES_INCLUDE_HOST_RULES +/* The input and output tables aren't currently used */ +# define VIR_NFTABLES_INPUT_CHAIN "guest_to_host" +# define VIR_NFTABLES_OUTPUT_CHAIN "host_to_guest" +#endif + #define VIR_NFTABLES_FORWARD_CHAIN "forward" #define VIR_NFTABLES_FWD_IN_CHAIN "guest_input" #define VIR_NFTABLES_FWD_OUT_CHAIN "guest_output" @@ -88,9 +92,14 @@ typedef struct { nftablesGlobalChain nftablesChains[] = { /* chains for filter rules */ + +#ifdef VIR_NFTABLES_INCLUDE_HOST_RULES +/* nothing is being added to these chains now, so they are effective NOPs */ {NULL, VIR_NFTABLES_INPUT_CHAIN, "{ type filter hook input priority 0; policy accept; }"}, -{NULL, VIR_NFTABLES_FORWARD_CHAIN, "{ type filter hook forward priority 0; policy accept; }"}, {NULL, VIR_NFTABLES_OUTPUT_CHAIN, "{ type filter hook output priority 0; policy accept; }"}, +#endif + +{NULL, VIR_NFTABLES_FORWARD_CHAIN, "{ type filter hook forward priority 0; policy accept; }"}, {"forward", VIR_NFTABLES_FWD_OUT_CHAIN, NULL}, {"forward", VIR_NFTABLES_FWD_IN_CHAIN, NULL}, {"forward", VIR_NFTABLES_FWD_X_CHAIN, NULL}, @@ -209,6 +218,11 @@ nftablesSetupPrivateChains(virFirewallLayer layer) } +#ifdef VIR_NFTABLES_INCLUDE_HOST_RULES +/* currently these functions aren't used, but they remain in the + * source (uncompiled) as examples of adding specific rules to permit + * input/output of packets. in case the need arises in the future + */ static void nftablesAddInput(virFirewall *fw, virFirewallLayer layer, @@ -315,6 +329,9 @@ nftablesAddUdpOutput(virFirewall *fw, } +#endif + + /** * nftablesAddForwardAllowOut: * @@ -801,6 +818,14 @@ nftablesAddGeneralIPv4FirewallRules(virFirewall *fw, break; } +#ifdef VIR_NFTABLES_INCLUDE_HOST_RULES +/* These rules copied from the iptables backend, have been removed + * from the nftab because they are redundant since we are using our own + * table that is default accept; there are no other users that + * could add a reject rule that we would need to / be able to + * override with these rules + */ + /* allow DHCP requests through to dnsmasq & back out */ nftablesAddTcpInput(fw, VIR_FIREWALL_LAYER_IPV4, def->bridge, 67); nftablesAddUdpInput(fw, VIR_FIREWALL_LAYER_IPV4, def->bridge, 67); @@ -818,6 +843,7 @@ nftablesAddGeneralIPv4FirewallRules(virFirewall *fw, nftablesAddUdpInput(fw, VIR_FIREWALL_LAYER_IPV4, def->bridge, 69); nftablesAddUdpOutput(fw,
[PATCH v4 29/30] network: rename chains used by network driver nftables backend
Because the chains added by the network driver nftables backend will go into a table used only by libvirt, we don't need to have "libvirt" in the chain names. Instead, we can make them more descriptive and less abrasive (by using lower case, and using full words rather than abbreviations). Also (again because nobody else is using the private "libvirt_network" table) we can directly put our rules into the input ("guest_to_host"), output ("host_to_guest"), and postrouting ("guest_nat") chains rather than creating a subordinate chain as done in the iptables backend. Signed-off-by: Laine Stump --- src/network/network_nftables.c| 30 - .../nat-default-linux.nftables| 36 +- .../nat-ipv6-linux.nftables | 58 .../nat-ipv6-masquerade-linux.nftables| 66 +-- .../nat-many-ips-linux.nftables | 64 +- .../nat-no-dhcp-linux.nftables| 58 .../nat-tftp-linux.nftables | 40 +-- .../route-default-linux.nftables | 26 8 files changed, 188 insertions(+), 190 deletions(-) diff --git a/src/network/network_nftables.c b/src/network/network_nftables.c index ec9194a8b8..fd0d0f82dc 100644 --- a/src/network/network_nftables.c +++ b/src/network/network_nftables.c @@ -40,12 +40,13 @@ VIR_LOG_INIT("network.nftables"); #define VIR_FROM_THIS VIR_FROM_NONE -#define VIR_NFTABLES_INPUT_CHAIN "LIBVIRT_INP" -#define VIR_NFTABLES_OUTPUT_CHAIN "LIBVIRT_OUT" -#define VIR_NFTABLES_FWD_IN_CHAIN "LIBVIRT_FWI" -#define VIR_NFTABLES_FWD_OUT_CHAIN "LIBVIRT_FWO" -#define VIR_NFTABLES_FWD_X_CHAIN "LIBVIRT_FWX" -#define VIR_NFTABLES_NAT_POSTROUTE_CHAIN "LIBVIRT_PRT" +#define VIR_NFTABLES_INPUT_CHAIN "guest_to_host" +#define VIR_NFTABLES_OUTPUT_CHAIN "host_to_guest" +#define VIR_NFTABLES_FORWARD_CHAIN "forward" +#define VIR_NFTABLES_FWD_IN_CHAIN "guest_input" +#define VIR_NFTABLES_FWD_OUT_CHAIN "guest_output" +#define VIR_NFTABLES_FWD_X_CHAIN "guest_cross" +#define VIR_NFTABLES_NAT_POSTROUTE_CHAIN "guest_nat" /* we must avoid using the standard "filter" table as used by * iptables, as any subsequent attempts to use iptables commands will @@ -87,18 +88,15 @@ typedef struct { nftablesGlobalChain nftablesChains[] = { /* chains for filter rules */ -{NULL, "INPUT", "{ type filter hook input priority 0; policy accept; }"}, -{NULL, "FORWARD", "{ type filter hook forward priority 0; policy accept; }"}, -{NULL, "OUTPUT", "{ type filter hook output priority 0; policy accept; }"}, -{"INPUT", VIR_NFTABLES_INPUT_CHAIN, NULL}, -{"OUTPUT", VIR_NFTABLES_OUTPUT_CHAIN, NULL}, -{"FORWARD", VIR_NFTABLES_FWD_OUT_CHAIN, NULL}, -{"FORWARD", VIR_NFTABLES_FWD_IN_CHAIN, NULL}, -{"FORWARD", VIR_NFTABLES_FWD_X_CHAIN, NULL}, +{NULL, VIR_NFTABLES_INPUT_CHAIN, "{ type filter hook input priority 0; policy accept; }"}, +{NULL, VIR_NFTABLES_FORWARD_CHAIN, "{ type filter hook forward priority 0; policy accept; }"}, +{NULL, VIR_NFTABLES_OUTPUT_CHAIN, "{ type filter hook output priority 0; policy accept; }"}, +{"forward", VIR_NFTABLES_FWD_OUT_CHAIN, NULL}, +{"forward", VIR_NFTABLES_FWD_IN_CHAIN, NULL}, +{"forward", VIR_NFTABLES_FWD_X_CHAIN, NULL}, /* chains for NAT rules */ -{NULL, "POSTROUTING", "{ type nat hook postrouting priority 100; policy accept; }"}, -{"POSTROUTING", VIR_NFTABLES_NAT_POSTROUTE_CHAIN, NULL}, +{NULL, "guest_nat", "{ type nat hook postrouting priority 100; policy accept; }"}, }; diff --git a/tests/networkxml2firewalldata/nat-default-linux.nftables b/tests/networkxml2firewalldata/nat-default-linux.nftables index 92b3dd7fc0..8b6e0ba406 100644 --- a/tests/networkxml2firewalldata/nat-default-linux.nftables +++ b/tests/networkxml2firewalldata/nat-default-linux.nftables @@ -3,7 +3,7 @@ nft \ rule \ ip \ libvirt_network \ -LIBVIRT_INP \ +guest_to_host \ iifname \ virbr0 \ tcp \ @@ -16,7 +16,7 @@ nft \ rule \ ip \ libvirt_network \ -LIBVIRT_INP \ +guest_to_host \ iifname \ virbr0 \ udp \ @@ -29,7 +29,7 @@ nft \ rule \ ip \ libvirt_network \ -LIBVIRT_OUT \ +host_to_guest \ oifname \ virbr0 \ tcp \ @@ -42,7 +42,7 @@ nft \ rule \ ip \ libvirt_network \ -LIBVIRT_OUT \ +host_to_guest \ oifname \ virbr0 \ udp \ @@ -55,7 +55,7 @@ nft \ rule \ ip \ libvirt_network \ -LIBVIRT_INP \ +guest_to_host \ iifname \ virbr0 \ tcp \ @@ -68,7 +68,7 @@ nft \ rule \ ip \ libvirt_network \ -LIBVIRT_INP \ +guest_to_host \ iifname \ virbr0 \ udp \ @@ -81,7 +81,7 @@ nft \ rule \ ip \ libvirt_network \ -LIBVIRT_OUT \ +host_to_guest \ oifname \ virbr0 \ tcp \ @@ -94,7 +94,7 @@ nft \ rule \ ip \ libvirt_network \ -LIBVIRT_OUT \ +host_to_guest \ oifname \ virbr0 \ udp \ @@ -107,7 +107,7 @@ nft \ rule \ ip \ libvirt_network \ -LIBVIRT_FWO \ +guest_output \ iifname \ virbr0 \ counter \ @@ -117,7 +117,7 @@ nft \ rule \ ip \
[PATCH v4 21/30] network: use previously saved list of firewall removal commands
When destroying a network, the network driver has always assumed that it knew what firewall rules had been added as the network was started. This was usually correct - I only recall one time in the past that the firewall rules added by libvirt were changed. But if the exact rules used for a network *were* ever changed from one build/version of libvirt to another, then we would end up attempting to remove rules that hadn't been added, and could possibly *not* remove rules that had been added. The solution to this to not make such brash assumptions about the past, but instead to save (in the network status object at network start time) a list of all the rules needed to remove the rules that were added for the network, and then use that saved list during network destroy to remove exactly what was previous added. Beyond making net-destroy more precise, there are other benefits: 1) We can change the details of the rules we add for networks from one build/release of libvirt to another and painlessly upgrade. 2) The user can switch from one firewall backend to another by simply changing the setting in network.conf and restarting libvirtd/virtnetworkd. In both cases, the restarted libvirtd/virtnetworkd will remove all the rules that had been previously added (based on the network status), and then add new rules (saving the new removal commands back into the network status) Signed-off-by: Laine Stump == NB: the current implementation saves only the commands necessary to remove the network's firewall, and names this in the status XML. It would be simple to instead save the *entire* virFirewall object (thus also including the commands that were used to add the firewall, as well as the commands needed to remove it) - although very verbose, it's possible it could be useful when debugging a firewall issue (since it's not obvious which rules were added for which network when just looking at the output of "nft list ruleset". Alternately, we could continue to store only the removal commands, but maybe change the name of the element in XML from to (which would leave the door open to expanding what is saved in the future). Any opinions on this? Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrangé --- src/network/bridge_driver.c | 35 ++ src/network/bridge_driver_linux.c| 26 + src/network/bridge_driver_nop.c | 6 ++-- src/network/bridge_driver_platform.h | 6 ++-- src/network/network_iptables.c | 43 +--- src/network/network_iptables.h | 2 +- tests/networkxml2firewalltest.c | 2 +- 7 files changed, 91 insertions(+), 29 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 38e4ab84ad..90c93d2805 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1685,6 +1685,7 @@ networkReloadFirewallRulesHelper(virNetworkObj *obj, g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(networkGetDriver()); VIR_LOCK_GUARD lock = virObjectLockGuard(obj); virNetworkDef *def = virNetworkObjGetDef(obj); +virFirewall *fwRemoval = NULL; if (virNetworkObjIsActive(obj)) { switch ((virNetworkForwardType) def->forward.type) { @@ -1696,8 +1697,9 @@ networkReloadFirewallRulesHelper(virNetworkObj *obj, * network type, forward='open', doesn't need this because it * has no iptables rules. */ -networkRemoveFirewallRules(def, cfg->firewallBackend); -ignore_value(networkAddFirewallRules(def, cfg->firewallBackend)); +networkRemoveFirewallRules(obj); +ignore_value(networkAddFirewallRules(def, cfg->firewallBackend, )); +virNetworkObjSetFwRemoval(obj, fwRemoval); break; case VIR_NETWORK_FORWARD_OPEN: @@ -1904,6 +1906,7 @@ networkStartNetworkVirtual(virNetworkDriverState *driver, bool devOnline = false; bool firewalRulesAdded = false; virSocketAddr *dnsServer = NULL; +virFirewall *fwRemoval = NULL; /* Check to see if any network IP collides with an existing route */ if (networkCheckRouteCollision(def) < 0) @@ -1949,9 +1952,11 @@ networkStartNetworkVirtual(virNetworkDriverState *driver, /* Add "once per network" rules */ if (def->forward.type != VIR_NETWORK_FORWARD_OPEN && -networkAddFirewallRules(def, cfg->firewallBackend) < 0) +networkAddFirewallRules(def, cfg->firewallBackend, ) < 0) { goto error; +} +virNetworkObjSetFwRemoval(obj, fwRemoval); firewalRulesAdded = true; for (i = 0; (ipdef = virNetworkDefGetIPByIndex(def, AF_UNSPEC, i)); i++) { @@ -2065,7 +2070,7 @@ networkStartNetworkVirtual(virNetworkDriverState *driver, if (firewalRulesAdded && def->forward.type != VIR_NETWORK_FORWARD_OPEN) -networkRemoveFirewallRules(def, cfg->firewallBackend); +networkRemoveFirewallRules(obj);
[PATCH v4 20/30] conf: add a virFirewall object to virNetworkObj
This virFirewall object will store the list of actions required to remove the firewall that was added for the currently active instance of the network, so it has been named "fwRemoval" (and when parsed into XML, the element will have the name "fwRemoval"). There are no uses of the fwRemoval object in the virNetworkObj yet, but everything is in place to add it to the XML when formatted, parse it from the XML when reading network status, and free the virFirewall object when the virNetworkObj is freed. Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrangé --- src/conf/virnetworkobj.c | 40 src/conf/virnetworkobj.h | 8 src/libvirt_private.syms | 2 ++ 3 files changed, 50 insertions(+) diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index d5aa121e20..f5db4c5804 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -55,6 +55,11 @@ struct _virNetworkObj { unsigned int taint; +/* fwRemoval contains all commands needed to remove the firewall + * that was added for this network. + */ +virFirewall *fwRemoval; + /* Immutable pointer, self locking APIs */ virMacMap *macmap; @@ -239,6 +244,24 @@ virNetworkObjSetFloorSum(virNetworkObj *obj, } +virFirewall * +virNetworkObjGetFwRemoval(virNetworkObj *obj) +{ +return obj->fwRemoval; +} + + +void +virNetworkObjSetFwRemoval(virNetworkObj *obj, + virFirewall *fwRemoval) +{ +obj->fwRemoval = fwRemoval; +/* give it a name so it's identifiable in the XML */ +if (fwRemoval) +virFirewallSetName(fwRemoval, "fwRemoval"); +} + + void virNetworkObjSetMacMap(virNetworkObj *obj, virMacMap **macmap) @@ -444,6 +467,7 @@ virNetworkObjDispose(void *opaque) virNetworkDefFree(obj->newDef); virBitmapFree(obj->classIdMap); virObjectUnref(obj->macmap); +virFirewallFree(obj->fwRemoval); } @@ -792,6 +816,9 @@ virNetworkObjFormat(virNetworkObj *obj, if (virNetworkDefFormatBuf(, obj->def, xmlopt, flags) < 0) return NULL; +if (obj->fwRemoval && virFirewallFormat(, obj->fwRemoval) < 0) +return NULL; + virBufferAdjustIndent(, -2); virBufferAddLit(, ""); @@ -826,6 +853,7 @@ virNetworkLoadState(virNetworkObjList *nets, g_autofree char *configFile = NULL; g_autoptr(virNetworkDef) def = NULL; virNetworkObj *obj = NULL; +g_autoptr(virFirewall) fwRemoval = NULL; g_autoptr(xmlDoc) xml = NULL; xmlNodePtr node = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; @@ -868,6 +896,7 @@ virNetworkLoadState(virNetworkObjList *nets, g_autofree char *classIdStr = NULL; g_autofree char *floor_sum = NULL; g_autofree xmlNodePtr *nodes = NULL; +xmlNodePtr fwNode; ctxt->node = node; if ((classIdStr = virXPathString("string(./class_id[1]/@bitmap)", @@ -902,6 +931,15 @@ virNetworkLoadState(virNetworkObjList *nets, taint |= (1 << flag); } } +if ((fwNode = virXPathNode("./firewall", ctxt))) { +g_autoptr(virFirewall) fwTmp = NULL; + +if (virFirewallParseXML(, fwNode, ctxt) < 0) +return NULL; + +if (STREQ_NULLABLE(virFirewallGetName(fwTmp), "fwRemoval")) +fwRemoval = g_steal_pointer(); +} } /* create the object */ @@ -910,6 +948,8 @@ virNetworkLoadState(virNetworkObjList *nets, def = NULL; +virNetworkObjSetFwRemoval(obj, g_steal_pointer()); + /* assign status data stored in the network object */ if (classIdMap) { virBitmapFree(obj->classIdMap); diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h index d3847d3422..6ebc6c49f5 100644 --- a/src/conf/virnetworkobj.h +++ b/src/conf/virnetworkobj.h @@ -23,6 +23,7 @@ #include "network_conf.h" #include "virnetworkportdef.h" +#include "virfirewall.h" typedef struct _virNetworkObj virNetworkObj; @@ -76,6 +77,13 @@ void virNetworkObjSetFloorSum(virNetworkObj *obj, unsigned long long floor_sum); +virFirewall * +virNetworkObjGetFwRemoval(virNetworkObj *obj); + +void +virNetworkObjSetFwRemoval(virNetworkObj *obj, + virFirewall *fwRemoval); + void virNetworkObjSetMacMap(virNetworkObj *obj, virMacMap **macmap); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9ba1edb37e..e8c15232d9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1300,6 +1300,7 @@ virNetworkObjGetClassIdMap; virNetworkObjGetDef; virNetworkObjGetDnsmasqPid; virNetworkObjGetFloorSum; +virNetworkObjGetFwRemoval; virNetworkObjGetMacMap; virNetworkObjGetMetadata; virNetworkObjGetNewDef; @@ -1331,6 +1332,7 @@ virNetworkObjSetDef; virNetworkObjSetDefTransient; virNetworkObjSetDnsmasqPid; virNetworkObjSetFloorSum; +virNetworkObjSetFwRemoval;
[PATCH v4 19/30] util: new functions virFirewallParseXML() and virFirewallFormat()
These functions convert a virFirewall object to/from XML so that it can be serialized to disk (in a virNetworkObj's status file) and restored later (e.g. after libvirtd/virtnetworkd is restarted). Signed-off-by: Laine Stump --- src/libvirt_private.syms | 2 + src/util/virfirewall.c | 219 +++ src/util/virfirewall.h | 9 ++ 3 files changed, 230 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 42d2f9480f..9ba1edb37e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2414,11 +2414,13 @@ virFirewallCmdAddArgList; virFirewallCmdAddArgSet; virFirewallCmdGetArgCount; virFirewallCmdToString; +virFirewallFormat; virFirewallFree; virFirewallGetBackend; virFirewallGetName; virFirewallNew; virFirewallNewFromRollback; +virFirewallParseXML; virFirewallRemoveCmd; virFirewallSetName; virFirewallStartRollback; diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 2f4f128cd1..649b2289c0 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -39,6 +39,14 @@ VIR_ENUM_IMPL(virFirewallBackend, VIR_FIREWALL_BACKEND_LAST, "iptables"); +VIR_ENUM_DECL(virFirewallLayer); +VIR_ENUM_IMPL(virFirewallLayer, + VIR_FIREWALL_LAYER_LAST, + "ethernet", + "ipv4", + "ipv6", +); + typedef struct _virFirewallGroup virFirewallGroup; VIR_ENUM_DECL(virFirewallLayerCommand); @@ -827,3 +835,214 @@ virFirewallNewFromRollback(virFirewall *original, return 0; } + + +/* virFirewallGetFlagsFromNode: + * @node: the xmlNode to check for an ignoreErrors attribute + * + * A short helper to get the setting of the ignorErrors attribute from + * an xmlNode. Returns -1 on error (with error reported), or the + * VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS bit set/reset according to + * the value of the attribute. + */ +static int +virFirewallGetFlagsFromNode(xmlNodePtr node) +{ +virTristateBool ignoreErrors; + +if (virXMLPropTristateBool(node, "ignoreErrors", VIR_XML_PROP_NONE, ) < 0) +return -1; + +if (ignoreErrors == VIR_TRISTATE_BOOL_YES) +return VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS; +return 0; +} + + +/** + * virFirewallParseXML: + * @firewall: pointer to virFirewall* to fill in with new virFirewall object + * + * Construct a new virFirewall object according to the XML in + * xmlNodePtr. Return 0 (and new object) on success, or -1 (with + * error reported) on error. + * + * Example of element XML: + * + * + * + * + * + * arg1 + * arg2 + * ... + * + * + * + * ... + + * ... + * + * ... + * + */ +int +virFirewallParseXML(virFirewall **firewall, +xmlNodePtr node, +xmlXPathContextPtr ctxt) +{ +g_autoptr(virFirewall) newfw = NULL; +virFirewallBackend backend; +g_autofree xmlNodePtr *groupNodes = NULL; +ssize_t ngroups; +size_t g; +VIR_XPATH_NODE_AUTORESTORE(ctxt); + +ctxt->node = node; + +if (virXMLPropEnum(node, "backend", virFirewallBackendTypeFromString, + VIR_XML_PROP_REQUIRED, ) < 0) { +return -1; +} + +newfw = virFirewallNew(backend); + +newfw->name = virXMLPropString(node, "name"); + +ngroups = virXPathNodeSet("./group", ctxt, ); +if (ngroups < 0) +return -1; + +for (g = 0; g < ngroups; g++) { +int flags = 0; +g_autofree xmlNodePtr *actionNodes = NULL; +ssize_t nactions; +size_t a; + +ctxt->node = groupNodes[g]; +nactions = virXPathNodeSet("./action", ctxt, ); +if (nactions < 0) +return -1; +if (nactions == 0) +continue; + +if ((flags = virFirewallGetFlagsFromNode(groupNodes[g])) < 0) +return -1; + +virFirewallStartTransaction(newfw, flags); + +for (a = 0; a < nactions; a++) { +g_autofree xmlNodePtr *argsNodes = NULL; +ssize_t nargs; +size_t i; +virFirewallLayer layer; +virFirewallCmd *action; +bool ignoreErrors; + +ctxt->node = actionNodes[a]; + +if (!(ctxt->node = virXPathNode("./args", ctxt))) +continue; + +if ((flags = virFirewallGetFlagsFromNode(actionNodes[a])) < 0) +return -1; + +ignoreErrors = flags & VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS; + +if (virXMLPropEnum(actionNodes[a], "layer", + virFirewallLayerTypeFromString, + VIR_XML_PROP_REQUIRED, ) < 0) { +return -1; +} + +nargs = virXPathNodeSet("./item", ctxt, ); +if (nargs < 0) +return -1; +if (nargs == 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", +
[PATCH v4 26/30] network: prefer the nftables backend over iptables
The patch that added the nftables backend for virtual networks left iptables as the default backend when both nftables and iptables are installed. The only functional difference between the two backends is that the nftables backend doesn't add any rules to fix up the checksum of DHCP packets, which will cause failures on guests with very old OSes (e.g. RHEL5) that have a virtio-net network interface using vhost packet processing (the default), connected to a libvirt virtual network, and configured to acquire the interface IP using DHCP. Since RHEL5 has been out of support for several years already, we might as well start off nftables support right by making it the default. Distros that aren't quite ready to default to nftables (e.g. maybe they're rebasing libvirt within a release and don't want to surprise anyone with an automatic switch from iptables to nftables) can simply run meson with "-Dfirewall_backend=iptables" during their official package build. In the extremely unlikely case that this causes a problem for a user, they can work around the failure by adding " to the guest element. Signed-off-by: Laine Stump --- meson_options.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson_options.txt b/meson_options.txt index 367629f5dc..b68bd52477 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -98,7 +98,7 @@ option('chrdev_lock_files', type: 'string', value: '', description: 'location fo option('dtrace', type: 'feature', value: 'auto', description: 'use dtrace for static probing') option('firewalld', type: 'feature', value: 'auto', description: 'firewalld support') option('firewalld_zone', type: 'feature', value: 'auto', description: 'whether to install firewalld libvirt zone') -option('firewall_backend', type: 'string', value: 'iptables', description: 'which firewall backend to use by default when none is specified') +option('firewall_backend', type: 'string', value: 'nftables', description: 'which firewall backend to use by default when none is specified') 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 : paths. Both pairs and list items are separated by a colon.') -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v4 27/30] spec: require either iptables or nftables if network driver is installed
This makes it possible to uninstall iptables, as long as nftables is installed. Signed-off-by: Laine Stump --- libvirt.spec.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index 05f7a7e7c0..55f32172b3 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -592,7 +592,7 @@ Summary: Network driver plugin for the libvirtd daemon Requires: libvirt-daemon-common = %{version}-%{release} Requires: libvirt-libs = %{version}-%{release} Requires: dnsmasq >= 2.41 -Requires: iptables +Requires: (iptables or nftables) %description daemon-driver-network The network driver plugin for the libvirtd daemon, providing -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v4 09/30] util: determine ignoreErrors value when creating virFirewallCmd, not when applying
We know at the time a virFirewallCmd is created (with virFirewallAddCmd*()) whether or not we will later want to ignore errors encountered when attempting to apply that command - if ignoreErrors is set in the AddCmd or if the group has already had VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS set, then we ignore the errors. Rather than setting the fwCmd->ignoreErrors only according to the arg sent to virFirewallAddCmdFull(), and then later (at ApplyCmd-time) combining that with the group transactionFlags setting (and passing it all the way down the call chain), just combine the two flags right away and store this final value in fwCmd->ignoreErrors when the virFirewallCmd is created (thus avoiding the need to look at anything other than fwCmd->ignoreErrors at the time the command is applied). Once that is done, we can simply grab ignoreErrors from the object down in virFirewallApply() rather than cluttering up the argument list on the entire call chain. Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrangé --- src/util/virfirewall.c | 28 +--- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index a57a79d4ce..56d43bfdde 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -211,14 +211,19 @@ virFirewallAddCmdFullV(virFirewall *firewall, fwCmd->layer = layer; fwCmd->queryCB = cb; fwCmd->queryOpaque = opaque; -fwCmd->ignoreErrors = ignoreErrors; while ((str = va_arg(args, char *)) != NULL) ADD_ARG(fwCmd, str); if (group->addingRollback) { +fwCmd->ignoreErrors = true; /* always ignore errors when rolling back */ VIR_APPEND_ELEMENT_COPY(group->rollback, group->nrollback, fwCmd); } else { +/* when not rolling back, ignore errors if this group (transaction) + * was started with VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS *or* + * if this specific rule was created with ignoreErrors == true + */ +fwCmd->ignoreErrors = ignoreErrors || (group->actionFlags & VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS); VIR_APPEND_ELEMENT_COPY(group->action, group->naction, fwCmd); } @@ -466,8 +471,7 @@ virFirewallCmdToString(const char *cmd, static int virFirewallApplyCmdDirect(virFirewallCmd *fwCmd, - bool ignoreErrors, - char **output) + char **output) { size_t i; const char *bin = virFirewallLayerCommandTypeToString(fwCmd->layer); @@ -511,7 +515,7 @@ virFirewallApplyCmdDirect(virFirewallCmd *fwCmd, return -1; if (status != 0) { -if (ignoreErrors) { +if (fwCmd->ignoreErrors) { VIR_DEBUG("Ignoring error running command"); } else { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -528,22 +532,18 @@ virFirewallApplyCmdDirect(virFirewallCmd *fwCmd, static int virFirewallApplyCmd(virFirewall *firewall, -virFirewallCmd *fwCmd, -bool ignoreErrors) +virFirewallCmd *fwCmd) { g_autofree char *output = NULL; g_auto(GStrv) lines = NULL; -if (fwCmd->ignoreErrors) -ignoreErrors = fwCmd->ignoreErrors; - if (fwCmd->argsLen == 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Can't apply empty firewall command")); return -1; } -if (virFirewallApplyCmdDirect(fwCmd, ignoreErrors, ) < 0) +if (virFirewallApplyCmdDirect(fwCmd, ) < 0) return -1; if (fwCmd->queryCB && output) { @@ -570,7 +570,7 @@ virFirewallApplyGroup(virFirewall *firewall, size_t idx) { virFirewallGroup *group = firewall->groups[idx]; -bool ignoreErrors = (group->actionFlags & VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS); + size_t i; VIR_INFO("Starting transaction for firewall=%p group=%p flags=0x%x", @@ -578,9 +578,7 @@ virFirewallApplyGroup(virFirewall *firewall, firewall->currentGroup = idx; group->addingRollback = false; for (i = 0; i < group->naction; i++) { -if (virFirewallApplyCmd(firewall, -group->action[i], -ignoreErrors) < 0) +if (virFirewallApplyCmd(firewall, group->action[i]) < 0) return -1; } return 0; @@ -598,7 +596,7 @@ virFirewallRollbackGroup(virFirewall *firewall, firewall->currentGroup = idx; group->addingRollback = true; for (i = 0; i < group->nrollback; i++) -ignore_value(virFirewallApplyCmd(firewall, group->rollback[i], true)); +ignore_value(virFirewallApplyCmd(firewall, group->rollback[i])); } -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v4 23/30] meson: stop looking for iptables/ip6tables/ebtables at build time
This was the only reason we required the iptables and ebtables packages at build time, and many other external commands already have their binaries found at runtime by looking through $PATH (virCommand automatically does this), so we may as well do it for these commands as well. Since we no longer need iptables or iptables at build time, we can also drop the BuildRequires for them from the rpm specfile. Inspired-by: 6aa2fa38b04b802f137e51ebbeb4ca9b67487575 Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrangé --- libvirt.spec.in | 2 -- meson.build | 3 --- src/network/bridge_driver_conf.c | 2 ++ src/util/virfirewall.h | 5 + 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index bde25c6f6e..05f7a7e7c0 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -356,8 +356,6 @@ BuildRequires: sanlock-devel >= 2.4 BuildRequires: libpcap-devel >= 1.5.0 BuildRequires: libnl3-devel BuildRequires: libselinux-devel -BuildRequires: iptables -BuildRequires: ebtables # For modprobe BuildRequires: kmod BuildRequires: cyrus-sasl-devel diff --git a/meson.build b/meson.build index 02340c09dd..b9379d6ce7 100644 --- a/meson.build +++ b/meson.build @@ -818,10 +818,7 @@ optional_test_programs = [ optional_programs = [ 'dmidecode', - 'ebtables', 'ip', - 'ip6tables', - 'iptables', 'iscsiadm', 'mdevctl', 'mm-ctl', diff --git a/src/network/bridge_driver_conf.c b/src/network/bridge_driver_conf.c index 25cbbf8933..e8acb70554 100644 --- a/src/network/bridge_driver_conf.c +++ b/src/network/bridge_driver_conf.c @@ -27,8 +27,10 @@ #include "virerror.h" #include "virfile.h" #include "virutil.h" +#include "virfirewall.h" /* for binary names */ #include "bridge_driver_conf.h" + #define VIR_FROM_THIS VIR_FROM_NETWORK VIR_LOG_INIT("network.bridge_driver"); diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h index 917745c315..4ac16f02b3 100644 --- a/src/util/virfirewall.h +++ b/src/util/virfirewall.h @@ -25,6 +25,11 @@ #include "virbuffer.h" #include "virxml.h" +/* various external programs executed when applying firewalls */ +#define EBTABLES "ebtables" +#define IPTABLES "iptables" +#define IP6TABLES "ip6tables" + typedef struct _virFirewall virFirewall; typedef struct _virFirewallCmd virFirewallCmd; -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v4 22/30] network: save network status when firewall rules are reloaded
In the case that a new version of libvirt is started that uses different rules to build the network firewall, we need to re-save the status so that when the network is destroyed (or the *next* time libvirt is restarted and wants to remove/re-add the firewall), it will have the proper information to perform the firewall removal. Signed-off-by: Laine Stump --- src/conf/virnetworkobj.c| 1 + src/network/bridge_driver.c | 8 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index f5db4c5804..19305798cb 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -834,6 +834,7 @@ virNetworkObjSaveStatus(const char *statusDir, int flags = 0; g_autofree char *xml = NULL; +VIR_DEBUG("Writing network status to disk"); if (!(xml = virNetworkObjFormat(obj, xmlopt, flags))) return -1; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 90c93d2805..88a8882376 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1686,6 +1686,7 @@ networkReloadFirewallRulesHelper(virNetworkObj *obj, VIR_LOCK_GUARD lock = virObjectLockGuard(obj); virNetworkDef *def = virNetworkObjGetDef(obj); virFirewall *fwRemoval = NULL; +bool saveStatus = false; if (virNetworkObjIsActive(obj)) { switch ((virNetworkForwardType) def->forward.type) { @@ -1700,6 +1701,7 @@ networkReloadFirewallRulesHelper(virNetworkObj *obj, networkRemoveFirewallRules(obj); ignore_value(networkAddFirewallRules(def, cfg->firewallBackend, )); virNetworkObjSetFwRemoval(obj, fwRemoval); +saveStatus = true; break; case VIR_NETWORK_FORWARD_OPEN: @@ -1717,6 +1719,11 @@ networkReloadFirewallRulesHelper(virNetworkObj *obj, } } +if (saveStatus) { +ignore_value(virNetworkObjSaveStatus(cfg->stateDir, obj, + network_driver->xmlopt)); +} + return 0; } @@ -2364,7 +2371,6 @@ networkStartNetwork(virNetworkDriverState *driver, /* Persist the live configuration now that anything autogenerated * is setup. */ -VIR_DEBUG("Writing network status to disk"); if (virNetworkObjSaveStatus(cfg->stateDir, obj, network_driver->xmlopt) < 0) goto cleanup; -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v4 12/30] network: support setting firewallBackend from network.conf
It still can have only one useful value ("iptables"), but once a 2nd value is supported, it will be selectable by setting "firewall_backend=nftables" in /etc/libvirt/network.conf. If firewall_backend isn't set in network.conf, then libvirt will check to see if the iptables binary is present on the system and set firewallBackend to iptables - if no iptables binary is found, that is considered a fatal error (since no networks can be started anyway), so an error is logged and startup of the network driver fails. NB: network.conf is itself created from network.conf.in at build time, and the advertised default setting of firewall_backend (in a commented out line) is set from the meson_options.txt setting "firewall_backend". This way the conf file will have correct information no matter what default backend is chosen at build time. Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrangé --- meson.build | 5 +++ meson_options.txt| 1 + src/network/bridge_driver.c | 22 ++- src/network/bridge_driver_conf.c | 50 src/network/bridge_driver_conf.h | 3 ++ src/network/bridge_driver_linux.c| 6 ++- src/network/bridge_driver_nop.c | 6 ++- src/network/bridge_driver_platform.h | 6 ++- src/network/libvirtd_network.aug | 5 ++- src/network/meson.build | 22 ++- src/network/network.conf.in | 8 src/network/test_libvirtd_network.aug.in | 3 ++ tests/networkxml2firewalltest.c | 2 +- 13 files changed, 119 insertions(+), 20 deletions(-) diff --git a/meson.build b/meson.build index 1518afa1cb..02340c09dd 100644 --- a/meson.build +++ b/meson.build @@ -1618,6 +1618,11 @@ endif if not get_option('driver_network').disabled() and conf.has('WITH_LIBVIRTD') conf.set('WITH_NETWORK', 1) + firewall_backend = get_option('firewall_backend') + conf.set_quoted('FIREWALL_BACKEND', firewall_backend) + if firewall_backend == 'iptables' +conf.set('FIREWALL_BACKEND_DEFAULT_IPTABLES', 1) + endif 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 ed91d97abf..367629f5dc 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -98,6 +98,7 @@ option('chrdev_lock_files', type: 'string', value: '', description: 'location fo option('dtrace', type: 'feature', value: 'auto', description: 'use dtrace for static probing') option('firewalld', type: 'feature', value: 'auto', description: 'firewalld support') option('firewalld_zone', type: 'feature', value: 'auto', description: 'whether to install firewalld libvirt zone') +option('firewall_backend', type: 'string', value: 'iptables', description: 'which firewall backend to use by default when none is specified') 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 : paths. Both pairs and list items are separated by a colon.') diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index d89700c6ee..38e4ab84ad 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1682,6 +1682,7 @@ static int networkReloadFirewallRulesHelper(virNetworkObj *obj, void *opaque G_GNUC_UNUSED) { +g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(networkGetDriver()); VIR_LOCK_GUARD lock = virObjectLockGuard(obj); virNetworkDef *def = virNetworkObjGetDef(obj); @@ -1695,8 +1696,8 @@ networkReloadFirewallRulesHelper(virNetworkObj *obj, * network type, forward='open', doesn't need this because it * has no iptables rules. */ -networkRemoveFirewallRules(def); -ignore_value(networkAddFirewallRules(def)); +networkRemoveFirewallRules(def, cfg->firewallBackend); +ignore_value(networkAddFirewallRules(def, cfg->firewallBackend)); break; case VIR_NETWORK_FORWARD_OPEN: @@ -1948,7 +1949,7 @@ networkStartNetworkVirtual(virNetworkDriverState *driver, /* Add "once per network" rules */ if (def->forward.type != VIR_NETWORK_FORWARD_OPEN && -networkAddFirewallRules(def) < 0) +networkAddFirewallRules(def, cfg->firewallBackend) < 0) goto error; firewalRulesAdded = true; @@ -2064,7 +2065,7 @@ networkStartNetworkVirtual(virNetworkDriverState *driver, if (firewalRulesAdded && def->forward.type != VIR_NETWORK_FORWARD_OPEN) -networkRemoveFirewallRules(def); +networkRemoveFirewallRules(def, cfg->firewallBackend);
[PATCH v4 18/30] util: new function virFirewallNewFromRollback()
virFirewallNewFromRollback() creates a new virFirewall object that contains a copy of the "rollback" commands from an existing virFirewall object, but in reverse order. The intent is that this virFirewall be saved and used later to remove the firewall rules that were added for a network. Signed-off-by: Laine Stump --- src/libvirt_private.syms | 1 + src/util/virfirewall.c | 59 src/util/virfirewall.h | 1 + 3 files changed, 61 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9e16ae4225..42d2f9480f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2418,6 +2418,7 @@ virFirewallFree; virFirewallGetBackend; virFirewallGetName; virFirewallNew; +virFirewallNewFromRollback; virFirewallRemoveCmd; virFirewallSetName; virFirewallStartRollback; diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index fe8c09c0ed..2f4f128cd1 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -768,3 +768,62 @@ virFirewallApply(virFirewall *firewall) return 0; } + + +/** + * virFirewallNewFromRollback: + + * @original: the original virFirewall object containing the rollback + *of interest + * @fwRemoval: a firewall object that, when applied, will remove @original + * + * Copy the rollback rules from the current virFirewall object as a + * new virFirewall. This virFirewall can then be saved to apply later + * and counteract everything done by the original. + * + * Returns 0 on success, -1 on error + */ +int +virFirewallNewFromRollback(virFirewall *original, + virFirewall **fwRemoval) +{ +size_t g; +g_autoptr(virFirewall) firewall = NULL; + +if (original->err) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("error in original firewall object")); +return -1; +} + +firewall = virFirewallNew(original->backend); + +/* add the rollback commands in reverse order of actions/groups of + * what was applied in the original firewall. + */ +for (g = original->ngroups; g > 0; g--) { +size_t r; +virFirewallGroup *group = original->groups[g - 1]; + +if (group->nrollback == 0) +continue; + +virFirewallStartTransaction(firewall, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS); + +for (r = group->nrollback; r > 0; r--) { +size_t i; +virFirewallCmd *origCmd = group->rollback[r - 1]; +virFirewallCmd *rbCmd = virFirewallAddCmd(firewall, origCmd->layer, NULL); + +for (i = 0; i < origCmd->argsLen; i++) +ADD_ARG(rbCmd, origCmd->args[i]); +} +} + +if (firewall->ngroups == 0) +VIR_DEBUG("original firewall object is empty"); +else +*fwRemoval = g_steal_pointer(); + +return 0; +} diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h index e8ad81056f..931dfb04cf 100644 --- a/src/util/virfirewall.h +++ b/src/util/virfirewall.h @@ -44,6 +44,7 @@ typedef enum { VIR_ENUM_DECL(virFirewallBackend); virFirewall *virFirewallNew(virFirewallBackend backend); +int virFirewallNewFromRollback(virFirewall *original, virFirewall **fwRemoval); void virFirewallFree(virFirewall *firewall); virFirewallBackend virFirewallGetBackend(virFirewall *firewall); const char *virFirewallGetName(virFirewall *firewall); -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v4 15/30] util: implement rollback rule autocreation for iptables commands
If the VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK flag is set, each time an iptables command is executed that is adding a rule or chain, a corresponding command that will *delete* the same rule/chain is constructed and added to the list of rollback commands. If we later want to undo the entire firewall, we can just run those commands. This isn't yet used anywhere, since VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK isn't being set. Signed-off-by: Laine Stump --- src/util/virfirewall.c | 55 -- 1 file changed, 48 insertions(+), 7 deletions(-) diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 9def8999d5..b9b4140ad6 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -470,7 +470,7 @@ void virFirewallStartTransaction(virFirewall *firewall, * Returns the virFirewallTransactionFlags for the currently active * group (transaction) in @firewall. */ -static virFirewallTransactionFlags G_GNUC_UNUSED +static virFirewallTransactionFlags virFirewallTransactionGetFlags(virFirewall *firewall) { return firewall->groups[firewall->currentGroup]->actionFlags; @@ -525,16 +525,25 @@ virFirewallCmdToString(const char *cmd, } +#define VIR_IPTABLES_ARG_IS_CREATE(arg) \ +(STREQ(arg, "--insert") || STREQ(arg, "-I") || \ + STREQ(arg, "--append") || STREQ(arg, "-A")) + + static int -virFirewallApplyCmdDirect(virFirewallCmd *fwCmd, - char **output) +virFirewallCmdIptablesApply(virFirewall *firewall, +virFirewallCmd *fwCmd, +char **output) { -size_t i; const char *bin = virFirewallLayerCommandTypeToString(fwCmd->layer); +bool checkRollback = (virFirewallTransactionGetFlags(firewall) & + VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK); +bool needRollback = false; g_autoptr(virCommand) cmd = NULL; g_autofree char *cmdStr = NULL; -int status; g_autofree char *error = NULL; +size_t i; +int status; if (!bin) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -558,8 +567,13 @@ virFirewallApplyCmdDirect(virFirewallCmd *fwCmd, break; } -for (i = 0; i < fwCmd->argsLen; i++) +for (i = 0; i < fwCmd->argsLen; i++) { +/* the -I/-A arg could be at any position in the list */ +if (checkRollback && VIR_IPTABLES_ARG_IS_CREATE(fwCmd->args[i])) +needRollback = true; + virCommandAddArg(cmd, fwCmd->args[i]); +} cmdStr = virCommandToString(cmd, false); VIR_INFO("Running firewall command '%s'", NULLSTR(cmdStr)); @@ -571,8 +585,10 @@ virFirewallApplyCmdDirect(virFirewallCmd *fwCmd, return -1; if (status != 0) { +/* the command failed, decide whether or not to report it */ if (fwCmd->ignoreErrors) { VIR_DEBUG("Ignoring error running command"); +return 0; } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to run firewall command %1$s: %2$s"), @@ -582,6 +598,31 @@ virFirewallApplyCmdDirect(virFirewallCmd *fwCmd, } } +/* the command was successful, see if we need to add a + * rollback command + */ + +if (needRollback) { +virFirewallCmd *rollback += virFirewallAddRollbackCmd(firewall, fwCmd->layer, NULL); +g_autofree char *rollbackStr = NULL; + +for (i = 0; i < fwCmd->argsLen; i++) { +/* iptables --delete wants the entire commandline that + * was used for --insert but with s/insert/delete/ + */ +if (VIR_IPTABLES_ARG_IS_CREATE(fwCmd->args[i])) { +virFirewallCmdAddArg(firewall, rollback, "--delete"); +} else { +virFirewallCmdAddArg(firewall, rollback, fwCmd->args[i]); +} +} + +rollbackStr = virFirewallCmdToString(virFirewallLayerCommandTypeToString(fwCmd->layer), + rollback); +VIR_DEBUG("Recording Rollback command '%s'", NULLSTR(rollbackStr)); +} + return 0; } @@ -599,7 +640,7 @@ virFirewallApplyCmd(virFirewall *firewall, return -1; } -if (virFirewallApplyCmdDirect(fwCmd, ) < 0) +if (virFirewallCmdIptablesApply(firewall, fwCmd, ) < 0) return -1; if (fwCmd->queryCB && output) { -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v4 17/30] util: add name attribute to virFirewall
This will be used to label (via "name='blah'") a firewall when it is formatted to XML and written to the network status. Signed-off-by: Laine Stump --- src/libvirt_private.syms | 2 ++ src/util/virfirewall.c | 20 +++- src/util/virfirewall.h | 2 ++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b3cbe1a833..9e16ae4225 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2416,8 +2416,10 @@ virFirewallCmdGetArgCount; virFirewallCmdToString; virFirewallFree; virFirewallGetBackend; +virFirewallGetName; virFirewallNew; virFirewallRemoveCmd; +virFirewallSetName; virFirewallStartRollback; virFirewallStartTransaction; diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index b9b4140ad6..fe8c09c0ed 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -78,6 +78,7 @@ struct _virFirewallGroup { struct _virFirewall { int err; +char *name; size_t ngroups; virFirewallGroup **groups; size_t currentGroup; @@ -119,6 +120,22 @@ virFirewallGetBackend(virFirewall *firewall) } +const char * +virFirewallGetName(virFirewall *firewall) +{ +return firewall->name; +} + + +void +virFirewallSetName(virFirewall *firewall, + const char *name) +{ +g_free(firewall->name); +firewall->name = g_strdup(name); +} + + static void virFirewallCmdFree(virFirewallCmd *fwCmd) { @@ -169,8 +186,9 @@ void virFirewallFree(virFirewall *firewall) for (i = 0; i < firewall->ngroups; i++) virFirewallGroupFree(firewall->groups[i]); -g_free(firewall->groups); +g_free(firewall->groups); +g_free(firewall->name); g_free(firewall); } diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h index e6aac365f1..e8ad81056f 100644 --- a/src/util/virfirewall.h +++ b/src/util/virfirewall.h @@ -46,6 +46,8 @@ VIR_ENUM_DECL(virFirewallBackend); virFirewall *virFirewallNew(virFirewallBackend backend); void virFirewallFree(virFirewall *firewall); virFirewallBackend virFirewallGetBackend(virFirewall *firewall); +const char *virFirewallGetName(virFirewall *firewall); +void virFirewallSetName(virFirewall *firewall, const char *name); /** * virFirewallAddCmd: -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v4 10/30] util/network: new virFirewallBackend enum
(This paragraph is for historical reference only, described only to avoid confusion of past use of the name with its new use) In a past life, virFirewallBackend had been a private static in virfirewall.c that was set at daemon init time, and used to globally (i.e. for all drivers in the daemon) determine whether to directly execute iptables commands, or to run them indirectly via the firewalld passthrough API. This was removed in commit d566cc55, since we decided that using the firewalld passthrough API is never appropriate. Now the same enum, virFirewallBackend, is being reintroduced, with a different meaning and usage pattern. It will be used to pick between using nftables commands or iptables commands (in either case directly handled by libvirt, *not* via firewalld). Additionally, rather than being a static known only within virfirewall.c and applying to all firewall commands for all drivers, each virFirewall object will have its own backend setting, which will be set during virFirewallNew() by the driver who wants to add a firewall rule. This will allow the nwfilter and network drivers to each have their own backend setting, even when they coexist in a single unified daemon. At least as important as that, it will also allow an instance of the network driver to remove iptables rules that had been added by a previous instance, and then add nftables rules for the new instance (in the case that an admin, or possibly an update, switches the driver backend from iptables to nftable) Initially, the enum will only have one usable value - VIR_FIREWALL_BACKEND_IPTABLES, and that will be hardcoded into all calls to virFirewallNew(). The other enum value (along with a method of setting it for each driver) will be added later, when it can be used (when the nftables backend is in the code). Signed-off-by: Laine Stump --- src/libvirt_private.syms | 3 +++ src/network/network_iptables.c| 6 +++--- src/nwfilter/nwfilter_ebiptables_driver.c | 16 src/util/virebtables.c| 4 ++-- src/util/virfirewall.c| 15 ++- src/util/virfirewall.h| 11 ++- tests/virfirewalltest.c | 20 ++-- 7 files changed, 50 insertions(+), 25 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 435be1b4c5..457b51e289 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2405,6 +2405,8 @@ virFileCacheSetPriv; # util/virfirewall.h virFirewallAddCmdFull; virFirewallApply; +virFirewallBackendTypeFromString; +virFirewallBackendTypeToString; virFirewallCmdAddArg; virFirewallCmdAddArgFormat; virFirewallCmdAddArgList; @@ -2412,6 +2414,7 @@ virFirewallCmdAddArgSet; virFirewallCmdGetArgCount; virFirewallCmdToString; virFirewallFree; +virFirewallGetBackend; virFirewallNew; virFirewallRemoveCmd; virFirewallStartRollback; diff --git a/src/network/network_iptables.c b/src/network/network_iptables.c index d7e749adf0..db35a4c5a0 100644 --- a/src/network/network_iptables.c +++ b/src/network/network_iptables.c @@ -131,7 +131,7 @@ iptablesPrivateChainCreate(virFirewall *fw, int iptablesSetupPrivateChains(virFirewallLayer layer) { -g_autoptr(virFirewall) fw = virFirewallNew(); +g_autoptr(virFirewall) fw = virFirewallNew(VIR_FIREWALL_BACKEND_IPTABLES); iptablesGlobalChain filter_chains[] = { {"INPUT", VIR_IPTABLES_INPUT_CHAIN}, {"OUTPUT", VIR_IPTABLES_OUTPUT_CHAIN}, @@ -1597,7 +1597,7 @@ iptablesAddFirewallRules(virNetworkDef *def) { size_t i; virNetworkIPDef *ipdef; -g_autoptr(virFirewall) fw = virFirewallNew(); +g_autoptr(virFirewall) fw = virFirewallNew(VIR_FIREWALL_BACKEND_IPTABLES); virFirewallStartTransaction(fw, 0); @@ -1632,7 +1632,7 @@ iptablesRemoveFirewallRules(virNetworkDef *def) { size_t i; virNetworkIPDef *ipdef; -g_autoptr(virFirewall) fw = virFirewallNew(); +g_autoptr(virFirewall) fw = virFirewallNew(VIR_FIREWALL_BACKEND_IPTABLES); virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS); iptablesRemoveChecksumFirewallRules(fw, def); diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 3ef1bb576e..5082b62577 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -2820,7 +2820,7 @@ static int ebtablesApplyBasicRules(const char *ifname, const virMacAddr *macaddr) { -g_autoptr(virFirewall) fw = virFirewallNew(); +g_autoptr(virFirewall) fw = virFirewallNew(VIR_FIREWALL_BACKEND_IPTABLES); char chain[MAX_CHAINNAME_LENGTH]; char chainPrefix = CHAINPREFIX_HOST_IN_TEMP; char macaddr_str[VIR_MAC_STRING_BUFLEN]; @@ -2893,7 +2893,7 @@ ebtablesApplyDHCPOnlyRules(const char *ifname, char macaddr_str[VIR_MAC_STRING_BUFLEN]; unsigned int idx = 0; unsigned int
[PATCH v4 11/30] network: add (empty) network.conf file to distribution files
This file is generated from network.conf.in because it will soon have an item that must be modified according to meson buildtime config. Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrangé --- libvirt.spec.in | 3 ++ src/network/libvirtd_network.aug | 36 src/network/meson.build | 16 +++ src/network/network.conf.in | 3 ++ src/network/test_libvirtd_network.aug.in | 2 ++ 5 files changed, 60 insertions(+) create mode 100644 src/network/libvirtd_network.aug create mode 100644 src/network/network.conf.in create mode 100644 src/network/test_libvirtd_network.aug.in diff --git a/libvirt.spec.in b/libvirt.spec.in index 64018192b6..bde25c6f6e 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2108,6 +2108,9 @@ exit 0 %config(noreplace) %{_sysconfdir}/libvirt/virtnetworkd.conf %{_datadir}/augeas/lenses/virtnetworkd.aug %{_datadir}/augeas/lenses/tests/test_virtnetworkd.aug +%config(noreplace) %{_sysconfdir}/libvirt/network.conf +%{_datadir}/augeas/lenses/libvirtd_network.aug +%{_datadir}/augeas/lenses/tests/test_libvirtd_network.aug %{_unitdir}/virtnetworkd.service %{_unitdir}/virtnetworkd.socket %{_unitdir}/virtnetworkd-ro.socket diff --git a/src/network/libvirtd_network.aug b/src/network/libvirtd_network.aug new file mode 100644 index 00..ae153d96a1 --- /dev/null +++ b/src/network/libvirtd_network.aug @@ -0,0 +1,36 @@ +(* /etc/libvirt/network.conf *) + +module Libvirtd_network = + autoload xfm + + let eol = del /[ \t]*\n/ "\n" + let value_sep = del /[ \t]*=[ \t]*/ " = " + let indent = del /[ \t]*/ "" + + let array_sep = del /,[ \t\n]*/ ", " + let array_start = del /\[[ \t\n]*/ "[ " + let array_end = del /\]/ "]" + + let str_val = del /\"/ "\"" . store /[^\"]*/ . del /\"/ "\"" + let bool_val = store /0|1/ + let int_val = store /[0-9]+/ + let str_array_element = [ seq "el" . str_val ] . del /[ \t\n]*/ "" + let str_array_val = counter "el" . array_start . ( str_array_element . ( array_sep . str_array_element ) * ) ? . array_end + + let str_entry (kw:string) = [ key kw . value_sep . str_val ] + let bool_entry (kw:string) = [ key kw . value_sep . bool_val ] + let int_entry (kw:string) = [ key kw . value_sep . int_val ] + let str_array_entry (kw:string) = [ key kw . value_sep . str_array_val ] + + (* Each entry in the config is one of the following *) + let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] + let empty = [ label "#empty" . eol ] + + let record = indent . eol + + let lns = ( record | comment | empty ) * + + let filter = incl "/etc/libvirt/network.conf" + . Util.stdexcl + + let xfm = transform lns filter diff --git a/src/network/meson.build b/src/network/meson.build index 305e2d52fb..0336435862 100644 --- a/src/network/meson.build +++ b/src/network/meson.build @@ -49,6 +49,22 @@ if conf.has('WITH_NETWORK') ], } + network_conf = configure_file( +input: 'network.conf.in', +output: 'network.conf', +configuration: configmake_conf, + ) + virt_conf_files += network_conf + virt_aug_files += files('libvirtd_network.aug') + virt_test_aug_files += { +'name': 'test_libvirtd_network.aug', +'aug': files('test_libvirtd_network.aug.in'), +'conf': network_conf, +'test_name': 'libvirtd_network', +'test_srcdir': meson.current_source_dir(), +'test_builddir': meson.current_build_dir(), + } + virt_daemon_confs += { 'name': 'virtnetworkd', } diff --git a/src/network/network.conf.in b/src/network/network.conf.in new file mode 100644 index 00..5c84003f6d --- /dev/null +++ b/src/network/network.conf.in @@ -0,0 +1,3 @@ +# Master configuration file for the network driver. +# All settings described here are optional - if omitted, sensible +# defaults are used. diff --git a/src/network/test_libvirtd_network.aug.in b/src/network/test_libvirtd_network.aug.in new file mode 100644 index 00..ffdca520ce --- /dev/null +++ b/src/network/test_libvirtd_network.aug.in @@ -0,0 +1,2 @@ +module Test_libvirtd_network = + @CONFIG@ -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v4 13/30] network: framework to call backend-specific function to init private filter chains
Modify networkSetupPrivateChains() in the network driver to accept a firewallBackend argument so it will know which backend to call. (right now it always calls the iptables version of the lower level function, but in the future it could instead call the nftables version based on configuration). But networkSetupPrivateChains() was being called with virOnce(), and virOnce() doesn't support calling functions that require an argument (it's based on pthread_once(), which accepts no arguments, so it's not something we can easily fix in our implementation of virOnce()). To solve this dilemma, this patch eliminates use of virOnce() by adding a static lock, and putting all of networkSetupPrivateChains() (including the setting of "chainInitDone") inside a lock guard - now the places that used to call it via virOnce() can safely call it directly instead (adding in the necessary argument to specify backend). (If it turns out to be significant, we could optimize this by checking for chainInitDone outside the lock guard, returning immediately if it's already set, and then moving the setting of chainInitDone up to the top of the guarded section.) Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrangé --- src/network/bridge_driver_linux.c | 57 +-- 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c index c2ef27f251..988362abef 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -34,25 +34,45 @@ VIR_LOG_INIT("network.bridge_driver_linux"); #define PROC_NET_ROUTE "/proc/net/route" -static virOnceControl createdOnce; +static virMutex chainInitLock = VIR_MUTEX_INITIALIZER; static bool chainInitDone; /* true iff networkSetupPrivateChains was ever called */ static virErrorPtr errInitV4; static virErrorPtr errInitV6; -/* Usually only called via virOnce, but can also be called directly in - * response to firewalld reload (if chainInitDone == true) - */ -static void networkSetupPrivateChains(void) + +static int +networkFirewallSetupPrivateChains(virFirewallBackend backend, + virFirewallLayer layer) +{ +switch (backend) { +case VIR_FIREWALL_BACKEND_IPTABLES: +return iptablesSetupPrivateChains(layer); + +case VIR_FIREWALL_BACKEND_LAST: +virReportEnumRangeError(virFirewallBackend, backend); +return -1; +} +return 0; +} + + +static void +networkSetupPrivateChains(virFirewallBackend backend, + bool force) { +VIR_LOCK_GUARD lock = virLockGuardLock(); int rc; +if (chainInitDone && !force) +return; + VIR_DEBUG("Setting up global firewall chains"); g_clear_pointer(, virFreeError); g_clear_pointer(, virFreeError); -rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV4); +rc = networkFirewallSetupPrivateChains(backend, VIR_FIREWALL_LAYER_IPV4); if (rc < 0) { VIR_DEBUG("Failed to create global IPv4 chains: %s", virGetLastErrorMessage()); @@ -65,7 +85,7 @@ static void networkSetupPrivateChains(void) VIR_DEBUG("Global IPv4 chains already exist"); } -rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV6); +rc = networkFirewallSetupPrivateChains(backend, VIR_FIREWALL_LAYER_IPV6); if (rc < 0) { VIR_DEBUG("Failed to create global IPv6 chains: %s", virGetLastErrorMessage()); @@ -138,6 +158,7 @@ networkPreReloadFirewallRules(virNetworkDriverState *driver, bool startup G_GNUC_UNUSED, bool force) { +g_autoptr(virNetworkDriverConfig) cfg = virNetworkDriverGetConfig(driver); /* * If there are any running networks, we need to * create the global rules upfront. This allows us @@ -157,14 +178,14 @@ networkPreReloadFirewallRules(virNetworkDriverState *driver, */ if (chainInitDone && force) { /* The Private chains have already been initialized once - * during this run of libvirtd, so 1) we can't do it again via - * virOnce(), and 2) we need to re-add the private chains even - * if there are currently no running networks, because the - * next time a network is started, libvirt will expect that - * the chains have already been added. So we call directly - * instead of via virOnce(). + * during this run of libvirtd/virtnetworkd (known because + * chainInitDone == true) so we need to re-add the private + * chains even if there are currently no running networks, + * because the next time a network is started, libvirt will + * expect that the chains have already been added. So we force + * the init. */ -networkSetupPrivateChains(); +networkSetupPrivateChains(cfg->firewallBackend, true); } else { if
[PATCH v4 14/30] util: new functions to support adding individual firewall rollback commands
In the past virFirewall required all rollback commands for a group (those commands necessary to "undo" any rules that had been added in that group in case of a later failure) to be manually added by switching into the virFirewall object into "rollback mode" and then re-calling the inverse of the exact virFirewallAddCmd*() APIs that had been called to add the original rules (ie. for each "iptables --insert" command, for rollback we would need to add a command with all arguments identical except that "--insert" would be replaced by "--delete"). Because nftables can't search for rules to remove by comparing all the arguments (it instead expects *only* a handle that is provided via stdout when the rule was originally added), we won't be able to follow the iptables method and manually construct the command to undo any given nft command by just duplicating all the args of the command (except the action). Instead we will need to be able to automatically create a rollback command at the time the rule-adding command is executed (e.g. an "nft delete rule" command that would include the rule handle returned in stdout by an "nft add rule" command). In order to make this happen, we need to be able to 1) learn whether the user of the virFirewall API desires this behavior (handled by a new transaction flag called VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK that can be retrieved with the new virFirewallTransactionGetFlags() API), and 2) add a new command to the current group's rollback command list (with the new virFirewallAddRollbackCmd()). We will actually use this capability in an upcoming patch. Signed-off-by: Laine Stump --- src/libvirt_private.syms | 1 + src/util/virfirewall.c | 55 +++- src/util/virfirewall.h | 7 + 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 457b51e289..b3cbe1a833 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2404,6 +2404,7 @@ virFileCacheSetPriv; # util/virfirewall.h virFirewallAddCmdFull; +virFirewallAddRollbackCmd; virFirewallApply; virFirewallBackendTypeFromString; virFirewallBackendTypeToString; diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 77de34533d..9def8999d5 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -198,10 +198,12 @@ void virFirewallFree(virFirewall *firewall) fwCmd->args[fwCmd->argsLen++] = g_strdup(str); \ } while (0) + static virFirewallCmd * virFirewallAddCmdFullV(virFirewall *firewall, virFirewallLayer layer, bool ignoreErrors, + bool isRollback, virFirewallQueryCallback cb, void *opaque, va_list args) @@ -218,18 +220,16 @@ virFirewallAddCmdFullV(virFirewall *firewall, } group = firewall->groups[firewall->currentGroup]; - fwCmd = g_new0(virFirewallCmd, 1); - fwCmd->layer = layer; -fwCmd->queryCB = cb; -fwCmd->queryOpaque = opaque; while ((str = va_arg(args, char *)) != NULL) ADD_ARG(fwCmd, str); -if (group->addingRollback) { +if (isRollback || group->addingRollback) { fwCmd->ignoreErrors = true; /* always ignore errors when rolling back */ +fwCmd->queryCB = NULL; /* rollback commands can't have a callback */ +fwCmd->queryOpaque = NULL; VIR_APPEND_ELEMENT_COPY(group->rollback, group->nrollback, fwCmd); } else { /* when not rolling back, ignore errors if this group (transaction) @@ -237,6 +237,8 @@ virFirewallAddCmdFullV(virFirewall *firewall, * if this specific rule was created with ignoreErrors == true */ fwCmd->ignoreErrors = ignoreErrors || (group->actionFlags & VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS); +fwCmd->queryCB = cb; +fwCmd->queryOpaque = opaque; VIR_APPEND_ELEMENT_COPY(group->action, group->naction, fwCmd); } @@ -277,7 +279,33 @@ virFirewallCmd *virFirewallAddCmdFull(virFirewall *firewall, virFirewallCmd *fwCmd; va_list args; va_start(args, opaque); -fwCmd = virFirewallAddCmdFullV(firewall, layer, ignoreErrors, cb, opaque, args); +fwCmd = virFirewallAddCmdFullV(firewall, layer, ignoreErrors, false, cb, opaque, args); +va_end(args); +return fwCmd; +} + + +/** + * virFirewallAddRollbackCmd: + * @firewall: firewall commands to add to + * @layer: the firewall layer to change + * @...: NULL terminated list of strings for the command + * + * Add a command to the current firewall command group "rollback". + * Rollback commands always ignore errors and don't support any + * callbacks. + * + * Returns the new Command + */ +virFirewallCmd * +virFirewallAddRollbackCmd(virFirewall *firewall, + virFirewallLayer layer, + ...) +{ +virFirewallCmd *fwCmd; +va_list args; +
[PATCH v4 16/30] network: turn on auto-rollback for the rules added for virtual networks
So far this will only affect what happens if there is some failure while applying the firewall rules; the rollback rules aren't yet persistent beyond that time. More work is needed to remember the rollback rules while the network is active, and use those rules to remove the firewall for the network when it is destroyed. Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrangé --- src/network/network_iptables.c | 15 +++ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/network/network_iptables.c b/src/network/network_iptables.c index db35a4c5a0..467d43c1e9 100644 --- a/src/network/network_iptables.c +++ b/src/network/network_iptables.c @@ -1599,7 +1599,7 @@ iptablesAddFirewallRules(virNetworkDef *def) virNetworkIPDef *ipdef; g_autoptr(virFirewall) fw = virFirewallNew(VIR_FIREWALL_BACKEND_IPTABLES); -virFirewallStartTransaction(fw, 0); +virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK); iptablesAddGeneralFirewallRules(fw, def); @@ -1610,17 +1610,8 @@ iptablesAddFirewallRules(virNetworkDef *def) return -1; } -virFirewallStartRollback(fw, 0); - -for (i = 0; - (ipdef = virNetworkDefGetIPByIndex(def, AF_UNSPEC, i)); - i++) { -if (iptablesRemoveIPSpecificFirewallRules(fw, def, ipdef) < 0) -return -1; -} -iptablesRemoveGeneralFirewallRules(fw, def); - -virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS); +virFirewallStartTransaction(fw, (VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS | + VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK)); iptablesAddChecksumFirewallRules(fw, def); return virFirewallApply(fw); -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v4 00/30] [PATCH v3 00/27] native support for nftables in virtual network driver
V3: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/HOCDIMI3SHS6UFFZJUAXUE6VLXE6SVHV/ V2: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/5RTZ6PC3N3CO6X353QUHLVOL43SWQ4JD/ 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). Changes from V3: * Fixed a bug (newly added in V3) that resulted in the firewall name attribute not being added to the XML. * renamed the table to "libvirt_network" (new patch 28) * renamed the chains to be more descriptive, and lower case rather than all caps. (new patch 29) * eliminated all the guest->host and host->guest rules since they are redundant in nftables. (new patch 30) Laine Stump (30): util/network: move viriptables.[ch] from util to network directory network: move all functions manipulating iptables rules into network_iptables.c network: make all iptables functions used only in network_iptables.c static util: #define the names used for private packet filter chains util: change name of virFirewallRule to virFirewallCmd util: rename virNetFilterAction to iptablesAction, and add VIR_ENUM_DECL/IMPL util: check for 0 args when applying iptables rule util: add -w/--concurrent when applying a FirewallCmd rather than when building it util: determine ignoreErrors value when creating virFirewallCmd, not when applying util/network: new virFirewallBackend enum network: add (empty) network.conf file to distribution files network: support setting firewallBackend from network.conf network: framework to call backend-specific function to init private filter chains util: new functions to support adding individual firewall rollback commands util: implement rollback rule autocreation for iptables commands network: turn on auto-rollback for the rules added for virtual networks util: add name attribute to virFirewall util: new function virFirewallNewFromRollback() util: new functions virFirewallParseXML() and virFirewallFormat() conf: add a virFirewall object to virNetworkObj network: use previously saved list of firewall removal commands network: save network status when firewall rules are reloaded meson: stop looking for iptables/ip6tables/ebtables at build time network: add an nftables backend for network driver's firewall construction tests: test cases for nftables backend network: prefer the nftables backend over iptables spec: require either iptables or nftables if network driver is installed network: name the nftables table "libvirt_network" rather than "libvirt" network: rename chains used by network driver nftables backend network: eliminate pointless host input/output rules from nftables backend libvirt.spec.in |7 +- meson.build | 10 +- meson_options.txt |1 + po/POTFILES |3 +- src/conf/virnetworkobj.c | 41 + src/conf/virnetworkobj.h |8 + src/libvirt_private.syms | 58 +- src/network/bridge_driver.c | 39 +- src/network/bridge_driver_conf.c | 64 + src/network/bridge_driver_conf.h |3 + src/network/bridge_driver_linux.c | 630 +-- src/network/bridge_driver_nop.c |6 +- src/network/bridge_driver_platform.h |6 +- src/network/libvirtd_network.aug | 39 + src/network/meson.build | 36 + src/network/network.conf.in | 28 + src/network/network_iptables.c| 1677 + src/network/network_iptables.h| 30 + src/network/network_nftables.c| 968 ++ src/network/network_nftables.h| 28 + src/network/test_libvirtd_network.aug.in |5 + src/nwfilter/nwfilter_ebiptables_driver.c | 1004 +- src/util/meson.build |1 - src/util/virebtables.c| 36 +- src/util/virfirewall.c| 820 ++-- src/util/virfirewall.h| 87 +- src/util/viriptables.c| 1072 --- src/util/viriptables.h| 155 -- .../{base.args => base.iptables} |0 tests/networkxml2firewalldata/base.nftables | 256 +++ ...-linux.args => nat-default-linux.iptables} |0 .../nat-default-linux.nftables| 144 ++ ...pv6-linux.args => nat-ipv6-linux.iptables} |0 .../nat-ipv6-linux.nftables | 202 ++ ...rgs => nat-ipv6-masquerade-linux.iptables} |0 .../nat-ipv6-masquerade-linux.nftables| 274 +++ ...linux.args => nat-many-ips-linux.iptables} |0
[PATCH v4 06/30] util: rename virNetFilterAction to iptablesAction, and add VIR_ENUM_DECL/IMPL
I had originally named these as VIR_NETFILTER_* because I assumed the same enum would eventually be used by our nftables backend as well as iptables. But it turns out that in most cases it's not possible to delete an nftables rule, so we just never used the enum anyway, so this patch is renaming the values to IPTABLES_ACTION_*, and taking advantage of the newly defined (via VIR_ENUM_DECL/IMPL) iptablesActionTypeToString() to replace all the ternary operators used to translate the enum into a string for the iptables commandline with iptablesActionTypeToString(). Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrangé --- src/network/network_iptables.c | 126 ++--- 1 file changed, 69 insertions(+), 57 deletions(-) diff --git a/src/network/network_iptables.c b/src/network/network_iptables.c index 5c16683cff..d7e749adf0 100644 --- a/src/network/network_iptables.c +++ b/src/network/network_iptables.c @@ -32,6 +32,7 @@ #include "virerror.h" #include "virlog.h" #include "virhash.h" +#include "virenum.h" #include "network_iptables.h" VIR_LOG_INIT("network.iptables"); @@ -45,10 +46,21 @@ VIR_LOG_INIT("network.iptables"); #define VIR_IPTABLES_FWD_X_CHAIN "LIBVIRT_FWX" #define VIR_IPTABLES_NAT_POSTROUTE_CHAIN "LIBVIRT_PRT" -enum { -VIR_NETFILTER_INSERT = 0, -VIR_NETFILTER_DELETE -}; +typedef enum { +IPTABLES_ACTION_INSERT, +IPTABLES_ACTION_APPEND, +IPTABLES_ACTION_DELETE, + +IPTABLES_ACTION_LAST +} iptablesAction; + +VIR_ENUM_DECL(iptablesAction); +VIR_ENUM_IMPL(iptablesAction, + IPTABLES_ACTION_LAST, + "--insert", + "--append", + "--delete", +); typedef struct { const char *parent; @@ -168,14 +180,14 @@ iptablesInput(virFirewall *fw, virFirewallLayer layer, const char *iface, int port, - int action, + iptablesAction action, int tcp) { g_autofree char *portstr = g_strdup_printf("%d", port); virFirewallAddCmd(fw, layer, "--table", "filter", - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", + iptablesActionTypeToString(action), VIR_IPTABLES_INPUT_CHAIN, "--in-interface", iface, "--protocol", tcp ? "tcp" : "udp", @@ -189,14 +201,14 @@ iptablesOutput(virFirewall *fw, virFirewallLayer layer, const char *iface, int port, - int action, + iptablesAction action, int tcp) { g_autofree char *portstr = g_strdup_printf("%d", port); virFirewallAddCmd(fw, layer, "--table", "filter", - action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", + iptablesActionTypeToString(action), VIR_IPTABLES_OUTPUT_CHAIN, "--out-interface", iface, "--protocol", tcp ? "tcp" : "udp", @@ -220,7 +232,7 @@ iptablesAddTcpInput(virFirewall *fw, const char *iface, int port) { -iptablesInput(fw, layer, iface, port, VIR_NETFILTER_INSERT, 1); +iptablesInput(fw, layer, iface, port, IPTABLES_ACTION_INSERT, 1); } /** @@ -238,7 +250,7 @@ iptablesRemoveTcpInput(virFirewall *fw, const char *iface, int port) { -iptablesInput(fw, layer, iface, port, VIR_NETFILTER_DELETE, 1); +iptablesInput(fw, layer, iface, port, IPTABLES_ACTION_DELETE, 1); } /** @@ -256,7 +268,7 @@ iptablesAddUdpInput(virFirewall *fw, const char *iface, int port) { -iptablesInput(fw, layer, iface, port, VIR_NETFILTER_INSERT, 0); +iptablesInput(fw, layer, iface, port, IPTABLES_ACTION_INSERT, 0); } /** @@ -274,7 +286,7 @@ iptablesRemoveUdpInput(virFirewall *fw, const char *iface, int port) { -iptablesInput(fw, layer, iface, port, VIR_NETFILTER_DELETE, 0); +iptablesInput(fw, layer, iface, port, IPTABLES_ACTION_DELETE, 0); } /** @@ -292,7 +304,7 @@ iptablesAddTcpOutput(virFirewall *fw, const char *iface, int port) { -iptablesOutput(fw, layer, iface, port, VIR_NETFILTER_INSERT, 1); +iptablesOutput(fw, layer, iface, port, IPTABLES_ACTION_INSERT, 1); } /** @@ -310,7 +322,7 @@ iptablesRemoveTcpOutput(virFirewall *fw, const char *iface, int port) { -iptablesOutput(fw, layer, iface, port, VIR_NETFILTER_DELETE, 1); +iptablesOutput(fw, layer, iface, port, IPTABLES_ACTION_DELETE, 1); } /** @@ -328,7 +340,7 @@ iptablesAddUdpOutput(virFirewall *fw, const char *iface, int port) { -iptablesOutput(fw, layer, iface, port,
[PATCH v4 08/30] util: add -w/--concurrent when applying a FirewallCmd rather than when building it
We will already need a separate function for virFirewallApplyCmd for iptables vs. nftables, but the only reason for needing a separate function for virFirewallAddCmd* is that iptables/ebtables need to have an extra arg added for locking (to prevent multiple iptables commands from running at the same time). We can just as well add in the -w/--concurrent during virFirewallApplyCmd, so move the arg-add to ApplyCmd to keep AddCmd simple. Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrangé --- src/util/virfirewall.c | 27 +-- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 1897a66070..a57a79d4ce 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -213,20 +213,6 @@ virFirewallAddCmdFullV(virFirewall *firewall, fwCmd->queryOpaque = opaque; fwCmd->ignoreErrors = ignoreErrors; -switch (fwCmd->layer) { -case VIR_FIREWALL_LAYER_ETHERNET: -ADD_ARG(fwCmd, "--concurrent"); -break; -case VIR_FIREWALL_LAYER_IPV4: -ADD_ARG(fwCmd, "-w"); -break; -case VIR_FIREWALL_LAYER_IPV6: -ADD_ARG(fwCmd, "-w"); -break; -case VIR_FIREWALL_LAYER_LAST: -break; -} - while ((str = va_arg(args, char *)) != NULL) ADD_ARG(fwCmd, str); @@ -499,6 +485,19 @@ virFirewallApplyCmdDirect(virFirewallCmd *fwCmd, cmd = virCommandNewArgList(bin, NULL); +/* lock to assure nobody else is messing with the tables while we are */ +switch (fwCmd->layer) { +case VIR_FIREWALL_LAYER_ETHERNET: +virCommandAddArg(cmd, "--concurrent"); +break; +case VIR_FIREWALL_LAYER_IPV4: +case VIR_FIREWALL_LAYER_IPV6: +virCommandAddArg(cmd, "-w"); +break; +case VIR_FIREWALL_LAYER_LAST: +break; +} + for (i = 0; i < fwCmd->argsLen; i++) virCommandAddArg(cmd, fwCmd->args[i]); -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v4 04/30] util: #define the names used for private packet filter chains
Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrangé --- src/network/network_iptables.c | 51 +++--- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/src/network/network_iptables.c b/src/network/network_iptables.c index 697ad5d8d6..ac3e60b79f 100644 --- a/src/network/network_iptables.c +++ b/src/network/network_iptables.c @@ -38,6 +38,13 @@ VIR_LOG_INIT("network.iptables"); #define VIR_FROM_THIS VIR_FROM_NONE +#define VIR_IPTABLES_INPUT_CHAIN "LIBVIRT_INP" +#define VIR_IPTABLES_OUTPUT_CHAIN "LIBVIRT_OUT" +#define VIR_IPTABLES_FWD_IN_CHAIN "LIBVIRT_FWI" +#define VIR_IPTABLES_FWD_OUT_CHAIN "LIBVIRT_FWO" +#define VIR_IPTABLES_FWD_X_CHAIN "LIBVIRT_FWX" +#define VIR_IPTABLES_NAT_POSTROUTE_CHAIN "LIBVIRT_PRT" + enum { VIR_NETFILTER_INSERT = 0, VIR_NETFILTER_DELETE @@ -114,14 +121,14 @@ iptablesSetupPrivateChains(virFirewallLayer layer) { g_autoptr(virFirewall) fw = virFirewallNew(); iptablesGlobalChain filter_chains[] = { -{"INPUT", "LIBVIRT_INP"}, -{"OUTPUT", "LIBVIRT_OUT"}, -{"FORWARD", "LIBVIRT_FWO"}, -{"FORWARD", "LIBVIRT_FWI"}, -{"FORWARD", "LIBVIRT_FWX"}, +{"INPUT", VIR_IPTABLES_INPUT_CHAIN}, +{"OUTPUT", VIR_IPTABLES_OUTPUT_CHAIN}, +{"FORWARD", VIR_IPTABLES_FWD_OUT_CHAIN}, +{"FORWARD", VIR_IPTABLES_FWD_IN_CHAIN}, +{"FORWARD", VIR_IPTABLES_FWD_X_CHAIN}, }; iptablesGlobalChain natmangle_chains[] = { -{"POSTROUTING", "LIBVIRT_PRT"}, +{"POSTROUTING", VIR_IPTABLES_NAT_POSTROUTE_CHAIN}, }; bool changed = false; iptablesGlobalChainData data[] = { @@ -169,7 +176,7 @@ iptablesInput(virFirewall *fw, virFirewallAddRule(fw, layer, "--table", "filter", action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", - "LIBVIRT_INP", + VIR_IPTABLES_INPUT_CHAIN, "--in-interface", iface, "--protocol", tcp ? "tcp" : "udp", "--destination-port", portstr, @@ -190,7 +197,7 @@ iptablesOutput(virFirewall *fw, virFirewallAddRule(fw, layer, "--table", "filter", action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", - "LIBVIRT_OUT", + VIR_IPTABLES_OUTPUT_CHAIN, "--out-interface", iface, "--protocol", tcp ? "tcp" : "udp", "--destination-port", portstr, @@ -365,7 +372,7 @@ iptablesForwardAllowOut(virFirewall *fw, virFirewallAddRule(fw, layer, "--table", "filter", action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", - "LIBVIRT_FWO", + VIR_IPTABLES_FWD_OUT_CHAIN, "--source", networkstr, "--in-interface", iface, "--out-interface", physdev, @@ -375,7 +382,7 @@ iptablesForwardAllowOut(virFirewall *fw, virFirewallAddRule(fw, layer, "--table", "filter", action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", - "LIBVIRT_FWO", + VIR_IPTABLES_FWD_OUT_CHAIN, "--source", networkstr, "--in-interface", iface, "--jump", "ACCEPT", @@ -455,7 +462,7 @@ iptablesForwardAllowRelatedIn(virFirewall *fw, virFirewallAddRule(fw, layer, "--table", "filter", action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", - "LIBVIRT_FWI", + VIR_IPTABLES_FWD_IN_CHAIN, "--destination", networkstr, "--in-interface", physdev, "--out-interface", iface, @@ -467,7 +474,7 @@ iptablesForwardAllowRelatedIn(virFirewall *fw, virFirewallAddRule(fw, layer, "--table", "filter", action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", - "LIBVIRT_FWI", + VIR_IPTABLES_FWD_IN_CHAIN, "--destination", networkstr, "--out-interface", iface, "--match", "conntrack", @@ -547,7 +554,7 @@ iptablesForwardAllowIn(virFirewall *fw, virFirewallAddRule(fw, layer, "--table", "filter", action == VIR_NETFILTER_INSERT ? "--insert" : "--delete", - "LIBVIRT_FWI", + VIR_IPTABLES_FWD_IN_CHAIN, "--destination", networkstr,
[PATCH v4 03/30] network: make all iptables functions used only in network_iptables.c static
Now that the toplevel iptables functions have been moved out of the linux bridge driver into network_iptables.c, all of the utility functions are used only within that same file, so simplify it. Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrangé --- src/network/network_iptables.c | 52 ++--- src/network/network_iptables.h | 130 - 2 files changed, 26 insertions(+), 156 deletions(-) diff --git a/src/network/network_iptables.c b/src/network/network_iptables.c index 7338237c21..697ad5d8d6 100644 --- a/src/network/network_iptables.c +++ b/src/network/network_iptables.c @@ -207,7 +207,7 @@ iptablesOutput(virFirewall *fw, * Add an input to the IP table allowing access to the given @port on * the given @iface interface for TCP packets */ -void +static void iptablesAddTcpInput(virFirewall *fw, virFirewallLayer layer, const char *iface, @@ -225,7 +225,7 @@ iptablesAddTcpInput(virFirewall *fw, * Removes an input from the IP table, hence forbidding access to the given * @port on the given @iface interface for TCP packets */ -void +static void iptablesRemoveTcpInput(virFirewall *fw, virFirewallLayer layer, const char *iface, @@ -243,7 +243,7 @@ iptablesRemoveTcpInput(virFirewall *fw, * Add an input to the IP table allowing access to the given @port on * the given @iface interface for UDP packets */ -void +static void iptablesAddUdpInput(virFirewall *fw, virFirewallLayer layer, const char *iface, @@ -261,7 +261,7 @@ iptablesAddUdpInput(virFirewall *fw, * Removes an input from the IP table, hence forbidding access to the given * @port on the given @iface interface for UDP packets */ -void +static void iptablesRemoveUdpInput(virFirewall *fw, virFirewallLayer layer, const char *iface, @@ -279,7 +279,7 @@ iptablesRemoveUdpInput(virFirewall *fw, * Add an output to the IP table allowing access to the given @port from * the given @iface interface for TCP packets */ -void +static void iptablesAddTcpOutput(virFirewall *fw, virFirewallLayer layer, const char *iface, @@ -297,7 +297,7 @@ iptablesAddTcpOutput(virFirewall *fw, * Removes an output from the IP table, hence forbidding access to the given * @port from the given @iface interface for TCP packets */ -void +static void iptablesRemoveTcpOutput(virFirewall *fw, virFirewallLayer layer, const char *iface, @@ -315,7 +315,7 @@ iptablesRemoveTcpOutput(virFirewall *fw, * Add an output to the IP table allowing access to the given @port from * the given @iface interface for UDP packets */ -void +static void iptablesAddUdpOutput(virFirewall *fw, virFirewallLayer layer, const char *iface, @@ -333,7 +333,7 @@ iptablesAddUdpOutput(virFirewall *fw, * Removes an output from the IP table, hence forbidding access to the given * @port from the given @iface interface for UDP packets */ -void +static void iptablesRemoveUdpOutput(virFirewall *fw, virFirewallLayer layer, const char *iface, @@ -397,7 +397,7 @@ iptablesForwardAllowOut(virFirewall *fw, * * Returns 0 in case of success or an error code otherwise */ -int +static int iptablesAddForwardAllowOut(virFirewall *fw, virSocketAddr *netaddr, unsigned int prefix, @@ -421,7 +421,7 @@ iptablesAddForwardAllowOut(virFirewall *fw, * * Returns 0 in case of success or an error code otherwise */ -int +static int iptablesRemoveForwardAllowOut(virFirewall *fw, virSocketAddr *netaddr, unsigned int prefix, @@ -491,7 +491,7 @@ iptablesForwardAllowRelatedIn(virFirewall *fw, * * Returns 0 in case of success or an error code otherwise */ -int +static int iptablesAddForwardAllowRelatedIn(virFirewall *fw, virSocketAddr *netaddr, unsigned int prefix, @@ -515,7 +515,7 @@ iptablesAddForwardAllowRelatedIn(virFirewall *fw, * * Returns 0 in case of success or an error code otherwise */ -int +static int iptablesRemoveForwardAllowRelatedIn(virFirewall *fw, virSocketAddr *netaddr, unsigned int prefix, @@ -578,7 +578,7 @@ iptablesForwardAllowIn(virFirewall *fw, * * Returns 0 in case of success or an error code otherwise */ -int +static int iptablesAddForwardAllowIn(virFirewall *fw, virSocketAddr *netaddr, unsigned int prefix, @@ -602,7 +602,7 @@ iptablesAddForwardAllowIn(virFirewall *fw, * * Returns 0 in case of success or an error code otherwise */ -int
[PATCH v4 01/30] util/network: move viriptables.[ch] from util to network directory
These functions are only ever used by the network driver, and are so specific to the network driver's usage of iptables that they likely won't ever be used elsewhere. The files are renamed to network_iptables.[ch] to be more in line with driver-specific file naming conventions. Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrangé --- po/POTFILES | 2 +- src/libvirt_private.syms | 31 --- src/network/bridge_driver_linux.c | 2 +- src/network/meson.build | 1 + .../network_iptables.c} | 6 ++-- .../network_iptables.h} | 2 +- src/util/meson.build | 1 - 7 files changed, 7 insertions(+), 38 deletions(-) rename src/{util/viriptables.c => network/network_iptables.c} (99%) rename src/{util/viriptables.h => network/network_iptables.h} (99%) diff --git a/po/POTFILES b/po/POTFILES index 6fbff4bef2..8b89fcf832 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -144,6 +144,7 @@ src/network/bridge_driver.c src/network/bridge_driver_conf.c src/network/bridge_driver_linux.c src/network/leaseshelper.c +src/network/network_iptables.c src/node_device/node_device_driver.c src/node_device/node_device_udev.c src/nwfilter/nwfilter_dhcpsnoop.c @@ -288,7 +289,6 @@ src/util/virhostmem.c src/util/virhostuptime.c src/util/viridentity.c src/util/virinitctl.c -src/util/viriptables.c src/util/viriscsi.c src/util/virjson.c src/util/virlease.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3186dd6d23..5925c53091 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2574,37 +2574,6 @@ virIdentitySetX509DName; virInitctlFifos; virInitctlSetRunLevel; - -# util/viriptables.h -iptablesAddDontMasquerade; -iptablesAddForwardAllowCross; -iptablesAddForwardAllowIn; -iptablesAddForwardAllowOut; -iptablesAddForwardAllowRelatedIn; -iptablesAddForwardMasquerade; -iptablesAddForwardRejectIn; -iptablesAddForwardRejectOut; -iptablesAddOutputFixUdpChecksum; -iptablesAddTcpInput; -iptablesAddTcpOutput; -iptablesAddUdpInput; -iptablesAddUdpOutput; -iptablesRemoveDontMasquerade; -iptablesRemoveForwardAllowCross; -iptablesRemoveForwardAllowIn; -iptablesRemoveForwardAllowOut; -iptablesRemoveForwardAllowRelatedIn; -iptablesRemoveForwardMasquerade; -iptablesRemoveForwardRejectIn; -iptablesRemoveForwardRejectOut; -iptablesRemoveOutputFixUdpChecksum; -iptablesRemoveTcpInput; -iptablesRemoveTcpOutput; -iptablesRemoveUdpInput; -iptablesRemoveUdpOutput; -iptablesSetupPrivateChains; - - # util/viriscsi.h virISCSIConnectionLogin; virISCSIConnectionLogout; diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c index b8893bfed2..fd4bf7b61c 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -22,11 +22,11 @@ #include #include "virfile.h" -#include "viriptables.h" #include "virstring.h" #include "virlog.h" #include "virfirewall.h" #include "virfirewalld.h" +#include "network_iptables.h" #define VIR_FROM_THIS VIR_FROM_NONE diff --git a/src/network/meson.build b/src/network/meson.build index eb171ae779..305e2d52fb 100644 --- a/src/network/meson.build +++ b/src/network/meson.build @@ -2,6 +2,7 @@ network_driver_sources = [ 'bridge_driver.c', 'bridge_driver_conf.c', 'bridge_driver_platform.c', + 'network_iptables.c', ] driver_source_files += files(network_driver_sources) diff --git a/src/util/viriptables.c b/src/network/network_iptables.c similarity index 99% rename from src/util/viriptables.c rename to src/network/network_iptables.c index 018021bc1b..362c0202a0 100644 --- a/src/util/viriptables.c +++ b/src/network/network_iptables.c @@ -1,5 +1,5 @@ /* - * viriptables.c: helper APIs for managing iptables + * network_iptables.c: helper APIs for managing iptables in network driver * * Copyright (C) 2007-2014 Red Hat, Inc. * @@ -27,13 +27,13 @@ #include #include "internal.h" -#include "viriptables.h" #include "virfirewalld.h" #include "virerror.h" #include "virlog.h" #include "virhash.h" +#include "network_iptables.h" -VIR_LOG_INIT("util.iptables"); +VIR_LOG_INIT("network.iptables"); #define VIR_FROM_THIS VIR_FROM_NONE diff --git a/src/util/viriptables.h b/src/network/network_iptables.h similarity index 99% rename from src/util/viriptables.h rename to src/network/network_iptables.h index bb13f3292d..bfb6bbe0e7 100644 --- a/src/util/viriptables.h +++ b/src/network/network_iptables.h @@ -1,5 +1,5 @@ /* - * viriptables.h: helper APIs for managing iptables + * network_iptables.h: helper APIs for managing iptables in network driver * * Copyright (C) 2007, 2008 Red Hat, Inc. * diff --git a/src/util/meson.build b/src/util/meson.build index c2175f1098..896c795150 100644 --- a/src/util/meson.build +++ b/src/util/meson.build @@ -46,7 +46,6 @@ util_sources = [ 'virhostuptime.c', 'viridentity.c', 'virinitctl.c',
[PATCH v4 07/30] util: check for 0 args when applying iptables rule
In normal practice a virFirewallCmd should never have 0 args by the time it gets to the Apply stage, but at some time while debugging one of the other patches in this series, exactly that happened (due to a bug that was since squashed), and having a check for it helped debugging, so let's permanently check for it. Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrangé --- src/util/virfirewall.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 902cb8e445..1897a66070 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -538,6 +538,12 @@ virFirewallApplyCmd(virFirewall *firewall, if (fwCmd->ignoreErrors) ignoreErrors = fwCmd->ignoreErrors; +if (fwCmd->argsLen == 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Can't apply empty firewall command")); +return -1; +} + if (virFirewallApplyCmdDirect(fwCmd, ignoreErrors, ) < 0) return -1; -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v4 02/30] network: move all functions manipulating iptables rules into network_iptables.c
Although initially we will add exactly the same rules for the nftables backend, the two may (hopefully) soon diverge as we take advantage of nftables features that weren't available in iptables. When we do that, there will need to be a different version of these functions (currently in bridge_driver_linux.c) for each backend: networkAddFirewallRules() networkRemoveFirewallRules() networkSetupPrivateChains() Although it will mean duplicating some amount of code (with just the function names changed) for the nftables backend, this patch moves all of the rule-related code in the above three functions into iptables*() functions in network_iptables.c, and changes the functions in bridge_driver_linux.c to call the iptables*() functions. When we make a different backend, it will only need to make equivalents of those 3 functions publicly available to the upper layer. Signed-off-by: Laine Stump Reviewed-by: Daniel P. Berrangé --- src/network/bridge_driver_linux.c | 556 + src/network/network_iptables.c| 562 +- src/network/network_iptables.h| 7 +- 3 files changed, 574 insertions(+), 551 deletions(-) diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c index fd4bf7b61c..4914d5c903 100644 --- a/src/network/bridge_driver_linux.c +++ b/src/network/bridge_driver_linux.c @@ -301,512 +301,10 @@ int networkCheckRouteCollision(virNetworkDef *def) return 0; } -static const char networkLocalMulticastIPv4[] = "224.0.0.0/24"; -static const char networkLocalMulticastIPv6[] = "ff02::/16"; -static const char networkLocalBroadcast[] = "255.255.255.255/32"; -static int -networkAddMasqueradingFirewallRules(virFirewall *fw, -virNetworkDef *def, -virNetworkIPDef *ipdef) -{ -int prefix = virNetworkIPDefPrefix(ipdef); -const char *forwardIf = virNetworkDefForwardIf(def, 0); -bool isIPv4 = VIR_SOCKET_ADDR_IS_FAMILY(>address, AF_INET); - -if (prefix < 0) { -virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid prefix or netmask for '%1$s'"), - def->bridge); -return -1; -} - -/* allow forwarding packets from the bridge interface */ -if (iptablesAddForwardAllowOut(fw, - >address, - prefix, - def->bridge, - forwardIf) < 0) -return -1; - -/* allow forwarding packets to the bridge interface if they are - * part of an existing connection - */ -if (iptablesAddForwardAllowRelatedIn(fw, - >address, - prefix, - def->bridge, - forwardIf) < 0) -return -1; - -/* - * Enable masquerading. - * - * We need to end up with 5 rules in the table in this order - * - * 1. do not masquerade packets targeting 224.0.0.0/24 - * 2. do not masquerade packets targeting 255.255.255.255/32 - * 3. masquerade protocol=tcp with sport mapping restriction - * 4. masquerade protocol=udp with sport mapping restriction - * 5. generic, masquerade any protocol - * - * 224.0.0.0/24 is the local network multicast range. Packets are not - * forwarded outside. - * - * 255.255.255.255/32 is the broadcast address of any local network. Again, - * such packets are never forwarded, but strict DHCP clients don't accept - * DHCP replies with changed source ports. - * - * The sport mappings are required, because default IPtables - * MASQUERADE maintain port numbers unchanged where possible. - * - * NFS can be configured to only "trust" port numbers < 1023. - * - * Guests using NAT thus need to be prevented from having port - * numbers < 1023, otherwise they can bypass the NFS "security" - * check on the source port number. - * - * Since we use '--insert' to add rules to the header of the - * chain, we actually need to add them in the reverse of the - * order just mentioned ! - */ - -/* First the generic masquerade rule for other protocols */ -if (iptablesAddForwardMasquerade(fw, - >address, - prefix, - forwardIf, - >forward.addr, - >forward.port, - NULL) < 0) -return -1; - -/* UDP with a source port restriction */ -if (iptablesAddForwardMasquerade(fw, - >address, - prefix, - forwardIf, -
Re: [PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years
On Tue, Apr 30, 2024 at 12:29:14PM +0200, Thomas Huth wrote: > On 30/04/2024 11.55, Daniel P. Berrangé wrote: > > On Tue, Apr 30, 2024 at 08:45:29AM +0200, Thomas Huth wrote: > > > Old machine types often have bugs or work-arounds that affect our > > > possibilities to move forward with the QEMU code base (see for example > > > https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely > > > cannot be fixed without breaking live migration with old machine types, > > > or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or > > > commit ea985d235b86). So instead of going through the process of manually > > > deprecating old machine types again and again, let's rather add an entry > > > that can stay, which declares that machine types older than 6 years are > > > considered as deprecated automatically. Six years should be sufficient to > > > support the release cycles of most Linux distributions. > > > > Reading this again, I think we're mixing two concepts here. > > > > With this 6 year cut off, we're declaring the actual *removal* date, > > not the deprecation date. > > > > A deprecation is something that happens prior to removal normally, > > to give people a warning of /future/ removal, as a suggestion > > that they stop using it. > > > > If we never set the 'deprecation_reason' on a machine type, then > > unless someone reads this doc, they'll never realize they are on > > a deprecated machine. > > > > When it comes to machine types, I see deprecation as a way to tell > > people they should not deploy a /new/ VM on a machine type, only > > use it for back compat (incoming migration / restore from saved > > image) with existing deployed VMs. > > > > If we delete a machine on the 6 year anniversary, then users > > don't want to be deploying /new/ VMs using that on the > > 5 year anniversary as it only gives a 1 year upgrade window. > > > > So how long far back do we consider it reasonable for a user > > to deploy a /new/ VM on an old machine type ? 1 year, 2 years, > > 3 years ? > > > > > > How about picking the half way point ? 3 years ? > > > > ie, set deprecation_reason for any machine that is 3 years > > old, but declare that our deprecation cycle lasts for > > 3 years, instead of the normal 1 year, when applied to > > machine types. > > > > This would give a strong hint that users should get off the > > old machine type, several years before its finally deleted. > > Sounds like a good idea, too! Since I have to drop this patch here anyway, > could you maybe write such a new patch? (or do you want me to try to > formulate this?) Yes, I'll send something for discussion soon. 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 :| ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v2 4/4] qemu: format machine virt ras feature and test it
Resolves: https://issues.redhat.com/browse/RHEL-7489 Signed-off-by: Kristina Hanicova --- src/qemu/qemu_command.c | 5 +++ .../aarch64-features-ras.aarch64-latest.args | 31 +++ .../aarch64-features-ras.aarch64-latest.xml | 1 + .../qemuxmlconfdata/aarch64-features-ras.xml | 26 tests/qemuxmlconftest.c | 2 ++ 5 files changed, 65 insertions(+) create mode 100644 tests/qemuxmlconfdata/aarch64-features-ras.aarch64-latest.args create mode 12 tests/qemuxmlconfdata/aarch64-features-ras.aarch64-latest.xml create mode 100644 tests/qemuxmlconfdata/aarch64-features-ras.xml diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8d4442c360..807f013713 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6858,6 +6858,11 @@ qemuAppendDomainFeaturesMachineParam(virBuffer *buf, virBufferAsprintf(buf, ",cap-ibs=%s", str); } +if (def->features[VIR_DOMAIN_FEATURE_RAS] != VIR_TRISTATE_SWITCH_ABSENT) { +const char *str = virTristateSwitchTypeToString(def->features[VIR_DOMAIN_FEATURE_RAS]); +virBufferAsprintf(buf, ",ras=%s", str); +} + return 0; } diff --git a/tests/qemuxmlconfdata/aarch64-features-ras.aarch64-latest.args b/tests/qemuxmlconfdata/aarch64-features-ras.aarch64-latest.args new file mode 100644 index 00..f903d7152f --- /dev/null +++ b/tests/qemuxmlconfdata/aarch64-features-ras.aarch64-latest.args @@ -0,0 +1,31 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-aarch64test \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-aarch64test/.config \ +/usr/bin/qemu-system-aarch64 \ +-name guest=aarch64test,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-aarch64test/master-key.aes"}' \ +-machine virt,usb=off,gic-version=3,ras=on,dump-guest-core=off,memory-backend=mach-virt.ram,acpi=off \ +-accel kvm \ +-cpu host \ +-m size=1048576k \ +-object '{"qom-type":"memory-backend-ram","id":"mach-virt.ram","size":1073741824}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxmlconfdata/aarch64-features-ras.aarch64-latest.xml b/tests/qemuxmlconfdata/aarch64-features-ras.aarch64-latest.xml new file mode 12 index 00..4abae8df8c --- /dev/null +++ b/tests/qemuxmlconfdata/aarch64-features-ras.aarch64-latest.xml @@ -0,0 +1 @@ +aarch64-features-ras.xml \ No newline at end of file diff --git a/tests/qemuxmlconfdata/aarch64-features-ras.xml b/tests/qemuxmlconfdata/aarch64-features-ras.xml new file mode 100644 index 00..bbe948dc97 --- /dev/null +++ b/tests/qemuxmlconfdata/aarch64-features-ras.xml @@ -0,0 +1,26 @@ + + aarch64test + 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 + 1048576 + 1048576 + 1 + +hvm + + + + + + + + + destroy + restart + destroy + +/usr/bin/qemu-system-aarch64 + + + + + diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 6165bb6f1d..619686b9a7 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2643,6 +2643,8 @@ mymain(void) /* SVE aarch64 CPU features work on modern QEMU */ DO_TEST_CAPS_ARCH_LATEST("aarch64-features-sve", "aarch64"); +DO_TEST_CAPS_ARCH_LATEST("aarch64-features-ras", "aarch64"); + DO_TEST_CAPS_ARCH_LATEST("clock-timer-armvtimer", "aarch64"); qemuTestSetHostArch(, VIR_ARCH_NONE); -- 2.42.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v2 3/4] qemu: validate machine virt ras feature
Signed-off-by: Kristina Hanicova --- src/qemu/qemu_validate.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index b33618b494..d4ac721c06 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -69,6 +69,7 @@ qemuValidateDomainDefPSeriesFeature(const virDomainDef *def, case VIR_DOMAIN_FEATURE_HTM: case VIR_DOMAIN_FEATURE_NESTED_HV: case VIR_DOMAIN_FEATURE_CCF_ASSIST: +case VIR_DOMAIN_FEATURE_RAS: if (!virTristateSwitchTypeToString(def->features[feature])) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Invalid setting for pseries feature '%1$s'"), @@ -227,6 +228,20 @@ qemuValidateDomainDefFeatures(const virDomainDef *def, break; case VIR_DOMAIN_FEATURE_RAS: +if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT && +!qemuDomainIsARMVirt(def)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ras feature is only supported with ARM Virt machines")); +return -1; +} +if (def->features[i] != VIR_TRISTATE_SWITCH_ABSENT && +!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_VIRT_RAS)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("ras feature is not available with this QEMU binary")); +return -1; +} +break; + case VIR_DOMAIN_FEATURE_SMM: case VIR_DOMAIN_FEATURE_KVM: case VIR_DOMAIN_FEATURE_XEN: -- 2.42.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v2 1/4] qemu: introduce QEMU_CAPS_MACHINE_VIRT_RAS capability
The capability can be used to detect if the qemu binary already supports 'ras' feature for 'virt' machine type. Signed-off-by: Kristina Hanicova --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_5.2.0_aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml | 1 + tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml | 1 + 9 files changed, 10 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 21f93c6774..9da7faf27d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -706,6 +706,7 @@ VIR_ENUM_IMPL(virQEMUCaps, "blockjob.backing-mask-protocol", /* QEMU_CAPS_BLOCKJOB_BACKING_MASK_PROTOCOL */ "display-reload", /* QEMU_CAPS_DISPLAY_RELOAD */ "usb-mtp", /* QEMU_CAPS_DEVICE_USB_MTP */ + "machine.virt.ras", /* QEMU_CAPS_MACHINE_VIRT_RAS */ ); @@ -1733,6 +1734,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsMachinePropsPSeries[] = { static struct virQEMUCapsStringFlags virQEMUCapsMachinePropsVirt[] = { { "iommu", QEMU_CAPS_MACHINE_VIRT_IOMMU }, +{ "ras", QEMU_CAPS_MACHINE_VIRT_RAS }, }; static struct virQEMUCapsStringFlags virQEMUCapsMachinePropsGeneric[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 5082967cba..618584a1ed 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -685,6 +685,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_BLOCKJOB_BACKING_MASK_PROTOCOL, /* backing-mask-protocol of block-commit/block-stream */ QEMU_CAPS_DISPLAY_RELOAD, /* 'display-reload' qmp command is supported */ QEMU_CAPS_DEVICE_USB_MTP, /* -device usb-mtp */ +QEMU_CAPS_MACHINE_VIRT_RAS, /* -machine virt,ras= */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_5.2.0_aarch64.xml b/tests/qemucapabilitiesdata/caps_5.2.0_aarch64.xml index ddd30feaff..905726f7c1 100644 --- a/tests/qemucapabilitiesdata/caps_5.2.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_5.2.0_aarch64.xml @@ -125,6 +125,7 @@ + 5002000 61700243 v5.2.0 diff --git a/tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml b/tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml index 7cf7c0707b..97bbcdeb68 100644 --- a/tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.0.0_aarch64.xml @@ -136,6 +136,7 @@ + 600 61700242 v6.0.0 diff --git a/tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml b/tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml index 13253f13c0..a7a6f54019 100644 --- a/tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_6.2.0_aarch64.xml @@ -149,6 +149,7 @@ + 6001050 61700244 diff --git a/tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml b/tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml index b359f22b03..ab21278e50 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0_aarch64+hvf.xml @@ -157,6 +157,7 @@ + 6002092 61700243 v7.0.0-rc2 diff --git a/tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml b/tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml index c6f31b94fd..f53c3ddcd8 100644 --- a/tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_7.0.0_aarch64.xml @@ -157,6 +157,7 @@ + 6002092 61700243 v7.0.0-rc2 diff --git a/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml b/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml index fe4c65c9b7..c9d99f56cb 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_aarch64.xml @@ -165,6 +165,7 @@ + 8002000 61700246 v8.2.0 diff --git a/tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml b/tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml index e26dc43963..b5a1d426dc 100644 --- a/tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml +++ b/tests/qemucapabilitiesdata/caps_8.2.0_armv7l.xml @@ -172,6 +172,7 @@ + 8002000 0 qemu-8.2.0-7.fc39 -- 2.42.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v2 2/4] conf: parse and format machine virt ras feature
Signed-off-by: Kristina Hanicova --- docs/formatdomain.rst | 6 ++ src/conf/domain_conf.c| 6 +- src/conf/domain_conf.h| 1 + src/conf/schemas/domaincommon.rng | 5 + src/qemu/qemu_validate.c | 1 + 5 files changed, 18 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 0211abaa1a..89e634e16a 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2021,6 +2021,7 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off. 128 + ... @@ -2256,6 +2257,11 @@ are: Depending on the ``enabled`` attribute (values ``yes``, ``no``) enable or disable QEMU asynchronous teardown to improve memory reclaiming on a guest. :since:`Since 9.6.0` (QEMU only) +``ras`` + Report host memory errors to a guest using ACPI and guest external abort + exceptions when enabled (``on``). If the attribute is not defined, the + hypervisor default will be used. + :since:`Since 10.4.0` (QEMU/KVM and ARM virt guests only) Time keeping diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9df2b76efb..9eaec76b07 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -184,6 +184,7 @@ VIR_ENUM_IMPL(virDomainFeature, "ibs", "tcg", "async-teardown", + "ras", ); VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, @@ -16842,7 +16843,8 @@ virDomainFeaturesDefParse(virDomainDef *def, case VIR_DOMAIN_FEATURE_HTM: case VIR_DOMAIN_FEATURE_NESTED_HV: -case VIR_DOMAIN_FEATURE_CCF_ASSIST: { +case VIR_DOMAIN_FEATURE_CCF_ASSIST: +case VIR_DOMAIN_FEATURE_RAS: { virTristateSwitch state; if (virXMLPropTristateSwitch(nodes[i], "state", @@ -20689,6 +20691,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, case VIR_DOMAIN_FEATURE_HTM: case VIR_DOMAIN_FEATURE_NESTED_HV: case VIR_DOMAIN_FEATURE_CCF_ASSIST: +case VIR_DOMAIN_FEATURE_RAS: if (src->features[i] != dst->features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of feature '%1$s' differs: source: '%2$s', destination: '%3$s'"), @@ -27392,6 +27395,7 @@ virDomainDefFormatFeatures(virBuffer *buf, case VIR_DOMAIN_FEATURE_HTM: case VIR_DOMAIN_FEATURE_NESTED_HV: case VIR_DOMAIN_FEATURE_CCF_ASSIST: +case VIR_DOMAIN_FEATURE_RAS: switch ((virTristateSwitch) def->features[i]) { case VIR_TRISTATE_SWITCH_LAST: case VIR_TRISTATE_SWITCH_ABSENT: diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 356c25405b..c2abdb9f52 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2171,6 +2171,7 @@ typedef enum { VIR_DOMAIN_FEATURE_IBS, VIR_DOMAIN_FEATURE_TCG, VIR_DOMAIN_FEATURE_ASYNC_TEARDOWN, +VIR_DOMAIN_FEATURE_RAS, VIR_DOMAIN_FEATURE_LAST } virDomainFeature; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index d84e030255..86d9e391d8 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -6795,6 +6795,11 @@ + + + + + diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index b02da95c1e..b33618b494 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -226,6 +226,7 @@ qemuValidateDomainDefFeatures(const virDomainDef *def, } break; +case VIR_DOMAIN_FEATURE_RAS: case VIR_DOMAIN_FEATURE_SMM: case VIR_DOMAIN_FEATURE_KVM: case VIR_DOMAIN_FEATURE_XEN: -- 2.42.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v2 0/4] implement 'ras' feature support
This is v2 of: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/VXSUCODTU5GNLAD4L55DF3AZN7THQ7WR/ changes since v1: * edited documentation * added validation for when the feature is ON as well as OFF (the first version checked just for the case when the feature was ON) Kristina Hanicova (4): qemu: introduce QEMU_CAPS_MACHINE_VIRT_RAS capability conf: parse and format machine virt ras feature qemu: validate machine virt ras feature qemu: format machine virt ras feature and test it docs/formatdomain.rst | 6 src/conf/domain_conf.c| 6 +++- src/conf/domain_conf.h| 1 + src/conf/schemas/domaincommon.rng | 5 +++ src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 5 +++ src/qemu/qemu_validate.c | 16 ++ .../caps_5.2.0_aarch64.xml| 1 + .../caps_6.0.0_aarch64.xml| 1 + .../caps_6.2.0_aarch64.xml| 1 + .../caps_7.0.0_aarch64+hvf.xml| 1 + .../caps_7.0.0_aarch64.xml| 1 + .../caps_8.2.0_aarch64.xml| 1 + .../caps_8.2.0_armv7l.xml | 1 + .../aarch64-features-ras.aarch64-latest.args | 31 +++ .../aarch64-features-ras.aarch64-latest.xml | 1 + .../qemuxmlconfdata/aarch64-features-ras.xml | 26 tests/qemuxmlconftest.c | 2 ++ 19 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxmlconfdata/aarch64-features-ras.aarch64-latest.args create mode 12 tests/qemuxmlconfdata/aarch64-features-ras.aarch64-latest.xml create mode 100644 tests/qemuxmlconfdata/aarch64-features-ras.xml -- 2.42.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v3 5/6] migration: Remove non-multifd compression
The 'compress' migration capability enables the old compression code which has shown issues over the years and is thought to be less stable and tested than the more recent multifd-based compression. The old compression code has been deprecated in 8.2 and now is time to remove it. Deprecation commit 864128df46 ("migration: Deprecate old compression method"). Acked-by: Markus Armbruster Signed-off-by: Fabiano Rosas --- docs/about/deprecated.rst | 11 - docs/about/removed-features.rst | 55 hw/core/machine.c | 1 - migration/meson.build | 1 - migration/migration-hmp-cmds.c | 47 --- migration/migration.c | 14 - migration/migration.h | 7 - migration/options.c | 164 -- migration/options.h | 5 - migration/qemu-file.c | 78 - migration/qemu-file.h | 4 - migration/ram-compress.c| 564 migration/ram-compress.h| 77 - migration/ram.c | 154 + qapi/migration.json | 112 --- tests/qtest/migration-test.c| 139 16 files changed, 64 insertions(+), 1369 deletions(-) delete mode 100644 migration/ram-compress.c delete mode 100644 migration/ram-compress.h diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 3b9b5ba6ef..0fb5c82640 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -464,14 +464,3 @@ both, older and future versions of QEMU. The ``blacklist`` config file option has been renamed to ``block-rpcs`` (to be in sync with the renaming of the corresponding command line option). - -Migration -- - -old compression method (since 8.2) -'' - -Compression method fails too much. Too many races. We are going to -remove it if nobody fixes it. For starters, migration-test -compression tests are disabled because they fail randomly. If you need -compression, use multifd compression methods. diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index 9e7d8ee4ff..fba0cfb0b0 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -505,6 +505,11 @@ configurations (e.g. -smp 8,sockets=0) is removed since 9.0, users have to ensure that all the topology members described with -smp are greater than zero. +``-global migration.decompress-error-check`` (removed in 9.1) +' + +Removed along with the ``compression`` migration capability. + User-mode emulator command line arguments - @@ -641,6 +646,31 @@ Block migration has been removed. For a replacement, see "QMP invocation for live storage migration with ``blockdev-mirror`` + NBD" in docs/interop/live-block-operations.rst. +``migrate-set-parameter`` ``compress-level`` option (removed in 9.1) + + +Use ``multifd-zlib-level`` or ``multifd-zstd-level`` instead. + +``migrate-set-parameter`` ``compress-threads`` option (removed in 9.1) +'' + +Use ``multifd-channels`` instead. + +``migrate-set-parameter`` ``compress-wait-thread`` option (removed in 9.1) +'' + +Removed with no replacement. + +``migrate-set-parameter`` ``decompress-threads`` option (removed in 9.1) + + +Use ``multifd-channels`` instead. + +``migrate-set-capability`` ``compress`` option (removed in 9.1) +''' + +Use ``multifd-compression`` instead. + Human Monitor Protocol (HMP) commands - @@ -722,6 +752,31 @@ Block migration has been removed. For a replacement, see "QMP invocation for live storage migration with ``blockdev-mirror`` + NBD" in docs/interop/live-block-operations.rst. +``migrate_set_parameter`` ``compress-level`` option (removed in 9.1) + + +Use ``multifd-zlib-level`` or ``multifd-zstd-level`` instead. + +``migrate_set_parameter`` ``compress-threads`` option (removed in 9.1) +'' + +Use ``multifd-channels`` instead. + +``migrate_set_parameter`` ``compress-wait-thread`` option (removed in 9.1) +'' + +Removed with no replacement. + +``migrate_set_parameter`` ``decompress-threads`` option (removed in 9.1) + + +Use ``multifd-channels`` instead. + +``migrate_set_capability`` ``compress`` option (removed in 9.1) +''' +
[PATCH v3 4/6] migration: Remove block migration
The block migration has been considered obsolete since QEMU 8.2 in favor of the more flexible storage migration provided by the blockdev-mirror driver. Two releases have passed so now it's time to remove it. Deprecation commit 66db46ca83 ("migration: Deprecate block migration"). Reviewed-by: Markus Armbruster Signed-off-by: Fabiano Rosas --- MAINTAINERS |1 - docs/about/deprecated.rst | 10 - docs/about/removed-features.rst | 14 + docs/devel/migration/main.rst |2 +- include/migration/misc.h|6 - meson.build |2 - meson_options.txt |2 - migration/block.c | 1018 --- migration/block.h | 52 -- migration/colo.c|1 - migration/meson.build |3 - migration/migration-hmp-cmds.c | 25 - migration/migration.c | 15 - migration/options.c | 25 - migration/options.h |1 - migration/ram.c | 15 - migration/savevm.c |5 - qapi/migration.json | 61 +- scripts/meson-buildoptions.sh |4 - 19 files changed, 26 insertions(+), 1236 deletions(-) delete mode 100644 migration/block.c delete mode 100644 migration/block.h diff --git a/MAINTAINERS b/MAINTAINERS index 302b6fd00c..b7eaee942e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2865,7 +2865,6 @@ F: util/aio-*.h F: util/defer-call.c F: util/fdmon-*.c F: block/io.c -F: migration/block* F: include/block/aio.h F: include/block/aio-wait.h F: include/qemu/defer-call.h diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index d739358bb1..3b9b5ba6ef 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -468,16 +468,6 @@ option). Migration - -block migration (since 8.2) -''' - -Block migration is too inflexible. It needs to migrate all block -devices or none. - -Please see "QMP invocation for live storage migration with -``blockdev-mirror`` + NBD" in docs/interop/live-block-operations.rst -for a detailed explanation. - old compression method (since 8.2) '' diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index a491c0..9e7d8ee4ff 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -634,6 +634,13 @@ Use blockdev-mirror with NBD instead. See "QMP invocation for live storage migration with ``blockdev-mirror`` + NBD" in docs/interop/live-block-operations.rst for a detailed explanation. +``migrate-set-capabilities`` ``block`` option (removed in 9.1) +'' + +Block migration has been removed. For a replacement, see "QMP +invocation for live storage migration with ``blockdev-mirror`` + NBD" +in docs/interop/live-block-operations.rst. + Human Monitor Protocol (HMP) commands - @@ -708,6 +715,13 @@ Use blockdev-mirror with NBD instead. See "QMP invocation for live storage migration with ``blockdev-mirror`` + NBD" in docs/interop/live-block-operations.rst for a detailed explanation. +``migrate_set_capability`` ``block`` option (removed in 9.1) + + +Block migration has been removed. For a replacement, see "QMP +invocation for live storage migration with ``blockdev-mirror`` + NBD" +in docs/interop/live-block-operations.rst. + Host Architectures -- diff --git a/docs/devel/migration/main.rst b/docs/devel/migration/main.rst index 54385a23e5..495cdcb112 100644 --- a/docs/devel/migration/main.rst +++ b/docs/devel/migration/main.rst @@ -454,7 +454,7 @@ Examples of such API functions are: Iterative device migration -- -Some devices, such as RAM, Block storage or certain platform devices, +Some devices, such as RAM or certain platform devices, have large amounts of data that would mean that the CPUs would be paused for too long if they were sent in one section. For these devices an *iterative* approach is taken. diff --git a/include/migration/misc.h b/include/migration/misc.h index c9e200f4eb..bf7339cc1e 100644 --- a/include/migration/misc.h +++ b/include/migration/misc.h @@ -45,12 +45,6 @@ bool migrate_ram_is_ignored(RAMBlock *block); /* migration/block.c */ -#ifdef CONFIG_LIVE_BLOCK_MIGRATION -void blk_mig_init(void); -#else -static inline void blk_mig_init(void) {} -#endif - AnnounceParameters *migrate_announce_params(void); /* migration/savevm.c */ diff --git a/meson.build b/meson.build index 5db2dbc12e..31d36509d4 100644 --- a/meson.build +++ b/meson.build @@ -2351,7 +2351,6 @@ config_host_data.set('CONFIG_DEBUG_MUTEX', get_option('debug_mutex')) config_host_data.set('CONFIG_DEBUG_STACK_USAGE', get_option('debug_stack_usage')) config_host_data.set('CONFIG_DEBUG_TCG', get_option('debug_tcg'))
[PATCH v3 6/6] migration: Deprecate fd: for file migration
The fd: URI can currently trigger two different types of migration, a TCP migration using sockets and a file migration using a plain file. This is in conflict with the recently introduced (8.2) QMP migrate API that takes structured data as JSON-like format. We cannot keep the same backend for both types of migration because with the new API the code is more tightly coupled to the type of transport. This means a TCP migration must use the 'socket' transport and a file migration must use the 'file' transport. If we keep allowing fd: when using a file, this creates an issue when the user converts the old-style (fd:) to the new style ("transport": "socket") invocation because the file descriptor in question has previously been allowed to be either a plain file or a socket. To avoid creating too much confusion, we can simply deprecate the fd: + file usage, which is thought to be rarely used currently and instead establish a 1:1 correspondence between fd: URI and socket transport, and file: URI and file transport. Signed-off-by: Fabiano Rosas --- docs/about/deprecated.rst | 14 ++ migration/fd.c| 12 2 files changed, 26 insertions(+) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 0fb5c82640..813f7996fe 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -464,3 +464,17 @@ both, older and future versions of QEMU. The ``blacklist`` config file option has been renamed to ``block-rpcs`` (to be in sync with the renaming of the corresponding command line option). + +Migration +- + +``fd:`` URI when used for file migration (since 9.1) + + +The ``fd:`` URI can currently provide a file descriptor that +references either a socket or a plain file. These are two different +types of migration. In order to reduce ambiguity, the ``fd:`` URI +usage of providing a file descriptor to a plain file has been +deprecated in favor of explicitly using the ``file:`` URI with the +file descriptor being passed as an ``fdset``. Refer to the ``add-fd`` +command documentation for details on the ``fdset`` usage. diff --git a/migration/fd.c b/migration/fd.c index 449adaa2de..aab5189eac 100644 --- a/migration/fd.c +++ b/migration/fd.c @@ -20,6 +20,8 @@ #include "file.h" #include "migration.h" #include "monitor/monitor.h" +#include "qemu/error-report.h" +#include "qemu/sockets.h" #include "io/channel-util.h" #include "trace.h" @@ -32,6 +34,11 @@ void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error ** return; } +if (!fd_is_socket(fd)) { +warn_report("fd: migration to a file is deprecated." +" Use file: instead."); +} + trace_migration_fd_outgoing(fd); ioc = qio_channel_new_fd(fd, errp); if (!ioc) { @@ -61,6 +68,11 @@ void fd_start_incoming_migration(const char *fdname, Error **errp) return; } +if (!fd_is_socket(fd)) { +warn_report("fd: migration to a file is deprecated." +" Use file: instead."); +} + trace_migration_fd_incoming(fd); ioc = qio_channel_new_fd(fd, errp); -- 2.35.3 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v3 3/6] migration: Remove 'blk/-b' option from migrate commands
The block migration is considered obsolete and has been deprecated in 8.2. Remove the migrate command option that enables it. This only affects the QMP and HMP commands, the feature can still be accessed by setting the migration 'block' capability. The whole feature will be removed in a future patch. Deprecation commit 8846b5bfca ("migration: migrate 'blk' command option is deprecated."). Reviewed-by: Markus Armbruster Signed-off-by: Fabiano Rosas --- .gitlab-ci.d/buildtest.yml | 2 +- docs/about/deprecated.rst| 9 -- docs/about/removed-features.rst | 14 +++ hmp-commands.hx | 12 +-- migration/migration-hmp-cmds.c | 9 +- migration/migration.c| 31 +-- migration/migration.h| 4 - migration/options.c | 14 +-- migration/options.h | 2 - qapi/migration.json | 8 -- tests/qemu-iotests/183 | 147 --- tests/qemu-iotests/183.out | 66 -- tests/qemu-iotests/common.filter | 7 -- 13 files changed, 25 insertions(+), 300 deletions(-) delete mode 100755 tests/qemu-iotests/183 delete mode 100644 tests/qemu-iotests/183.out diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml index 6394b8f41e..3d975f8c34 100644 --- a/.gitlab-ci.d/buildtest.yml +++ b/.gitlab-ci.d/buildtest.yml @@ -342,7 +342,7 @@ build-tcg-disabled: - cd tests/qemu-iotests/ - ./check -raw 001 002 003 004 005 008 009 010 011 012 021 025 032 033 048 052 063 077 086 101 104 106 113 148 150 151 152 157 159 160 163 -170 171 183 184 192 194 208 221 226 227 236 253 277 image-fleecing +170 171 184 192 194 208 221 226 227 236 253 277 image-fleecing - ./check -qcow2 028 051 056 057 058 065 068 082 085 091 095 096 102 122 124 132 139 142 144 145 151 152 155 157 165 194 196 200 202 208 209 216 218 227 234 246 247 248 250 254 255 257 258 diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index ebe53821ed..d739358bb1 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -468,15 +468,6 @@ option). Migration - -``blk`` migrate command option (since 8.2) -'' - -Use blockdev-mirror with NBD instead. - -As an intermediate step the ``blk`` functionality can be achieved by -setting the ``block`` migration capability to ``true``. But this -capability is also deprecated. - block migration (since 8.2) ''' diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index 7da4b3df14..a491c0 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -627,6 +627,13 @@ Use blockdev-mirror with NBD instead. See "QMP invocation for live storage migration with ``blockdev-mirror`` + NBD" in docs/interop/live-block-operations.rst for a detailed explanation. +``migrate`` command option ``blk`` (removed in 9.1) +''' + +Use blockdev-mirror with NBD instead. See "QMP invocation for live +storage migration with ``blockdev-mirror`` + NBD" in +docs/interop/live-block-operations.rst for a detailed explanation. + Human Monitor Protocol (HMP) commands - @@ -694,6 +701,13 @@ Use blockdev-mirror with NBD instead. See "QMP invocation for live storage migration with ``blockdev-mirror`` + NBD" in docs/interop/live-block-operations.rst for a detailed explanation. +``migrate`` command ``-b`` option (removed in 9.1) +'' + +Use blockdev-mirror with NBD instead. See "QMP invocation for live +storage migration with ``blockdev-mirror`` + NBD" in +docs/interop/live-block-operations.rst for a detailed explanation. + Host Architectures -- diff --git a/hmp-commands.hx b/hmp-commands.hx index 7978302949..ebca2cdced 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -909,21 +909,17 @@ ERST { .name = "migrate", -.args_type = "detach:-d,blk:-b,resume:-r,uri:s", -.params = "[-d] [-b] [-r] uri", +.args_type = "detach:-d,resume:-r,uri:s", +.params = "[-d] [-r] uri", .help = "migrate to URI (using -d to not wait for completion)" - "\n\t\t\t -b for migration without shared storage with" - " full copy of disk\n\t\t\t -r to resume a paused migration", + "\n\t\t\t -r to resume a paused migration", .cmd= hmp_migrate, }, SRST -``migrate [-d] [-b]`` *uri* +``migrate [-d]`` *uri* Migrate to *uri* (using -d to not wait for completion). - - ``-b`` -for migration with full copy of disk ERST { diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c index f49f061be1..5ab204d91d 100644 --- a/migration/migration-hmp-cmds.c +++
[PATCH v3 2/6] migration: Remove 'inc' option from migrate command
The block incremental option for block migration has been deprecated in 8.2 in favor of using the block-mirror feature. Remove it now. Deprecation commit 40101f320d ("migration: migrate 'inc' command option is deprecated."). Reviewed-by: Markus Armbruster Signed-off-by: Fabiano Rosas --- docs/about/deprecated.rst | 9 -- docs/about/removed-features.rst | 14 + hmp-commands.hx | 13 +++- migration/block.c | 1 - migration/migration-hmp-cmds.c | 18 ++- migration/migration.c | 24 +-- migration/options.c | 30 +- migration/options.h | 5 --- qapi/migration.json | 54 +++-- 9 files changed, 39 insertions(+), 129 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 1c03a358d1..ebe53821ed 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -468,15 +468,6 @@ option). Migration - -``inc`` migrate command option (since 8.2) -'' - -Use blockdev-mirror with NBD instead. - -As an intermediate step the ``inc`` functionality can be achieved by -setting the ``block-incremental`` migration parameter to ``true``. -But this parameter is also deprecated. - ``blk`` migrate command option (since 8.2) '' diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index c4cb2692d0..7da4b3df14 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -620,6 +620,13 @@ was superseded by ``sections``. Member ``skipped`` of the ``MigrationStats`` struct hasn't been used for more than 10 years. Removed with no replacement. +``migrate`` command option ``inc`` (removed in 9.1) +''' + +Use blockdev-mirror with NBD instead. See "QMP invocation for live +storage migration with ``blockdev-mirror`` + NBD" in +docs/interop/live-block-operations.rst for a detailed explanation. + Human Monitor Protocol (HMP) commands - @@ -680,6 +687,13 @@ This command didn't produce any output already. Removed with no replacement. The ``singlestep`` command has been replaced by the ``one-insn-per-tb`` command, which has the same behaviour but a less misleading name. +``migrate`` command ``-i`` option (removed in 9.1) +'' + +Use blockdev-mirror with NBD instead. See "QMP invocation for live +storage migration with ``blockdev-mirror`` + NBD" in +docs/interop/live-block-operations.rst for a detailed explanation. + Host Architectures -- diff --git a/hmp-commands.hx b/hmp-commands.hx index 2e2a3bcf98..7978302949 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -909,26 +909,21 @@ ERST { .name = "migrate", -.args_type = "detach:-d,blk:-b,inc:-i,resume:-r,uri:s", -.params = "[-d] [-b] [-i] [-r] uri", +.args_type = "detach:-d,blk:-b,resume:-r,uri:s", +.params = "[-d] [-b] [-r] uri", .help = "migrate to URI (using -d to not wait for completion)" "\n\t\t\t -b for migration without shared storage with" - " full copy of disk\n\t\t\t -i for migration without " - "shared storage with incremental copy of disk " - "(base image shared between src and destination)" - "\n\t\t\t -r to resume a paused migration", + " full copy of disk\n\t\t\t -r to resume a paused migration", .cmd= hmp_migrate, }, SRST -``migrate [-d] [-b] [-i]`` *uri* +``migrate [-d] [-b]`` *uri* Migrate to *uri* (using -d to not wait for completion). ``-b`` for migration with full copy of disk - ``-i`` -for migration with incremental copy of disk (base image is shared) ERST { diff --git a/migration/block.c b/migration/block.c index bae6e94891..87ec1a7e68 100644 --- a/migration/block.c +++ b/migration/block.c @@ -419,7 +419,6 @@ static int init_blk_migration(QEMUFile *f, Error **errp) bmds->bulk_completed = 0; bmds->total_sectors = sectors; bmds->completed_sectors = 0; -bmds->shared_base = migrate_block_incremental(); assert(i < num_bs); bmds_bs[i].bmds = bmds; diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c index 28f776d06d..f49f061be1 100644 --- a/migration/migration-hmp-cmds.c +++ b/migration/migration-hmp-cmds.c @@ -332,10 +332,6 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict) monitor_printf(mon, "%s: %u ms\n", MigrationParameter_str(MIGRATION_PARAMETER_X_CHECKPOINT_DELAY), params->x_checkpoint_delay); -assert(params->has_block_incremental); -monitor_printf(mon, "%s:
[PATCH v3 1/6] migration: Remove 'skipped' field from MigrationStats
The 'skipped' field of the MigrationStats struct has been deprecated in 8.1. Time to remove it. Deprecation commit 7b24d32634 ("migration: skipped field is really obsolete."). Reviewed-by: Markus Armbruster Reviewed-by: Peter Xu Signed-off-by: Fabiano Rosas --- docs/about/deprecated.rst | 6 -- docs/about/removed-features.rst | 6 ++ migration/migration-hmp-cmds.c | 2 -- migration/migration.c | 2 -- qapi/migration.json | 8 5 files changed, 6 insertions(+), 18 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 7b8aafa15b..1c03a358d1 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -468,12 +468,6 @@ option). Migration - -``skipped`` MigrationStats field (since 8.1) - - -``skipped`` field in Migration stats has been deprecated. It hasn't -been used for more than 10 years. - ``inc`` migrate command option (since 8.2) '' diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index 53ca08aba9..c4cb2692d0 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -614,6 +614,12 @@ was superseded by ``sections``. Member ``section-size`` in the return value of ``query-sgx-capabilities`` was superseded by ``sections``. +``query-migrate`` return value member ``skipped`` (removed in 9.1) +'' + +Member ``skipped`` of the ``MigrationStats`` struct hasn't been used +for more than 10 years. Removed with no replacement. + Human Monitor Protocol (HMP) commands - diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c index 7e96ae6ffd..28f776d06d 100644 --- a/migration/migration-hmp-cmds.c +++ b/migration/migration-hmp-cmds.c @@ -105,8 +105,6 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict) info->ram->total >> 10); monitor_printf(mon, "duplicate: %" PRIu64 " pages\n", info->ram->duplicate); -monitor_printf(mon, "skipped: %" PRIu64 " pages\n", - info->ram->skipped); monitor_printf(mon, "normal: %" PRIu64 " pages\n", info->ram->normal); monitor_printf(mon, "normal bytes: %" PRIu64 " kbytes\n", diff --git a/migration/migration.c b/migration/migration.c index b5af6b5105..266652668d 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1149,8 +1149,6 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s) info->ram->transferred = migration_transferred_bytes(); info->ram->total = ram_bytes_total(); info->ram->duplicate = stat64_get(_stats.zero_pages); -/* legacy value. It is not used anymore */ -info->ram->skipped = 0; info->ram->normal = stat64_get(_stats.normal_pages); info->ram->normal_bytes = info->ram->normal * page_size; info->ram->mbps = s->mbps; diff --git a/qapi/migration.json b/qapi/migration.json index 8c65b90328..401b8e24ac 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -23,9 +23,6 @@ # # @duplicate: number of duplicate (zero) pages (since 1.2) # -# @skipped: number of skipped zero pages. Always zero, only provided -# for compatibility (since 1.5) -# # @normal: number of normal pages (since 1.2) # # @normal-bytes: number of normal bytes sent (since 1.2) @@ -63,16 +60,11 @@ # between 0 and @dirty-sync-count * @multifd-channels. (since # 7.1) # -# Features: -# -# @deprecated: Member @skipped is always zero since 1.5.3 -# # Since: 0.14 ## { 'struct': 'MigrationStats', 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , 'duplicate': 'int', - 'skipped': { 'type': 'int', 'features': [ 'deprecated' ] }, 'normal': 'int', 'normal-bytes': 'int', 'dirty-pages-rate': 'int', 'mbps': 'number', 'dirty-sync-count': 'int', -- 2.35.3 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH v3 0/6] migration removals & deprecations
since v2: - removed some more stuff which I missed: blk/inc options from hmp-commands.hx the entire ram-compress.h unused declarations from options.h unused compression functions from qemu-file.c - removed must_remove_block_options earlier in the 'blk' patch - added a deprecation warning to outgoing/incoming fd CI run: https://gitlab.com/farosas/qemu/-/pipelines/1272385260 v2: https://lore.kernel.org/r/20240426131408.25410-1-faro...@suse.de v1: https://lore.kernel.org/r/20240425150939.19268-1-faro...@suse.de Hi everyone, Here's some cleaning up of deprecated code. It removes the old block migration and compression code. Both have suitable replacements in the form of the blockdev-mirror driver and multifd compression, respectively. There's also a deprecation for fd: + file to cope with the fact that the new MigrationAddress API defines transports instead of protocols (loose terms) like the old string API did. So we cannot map 1:1 from fd: to any transport because fd: allows *both* file migration and socket migration. Fabiano Rosas (6): migration: Remove 'skipped' field from MigrationStats migration: Remove 'inc' option from migrate command migration: Remove 'blk/-b' option from migrate commands migration: Remove block migration migration: Remove non-multifd compression migration: Deprecate fd: for file migration .gitlab-ci.d/buildtest.yml |2 +- MAINTAINERS |1 - docs/about/deprecated.rst| 51 +- docs/about/removed-features.rst | 103 +++ docs/devel/migration/main.rst|2 +- hmp-commands.hx | 17 +- hw/core/machine.c|1 - include/migration/misc.h |6 - meson.build |2 - meson_options.txt|2 - migration/block.c| 1019 -- migration/block.h| 52 -- migration/colo.c |1 - migration/fd.c | 12 + migration/meson.build|4 - migration/migration-hmp-cmds.c | 97 +-- migration/migration.c| 70 +- migration/migration.h| 11 - migration/options.c | 229 --- migration/options.h | 13 - migration/qemu-file.c| 78 --- migration/qemu-file.h|4 - migration/ram-compress.c | 564 - migration/ram-compress.h | 77 --- migration/ram.c | 169 + migration/savevm.c |5 - qapi/migration.json | 205 +- scripts/meson-buildoptions.sh|4 - tests/qemu-iotests/183 | 147 - tests/qemu-iotests/183.out | 66 -- tests/qemu-iotests/common.filter |7 - tests/qtest/migration-test.c | 139 32 files changed, 147 insertions(+), 3013 deletions(-) delete mode 100644 migration/block.c delete mode 100644 migration/block.h delete mode 100644 migration/ram-compress.c delete mode 100644 migration/ram-compress.h delete mode 100755 tests/qemu-iotests/183 delete mode 100644 tests/qemu-iotests/183.out base-commit: fd87be1dada5672f877e03c2ca8504458292c479 -- 2.35.3 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH] NEWS: Document my contributions for upcoming release
Signed-off-by: Michal Privoznik --- NEWS.rst | 22 ++ 1 file changed, 22 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 852dadf532..4958326875 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,8 +17,20 @@ v10.3.0 (unreleased) * **New features** + * conf: Introduce memReserve to + +Some PCI devices have large non-prefetchable memory. This is not a problem +for coldplug because firmware sets up such devices properly. But it may be +a problem for hotplug devices. To resolve this, new ``memReserve`` +attribute is introduced which allows overriding value computed by firmware. + * **Improvements** + * Extend libvirt-guests to shutdown only persistent VMs + +Users can now chose to shutdown only persistent VMs when the host is being +shut down. + * **Bug fixes** * qemu: Fix migration with custom XML @@ -29,6 +41,16 @@ v10.3.0 (unreleased) depends on the guest CPU definition and the host on which a particular domain was running. + * Create OVS ports as transient + +Libvirt now creates OVS ports as transient which prevents them from +reappearing or going stale on sudden reboots. + + * Clear OVS QoS settings when domain shuts down + +Libvirt now clears QoS settings on domain shutdown, so they no longer pile +up in OVS database. + v10.2.0 (2024-04-02) -- 2.43.2 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years
On 30/04/2024 11.55, Daniel P. Berrangé wrote: On Tue, Apr 30, 2024 at 08:45:29AM +0200, Thomas Huth wrote: Old machine types often have bugs or work-arounds that affect our possibilities to move forward with the QEMU code base (see for example https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely cannot be fixed without breaking live migration with old machine types, or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or commit ea985d235b86). So instead of going through the process of manually deprecating old machine types again and again, let's rather add an entry that can stay, which declares that machine types older than 6 years are considered as deprecated automatically. Six years should be sufficient to support the release cycles of most Linux distributions. Reading this again, I think we're mixing two concepts here. With this 6 year cut off, we're declaring the actual *removal* date, not the deprecation date. A deprecation is something that happens prior to removal normally, to give people a warning of /future/ removal, as a suggestion that they stop using it. If we never set the 'deprecation_reason' on a machine type, then unless someone reads this doc, they'll never realize they are on a deprecated machine. When it comes to machine types, I see deprecation as a way to tell people they should not deploy a /new/ VM on a machine type, only use it for back compat (incoming migration / restore from saved image) with existing deployed VMs. If we delete a machine on the 6 year anniversary, then users don't want to be deploying /new/ VMs using that on the 5 year anniversary as it only gives a 1 year upgrade window. So how long far back do we consider it reasonable for a user to deploy a /new/ VM on an old machine type ? 1 year, 2 years, 3 years ? How about picking the half way point ? 3 years ? ie, set deprecation_reason for any machine that is 3 years old, but declare that our deprecation cycle lasts for 3 years, instead of the normal 1 year, when applied to machine types. This would give a strong hint that users should get off the old machine type, several years before its finally deleted. Sounds like a good idea, too! Since I have to drop this patch here anyway, could you maybe write such a new patch? (or do you want me to try to formulate this?) Thomas ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years
On Tue, Apr 30, 2024 at 10:55:38AM +0100, Daniel P. Berrangé wrote: > On Tue, Apr 30, 2024 at 08:45:29AM +0200, Thomas Huth wrote: > > Old machine types often have bugs or work-arounds that affect our > > possibilities to move forward with the QEMU code base (see for example > > https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely > > cannot be fixed without breaking live migration with old machine types, > > or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or > > commit ea985d235b86). So instead of going through the process of manually > > deprecating old machine types again and again, let's rather add an entry > > that can stay, which declares that machine types older than 6 years are > > considered as deprecated automatically. Six years should be sufficient to > > support the release cycles of most Linux distributions. > > Reading this again, I think we're mixing two concepts here. > > With this 6 year cut off, we're declaring the actual *removal* date, > not the deprecation date. > > A deprecation is something that happens prior to removal normally, > to give people a warning of /future/ removal, as a suggestion > that they stop using it. > > If we never set the 'deprecation_reason' on a machine type, then > unless someone reads this doc, they'll never realize they are on > a deprecated machine. > > When it comes to machine types, I see deprecation as a way to tell > people they should not deploy a /new/ VM on a machine type, only > use it for back compat (incoming migration / restore from saved > image) with existing deployed VMs. > > If we delete a machine on the 6 year anniversary, then users > don't want to be deploying /new/ VMs using that on the > 5 year anniversary as it only gives a 1 year upgrade window. > > So how long far back do we consider it reasonable for a user > to deploy a /new/ VM on an old machine type ? 1 year, 2 years, > 3 years ? > > > How about picking the half way point ? 3 years ? > > ie, set deprecation_reason for any machine that is 3 years > old, but declare that our deprecation cycle lasts for > 3 years, instead of the normal 1 year, when applied to > machine types. > > This would give a strong hint that users should get off the > old machine type, several years before its finally deleted. The m68k/arm archs have a nice macro for defining versions that exposes major/minor directly. That would let us automatically set the deprecation flag after 3 years, avoiding manually writing patches for each release: diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 3c93c0c0a6..e40209f60a 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -101,6 +101,11 @@ static void arm_virt_compat_set(MachineClass *mc) arm_virt_compat_len); } +#define MACHINE_IS_DEPRECATED(major, minor) \ +((QEMU_VERSION_MAJOR - major) > 3 || \ + ((QEMU_VERSION_MAJOR - major) == 3 && \ + (QEMU_VERSION_MINOR - minor) > 0)) + #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \ static void virt_##major##_##minor##_class_init(ObjectClass *oc, \ void *data) \ @@ -109,6 +114,9 @@ static void arm_virt_compat_set(MachineClass *mc) arm_virt_compat_set(mc); \ virt_machine_##major##_##minor##_options(mc); \ mc->desc = "QEMU " # major "." # minor " ARM Virtual Machine"; \ +if (MACHINE_IS_DEPRECATED(major, minor)) { \ +mc->deprecation_reason = "machine virt-" # major "." # minor " is not recommended for newly deployed VMs"; \ +} \ if (latest) { \ mc->alias = "virt"; \ } \ we could easily change other arches to enable the same thing. Then all we need do manually is the actual deletion. We would make it a BUILD_BUG_ON after say 20 releases to force us to remember the actual deletion at the 6 year point, without creating an immediate build fail in that exact 18th release cycle. 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 :| ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years
On 30/04/2024 11.40, Philippe Mathieu-Daudé wrote: Hi Thomas, On 30/4/24 08:45, Thomas Huth wrote: Old machine types often have bugs or work-arounds that affect our possibilities to move forward with the QEMU code base (see for example https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely cannot be fixed without breaking live migration with old machine types, or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or commit ea985d235b86). So instead of going through the process of manually deprecating old machine types again and again, let's rather add an entry that can stay, which declares that machine types older than 6 years are considered as deprecated automatically. Six years should be sufficient to support the release cycles of most Linux distributions. Thanks for taking that out of my plate :) IIRC 6 years was because of some old RHEL version, otherwise could 5 years be enough? (maybe it could be good enough for this old RHEL version as of QEMU v10.0). My thinking was like this: 1) We recently talked about marking all machine types up to 2.12 as deprecated recently. 2.12 has been released in 2018, so if we feel confident that those old machine types could go away now/soon, 2024 - 2018 = 6 years seem to be a good rule of thumb. 2) RHEL and OpenSuSE tend to provide feature updates in their enterprise distros for 5 (RHEL) or 6 (current OpenSuSE AFAIK) years, so keeping old machine types in upstream QEMU for that amount of time certainly also helps with downstream distros. diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 6d595de3b6..fe69e2d44c 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -220,6 +220,17 @@ is a chance the code will bitrot without anyone noticing. System emulator machines +Versioned machine types older than 6 years +'' + +Starting with the release of QEMU 10.0, versioned machine types older than Why can't we start with QEMU 9.1? If I put 9.1 now there, that would make it possible to immediately remove machines in 9.2, i.e. we would skip the must-be-marked-as-deprecated-for-two-releases rule here. Thomas ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years
On Tue, Apr 30, 2024 at 11:40:42AM +0200, Philippe Mathieu-Daudé wrote: > Hi Thomas, > > On 30/4/24 08:45, Thomas Huth wrote: > > Old machine types often have bugs or work-arounds that affect our > > possibilities to move forward with the QEMU code base (see for example > > https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely > > cannot be fixed without breaking live migration with old machine types, > > or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or > > commit ea985d235b86). So instead of going through the process of manually > > deprecating old machine types again and again, let's rather add an entry > > that can stay, which declares that machine types older than 6 years are > > considered as deprecated automatically. Six years should be sufficient to > > support the release cycles of most Linux distributions. > > Thanks for taking that out of my plate :) > > IIRC 6 years was because of some old RHEL version, otherwise could > 5 years be enough? (maybe it could be good enough for this old RHEL > version as of QEMU v10.0). With my RHEL hat on, 6 years gives an approximate alignment with RHEL lifecycles, which have ended up being roughly 3 years apart. RHEL-N, wants to support all machine types from RHEL-(N-1), so in the worst case that gives up to 6 years worth of QEMU versions. 5 years could well be enough in many cases, depending on how frequently RHEL rebases QEMU, but could also trip us up if the timelines do something unexpected. So 6 years is a bit safer from Red Hat's POV, if the community is willing. > > Signed-off-by: Thomas Huth > > --- > > docs/about/deprecated.rst | 11 +++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > > index 6d595de3b6..fe69e2d44c 100644 > > --- a/docs/about/deprecated.rst > > +++ b/docs/about/deprecated.rst > > @@ -220,6 +220,17 @@ is a chance the code will bitrot without anyone > > noticing. > > System emulator machines > > > > +Versioned machine types older than 6 years > > +'' > > + > > +Starting with the release of QEMU 10.0, versioned machine types older than > > Why can't we start with QEMU 9.1? > > > +6 years will automatically be considered as deprecated and might be due to > > +removal without furthor notice. For example, this affects machine types > > like > > +pc-i440fx-X.Y, pc-q35-X.Y, pseries-X.Y, s390-ccw-virtio-X.Y or virt-X.Y > > where > > +X is the major number and Y is the minor number of the old QEMU version. > > +If you are still using machine types from QEMU versions older than 6 years, > > +please update your setting to use a newer versioned machine type instead. > > Reviewed-by: Philippe Mathieu-Daudé > 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 :| ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years
On 4/30/24 11:45, Daniel P. Berrangé wrote: On Tue, Apr 30, 2024 at 08:45:29AM +0200, Thomas Huth wrote: Old machine types often have bugs or work-arounds that affect our possibilities to move forward with the QEMU code base (see for example https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely cannot be fixed without breaking live migration with old machine types, or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or commit ea985d235b86). So instead of going through the process of manually deprecating old machine types again and again, let's rather add an entry that can stay, which declares that machine types older than 6 years are considered as deprecated automatically. Six years should be sufficient to support the release cycles of most Linux distributions. If anyone thinks 6 years is not very long, consider that this implies QEMU will be maintaining *18* versions for each versioned machine type. So across aarch64 'virt', x86 'pc' & 'q35', ppc 'spapr', s390x 'ccw', and m68k 'virt', that's upto 108 machines we're keeping ABI preserved for in the worst case... We will probably have RISC-V machines to support also. Anyhow, 6 years looks good to me. Thanks, C. ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years
On Tue, Apr 30, 2024 at 08:45:29AM +0200, Thomas Huth wrote: > Old machine types often have bugs or work-arounds that affect our > possibilities to move forward with the QEMU code base (see for example > https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely > cannot be fixed without breaking live migration with old machine types, > or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or > commit ea985d235b86). So instead of going through the process of manually > deprecating old machine types again and again, let's rather add an entry > that can stay, which declares that machine types older than 6 years are > considered as deprecated automatically. Six years should be sufficient to > support the release cycles of most Linux distributions. Reading this again, I think we're mixing two concepts here. With this 6 year cut off, we're declaring the actual *removal* date, not the deprecation date. A deprecation is something that happens prior to removal normally, to give people a warning of /future/ removal, as a suggestion that they stop using it. If we never set the 'deprecation_reason' on a machine type, then unless someone reads this doc, they'll never realize they are on a deprecated machine. When it comes to machine types, I see deprecation as a way to tell people they should not deploy a /new/ VM on a machine type, only use it for back compat (incoming migration / restore from saved image) with existing deployed VMs. If we delete a machine on the 6 year anniversary, then users don't want to be deploying /new/ VMs using that on the 5 year anniversary as it only gives a 1 year upgrade window. So how long far back do we consider it reasonable for a user to deploy a /new/ VM on an old machine type ? 1 year, 2 years, 3 years ? How about picking the half way point ? 3 years ? ie, set deprecation_reason for any machine that is 3 years old, but declare that our deprecation cycle lasts for 3 years, instead of the normal 1 year, when applied to machine types. This would give a strong hint that users should get off the old machine type, several years before its finally deleted. > Signed-off-by: Thomas Huth > --- > docs/about/deprecated.rst | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 6d595de3b6..fe69e2d44c 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -220,6 +220,17 @@ is a chance the code will bitrot without anyone noticing. > System emulator machines > > > +Versioned machine types older than 6 years > +'' > + > +Starting with the release of QEMU 10.0, versioned machine types older than > +6 years will automatically be considered as deprecated and might be due to > +removal without furthor notice. For example, this affects machine types like > +pc-i440fx-X.Y, pc-q35-X.Y, pseries-X.Y, s390-ccw-virtio-X.Y or virt-X.Y where > +X is the major number and Y is the minor number of the old QEMU version. > +If you are still using machine types from QEMU versions older than 6 years, > +please update your setting to use a newer versioned machine type instead. > + > Arm ``virt`` machine ``dtb-kaslr-seed`` property (since 7.1) > > > -- > 2.44.0 > 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 :| ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years
On Tue, Apr 30, 2024 at 08:45:29AM +0200, Thomas Huth wrote: > Old machine types often have bugs or work-arounds that affect our > possibilities to move forward with the QEMU code base (see for example > https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely > cannot be fixed without breaking live migration with old machine types, > or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or > commit ea985d235b86). So instead of going through the process of manually > deprecating old machine types again and again, let's rather add an entry > that can stay, which declares that machine types older than 6 years are > considered as deprecated automatically. Six years should be sufficient to > support the release cycles of most Linux distributions. If anyone thinks 6 years is not very long, consider that this implies QEMU will be maintaining *18* versions for each versioned machine type. So across aarch64 'virt', x86 'pc' & 'q35', ppc 'spapr', s390x 'ccw', and m68k 'virt', that's upto 108 machines we're keeping ABI preserved for in the worst case... > > Signed-off-by: Thomas Huth > --- > docs/about/deprecated.rst | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 6d595de3b6..fe69e2d44c 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -220,6 +220,17 @@ is a chance the code will bitrot without anyone noticing. > System emulator machines > > > +Versioned machine types older than 6 years > +'' > + > +Starting with the release of QEMU 10.0, versioned machine types older than > +6 years will automatically be considered as deprecated and might be due to How about: s/6 years/6 years (equivalent to 18 releases)/ Also s/as deprecated/deprecated/ s/due to/liable for/ > +removal without furthor notice. For example, this affects machine types like > +pc-i440fx-X.Y, pc-q35-X.Y, pseries-X.Y, s390-ccw-virtio-X.Y or virt-X.Y where > +X is the major number and Y is the minor number of the old QEMU version. > +If you are still using machine types from QEMU versions older than 6 years, Again s/6 years/6 years (equivalent to 18 releases)/ > +please update your setting to use a newer versioned machine type instead. s/setting/configuration/ > + > Arm ``virt`` machine ``dtb-kaslr-seed`` property (since 7.1) > 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 :| ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years
Hi Thomas, On 30/4/24 08:45, Thomas Huth wrote: Old machine types often have bugs or work-arounds that affect our possibilities to move forward with the QEMU code base (see for example https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely cannot be fixed without breaking live migration with old machine types, or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or commit ea985d235b86). So instead of going through the process of manually deprecating old machine types again and again, let's rather add an entry that can stay, which declares that machine types older than 6 years are considered as deprecated automatically. Six years should be sufficient to support the release cycles of most Linux distributions. Thanks for taking that out of my plate :) IIRC 6 years was because of some old RHEL version, otherwise could 5 years be enough? (maybe it could be good enough for this old RHEL version as of QEMU v10.0). Signed-off-by: Thomas Huth --- docs/about/deprecated.rst | 11 +++ 1 file changed, 11 insertions(+) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 6d595de3b6..fe69e2d44c 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -220,6 +220,17 @@ is a chance the code will bitrot without anyone noticing. System emulator machines +Versioned machine types older than 6 years +'' + +Starting with the release of QEMU 10.0, versioned machine types older than Why can't we start with QEMU 9.1? +6 years will automatically be considered as deprecated and might be due to +removal without furthor notice. For example, this affects machine types like +pc-i440fx-X.Y, pc-q35-X.Y, pseries-X.Y, s390-ccw-virtio-X.Y or virt-X.Y where +X is the major number and Y is the minor number of the old QEMU version. +If you are still using machine types from QEMU versions older than 6 years, +please update your setting to use a newer versioned machine type instead. Reviewed-by: Philippe Mathieu-Daudé ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years
On Tue, 30 Apr 2024 at 07:45, Thomas Huth wrote: > > Old machine types often have bugs or work-arounds that affect our > possibilities to move forward with the QEMU code base (see for example > https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely > cannot be fixed without breaking live migration with old machine types, > or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or > commit ea985d235b86). So instead of going through the process of manually > deprecating old machine types again and again, let's rather add an entry > that can stay, which declares that machine types older than 6 years are > considered as deprecated automatically. Six years should be sufficient to > support the release cycles of most Linux distributions. Sounds like a good idea. > Signed-off-by: Thomas Huth > --- > docs/about/deprecated.rst | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 6d595de3b6..fe69e2d44c 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -220,6 +220,17 @@ is a chance the code will bitrot without anyone noticing. > System emulator machines > > > +Versioned machine types older than 6 years "more than 6 years old" > +'' > + > +Starting with the release of QEMU 10.0, versioned machine types older than > +6 years ditto will automatically be considered as deprecated and might be due to > +removal without furthor notice. For example, this affects machine types like "further" > +pc-i440fx-X.Y, pc-q35-X.Y, pseries-X.Y, s390-ccw-virtio-X.Y or virt-X.Y where > +X is the major number and Y is the minor number of the old QEMU version. > +If you are still using machine types from QEMU versions older than 6 years, ditto > +please update your setting to use a newer versioned machine type instead. > + > Arm ``virt`` machine ``dtb-kaslr-seed`` property (since 7.1) > > Otherwise Reviewed-by: Peter Maydell thanks -- PMM ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH 2/2] NEWS: Document TLS migration bug, usb-net support and two fixes
On Tue, Apr 30, 2024 at 10:19:45 +0200, Peter Krempa wrote: > Signed-off-by: Peter Krempa > --- > NEWS.rst | 25 + > 1 file changed, 25 insertions(+) > > diff --git a/NEWS.rst b/NEWS.rst > index da3fdb21ac..6b92270c9d 100644 > --- a/NEWS.rst > +++ b/NEWS.rst > @@ -17,8 +17,24 @@ v10.3.0 (unreleased) > > * **New features** > > + * qemu: Proper support for USB network device > + > +USB network devices now get properly assigned a USB address and thus can "get properly assigned a USB address" is weird > +be used without manual configuration. > + > * **Improvements** > > + * Improve validation of USB devices > + > +Certain USB device types ('sound', 'fs', 'chr', 'ccid' and 'net') were > not > +properly handled in the check whether the VM config supports USB and thus > +would result in poor error messages. > + > + * virsh: Fix behaviour of ``--name`` and ``--parent`` used together when > listing checkpoint and snapshots > + > +The ``checkpoint-list`` and ``snapshot-list`` commands would ignore the > +``--name`` option to print only the name when used with ``--parent``. > + > * **Bug fixes** > >* qemu: Fix migration with custom XML > @@ -29,6 +45,15 @@ v10.3.0 (unreleased) > depends on the guest CPU definition and the host on which a particular > domain was running. > > + * qemu: Fix TLS hostname verification failure in certain non-shared > storage migration scenarios > + > +In certain scenarios (parallel migration, newly also post-copy migration) > +libvirt would wrongly pass an empty hostname to QEMU to be used for TLS > +certificate hostname validation, which would result into failure of the > +non-shared storage migration step:: > + > + error: internal error: unable to execute QEMU command 'blockdev-add': > Certificate does not match the hostname > + Reviewed-by: Jiri Denemark ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 2/2] NEWS: Document TLS migration bug, usb-net support and two fixes
Signed-off-by: Peter Krempa --- NEWS.rst | 25 + 1 file changed, 25 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index da3fdb21ac..6b92270c9d 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -17,8 +17,24 @@ v10.3.0 (unreleased) * **New features** + * qemu: Proper support for USB network device + +USB network devices now get properly assigned a USB address and thus can +be used without manual configuration. + * **Improvements** + * Improve validation of USB devices + +Certain USB device types ('sound', 'fs', 'chr', 'ccid' and 'net') were not +properly handled in the check whether the VM config supports USB and thus +would result in poor error messages. + + * virsh: Fix behaviour of ``--name`` and ``--parent`` used together when listing checkpoint and snapshots + +The ``checkpoint-list`` and ``snapshot-list`` commands would ignore the +``--name`` option to print only the name when used with ``--parent``. + * **Bug fixes** * qemu: Fix migration with custom XML @@ -29,6 +45,15 @@ v10.3.0 (unreleased) depends on the guest CPU definition and the host on which a particular domain was running. + * qemu: Fix TLS hostname verification failure in certain non-shared storage migration scenarios + +In certain scenarios (parallel migration, newly also post-copy migration) +libvirt would wrongly pass an empty hostname to QEMU to be used for TLS +certificate hostname validation, which would result into failure of the +non-shared storage migration step:: + + error: internal error: unable to execute QEMU command 'blockdev-add': Certificate does not match the hostname + v10.2.0 (2024-04-02) -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 0/2] NEWS fix/update
Peter Krempa (2): NEWS: Fix marking of 'path' as inline literal NEWS: Document TLS migration bug, usb-net support and two fixes NEWS.rst | 27 ++- 1 file changed, 26 insertions(+), 1 deletion(-) -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
[PATCH 1/2] NEWS: Fix marking of 'path' as inline literal
Signed-off-by: Peter Krempa --- NEWS.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.rst b/NEWS.rst index 852dadf532..da3fdb21ac 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -39,7 +39,7 @@ v10.2.0 (2024-04-02) The ch driver now supports basic save and restore operations. This is functional on domains without any network, host device config defined. -The `path` parameter for save and restore should be a directory. +The ``path`` parameter for save and restore should be a directory. * qemu: Support for driver type ``mtp`` in devices -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling
On Tue, Apr 30, 2024 at 09:15:03AM +0200, Markus Armbruster wrote: > Peter Xu writes: > > > On Mon, Apr 29, 2024 at 08:08:10AM -0500, Michael Galaxy wrote: > >> Hi All (and Peter), > > > > Hi, Michael, > > > >> > >> My name is Michael Galaxy (formerly Hines). Yes, I changed my last name > >> (highly irregular for a male) and yes, that's my real last name: > >> https://www.linkedin.com/in/mrgalaxy/) > >> > >> I'm the original author of the RDMA implementation. I've been discussing > >> with Yu Zhang for a little bit about potentially handing over > >> maintainership > >> of the codebase to his team. > >> > >> I simply have zero access to RoCE or Infiniband hardware at all, > >> unfortunately. so I've never been able to run tests or use what I wrote at > >> work, and as all of you know, if you don't have a way to test something, > >> then you can't maintain it. > >> > >> Yu Zhang put a (very kind) proposal forward to me to ask the community if > >> they feel comfortable training his team to maintain the codebase (and run > >> tests) while they learn about it. > > > > The "while learning" part is fine at least to me. IMHO the "ownership" to > > the code, or say, taking over the responsibility, may or may not need 100% > > mastering the code base first. There should still be some fundamental > > confidence to work on the code though as a starting point, then it's about > > serious use case to back this up, and careful testings while getting more > > familiar with it. > > How much experience we expect of maintainers depends on the subsystem > and other circumstances. The hard requirement isn't experience, it's > trust. See the recent attack on xz. > > I do not mean to express any doubts whatsoever on Yu Zhang's integrity! > I'm merely reminding y'all what's at stake. I think we shouldn't overly obsess[1] about 'xz', because the overwhealmingly common scenario is that volunteer maintainers are honest people. QEMU is in a massively better peer review situation. With xz there was basically no oversight of the new maintainer. With QEMU, we have oversight from 1000's of people on the list, a huge pool of general maintainers, the specific migration maintainers, and the release manager merging code. With a lack of historical experiance with QEMU maintainership, I'd suggest that new RDMA volunteers would start by adding themselves to the "MAINTAINERS" file with only the 'Reviewer' classification. The main migration maintainers would still handle pull requests, but wait for a R-b from one of the RMDA volunteers. After some period of time the RDMA folks could graduate to full maintainer status if the migration maintainers needed to reduce their load. I suspect that might prove unneccesary though, given RDMA isn't an area of code with a high turnover of patches. With regards, Daniel [1] If we do want to obsess about something bad though, we should look at our handling of binary blobs in the repo and tarballs. ie the firmware binaries that all get built in an arbitrary environment of their respective maintainer. If we need firmware blobs in tree, we should strive to come up with a reprodicble build environment that gives us byte-for-byte identical results, so the blobs can be verified. This is rather a tangent from this thread though :) -- |: 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 :| ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling
Peter Xu writes: > On Mon, Apr 29, 2024 at 08:08:10AM -0500, Michael Galaxy wrote: >> Hi All (and Peter), > > Hi, Michael, > >> >> My name is Michael Galaxy (formerly Hines). Yes, I changed my last name >> (highly irregular for a male) and yes, that's my real last name: >> https://www.linkedin.com/in/mrgalaxy/) >> >> I'm the original author of the RDMA implementation. I've been discussing >> with Yu Zhang for a little bit about potentially handing over maintainership >> of the codebase to his team. >> >> I simply have zero access to RoCE or Infiniband hardware at all, >> unfortunately. so I've never been able to run tests or use what I wrote at >> work, and as all of you know, if you don't have a way to test something, >> then you can't maintain it. >> >> Yu Zhang put a (very kind) proposal forward to me to ask the community if >> they feel comfortable training his team to maintain the codebase (and run >> tests) while they learn about it. > > The "while learning" part is fine at least to me. IMHO the "ownership" to > the code, or say, taking over the responsibility, may or may not need 100% > mastering the code base first. There should still be some fundamental > confidence to work on the code though as a starting point, then it's about > serious use case to back this up, and careful testings while getting more > familiar with it. How much experience we expect of maintainers depends on the subsystem and other circumstances. The hard requirement isn't experience, it's trust. See the recent attack on xz. I do not mean to express any doubts whatsoever on Yu Zhang's integrity! I'm merely reminding y'all what's at stake. [...] ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org
Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling
Hello Michael and Peter, We are very glad at your quick and kind reply about our plan to take over the maintenance of your code. The message is for presenting our plan and working together. If we were able to obtain the maintainer's role, our plan is: 1. Create the necessary unit-test cases and get them integrated into the current QEMU GitLab-CI pipeline 2. Review and test the code changes by other developers to ensure that nothing is broken in the changed code before being merged by the community 3. Based on our current practice and application scenario, look for possible improvements when necessary Besides that, a patch is attached to announce this change in the community. With your generous support, we hope that the development community will make a positive decision for us. Kind regards, Yu Zhang@ IONOS Cloud On Mon, Apr 29, 2024 at 4:57 PM Peter Xu wrote: > > On Mon, Apr 29, 2024 at 08:08:10AM -0500, Michael Galaxy wrote: > > Hi All (and Peter), > > Hi, Michael, > > > > > My name is Michael Galaxy (formerly Hines). Yes, I changed my last name > > (highly irregular for a male) and yes, that's my real last name: > > https://www.linkedin.com/in/mrgalaxy/) > > > > I'm the original author of the RDMA implementation. I've been discussing > > with Yu Zhang for a little bit about potentially handing over maintainership > > of the codebase to his team. > > > > I simply have zero access to RoCE or Infiniband hardware at all, > > unfortunately. so I've never been able to run tests or use what I wrote at > > work, and as all of you know, if you don't have a way to test something, > > then you can't maintain it. > > > > Yu Zhang put a (very kind) proposal forward to me to ask the community if > > they feel comfortable training his team to maintain the codebase (and run > > tests) while they learn about it. > > The "while learning" part is fine at least to me. IMHO the "ownership" to > the code, or say, taking over the responsibility, may or may not need 100% > mastering the code base first. There should still be some fundamental > confidence to work on the code though as a starting point, then it's about > serious use case to back this up, and careful testings while getting more > familiar with it. > > > > > If you don't mind, I'd like to let him send over his (very detailed) > > proposal, > > Yes please, it's exactly the time to share the plan. The hope is we try to > reach a consensus before or around the middle of this release (9.1). > Normally QEMU has a 3~4 months window for each release and 9.1 schedule is > not yet out, but I think it means we make a decision before or around > middle of June. > > Thanks, > > > > > - Michael > > > > On 4/11/24 11:36, Yu Zhang wrote: > > > > 1) Either a CI test covering at least the major RDMA paths, or at least > > > > periodically tests for each QEMU release will be needed. > > > We use a batch of regression test cases for the stack, which covers the > > > test for QEMU. I did such test for most of the QEMU releases planned as > > > candidates for rollout. > > > > > > The migration test needs a pair of (either physical or virtual) servers > > > with > > > InfiniBand network, which makes it difficult to do on a single server. The > > > nested VM could be a possible approach, for which we may need virtual > > > InfiniBand network. Is SoftRoCE [1] a choice? I will try it and let you > > > know. > > > > > > [1] > > > https://urldefense.com/v3/__https://enterprise-support.nvidia.com/s/article/howto-configure-soft-roce__;!!GjvTz_vk!VEqNfg3Kdf58Oh1FkGL6ErDLfvUXZXPwMTaXizuIQeIgJiywPzuwbqx8wM0KUsyopw_EYQxWvGHE3ig$ > > > > > > Thanks and best regards! > > > > > > On Thu, Apr 11, 2024 at 4:20 PM Peter Xu wrote: > > > > On Wed, Apr 10, 2024 at 09:49:15AM -0400, Peter Xu wrote: > > > > > On Wed, Apr 10, 2024 at 02:28:59AM +, Zhijian Li (Fujitsu) via > > > > > wrote: > > > > > > > > > > > > on 4/10/2024 3:46 AM, Peter Xu wrote: > > > > > > > > > > > > > > Is there document/link about the unittest/CI for migration > > > > > > > > tests, Why > > > > > > > > are those tests missing? > > > > > > > > Is it hard or very special to set up an environment for that? > > > > > > > > maybe we > > > > > > > > can help in this regards. > > > > > > > See tests/qtest/migration-test.c. We put most of our migration > > > > > > > tests > > > > > > > there and that's covered in CI. > > > > > > > > > > > > > > I think one major issue is CI systems don't normally have rdma > > > > > > > devices. > > > > > > > Can rdma migration test be carried out without a real hardware? > > > > > > Yeah, RXE aka. SOFT-RoCE is able to emulate the RDMA, for example > > > > > > $ sudo rdma link add rxe_eth0 type rxe netdev eth0 # on host > > > > > > then we can get a new RDMA interface "rxe_eth0". > > > > > > This new RDMA interface is able to do the QEMU RDMA migration. > > > > > > > > > > > > Also, the loopback(lo) device is able to emulate the RDMA interface > > > > > > "rxe_lo", however
[PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years
Old machine types often have bugs or work-arounds that affect our possibilities to move forward with the QEMU code base (see for example https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely cannot be fixed without breaking live migration with old machine types, or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or commit ea985d235b86). So instead of going through the process of manually deprecating old machine types again and again, let's rather add an entry that can stay, which declares that machine types older than 6 years are considered as deprecated automatically. Six years should be sufficient to support the release cycles of most Linux distributions. Signed-off-by: Thomas Huth --- docs/about/deprecated.rst | 11 +++ 1 file changed, 11 insertions(+) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 6d595de3b6..fe69e2d44c 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -220,6 +220,17 @@ is a chance the code will bitrot without anyone noticing. System emulator machines +Versioned machine types older than 6 years +'' + +Starting with the release of QEMU 10.0, versioned machine types older than +6 years will automatically be considered as deprecated and might be due to +removal without furthor notice. For example, this affects machine types like +pc-i440fx-X.Y, pc-q35-X.Y, pseries-X.Y, s390-ccw-virtio-X.Y or virt-X.Y where +X is the major number and Y is the minor number of the old QEMU version. +If you are still using machine types from QEMU versions older than 6 years, +please update your setting to use a newer versioned machine type instead. + Arm ``virt`` machine ``dtb-kaslr-seed`` property (since 7.1) -- 2.44.0 ___ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-le...@lists.libvirt.org