On Mon, Sep 30, 2024 at 11:30:38AM -0400, Laine Stump wrote:
Commit a37bd2a15b8f2e7aa09519c86fe1ba1e59ce113f eliminated a failure
to update *any* change in an interface that was connected via a
network that consisted of a pool of VFs using macvtap passthrough
mode. Unfortunately it caused a regression that results in failure to
update changes to bandwidth/vlan/trustGuestRxFilters in any interface
connected via a network that uses a bridge to connect tap devices.
This fixes that problem by narrowing the usage of the fix in the
earlier patch to only be done in the case that the the interface is
connected via a macvtap+passthrough network.
Signed-off-by: Laine Stump <laine@xxxxxxxxxx>
Fixes: a37bd2a15b8f2e7aa09519c86fe1ba1e59ce113f
---
The alternatives to this are:
1) revert a37bd2a15b8f2e7aa09519c86fe1ba1e59ce113f (but I haven't
checked yet if that will cause problems with the other patches in
that same series) to eliminate the regression but also unfix the
bug that was fixed, or
2) Do nothing and release with the regression.
Wish I'd found this a couple days earlier :-/
src/qemu/qemu_hotplug.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 09a37caf85..4d4bcde1bc 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3918,12 +3918,19 @@ qemuDomainChangeNet(virQEMUDriver *driver,
* free it if we fail for any reason
*/
if (newdev->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
- if (olddev->type == VIR_DOMAIN_NET_TYPE_NETWORK
- && STREQ(olddev->data.network.name, newdev->data.network.name)) {
+ if (olddev->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+ oldType == VIR_DOMAIN_NET_TYPE_DIRECT &&
+ virDomainNetGetActualDirectMode(olddev) == VIR_NETDEV_MACVLAN_MODE_PASSTHRU &&
+ STREQ(olddev->data.network.name, newdev->data.network.name)) {
This also fixes the && not residing on the first line ;) Anyway, I *think* this should be fine. I spent a significant amount of time trying to review the previous series until Michal beat me to sending a review. And now I'm trying to dive into this function again and it is not a simple one to say the least. One thing that should also be fixed is the comment in the other (else) branch which is now not completely true. But it seems it won't make the state worse, so I myself would vote for including this patch in the release. Reviewed-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
/* old and new are type='network', and the network name
- * hasn't changed. In this case we *don't* want to get a
- * new port ("actual device") from the network because we
- * can use the old one (since it hasn't changed).
+ * hasn't changed *and* this is a network where each
+ * connection is allocated exclusive use of a VF
+ * device. In this case we *don't* want to get a new port
+ * ("actual device") from the network because attempting
+ * to allocate a new device would also allocate a
+ * new/different VF, causing the update to fail. And
+ * anyway we can use olddev's actualNetDef (since it
+ * hasn't changed).
*
* So instead we just duplicate *the pointer to* the
* actualNetDef from olddev to newdev so that comparisons
--
2.46.1
Attachment:
signature.asc
Description: PGP signature
