In one of my recent commits, I've introduced
virDomainInterfaceClearQoS() which is a helper that either calls
virNetDevBandwidthClear() ('tc' implementation) or
virNetDevOpenvswitchInterfaceClearQos() (for ovs ifaces). But I
made a micro optimization which leads to a bug: the function
checks whether passed iface has any QoS set and returns early if
it has none. In majority of cases this is right thing to do, but
when removing QoS on virDomainUpdateDeviceFlags() this is
problematic. The new definition (passed as argument to
virDomainInterfaceClearQoS()) contains no QoS (because user
requested its removal) and thus instead of removing the old QoS
setting nothing is done.

Fortunately, the fix is simple - pass olddev which contains the
old QoS setting.

Fixes: 812a146dfe784315edece43d09f8d9e432f8230e
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_hotplug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 4739beead8..c98b0b5d52 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4071,7 +4071,7 @@ qemuDomainChangeNet(virQEMUDriver *driver,
                 goto cleanup;
             }
         } else {
-            if (virDomainInterfaceClearQoS(vm->def, newdev) < 0)
+            if (virDomainInterfaceClearQoS(vm->def, olddev) < 0)
                 goto cleanup;
         }
 
-- 
2.44.1

On Thu, May 30, 2024 at 11:59:36 +0200, Michal Privoznik wrote:
> In one of my recent commits, I've introduced
> virDomainInterfaceClearQoS() which is a helper that either calls
> virNetDevBandwidthClear() ('tc' implementation) or
> virNetDevOpenvswitchInterfaceClearQos() (for ovs ifaces). But I
> made a micro optimization which leads to a bug: the function
> checks whether passed iface has any QoS set and returns early if
> it has none. In majority of cases this is right thing to do, but
> when removing QoS on virDomainUpdateDeviceFlags() this is
> problematic. The new definition (passed as argument to
> virDomainInterfaceClearQoS()) contains no QoS (because user
> requested its removal) and thus instead of removing the old QoS
> setting nothing is done.
> 
> Fortunately, the fix is simple - pass olddev which contains the
> old QoS setting.
> 
> Fixes: 812a146dfe784315edece43d09f8d9e432f8230e
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_hotplug.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 4739beead8..c98b0b5d52 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -4071,7 +4071,7 @@ qemuDomainChangeNet(virQEMUDriver *driver,
>                  goto cleanup;
>              }
>          } else {
> -            if (virDomainInterfaceClearQoS(vm->def, newdev) < 0)
> +            if (virDomainInterfaceClearQoS(vm->def, olddev) < 0)
>                  goto cleanup;
>          }
>  

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>