[libvirt] [PATCH] libxl: implement virDomainObjCheckIsActive
Jim Fehlig
jfehlig at suse.com
Mon Feb 13 18:24:44 UTC 2017
Sagar Ghuge wrote:
> 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.
Ah, yes, this one
http://wiki.libvirt.org/page/BiteSizedTasks#Add_function_that_raises_error_if_domain_is_not_active
> 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.
It looks good to me, although I'm not sure how to best split up the work. Maybe
a patch that adds virDomainObjCheckIsActive, and then a patch for each driver
that replaces virDomainObjIsActive with virDomainObjCheckIsActive?
AFAICT, Cole added that task to the wiki. Perhaps he has a suggestion on the
best way to organize the work.
Regards,
Jim
>
> 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) {
>>>
>
>
>
More information about the libvir-list
mailing list