[libvirt] [PATCH] qemuDomainSetInterfaceParameters: Allow bandwidth clear out
We allow translation from no_bandwidth to has_bandwidth for a vnic. However, going in the opposite direction is not implemented. It's not limitation of the API rather than internal implementation. The problem is, we correctly detect that user hasn't specified any outbound (say he wants to clear out outbound). However, this gets overwritten by current vnic outbound settings. Then, virNetDevBandwidthSet doesn't change anything. We need to stop overwriting the outbound if users don't want us to. Same applies for inbound. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 15 +++++++++++---- tools/virsh-domain.c | 4 ++-- tools/virsh.pod | 3 +++ 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fc382a5..26ac79b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9782,6 +9782,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, virNetDevBandwidthPtr bandwidth = NULL, newBandwidth = NULL; virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + bool inboundSpecified = false, outboundSpecified = false; virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); @@ -9843,12 +9844,14 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE)) { bandwidth->in->average = params[i].value.ui; + inboundSpecified = true; } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_PEAK)) { bandwidth->in->peak = params[i].value.ui; } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_BURST)) { bandwidth->in->burst = params[i].value.ui; } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE)) { bandwidth->out->average = params[i].value.ui; + outboundSpecified = true; } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK)) { bandwidth->out->peak = params[i].value.ui; } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_BURST)) { @@ -9874,7 +9877,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, * bandwidth parameters, so merge with old bandwidth parameters * here to prevent them from being lost. */ if (bandwidth->in || - (net->bandwidth && net->bandwidth->in)) { + (!inboundSpecified && net->bandwidth && net->bandwidth->in)) { if (VIR_ALLOC(newBandwidth->in) < 0) goto cleanup; @@ -9883,7 +9886,7 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, sizeof(*newBandwidth->in)); } if (bandwidth->out || - (net->bandwidth && net->bandwidth->out)) { + (!outboundSpecified && net->bandwidth && net->bandwidth->out)) { if (VIR_ALLOC(newBandwidth->out) < 0) goto cleanup; @@ -9900,8 +9903,12 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, } virNetDevBandwidthFree(net->bandwidth); - net->bandwidth = newBandwidth; - newBandwidth = NULL; + if (newBandwidth->in || newBandwidth->out) { + net->bandwidth = newBandwidth; + newBandwidth = NULL; + } else { + net->bandwidth = NULL; + } } if (flags & VIR_DOMAIN_AFFECT_CONFIG) { if (!persistentNet->bandwidth) { diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index fbde3da..cb6bf63 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2615,7 +2615,7 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd) vshError(ctl, _("inbound format is incorrect")); goto cleanup; } - if (inbound.average == 0) { + if (inbound.average == 0 && (inbound.burst || inbound.peak)) { vshError(ctl, _("inbound average is mandatory")); goto cleanup; } @@ -2643,7 +2643,7 @@ cmdDomIftune(vshControl *ctl, const vshCmd *cmd) vshError(ctl, _("outbound format is incorrect")); goto cleanup; } - if (outbound.average == 0) { + if (outbound.average == 0 && (outbound.burst || outbound.peak)) { vshError(ctl, _("outbound average is mandatory")); goto cleanup; } diff --git a/tools/virsh.pod b/tools/virsh.pod index 5c74bb3..3e721e0 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -698,6 +698,9 @@ expressed in kilobytes per second, while I<burst> is expressed in kilobytes in a single burst at -I<peak> speed as described in the Network XML documentation at L<http://libvirt.org/formatnetwork.html#elementQoS>. +To clear inbound or outbound settings, use I<--inbound> or I<--outbound> +respectfully with average of value zero. + If I<--live> is specified, affect a running guest. If I<--config> is specified, affect the next boot of a persistent guest. If I<--current> is specified, affect the current guest state. -- 1.9.0
On 03/18/2014 04:01 AM, Michal Privoznik wrote:
We allow translation from no_bandwidth to has_bandwidth for a vnic. However, going in the opposite direction is not implemented. It's not limitation of the API rather than internal implementation.
Awkward. How about: It's not a limitation of the API, rather a missing part of the internal implementation.
The problem is, we correctly detect that user hasn't specified any outbound (say he wants to clear out outbound). However, this gets overwritten by current vnic outbound settings. Then, virNetDevBandwidthSet doesn't change anything. We need to stop overwriting the outbound if users don't want us to. Same applies for inbound.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 15 +++++++++++---- tools/virsh-domain.c | 4 ++-- tools/virsh.pod | 3 +++ 3 files changed, 16 insertions(+), 6 deletions(-)
+To clear inbound or outbound settings, use I<--inbound> or I<--outbound> +respectfully with average of value zero.
s/of value/value of/ ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 18.03.2014 16:59, Eric Blake wrote:
On 03/18/2014 04:01 AM, Michal Privoznik wrote:
We allow translation from no_bandwidth to has_bandwidth for a vnic. However, going in the opposite direction is not implemented. It's not limitation of the API rather than internal implementation.
Awkward. How about:
It's not a limitation of the API, rather a missing part of the internal implementation.
The problem is, we correctly detect that user hasn't specified any outbound (say he wants to clear out outbound). However, this gets overwritten by current vnic outbound settings. Then, virNetDevBandwidthSet doesn't change anything. We need to stop overwriting the outbound if users don't want us to. Same applies for inbound.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 15 +++++++++++---- tools/virsh-domain.c | 4 ++-- tools/virsh.pod | 3 +++ 3 files changed, 16 insertions(+), 6 deletions(-)
+To clear inbound or outbound settings, use I<--inbound> or I<--outbound> +respectfully with average of value zero.
s/of value/value of/
ACK
Unfortunately, I've mistakenly pushed this patch already. So I'm proposing a follow up patch which fixes the wording. Michal
participants (2)
-
Eric Blake -
Michal Privoznik