[libvirt] [PATCH 16/35] qemu: Add helper to update domain balloon size and refactor usage places

[John Ferlan]

On 06/03/2015 09:16 AM, Ján Tomko wrote:
> On Fri, May 29, 2015 at 03:33:37PM +0200, Peter Krempa wrote:
>> When qemu does not support the balloon event the current memory size
>> needs to be queried. Since there are two places that implement the same
>> logic, split it out into a function and reuse.
>> ---
>>  src/qemu/qemu_domain.c | 64 ++++++++++++++++++++++++++++++++++++++
>>  src/qemu/qemu_domain.h |  3 ++
>>  src/qemu/qemu_driver.c | 84 +++++---------------------------------------------
>>  3 files changed, 75 insertions(+), 76 deletions(-)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index db8554b..661181f 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -3182,3 +3182,67 @@ qemuDomainMachineIsI440FX(const virDomainDef *def)
>>              STRPREFIX(def->os.machine, "pc-i440") ||
>>              STRPREFIX(def->os.machine, "rhel"));
>>  }
>> +
>> +
>> +/**
>> + * qemuDomainUpdateCurrentMemorySize:
>> + *
>> + * Updates the current balloon size from the monitor if necessary. In case when
>> + * the balloon is not present for the domain, the function recalculates the
>> + * maximum size to reflect possible changes.
>> + *
>> + * Returns 0 on success and updates vm->def->mem.cur_balloon if necessary, -1 on
>> + * error and reports libvirt error.
>> + */
>> +int
>> +qemuDomainUpdateCurrentMemorySize(virQEMUDriverPtr driver,
>> +                                  virDomainObjPtr vm)
>> +{
>> +    qemuDomainObjPrivatePtr priv = vm->privateData;
>> +    unsigned long long balloon;
>> +    int ret = -1;
>> +
>> +    /* inactive domain doesn't need size update */
>> +    if (!virDomainObjIsActive(vm))
>> +        return 0;
>> +
>> +    /* if no balloning is available, the current size equals to the current
>> +     * full memory size */
>> +    if (!vm->def->memballoon ||
>> +        vm->def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE) {
>> +        vm->def->mem.cur_balloon = virDomainDefGetMemoryActual(vm->def);
>> +        return 0;
>> +    }
>> +
>> +    /* current size is always automagically updated via the event */
>> +    if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BALLOON_EVENT))
>> +        return 0;
>> +
>> +    /* here we need to ask the monitor */
>> +
>> +    /* Don't delay if someone's using the monitor, just use existing most
>> +     * recent data instead */
>> +    if (qemuDomainJobAllowed(priv, QEMU_JOB_QUERY)) {
>> +        if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
>> +            return -1;
>> +
>> +        if (!virDomainObjIsActive(vm)) {
>> +            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> +                           _("domain is not running"));
>> +            goto endjob;
>> +        }
>> +
>> +        qemuDomainObjEnterMonitor(driver, vm);
>> +        ret = qemuMonitorGetBalloonInfo(priv->mon, &balloon);
>> +        if (qemuDomainObjExitMonitor(driver, vm) < 0)
>> +            ret = -1;
>> +
>> + endjob:
>> +        qemuDomainObjEndJob(driver, vm);
>> +
>> +        if (ret < 0)
>> +            return -1;
> 
> ACK if you actually use the 'balloon' value to update cur_balloon here.
> 
> Jan
> 

Making Coverity unhappy...

3215 	    qemuDomainObjPrivatePtr priv = vm->privateData;

(1) Event var_decl: 	Declaring variable "balloon" without initializer.
Also see events: 	[uninit_use]

3216 	    unsigned long long balloon;

...
238 	     * recent data instead */

(9) Event cond_false: 	Condition "qemuDomainJobAllowed(priv,
QEMU_JOB_QUERY)", taking false branch

3239 	    if (qemuDomainJobAllowed(priv, QEMU_JOB_QUERY)) {

...

3258 	            return -1;

(10) Event if_end: 	End of if statement

3259 	    }
3260 	

(11) Event uninit_use: 	Using uninitialized value "balloon".
Also see events: 	[var_decl]

3261 	    vm->def->mem.cur_balloon = balloon;
3262 	


So if QEMU_JOB_QUERY is not allowed, then balloon is not initialized and
setting to cur_balloon is bad

Adding an "if (ret == 0)" prior to the setting works.


John

>> +    }
>> +
>> +    return 0;
>> +}
>>
>>
>> --
>> libvir-list mailing list
>> libvir-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list