[PATCH v4 24/30] network: add an nftables backend for network driver's firewall construction

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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"

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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()

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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()

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
(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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Laine Stump
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

2024-04-30 Thread Daniel P . Berrangé
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

2024-04-30 Thread Kristina Hanicova
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

2024-04-30 Thread Kristina Hanicova
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

2024-04-30 Thread Kristina Hanicova
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

2024-04-30 Thread Kristina Hanicova
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

2024-04-30 Thread Kristina Hanicova
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

2024-04-30 Thread Fabiano Rosas
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

2024-04-30 Thread Fabiano Rosas
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

2024-04-30 Thread Fabiano Rosas
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

2024-04-30 Thread Fabiano Rosas
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

2024-04-30 Thread Fabiano Rosas
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

2024-04-30 Thread Fabiano Rosas
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

2024-04-30 Thread Fabiano Rosas
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

2024-04-30 Thread Michal Privoznik
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

2024-04-30 Thread Thomas Huth

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

2024-04-30 Thread Daniel P . Berrangé
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

2024-04-30 Thread Thomas Huth

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

2024-04-30 Thread Daniel P . Berrangé
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

2024-04-30 Thread Cédric Le Goater

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

2024-04-30 Thread Daniel P . Berrangé
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

2024-04-30 Thread Daniel P . Berrangé
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

2024-04-30 Thread Philippe Mathieu-Daudé

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

2024-04-30 Thread Peter Maydell
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

2024-04-30 Thread Jiri Denemark
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

2024-04-30 Thread Peter Krempa
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

2024-04-30 Thread Peter Krempa
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

2024-04-30 Thread Peter Krempa
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

2024-04-30 Thread Daniel P . Berrangé
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

2024-04-30 Thread Markus Armbruster
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

2024-04-30 Thread Yu Zhang via Devel
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

2024-04-30 Thread Thomas Huth
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