[libvirt] [PATCH 1/2] qemu: Fix updating bandwidth limits in live XML
Erik Skultety
eskultet at redhat.com
Wed Oct 1 15:17:16 UTC 2014
On 10/01/2014 01:55 AM, John Ferlan wrote:
>
>
> On 09/22/2014 06:41 AM, Erik Skultety wrote:
>> When trying to update bandwidth limits on a running domain, limits get
>> updated in our internal structures, however XML parser reads
>> bandwidth limits from network 'actual' definition. Commiting this patch
>
> s/Commiting/Committing
>
>> it is now available to update bandwidth 'actual' definition as well,
>> thus updating domain runtime XML
>> ---
>> src/qemu/qemu_driver.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 702d3cc..ede8880 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -10028,7 +10028,19 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
>> } else {
>> net->bandwidth = NULL;
>> }
>> +
>> + if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
>> + virNetDevBandwidthFree(net->data.network.actual->bandwidth);
>
> This will set net->data.network.actual->bandwidth to NULL
>
> It's also remove it when net->bandwidth == NULL thus
> causing actual to be lost, which it doesn't seem is
> desired, but perhaps it is. Gues
>
>> + if (!net->bandwidth ||
>> + virNetDevBandwidthCopy(&net->data.network.actual->bandwidth,
>> + net->bandwidth) < 0)
>> + net->data.network.actual->bandwidth = NULL;
>
> Making this irrelevant, but I wonder if the < 0 here meant to do
> something else perhaps?
>
>> + }
>
> The above hunk needs some space formatting. Also since the
> virNetDevBandwidthCopy() has a if (!src) check, "(!net->bandwidth) ||"
> is unnecessary.
>
> if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
> virNetDevBandwidthFree(net->data.network.actual->bandwidth);
> if (virNetDevBandwidthCopy(&net->data.network.actual->bandwidth,
> net->bandwidth) < 0)
> goto cleanup
> }
>
Looks better, thanks Jon, but you'd still loose 'actual' in the hunk
above, maybe add a check like
if (net->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
net->bandwidth)
if you want to preserve 'actual' when net->bandwidth is NULL??
> Whether this is what is "expected" is perhaps something Laine can answer...
>
> John
>
>> +
>> + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
>> + goto cleanup;
>> }
>> +
>> if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
>> if (!persistentNet->bandwidth) {
>> persistentNet->bandwidth = bandwidth;
>>
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
Erik
More information about the libvir-list
mailing list