commit
97ed275e1d
30 changed files with 17675 additions and 0 deletions
@ -0,0 +1,4 @@ |
|||||
|
*.tar.xz filter=lfs diff=lfs merge=lfs -text |
||||
|
*.tar.gz filter=lfs diff=lfs merge=lfs -text |
||||
|
*.tar filter=lfs diff=lfs merge=lfs -text |
||||
|
*.tar.bz2 filter=lfs diff=lfs merge=lfs -text |
||||
@ -0,0 +1,4 @@ |
|||||
|
SRPMS |
||||
|
RPMS |
||||
|
BUILD |
||||
|
BUILDROOT |
||||
@ -0,0 +1,3 @@ |
|||||
|
version https://git-lfs.github.com/spec/v1 |
||||
|
oid sha256:f7882fe65302bbcf804b573e0128c4fc6bfc52c9c3f44852a04de2391d858e34 |
||||
|
size 10093332 |
||||
File diff suppressed because it is too large
@ -0,0 +1,3 @@ |
|||||
|
version https://git-lfs.github.com/spec/v1 |
||||
|
oid sha256:f7882fe65302bbcf804b573e0128c4fc6bfc52c9c3f44852a04de2391d858e34 |
||||
|
size 10093332 |
||||
File diff suppressed because it is too large
@ -0,0 +1,29 @@ |
|||||
|
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,34 @@ |
|||||
|
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,46 @@ |
|||||
|
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,315 @@ |
|||||
|
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,185 @@ |
|||||
|
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 |
||||
|
|
||||
@ -0,0 +1,98 @@ |
|||||
|
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 |
||||
|
|
||||
@ -0,0 +1,107 @@ |
|||||
|
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 |
||||
|
|
||||
@ -0,0 +1,171 @@ |
|||||
|
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 |
||||
|
|
||||
@ -0,0 +1,687 @@ |
|||||
|
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 |
||||
|
|
||||
@ -0,0 +1,51 @@ |
|||||
|
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 |
||||
|
|
||||
@ -0,0 +1,64 @@ |
|||||
|
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);
|
||||
|
+ }
|
||||
|
} |
||||
|
} |
||||
|
} |
||||
@ -0,0 +1,3 @@ |
|||||
|
version https://git-lfs.github.com/spec/v1 |
||||
|
oid sha256:a495b2a26faca841ac0073c7dd7f60857ca81adac9047dac5f698fd75f1342cd |
||||
|
size 9481992 |
||||
File diff suppressed because it is too large
@ -0,0 +1,31 @@ |
|||||
|
From 5629ebcb4234fde10fd9468d5fc5dd4947ed8677 Mon Sep 17 00:00:00 2001 |
||||
|
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com> |
||||
|
Date: Tue, 29 Apr 2025 15:49:10 +0100 |
||||
|
Subject: [PATCH] Fix mocking of virQEMUCapsProbeHVF function |
||||
|
MIME-Version: 1.0 |
||||
|
Content-Type: text/plain; charset=UTF-8 |
||||
|
Content-Transfer-Encoding: 8bit |
||||
|
|
||||
|
From: Daniel P. Berrangé <berrange@redhat.com> |
||||
|
|
||||
|
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> |
||||
|
---
|
||||
|
src/qemu/qemu_capabilities.h | 2 +- |
||||
|
1 file changed, 1 insertion(+), 1 deletion(-) |
||||
|
|
||||
|
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
|
||||
|
index ea7c14daa9..488a1a058e 100644
|
||||
|
--- a/src/qemu/qemu_capabilities.h
|
||||
|
+++ b/src/qemu/qemu_capabilities.h
|
||||
|
@@ -943,7 +943,7 @@ bool
|
||||
|
virQEMUCapsGetKVMSupportsSecureGuest(virQEMUCaps *qemuCaps) G_NO_INLINE; |
||||
|
|
||||
|
bool |
||||
|
-virQEMUCapsProbeHVF(virQEMUCaps *qemuCaps) G_NO_INLINE;
|
||||
|
+virQEMUCapsProbeHVF(virQEMUCaps *qemuCaps) G_NO_INLINE __attribute__((noipa));
|
||||
|
|
||||
|
virArch virQEMUCapsArchFromString(const char *arch); |
||||
|
const char *virQEMUCapsArchToString(virArch arch); |
||||
|
--
|
||||
|
2.49.0 |
||||
|
|
||||
@ -0,0 +1,85 @@ |
|||||
|
From 63a3d70697dc44ef2f8b40f7c8e9aa869227a7da Mon Sep 17 00:00:00 2001 |
||||
|
From: Jiang XueQian <jiangxueqian@gmail.com> |
||||
|
Date: Sat, 18 Jan 2025 16:32:10 +0800 |
||||
|
Subject: [PATCH] nss: Skip empty files and avoid use of uninitialized value |
||||
|
Content-type: text/plain |
||||
|
|
||||
|
JSON parser isn't called when reading empty files so `jerr` will be used |
||||
|
uninitialized in the original code. Empty files appear when a network |
||||
|
has no dhcp clients. |
||||
|
|
||||
|
This patch checks for such files and skip them. |
||||
|
|
||||
|
Fixes: a8d828c88bbdaf83ae78dc06cdd84d5667fcc424 |
||||
|
Signed-off-by: Jiang XueQian <jiangxueqian@gmail.com> |
||||
|
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> |
||||
|
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> |
||||
|
---
|
||||
|
tools/nss/libvirt_nss_leases.c | 9 +++++++-- |
||||
|
tools/nss/libvirt_nss_macs.c | 9 +++++++-- |
||||
|
2 files changed, 14 insertions(+), 4 deletions(-) |
||||
|
|
||||
|
diff --git a/tools/nss/libvirt_nss_leases.c b/tools/nss/libvirt_nss_leases.c
|
||||
|
index aea81bb56e..25ea6b0ce2 100644
|
||||
|
--- a/tools/nss/libvirt_nss_leases.c
|
||||
|
+++ b/tools/nss/libvirt_nss_leases.c
|
||||
|
@@ -263,7 +263,7 @@ findLeases(const char *file,
|
||||
|
enum json_tokener_error jerr; |
||||
|
int jsonflags = JSON_TOKENER_STRICT | JSON_TOKENER_VALIDATE_UTF8; |
||||
|
char line[1024]; |
||||
|
- ssize_t nreadTotal = 0;
|
||||
|
+ size_t nreadTotal = 0;
|
||||
|
int rv; |
||||
|
|
||||
|
if ((fd = open(file, O_RDONLY)) < 0) { |
||||
|
@@ -290,12 +290,17 @@ findLeases(const char *file,
|
||||
|
jerr = json_tokener_get_error(tok); |
||||
|
} while (jerr == json_tokener_continue); |
||||
|
|
||||
|
+ if (nreadTotal == 0) {
|
||||
|
+ ret = 0;
|
||||
|
+ goto cleanup;
|
||||
|
+ }
|
||||
|
+
|
||||
|
if (jerr == json_tokener_continue) { |
||||
|
ERROR("Cannot parse %s: incomplete json found", file); |
||||
|
goto cleanup; |
||||
|
} |
||||
|
|
||||
|
- if (nreadTotal > 0 && jerr != json_tokener_success) {
|
||||
|
+ if (jerr != json_tokener_success) {
|
||||
|
ERROR("Cannot parse %s: %s", file, json_tokener_error_desc(jerr)); |
||||
|
goto cleanup; |
||||
|
} |
||||
|
diff --git a/tools/nss/libvirt_nss_macs.c b/tools/nss/libvirt_nss_macs.c
|
||||
|
index 23229a18f3..bac8c0e1bb 100644
|
||||
|
--- a/tools/nss/libvirt_nss_macs.c
|
||||
|
+++ b/tools/nss/libvirt_nss_macs.c
|
||||
|
@@ -124,7 +124,7 @@ findMACs(const char *file,
|
||||
|
json_tokener *tok = NULL; |
||||
|
enum json_tokener_error jerr; |
||||
|
int jsonflags = JSON_TOKENER_STRICT | JSON_TOKENER_VALIDATE_UTF8; |
||||
|
- ssize_t nreadTotal = 0;
|
||||
|
+ size_t nreadTotal = 0;
|
||||
|
int rv; |
||||
|
size_t i; |
||||
|
|
||||
|
@@ -152,12 +152,17 @@ findMACs(const char *file,
|
||||
|
jerr = json_tokener_get_error(tok); |
||||
|
} while (jerr == json_tokener_continue); |
||||
|
|
||||
|
+ if (nreadTotal == 0) {
|
||||
|
+ ret = 0;
|
||||
|
+ goto cleanup;
|
||||
|
+ }
|
||||
|
+
|
||||
|
if (jerr == json_tokener_continue) { |
||||
|
ERROR("Cannot parse %s: incomplete json found", file); |
||||
|
goto cleanup; |
||||
|
} |
||||
|
|
||||
|
- if (nreadTotal > 0 && jerr != json_tokener_success) {
|
||||
|
+ if (jerr != json_tokener_success) {
|
||||
|
ERROR("Cannot parse %s: %s", file, json_tokener_error_desc(jerr)); |
||||
|
goto cleanup; |
||||
|
} |
||||
@ -0,0 +1,68 @@ |
|||||
|
From cd0de70e05475d5f4aa46e578fbb98033d38c06b Mon Sep 17 00:00:00 2001 |
||||
|
From: Michal Privoznik <mprivozn@redhat.com> |
||||
|
Date: Mon, 16 Jun 2025 10:28:37 +0200 |
||||
|
Subject: [PATCH] qemu: Be more forgiving when acquiring QUERY job when |
||||
|
formatting domain XML |
||||
|
Content-type: text/plain |
||||
|
|
||||
|
In my previous commit of v11.0.0-rc1~115 I've made QEMU driver |
||||
|
implementation for virDomainGetXMLDesc() (qemuDomainGetXMLDesc()) |
||||
|
acquire QERY job. See its commit message for more info. But this |
||||
|
unfortunately broke apps witch fetch domain XML for incoming |
||||
|
migration (like virt-manager). The reason is that for incoming |
||||
|
migration the VIR_ASYNC_JOB_MIGRATION_IN async job is set, but |
||||
|
the mask of allowed synchronous jobs is empty (because QEMU can't |
||||
|
talk on monitor really). This makes virDomainObjBeginJob() fail |
||||
|
which in turn makes qemuDomainGetXMLDesc() fail too. |
||||
|
|
||||
|
It makes sense for qemuDomainGetXMLDesc() to acquire the job |
||||
|
(e.g. so that it's coherent with another thread that might be in |
||||
|
the middle of a MODIFY job). But failure to dump XML may be |
||||
|
treated as broken daemon (e.g. virt-manager does so). |
||||
|
|
||||
|
Therefore, still try to acquire the QUERY job (if job mask |
||||
|
permits it) but, do not treat failure as an error. |
||||
|
|
||||
|
Fixes: 6cc93bf28842526be2fd596a607ebca796b7fb2e |
||||
|
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2369243 |
||||
|
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> |
||||
|
Reviewed-by: Pavel Hrdina <phrdina@redhat.com> |
||||
|
---
|
||||
|
src/qemu/qemu_driver.c | 10 +++++++--- |
||||
|
1 file changed, 7 insertions(+), 3 deletions(-) |
||||
|
|
||||
|
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
|
||||
|
index d2eddbd9ae..6bdeede2e8 100644
|
||||
|
--- a/src/qemu/qemu_driver.c
|
||||
|
+++ b/src/qemu/qemu_driver.c
|
||||
|
@@ -6158,6 +6158,7 @@ static char
|
||||
|
{ |
||||
|
virQEMUDriver *driver = dom->conn->privateData; |
||||
|
virDomainObj *vm; |
||||
|
+ bool hasJob = false;
|
||||
|
char *ret = NULL; |
||||
|
|
||||
|
virCheckFlags(VIR_DOMAIN_XML_COMMON_FLAGS | VIR_DOMAIN_XML_UPDATE_CPU, |
||||
|
@@ -6169,8 +6170,10 @@ static char
|
||||
|
if (virDomainGetXMLDescEnsureACL(dom->conn, vm->def, flags) < 0) |
||||
|
goto cleanup; |
||||
|
|
||||
|
- if (virDomainObjBeginJob(vm, VIR_JOB_QUERY) < 0)
|
||||
|
- goto cleanup;
|
||||
|
+ if (virDomainNestedJobAllowed(vm->job, VIR_JOB_QUERY) &&
|
||||
|
+ virDomainObjBeginJob(vm, VIR_JOB_QUERY) >= 0) {
|
||||
|
+ hasJob = true;
|
||||
|
+ }
|
||||
|
|
||||
|
qemuDomainUpdateCurrentMemorySize(vm); |
||||
|
|
||||
|
@@ -6186,7 +6189,8 @@ static char
|
||||
|
|
||||
|
ret = qemuDomainFormatXML(driver, vm, flags); |
||||
|
|
||||
|
- virDomainObjEndJob(vm);
|
||||
|
+ if (hasJob)
|
||||
|
+ virDomainObjEndJob(vm);
|
||||
|
|
||||
|
cleanup: |
||||
|
virDomainObjEndAPI(&vm); |
||||
@ -0,0 +1,94 @@ |
|||||
|
From 63e4cbd109374f44e8bd4f8d1af5e2a2c67611bc Mon Sep 17 00:00:00 2001 |
||||
|
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com> |
||||
|
Date: Mon, 28 Apr 2025 11:42:13 +0100 |
||||
|
Subject: [PATCH] storage: stop hardcoding paths for mkfs, mount, umount |
||||
|
MIME-Version: 1.0 |
||||
|
Content-Type: text/plain; charset=UTF-8 |
||||
|
Content-Transfer-Encoding: 8bit |
||||
|
|
||||
|
From: Daniel P. Berrangé <berrange@redhat.com> |
||||
|
|
||||
|
This was always undesirable but now causes problems on Fedora 42 |
||||
|
where at build time we detect a /sbin path but at runtime this |
||||
|
will only exist on upgraded machines, not fresh installs. |
||||
|
|
||||
|
Reviewed-by: Peter Krempa <pkrempa@redhat.com> |
||||
|
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> |
||||
|
---
|
||||
|
meson.build | 13 ------------- |
||||
|
src/storage/storage_backend_fs.c | 17 +++-------------- |
||||
|
2 files changed, 3 insertions(+), 27 deletions(-) |
||||
|
|
||||
|
diff --git a/meson.build b/meson.build
|
||||
|
index 37b1caa566..14c98b49a1 100644
|
||||
|
--- a/meson.build
|
||||
|
+++ b/meson.build
|
||||
|
@@ -1827,23 +1827,10 @@ if conf.has('WITH_LIBVIRTD')
|
||||
|
endif |
||||
|
endif |
||||
|
|
||||
|
- if fs_enable
|
||||
|
- mount_prog = find_program('mount', required: get_option('storage_fs'), dirs: libvirt_sbin_path)
|
||||
|
- umount_prog = find_program('umount', required: get_option('storage_fs'), dirs: libvirt_sbin_path)
|
||||
|
- mkfs_prog = find_program('mkfs', required: get_option('storage_fs'), dirs: libvirt_sbin_path)
|
||||
|
-
|
||||
|
- if not mount_prog.found() or not umount_prog.found() or not mkfs_prog.found()
|
||||
|
- fs_enable = false
|
||||
|
- endif
|
||||
|
- endif
|
||||
|
-
|
||||
|
if fs_enable |
||||
|
use_storage = true |
||||
|
|
||||
|
conf.set('WITH_STORAGE_FS', 1) |
||||
|
- conf.set_quoted('MOUNT', mount_prog.full_path())
|
||||
|
- conf.set_quoted('UMOUNT', umount_prog.full_path())
|
||||
|
- conf.set_quoted('MKFS', mkfs_prog.full_path())
|
||||
|
endif |
||||
|
endif |
||||
|
|
||||
|
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
|
||||
|
index fce395d60f..6ec359625a 100644
|
||||
|
--- a/src/storage/storage_backend_fs.c
|
||||
|
+++ b/src/storage/storage_backend_fs.c
|
||||
|
@@ -304,7 +304,7 @@ virStorageBackendFileSystemMount(virStoragePoolObj *pool)
|
||||
|
if (!(src = virStorageBackendFileSystemGetPoolSource(pool))) |
||||
|
return -1; |
||||
|
|
||||
|
- cmd = virStorageBackendFileSystemMountCmd(MOUNT, def, src);
|
||||
|
+ cmd = virStorageBackendFileSystemMountCmd("mount", def, src);
|
||||
|
|
||||
|
/* Mounting a shared FS might take a long time. Don't hold |
||||
|
* the pool locked meanwhile. */ |
||||
|
@@ -362,7 +362,7 @@ virStorageBackendFileSystemStop(virStoragePoolObj *pool)
|
||||
|
if ((rc = virStorageBackendFileSystemIsMounted(pool)) != 1) |
||||
|
return rc; |
||||
|
|
||||
|
- cmd = virCommandNewArgList(UMOUNT, def->target.path, NULL);
|
||||
|
+ cmd = virCommandNewArgList("umount", def->target.path, NULL);
|
||||
|
return virCommandRun(cmd, NULL); |
||||
|
} |
||||
|
#endif /* WITH_STORAGE_FS */ |
||||
|
@@ -402,18 +402,7 @@ virStorageBackendExecuteMKFS(const char *device,
|
||||
|
g_autoptr(virCommand) cmd = NULL; |
||||
|
g_autofree char *mkfs = NULL; |
||||
|
|
||||
|
-#if WITH_STORAGE_FS
|
||||
|
- mkfs = virFindFileInPath(MKFS);
|
||||
|
-#endif /* WITH_STORAGE_FS */
|
||||
|
-
|
||||
|
- if (!mkfs) {
|
||||
|
- virReportError(VIR_ERR_INTERNAL_ERROR,
|
||||
|
- _("mkfs is not available on this platform: Failed to make filesystem of type '%1$s' on device '%2$s'"),
|
||||
|
- format, device);
|
||||
|
- return -1;
|
||||
|
- }
|
||||
|
-
|
||||
|
- cmd = virCommandNewArgList(mkfs, "-t", format, NULL);
|
||||
|
+ cmd = virCommandNewArgList("mkfs", "-t", format, NULL);
|
||||
|
|
||||
|
/* use the force, otherwise mkfs.xfs won't overwrite existing fs. |
||||
|
* Similarly mkfs.ext2, mkfs.ext3, and mkfs.ext4 require supplying -F |
||||
|
--
|
||||
|
2.49.0 |
||||
|
|
||||
@ -0,0 +1,43 @@ |
|||||
|
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com> |
||||
|
To: devel@lists.libvirt.org |
||||
|
Subject: [PATCH] util: avoid overflow in hextable buffer |
||||
|
Date: Mon, 20 Jan 2025 10:09:24 +0000 |
||||
|
Message-ID: <20250120100924.3864818-1-berrange@redhat.com> |
||||
|
MIME-Version: 1.0 |
||||
|
Content-Type: text/plain; charset=UTF-8 |
||||
|
Content-Transfer-Encoding: 8bit |
||||
|
|
||||
|
The assigned string is 17 chars long once the trailing nul is taken |
||||
|
into account. This triggers a warning with GCC 15 |
||||
|
|
||||
|
src/util/virsystemd.c: In function ‘virSystemdEscapeName’: |
||||
|
src/util/virsystemd.c:59:38: error: initializer-string for array of ‘char’ is too long [-Werror=unterminated-string-initialization] |
||||
|
59 | static const char hextable[16] = "0123456789abcdef"; |
||||
|
| ^~~~~~~~~~~~~~~~~~ |
||||
|
|
||||
|
Switch to a dynamically sized array as used in all the other places |
||||
|
we have a hextable array. |
||||
|
|
||||
|
See also: https://gcc.gnu.org/PR115185 |
||||
|
Reported-by: Yaakov Selkowitz <yselkowi@redhat.com> |
||||
|
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> |
||||
|
---
|
||||
|
src/util/virsystemd.c | 2 +- |
||||
|
1 file changed, 1 insertion(+), 1 deletion(-) |
||||
|
|
||||
|
diff --git a/src/util/virsystemd.c b/src/util/virsystemd.c
|
||||
|
index 5b772e29dd..d46e5f74fc 100644
|
||||
|
--- a/src/util/virsystemd.c
|
||||
|
+++ b/src/util/virsystemd.c
|
||||
|
@@ -56,7 +56,7 @@ struct _virSystemdActivationEntry {
|
||||
|
static void virSystemdEscapeName(virBuffer *buf, |
||||
|
const char *name) |
||||
|
{ |
||||
|
- static const char hextable[16] = "0123456789abcdef";
|
||||
|
+ static const char hextable[] = "0123456789abcdef";
|
||||
|
|
||||
|
#define ESCAPE(c) \ |
||||
|
do { \ |
||||
|
--
|
||||
|
2.47.1 |
||||
|
|
||||
@ -0,0 +1,58 @@ |
|||||
|
From 7ab0f1c2a3fddf46d381f055e49111e3063b4829 Mon Sep 17 00:00:00 2001 |
||||
|
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com> |
||||
|
Date: Mon, 28 Apr 2025 11:47:34 +0100 |
||||
|
Subject: [PATCH] util: stop hardcoding numad path |
||||
|
MIME-Version: 1.0 |
||||
|
Content-Type: text/plain; charset=UTF-8 |
||||
|
Content-Transfer-Encoding: 8bit |
||||
|
|
||||
|
From: Daniel P. Berrangé <berrange@redhat.com> |
||||
|
|
||||
|
Change the meson rules to always enable numad if on a Linux host, unless |
||||
|
the meson options say not to. |
||||
|
|
||||
|
Reviewed-by: Peter Krempa <pkrempa@redhat.com> |
||||
|
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> |
||||
|
---
|
||||
|
meson.build | 10 +++------- |
||||
|
src/util/virnuma.c | 2 +- |
||||
|
2 files changed, 4 insertions(+), 8 deletions(-) |
||||
|
|
||||
|
diff --git a/meson.build b/meson.build
|
||||
|
index 14c98b49a1..767205f44b 100644
|
||||
|
--- a/meson.build
|
||||
|
+++ b/meson.build
|
||||
|
@@ -2028,14 +2028,10 @@ if not get_option('nss').disabled()
|
||||
|
endif |
||||
|
endif |
||||
|
|
||||
|
-if not get_option('numad').disabled() and numactl_dep.found()
|
||||
|
- numad_prog = find_program('numad', required: get_option('numad'), dirs: libvirt_sbin_path)
|
||||
|
- if numad_prog.found()
|
||||
|
- conf.set('WITH_NUMAD', 1)
|
||||
|
- conf.set_quoted('NUMAD', numad_prog.full_path())
|
||||
|
- endif
|
||||
|
+if not get_option('numad').disabled() and numactl_dep.found() and host_machine.system() == 'linux'
|
||||
|
+ conf.set('WITH_NUMAD', 1)
|
||||
|
elif get_option('numad').enabled() |
||||
|
- error('You must have numactl enabled for numad support.')
|
||||
|
+ error('You must have a Linux host with numactl enabled for numad support.')
|
||||
|
endif |
||||
|
|
||||
|
# nwfilter should only be compiled for linux, and only if the |
||||
|
diff --git a/src/util/virnuma.c b/src/util/virnuma.c
|
||||
|
index 9393c20875..67c51630c7 100644
|
||||
|
--- a/src/util/virnuma.c
|
||||
|
+++ b/src/util/virnuma.c
|
||||
|
@@ -61,7 +61,7 @@ virNumaGetAutoPlacementAdvice(unsigned short vcpus,
|
||||
|
g_autoptr(virCommand) cmd = NULL; |
||||
|
char *output = NULL; |
||||
|
|
||||
|
- cmd = virCommandNewArgList(NUMAD, "-w", NULL);
|
||||
|
+ cmd = virCommandNewArgList("numad", "-w", NULL);
|
||||
|
virCommandAddArgFormat(cmd, "%d:%llu", vcpus, |
||||
|
VIR_DIV_UP(balloon, 1024)); |
||||
|
|
||||
|
--
|
||||
|
2.49.0 |
||||
|
|
||||
@ -0,0 +1,3 @@ |
|||||
|
version https://git-lfs.github.com/spec/v1 |
||||
|
oid sha256:01a176ff4042ad58cf83c09fe0925d6bc8eed0ecce1e0ee19b8ef4c1ffa3806e |
||||
|
size 9700388 |
||||
File diff suppressed because it is too large
@ -0,0 +1,3 @@ |
|||||
|
version https://git-lfs.github.com/spec/v1 |
||||
|
oid sha256:cc0e8c226559b479833c8bc9c77a8ec301482ab0305fcd98d27f11cc6877fd23 |
||||
|
size 9960064 |
||||
File diff suppressed because it is too large
@ -0,0 +1,20 @@ |
|||||
|
#!/bin/bash |
||||
|
|
||||
|
set -Eeuo pipefail |
||||
|
|
||||
|
for v in 41 42 43; do |
||||
|
dist="fedora-$v" |
||||
|
mkdir -p "$dist/SRPMS" |
||||
|
echo "Downloading libvirt source for Fedora $v..." |
||||
|
dnf download --repofrompath=$dist,https://dl.fedoraproject.org/pub/fedora/linux/releases/$v/Everything/source/tree/ --repofrompath=$dist-updates,https://dl.fedoraproject.org/pub/fedora/linux/updates/$v/Everything/source/tree/ --repo="$dist" --repo="$dist-updates" --source libvirt --destdir $dist/SRPMS/ |
||||
|
rpm -ivh "--define=_topdir $PWD/$dist" $dist/SRPMS/libvirt-*.src.rpm |
||||
|
done |
||||
|
|
||||
|
for v in 9 10; do |
||||
|
dist="centos-$v" |
||||
|
mkdir -p "$dist/SRPMS" |
||||
|
echo "Downloading libvirt source for CentOS Stream $v..." |
||||
|
dnf download --repofrompath=$dist,https://mirror.stream.centos.org/$v-stream/AppStream/source/tree/ --repo="$dist" --source libvirt --destdir $dist/SRPMS/ |
||||
|
rpm -ivh "--define=_topdir $PWD/$dist" $dist/SRPMS/libvirt-*.src.rpm |
||||
|
done |
||||
|
|
||||
Loading…
Reference in new issue