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(-)
Add function which raises error if domain is
not active
Signed-off-by: Sagar Ghuge <ghugesss@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"));
+ if (virDomainObjCheckIsActive(vm) < 0)
goto endjob;
- }
if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) {
if (libxl_domain_pause(cfg->ctx, vm->def->id) != 0) {
--
2.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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@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) { > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@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@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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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@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@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) { >>> > > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 02/13/2017 01:24 PM, Jim Fehlig wrote: > 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. > Sorry for the late response. For these types of patches, please link to the BiteSizedTask in the mail after the -- break generated by git format-patch: this keeps the link out of the commit message since it's not really relevant there, but gives more info to potential patch reviewers. For this item I'd say do a patch series containing more instances of conversions, to show that it's actually simplifying a common code pattern. So maybe a patch series like: 1) Add the function 2) Convert src/test/* 3) Convert src/libxl/* 4) Convert src/xen/* That shouldn't be too much work and will give a bit more to comment on, and if there's no objections, send the remaining conversions as an additional patch series. - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.