[libvirt] [PATCH 00/15] Introduce virDomain{Resume,Suspend}Flags
So far, nothing interesting is happening here. The real work is yet to come :) Michal Privoznik (15): Introduce virDomainSuspendFlags Introduce virDomainResumeFlags esxDriver: Adapt to virDomain{Resume,Suspend}Flags hypervDriver: Adapt to virDomain{Resume,Suspend}Flags libxlDriver: Adapt to virDomain{Resume,Suspend}Flags lxcDriver: Adapt to virDomain{Resume,Suspend}Flags openvzDriver: Adapt to virDomain{Resume,Suspend}Flags parallelsDriver: Adapt to virDomain{Resume,Suspend}Flags qemuDriver: Adapt to virDomain{Resume,Suspend}Flags testDriver: Adapt to virDomain{Resume,Suspend}Flags vboxDriver: Adapt to virDomain{Resume,Suspend}Flags vmwareDriver: Adapt to virDomain{Resume,Suspend}Flags xenUnifiedDriver: Adapt to virDomain{Resume,Suspend}Flags xenapiDriver: Adapt to virDomain{Resume,Suspend}Flags virsh: Adapt to virDomain{Resume,Suspend}Flags include/libvirt/libvirt.h.in | 4 ++ src/driver.h | 10 ++++ src/esx/esx_driver.c | 28 ++++++++++- src/hyperv/hyperv_driver.c | 28 ++++++++++- src/libvirt.c | 104 ++++++++++++++++++++++++++++++++++----- src/libvirt_public.syms | 6 +++ src/libxl/libxl_driver.c | 28 +++++++++-- src/lxc/lxc_driver.c | 30 +++++++++-- src/openvz/openvz_driver.c | 28 ++++++++++- src/parallels/parallels_driver.c | 24 ++++++++- src/qemu/qemu_driver.c | 35 +++++++++++-- src/remote/remote_driver.c | 2 + src/remote/remote_protocol.x | 24 ++++++++- src/remote_protocol-structs | 10 ++++ src/test/test_driver.c | 26 +++++++++- src/vbox/vbox_tmpl.c | 28 ++++++++++- src/vmware/vmware_driver.c | 24 ++++++++- src/xen/xen_driver.c | 28 +++++++++-- src/xenapi/xenapi_driver.c | 42 ++++++++++++++-- tools/virsh-domain.c | 52 +++++++++++++++----- 20 files changed, 502 insertions(+), 59 deletions(-) -- 1.8.5.2
So far, we have just bare virDomainSuspend() API that suspends a domain. However, in the future there might occur a case, in which we may want to modify suspend behavior slightly. In that case, @flags are useful. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt.h.in | 2 ++ src/driver.h | 5 +++++ src/libvirt.c | 49 ++++++++++++++++++++++++++++++++++++++++---- src/libvirt_public.syms | 5 +++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 +++++++++++- src/remote_protocol-structs | 5 +++++ 7 files changed, 75 insertions(+), 5 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index b3ce000..cc916c7 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1661,6 +1661,8 @@ int virDomainFree (virDomainPtr domain); * Domain suspend/resume */ int virDomainSuspend (virDomainPtr domain); +int virDomainSuspendFlags (virDomainPtr domain, + unsigned int flags); int virDomainResume (virDomainPtr domain); int virDomainPMSuspendForDuration (virDomainPtr domain, unsigned int target, diff --git a/src/driver.h b/src/driver.h index 5f4cd8d..d325e18 100644 --- a/src/driver.h +++ b/src/driver.h @@ -160,6 +160,10 @@ typedef int (*virDrvDomainSuspend)(virDomainPtr domain); typedef int +(*virDrvDomainSuspendFlags)(virDomainPtr domain, + unsigned int flags); + +typedef int (*virDrvDomainResume)(virDomainPtr domain); typedef int @@ -1167,6 +1171,7 @@ struct _virDriver { virDrvDomainLookupByUUID domainLookupByUUID; virDrvDomainLookupByName domainLookupByName; virDrvDomainSuspend domainSuspend; + virDrvDomainSuspendFlags domainSuspendFlags; virDrvDomainResume domainResume; virDrvDomainPMSuspendForDuration domainPMSuspendForDuration; virDrvDomainPMWakeup domainPMWakeup; diff --git a/src/libvirt.c b/src/libvirt.c index 9cc5b1c..e39031a 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2347,13 +2347,54 @@ error: /** + * virDomainSuspendFlags: + * @domain: a domain object + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Suspends an active domain, the process is frozen without further + * access to CPU resources and I/O but the memory used by the domain at + * the hypervisor level will stay allocated. Use virDomainResume() to + * reactivate the domain. This function may require privileged access. + * Moreover, suspend may not be supported if domain is in some special + * state like VIR_DOMAIN_PMSUSPENDED. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainSuspendFlags(virDomainPtr domain, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(domain, "flags=%x", flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + virCheckReadOnlyGoto(domain->conn->flags, error); + + if (domain->conn->driver->domainSuspendFlags) { + int ret; + ret = domain->conn->driver->domainSuspendFlags(domain, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + +error: + virDispatchError(domain->conn); + return -1; +} + + +/** * virDomainResume: * @domain: a domain object * - * Resume a suspended domain, the process is restarted from the state where - * it was frozen by calling virDomainSuspend(). - * This function may require privileged access - * Moreover, resume may not be supported if domain is in some + * Resume a suspended domain, the process is restarted from the state + * where it was frozen by calling virDomainSuspend() or + * virDomainSuspendFlags(). This function may require privileged + * access Moreover, resume may not be supported if domain is in some * special state like VIR_DOMAIN_PMSUSPENDED. * * Returns 0 in case of success and -1 in case of failure. diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 6ed6ce6..b04587a 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -645,5 +645,10 @@ LIBVIRT_1.2.1 { virConnectNetworkEventDeregisterAny; } LIBVIRT_1.1.3; +LIBVIRT_1.2.2 { + global: + virDomainSuspendFlags; +} LIBVIRT_1.2.1; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index ca86e3c..a70cb6d 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6977,6 +6977,7 @@ static virDriver remote_driver = { .domainLookupByUUID = remoteDomainLookupByUUID, /* 0.3.0 */ .domainLookupByName = remoteDomainLookupByName, /* 0.3.0 */ .domainSuspend = remoteDomainSuspend, /* 0.3.0 */ + .domainSuspendFlags = remoteDomainSuspendFlags, /* 1.2.2 */ .domainResume = remoteDomainResume, /* 0.3.0 */ .domainPMSuspendForDuration = remoteDomainPMSuspendForDuration, /* 0.9.10 */ .domainPMWakeup = remoteDomainPMWakeup, /* 0.9.11 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 790a020..e25ecf3 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -772,6 +772,11 @@ struct remote_domain_suspend_args { remote_nonnull_domain dom; }; +struct remote_domain_suspend_flags_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + struct remote_domain_resume_args { remote_nonnull_domain dom; }; @@ -5068,5 +5073,11 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_NETWORK_EVENT_LIFECYCLE = 315 + REMOTE_PROC_NETWORK_EVENT_LIFECYCLE = 315, + + /** + * @generate: both + * @acl: domain:suspend + */ + REMOTE_PROC_DOMAIN_SUSPEND_FLAGS = 316 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index e58482e..4adf93a 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -451,6 +451,10 @@ struct remote_domain_lookup_by_name_ret { struct remote_domain_suspend_args { remote_nonnull_domain dom; }; +struct remote_domain_suspend_flags_args { + remote_nonnull_domain dom; + u_int flags; +}; struct remote_domain_resume_args { remote_nonnull_domain dom; }; @@ -2660,4 +2664,5 @@ enum remote_procedure { REMOTE_PROC_CONNECT_NETWORK_EVENT_REGISTER_ANY = 313, REMOTE_PROC_CONNECT_NETWORK_EVENT_DEREGISTER_ANY = 314, REMOTE_PROC_NETWORK_EVENT_LIFECYCLE = 315, + REMOTE_PROC_DOMAIN_SUSPEND_FLAGS = 316, }; -- 1.8.5.2
On 02/03/2014 09:16 AM, Michal Privoznik wrote:
So far, we have just bare virDomainSuspend() API that suspends a domain. However, in the future there might occur a case, in which we may want to modify suspend behavior slightly. In that case, @flags are useful.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt.h.in | 2 ++ src/driver.h | 5 +++++ src/libvirt.c | 49 ++++++++++++++++++++++++++++++++++++++++---- src/libvirt_public.syms | 5 +++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 +++++++++++- src/remote_protocol-structs | 5 +++++ 7 files changed, 75 insertions(+), 5 deletions(-)
Back when we added virDomainShutdownFlags in 0.9.10, I asked if we should also add *Flags variants for remaining functions without them; at the time, we decided against it, but I'm not quite sure why. I'm perfectly fine with adding this for the sake of making future additions easier, even if we don't have a use for the flags now - it's easier to support a flag than it is to rebase to pick up a new function for any situation where the .so contains a flags function, but it may be worth getting a second opinion before pushing, if you don't have a plan to use flags right away. As to the code itself:
+++ b/src/libvirt.c @@ -2347,13 +2347,54 @@ error:
/** + * virDomainSuspendFlags: + * @domain: a domain object + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Suspends an active domain, the process is frozen without further + * access to CPU resources and I/O but the memory used by the domain at + * the hypervisor level will stay allocated. Use virDomainResume() to + * reactivate the domain. This function may require privileged access. + * Moreover, suspend may not be supported if domain is in some special + * state like VIR_DOMAIN_PMSUSPENDED. + * + * Returns 0 in case of success and -1 in case of failure.
Might be worth adding a comment to virDomainSuspend() that mentions it is equivalent to virDomainSuspendFlags() with flags 0.
* virDomainResume: * @domain: a domain object * - * Resume a suspended domain, the process is restarted from the state where - * it was frozen by calling virDomainSuspend(). - * This function may require privileged access - * Moreover, resume may not be supported if domain is in some + * Resume a suspended domain, the process is restarted from the state + * where it was frozen by calling virDomainSuspend() or + * virDomainSuspendFlags(). This function may require privileged + * access Moreover, resume may not be supported if domain is in some
s/access/access./
+++ b/src/remote/remote_protocol.x
+ + /** + * @generate: both + * @acl: domain:suspend + */ + REMOTE_PROC_DOMAIN_SUSPEND_FLAGS = 316
Trivial conflict with my pending review for server-side event filtering. Otherwise looks okay. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Feb 03, 2014 at 10:12:58AM -0700, Eric Blake wrote:
On 02/03/2014 09:16 AM, Michal Privoznik wrote:
So far, we have just bare virDomainSuspend() API that suspends a domain. However, in the future there might occur a case, in which we may want to modify suspend behavior slightly. In that case, @flags are useful.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt.h.in | 2 ++ src/driver.h | 5 +++++ src/libvirt.c | 49 ++++++++++++++++++++++++++++++++++++++++---- src/libvirt_public.syms | 5 +++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 +++++++++++- src/remote_protocol-structs | 5 +++++ 7 files changed, 75 insertions(+), 5 deletions(-)
Back when we added virDomainShutdownFlags in 0.9.10, I asked if we should also add *Flags variants for remaining functions without them; at the time, we decided against it, but I'm not quite sure why.
I'm perfectly fine with adding this for the sake of making future additions easier, even if we don't have a use for the flags now - it's easier to support a flag than it is to rebase to pick up a new function for any situation where the .so contains a flags function, but it may be worth getting a second opinion before pushing, if you don't have a plan to use flags right away.
I like this approach as there are many issues that can be easily solved in case there is a 'Flags' version of some API. That's why we advocate usage of a flags parameter in new APIs even when it is not yet used. Although I was wondering whether it would be too much overkill to use 'Params' instead of 'Flags' as Jiri did with migrations as that has way more power. And that's for both new APIs and this change proposed by Michal. Have a nice day, Martin
On 04.02.2014 08:05, Martin Kletzander wrote:
On Mon, Feb 03, 2014 at 10:12:58AM -0700, Eric Blake wrote:
On 02/03/2014 09:16 AM, Michal Privoznik wrote:
So far, we have just bare virDomainSuspend() API that suspends a domain. However, in the future there might occur a case, in which we may want to modify suspend behavior slightly. In that case, @flags are useful.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt.h.in | 2 ++ src/driver.h | 5 +++++ src/libvirt.c | 49 ++++++++++++++++++++++++++++++++++++++++---- src/libvirt_public.syms | 5 +++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 +++++++++++- src/remote_protocol-structs | 5 +++++ 7 files changed, 75 insertions(+), 5 deletions(-)
Back when we added virDomainShutdownFlags in 0.9.10, I asked if we should also add *Flags variants for remaining functions without them; at the time, we decided against it, but I'm not quite sure why.
I'm perfectly fine with adding this for the sake of making future additions easier, even if we don't have a use for the flags now - it's easier to support a flag than it is to rebase to pick up a new function for any situation where the .so contains a flags function, but it may be worth getting a second opinion before pushing, if you don't have a plan to use flags right away.
I like this approach as there are many issues that can be easily solved in case there is a 'Flags' version of some API. That's why we advocate usage of a flags parameter in new APIs even when it is not yet used.
Although I was wondering whether it would be too much overkill to use 'Params' instead of 'Flags' as Jiri did with migrations as that has way more power. And that's for both new APIs and this change proposed by Michal.
I think migration API is different to these ones. I mean, with migration you want to pass tons of arguments (from spoofing domain XML, through changing domain name on the dst or graphic listen address, to limiting migration bandwidth). However, with suspend or resume it's different. We haven't been missing even flags till now, nor Typed Params. For instance, one usage scenario (and this answers Dan's question): At the resume, when domain's CPUs were not running for a certain period of time (and guest basically doesn't know nothing about it), users might want to resync the guest time. There's currently one approach being discussed on the qemu-devel: The qemu-ga has this guest-set-time which requires a time to be passed (currently). With the approach, the time argument becomes optional, so that whenever it is not passed with the command, the qemu-ga reads current time from guest's RTC. So in libvirt we could then just: virDomainResume(dom, VIR_DOMAIN_RESUME_SYNC_TIME); or something. And for virDomainSupsend I don't have any particular example, but since suspend and resume are a pair, I've changed both of them. tl;dr - TypedParams are bit overkill IMO.
Have a nice day,
U2
Martin
Michal
On Tue, Feb 04, 2014 at 08:37:28AM +0100, Michal Privoznik wrote:
On 04.02.2014 08:05, Martin Kletzander wrote:
On Mon, Feb 03, 2014 at 10:12:58AM -0700, Eric Blake wrote:
On 02/03/2014 09:16 AM, Michal Privoznik wrote:
So far, we have just bare virDomainSuspend() API that suspends a domain. However, in the future there might occur a case, in which we may want to modify suspend behavior slightly. In that case, @flags are useful.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt.h.in | 2 ++ src/driver.h | 5 +++++ src/libvirt.c | 49 ++++++++++++++++++++++++++++++++++++++++---- src/libvirt_public.syms | 5 +++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 +++++++++++- src/remote_protocol-structs | 5 +++++ 7 files changed, 75 insertions(+), 5 deletions(-)
Back when we added virDomainShutdownFlags in 0.9.10, I asked if we should also add *Flags variants for remaining functions without them; at the time, we decided against it, but I'm not quite sure why.
I'm perfectly fine with adding this for the sake of making future additions easier, even if we don't have a use for the flags now - it's easier to support a flag than it is to rebase to pick up a new function for any situation where the .so contains a flags function, but it may be worth getting a second opinion before pushing, if you don't have a plan to use flags right away.
I like this approach as there are many issues that can be easily solved in case there is a 'Flags' version of some API. That's why we advocate usage of a flags parameter in new APIs even when it is not yet used.
Although I was wondering whether it would be too much overkill to use 'Params' instead of 'Flags' as Jiri did with migrations as that has way more power. And that's for both new APIs and this change proposed by Michal.
I think migration API is different to these ones. I mean, with migration you want to pass tons of arguments (from spoofing domain XML, through changing domain name on the dst or graphic listen address, to limiting migration bandwidth). However, with suspend or resume it's different. We haven't been missing even flags till now, nor Typed Params.
For instance, one usage scenario (and this answers Dan's question): At the resume, when domain's CPUs were not running for a certain period of time (and guest basically doesn't know nothing about it), users might want to resync the guest time. There's currently one approach being discussed on the qemu-devel: The qemu-ga has this guest-set-time which requires a time to be passed (currently). With the approach, the time argument becomes optional, so that whenever it is not passed with the command, the qemu-ga reads current time from guest's RTC. So in libvirt we could then just:
virDomainResume(dom, VIR_DOMAIN_RESUME_SYNC_TIME);
or something. And for virDomainSupsend I don't have any particular example, but since suspend and resume are a pair, I've changed both of them.
So there's interesting, and I'm not sure I entirely agree about adding flags here. It seems to me that if the QEMU agent has a "set-time" capability we'd be better off having an explicit API virDomainSetTime(...). The action of resume + set time cannot be atomic so I don't see any point in overloading the "set time" functionality into the resume API call. Just let apps call the set time method if they so desire.
tl;dr - TypedParams are bit overkill IMO.
Agreed, TypedParams are also pretty nasty to work with as an application developer, so we should only use them where absolutely required. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
So far, we have just bare virDomainResume() API that resumes a domain. However, in the future there might occur a case, in which we may want to modify resume behavior slightly. In that case, @flags are useful. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt.h.in | 2 ++ src/driver.h | 5 ++++ src/libvirt.c | 63 ++++++++++++++++++++++++++++++++++++-------- src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 ++++++++- src/remote_protocol-structs | 5 ++++ 7 files changed, 78 insertions(+), 12 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index cc916c7..3d326cc 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1664,6 +1664,8 @@ int virDomainSuspend (virDomainPtr domain); int virDomainSuspendFlags (virDomainPtr domain, unsigned int flags); int virDomainResume (virDomainPtr domain); +int virDomainResumeFlags (virDomainPtr domain, + unsigned int flags); int virDomainPMSuspendForDuration (virDomainPtr domain, unsigned int target, unsigned long long duration, diff --git a/src/driver.h b/src/driver.h index d325e18..cbb8177 100644 --- a/src/driver.h +++ b/src/driver.h @@ -167,6 +167,10 @@ typedef int (*virDrvDomainResume)(virDomainPtr domain); typedef int +(*virDrvDomainResumeFlags)(virDomainPtr domain, + unsigned int flags); + +typedef int (*virDrvDomainPMSuspendForDuration)(virDomainPtr, unsigned int target, unsigned long long duration, @@ -1173,6 +1177,7 @@ struct _virDriver { virDrvDomainSuspend domainSuspend; virDrvDomainSuspendFlags domainSuspendFlags; virDrvDomainResume domainResume; + virDrvDomainResumeFlags domainResumeFlags; virDrvDomainPMSuspendForDuration domainPMSuspendForDuration; virDrvDomainPMWakeup domainPMWakeup; virDrvDomainShutdown domainShutdown; diff --git a/src/libvirt.c b/src/libvirt.c index e39031a..27841ea 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2306,13 +2306,13 @@ virDomainRef(virDomainPtr domain) * virDomainSuspend: * @domain: a domain object * - * Suspends an active domain, the process is frozen without further access - * to CPU resources and I/O but the memory used by the domain at the - * hypervisor level will stay allocated. Use virDomainResume() to reactivate - * the domain. - * This function may require privileged access. - * Moreover, suspend may not be supported if domain is in some - * special state like VIR_DOMAIN_PMSUSPENDED. + * Suspends an active domain, the process is frozen without further + * access to CPU resources and I/O but the memory used by the domain at + * the hypervisor level will stay allocated. Use virDomainResume() or + * virDomainResumeFlags() to reactivate the domain. This function + * may require privileged access. Moreover, suspend may not be + * supported if domain is in some special state like + * VIR_DOMAIN_PMSUSPENDED. * * Returns 0 in case of success and -1 in case of failure. */ @@ -2353,10 +2353,11 @@ error: * * Suspends an active domain, the process is frozen without further * access to CPU resources and I/O but the memory used by the domain at - * the hypervisor level will stay allocated. Use virDomainResume() to - * reactivate the domain. This function may require privileged access. - * Moreover, suspend may not be supported if domain is in some special - * state like VIR_DOMAIN_PMSUSPENDED. + * the hypervisor level will stay allocated. Use virDomainResume() or + * virDomainResumeFlags() to reactivate the domain. This function + * may require privileged access. Moreover, suspend may not be + * supported if domain is in some special state like + * VIR_DOMAIN_PMSUSPENDED. * * Returns 0 in case of success and -1 in case of failure. */ @@ -2430,6 +2431,46 @@ error: /** + * virDomainResumeFlags: + * @domain: a domain object + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Resume a suspended domain, the process is restarted from the state + * where it was frozen by calling virDomainSuspend() or + * virDomainSuspendFlags(). This function may require privileged + * access Moreover, resume may not be supported if domain is in some + * special state like VIR_DOMAIN_PMSUSPENDED. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainResumeFlags(virDomainPtr domain, + unsigned int flags) +{ + VIR_DOMAIN_DEBUG(domain, "flags=%x", flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + virCheckReadOnlyGoto(domain->conn->flags, error); + + if (domain->conn->driver->domainResumeFlags) { + int ret; + ret = domain->conn->driver->domainResumeFlags(domain, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + +error: + virDispatchError(domain->conn); + return -1; +} + + +/** * virDomainPMSuspendForDuration: * @dom: a domain object * @target: a value from virNodeSuspendTarget diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index b04587a..ead4f40 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -648,6 +648,7 @@ LIBVIRT_1.2.1 { LIBVIRT_1.2.2 { global: virDomainSuspendFlags; + virDomainResumeFlags; } LIBVIRT_1.2.1; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index a70cb6d..efafe16 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6979,6 +6979,7 @@ static virDriver remote_driver = { .domainSuspend = remoteDomainSuspend, /* 0.3.0 */ .domainSuspendFlags = remoteDomainSuspendFlags, /* 1.2.2 */ .domainResume = remoteDomainResume, /* 0.3.0 */ + .domainResumeFlags = remoteDomainResumeFlags, /* 1.2.2 */ .domainPMSuspendForDuration = remoteDomainPMSuspendForDuration, /* 0.9.10 */ .domainPMWakeup = remoteDomainPMWakeup, /* 0.9.11 */ .domainShutdown = remoteDomainShutdown, /* 0.3.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index e25ecf3..359185a 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -781,6 +781,11 @@ struct remote_domain_resume_args { remote_nonnull_domain dom; }; +struct remote_domain_resume_flags_args { + remote_nonnull_domain dom; + unsigned int flags; +}; + struct remote_domain_pm_suspend_for_duration_args { remote_nonnull_domain dom; unsigned int target; @@ -5079,5 +5084,11 @@ enum remote_procedure { * @generate: both * @acl: domain:suspend */ - REMOTE_PROC_DOMAIN_SUSPEND_FLAGS = 316 + REMOTE_PROC_DOMAIN_SUSPEND_FLAGS = 316, + + /** + * @generate: both + * @acl: domain:suspend + */ + REMOTE_PROC_DOMAIN_RESUME_FLAGS = 317 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 4adf93a..a3f65a7 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -458,6 +458,10 @@ struct remote_domain_suspend_flags_args { struct remote_domain_resume_args { remote_nonnull_domain dom; }; +struct remote_domain_resume_flags_args { + remote_nonnull_domain dom; + u_int flags; +}; struct remote_domain_pm_suspend_for_duration_args { remote_nonnull_domain dom; u_int target; @@ -2665,4 +2669,5 @@ enum remote_procedure { REMOTE_PROC_CONNECT_NETWORK_EVENT_DEREGISTER_ANY = 314, REMOTE_PROC_NETWORK_EVENT_LIFECYCLE = 315, REMOTE_PROC_DOMAIN_SUSPEND_FLAGS = 316, + REMOTE_PROC_DOMAIN_RESUME_FLAGS = 317, }; -- 1.8.5.2
On 02/03/2014 09:16 AM, Michal Privoznik wrote:
So far, we have just bare virDomainResume() API that resumes a domain. However, in the future there might occur a case, in which we may want to modify resume behavior slightly. In that case, @flags are useful.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- include/libvirt/libvirt.h.in | 2 ++ src/driver.h | 5 ++++ src/libvirt.c | 63 ++++++++++++++++++++++++++++++++++++-------- src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 ++++++++- src/remote_protocol-structs | 5 ++++ 7 files changed, 78 insertions(+), 12 deletions(-)
Same story as 1/15, where I like the change even if you don't have a plan for the flags, but you should get a second opinion. Also, I could see squashing 1 and 2 into a single patch - it doesn't make sense to introduce one function without the other, especially since you are touching cross-documentation to call out the new function names.
/** + * virDomainResumeFlags: + * @domain: a domain object + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Resume a suspended domain, the process is restarted from the state + * where it was frozen by calling virDomainSuspend() or + * virDomainSuspendFlags(). This function may require privileged + * access Moreover, resume may not be supported if domain is in some + * special state like VIR_DOMAIN_PMSUSPENDED. + *
As in patch 1, you need to add a comment to virDomainResume() mentioning that it is short for virDomainResumeFlags(,0).
+++ b/src/libvirt_public.syms @@ -648,6 +648,7 @@ LIBVIRT_1.2.1 { LIBVIRT_1.2.2 { global: virDomainSuspendFlags; + virDomainResumeFlags;
Not essential, but I like listing new APIs in alphabetical order. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/esx/esx_driver.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 886d984..9f8d673 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -1693,7 +1693,8 @@ esxDomainLookupByName(virConnectPtr conn, const char *name) static int -esxDomainSuspend(virDomainPtr domain) +esxDomainSuspendFlags(virDomainPtr domain, + unsigned int flags) { int result = -1; esxPrivate *priv = domain->conn->privateData; @@ -1704,6 +1705,8 @@ esxDomainSuspend(virDomainPtr domain) esxVI_TaskInfoState taskInfoState; char *taskInfoErrorMessage = NULL; + virCheckFlags(0, -1); + if (esxVI_EnsureSession(priv->primary) < 0) { return -1; } @@ -1751,7 +1754,16 @@ esxDomainSuspend(virDomainPtr domain) static int -esxDomainResume(virDomainPtr domain) +esxDomainSuspend(virDomainPtr domain) +{ + return esxDomainSuspendFlags(domain, 0); +} + + + +static int +esxDomainResumeFlags(virDomainPtr domain, + unsigned int flags) { int result = -1; esxPrivate *priv = domain->conn->privateData; @@ -1762,6 +1774,8 @@ esxDomainResume(virDomainPtr domain) esxVI_TaskInfoState taskInfoState; char *taskInfoErrorMessage = NULL; + virCheckFlags(0, -1); + if (esxVI_EnsureSession(priv->primary) < 0) { return -1; } @@ -1809,6 +1823,14 @@ esxDomainResume(virDomainPtr domain) static int +esxDomainResume(virDomainPtr domain) +{ + return esxDomainResumeFlags(domain, 0); +} + + + +static int esxDomainShutdownFlags(virDomainPtr domain, unsigned int flags) { int result = -1; @@ -5203,7 +5225,9 @@ static virDriver esxDriver = { .domainLookupByUUID = esxDomainLookupByUUID, /* 0.7.0 */ .domainLookupByName = esxDomainLookupByName, /* 0.7.0 */ .domainSuspend = esxDomainSuspend, /* 0.7.0 */ + .domainSuspendFlags = esxDomainSuspendFlags, /* 1.2.2 */ .domainResume = esxDomainResume, /* 0.7.0 */ + .domainResumeFlags = esxDomainResumeFlags, /* 1.2.2 */ .domainShutdown = esxDomainShutdown, /* 0.7.0 */ .domainShutdownFlags = esxDomainShutdownFlags, /* 0.9.10 */ .domainReboot = esxDomainReboot, /* 0.7.0 */ -- 1.8.5.2
On 02/03/2014 09:17 AM, Michal Privoznik wrote:
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/esx/esx_driver.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-)
+} + + + +static int
Why 3 blank lines?
esxDomainShutdownFlags(virDomainPtr domain, unsigned int flags) { int result = -1; @@ -5203,7 +5225,9 @@ static virDriver esxDriver = { .domainLookupByUUID = esxDomainLookupByUUID, /* 0.7.0 */ .domainLookupByName = esxDomainLookupByName, /* 0.7.0 */ .domainSuspend = esxDomainSuspend, /* 0.7.0 */ + .domainSuspendFlags = esxDomainSuspendFlags, /* 1.2.2 */ .domainResume = esxDomainResume, /* 0.7.0 */ + .domainResumeFlags = esxDomainResumeFlags, /* 1.2.2 */
Otherwise trivial; and ACK if the concept of 1 and 2 is approved. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/hyperv/hyperv_driver.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 6680e66..4993da2 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -541,12 +541,15 @@ hypervDomainLookupByName(virConnectPtr conn, const char *name) static int -hypervDomainSuspend(virDomainPtr domain) +hypervDomainSuspendFlags(virDomainPtr domain, + unsigned int flags) { int result = -1; hypervPrivate *priv = domain->conn->privateData; Msvm_ComputerSystem *computerSystem = NULL; + virCheckFlags(0, -1); + if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) { goto cleanup; } @@ -570,12 +573,23 @@ hypervDomainSuspend(virDomainPtr domain) static int -hypervDomainResume(virDomainPtr domain) +hypervDomainSuspend(virDomainPtr domain) +{ + return hypervDomainSuspendFlags(domain, 0); +} + + + +static int +hypervDomainResumeFlags(virDomainPtr domain, + unsigned int flags) { int result = -1; hypervPrivate *priv = domain->conn->privateData; Msvm_ComputerSystem *computerSystem = NULL; + virCheckFlags(0, -1); + if (hypervMsvmComputerSystemFromDomain(domain, &computerSystem) < 0) { goto cleanup; } @@ -599,6 +613,14 @@ hypervDomainResume(virDomainPtr domain) static int +hypervDomainResume(virDomainPtr domain) +{ + return hypervDomainResumeFlags(domain, 0); +} + + + +static int hypervDomainDestroyFlags(virDomainPtr domain, unsigned int flags) { int result = -1; @@ -1370,7 +1392,9 @@ static virDriver hypervDriver = { .domainLookupByUUID = hypervDomainLookupByUUID, /* 0.9.5 */ .domainLookupByName = hypervDomainLookupByName, /* 0.9.5 */ .domainSuspend = hypervDomainSuspend, /* 0.9.5 */ + .domainSuspendFlags = hypervDomainSuspendFlags, /* 1.2.2 */ .domainResume = hypervDomainResume, /* 0.9.5 */ + .domainResumeFlags = hypervDomainResumeFlags, /* 1.2.2 */ .domainDestroy = hypervDomainDestroy, /* 0.9.5 */ .domainDestroyFlags = hypervDomainDestroyFlags, /* 0.9.5 */ .domainGetOSType = hypervDomainGetOSType, /* 0.9.5 */ -- 1.8.5.2
On 02/03/2014 09:17 AM, Michal Privoznik wrote:
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/hyperv/hyperv_driver.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-)
+} + + + +static int hypervDomainDestroyFlags(virDomainPtr domain, unsigned int flags)
Why 3 blank lines?
{ int result = -1; @@ -1370,7 +1392,9 @@ static virDriver hypervDriver = { .domainLookupByUUID = hypervDomainLookupByUUID, /* 0.9.5 */ .domainLookupByName = hypervDomainLookupByName, /* 0.9.5 */ .domainSuspend = hypervDomainSuspend, /* 0.9.5 */ + .domainSuspendFlags = hypervDomainSuspendFlags, /* 1.2.2 */ .domainResume = hypervDomainResume, /* 0.9.5 */ + .domainResumeFlags = hypervDomainResumeFlags, /* 1.2.2 */
Otherwise good to go if the concept is approved. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libxl/libxl_driver.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index cb3deec..f101498 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1294,7 +1294,8 @@ libxlDomainLookupByName(virConnectPtr conn, const char *name) } static int -libxlDomainSuspend(virDomainPtr dom) +libxlDomainSuspendFlags(virDomainPtr dom, + unsigned int flags) { libxlDriverPrivatePtr driver = dom->conn->privateData; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); @@ -1303,10 +1304,12 @@ libxlDomainSuspend(virDomainPtr dom) virObjectEventPtr event = NULL; int ret = -1; + virCheckFlags(0, -1); + if (!(vm = libxlDomObjFromDomain(dom))) goto cleanup; - if (virDomainSuspendEnsureACL(dom->conn, vm->def) < 0) + if (virDomainSuspendFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -1346,7 +1349,14 @@ cleanup: static int -libxlDomainResume(virDomainPtr dom) +libxlDomainSuspend(virDomainPtr dom) +{ + return libxlDomainSuspendFlags(dom, 0); +} + +static int +libxlDomainResumeFlags(virDomainPtr dom, + unsigned int flags) { libxlDriverPrivatePtr driver = dom->conn->privateData; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); @@ -1355,10 +1365,12 @@ libxlDomainResume(virDomainPtr dom) virObjectEventPtr event = NULL; int ret = -1; + virCheckFlags(0, -1); + if (!(vm = libxlDomObjFromDomain(dom))) goto cleanup; - if (virDomainResumeEnsureACL(dom->conn, vm->def) < 0) + if (virDomainResumeFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -1398,6 +1410,12 @@ cleanup: } static int +libxlDomainResume(virDomainPtr dom) +{ + return libxlDomainResumeFlags(dom, 0); +} + +static int libxlDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { virDomainObjPtr vm; @@ -4344,7 +4362,9 @@ static virDriver libxlDriver = { .domainLookupByUUID = libxlDomainLookupByUUID, /* 0.9.0 */ .domainLookupByName = libxlDomainLookupByName, /* 0.9.0 */ .domainSuspend = libxlDomainSuspend, /* 0.9.0 */ + .domainSuspendFlags = libxlDomainSuspendFlags, /* 1.2.2 */ .domainResume = libxlDomainResume, /* 0.9.0 */ + .domainResumeFlags = libxlDomainResumeFlags, /* 1.2.2 */ .domainShutdown = libxlDomainShutdown, /* 0.9.0 */ .domainShutdownFlags = libxlDomainShutdownFlags, /* 0.9.10 */ .domainReboot = libxlDomainReboot, /* 0.9.0 */ -- 1.8.5.2
On 02/03/2014 09:17 AM, Michal Privoznik wrote:
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libxl/libxl_driver.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-)
+} + +static int +libxlDomainResumeFlags(virDomainPtr dom,
Lately we've been favoring 2 blank lines before a function, but you're being consistent with the rest of this file. ACK if the concept is approved. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/lxc/lxc_driver.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 138c706..656fd13 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3055,7 +3055,9 @@ cleanup: return ret; } -static int lxcDomainSuspend(virDomainPtr dom) +static int +lxcDomainSuspendFlags(virDomainPtr dom, + unsigned int flags) { virLXCDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; @@ -3063,10 +3065,12 @@ static int lxcDomainSuspend(virDomainPtr dom) int ret = -1; virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver); + virCheckFlags(0, -1); + if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; - if (virDomainSuspendEnsureACL(dom->conn, vm->def) < 0) + if (virDomainSuspendFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -3101,7 +3105,15 @@ cleanup: return ret; } -static int lxcDomainResume(virDomainPtr dom) +static int +lxcDomainSuspend(virDomainPtr dom) +{ + return lxcDomainSuspendFlags(dom, 0); +} + +static int +lxcDomainResumeFlags(virDomainPtr dom, + unsigned int flags) { virLXCDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; @@ -3110,12 +3122,14 @@ static int lxcDomainResume(virDomainPtr dom) virLXCDomainObjPrivatePtr priv; virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver); + virCheckFlags(0, -1); + if (!(vm = lxcDomObjFromDomain(dom))) goto cleanup; priv = vm->privateData; - if (virDomainResumeEnsureACL(dom->conn, vm->def) < 0) + if (virDomainResumeFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -3152,6 +3166,12 @@ cleanup: } static int +lxcDomainResume(virDomainPtr dom) +{ + return lxcDomainResumeFlags(dom, 0); +} + +static int lxcDomainOpenConsole(virDomainPtr dom, const char *dev_name, virStreamPtr st, @@ -5360,7 +5380,9 @@ static virDriver lxcDriver = { .domainLookupByUUID = lxcDomainLookupByUUID, /* 0.4.2 */ .domainLookupByName = lxcDomainLookupByName, /* 0.4.2 */ .domainSuspend = lxcDomainSuspend, /* 0.7.2 */ + .domainSuspendFlags = lxcDomainSuspendFlags, /* 1.2.2 */ .domainResume = lxcDomainResume, /* 0.7.2 */ + .domainResumeFlags = lxcDomainResumeFlags, /* 1.2.2 */ .domainDestroy = lxcDomainDestroy, /* 0.4.4 */ .domainDestroyFlags = lxcDomainDestroyFlags, /* 0.9.4 */ .domainGetOSType = lxcDomainGetOSType, /* 0.4.2 */ -- 1.8.5.2
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/openvz/openvz_driver.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index b27ac4c..54b7c64 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -577,12 +577,17 @@ static void openvzSetProgramSentinal(const char **prog, const char *key) } } -static int openvzDomainSuspend(virDomainPtr dom) { +static int +openvzDomainSuspendFlags(virDomainPtr dom, + unsigned int flags) +{ struct openvz_driver *driver = dom->conn->privateData; virDomainObjPtr vm; const char *prog[] = {VZCTL, "--quiet", "chkpnt", PROGRAM_SENTINEL, "--suspend", NULL}; int ret = -1; + virCheckFlags(0, -1); + openvzDriverLock(driver); vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); openvzDriverUnlock(driver); @@ -615,12 +620,23 @@ cleanup: return ret; } -static int openvzDomainResume(virDomainPtr dom) { +static int +openvzDomainSuspend(virDomainPtr dom) +{ + return openvzDomainSuspendFlags(dom, 0); +} + +static int +openvzDomainResumeFlags(virDomainPtr dom, + unsigned int flags) +{ struct openvz_driver *driver = dom->conn->privateData; virDomainObjPtr vm; const char *prog[] = {VZCTL, "--quiet", "chkpnt", PROGRAM_SENTINEL, "--resume", NULL}; int ret = -1; + virCheckFlags(0, -1); + openvzDriverLock(driver); vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); openvzDriverUnlock(driver); @@ -654,6 +670,12 @@ cleanup: } static int +openvzDomainResume(virDomainPtr dom) +{ + return openvzDomainResumeFlags(dom, 0); +} + +static int openvzDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { struct openvz_driver *driver = dom->conn->privateData; @@ -2210,7 +2232,9 @@ static virDriver openvzDriver = { .domainLookupByUUID = openvzDomainLookupByUUID, /* 0.3.1 */ .domainLookupByName = openvzDomainLookupByName, /* 0.3.1 */ .domainSuspend = openvzDomainSuspend, /* 0.8.3 */ + .domainSuspendFlags = openvzDomainSuspendFlags, /* 1.2.2 */ .domainResume = openvzDomainResume, /* 0.8.3 */ + .domainResumeFlags = openvzDomainResumeFlags, /* 1.2.2 */ .domainShutdown = openvzDomainShutdown, /* 0.3.1 */ .domainShutdownFlags = openvzDomainShutdownFlags, /* 0.9.10 */ .domainReboot = openvzDomainReboot, /* 0.3.1 */ -- 1.8.5.2
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/parallels/parallels_driver.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index 33260ef..ad41dbd 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -1388,28 +1388,46 @@ static int parallelsPause(virDomainObjPtr privdom) } static int -parallelsDomainSuspend(virDomainPtr domain) +parallelsDomainSuspendFlags(virDomainPtr domain, + unsigned int flags) { + virCheckFlags(0, -1); + return parallelsDomainChangeState(domain, VIR_DOMAIN_RUNNING, "running", parallelsPause, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_USER); } +static int +parallelsDomainSuspend(virDomainPtr domain) +{ + return parallelsDomainSuspendFlags(domain, 0); +} + static int parallelsResume(virDomainObjPtr privdom) { return parallelsCmdRun(PRLCTL, "resume", PARALLELS_UUID(privdom), NULL); } static int -parallelsDomainResume(virDomainPtr domain) +parallelsDomainResumeFlags(virDomainPtr domain, + unsigned int flags) { + virCheckFlags(0, -1); + return parallelsDomainChangeState(domain, VIR_DOMAIN_PAUSED, "paused", parallelsResume, VIR_DOMAIN_RUNNING, VIR_DOMAIN_RUNNING_UNPAUSED); } +static int +parallelsDomainResume(virDomainPtr domain) +{ + return parallelsDomainResumeFlags(domain, 0); +} + static int parallelsStart(virDomainObjPtr privdom) { return parallelsCmdRun(PRLCTL, "start", PARALLELS_UUID(privdom), NULL); @@ -2401,7 +2419,9 @@ static virDriver parallelsDriver = { .domainIsPersistent = parallelsDomainIsPersistent, /* 0.10.0 */ .domainGetAutostart = parallelsDomainGetAutostart, /* 0.10.0 */ .domainSuspend = parallelsDomainSuspend, /* 0.10.0 */ + .domainSuspendFlags = parallelsDomainSuspendFlags, /* 1.2.2 */ .domainResume = parallelsDomainResume, /* 0.10.0 */ + .domainResumeFlags = parallelsDomainResumeFlags, /* 1.2.2 */ .domainDestroy = parallelsDomainDestroy, /* 0.10.0 */ .domainShutdown = parallelsDomainShutdown, /* 0.10.0 */ .domainCreate = parallelsDomainCreate, /* 0.10.0 */ -- 1.8.5.2
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0128356..4aa8e1f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1663,7 +1663,10 @@ cleanup: } -static int qemuDomainSuspend(virDomainPtr dom) { +static int +qemuDomainSuspendFlags(virDomainPtr dom, + unsigned int flags) +{ virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; @@ -1674,10 +1677,12 @@ static int qemuDomainSuspend(virDomainPtr dom) { int state; virQEMUDriverConfigPtr cfg = NULL; + virCheckFlags(0, -1); + if (!(vm = qemuDomObjFromDomain(dom))) return -1; - if (virDomainSuspendEnsureACL(dom->conn, vm->def) < 0) + if (virDomainSuspendFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (!virDomainObjIsActive(vm)) { @@ -1744,7 +1749,17 @@ cleanup: } -static int qemuDomainResume(virDomainPtr dom) { +static int +qemuDomainSuspend(virDomainPtr dom) +{ + return qemuDomainSuspendFlags(dom, 0); +} + + +static int +qemuDomainResumeFlags(virDomainPtr dom, + unsigned int flags) +{ virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; int ret = -1; @@ -1753,12 +1768,14 @@ static int qemuDomainResume(virDomainPtr dom) { virQEMUDriverConfigPtr cfg = NULL; virCapsPtr caps = NULL; + virCheckFlags(0, -1); + if (!(vm = qemuDomObjFromDomain(dom))) return -1; cfg = virQEMUDriverGetConfig(driver); - if (virDomainResumeEnsureACL(dom->conn, vm->def) < 0) + if (virDomainResumeFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) @@ -1808,6 +1825,14 @@ cleanup: return ret; } + +static int +qemuDomainResume(virDomainPtr dom) +{ + return qemuDomainResumeFlags(dom, 0); +} + + static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; @@ -16487,7 +16512,9 @@ static virDriver qemuDriver = { .domainLookupByUUID = qemuDomainLookupByUUID, /* 0.2.0 */ .domainLookupByName = qemuDomainLookupByName, /* 0.2.0 */ .domainSuspend = qemuDomainSuspend, /* 0.2.0 */ + .domainSuspendFlags = qemuDomainSuspendFlags, /* 1.2.2 */ .domainResume = qemuDomainResume, /* 0.2.0 */ + .domainResumeFlags = qemuDomainResumeFlags, /* 1.2.2 */ .domainShutdown = qemuDomainShutdown, /* 0.2.0 */ .domainShutdownFlags = qemuDomainShutdownFlags, /* 0.9.10 */ .domainReboot = qemuDomainReboot, /* 0.9.3 */ -- 1.8.5.2
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/test/test_driver.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 4c277bd..a68cf2a 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1936,13 +1936,17 @@ cleanup: return ret; } -static int testDomainResume(virDomainPtr domain) +static int +testDomainResumeFlags(virDomainPtr domain, + unsigned int flags) { testConnPtr privconn = domain->conn->privateData; virDomainObjPtr privdom; virObjectEventPtr event = NULL; int ret = -1; + virCheckFlags(0, -1); + testDriverLock(privconn); privdom = virDomainObjListFindByName(privconn->domains, domain->name); @@ -1977,7 +1981,15 @@ cleanup: return ret; } -static int testDomainSuspend(virDomainPtr domain) +static int +testDomainResume(virDomainPtr domain) +{ + return testDomainResumeFlags(domain, 0); +} + +static int +testDomainSuspendFlags(virDomainPtr domain, + unsigned int flags) { testConnPtr privconn = domain->conn->privateData; virDomainObjPtr privdom; @@ -1985,6 +1997,8 @@ static int testDomainSuspend(virDomainPtr domain) int ret = -1; int state; + virCheckFlags(0, -1); + testDriverLock(privconn); privdom = virDomainObjListFindByName(privconn->domains, domain->name); @@ -2020,6 +2034,12 @@ cleanup: return ret; } +static int +testDomainSuspend(virDomainPtr domain) +{ + return testDomainSuspendFlags(domain, 0); +} + static int testDomainShutdownFlags(virDomainPtr domain, unsigned int flags) { @@ -7337,7 +7357,9 @@ static virDriver testDriver = { .domainLookupByUUID = testDomainLookupByUUID, /* 0.1.1 */ .domainLookupByName = testDomainLookupByName, /* 0.1.1 */ .domainSuspend = testDomainSuspend, /* 0.1.1 */ + .domainSuspendFlags = testDomainSuspendFlags, /* 1.2.2 */ .domainResume = testDomainResume, /* 0.1.1 */ + .domainResumeFlags = testDomainResumeFlags, /* 1.2.2 */ .domainShutdown = testDomainShutdown, /* 0.1.1 */ .domainShutdownFlags = testDomainShutdownFlags, /* 0.9.10 */ .domainReboot = testDomainReboot, /* 0.1.1 */ -- 1.8.5.2
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/vbox/vbox_tmpl.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 1be4dc4..6576125 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -1573,7 +1573,10 @@ cleanup: return ret; } -static int vboxDomainSuspend(virDomainPtr dom) { +static int +vboxDomainSuspendFlags(virDomainPtr dom, + unsigned int flags) +{ VBOX_OBJECT_CHECK(dom->conn, int, -1); IMachine *machine = NULL; vboxIID iid = VBOX_IID_INITIALIZER; @@ -1582,6 +1585,8 @@ static int vboxDomainSuspend(virDomainPtr dom) { PRUint32 state; nsresult rc; + virCheckFlags(0, -1); + vboxIIDFromUUID(&iid, dom->uuid); rc = VBOX_OBJECT_GET_MACHINE(iid.value, &machine); if (NS_FAILED(rc)) { @@ -1624,7 +1629,16 @@ cleanup: return ret; } -static int vboxDomainResume(virDomainPtr dom) { +static int +vboxDomainSuspend(virDomainPtr dom) +{ + return vboxDomainSuspendFlags(dom, 0); +} + +static int +vboxDomainResumeFlags(virDomainPtr dom, + unsigned int flags) +{ VBOX_OBJECT_CHECK(dom->conn, int, -1); IMachine *machine = NULL; vboxIID iid = VBOX_IID_INITIALIZER; @@ -1634,6 +1648,8 @@ static int vboxDomainResume(virDomainPtr dom) { PRBool isAccessible = PR_FALSE; + virCheckFlags(0, -1); + vboxIIDFromUUID(&iid, dom->uuid); rc = VBOX_OBJECT_GET_MACHINE(iid.value, &machine); if (NS_FAILED(rc)) { @@ -1676,6 +1692,12 @@ cleanup: return ret; } +static int +vboxDomainResume(virDomainPtr dom) +{ + return vboxDomainResumeFlags(dom, 0); +} + static int vboxDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { VBOX_OBJECT_CHECK(dom->conn, int, -1); @@ -9500,7 +9522,9 @@ virDriver NAME(Driver) = { .domainLookupByUUID = vboxDomainLookupByUUID, /* 0.6.3 */ .domainLookupByName = vboxDomainLookupByName, /* 0.6.3 */ .domainSuspend = vboxDomainSuspend, /* 0.6.3 */ + .domainSuspendFlags = vboxDomainSuspendFlags, /* 1.2.2 */ .domainResume = vboxDomainResume, /* 0.6.3 */ + .domainResumeFlags = vboxDomainResumeFlags, /* 1.2.2 */ .domainShutdown = vboxDomainShutdown, /* 0.6.3 */ .domainShutdownFlags = vboxDomainShutdownFlags, /* 0.9.10 */ .domainReboot = vboxDomainReboot, /* 0.6.3 */ -- 1.8.5.2
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/vmware/vmware_driver.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 0e7a37f..ba1b8ed 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -472,7 +472,8 @@ vmwareDomainDestroyFlags(virDomainPtr dom, } static int -vmwareDomainSuspend(virDomainPtr dom) +vmwareDomainSuspendFlags(virDomainPtr dom, + unsigned int flags) { struct vmware_driver *driver = dom->conn->privateData; @@ -483,6 +484,8 @@ vmwareDomainSuspend(virDomainPtr dom) }; int ret = -1; + virCheckFlags(0, -1); + if (driver->type == VMWARE_DRIVER_PLAYER) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("vmplayer does not support libvirt suspend/resume" @@ -521,7 +524,14 @@ vmwareDomainSuspend(virDomainPtr dom) } static int -vmwareDomainResume(virDomainPtr dom) +vmwareDomainSuspend(virDomainPtr dom) +{ + return vmwareDomainSuspendFlags(dom, 0); +} + +static int +vmwareDomainResumeFlags(virDomainPtr dom, + unsigned int flags) { struct vmware_driver *driver = dom->conn->privateData; @@ -532,6 +542,8 @@ vmwareDomainResume(virDomainPtr dom) }; int ret = -1; + virCheckFlags(0, -1); + if (driver->type == VMWARE_DRIVER_PLAYER) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("vmplayer does not support libvirt suspend/resume " @@ -570,6 +582,12 @@ vmwareDomainResume(virDomainPtr dom) } static int +vmwareDomainResume(virDomainPtr dom) +{ + return vmwareDomainResumeFlags(dom, 0); +} + +static int vmwareDomainReboot(virDomainPtr dom, unsigned int flags) { struct vmware_driver *driver = dom->conn->privateData; @@ -1175,7 +1193,9 @@ static virDriver vmwareDriver = { .domainLookupByUUID = vmwareDomainLookupByUUID, /* 0.8.7 */ .domainLookupByName = vmwareDomainLookupByName, /* 0.8.7 */ .domainSuspend = vmwareDomainSuspend, /* 0.8.7 */ + .domainSuspendFlags = vmwareDomainSuspendFlags, /* 1.2.2 */ .domainResume = vmwareDomainResume, /* 0.8.7 */ + .domainResumeFlags = vmwareDomainResumeFlags, /* 1.2.2 */ .domainShutdown = vmwareDomainShutdown, /* 0.8.7 */ .domainShutdownFlags = vmwareDomainShutdownFlags, /* 0.9.10 */ .domainReboot = vmwareDomainReboot, /* 0.8.7 */ -- 1.8.5.2
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/xen/xen_driver.c | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index c45d980..704171f 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -903,15 +903,18 @@ xenUnifiedDomainIsUpdated(virDomainPtr dom ATTRIBUTE_UNUSED) } static int -xenUnifiedDomainSuspend(virDomainPtr dom) +xenUnifiedDomainSuspendFlags(virDomainPtr dom, + unsigned int flags) { int ret = -1; virDomainDefPtr def; + virCheckFlags(0, -1); + if (!(def = xenGetDomainDefForDom(dom))) goto cleanup; - if (virDomainSuspendEnsureACL(dom->conn, def) < 0) + if (virDomainSuspendFlagsEnsureACL(dom->conn, def) < 0) goto cleanup; ret = xenDaemonDomainSuspend(dom->conn, def); @@ -922,15 +925,24 @@ cleanup: } static int -xenUnifiedDomainResume(virDomainPtr dom) +xenUnifiedDomainSuspend(virDomainPtr dom) +{ + return xenUnifiedDomainSuspendFlags(dom, 0); +} + +static int +xenUnifiedDomainResumeFlags(virDomainPtr dom, + unsigned int flags) { int ret = -1; virDomainDefPtr def; + virCheckFlags(0, -1); + if (!(def = xenGetDomainDefForDom(dom))) goto cleanup; - if (virDomainResumeEnsureACL(dom->conn, def) < 0) + if (virDomainResumeFlagsEnsureACL(dom->conn, def) < 0) goto cleanup; ret = xenDaemonDomainResume(dom->conn, def); @@ -941,6 +953,12 @@ cleanup: } static int +xenUnifiedDomainResume(virDomainPtr dom) +{ + return xenUnifiedDomainResumeFlags(dom, 0); +} + +static int xenUnifiedDomainShutdownFlags(virDomainPtr dom, unsigned int flags) { @@ -2737,7 +2755,9 @@ static virDriver xenUnifiedDriver = { .domainLookupByUUID = xenUnifiedDomainLookupByUUID, /* 0.0.5 */ .domainLookupByName = xenUnifiedDomainLookupByName, /* 0.0.3 */ .domainSuspend = xenUnifiedDomainSuspend, /* 0.0.3 */ + .domainSuspendFlags = xenUnifiedDomainSuspendFlags, /* 1.2.2 */ .domainResume = xenUnifiedDomainResume, /* 0.0.3 */ + .domainResumeFlags = xenUnifiedDomainResumeFlags, /* 1.2.2 */ .domainShutdown = xenUnifiedDomainShutdown, /* 0.0.3 */ .domainShutdownFlags = xenUnifiedDomainShutdownFlags, /* 0.9.10 */ .domainReboot = xenUnifiedDomainReboot, /* 0.1.0 */ -- 1.8.5.2
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/xenapi/xenapi_driver.c | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index a547306..01bfc98 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -712,18 +712,22 @@ xenapiDomainLookupByName(virConnectPtr conn, } /* - * xenapiDomainSuspend + * xenapiDomainSuspendFlags * * a VM is paused * Returns 0 on success or -1 in case of error */ static int -xenapiDomainSuspend(virDomainPtr dom) +xenapiDomainSuspendFlags(virDomainPtr dom, + unsigned int flags) { /* vm.pause() */ xen_vm vm; xen_vm_set *vms = NULL; xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session; + + virCheckFlags(0, -1); + if (xen_vm_get_by_name_label(session, &vms, dom->name) && vms->size > 0) { if (vms->size != 1) { xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, @@ -748,18 +752,34 @@ xenapiDomainSuspend(virDomainPtr dom) } /* - * xenapiDomainResume + * xenapiDomainSuspend + * + * a VM is paused + * Returns 0 on success or -1 in case of error + */ +static int +xenapiDomainSuspend(virDomainPtr dom) +{ + return xenapiDomainSuspendFlags(dom, 0); +} + +/* + * xenapiDomainResumeFlags * * Resumes a VM * Returns 0 on success or -1 in case of error */ static int -xenapiDomainResume(virDomainPtr dom) +xenapiDomainResumeFlags(virDomainPtr dom, + unsigned int flags) { /* vm.unpause() */ xen_vm vm; xen_vm_set *vms; xen_session *session = ((struct _xenapiPrivate *)(dom->conn->privateData))->session; + + virCheckFlags(0, -1); + if (xen_vm_get_by_name_label(session, &vms, dom->name) && vms->size > 0) { if (vms->size != 1) { xenapiSessionErrorHandler(dom->conn, VIR_ERR_INTERNAL_ERROR, @@ -784,6 +804,18 @@ xenapiDomainResume(virDomainPtr dom) } /* + * xenapiDomainResume + * + * Resumes a VM + * Returns 0 on success or -1 in case of error + */ +static int +xenapiDomainResume(virDomainPtr dom) +{ + return xenapiDomainResumeFlags(dom, 0); +} + +/* * xenapiDomainShutdown * * shutsdown a VM @@ -1962,7 +1994,9 @@ static virDriver xenapiDriver = { .domainLookupByUUID = xenapiDomainLookupByUUID, /* 0.8.0 */ .domainLookupByName = xenapiDomainLookupByName, /* 0.8.0 */ .domainSuspend = xenapiDomainSuspend, /* 0.8.0 */ + .domainSuspendFlags = xenapiDomainSuspendFlags, /* 1.2.2 */ .domainResume = xenapiDomainResume, /* 0.8.0 */ + .domainResumeFlags = xenapiDomainResumeFlags, /* 1.2.2 */ .domainShutdown = xenapiDomainShutdown, /* 0.8.0 */ .domainShutdownFlags = xenapiDomainShutdownFlags, /* 0.9.10 */ .domainReboot = xenapiDomainReboot, /* 0.8.0 */ -- 1.8.5.2
On 02/03/2014 09:17 AM, Michal Privoznik wrote:
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/xenapi/xenapi_driver.c | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-)
6-14 are all mechanical, ACK to all if concept is approved. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 02/03/2014 10:50 AM, Eric Blake wrote:
On 02/03/2014 09:17 AM, Michal Privoznik wrote:
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/xenapi/xenapi_driver.c | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-)
6-14 are all mechanical, ACK to all if concept is approved.
Oh, and forgot to mention - almost all of them also have the same comment about inconsistencies on 2 blanks between functions. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03.02.2014 18:51, Eric Blake wrote:
On 02/03/2014 10:50 AM, Eric Blake wrote:
On 02/03/2014 09:17 AM, Michal Privoznik wrote:
So far, there are no flags to implement, so just call the corresponding function with 0 passed as @flags.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/xenapi/xenapi_driver.c | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-)
6-14 are all mechanical, ACK to all if concept is approved.
Oh, and forgot to mention - almost all of them also have the same comment about inconsistencies on 2 blanks between functions.
I've just tried to keep up with the structure of files I've been changing. Michal
Things get complicated once we want to fall back to using the older versions of APIs when talking to older libvirtd. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain.c | 52 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 13 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c3db94c..9f71770 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2767,18 +2767,31 @@ cmdSuspend(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; const char *name; - bool ret = true; + bool ret = false; + unsigned int flags = 0; if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) - return false; + return ret; - if (virDomainSuspend(dom) == 0) { - vshPrint(ctl, _("Domain %s suspended\n"), name); - } else { - vshError(ctl, _("Failed to suspend domain %s"), name); - ret = false; + if (virDomainSuspendFlags(dom, flags) < 0) { + /* Fallback to older API iff no flag was requested */ + if (flags || last_error->code != VIR_ERR_NO_SUPPORT) { + vshError(ctl, _("Failed to suspend domain %s"), name); + goto cleanup; + } + + virResetLastError(); + + if (virDomainSuspend(dom) < 0) { + vshError(ctl, _("Failed to suspend domain %s"), name); + goto cleanup; + } } + vshPrint(ctl, _("Domain %s suspended\n"), name); + ret = true; + +cleanup: virDomainFree(dom); return ret; } @@ -4791,19 +4804,32 @@ static bool cmdResume(vshControl *ctl, const vshCmd *cmd) { virDomainPtr dom; - bool ret = true; + bool ret = false; const char *name; + unsigned int flags = 0; if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - if (virDomainResume(dom) == 0) { - vshPrint(ctl, _("Domain %s resumed\n"), name); - } else { - vshError(ctl, _("Failed to resume domain %s"), name); - ret = false; + if (virDomainResumeFlags(dom, flags) < 0) { + /* Fallback to older API iff no flag was requested */ + if (flags || last_error->code != VIR_ERR_NO_SUPPORT) { + vshError(ctl, _("Failed to resume domain %s"), name); + goto cleanup; + } + + virResetLastError(); + + if (virDomainResume(dom) < 0) { + vshError(ctl, _("Failed to resume domain %s"), name); + goto cleanup; + } } + vshPrint(ctl, _("Domain %s resumed\n"), name); + ret = true; + +cleanup: virDomainFree(dom); return ret; } -- 1.8.5.2
On 02/03/2014 09:17 AM, Michal Privoznik wrote:
Things get complicated once we want to fall back to using the older versions of APIs when talking to older libvirtd.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain.c | 52 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 13 deletions(-)
- if (virDomainSuspend(dom) == 0) { - vshPrint(ctl, _("Domain %s suspended\n"), name); - } else { - vshError(ctl, _("Failed to suspend domain %s"), name); - ret = false; + if (virDomainSuspendFlags(dom, flags) < 0) { + /* Fallback to older API iff no flag was requested */ + if (flags || last_error->code != VIR_ERR_NO_SUPPORT) {
Your logic: try new API if (fail) { if (flags requested or failure is unrelated to unknown function) goto done try fallback of old api } Elsewhere, we have coded things to be more efficient, with only one API attempt that always favors the oldest API known to serve the user's request: if (user requested flags) call new API else call old API But that only works if there is an addition of a flag to make it worth calling the new API. Which goes back to the question in patch 1 on what we plan on adding to make use of a flag. So your conversion looks okay to me for being easier to modify if we add a flag in the future. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Feb 03, 2014 at 05:16:57PM +0100, Michal Privoznik wrote:
So far, nothing interesting is happening here. The real work is yet to come :)
There's nothing wrong with the patches, but I think we really should have info about what's motivating this change before we add new APIs. There's always the possibility that we need other API parameters added beyond just the flags, but we can't tell that without more info... Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Martin Kletzander -
Michal Privoznik