On 10/01/2014 11:17 AM, Erik Skultety wrote:
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??
Right - I thought about that too, but then I thought the purpose of making the change to actual was to copy what happened to net->bandwidth. Looking at the hunk just above: virNetDevBandwidthFree(net->bandwidth); if (newBandwidth->in || newBandwidth->out) { net->bandwidth = newBandwidth; newBandwidth = NULL; } else { net->bandwidth = NULL; } That says to me in this live path, we're freeing net->bandwidth and only replacing it with 'newBandwidth' if in/out were found. Thus, it seems reasonable that we'd want to remove the bandwidth from actual in this case as well which we wouldn't do if the the "&& net->bandwidth" was added to the condition. Also, upon further reflection the "net->data.network.actual->bandwidth = NULL;" after the virNetDevBandwidthFree() will be necessary since we're passing by value and not reference... John
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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Erik
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list