[libvirt] [PATCH 2/2] qemu: Fix updating balloon period in live XML

[Erik Skultety]

On 10/01/2014 01:55 AM, John Ferlan wrote:
>
>
> On 09/22/2014 06:41 AM, Erik Skultety wrote:
>> Up until now, we set memballoon period in monitor successfully, however
>> we did not update domain definition structure, thus dumpxml was omitting
>> period attribute in memballoon element
>>
>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140960
>> ---
>>   src/qemu/qemu_driver.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index ede8880..d73288a 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -2460,9 +2460,15 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int period,
>>           qemuDomainObjEnterMonitor(driver, vm);
>>           r = qemuMonitorSetMemoryStatsPeriod(priv->mon, period);
>>           qemuDomainObjExitMonitor(driver, vm);
>> -        if (r < 0)
>> +        if (r < 0) {
>>               virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>>                              _("unable to set balloon driver collection period"));
>> +            goto endjob;
>> +        }
>
> I'm trying to remember if there was a reason for not jumping to error.
> It probably has to do with "at some point in time" during development
> this setting would/could be done through calls via qemu_process.c and
> causing a failure through that path wasn't good.  Now since this only
> accessible via a virsh command - I agree going to endjob is right...
>
> If you care to walk the history - start here:
>
> http://www.redhat.com/archives/libvir-list/2013-July/msg00770.html

I had a little look at the history you pointed out. From what I've 
found, the sub-element 'period' was introduced along with qemu 
driver-specific API and libvirt domain API in 10-part v3 patch series 
(http://www.redhat.com/archives/libvir-list/2013-July/msg00774.html). I 
detached HEAD before these commits were pushed and I didn't find any 
trace about memballoon period setting. All the APIs and necessary 
structures/members were introduced by the patch series mentioned above, 
is that correct? :) (in that case I guess it's fine to add the jump to 
'endjob' label)
> ACK (to what's here)
>
> John
>> +
>> +        vm->def->memballoon->period = period;
>> +        if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
>> +            goto endjob;
>>       }
>>
>>       if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
>>
>
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>
Erik