[PATCH 0/2] qemu: Restore default root qdisc when QoS is cleared out
This fixes my previous series: https://www.redhat.com/archives/libvir-list/2020-October/msg00636.html Michal Prívozník (2): qemu: Set default qdisc before setting bandwidth qemu: Restore default root qdisc when QoS is cleared out src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_driver.c | 9 +++++++++ src/qemu/qemu_hotplug.c | 13 +++++++++++-- 3 files changed, 22 insertions(+), 4 deletions(-) -- 2.26.2
While the code that's setting default qdisc is clever enough to not overwrite any bandwidth (potentially) set by virNetDevBandwidthSet() (and thus the root qdisc htb is not replaced with noqueue), it does print a debug message when that's the case. It's needless. We can set the root qdisc beforehand and let virNetDevBandwidthSet() overwrite it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_hotplug.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5aff89188a..ece7f6a4d3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8127,6 +8127,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, break; } + qemuDomainInterfaceSetDefaultQDisc(driver, net); + /* Set bandwidth or warn if requested and not supported. */ actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { @@ -8141,8 +8143,6 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, } } - qemuDomainInterfaceSetDefaultQDisc(driver, net); - if (net->mtu && virNetDevSetMTU(net->ifname, net->mtu) < 0) goto cleanup; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 2c12ef60af..235e9e177a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1361,6 +1361,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, if (qemuInterfaceStartDevice(net) < 0) goto cleanup; + qemuDomainInterfaceSetDefaultQDisc(driver, net); + /* Set bandwidth or warn if requested and not supported. */ actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { @@ -1379,8 +1381,6 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, virNetDevSetMTU(net->ifname, net->mtu) < 0) goto cleanup; - qemuDomainInterfaceSetDefaultQDisc(driver, net); - for (i = 0; i < tapfdSize; i++) { if (qemuSecuritySetTapFDLabel(driver->securityManager, vm->def, tapfd[i]) < 0) -- 2.26.2
On Fri, Dec 04, 2020 at 02:11:23PM +0100, Michal Privoznik wrote:
While the code that's setting default qdisc is clever enough to not overwrite any bandwidth (potentially) set by virNetDevBandwidthSet() (and thus the root qdisc htb is not replaced with noqueue), it does print a debug message when that's the case. It's needless. We can set the root qdisc beforehand and let virNetDevBandwidthSet() overwrite it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_hotplug.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
When an interface has some bandwidth limitation set (it's root qdisc is htb in that case) but this gets cleared out via public API call (virDomainSetInterfaceParameters() or virDomainUpdateDeviceFlags()) then virNetDevBandwidthSet() clears out whatever qdiscs were set on the interface and kernel places the default qdisc at the root. What we need to do next is to replace the root qdisc with the one we want. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1329644 Fixes: 0b66196d86ea375234cb0ee99289c486f9921820 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 9 +++++++++ src/qemu/qemu_hotplug.c | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4fd70ed300..6eca2645de 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10358,6 +10358,15 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, goto endjob; } + /* If the old bandwidth was cleared out, restore qdisc. */ + if (virDomainNetTypeSharesHostView(net)) { + if (!newBandwidth->out || newBandwidth->out->average == 0) + qemuDomainInterfaceSetDefaultQDisc(driver, net); + } else { + if (!newBandwidth->in || newBandwidth->in->average == 0) + qemuDomainInterfaceSetDefaultQDisc(driver, net); + } + virNetDevBandwidthFree(net->bandwidth); if (newBandwidth->in || newBandwidth->out) { net->bandwidth = newBandwidth; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 235e9e177a..3aa6fb8a27 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3871,6 +3871,15 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, */ virNetDevBandwidthClear(newdev->ifname); } + + /* If the old bandwidth was cleared out, restore qdisc. */ + if (virDomainNetTypeSharesHostView(newdev)) { + if (!newb->out || newb->out->average == 0) + qemuDomainInterfaceSetDefaultQDisc(driver, newdev); + } else { + if (!newb->in || newb->in->average == 0) + qemuDomainInterfaceSetDefaultQDisc(driver, newdev); + } needReplaceDevDef = true; } -- 2.26.2
On Fri, Dec 04, 2020 at 02:11:24PM +0100, Michal Privoznik wrote:
When an interface has some bandwidth limitation set (it's root qdisc is htb in that case) but this gets cleared out via public API call (virDomainSetInterfaceParameters() or virDomainUpdateDeviceFlags()) then virNetDevBandwidthSet() clears out whatever qdiscs were set on the interface and kernel places the default qdisc at the root. What we need to do next is to replace the root qdisc with the one we want.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1329644 Fixes: 0b66196d86ea375234cb0ee99289c486f9921820 Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 9 +++++++++ src/qemu/qemu_hotplug.c | 9 +++++++++ 2 files changed, 18 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrangé -
Michal Privoznik