[libvirt] [PATCH] libxl: implement virDomainObjCheckIsActive

[Sagar Ghuge]

Hi Jim,

Thank you for replying. I am planning to participate in GSoC'17 and
this is my first patch towards it. This is from Byte sized task which
I picked up and trying to solve it. Yes you are right, pattern is used
throughout the libxl driver and many others but this is just a
rudimentary patch which will help me to get suggestion that am I on
right path. if yes then I can I can go ahead and make changes
accordingly.

On Sun, Feb 12, 2017 at 7:23 PM, Jim Fehlig <jfehlig at suse.com> wrote:
> On 02/11/2017 01:31 AM, Sagar Ghuge wrote:
>>
>> Add function which raises error if domain is
>> not active
>>
>> Signed-off-by: Sagar Ghuge <ghugesss at gmail.com>
>> ---
>>  src/conf/domain_conf.c   | 13 +++++++++++++
>>  src/conf/domain_conf.h   |  1 +
>>  src/libxl/libxl_driver.c |  4 +---
>>  3 files changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 1bc72a4..10a69af 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -2995,6 +2995,19 @@ virDomainObjWait(virDomainObjPtr vm)
>>  }
>>
>>
>> +int
>> +virDomainObjCheckIsActive(virDomainObjPtr vm)
>> +{
>> +    if (!virDomainObjIsActive(vm)) {
>> +        virReportError(VIR_ERR_OPERATION_FAILED, "%s",
>> +                       _("domain is not running"));
>> +        return -1;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +
>>  /**
>>   * Waits for domain condition to be triggered for a specific period of
>> time.
>>   *
>> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
>> index dd79206..b6c7826 100644
>> --- a/src/conf/domain_conf.h
>> +++ b/src/conf/domain_conf.h
>> @@ -2559,6 +2559,7 @@ bool virDomainObjTaint(virDomainObjPtr obj,
>>
>>  void virDomainObjBroadcast(virDomainObjPtr vm);
>>  int virDomainObjWait(virDomainObjPtr vm);
>> +int virDomainObjCheckIsActive(virDomainObjPtr vm);
>>  int virDomainObjWaitUntil(virDomainObjPtr vm,
>>                            unsigned long long whenms);
>>
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index 74cb05a..3a487ac 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -1183,10 +1183,8 @@ libxlDomainSuspend(virDomainPtr dom)
>>      if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0)
>>          goto cleanup;
>>
>> -    if (!virDomainObjIsActive(vm)) {
>> -        virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("Domain is not
>> running"));
>
>
> This is the standard pattern used throughout the libxl driver and many other
> drivers as well. Why do you only want to change this one instance? IMO if
> such a change is desired it should be done consistently across the code
> base.
>
> Regards,
> Jim
>
>
>> +    if (virDomainObjCheckIsActive(vm) < 0)
>>          goto endjob;
>> -    }
>>
>>      if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) {
>>          if (libxl_domain_pause(cfg->ctx, vm->def->id) != 0) {
>>
>



-- 
Regards,
Sagar