[libvirt] [PATCH v2 2/4] libxlDomainSetMemoryFlags : reuse virDomainLiveConfigHelperMethod

[Jim Fehlig]

On 03/01/2016 08:46 AM, John Ferlan wrote:
> $subj:
>
> Use the "libxl: ..." prefix please
>
> That way someone filtering on libxl patches may pick up on this. I can
> fix this for this one...
>
> On 02/24/2016 03:38 AM, Nikolay Shirokovskiy wrote:
>> Flag expansion is the same as in virDomainObjUpdateModificationImpact
>> which virDomainLiveConfigHelperMethod calls internally. The difference
>> is merely in implementation. Original code don't use mask
>> and handle case when VIR_DOMAIN_MEM_MAXIMUM is set explicitly.
>>
>> Other parts are no different.
>>
>> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy at virtuozzo.com>
>> ---
>>  src/libxl/libxl_driver.c | 34 ++--------------------------------
>>  1 file changed, 2 insertions(+), 32 deletions(-)
>>
> I think what's being done is OK, but just to be save, I've CC'd Jim
> Fehlig who typically reviews libxl patches just to be sure before I push
> the change.

The change looks good to me. In fact, it seems to be an improvement over the
existing logic. ACK.

Regards,
Jim

>
> John
>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 404016e..06feea5 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -1444,7 +1444,6 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
>>      libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
>>      virDomainObjPtr vm;
>>      virDomainDefPtr persistentDef = NULL;
>> -    bool isActive;
>>      int ret = -1;
>>  
>>      virCheckFlags(VIR_DOMAIN_MEM_LIVE |
>> @@ -1460,38 +1459,9 @@ libxlDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
>>      if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
>>          goto cleanup;
>>  
>> -    isActive = virDomainObjIsActive(vm);
>> -
>> -    if (flags == VIR_DOMAIN_MEM_CURRENT) {
>> -        if (isActive)
>> -            flags = VIR_DOMAIN_MEM_LIVE;
>> -        else
>> -            flags = VIR_DOMAIN_MEM_CONFIG;
>> -    }
>> -    if (flags == VIR_DOMAIN_MEM_MAXIMUM) {
>> -        if (isActive)
>> -            flags = VIR_DOMAIN_MEM_LIVE | VIR_DOMAIN_MEM_MAXIMUM;
>> -        else
>> -            flags = VIR_DOMAIN_MEM_CONFIG | VIR_DOMAIN_MEM_MAXIMUM;
>> -    }
>> -
>> -    if (!isActive && (flags & VIR_DOMAIN_MEM_LIVE)) {
>> -        virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> -                       _("cannot set memory on an inactive domain"));
>> +    if (virDomainLiveConfigHelperMethod(cfg->caps, driver->xmlopt, vm, &flags,
>> +                                        &persistentDef) < 0)
>>          goto endjob;
>> -    }
>> -
>> -    if (flags & VIR_DOMAIN_MEM_CONFIG) {
>> -        if (!vm->persistent) {
>> -            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
>> -                           _("cannot change persistent config of a transient domain"));
>> -            goto endjob;
>> -        }
>> -        if (!(persistentDef = virDomainObjGetPersistentDef(cfg->caps,
>> -                                                           driver->xmlopt,
>> -                                                           vm)))
>> -            goto endjob;
>> -    }
>>  
>>      if (flags & VIR_DOMAIN_MEM_MAXIMUM) {
>>          /* resize the maximum memory */
>>