Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 0aea5c88f2 | |||
| 1ab72850ed |
@@ -0,0 +1,36 @@
|
||||
From: Michal Privoznik <mprivozn@redhat.com>
|
||||
Date: Mon, 9 Mar 2020 16:40:57 +0100
|
||||
Subject: [PATCH] virDomainDiskTranslateSourcePool: Check for disk type
|
||||
correctly
|
||||
|
||||
When rewriting the virDomainDiskTranslateSourcePool() function in
|
||||
v6.1.0-rc1~184 a typo was introduced. Previously, we allowed
|
||||
startup policy only for those volumes which translated to
|
||||
VIR_STORAGE_TYPE_FILE. But starting with the referenced commit,
|
||||
the value we checked for was changed to VIR_STORAGE_VOL_FILE
|
||||
which comes from a different enum and has a different value too.
|
||||
This is wrong, because virStorageSourceGetActualType() returns a
|
||||
value from the original enum.
|
||||
|
||||
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1811728
|
||||
|
||||
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
|
||||
(cherry picked from commit 3918dbd84e4951b43f93fbf50ef52be00274850c)
|
||||
---
|
||||
src/conf/domain_conf.c | 2 +-
|
||||
1 file changed, 1 insertion(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
|
||||
index 17867eeece..fd2e8f4eb5 100644
|
||||
--- a/src/conf/domain_conf.c
|
||||
+++ b/src/conf/domain_conf.c
|
||||
@@ -31746,7 +31746,7 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def)
|
||||
}
|
||||
|
||||
if (def->startupPolicy != 0 &&
|
||||
- virStorageSourceGetActualType(def->src) != VIR_STORAGE_VOL_FILE) {
|
||||
+ virStorageSourceGetActualType(def->src) != VIR_STORAGE_TYPE_FILE) {
|
||||
virReportError(VIR_ERR_XML_ERROR, "%s",
|
||||
_("'startupPolicy' is only valid for "
|
||||
"'file' type volume"));
|
||||
@@ -1,29 +0,0 @@
|
||||
From: Martin Kletzander <mkletzan@redhat.com>
|
||||
Date: Fri, 16 Aug 2024 13:56:51 +0200
|
||||
Subject: [PATCH] virarptable: Properly calculate rtattr length
|
||||
Content-type: text/plain
|
||||
|
||||
Use convenience macro which does almost the same thing we were doing,
|
||||
but also pads out the payload length to a multiple of NLMSG_ALIGNTO (4)
|
||||
bytes.
|
||||
|
||||
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
|
||||
Reviewed-by: Laine Stump <laine@redhat.com>
|
||||
---
|
||||
src/util/virarptable.c | 3 +--
|
||||
1 file changed, 1 insertion(+), 2 deletions(-)
|
||||
|
||||
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
|
||||
index 299dddd664..d8e41c5a86 100644
|
||||
--- a/src/util/virarptable.c
|
||||
+++ b/src/util/virarptable.c
|
||||
@@ -102,8 +102,7 @@ virArpTableGet(void)
|
||||
return table;
|
||||
|
||||
VIR_WARNINGS_NO_CAST_ALIGN
|
||||
- parse_rtattr(tb, NDA_MAX, NDA_RTA(r),
|
||||
- nh->nlmsg_len - NLMSG_LENGTH(sizeof(*r)));
|
||||
+ parse_rtattr(tb, NDA_MAX, NDA_RTA(r), NLMSG_PAYLOAD(nh, sizeof(*r)));
|
||||
VIR_WARNINGS_RESET
|
||||
|
||||
if (tb[NDA_DST] == NULL || tb[NDA_LLADDR] == NULL)
|
||||
@@ -0,0 +1,55 @@
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Thu, 7 May 2020 22:32:59 -0400
|
||||
Subject: [PATCH] network: make it safe to call networkSetupPrivateChains()
|
||||
multiple times
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
networkSetupPrivateChains() is currently called only once per run of
|
||||
libvirtd, so it can assume that errInitV4 and errInitV6 are empty/null
|
||||
when it is called. In preparation for potentially calling this
|
||||
function multiple times during one run, this patch moves the reset of
|
||||
errInitV[46] to the top of the function, to assure no memory is
|
||||
leaked.
|
||||
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
|
||||
(cherry picked from commit de110f110fb917a31b9f33ad8e4b3c1d3284766a)
|
||||
---
|
||||
src/network/bridge_driver_linux.c | 8 ++++----
|
||||
1 file changed, 4 insertions(+), 4 deletions(-)
|
||||
|
||||
diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
|
||||
index 7bbde5c6a9..80bd2409e1 100644
|
||||
--- a/src/network/bridge_driver_linux.c
|
||||
+++ b/src/network/bridge_driver_linux.c
|
||||
@@ -48,6 +48,10 @@ static void networkSetupPrivateChains(void)
|
||||
VIR_DEBUG("Setting up global firewall chains");
|
||||
|
||||
createdChains = false;
|
||||
+ virFreeError(errInitV4);
|
||||
+ errInitV4 = NULL;
|
||||
+ virFreeError(errInitV6);
|
||||
+ errInitV6 = NULL;
|
||||
|
||||
rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV4);
|
||||
if (rc < 0) {
|
||||
@@ -56,8 +60,6 @@ static void networkSetupPrivateChains(void)
|
||||
errInitV4 = virSaveLastError();
|
||||
virResetLastError();
|
||||
} else {
|
||||
- virFreeError(errInitV4);
|
||||
- errInitV4 = NULL;
|
||||
if (rc) {
|
||||
VIR_DEBUG("Created global IPv4 chains");
|
||||
createdChains = true;
|
||||
@@ -73,8 +75,6 @@ static void networkSetupPrivateChains(void)
|
||||
errInitV6 = virSaveLastError();
|
||||
virResetLastError();
|
||||
} else {
|
||||
- virFreeError(errInitV6);
|
||||
- errInitV6 = NULL;
|
||||
if (rc) {
|
||||
VIR_DEBUG("Created global IPv6 chains");
|
||||
createdChains = true;
|
||||
@@ -1,34 +0,0 @@
|
||||
From: Martin Kletzander <mkletzan@redhat.com>
|
||||
Date: Fri, 16 Aug 2024 13:59:15 +0200
|
||||
Subject: [PATCH] virarptable: Fix check for message length
|
||||
Content-type: text/plain
|
||||
|
||||
The previous check was all wrong since it calculated the how long would
|
||||
the netlink message be if the netlink header was the payload and then
|
||||
subtracted that from the whole message length, a variable that was not
|
||||
used later in the code. This check can fail if there are no additional
|
||||
payloads, struct rtattr in particular, which we are parsing later,
|
||||
however the RTA_OK macro would've caught that anyway.
|
||||
|
||||
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
|
||||
Reviewed-by: Laine Stump <laine@redhat.com>
|
||||
---
|
||||
src/util/virarptable.c | 3 +--
|
||||
1 file changed, 1 insertion(+), 2 deletions(-)
|
||||
|
||||
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
|
||||
index d8e41c5a86..45ee76766f 100644
|
||||
--- a/src/util/virarptable.c
|
||||
+++ b/src/util/virarptable.c
|
||||
@@ -81,10 +81,9 @@ virArpTableGet(void)
|
||||
for (; NLMSG_OK(nh, msglen); nh = NLMSG_NEXT(nh, msglen)) {
|
||||
VIR_WARNINGS_RESET
|
||||
struct ndmsg *r = NLMSG_DATA(nh);
|
||||
- int len = nh->nlmsg_len;
|
||||
void *addr;
|
||||
|
||||
- if ((len -= NLMSG_LENGTH(sizeof(*nh))) < 0) {
|
||||
+ if (nh->nlmsg_len < NLMSG_SPACE(sizeof(*r))) {
|
||||
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
|
||||
_("wrong nlmsg len"));
|
||||
goto cleanup;
|
||||
@@ -0,0 +1,265 @@
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Thu, 7 May 2020 21:54:39 -0400
|
||||
Subject: [PATCH] network: force re-creation of iptables private chains on
|
||||
firewalld restart
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
When firewalld is stopped, it removes *all* iptables rules and chains,
|
||||
including those added by libvirt. Since restarting firewalld means
|
||||
stopping and then starting it, any time it is restarted, libvirt needs
|
||||
to recreate all the private iptables chains it uses, along with all
|
||||
the rules it adds.
|
||||
|
||||
We already have code in place to call networkReloadFirewallRules() any
|
||||
time we're notified of a firewalld start, and
|
||||
networkReloadFirewallRules() will call
|
||||
networkPreReloadFirewallRules(), which calls
|
||||
networkSetupPrivateChains(); unfortunately that last call is called
|
||||
using virOnce(), meaning that it will only be called the first time
|
||||
through networkPreReloadFirewallRules() after libvirtd starts - so of
|
||||
course when firewalld is later restarted, the call to
|
||||
networkSetupPrivateChains() is skipped.
|
||||
|
||||
The neat and tidy way to fix this would be if there was a standard way
|
||||
to reset a pthread_once_t object so that the next time virOnce was
|
||||
called, it would think the function hadn't been called, and call it
|
||||
again. Unfortunately, there isn't any official way of doing that (we
|
||||
*could* just fill it with 0 and hope for the best, but that doesn't
|
||||
seem very safe.
|
||||
|
||||
So instead, this patch just adds a static variable called
|
||||
chainInitDone, which is set to true after networkSetupPrivateChains()
|
||||
is called for the first time, and then during calls to
|
||||
networkPreReloadFirewallRules(), if chainInitDone is set, we call
|
||||
networkSetupPrivateChains() directly instead of via virOnce().
|
||||
|
||||
It may seem unsafe to directly call a function that is meant to be
|
||||
called only once, but I think in this case we're safe - there's
|
||||
nothing in the function that is inherently "once only" - it doesn't
|
||||
initialize anything that can't safely be re-initialized (as long as
|
||||
two threads don't try to do it at the same time), and it only happens
|
||||
when responding to a dbus message that firewalld has been started (and
|
||||
I don't think it's possible for us to be processing two of those at
|
||||
once), and even then only if the initial call to the function has
|
||||
already been completed (so we're safe if we receive a firewalld
|
||||
restart call at a time when we haven't yet called it, or even if
|
||||
another thread is already in the process of executing it. The only
|
||||
problematic bit I can think of is if another thread is in the process
|
||||
of adding an iptable rule at the time we're executing this function,
|
||||
but 1) none of those threads will be trying to add chains, and 2) if
|
||||
there was a concurrency problem with other threads adding iptables
|
||||
rules while firewalld was being restarted, it would still be a problem
|
||||
even without this change.
|
||||
|
||||
This is yet another patch that fixes an occurrence of this error:
|
||||
|
||||
COMMAND_FAILED: '/usr/sbin/iptables -w10 -w --table filter --insert LIBVIRT_INP --in-interface virbr0 --protocol tcp --destination-port 67 --jump ACCEPT' failed: iptables: No chain/target/match by that name.
|
||||
|
||||
In particular, this resolves: https://bugzilla.redhat.com/1813830
|
||||
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
|
||||
(cherry picked from commit f5418b427e7d2f26803880309478de9103680826)
|
||||
---
|
||||
src/network/bridge_driver.c | 16 ++++---
|
||||
src/network/bridge_driver_linux.c | 69 ++++++++++++++++++----------
|
||||
src/network/bridge_driver_nop.c | 3 +-
|
||||
src/network/bridge_driver_platform.h | 2 +-
|
||||
4 files changed, 58 insertions(+), 32 deletions(-)
|
||||
|
||||
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
|
||||
index 369e80a889..aaf14defe4 100644
|
||||
--- a/src/network/bridge_driver.c
|
||||
+++ b/src/network/bridge_driver.c
|
||||
@@ -273,7 +273,9 @@ static int
|
||||
networkShutdownNetworkExternal(virNetworkObjPtr obj);
|
||||
|
||||
static void
|
||||
-networkReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup);
|
||||
+networkReloadFirewallRules(virNetworkDriverStatePtr driver,
|
||||
+ bool startup,
|
||||
+ bool force);
|
||||
|
||||
static void
|
||||
networkRefreshDaemons(virNetworkDriverStatePtr driver);
|
||||
@@ -689,7 +691,7 @@ firewalld_dbus_filter_bridge(DBusConnection *connection G_GNUC_UNUSED,
|
||||
|
||||
if (reload) {
|
||||
VIR_DEBUG("Reload in bridge_driver because of firewalld.");
|
||||
- networkReloadFirewallRules(driver, false);
|
||||
+ networkReloadFirewallRules(driver, false, true);
|
||||
}
|
||||
|
||||
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
|
||||
@@ -798,7 +800,7 @@ networkStateInitialize(bool privileged,
|
||||
virNetworkObjListPrune(network_driver->networks,
|
||||
VIR_CONNECT_LIST_NETWORKS_INACTIVE |
|
||||
VIR_CONNECT_LIST_NETWORKS_TRANSIENT);
|
||||
- networkReloadFirewallRules(network_driver, true);
|
||||
+ networkReloadFirewallRules(network_driver, true, false);
|
||||
networkRefreshDaemons(network_driver);
|
||||
|
||||
if (virDriverShouldAutostart(network_driver->stateDir, &autostart) < 0)
|
||||
@@ -868,7 +870,7 @@ networkStateReload(void)
|
||||
network_driver->networkConfigDir,
|
||||
network_driver->networkAutostartDir,
|
||||
network_driver->xmlopt);
|
||||
- networkReloadFirewallRules(network_driver, false);
|
||||
+ networkReloadFirewallRules(network_driver, false, false);
|
||||
networkRefreshDaemons(network_driver);
|
||||
virNetworkObjListForEach(network_driver->networks,
|
||||
networkAutostartConfig,
|
||||
@@ -2236,14 +2238,16 @@ networkReloadFirewallRulesHelper(virNetworkObjPtr obj,
|
||||
|
||||
|
||||
static void
|
||||
-networkReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup)
|
||||
+networkReloadFirewallRules(virNetworkDriverStatePtr driver,
|
||||
+ bool startup,
|
||||
+ bool force)
|
||||
{
|
||||
VIR_INFO("Reloading iptables rules");
|
||||
/* Ideally we'd not even register the driver when unprivilegd
|
||||
* but until we untangle the virt driver that's not viable */
|
||||
if (!driver->privileged)
|
||||
return;
|
||||
- networkPreReloadFirewallRules(driver, startup);
|
||||
+ networkPreReloadFirewallRules(driver, startup, force);
|
||||
virNetworkObjListForEach(driver->networks,
|
||||
networkReloadFirewallRulesHelper,
|
||||
NULL);
|
||||
diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
|
||||
index 80bd2409e1..b0bd207250 100644
|
||||
--- a/src/network/bridge_driver_linux.c
|
||||
+++ b/src/network/bridge_driver_linux.c
|
||||
@@ -36,11 +36,14 @@ VIR_LOG_INIT("network.bridge_driver_linux");
|
||||
#define PROC_NET_ROUTE "/proc/net/route"
|
||||
|
||||
static virOnceControl createdOnce;
|
||||
-static bool createdChains;
|
||||
+static bool chainInitDone; /* true iff networkSetupPrivateChains was ever called */
|
||||
+static bool createdChains; /* true iff networkSetupPrivateChains created chains during most recent call */
|
||||
static virErrorPtr errInitV4;
|
||||
static virErrorPtr errInitV6;
|
||||
|
||||
-/* Only call via virOnce */
|
||||
+/* Usually only called via virOnce, but can also be called directly in
|
||||
+ * response to firewalld reload (if chainInitDone == true)
|
||||
+ */
|
||||
static void networkSetupPrivateChains(void)
|
||||
{
|
||||
int rc;
|
||||
@@ -82,6 +85,8 @@ static void networkSetupPrivateChains(void)
|
||||
VIR_DEBUG("Global IPv6 chains already exist");
|
||||
}
|
||||
}
|
||||
+
|
||||
+ chainInitDone = true;
|
||||
}
|
||||
|
||||
|
||||
@@ -111,7 +116,10 @@ networkHasRunningNetworks(virNetworkDriverStatePtr driver)
|
||||
}
|
||||
|
||||
|
||||
-void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup)
|
||||
+void
|
||||
+networkPreReloadFirewallRules(virNetworkDriverStatePtr driver,
|
||||
+ bool startup,
|
||||
+ bool force)
|
||||
{
|
||||
/*
|
||||
* If there are any running networks, we need to
|
||||
@@ -130,29 +138,42 @@ void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup
|
||||
* of starting the network though as that makes them
|
||||
* more likely to be seen by a human
|
||||
*/
|
||||
- if (!networkHasRunningNetworks(driver)) {
|
||||
- VIR_DEBUG("Delayed global rule setup as no networks are running");
|
||||
- return;
|
||||
- }
|
||||
+ 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().
|
||||
+ */
|
||||
+ networkSetupPrivateChains();
|
||||
|
||||
- ignore_value(virOnce(&createdOnce, networkSetupPrivateChains));
|
||||
+ } else {
|
||||
+ if (!networkHasRunningNetworks(driver)) {
|
||||
+ VIR_DEBUG("Delayed global rule setup as no networks are running");
|
||||
+ return;
|
||||
+ }
|
||||
|
||||
- /*
|
||||
- * If this is initial startup, and we just created the
|
||||
- * top level private chains we either
|
||||
- *
|
||||
- * - upgraded from old libvirt
|
||||
- * - freshly booted from clean state
|
||||
- *
|
||||
- * In the first case we must delete the old rules from
|
||||
- * the built-in chains, instead of our new private chains.
|
||||
- * In the second case it doesn't matter, since no existing
|
||||
- * rules will be present. Thus we can safely just tell it
|
||||
- * to always delete from the builin chain
|
||||
- */
|
||||
- if (startup && createdChains) {
|
||||
- VIR_DEBUG("Requesting cleanup of legacy firewall rules");
|
||||
- iptablesSetDeletePrivate(false);
|
||||
+ ignore_value(virOnce(&createdOnce, networkSetupPrivateChains));
|
||||
+
|
||||
+ /*
|
||||
+ * If this is initial startup, and we just created the
|
||||
+ * top level private chains we either
|
||||
+ *
|
||||
+ * - upgraded from old libvirt
|
||||
+ * - freshly booted from clean state
|
||||
+ *
|
||||
+ * In the first case we must delete the old rules from
|
||||
+ * the built-in chains, instead of our new private chains.
|
||||
+ * In the second case it doesn't matter, since no existing
|
||||
+ * rules will be present. Thus we can safely just tell it
|
||||
+ * to always delete from the builin chain
|
||||
+ */
|
||||
+ if (startup && createdChains) {
|
||||
+ VIR_DEBUG("Requesting cleanup of legacy firewall rules");
|
||||
+ iptablesSetDeletePrivate(false);
|
||||
+ }
|
||||
}
|
||||
}
|
||||
|
||||
diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c
|
||||
index 08d737511f..db89c10023 100644
|
||||
--- a/src/network/bridge_driver_nop.c
|
||||
+++ b/src/network/bridge_driver_nop.c
|
||||
@@ -20,7 +20,8 @@
|
||||
#include <config.h>
|
||||
|
||||
void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver G_GNUC_UNUSED,
|
||||
- bool startup G_GNUC_UNUSED)
|
||||
+ bool startup G_GNUC_UNUSED,
|
||||
+ bool force G_GNUC_UNUSED)
|
||||
{
|
||||
}
|
||||
|
||||
diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
|
||||
index 169417a6c0..48ab52c160 100644
|
||||
--- a/src/network/bridge_driver_platform.h
|
||||
+++ b/src/network/bridge_driver_platform.h
|
||||
@@ -62,7 +62,7 @@ struct _virNetworkDriverState {
|
||||
typedef struct _virNetworkDriverState virNetworkDriverState;
|
||||
typedef virNetworkDriverState *virNetworkDriverStatePtr;
|
||||
|
||||
-void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup);
|
||||
+void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup, bool force);
|
||||
void networkPostReloadFirewallRules(bool startup);
|
||||
|
||||
int networkCheckRouteCollision(virNetworkDefPtr def);
|
||||
@@ -1,46 +0,0 @@
|
||||
From: Martin Kletzander <mkletzan@redhat.com>
|
||||
Date: Fri, 16 Aug 2024 14:02:48 +0200
|
||||
Subject: [PATCH] virarptable: End parsing earlier in case of NLMSG_DONE
|
||||
Content-type: text/plain
|
||||
|
||||
Check for the last multipart message right as the first thing. The
|
||||
presumption probably was that the last message might still contain a
|
||||
payload we want to parse. However that cannot be true since that would
|
||||
have to be a type RTM_NEWNEIGH. This was not caught because older
|
||||
kernels were note sending NLMSG_DONE and probably relied on the fact
|
||||
that the parsing just stops after all the messages are walked through,
|
||||
which the NLMSG_OK macro successfully did.
|
||||
|
||||
Resolves: https://issues.redhat.com/browse/RHEL-52449
|
||||
Resolves: https://bugzilla.redhat.com/2302245
|
||||
Fixes: a176d67cdfaf5b8237a7e3a80d8be0e6bdf2d8fd
|
||||
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
|
||||
Reviewed-by: Laine Stump <laine@redhat.com>
|
||||
---
|
||||
src/util/virarptable.c | 6 +++---
|
||||
1 file changed, 3 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
|
||||
index 45ee76766f..20d11f97b0 100644
|
||||
--- a/src/util/virarptable.c
|
||||
+++ b/src/util/virarptable.c
|
||||
@@ -83,6 +83,9 @@ virArpTableGet(void)
|
||||
struct ndmsg *r = NLMSG_DATA(nh);
|
||||
void *addr;
|
||||
|
||||
+ if (nh->nlmsg_type == NLMSG_DONE)
|
||||
+ break;
|
||||
+
|
||||
if (nh->nlmsg_len < NLMSG_SPACE(sizeof(*r))) {
|
||||
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
|
||||
_("wrong nlmsg len"));
|
||||
@@ -97,9 +100,6 @@ virArpTableGet(void)
|
||||
(!(r->ndm_state == NUD_STALE || r->ndm_state == NUD_REACHABLE)))
|
||||
continue;
|
||||
|
||||
- if (nh->nlmsg_type == NLMSG_DONE)
|
||||
- return table;
|
||||
-
|
||||
VIR_WARNINGS_NO_CAST_ALIGN
|
||||
parse_rtattr(tb, NDA_MAX, NDA_RTA(r), NLMSG_PAYLOAD(nh, sizeof(*r)));
|
||||
VIR_WARNINGS_RESET
|
||||
@@ -0,0 +1,100 @@
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Fri, 1 May 2020 00:05:50 -0400
|
||||
Subject: [PATCH] systemd: start libvirtd after firewalld/iptables services
|
||||
|
||||
When a system has enabled the iptables/ip6tables services rather than
|
||||
firewalld, there is no explicit ordering of the start of those
|
||||
services vs. libvirtd. This creates a problem when libvirtd.service is
|
||||
started before ip[6]tables, as the latter, when it finally is started,
|
||||
will remove all of the iptables rules that had previously been added
|
||||
by libvirt, including the custom chains where libvirt's rules are
|
||||
kept. This results in an error message similar to the following when a
|
||||
user subsequently tries to start a new libvirt network:
|
||||
|
||||
"Error while activating network: Call to virNetworkCreate failed:
|
||||
internal error: Failed to apply firewall rules
|
||||
/usr/sbin/ip6tables -w --table filter --insert LIBVIRT_FWO \
|
||||
--in-interface virbr2 --jump REJECT:
|
||||
ip6tables: No chain/target/match by that name."
|
||||
|
||||
(Prior to logging this error, it also would have caused failure to
|
||||
forward (or block) traffic in some cases, e.g. for guests on a NATed
|
||||
network, since libvirt's rules to forward/block had all been deleted
|
||||
and libvirt didn't know about it, so it couldn't fix the problem)
|
||||
|
||||
When this happens, the problem can be remedied by simply restarting
|
||||
libvirtd.service (which has the side-effect of reloading all
|
||||
libvirt-generated firewall rules)
|
||||
|
||||
Instead, we can just explicitly stating in the libvirtd.service file
|
||||
that libvirtd.service should start after ip6tables.service and
|
||||
ip6tables.service, eliminating the race condition that leads to the
|
||||
error.
|
||||
|
||||
There is also nothing (that I can see) in the systemd .service files
|
||||
to guarantee that firewalld.service will be started (if enabled) prior
|
||||
to libvirtd.service. The same error scenario given above would occur
|
||||
if libvirtd.service started before firewalld.service. Even before
|
||||
that, though libvirtd would have detected that firewalld.service was
|
||||
disabled, and then turn off all firewalld support. So, for example,
|
||||
firewalld's libvirt zone wouldn't be used, and most likely traffic
|
||||
from guests would therefore be blocked (all with no external
|
||||
indication of the source of the problem other than a debug-level log
|
||||
when libvirtd was started saying that firewalld wasn't in use); also
|
||||
libvirtd wouldn't notice when firewalld reloaded its rules (which also
|
||||
simultaneously deletes all of libvirt's rules).
|
||||
|
||||
I'm not aware of any reports that have been traced back to
|
||||
libvirtd.service starting before firewalld.service, but have seen that
|
||||
error reported multiple times, and also don't see an existing
|
||||
dependency that would guarantee firewalld.service starts before
|
||||
libvirtd.service, so it's possible it's been happening and we just
|
||||
haven't gotten to the bottom of it.
|
||||
|
||||
This patch adds an After= line to the libvirtd.service file for each
|
||||
of iptables.service, ip6tables.service, and firewalld.servicee, which
|
||||
should guarantee that libvirtd.service isn't started until systemd has
|
||||
started whichever of the others is enabled.
|
||||
|
||||
This race was diagnosed, and patch proposed, by Jason Montleon in
|
||||
https://bugzilla.redhat.com/1723698 . At the time (April 2019) danpb
|
||||
agreed with him that this change to libvirtd.service was a reasonable
|
||||
thing to do, but I guess everyone thought someone else was going to
|
||||
post a patch, so in the end nobody did.
|
||||
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
(cherry picked from commit 0756415f147dda15a417bd79eef9a62027d176e6)
|
||||
---
|
||||
src/network/virtnetworkd.service.in | 3 +++
|
||||
src/remote/libvirtd.service.in | 3 +++
|
||||
2 files changed, 6 insertions(+)
|
||||
|
||||
diff --git a/src/network/virtnetworkd.service.in b/src/network/virtnetworkd.service.in
|
||||
index 656e8b4f84..56182e1693 100644
|
||||
--- a/src/network/virtnetworkd.service.in
|
||||
+++ b/src/network/virtnetworkd.service.in
|
||||
@@ -5,6 +5,9 @@ Requires=virtnetworkd.socket
|
||||
Requires=virtnetworkd-ro.socket
|
||||
Requires=virtnetworkd-admin.socket
|
||||
After=network.target
|
||||
+After=firewalld.service
|
||||
+After=iptables.service
|
||||
+After=ip6tables.service
|
||||
After=dbus.service
|
||||
After=apparmor.service
|
||||
After=local-fs.target
|
||||
diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in
|
||||
index 90b2cad5b0..cc0d4e3693 100644
|
||||
--- a/src/remote/libvirtd.service.in
|
||||
+++ b/src/remote/libvirtd.service.in
|
||||
@@ -11,6 +11,9 @@ Wants=libvirtd-admin.socket
|
||||
Wants=systemd-machined.service
|
||||
Before=libvirt-guests.service
|
||||
After=network.target
|
||||
+After=firewalld.service
|
||||
+After=iptables.service
|
||||
+After=ip6tables.service
|
||||
After=dbus.service
|
||||
After=iscsid.service
|
||||
After=apparmor.service
|
||||
@@ -1,315 +0,0 @@
|
||||
From 807e2670f2704c41f0a1dca81a5d2f2f9336137c Mon Sep 17 00:00:00 2001
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Mon, 25 Nov 2024 22:24:44 -0500
|
||||
Subject: [PATCH 4/9] util: use a single flags arg for virNetDevBandwidthSet(),
|
||||
not multiple bools
|
||||
|
||||
Having two bools in the arg list is on the borderline of being
|
||||
confusing to anyone trying to read the code, but we're about to add a
|
||||
3rd. This patch replaces the two bools with a single flags argument
|
||||
which will instead have one or more bits from virNetDevBandwidthFlags
|
||||
set.
|
||||
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
---
|
||||
src/lxc/lxc_driver.c | 8 ++++++--
|
||||
src/lxc/lxc_process.c | 8 ++++++--
|
||||
src/network/bridge_driver.c | 10 ++++++++--
|
||||
src/qemu/qemu_command.c | 11 ++++++++---
|
||||
src/qemu/qemu_driver.c | 29 ++++++++++++++-------------
|
||||
src/qemu/qemu_hotplug.c | 22 +++++++++++++++------
|
||||
src/util/virnetdevbandwidth.c | 36 ++++++++++++++++++++--------------
|
||||
src/util/virnetdevbandwidth.h | 9 +++++++--
|
||||
tests/virnetdevbandwidthtest.c | 8 +++++++-
|
||||
9 files changed, 94 insertions(+), 47 deletions(-)
|
||||
|
||||
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
|
||||
index 534e257f30..b693980dbb 100644
|
||||
--- a/src/lxc/lxc_driver.c
|
||||
+++ b/src/lxc/lxc_driver.c
|
||||
@@ -3570,8 +3570,12 @@ lxcDomainAttachDeviceNetLive(virLXCDriver *driver,
|
||||
actualBandwidth = virDomainNetGetActualBandwidth(net);
|
||||
if (actualBandwidth) {
|
||||
if (virNetDevSupportsBandwidth(actualType)) {
|
||||
- if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false,
|
||||
- !virDomainNetTypeSharesHostView(net)) < 0)
|
||||
+ unsigned int flags = 0;
|
||||
+
|
||||
+ if (!virDomainNetTypeSharesHostView(net))
|
||||
+ flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+
|
||||
+ if (virNetDevBandwidthSet(net->ifname, actualBandwidth, flags) < 0)
|
||||
goto cleanup;
|
||||
} else {
|
||||
VIR_WARN("setting bandwidth on interfaces of "
|
||||
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
|
||||
index f5eb5383ec..0e689fbb70 100644
|
||||
--- a/src/lxc/lxc_process.c
|
||||
+++ b/src/lxc/lxc_process.c
|
||||
@@ -605,8 +605,12 @@ virLXCProcessSetupInterfaces(virLXCDriver *driver,
|
||||
actualBandwidth = virDomainNetGetActualBandwidth(net);
|
||||
if (actualBandwidth) {
|
||||
if (virNetDevSupportsBandwidth(type)) {
|
||||
- if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false,
|
||||
- !virDomainNetTypeSharesHostView(net)) < 0)
|
||||
+ unsigned int flags = 0;
|
||||
+
|
||||
+ if (!virDomainNetTypeSharesHostView(net))
|
||||
+ flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+
|
||||
+ if (virNetDevBandwidthSet(net->ifname, actualBandwidth, flags) < 0)
|
||||
goto cleanup;
|
||||
} else {
|
||||
VIR_WARN("setting bandwidth on interfaces of "
|
||||
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
|
||||
index 32572c755f..1c53636450 100644
|
||||
--- a/src/network/bridge_driver.c
|
||||
+++ b/src/network/bridge_driver.c
|
||||
@@ -2058,8 +2058,11 @@ networkStartNetworkVirtual(virNetworkDriverState *driver,
|
||||
}
|
||||
}
|
||||
|
||||
- if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0)
|
||||
+ if (virNetDevBandwidthSet(def->bridge, def->bandwidth,
|
||||
+ VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS
|
||||
+ | VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED) < 0) {
|
||||
goto error;
|
||||
+ }
|
||||
|
||||
return 0;
|
||||
|
||||
@@ -2141,8 +2144,11 @@ networkStartNetworkBridge(virNetworkObj *obj)
|
||||
* type BRIDGE, is started. On failure, undo anything you've done,
|
||||
* and return -1. On success return 0.
|
||||
*/
|
||||
- if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0)
|
||||
+ if (virNetDevBandwidthSet(def->bridge, def->bandwidth,
|
||||
+ VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS
|
||||
+ | VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED) < 0) {
|
||||
goto error;
|
||||
+ }
|
||||
|
||||
if (networkStartHandleMACTableManagerMode(obj) < 0)
|
||||
goto error;
|
||||
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
|
||||
index f15e6bda1e..b4815e5e71 100644
|
||||
--- a/src/qemu/qemu_command.c
|
||||
+++ b/src/qemu/qemu_command.c
|
||||
@@ -8840,9 +8840,14 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver,
|
||||
def->uuid,
|
||||
!virDomainNetTypeSharesHostView(net)) < 0)
|
||||
goto cleanup;
|
||||
- } else if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false,
|
||||
- !virDomainNetTypeSharesHostView(net)) < 0) {
|
||||
- goto cleanup;
|
||||
+ } else {
|
||||
+ unsigned int flags = 0;
|
||||
+
|
||||
+ if (!virDomainNetTypeSharesHostView(net))
|
||||
+ flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+
|
||||
+ if (virNetDevBandwidthSet(net->ifname, actualBandwidth, flags) < 0)
|
||||
+ goto cleanup;
|
||||
}
|
||||
} else {
|
||||
VIR_WARN("setting bandwidth on interfaces of "
|
||||
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
|
||||
index 736602333e..14929616e5 100644
|
||||
--- a/src/qemu/qemu_driver.c
|
||||
+++ b/src/qemu/qemu_driver.c
|
||||
@@ -9941,21 +9941,22 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
|
||||
virErrorRestore(&orig_err);
|
||||
goto endjob;
|
||||
}
|
||||
- } else if (virNetDevBandwidthSet(net->ifname, newBandwidth, false,
|
||||
- !virDomainNetTypeSharesHostView(net)) < 0) {
|
||||
- virErrorPtr orig_err;
|
||||
-
|
||||
- virErrorPreserveLast(&orig_err);
|
||||
- ignore_value(virNetDevBandwidthSet(net->ifname,
|
||||
- net->bandwidth,
|
||||
- false,
|
||||
- !virDomainNetTypeSharesHostView(net)));
|
||||
- if (net->bandwidth) {
|
||||
- ignore_value(virDomainNetBandwidthUpdate(net,
|
||||
- net->bandwidth));
|
||||
+ } else {
|
||||
+ unsigned int bwflags = 0;
|
||||
+
|
||||
+ if (!virDomainNetTypeSharesHostView(net))
|
||||
+ bwflags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+
|
||||
+ if (virNetDevBandwidthSet(net->ifname, newBandwidth, bwflags) < 0) {
|
||||
+ virErrorPtr orig_err;
|
||||
+
|
||||
+ virErrorPreserveLast(&orig_err);
|
||||
+ ignore_value(virNetDevBandwidthSet(net->ifname, net->bandwidth, bwflags));
|
||||
+ if (net->bandwidth)
|
||||
+ ignore_value(virDomainNetBandwidthUpdate(net, net->bandwidth));
|
||||
+ virErrorRestore(&orig_err);
|
||||
+ goto endjob;
|
||||
}
|
||||
- virErrorRestore(&orig_err);
|
||||
- goto endjob;
|
||||
}
|
||||
|
||||
/* If the old bandwidth was cleared out, restore qdisc. */
|
||||
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
|
||||
index 7cb1800504..d5e7e99359 100644
|
||||
--- a/src/qemu/qemu_hotplug.c
|
||||
+++ b/src/qemu/qemu_hotplug.c
|
||||
@@ -1279,9 +1279,14 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
|
||||
vm->def->uuid,
|
||||
!virDomainNetTypeSharesHostView(net)) < 0)
|
||||
goto cleanup;
|
||||
- } else if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false,
|
||||
- !virDomainNetTypeSharesHostView(net)) < 0) {
|
||||
- goto cleanup;
|
||||
+ } else {
|
||||
+ int flags = 0;
|
||||
+
|
||||
+ if (!virDomainNetTypeSharesHostView(net))
|
||||
+ flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+
|
||||
+ if (virNetDevBandwidthSet(net->ifname, actualBandwidth, flags) < 0)
|
||||
+ goto cleanup;
|
||||
}
|
||||
} else {
|
||||
VIR_WARN("setting bandwidth on interfaces of "
|
||||
@@ -4082,9 +4087,14 @@ qemuDomainChangeNet(virQEMUDriver *driver,
|
||||
vm->def->uuid,
|
||||
!virDomainNetTypeSharesHostView(newdev)) < 0)
|
||||
goto cleanup;
|
||||
- } else if (virNetDevBandwidthSet(newdev->ifname, newb, false,
|
||||
- !virDomainNetTypeSharesHostView(newdev)) < 0) {
|
||||
- goto cleanup;
|
||||
+ } else {
|
||||
+ int flags = 0;
|
||||
+
|
||||
+ if (!virDomainNetTypeSharesHostView(newdev))
|
||||
+ flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+
|
||||
+ if (virNetDevBandwidthSet(newdev->ifname, newb, flags) < 0)
|
||||
+ goto cleanup;
|
||||
}
|
||||
} else {
|
||||
if (virDomainInterfaceClearQoS(vm->def, olddev) < 0)
|
||||
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
|
||||
index 2b58c58d3e..1baad849c6 100644
|
||||
--- a/src/util/virnetdevbandwidth.c
|
||||
+++ b/src/util/virnetdevbandwidth.c
|
||||
@@ -173,30 +173,35 @@ virNetDevBandwidthManipulateFilter(const char *ifname,
|
||||
* virNetDevBandwidthSet:
|
||||
* @ifname: on which interface
|
||||
* @bandwidth: rates to set (may be NULL)
|
||||
- * @hierarchical_class: whether to create hierarchical class
|
||||
- * @swapped: true if IN/OUT should be set contrariwise
|
||||
+ * @flags: bits indicating certain optional actions
|
||||
*
|
||||
+
|
||||
* This function enables QoS on specified interface
|
||||
* and set given traffic limits for both, incoming
|
||||
- * and outgoing traffic. Any previous setting get
|
||||
- * overwritten. If @hierarchical_class is TRUE, create
|
||||
- * hierarchical class. It is used to guarantee minimal
|
||||
- * throughput ('floor' attribute in NIC).
|
||||
+ * and outgoing traffic.
|
||||
+ *
|
||||
+ * @flags bits and their meanings:
|
||||
+ *
|
||||
+ * VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS
|
||||
+ * whether to create a hierarchical class
|
||||
+ * A hiearchical class structure is used to implement a minimal
|
||||
+ * throughput guarantee ('floor' attribute in NIC).
|
||||
*
|
||||
- * If @swapped is set, the IN part of @bandwidth is set on
|
||||
- * @ifname's TX, and vice versa. If it is not set, IN is set on
|
||||
- * RX and OUT on TX. This is because for some types of interfaces
|
||||
- * domain and the host live on the same side of the interface (so
|
||||
- * domain's RX/TX is host's RX/TX), and for some it's swapped
|
||||
- * (domain's RX/TX is hosts's TX/RX).
|
||||
+ * VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED
|
||||
+ * set if IN/OUT should be set backwards from what's indicated in
|
||||
+ * the bandwidth, i.e. the IN part of @bandwidth is set on
|
||||
+ * @ifname's TX, and the OUT part of @bandwidth is set on
|
||||
+ * @ifname's RX. This is needed because for some types of
|
||||
+ * interfaces the domain and the host live on the same side of the
|
||||
+ * interface (so domain's RX/TX is host's RX/TX), and for some
|
||||
+ * it's swapped (domain's RX/TX is hosts's TX/RX).
|
||||
*
|
||||
* Return 0 on success, -1 otherwise.
|
||||
*/
|
||||
int
|
||||
virNetDevBandwidthSet(const char *ifname,
|
||||
const virNetDevBandwidth *bandwidth,
|
||||
- bool hierarchical_class,
|
||||
- bool swapped)
|
||||
+ unsigned int flags)
|
||||
{
|
||||
int ret = -1;
|
||||
virNetDevBandwidthRate *rx = NULL; /* From domain POV */
|
||||
@@ -205,6 +210,7 @@ virNetDevBandwidthSet(const char *ifname,
|
||||
char *average = NULL;
|
||||
char *peak = NULL;
|
||||
char *burst = NULL;
|
||||
+ bool hierarchical_class = flags & VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS;
|
||||
|
||||
if (!bandwidth) {
|
||||
/* nothing to be enabled */
|
||||
@@ -224,7 +230,7 @@ virNetDevBandwidthSet(const char *ifname,
|
||||
return -1;
|
||||
}
|
||||
|
||||
- if (swapped) {
|
||||
+ if (flags & VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED) {
|
||||
rx = bandwidth->out;
|
||||
tx = bandwidth->in;
|
||||
} else {
|
||||
diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
|
||||
index 6d268fb119..80dc654486 100644
|
||||
--- a/src/util/virnetdevbandwidth.h
|
||||
+++ b/src/util/virnetdevbandwidth.h
|
||||
@@ -39,11 +39,16 @@ void virNetDevBandwidthFree(virNetDevBandwidth *def);
|
||||
|
||||
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetDevBandwidth, virNetDevBandwidthFree);
|
||||
|
||||
+typedef enum {
|
||||
+ VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS = (1 << 0),
|
||||
+ VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED = (1 << 1),
|
||||
+} virNetDevBandwidthSetFlags;
|
||||
+
|
||||
int virNetDevBandwidthSet(const char *ifname,
|
||||
const virNetDevBandwidth *bandwidth,
|
||||
- bool hierarchical_class,
|
||||
- bool swapped)
|
||||
+ unsigned int flags)
|
||||
G_GNUC_WARN_UNUSED_RESULT;
|
||||
+
|
||||
int virNetDevBandwidthClear(const char *ifname);
|
||||
int virNetDevBandwidthCopy(virNetDevBandwidth **dest,
|
||||
const virNetDevBandwidth *src)
|
||||
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
|
||||
index f7c38faa2e..6529ff4026 100644
|
||||
--- a/tests/virnetdevbandwidthtest.c
|
||||
+++ b/tests/virnetdevbandwidthtest.c
|
||||
@@ -82,8 +82,14 @@ testVirNetDevBandwidthSet(const void *data)
|
||||
if (virNetDevOpenvswitchInterfaceSetQos(iface, band, info->uuid, true) < 0)
|
||||
return -1;
|
||||
} else {
|
||||
+ unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+
|
||||
+ if (info->hierarchical_class)
|
||||
+ flags |= VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS;
|
||||
+
|
||||
exp_cmd = info->exp_cmd_tc;
|
||||
- if (virNetDevBandwidthSet(iface, band, info->hierarchical_class, true) < 0)
|
||||
+
|
||||
+ if (virNetDevBandwidthSet(iface, band, flags) < 0)
|
||||
return -1;
|
||||
}
|
||||
|
||||
--
|
||||
2.47.1
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
From: Jim Fehlig <jfehlig@suse.com>
|
||||
Date: Fri, 3 Apr 2020 15:51:48 -0600
|
||||
Subject: [PATCH] libxl: fix crash when initializing driver
|
||||
MIME-Version: 1.0
|
||||
Content-Type: text/plain; charset=UTF-8
|
||||
Content-Transfer-Encoding: 8bit
|
||||
|
||||
Commit 54a401af478 split out DriverConfigInit from DriverConfigNew, but
|
||||
then called it a bit late from libxlStateInitialize. The cfg is used in
|
||||
libxlDriverConfigLoadFile and when uninitialized results in a crash.
|
||||
Calling DriverConfigInit immediately after DriverConfigNew fixes the
|
||||
crash.
|
||||
|
||||
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
|
||||
Reviewed-by: Erik Skultety <eskultet@redhat.com>
|
||||
Reviewed-by: Ján Tomko <jtomko@redhat.com>
|
||||
(cherry picked from commit 88011ed280c4f946a7b8e7ffcea2335eb075de60)
|
||||
---
|
||||
src/libxl/libxl_driver.c | 6 +++---
|
||||
1 file changed, 3 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
|
||||
index f2387e2a20..c4fb791fa0 100644
|
||||
--- a/src/libxl/libxl_driver.c
|
||||
+++ b/src/libxl/libxl_driver.c
|
||||
@@ -703,14 +703,14 @@ libxlStateInitialize(bool privileged,
|
||||
if (!(cfg = libxlDriverConfigNew()))
|
||||
goto error;
|
||||
|
||||
+ if (libxlDriverConfigInit(cfg) < 0)
|
||||
+ goto error;
|
||||
+
|
||||
driverConf = g_strdup_printf("%s/libxl.conf", cfg->configBaseDir);
|
||||
|
||||
if (libxlDriverConfigLoadFile(cfg, driverConf) < 0)
|
||||
goto error;
|
||||
|
||||
- if (libxlDriverConfigInit(cfg) < 0)
|
||||
- goto error;
|
||||
-
|
||||
/* Register the callbacks providing access to libvirt's event loop */
|
||||
libxl_osevent_register_hooks(cfg->ctx, &libxl_osevent_callbacks, cfg->ctx);
|
||||
|
||||
@@ -1,185 +0,0 @@
|
||||
From 490f58382dca2a415a5f16b6133f298d853bb379 Mon Sep 17 00:00:00 2001
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Mon, 25 Nov 2024 22:24:45 -0500
|
||||
Subject: [PATCH 5/9] util: make it optional to clear existing tc
|
||||
qdiscs/filters in virNetDevBandwidthSet()
|
||||
|
||||
virNetDevBandwidthSet() always clears all existing qdiscs and their
|
||||
subordinate filters before adding all the new qdiscs/filters. This is
|
||||
normally exactly what we want, but there is one case (the network
|
||||
driver) where the Qdisc added by virNetDevBandwidthSet() may already
|
||||
be in use by the nftables backend (which will add a rule to fix the
|
||||
checksum of dhcp packets); in that case, we *don't* want
|
||||
virNetDevBandwidthSet() to clear out the qdisc that was already added
|
||||
for nftables, and none of the bandwidth filters have been added yet,
|
||||
so there already aren't any "old" filters that need to be removed
|
||||
either - it is safe to just skip virNetDevBandwidthClear() in this
|
||||
case.
|
||||
|
||||
To allow the network driver to set bandwidth without first clearing
|
||||
it, this patch adds the flag VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL to the
|
||||
virNetDevBandwidthSetFlags enum, and recognizes it in
|
||||
virNetDevBandwidthSet() - if the flag is set, then
|
||||
virNetDevBandwidth() will call virNetDevBandwidthClear() just as it
|
||||
always has. But if the flag isn't set it *won't* call
|
||||
virNetDevBandwidthClear().
|
||||
|
||||
As suggested above, VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL is set for all
|
||||
calls to virNetdevBandwidthSet() except for two places in the network
|
||||
driver.
|
||||
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
---
|
||||
src/lxc/lxc_driver.c | 2 +-
|
||||
src/lxc/lxc_process.c | 2 +-
|
||||
src/qemu/qemu_command.c | 2 +-
|
||||
src/qemu/qemu_driver.c | 2 +-
|
||||
src/qemu/qemu_hotplug.c | 4 ++--
|
||||
src/util/virnetdevbandwidth.c | 21 ++++++++++++++++++++-
|
||||
src/util/virnetdevbandwidth.h | 1 +
|
||||
tests/virnetdevbandwidthtest.c | 3 ++-
|
||||
8 files changed, 29 insertions(+), 8 deletions(-)
|
||||
|
||||
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
|
||||
index b693980dbb..81581c74df 100644
|
||||
--- a/src/lxc/lxc_driver.c
|
||||
+++ b/src/lxc/lxc_driver.c
|
||||
@@ -3570,7 +3570,7 @@ lxcDomainAttachDeviceNetLive(virLXCDriver *driver,
|
||||
actualBandwidth = virDomainNetGetActualBandwidth(net);
|
||||
if (actualBandwidth) {
|
||||
if (virNetDevSupportsBandwidth(actualType)) {
|
||||
- unsigned int flags = 0;
|
||||
+ unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
|
||||
|
||||
if (!virDomainNetTypeSharesHostView(net))
|
||||
flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
|
||||
index 0e689fbb70..081ce03a57 100644
|
||||
--- a/src/lxc/lxc_process.c
|
||||
+++ b/src/lxc/lxc_process.c
|
||||
@@ -605,7 +605,7 @@ virLXCProcessSetupInterfaces(virLXCDriver *driver,
|
||||
actualBandwidth = virDomainNetGetActualBandwidth(net);
|
||||
if (actualBandwidth) {
|
||||
if (virNetDevSupportsBandwidth(type)) {
|
||||
- unsigned int flags = 0;
|
||||
+ unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
|
||||
|
||||
if (!virDomainNetTypeSharesHostView(net))
|
||||
flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
|
||||
index b4815e5e71..ed54fd4c5b 100644
|
||||
--- a/src/qemu/qemu_command.c
|
||||
+++ b/src/qemu/qemu_command.c
|
||||
@@ -8841,7 +8841,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriver *driver,
|
||||
!virDomainNetTypeSharesHostView(net)) < 0)
|
||||
goto cleanup;
|
||||
} else {
|
||||
- unsigned int flags = 0;
|
||||
+ unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
|
||||
|
||||
if (!virDomainNetTypeSharesHostView(net))
|
||||
flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
|
||||
index 14929616e5..9549065b1f 100644
|
||||
--- a/src/qemu/qemu_driver.c
|
||||
+++ b/src/qemu/qemu_driver.c
|
||||
@@ -9942,7 +9942,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
|
||||
goto endjob;
|
||||
}
|
||||
} else {
|
||||
- unsigned int bwflags = 0;
|
||||
+ unsigned int bwflags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
|
||||
|
||||
if (!virDomainNetTypeSharesHostView(net))
|
||||
bwflags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
|
||||
index d5e7e99359..ceda4119cd 100644
|
||||
--- a/src/qemu/qemu_hotplug.c
|
||||
+++ b/src/qemu/qemu_hotplug.c
|
||||
@@ -1280,7 +1280,7 @@ qemuDomainAttachNetDevice(virQEMUDriver *driver,
|
||||
!virDomainNetTypeSharesHostView(net)) < 0)
|
||||
goto cleanup;
|
||||
} else {
|
||||
- int flags = 0;
|
||||
+ int flags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
|
||||
|
||||
if (!virDomainNetTypeSharesHostView(net))
|
||||
flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
@@ -4088,7 +4088,7 @@ qemuDomainChangeNet(virQEMUDriver *driver,
|
||||
!virDomainNetTypeSharesHostView(newdev)) < 0)
|
||||
goto cleanup;
|
||||
} else {
|
||||
- int flags = 0;
|
||||
+ int flags = VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
|
||||
|
||||
if (!virDomainNetTypeSharesHostView(newdev))
|
||||
flags |= VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
|
||||
index 1baad849c6..9c48844c5d 100644
|
||||
--- a/src/util/virnetdevbandwidth.c
|
||||
+++ b/src/util/virnetdevbandwidth.c
|
||||
@@ -196,6 +196,21 @@ virNetDevBandwidthManipulateFilter(const char *ifname,
|
||||
* interface (so domain's RX/TX is host's RX/TX), and for some
|
||||
* it's swapped (domain's RX/TX is hosts's TX/RX).
|
||||
*
|
||||
+ * VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL
|
||||
+ * If VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL is set, then the root
|
||||
+ * qdisc is deleted before adding any new qdisc/class/filter,
|
||||
+ * which causes any pre-existing filters to also be deleted. If
|
||||
+ * not set, then it's assumed that there are no existing rules (or
|
||||
+ * that those already there need to be kept). The caller should
|
||||
+ * set this flag for an existing interface that is having its
|
||||
+ * bandwidth settings modified, but can leave it unset if the
|
||||
+ * interface was newly created and this is the first time
|
||||
+ * bandwidth has been set, but someone else might have already
|
||||
+ * added the qdisc (e.g. this is the case when the network driver
|
||||
+ * is setting bandwidth for a virtual network bridge device - the
|
||||
+ * nftables backend may have already added qdisc handle 1:0 and a
|
||||
+ * filter, and we don't want to delete them)
|
||||
+ *
|
||||
* Return 0 on success, -1 otherwise.
|
||||
*/
|
||||
int
|
||||
@@ -238,7 +253,11 @@ virNetDevBandwidthSet(const char *ifname,
|
||||
tx = bandwidth->out;
|
||||
}
|
||||
|
||||
- virNetDevBandwidthClear(ifname);
|
||||
+ /* Only if the caller requests, clear everything including root
|
||||
+ * qdisc and all filters before adding everything.
|
||||
+ */
|
||||
+ if (flags & VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL)
|
||||
+ virNetDevBandwidthClear(ifname);
|
||||
|
||||
if (tx && tx->average) {
|
||||
average = g_strdup_printf("%llukbps", tx->average);
|
||||
diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
|
||||
index 80dc654486..744aa4c826 100644
|
||||
--- a/src/util/virnetdevbandwidth.h
|
||||
+++ b/src/util/virnetdevbandwidth.h
|
||||
@@ -42,6 +42,7 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetDevBandwidth, virNetDevBandwidthFree);
|
||||
typedef enum {
|
||||
VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS = (1 << 0),
|
||||
VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED = (1 << 1),
|
||||
+ VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL = (1 << 2),
|
||||
} virNetDevBandwidthSetFlags;
|
||||
|
||||
int virNetDevBandwidthSet(const char *ifname,
|
||||
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
|
||||
index 6529ff4026..6d5c847ad7 100644
|
||||
--- a/tests/virnetdevbandwidthtest.c
|
||||
+++ b/tests/virnetdevbandwidthtest.c
|
||||
@@ -82,7 +82,8 @@ testVirNetDevBandwidthSet(const void *data)
|
||||
if (virNetDevOpenvswitchInterfaceSetQos(iface, band, info->uuid, true) < 0)
|
||||
return -1;
|
||||
} else {
|
||||
- unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED;
|
||||
+ unsigned int flags = VIR_NETDEV_BANDWIDTH_SET_DIR_SWAPPED |
|
||||
+ VIR_NETDEV_BANDWIDTH_SET_CLEAR_ALL;
|
||||
|
||||
if (info->hierarchical_class)
|
||||
flags |= VIR_NETDEV_BANDWIDTH_SET_HIERARCHICAL_CLASS;
|
||||
--
|
||||
2.47.1
|
||||
|
||||
@@ -1,98 +0,0 @@
|
||||
From faebbbbfa3b1bd4120852b3f416c8073ab82d5c5 Mon Sep 17 00:00:00 2001
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Mon, 25 Nov 2024 22:24:46 -0500
|
||||
Subject: [PATCH 6/9] util: put the command that adds a tx filter qdisc into a
|
||||
separate function
|
||||
|
||||
virNetDevBandwidthSet() adds a queue discipline (qdisc) for each
|
||||
interface that it will need to add tc transmit filters to, and the
|
||||
filters are then attached to the qdisc.
|
||||
|
||||
There are other circumstances where some other function will need to
|
||||
add tc transmit filters to an interface (in particular an upcoming
|
||||
patch to the network driver nftables backend that will use a tc tx
|
||||
filter to fix the checksum of dhcp packets), so that function will
|
||||
also need a qdisc for the tx filter. To assure both always use exactly
|
||||
the same qdisc, this patch puts the command that adds the tx filter
|
||||
qdisc into a separate helper function that can (and will) be called
|
||||
from either place
|
||||
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
---
|
||||
src/libvirt_private.syms | 1 +
|
||||
src/util/virnetdevbandwidth.c | 30 +++++++++++++++++++++++++-----
|
||||
src/util/virnetdevbandwidth.h | 3 +++
|
||||
3 files changed, 29 insertions(+), 5 deletions(-)
|
||||
|
||||
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
|
||||
index d15d6a6a9d..0211cee967 100644
|
||||
--- a/src/libvirt_private.syms
|
||||
+++ b/src/libvirt_private.syms
|
||||
@@ -2859,6 +2859,7 @@ virNetDevVFInterfaceStats;
|
||||
|
||||
|
||||
# util/virnetdevbandwidth.h
|
||||
+virNetDevBandWidthAddTxFilterParentQdisc;
|
||||
virNetDevBandwidthClear;
|
||||
virNetDevBandwidthCopy;
|
||||
virNetDevBandwidthEqual;
|
||||
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
|
||||
index 9c48844c5d..90eebe6576 100644
|
||||
--- a/src/util/virnetdevbandwidth.c
|
||||
+++ b/src/util/virnetdevbandwidth.c
|
||||
@@ -266,11 +266,7 @@ virNetDevBandwidthSet(const char *ifname,
|
||||
if (tx->burst)
|
||||
burst = g_strdup_printf("%llukb", tx->burst);
|
||||
|
||||
- cmd = virCommandNew(TC);
|
||||
- virCommandAddArgList(cmd, "qdisc", "add", "dev", ifname, "root",
|
||||
- "handle", "1:", "htb", "default",
|
||||
- hierarchical_class ? "2" : "1", NULL);
|
||||
- if (virCommandRun(cmd, NULL) < 0)
|
||||
+ if (virNetDevBandWidthAddTxFilterParentQdisc(ifname, hierarchical_class) < 0)
|
||||
goto cleanup;
|
||||
|
||||
/* If we are creating a hierarchical class, all non guaranteed traffic
|
||||
@@ -794,3 +790,27 @@ virNetDevBandwidthSetRootQDisc(const char *ifname,
|
||||
|
||||
return 0;
|
||||
}
|
||||
+
|
||||
+/**
|
||||
+ * virNetDevBandwidthAddTxFilterParentQdisc:
|
||||
+ * @ifname: name of interface that needs a qdisc to attach tx filters to
|
||||
+ * @hierarchical_class: true if hierarchical classes will be used on this interface
|
||||
+ *
|
||||
+ * Add a root Qdisc (Queueing Discipline) for attaching Tx filters to
|
||||
+ * @ifname.
|
||||
+ *
|
||||
+ * returns 0 on success, -1 on failure
|
||||
+ */
|
||||
+int
|
||||
+virNetDevBandWidthAddTxFilterParentQdisc(const char *ifname,
|
||||
+ bool hierarchical_class)
|
||||
+{
|
||||
+ g_autoptr(virCommand) cmd = NULL;
|
||||
+
|
||||
+ cmd = virCommandNew(TC);
|
||||
+ virCommandAddArgList(cmd, "qdisc", "add", "dev", ifname, "root",
|
||||
+ "handle", "1:", "htb", "default",
|
||||
+ hierarchical_class ? "2" : "1", NULL);
|
||||
+
|
||||
+ return virCommandRun(cmd, NULL);
|
||||
+}
|
||||
diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
|
||||
index 744aa4c826..65c1500637 100644
|
||||
--- a/src/util/virnetdevbandwidth.h
|
||||
+++ b/src/util/virnetdevbandwidth.h
|
||||
@@ -84,3 +84,6 @@ int virNetDevBandwidthUpdateFilter(const char *ifname,
|
||||
int virNetDevBandwidthSetRootQDisc(const char *ifname,
|
||||
const char *qdisc)
|
||||
G_NO_INLINE;
|
||||
+
|
||||
+int virNetDevBandWidthAddTxFilterParentQdisc(const char *ifname,
|
||||
+ bool hierarchical_class);
|
||||
--
|
||||
2.47.1
|
||||
|
||||
@@ -1,107 +0,0 @@
|
||||
From 73c0fb19ce5b816ee81ede691252855c75391c9a Mon Sep 17 00:00:00 2001
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Mon, 25 Nov 2024 22:24:47 -0500
|
||||
Subject: [PATCH 7/9] util: don't re-add the qdisc used for tx filters if it
|
||||
already exists
|
||||
|
||||
There will soon be two separate users of tc on virtual networks, and
|
||||
both will use the "qdisc root handle 1: htb" to add tx filters. One or the
|
||||
other could get the first chance to add the qdisc, and then if at a
|
||||
later time the other decides to use it, we need to prevent the 2nd
|
||||
user from attempting to re-add the qdisc (because that just generates
|
||||
an error).
|
||||
|
||||
We do this by running "tc qdisc show dev $bridge handle 1:" then
|
||||
checking if the output of that command contains both "qdisc" and " 1:
|
||||
".[*] If it does then the qdisc has already been added. If not then we
|
||||
need to add it now.
|
||||
|
||||
[*]As of this writing, the output more exactly starts with "qdisc
|
||||
htb 1: root", but our comparison is made purposefully generous to
|
||||
increase the chances that it will continue to work properly if tc
|
||||
modifies the format of its output.
|
||||
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
---
|
||||
src/util/virnetdevbandwidth.c | 35 ++++++++++++++++++++++++++++------
|
||||
tests/virnetdevbandwidthtest.c | 3 +++
|
||||
2 files changed, 32 insertions(+), 6 deletions(-)
|
||||
|
||||
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
|
||||
index 90eebe6576..5c6a65528c 100644
|
||||
--- a/src/util/virnetdevbandwidth.c
|
||||
+++ b/src/util/virnetdevbandwidth.c
|
||||
@@ -805,12 +805,35 @@ int
|
||||
virNetDevBandWidthAddTxFilterParentQdisc(const char *ifname,
|
||||
bool hierarchical_class)
|
||||
{
|
||||
- g_autoptr(virCommand) cmd = NULL;
|
||||
+ g_autoptr(virCommand) testCmd = NULL;
|
||||
+ g_autofree char *testResult = NULL;
|
||||
|
||||
- cmd = virCommandNew(TC);
|
||||
- virCommandAddArgList(cmd, "qdisc", "add", "dev", ifname, "root",
|
||||
- "handle", "1:", "htb", "default",
|
||||
- hierarchical_class ? "2" : "1", NULL);
|
||||
+ /* first check it the qdisc with handle 1: was already added for
|
||||
+ * this interface by someone else
|
||||
+ */
|
||||
+ testCmd = virCommandNew(TC);
|
||||
+ virCommandAddArgList(testCmd, "qdisc", "show", "dev", ifname,
|
||||
+ "handle", "1:", NULL);
|
||||
+ virCommandSetOutputBuffer(testCmd, &testResult);
|
||||
|
||||
- return virCommandRun(cmd, NULL);
|
||||
+ if (virCommandRun(testCmd, NULL) < 0)
|
||||
+ return -1;
|
||||
+
|
||||
+ /* output will be something like: "qdisc htb 1: root refcnt ..."
|
||||
+ * if the qdisc was already added. We just search for "qdisc" and
|
||||
+ * " 1: " anywhere in the output to allow for tc changing its
|
||||
+ * output format.
|
||||
+ */
|
||||
+ if (!(testResult && strstr(testResult, "qdisc") && strstr(testResult, " 1: "))) {
|
||||
+ /* didn't find qdisc in output, so we need to add one */
|
||||
+ g_autoptr(virCommand) addCmd = virCommandNew(TC);
|
||||
+
|
||||
+ virCommandAddArgList(addCmd, "qdisc", "add", "dev", ifname, "root",
|
||||
+ "handle", "1:", "htb", "default",
|
||||
+ hierarchical_class ? "2" : "1", NULL);
|
||||
+
|
||||
+ return virCommandRun(addCmd, NULL);
|
||||
+ }
|
||||
+
|
||||
+ return 0;
|
||||
}
|
||||
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
|
||||
index 6d5c847ad7..31aa7f469d 100644
|
||||
--- a/tests/virnetdevbandwidthtest.c
|
||||
+++ b/tests/virnetdevbandwidthtest.c
|
||||
@@ -147,6 +147,7 @@ mymain(void)
|
||||
"</bandwidth>",
|
||||
TC " qdisc del dev eth0 root\n"
|
||||
TC " qdisc del dev eth0 ingress\n"
|
||||
+ TC " qdisc show dev eth0 handle 1:\n"
|
||||
TC " qdisc add dev eth0 root handle 1: htb default 1\n"
|
||||
TC " class add dev eth0 parent 1: classid 1:1 htb rate 1024kbps quantum 87\n"
|
||||
TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n"
|
||||
@@ -177,6 +178,7 @@ mymain(void)
|
||||
"</bandwidth>",
|
||||
TC " qdisc del dev eth0 root\n"
|
||||
TC " qdisc del dev eth0 ingress\n"
|
||||
+ TC " qdisc show dev eth0 handle 1:\n"
|
||||
TC " qdisc add dev eth0 root handle 1: htb default 1\n"
|
||||
TC " class add dev eth0 parent 1: classid 1:1 htb rate 1kbps ceil 2kbps burst 4kb quantum 1\n"
|
||||
TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n"
|
||||
@@ -199,6 +201,7 @@ mymain(void)
|
||||
"</bandwidth>",
|
||||
TC " qdisc del dev eth0 root\n"
|
||||
TC " qdisc del dev eth0 ingress\n"
|
||||
+ TC " qdisc show dev eth0 handle 1:\n"
|
||||
TC " qdisc add dev eth0 root handle 1: htb default 1\n"
|
||||
TC " class add dev eth0 parent 1: classid 1:1 htb rate 4294967295kbps quantum 366503875\n"
|
||||
TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n"
|
||||
--
|
||||
2.47.1
|
||||
|
||||
@@ -1,171 +0,0 @@
|
||||
From dac9cb9030ac03d18f59884864a0a253e3c9f8f1 Mon Sep 17 00:00:00 2001
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Mon, 25 Nov 2024 22:24:48 -0500
|
||||
Subject: [PATCH 8/9] util: add new "tc" layer for virFirewallCmd objects
|
||||
|
||||
If the layer of a virFirewallCmd is "tc", then the "tc" utility will
|
||||
be executed using the arguments that had been added to the
|
||||
virFirewallCmd
|
||||
|
||||
tc layer doesn't support auto-rollback command creation (any rollback
|
||||
needs to be added manually with virFirewallAddRollbackCmd()), and also
|
||||
tc layer isn't supported by the iptables backend (it would have been
|
||||
straightforward to add, but the iptables backend doesn't need it, and
|
||||
I didn't want to take the chance of causing a regression in that
|
||||
code for no good reason).
|
||||
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
---
|
||||
src/network/network_nftables.c | 1 +
|
||||
src/util/virfirewall.c | 66 +++++++++++++++++++++-------------
|
||||
src/util/virfirewall.h | 1 +
|
||||
src/util/virfirewalld.c | 1 +
|
||||
4 files changed, 44 insertions(+), 25 deletions(-)
|
||||
|
||||
diff --git a/src/network/network_nftables.c b/src/network/network_nftables.c
|
||||
index 268d1f12ca..cc184105c3 100644
|
||||
--- a/src/network/network_nftables.c
|
||||
+++ b/src/network/network_nftables.c
|
||||
@@ -73,6 +73,7 @@ VIR_ENUM_IMPL(nftablesLayer,
|
||||
"",
|
||||
"ip",
|
||||
"ip6",
|
||||
+ "",
|
||||
);
|
||||
|
||||
|
||||
diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c
|
||||
index 811b787ecc..9389bcf541 100644
|
||||
--- a/src/util/virfirewall.c
|
||||
+++ b/src/util/virfirewall.c
|
||||
@@ -47,6 +47,7 @@ VIR_ENUM_IMPL(virFirewallLayer,
|
||||
"ethernet",
|
||||
"ipv4",
|
||||
"ipv6",
|
||||
+ "tc",
|
||||
);
|
||||
|
||||
typedef struct _virFirewallGroup virFirewallGroup;
|
||||
@@ -57,6 +58,7 @@ VIR_ENUM_IMPL(virFirewallLayerCommand,
|
||||
EBTABLES,
|
||||
IPTABLES,
|
||||
IP6TABLES,
|
||||
+ TC,
|
||||
);
|
||||
|
||||
struct _virFirewallCmd {
|
||||
@@ -591,6 +593,7 @@ virFirewallCmdIptablesApply(virFirewall *firewall,
|
||||
case VIR_FIREWALL_LAYER_IPV6:
|
||||
virCommandAddArg(cmd, "-w");
|
||||
break;
|
||||
+ case VIR_FIREWALL_LAYER_TC:
|
||||
case VIR_FIREWALL_LAYER_LAST:
|
||||
break;
|
||||
}
|
||||
@@ -672,39 +675,52 @@ virFirewallCmdNftablesApply(virFirewall *firewall G_GNUC_UNUSED,
|
||||
size_t i;
|
||||
int status;
|
||||
|
||||
- cmd = virCommandNew(NFT);
|
||||
+ if (fwCmd->layer == VIR_FIREWALL_LAYER_TC) {
|
||||
|
||||
- if ((virFirewallTransactionGetFlags(firewall) & VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK) &&
|
||||
- fwCmd->argsLen > 1) {
|
||||
- /* skip any leading options to get to command verb */
|
||||
- for (i = 0; i < fwCmd->argsLen - 1; i++) {
|
||||
- if (fwCmd->args[i][0] != '-')
|
||||
- break;
|
||||
- }
|
||||
+ /* for VIR_FIREWALL_LAYER_TC, we run the 'tc' (traffic control) command with
|
||||
+ * the supplied args.
|
||||
+ */
|
||||
+ cmd = virCommandNew(TC);
|
||||
|
||||
- if (i + 1 < fwCmd->argsLen &&
|
||||
- VIR_NFTABLES_ARG_IS_CREATE(fwCmd->args[i])) {
|
||||
+ /* NB: RAW commands don't support auto-rollback command creation */
|
||||
|
||||
- cmdIdx = i;
|
||||
- objectType = fwCmd->args[i + 1];
|
||||
+ } else {
|
||||
|
||||
- /* we currently only handle auto-rollback for rules,
|
||||
- * chains, and tables, and those all can be "rolled
|
||||
- * back" by a delete command using the handle that is
|
||||
- * returned when "-ae" is added to the add/insert
|
||||
- * command.
|
||||
- */
|
||||
- if (STREQ_NULLABLE(objectType, "rule") ||
|
||||
- STREQ_NULLABLE(objectType, "chain") ||
|
||||
- STREQ_NULLABLE(objectType, "table")) {
|
||||
+ cmd = virCommandNew(NFT);
|
||||
|
||||
- needRollback = true;
|
||||
- /* this option to nft instructs it to add the
|
||||
- * "handle" of the created object to stdout
|
||||
+ if ((virFirewallTransactionGetFlags(firewall) & VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK) &&
|
||||
+ fwCmd->argsLen > 1) {
|
||||
+ /* skip any leading options to get to command verb */
|
||||
+ for (i = 0; i < fwCmd->argsLen - 1; i++) {
|
||||
+ if (fwCmd->args[i][0] != '-')
|
||||
+ break;
|
||||
+ }
|
||||
+
|
||||
+ if (i + 1 < fwCmd->argsLen &&
|
||||
+ VIR_NFTABLES_ARG_IS_CREATE(fwCmd->args[i])) {
|
||||
+
|
||||
+ cmdIdx = i;
|
||||
+ objectType = fwCmd->args[i + 1];
|
||||
+
|
||||
+ /* we currently only handle auto-rollback for rules,
|
||||
+ * chains, and tables, and those all can be "rolled
|
||||
+ * back" by a delete command using the handle that is
|
||||
+ * returned when "-ae" is added to the add/insert
|
||||
+ * command.
|
||||
*/
|
||||
- virCommandAddArg(cmd, "-ae");
|
||||
+ if (STREQ_NULLABLE(objectType, "rule") ||
|
||||
+ STREQ_NULLABLE(objectType, "chain") ||
|
||||
+ STREQ_NULLABLE(objectType, "table")) {
|
||||
+
|
||||
+ needRollback = true;
|
||||
+ /* this option to nft instructs it to add the
|
||||
+ * "handle" of the created object to stdout
|
||||
+ */
|
||||
+ virCommandAddArg(cmd, "-ae");
|
||||
+ }
|
||||
}
|
||||
}
|
||||
+
|
||||
}
|
||||
|
||||
for (i = 0; i < fwCmd->argsLen; i++)
|
||||
diff --git a/src/util/virfirewall.h b/src/util/virfirewall.h
|
||||
index bce51259d2..d42e60884b 100644
|
||||
--- a/src/util/virfirewall.h
|
||||
+++ b/src/util/virfirewall.h
|
||||
@@ -39,6 +39,7 @@ typedef enum {
|
||||
VIR_FIREWALL_LAYER_ETHERNET,
|
||||
VIR_FIREWALL_LAYER_IPV4,
|
||||
VIR_FIREWALL_LAYER_IPV6,
|
||||
+ VIR_FIREWALL_LAYER_TC,
|
||||
|
||||
VIR_FIREWALL_LAYER_LAST,
|
||||
} virFirewallLayer;
|
||||
diff --git a/src/util/virfirewalld.c b/src/util/virfirewalld.c
|
||||
index 827e201dbb..124523c420 100644
|
||||
--- a/src/util/virfirewalld.c
|
||||
+++ b/src/util/virfirewalld.c
|
||||
@@ -43,6 +43,7 @@ VIR_LOG_INIT("util.firewalld");
|
||||
VIR_ENUM_DECL(virFirewallLayerFirewallD);
|
||||
VIR_ENUM_IMPL(virFirewallLayerFirewallD,
|
||||
VIR_FIREWALL_LAYER_LAST,
|
||||
+ "",
|
||||
"eb",
|
||||
"ipv4",
|
||||
"ipv6",
|
||||
--
|
||||
2.47.1
|
||||
|
||||
@@ -1,687 +0,0 @@
|
||||
From b1e2318a0d609fcdff04fcf88953ea87cdd02b95 Mon Sep 17 00:00:00 2001
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Mon, 25 Nov 2024 22:24:49 -0500
|
||||
Subject: [PATCH 9/9] network: add tc filter rule to nftables backend to fix
|
||||
checksum of DHCP responses
|
||||
|
||||
Please see the commit log for commit v10.9.0-rc1-1-g42ab0148dd for the
|
||||
history and explanation of the problem that this patch is fixing.
|
||||
|
||||
A shorter explanation is that when a guest is connected to a libvirt
|
||||
virtual network using a virtio-net adapter with in-kernel "vhost-net"
|
||||
packet processing enabled, it will fail to acquire an IP address from
|
||||
a DHCP seever running on the host.
|
||||
|
||||
In commit v10.9.0-rc1-1-g42ab0148dd we tried fixing this by *zeroing
|
||||
out* the checksums of these packets with an nftables rule (nftables
|
||||
can't recompute the checksum, but it can set it to 0) . This
|
||||
*appeared* to work initially, but it turned out that zeroing the
|
||||
checksum ends up breaking dhcp packets on *non* virtio/vhost-net guest
|
||||
interfaces. That attempt was reverted in commit v10.9.0-rc2.
|
||||
|
||||
Fortunately, there is an existing way to recompute the checksum of a
|
||||
packet as it leaves an interface - the "tc" (traffic control) utility
|
||||
that libvirt already uses for bandwidth management. This patch uses a
|
||||
tc filter rule to match dhcp response packets on the bridge and
|
||||
recompute their checksum.
|
||||
|
||||
The filter rule must be attached to a tc qdisc, which may also have a
|
||||
filter attached for bandwidth management (in the <bandwidth> element
|
||||
of the network config). Not only must we add the qdisc only once
|
||||
(which was already handled by the patch two prior to this one), but
|
||||
also the filter rule for checksum fixing and the filter rule for
|
||||
bandwidth management must be different priorities so they don't clash;
|
||||
this is solved by adding the checksum-fix filter with "priority 2",
|
||||
while the bandwidth management filter remains "priority 1" (both will
|
||||
always be evaluated anyway, it's just a matter of which is evaluated
|
||||
first).
|
||||
|
||||
So far this method has worked with every different guest we could
|
||||
throw at it, including several that failed with the previous method.
|
||||
|
||||
Fixes: b89c4991daa0ee9371f10937fab3b03c5ffdabc6
|
||||
Reported-by: Rich Jones <rjones@redhat.com>
|
||||
Reported-by: Andrea Bolognani <abologna@redhat.com>
|
||||
Fix-Suggested-by: Eric Garver <egarver@redhat.com>
|
||||
Fix-Suggested-by: Phil Sutter <psutter@redhat.com>
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
|
||||
---
|
||||
src/network/network_nftables.c | 68 +++++++++++++++++++
|
||||
.../forward-dev-linux.nftables | 40 +++++++++++
|
||||
.../isolated-linux.nftables | 40 +++++++++++
|
||||
.../nat-default-linux.nftables | 40 +++++++++++
|
||||
.../nat-ipv6-linux.nftables | 40 +++++++++++
|
||||
.../nat-ipv6-masquerade-linux.nftables | 40 +++++++++++
|
||||
.../nat-many-ips-linux.nftables | 40 +++++++++++
|
||||
.../nat-no-dhcp-linux.nftables | 40 +++++++++++
|
||||
.../nat-port-range-ipv6-linux.nftables | 40 +++++++++++
|
||||
.../nat-port-range-linux.nftables | 40 +++++++++++
|
||||
.../nat-tftp-linux.nftables | 40 +++++++++++
|
||||
.../route-default-linux.nftables | 40 +++++++++++
|
||||
12 files changed, 508 insertions(+)
|
||||
|
||||
diff --git a/src/network/network_nftables.c b/src/network/network_nftables.c
|
||||
index cc184105c3..748edb0273 100644
|
||||
--- a/src/network/network_nftables.c
|
||||
+++ b/src/network/network_nftables.c
|
||||
@@ -29,6 +29,7 @@
|
||||
|
||||
#include "internal.h"
|
||||
#include "virfirewalld.h"
|
||||
+#include "vircommand.h"
|
||||
#include "virerror.h"
|
||||
#include "virlog.h"
|
||||
#include "virhash.h"
|
||||
@@ -924,6 +925,67 @@ nftablesAddIPSpecificFirewallRules(virFirewall *fw,
|
||||
}
|
||||
|
||||
|
||||
+/**
|
||||
+ * nftablesAddUdpChecksumFixWithTC:
|
||||
+ *
|
||||
+ * Add a tc filter rule to @ifname (the bridge device of this network)
|
||||
+ * that will recompute the checksum of udp packets output from @iface with
|
||||
+ * destination port @port.
|
||||
+ *
|
||||
+ * Normally the checksum should be filled by some part of the basic
|
||||
+ * network stack, but there are cases (e.g. DHCP response packets sent
|
||||
+ * from virtualization host to a QEMU guest when the guest NIC uses
|
||||
+ * vhost-net packet processing) when the host (sender) thinks that
|
||||
+ * packet checksums will be computed elsewhere (and so leaves a
|
||||
+ * partially computed checksum in the packet header) while the guest
|
||||
+ * (receiver) thinks that the checksum has already been fully
|
||||
+ * computed; in the meantime none of the code in between has actually
|
||||
+ * finished computing the checksum.
|
||||
+ *
|
||||
+ * An example of this is DHCP response packets from host to guest. If
|
||||
+ * the checksum of each of these packets isn't properly computed, then
|
||||
+ * many guests (e.g. FreeBSD) will drop them with reason BAD CHECKSUM;
|
||||
+ * this tc filter rule will fix the ip and udp checksums, and the
|
||||
+ * FreeBSD dhcp client will happily accept the packet.
|
||||
+ *
|
||||
+ * (NB: if you're wondering how the tc qdisc and filter are removed
|
||||
+ * when the network is destroyed, the answer is that the kernel
|
||||
+ * automatically (and properly) removes them for us, so we don't need
|
||||
+ * to worry about keeping track/deleting as we do with nftables rules)
|
||||
+ */
|
||||
+static int
|
||||
+nftablesAddUdpChecksumFixWithTC(virFirewall *fw,
|
||||
+ const char *iface,
|
||||
+ int port)
|
||||
+{
|
||||
+ g_autofree char *portstr = g_strdup_printf("%d", port);
|
||||
+
|
||||
+ /* this will add the qdisc (that the filter below is attached to)
|
||||
+ * unless it already exists
|
||||
+ */
|
||||
+ if (virNetDevBandWidthAddTxFilterParentQdisc(iface, true) < 0)
|
||||
+ return -1;
|
||||
+
|
||||
+ /* add a filter to catch all udp packets with dst "port" and
|
||||
+ * recompute their checksum
|
||||
+ */
|
||||
+ virFirewallAddCmd(fw, VIR_FIREWALL_LAYER_TC,
|
||||
+ "filter", "add", "dev", iface,
|
||||
+ "prio", "2", "protocol", "ip", "parent", "1:",
|
||||
+ "u32", "match", "ip", "dport", portstr, "ffff",
|
||||
+ "action", "csum", "ip", "and", "udp",
|
||||
+ NULL);
|
||||
+
|
||||
+ virFirewallAddRollbackCmd(fw, VIR_FIREWALL_LAYER_TC,
|
||||
+ "filter", "del", "dev", iface,
|
||||
+ "prio", "2", "protocol", "ip", "parent", "1:",
|
||||
+ "u32", "match", "ip", "dport", portstr, "ffff",
|
||||
+ "action", "csum", "ip", "and", "udp",
|
||||
+ NULL);
|
||||
+ return 0;
|
||||
+}
|
||||
+
|
||||
+
|
||||
/* nftablesAddFirewallrules:
|
||||
*
|
||||
* @def - the network that needs an nftables firewall added
|
||||
@@ -944,6 +1006,12 @@ nftablesAddFirewallRules(virNetworkDef *def, virFirewall **fwRemoval)
|
||||
|
||||
virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK);
|
||||
|
||||
+ /* add the tc filter rule needed to fixup the checksum of dhcp
|
||||
+ * response packets going from host to guest.
|
||||
+ */
|
||||
+ if (nftablesAddUdpChecksumFixWithTC(fw, def->bridge, 68) < 0)
|
||||
+ return -1;
|
||||
+
|
||||
nftablesAddGeneralFirewallRules(fw, def);
|
||||
|
||||
for (i = 0;
|
||||
diff --git a/tests/networkxml2firewalldata/forward-dev-linux.nftables b/tests/networkxml2firewalldata/forward-dev-linux.nftables
|
||||
index 8badb74beb..6772383b37 100644
|
||||
--- a/tests/networkxml2firewalldata/forward-dev-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/forward-dev-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/isolated-linux.nftables b/tests/networkxml2firewalldata/isolated-linux.nftables
|
||||
index d1b4dac178..546a18b75a 100644
|
||||
--- a/tests/networkxml2firewalldata/isolated-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/isolated-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-default-linux.nftables b/tests/networkxml2firewalldata/nat-default-linux.nftables
|
||||
index 28508292f9..08623c1381 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-default-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-default-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-ipv6-linux.nftables b/tests/networkxml2firewalldata/nat-ipv6-linux.nftables
|
||||
index d8a9ba706d..3fd6b94eef 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-ipv6-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-ipv6-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables b/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables
|
||||
index a7f09cda59..2811e098d1 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-ipv6-masquerade-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-many-ips-linux.nftables b/tests/networkxml2firewalldata/nat-many-ips-linux.nftables
|
||||
index b826fe6134..5409d5b552 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-many-ips-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-many-ips-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-no-dhcp-linux.nftables b/tests/networkxml2firewalldata/nat-no-dhcp-linux.nftables
|
||||
index d8a9ba706d..3fd6b94eef 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-no-dhcp-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-no-dhcp-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables b/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables
|
||||
index ceaed6fa40..d74417cdb3 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-port-range-ipv6-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-port-range-linux.nftables b/tests/networkxml2firewalldata/nat-port-range-linux.nftables
|
||||
index 1dc37a26ec..b55bb287a9 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-port-range-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-port-range-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/nat-tftp-linux.nftables b/tests/networkxml2firewalldata/nat-tftp-linux.nftables
|
||||
index 28508292f9..08623c1381 100644
|
||||
--- a/tests/networkxml2firewalldata/nat-tftp-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/nat-tftp-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
diff --git a/tests/networkxml2firewalldata/route-default-linux.nftables b/tests/networkxml2firewalldata/route-default-linux.nftables
|
||||
index 282c9542a5..76d6902517 100644
|
||||
--- a/tests/networkxml2firewalldata/route-default-linux.nftables
|
||||
+++ b/tests/networkxml2firewalldata/route-default-linux.nftables
|
||||
@@ -1,3 +1,43 @@
|
||||
+tc \
|
||||
+qdisc \
|
||||
+show \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+handle \
|
||||
+1:
|
||||
+tc \
|
||||
+qdisc \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+root \
|
||||
+handle \
|
||||
+1: \
|
||||
+htb \
|
||||
+default \
|
||||
+2
|
||||
+tc \
|
||||
+filter \
|
||||
+add \
|
||||
+dev \
|
||||
+virbr0 \
|
||||
+prio \
|
||||
+2 \
|
||||
+protocol \
|
||||
+ip \
|
||||
+parent \
|
||||
+1: \
|
||||
+u32 \
|
||||
+match \
|
||||
+ip \
|
||||
+dport \
|
||||
+68 \
|
||||
+ffff \
|
||||
+action \
|
||||
+csum \
|
||||
+ip \
|
||||
+and \
|
||||
+udp
|
||||
nft \
|
||||
-ae insert \
|
||||
rule \
|
||||
--
|
||||
2.47.1
|
||||
|
||||
@@ -1,51 +0,0 @@
|
||||
From 114c0ec656e879ab4d67919914bb24cf5993106d Mon Sep 17 00:00:00 2001
|
||||
Message-ID: <114c0ec656e879ab4d67919914bb24cf5993106d.1734201785.git.crobinso@redhat.com>
|
||||
From: Laine Stump <laine@redhat.com>
|
||||
Date: Mon, 2 Sep 2024 16:13:08 -0400
|
||||
Subject: [PATCH] network: permit <forward mode='open'/> when a network has no
|
||||
IP address
|
||||
Content-type: text/plain
|
||||
|
||||
The whole point of <forward mode='open'/> is to supress libvirt from
|
||||
adding any firewall rules for a network, and someone might want to
|
||||
create a network with no IP address (i.e. they don't want the guests
|
||||
to have connectivity to the host via this interface) and no firewall
|
||||
rules (they don't want any, or they want to add their own). So there's
|
||||
no reason to fail when a network has <forward mode='open'/> and also
|
||||
has no IP address.
|
||||
|
||||
Kind-of-Resolves: https://gitlab.com/libvirt/libvirt/-/issues/588
|
||||
Signed-off-by: Laine Stump <laine@redhat.com>
|
||||
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
|
||||
Signed-off-by: Cole Robinson <crobinso@redhat.com>
|
||||
---
|
||||
src/conf/network_conf.c | 5 +++--
|
||||
1 file changed, 3 insertions(+), 2 deletions(-)
|
||||
|
||||
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
|
||||
index 5cf419acf1..320e1b089a 100644
|
||||
--- a/src/conf/network_conf.c
|
||||
+++ b/src/conf/network_conf.c
|
||||
@@ -1789,7 +1789,6 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt,
|
||||
|
||||
case VIR_NETWORK_FORWARD_ROUTE:
|
||||
case VIR_NETWORK_FORWARD_NAT:
|
||||
- case VIR_NETWORK_FORWARD_OPEN:
|
||||
/* It's pointless to specify L3 forwarding without specifying
|
||||
* the network we're on.
|
||||
*/
|
||||
@@ -1806,8 +1805,10 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt,
|
||||
def->name);
|
||||
return NULL;
|
||||
}
|
||||
+ break;
|
||||
|
||||
- if (def->forward.type == VIR_NETWORK_FORWARD_OPEN && def->forward.nifs) {
|
||||
+ case VIR_NETWORK_FORWARD_OPEN:
|
||||
+ if (def->forward.nifs) {
|
||||
/* an open network by definition can't place any restrictions
|
||||
* on what traffic is allowed or where it goes, so specifying
|
||||
* a forwarding device is nonsensical.
|
||||
--
|
||||
2.47.1
|
||||
|
||||
@@ -1,64 +0,0 @@
|
||||
From d51179fa82448f4720f1645f0b7100df80508cc4 Mon Sep 17 00:00:00 2001
|
||||
From: Pavel Hrdina <phrdina@redhat.com>
|
||||
Date: Thu, 9 Jan 2025 16:23:44 +0100
|
||||
Subject: [PATCH] qemu: snapshot: delete disk image only if parent snapshot is
|
||||
external
|
||||
Content-type: text/plain
|
||||
|
||||
When we are deleting external snapshot that is not active we only need
|
||||
to delete overlay disk image of the parent snapshot. This works
|
||||
correctly even if parent snapshot is external and active as it will have
|
||||
another overlay created when user reverted to that snapshot.
|
||||
|
||||
In case the parent snapshot is internal there are no overlay disk images
|
||||
created as everything is stored internally within the disk image. In
|
||||
this case we would delete the actual disk image storing internal
|
||||
snapshots and most likely the original disk image as well resulting in
|
||||
data loss once the VM is shutoff.
|
||||
|
||||
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/734
|
||||
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
|
||||
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
|
||||
---
|
||||
src/qemu/qemu_snapshot.c | 14 ++++++++------
|
||||
1 file changed, 8 insertions(+), 6 deletions(-)
|
||||
|
||||
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
|
||||
index 18b2e478f6..80cd54bf33 100644
|
||||
--- a/src/qemu/qemu_snapshot.c
|
||||
+++ b/src/qemu/qemu_snapshot.c
|
||||
@@ -3144,6 +3144,8 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
|
||||
return -1;
|
||||
}
|
||||
|
||||
+ data->parentSnap = qemuSnapshotFindParentSnapForDisk(snap, data->snapDisk);
|
||||
+
|
||||
if (data->merge) {
|
||||
virStorageSource *snapDiskSrc = NULL;
|
||||
|
||||
@@ -3185,8 +3187,6 @@ qemuSnapshotDeleteExternalPrepareData(virDomainObj *vm,
|
||||
qemuSnapshotGetDisksWithBackingStore(vm, snap, data);
|
||||
}
|
||||
|
||||
- data->parentSnap = qemuSnapshotFindParentSnapForDisk(snap, data->snapDisk);
|
||||
-
|
||||
if (data->parentSnap && !virDomainSnapshotIsExternal(data->parentSnap)) {
|
||||
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
|
||||
_("deleting external snapshot that has internal snapshot as parent not supported"));
|
||||
@@ -3642,10 +3642,12 @@ qemuSnapshotDiscardExternal(virDomainObj *vm,
|
||||
if (!data->job)
|
||||
goto error;
|
||||
} else {
|
||||
- if (virStorageSourceInit(data->parentDomDisk->src) < 0 ||
|
||||
- virStorageSourceUnlink(data->parentDomDisk->src) < 0) {
|
||||
- VIR_WARN("Failed to remove snapshot image '%s'",
|
||||
- data->snapDisk->name);
|
||||
+ if (data->parentSnap && virDomainSnapshotIsExternal(data->parentSnap)) {
|
||||
+ if (virStorageSourceInit(data->parentDomDisk->src) < 0 ||
|
||||
+ virStorageSourceUnlink(data->parentDomDisk->src) < 0) {
|
||||
+ VIR_WARN("Failed to remove snapshot image '%s'",
|
||||
+ data->snapDisk->name);
|
||||
+ }
|
||||
}
|
||||
}
|
||||
}
|
||||
+937
-1740
File diff suppressed because it is too large
Load Diff
@@ -1 +1 @@
|
||||
SHA512 (libvirt-10.6.0.tar.xz) = edec79e89669d5e9a46be35e0d6334a6ed3bbf32426679549bd998bde24cba52b0378843f41a3abb5d781ad53e2a6a54619a0bad3f168c11fb41736cc6af6568
|
||||
SHA512 (libvirt-6.1.0.tar.xz) = 17a2641f300a4a05149261bae74ac856e9a2511a259146595d2e2412c4a0601d88369b0544ba86edc80e433a47cf828317d8de38c6ec86a1b3efaca75294a606
|
||||
|
||||
Reference in New Issue
Block a user