[libvirt] [PATCH v2 0/5] Allow hibernation on guests
As we've added guest agent recently, the whole world of new functionality has opened. As this patch set, which allows domains to enter S4 state. What is needed for this? Patched qemu. As this is not in qemu git, but patches are await to be pushed in very short future. They can be found here: http://lists.gnu.org/archive/html/qemu-devel/2012-01/msg03261.html Despite that, if anybody is willing to give me review if I am going the right way, I'd appreciate it. One thing, that you'll probably notice is this 'set-support-level' command. Basically, it tells GA what qemu version is it running on. Ideally, this should be done as soon as GA starts up. However, that cannot be determined from outside world as GA doesn't emit any events yet. Ideally^2 this command should be left out as it should be qemu who tells its own agent this kind of information. Anyway, I was going to call this command in qemuProcess{Startup, Reconnect,Attach}, but it won't work. We need to un-pause guest CPUs so guest can boot and start GA, but that implies returning from qemuProcess*. So I am setting this just before 'guest-suspend' command, as there is one more thing about GA. It is unable to remember anything upon its restart (GA process). Which has BTW show flaw in our current code with FS freeze & thaw. If we freeze guest FS, and somebody restart GA, the simple FS Thaw will not succeed as GA thinks FS are not frozen. But that's a different cup of tea. Because of what written above, we need to call set-level on every suspend. diff to v1: -move from misusing virDomainSuspend to brand new virDomainSuspendForDuration API Michal Privoznik (5): qemu-agent: Allow setting supported level qemu-agent: Create suspend function Introduce virDomainSuspendForDuration API qemu: Wire up virDomainSuspendForDuration API virsh: Expose new virDomainSuspendForDuration API include/libvirt/libvirt.h.in | 5 ++- src/driver.h | 6 ++ src/libvirt.c | 58 +++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/qemu/qemu_agent.c | 105 ++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 13 +++++ src/qemu/qemu_driver.c | 93 +++++++++++++++++++++++++++++++++++++ src/qemu/qemu_process.c | 45 +++++++++++++++++- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 11 ++++- src/remote_protocol-structs | 7 +++ tools/virsh.c | 67 +++++++++++++++++++++++++++ tools/virsh.pod | 8 +++ 13 files changed, 415 insertions(+), 5 deletions(-) -- 1.7.3.4
Some qemu guest-agent commands require so called 'supported level' to be set. In other words, GA needs to be told which version of qemu is being used. Moreover, guest-agent can't remember this information during its restarts. But QEMU developers are trying to learn guest-agent to remember it. --- src/qemu/qemu_agent.c | 35 +++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 2 ++ src/qemu/qemu_process.c | 45 ++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 9df5546..536724f 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -106,6 +106,16 @@ struct _qemuAgent { /* If anything went wrong, this will be fed back * the next monitor msg */ virError lastError; + + /* Some guest-agent commands are available only + * after telling guest-agent what qemu version it + * deals with. However, currently guest-agent is not + * capable of remembering this information. + * Therefore we need to memorize it and set + * before such commands. + * XXX Remove this in the future + */ + unsigned int version; }; #if DEBUG_RAW_IO @@ -1184,3 +1194,28 @@ cleanup: virJSONValueFree(reply); return ret; } + +/* + * qemuAgentSetSupportedLevelCommand: + * @mon: Agent + * @version: QEMU version + * + * Let GA knows what qemu version it deals with. + * This enables/disables some functionality on + * one hand, but on the other GA should cope nicely + * after this. + * + * QEMU @version format: + * major * 1000 * 1000 + minor * 1000 + micro + * as returned by qemuCapsExtractVersionInfo(). + * + * The minimum required by QEMU is 1.0.0, which + * makes @version MUST be >= 1000*1000 + */ +void qemuAgentSetSupportedLevel(qemuAgentPtr mon, + unsigned int version) +{ + qemuAgentLock(mon); + mon->version = version; + qemuAgentUnlock(mon); +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index df59ef7..2858207 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -69,4 +69,6 @@ int qemuAgentShutdown(qemuAgentPtr mon, int qemuAgentFSFreeze(qemuAgentPtr mon); int qemuAgentFSThaw(qemuAgentPtr mon); +void qemuAgentSetSupportedLevel(qemuAgentPtr mon, + unsigned int version); #endif /* __QEMU_AGENT_H__ */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index d22020b..a0b0159 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2825,6 +2825,7 @@ qemuProcessReconnect(void *opaque) struct qemuDomainJobObj oldjob; int state; int reason; + unsigned int version; memcpy(&oldjob, &data->oldjob, sizeof(oldjob)); @@ -2876,7 +2877,7 @@ qemuProcessReconnect(void *opaque) */ if (!priv->qemuCaps && qemuCapsExtractVersionInfo(obj->def->emulator, obj->def->os.arch, - NULL, + &version, &priv->qemuCaps) < 0) goto error; @@ -2925,6 +2926,18 @@ qemuProcessReconnect(void *opaque) if (obj->def->id >= driver->nextvmid) driver->nextvmid = obj->def->id + 1; + if (priv->agent) { + if (priv->agentError) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to set guest-supported-level" + " due to agent error")); + goto error; + } + + VIR_DEBUG("Setting guest-supported-level to %u", version); + qemuAgentSetSupportedLevel(priv->agent, version); + } + endjob: if (qemuDomainObjEndJob(driver, obj) == 0) obj = NULL; @@ -3083,6 +3096,7 @@ int qemuProcessStart(virConnectPtr conn, virCommandPtr cmd = NULL; struct qemuProcessHookData hookData; unsigned long cur_balloon; + unsigned int version; hookData.conn = conn; hookData.vm = vm; @@ -3212,7 +3226,7 @@ int qemuProcessStart(virConnectPtr conn, qemuCapsFree(priv->qemuCaps); priv->qemuCaps = NULL; if (qemuCapsExtractVersionInfo(vm->def->emulator, vm->def->os.arch, - NULL, + &version, &priv->qemuCaps) < 0) goto cleanup; @@ -3432,6 +3446,18 @@ int qemuProcessStart(virConnectPtr conn, priv->agentError = true; } + if (priv->agent) { + if (priv->agentError) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to set guest-supported-level" + " due to agent error")); + goto cleanup; + } + + VIR_DEBUG("Setting guest-supported-level to %u", version); + qemuAgentSetSupportedLevel(priv->agent, version); + } + VIR_DEBUG("Detecting VCPU PIDs"); if (qemuProcessDetectVcpuPIDs(driver, vm) < 0) goto cleanup; @@ -3782,6 +3808,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, bool running = true; virDomainPausedReason reason; virSecurityLabelPtr seclabel = NULL; + unsigned int version; VIR_DEBUG("Beginning VM attach process"); @@ -3835,7 +3862,7 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, priv->qemuCaps = NULL; if (qemuCapsExtractVersionInfo(vm->def->emulator, vm->def->os.arch, - NULL, + &version, &priv->qemuCaps) < 0) goto cleanup; @@ -3901,6 +3928,18 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, priv->agentError = true; } + if (priv->agent) { + if (priv->agentError) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to set guest-supported-level" + " due to agent error")); + goto cleanup; + } + + VIR_DEBUG("Setting guest-supported-level to %u", version); + qemuAgentSetSupportedLevel(priv->agent, version); + } + VIR_DEBUG("Detecting VCPU PIDs"); if (qemuProcessDetectVcpuPIDs(driver, vm) < 0) goto cleanup; -- 1.7.3.4
On Thu, Jan 26, 2012 at 08:59:43PM +0100, Michal Privoznik wrote:
Some qemu guest-agent commands require so called 'supported level' to be set. In other words, GA needs to be told which version of qemu is being used. Moreover, guest-agent can't remember this information during its restarts. But QEMU developers are trying to learn guest-agent to remember it. --- src/qemu/qemu_agent.c | 35 +++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 2 ++ src/qemu/qemu_process.c | 45 ++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 79 insertions(+), 3 deletions(-)
Given the uncertainty around this command in QEMU, we should drop this patch for now, until QEMU settles.Likewise for patch 2. We can still do the public API though. 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 :|
which allows guest to enter one of these modes: -hibernation: RAM is stored onto disk and guest is powered off (S4) -sleep: RAM stays powered, but CPUs are powered off (S3) -hybrid: RAM is stored onto disk but guest is not powered off --- src/qemu/qemu_agent.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_agent.h | 11 +++++++ 2 files changed, 81 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 536724f..f2cae94 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1219,3 +1219,73 @@ void qemuAgentSetSupportedLevel(qemuAgentPtr mon, mon->version = version; qemuAgentUnlock(mon); } + +static int +qemuAgentSetSupportedLevelCommand(qemuAgentPtr mon) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + unsigned int major, minor, micro; + unsigned int version = mon->version; + + if (version < 1000*1000) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid qemu version set: %u"), + version); + return -1; + } + + major = version / (1000 * 1000); + version %= 1000*1000; + minor = version / 1000; + version %= 1000; + micro = version; + + cmd = qemuAgentMakeCommand("guest-set-support-level", + "u:major", major, + "u:minor", minor, + "u:micro", micro, + NULL); + if (!cmd) + return -1; + + ret = qemuAgentCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuAgentCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + +VIR_ENUM_DECL(qemuAgentSuspendMode); + +VIR_ENUM_IMPL(qemuAgentSuspendMode, + QEMU_AGENT_SUSPEND_LAST, + "hibernate", "sleep", "hybrid"); + +int qemuAgentSuspend(qemuAgentPtr mon, + qemuAgentSuspendMode mode) +{ + int ret = -1; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuAgentMakeCommand("guest-suspend", + "s:mode", qemuAgentSuspendModeTypeToString(mode), + NULL); + if (!cmd) + return -1; + + /* XXX Unconditionally set supported level until + * qemu learns to keep this information */ + if ((ret = qemuAgentSetSupportedLevelCommand(mon)) == 0 && + (ret = qemuAgentCommand(mon, cmd, &reply)) == 0) + ret = qemuAgentCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 2858207..56e5573 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -71,4 +71,15 @@ int qemuAgentFSThaw(qemuAgentPtr mon); void qemuAgentSetSupportedLevel(qemuAgentPtr mon, unsigned int version); + +typedef enum { + QEMU_AGENT_SUSPEND_HIBERNATE, + QEMU_AGENT_SUSPEND_SLEEP, + QEMU_AGENT_SUSPEND_HYBRID, + + QEMU_AGENT_SUSPEND_LAST, +} qemuAgentSuspendMode; + +int qemuAgentSuspend(qemuAgentPtr mon, + qemuAgentSuspendMode mode); #endif /* __QEMU_AGENT_H__ */ -- 1.7.3.4
This API allows a domain to be put into one of S# ACPI states. Currently, S3 and S4 are supported. These states are shared with virNodeSuspendForDuration. However, for now we don't support any duration other than zero. The same apply for flags. --- include/libvirt/libvirt.h.in | 5 +++- src/driver.h | 6 ++++ src/libvirt.c | 58 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 11 +++++++- src/remote_protocol-structs | 7 +++++ 7 files changed, 87 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e99cd00..0117333 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1232,7 +1232,10 @@ int virDomainFree (virDomainPtr domain); */ int virDomainSuspend (virDomainPtr domain); int virDomainResume (virDomainPtr domain); - +int virDomainSuspendForDuration (virDomainPtr domain, + unsigned int target, + unsigned long long duration, + unsigned int flags); /* * Domain save/restore */ diff --git a/src/driver.h b/src/driver.h index df2aa60..a39b228 100644 --- a/src/driver.h +++ b/src/driver.h @@ -120,6 +120,11 @@ typedef int typedef int (*virDrvDomainResume) (virDomainPtr domain); typedef int + (*virDrvDomainSuspendForDuration) (virDomainPtr, + unsigned int target, + unsigned long long duration, + unsigned int flags); +typedef int (*virDrvDomainShutdown) (virDomainPtr domain); typedef int (*virDrvDomainReboot) (virDomainPtr domain, @@ -831,6 +836,7 @@ struct _virDriver { virDrvDomainLookupByUUID domainLookupByUUID; virDrvDomainLookupByName domainLookupByName; virDrvDomainSuspend domainSuspend; + virDrvDomainSuspendForDuration domainSuspendForDuration; virDrvDomainResume domainResume; virDrvDomainShutdown domainShutdown; virDrvDomainShutdownFlags domainShutdownFlags; diff --git a/src/libvirt.c b/src/libvirt.c index e9d638b..886776f 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2433,6 +2433,64 @@ error: } /** + * virDomainSuspendForDuration: + * @dom: a domain object + * @target: an OR'ed set of virNodeSuspendTarget + * @duration: currently unused, pass 0 + * @flags: ditto + * + * Attempt to suspend given domain. However, more + * states are supported than in virDomainSuspend. + * + * Dependent on hypervisor used, this may require + * guest agent to be available, e.g. QEMU. + * + * Returns: 0 on success, + * -1 on failure. + */ +int +virDomainSuspendForDuration(virDomainPtr dom, + unsigned int target, + unsigned long long duration, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(dom, "target=%u duration=%llu flags=%x", + target, duration, flags); + + virResetLastError(); + + if (!VIR_IS_CONNECTED_DOMAIN(dom)) { + virLibDomainError(VIR_ERR_INVALID_DOMAIN, __FUNCTION__); + virDispatchError(NULL); + return -1; + } + + if (dom->conn->flags & VIR_CONNECT_RO) { + virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + conn = dom->conn; + + if (conn->driver->domainSuspendForDuration) { + int ret; + ret = conn->driver->domainSuspendForDuration(dom, target, + duration, flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(conn); + return -1; +} + +/** * virDomainSave: * @domain: a domain object * @to: path for the output file diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 1340b0c..ef17ffc 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -520,6 +520,7 @@ LIBVIRT_0.9.10 { global: virDomainShutdownFlags; virStorageVolWipePattern; + virDomainSuspendForDuration; } LIBVIRT_0.9.9; # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index f79f53e..fca4319 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -4616,6 +4616,7 @@ static virDriver remote_driver = { .domainLookupByName = remoteDomainLookupByName, /* 0.3.0 */ .domainSuspend = remoteDomainSuspend, /* 0.3.0 */ .domainResume = remoteDomainResume, /* 0.3.0 */ + .domainSuspendForDuration = remoteDomainSuspendForDuration, /* 0.9.10 */ .domainShutdown = remoteDomainShutdown, /* 0.3.0 */ .domainShutdownFlags = remoteDomainShutdownFlags, /* 0.9.10 */ .domainReboot = remoteDomainReboot, /* 0.3.0 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 0f354bb..c0d0979 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -732,6 +732,13 @@ struct remote_domain_suspend_args { remote_nonnull_domain dom; }; +struct remote_domain_suspend_for_duration_args { + remote_nonnull_domain dom; + unsigned int target; + unsigned hyper duration; + unsigned int flags; +}; + struct remote_domain_resume_args { remote_nonnull_domain dom; }; @@ -2667,7 +2674,9 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_INTERFACE_PARAMETERS = 256, /* autogen autogen */ REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, /* skipgen skipgen */ REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS = 258, /* autogen autogen */ - REMOTE_PROC_STORAGE_VOL_WIPE_PATTERN = 259 /* autogen autogen */ + REMOTE_PROC_STORAGE_VOL_WIPE_PATTERN = 259, /* autogen autogen */ + + REMOTE_PROC_DOMAIN_SUSPEND_FOR_DURATION = 260 /* autogen autogen */ /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index de85862..e70dfd0 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -440,6 +440,12 @@ struct remote_domain_lookup_by_name_ret { struct remote_domain_suspend_args { remote_nonnull_domain dom; }; +struct remote_domain_suspend_for_duration_args { + remote_nonnull_domain dom; + u_int target; + uint64_t duration; + u_int flags; +}; struct remote_domain_resume_args { remote_nonnull_domain dom; }; @@ -2101,4 +2107,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_INTERFACE_PARAMETERS = 257, REMOTE_PROC_DOMAIN_SHUTDOWN_FLAGS = 258, REMOTE_PROC_STORAGE_VOL_WIPE_PATTERN = 259, + REMOTE_PROC_DOMAIN_SUSPEND_FOR_DURATION = 260, }; -- 1.7.3.4
On Thu, Jan 26, 2012 at 08:59:45PM +0100, Michal Privoznik wrote:
This API allows a domain to be put into one of S# ACPI states. Currently, S3 and S4 are supported. These states are shared with virNodeSuspendForDuration. However, for now we don't support any duration other than zero. The same apply for flags. --- include/libvirt/libvirt.h.in | 5 +++- src/driver.h | 6 ++++ src/libvirt.c | 58 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 11 +++++++- src/remote_protocol-structs | 7 +++++ 7 files changed, 87 insertions(+), 2 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e99cd00..0117333 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1232,7 +1232,10 @@ int virDomainFree (virDomainPtr domain); */ int virDomainSuspend (virDomainPtr domain); int virDomainResume (virDomainPtr domain); - +int virDomainSuspendForDuration (virDomainPtr domain, + unsigned int target, + unsigned long long duration, + unsigned int flags);
NB, I would prefer to have this called either virDomainPMSuspendForDuration virDomainNodeSuspendForDuration Since just using ""virDomainSuspendXXX" prefix, implies it is related to the existing API of that name. ACK if it is renamed to either one of those choices, or another suggestion someone might have ? 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 :|
On 27.01.2012 14:25, Daniel P. Berrange wrote:
On Thu, Jan 26, 2012 at 08:59:45PM +0100, Michal Privoznik wrote:
This API allows a domain to be put into one of S# ACPI states. Currently, S3 and S4 are supported. These states are shared with virNodeSuspendForDuration. However, for now we don't support any duration other than zero. The same apply for flags. --- include/libvirt/libvirt.h.in | 5 +++- src/driver.h | 6 ++++ src/libvirt.c | 58 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 11 +++++++- src/remote_protocol-structs | 7 +++++ 7 files changed, 87 insertions(+), 2 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e99cd00..0117333 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1232,7 +1232,10 @@ int virDomainFree (virDomainPtr domain); */ int virDomainSuspend (virDomainPtr domain); int virDomainResume (virDomainPtr domain); - +int virDomainSuspendForDuration (virDomainPtr domain, + unsigned int target, + unsigned long long duration, + unsigned int flags);
NB, I would prefer to have this called either
virDomainPMSuspendForDuration virDomainNodeSuspendForDuration
Since just using ""virDomainSuspendXXX" prefix, implies it is related to the existing API of that name.
Yes, but on the other hand, virDomainNode make me think it's (somehow, magically) related to the host :) So I'll choose virDomainPMSuspendForDuration here, and dompmsuspend in virsh (the last patch of this set). However, I am postponing the push for a while so others can chime in (Eric?) as it seems we need a wider consensus on this.
ACK if it is renamed to either one of those choices, or another suggestion someone might have ?
Daniel
Thanks. Michal
On Fri, Jan 27, 2012 at 02:34:45PM +0100, Michal Privoznik wrote:
On 27.01.2012 14:25, Daniel P. Berrange wrote:
On Thu, Jan 26, 2012 at 08:59:45PM +0100, Michal Privoznik wrote:
This API allows a domain to be put into one of S# ACPI states. Currently, S3 and S4 are supported. These states are shared with virNodeSuspendForDuration. However, for now we don't support any duration other than zero. The same apply for flags. --- include/libvirt/libvirt.h.in | 5 +++- src/driver.h | 6 ++++ src/libvirt.c | 58 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 11 +++++++- src/remote_protocol-structs | 7 +++++ 7 files changed, 87 insertions(+), 2 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index e99cd00..0117333 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1232,7 +1232,10 @@ int virDomainFree (virDomainPtr domain); */ int virDomainSuspend (virDomainPtr domain); int virDomainResume (virDomainPtr domain); - +int virDomainSuspendForDuration (virDomainPtr domain, + unsigned int target, + unsigned long long duration, + unsigned int flags);
NB, I would prefer to have this called either
virDomainPMSuspendForDuration virDomainNodeSuspendForDuration
Since just using ""virDomainSuspendXXX" prefix, implies it is related to the existing API of that name.
Yes, but on the other hand, virDomainNode make me think it's (somehow, magically) related to the host :)
So I'll choose virDomainPMSuspendForDuration here, and dompmsuspend in virsh (the last patch of this set).
Ok, that's fine with me 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 :|
On 01/27/2012 06:34 AM, Michal Privoznik wrote:
+int virDomainSuspendForDuration (virDomainPtr domain, + unsigned int target, + unsigned long long duration, + unsigned int flags);
NB, I would prefer to have this called either
virDomainPMSuspendForDuration virDomainNodeSuspendForDuration
Since just using ""virDomainSuspendXXX" prefix, implies it is related to the existing API of that name.
Yes, but on the other hand, virDomainNode make me think it's (somehow, magically) related to the host :)
I also dislike virDomainNode..., but think that virDomainPMSuspendForDuration sounds reasonable. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/26/2012 12:59 PM, Michal Privoznik wrote:
This API allows a domain to be put into one of S# ACPI states. Currently, S3 and S4 are supported. These states are shared with virNodeSuspendForDuration. However, for now we don't support any duration other than zero. The same apply for flags.
I'm not reviewing 1, 2, or 4(as Dan said, there's still some churn on the qemu side), but I can review this one and 5 for accuracy, assuming you make the agreed name changes.
/** + * virDomainSuspendForDuration: + * @dom: a domain object + * @target: an OR'ed set of virNodeSuspendTarget
This is _not_ an OR'd set, but a linear list. I'd say: @target: a value from virNodeSuspendTarget
+ * @duration: currently unused, pass 0
I'd still document this for its intended purpose: @duration: duration in seconds to suspend, or 0 for indefinite as well as a disclaimer in the rest of the text stating that some hypervisors require 0 for @duration.
+ * @flags: ditto
Rather than abbreviating with "ditto", I'd copy and paste one of the other lines where we say things are unused, pass 0.
+ * + * Attempt to suspend given domain. However, more + * states are supported than in virDomainSuspend.
That didn't read well. I'd say something like: Attempt to have the guest enter the given @target power management suspension level. If @duration is non-zero, also schedule the guest to resume normal operation after that many seconds, if nothing else has resumed it earlier. Some hypervisors require that @duration be 0, for an indefinite suspension.
+++ b/src/libvirt_public.syms @@ -520,6 +520,7 @@ LIBVIRT_0.9.10 { global: virDomainShutdownFlags; virStorageVolWipePattern; + virDomainSuspendForDuration; } LIBVIRT_0.9.9;
Looks like you get to rebase on top of other API added today. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/27/2012 09:12 PM, Eric Blake wrote:
On 01/26/2012 12:59 PM, Michal Privoznik wrote:
This API allows a domain to be put into one of S# ACPI states. Currently, S3 and S4 are supported. These states are shared with virNodeSuspendForDuration. However, for now we don't support any duration other than zero. The same apply for flags.
I'm not reviewing 1, 2, or 4(as Dan said, there's still some churn on the qemu side), but I can review this one and 5 for accuracy, assuming you make the agreed name changes.
/** + * virDomainSuspendForDuration: + * @dom: a domain object + * @target: an OR'ed set of virNodeSuspendTarget
This is _not_ an OR'd set, but a linear list. I'd say:
@target: a value from virNodeSuspendTarget ...
I see you pushed before reading my email, so I'm pushing my proposed cleanups under the trivial rule. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
This makes use of QEMU guest agent to implement the virDomainSuspendForDuration API. --- src/qemu/qemu_driver.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 93 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ab69dca..277b152 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1457,6 +1457,98 @@ cleanup: return ret; } +static int +qemuDomainSuspendForDuration(virDomainPtr dom, + unsigned int target, + unsigned long long duration, + unsigned int flags) +{ + struct qemud_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + qemuDomainObjPrivatePtr priv; + int agent_mode; + + virCheckFlags(0, -1); + + if (duration) { + qemuReportError(VIR_ERR_INVALID_ARG, "%s", + _("Suspending for duration is not supported.")); + return -1; + } + + switch (target) { + case VIR_NODE_SUSPEND_TARGET_MEM: + agent_mode = QEMU_AGENT_SUSPEND_SLEEP; + break; + case VIR_NODE_SUSPEND_TARGET_DISK: + agent_mode = QEMU_AGENT_SUSPEND_HIBERNATE; + break; + case VIR_NODE_SUSPEND_TARGET_HYBRID: + agent_mode = QEMU_AGENT_SUSPEND_HYBRID; + break; + default: + qemuReportError(VIR_ERR_INVALID_ARG, + _("Unsupported suspend target: %u"), + target); + return -1; + } + + qemuDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + qemuDriverUnlock(driver); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + } + + priv = vm->privateData; + + if (!priv->agent) { + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU guest agent is not configured")); + goto cleanup; + } + + if (priv->agentError) { + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QEMU guest agent is not " + "available due to an error")); + goto cleanup; + } + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_SUSPEND) < 0) + goto cleanup; + + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto endjob; + } + + qemuDomainObjEnterAgent(driver, vm); + ret = qemuAgentSuspend(priv->agent, agent_mode); + qemuDomainObjExitAgent(driver, vm); + +endjob: + if (qemuDomainObjEndJob(driver, vm) == 0) + vm = NULL; + +cleanup: + if (vm) + virDomainObjUnlock(vm); + return ret; +} static int qemudDomainResume(virDomainPtr dom) { struct qemud_driver *driver = dom->conn->privateData; @@ -11761,6 +11853,7 @@ static virDriver qemuDriver = { .domainLookupByName = qemudDomainLookupByName, /* 0.2.0 */ .domainSuspend = qemudDomainSuspend, /* 0.2.0 */ .domainResume = qemudDomainResume, /* 0.2.0 */ + .domainSuspendForDuration = qemuDomainSuspendForDuration, /* 0.9.10 */ .domainShutdown = qemuDomainShutdown, /* 0.2.0 */ .domainShutdownFlags = qemuDomainShutdownFlags, /* 0.9.10 */ .domainReboot = qemuDomainReboot, /* 0.9.3 */ -- 1.7.3.4
On 01/26/2012 12:59 PM, Michal Privoznik wrote:
This makes use of QEMU guest agent to implement the virDomainSuspendForDuration API. --- src/qemu/qemu_driver.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 93 insertions(+), 0 deletions(-)
+ + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + }
Same question as for quiesce: putting the guest into S3 will only work if the agent can respond, so checking merely for active is not enough. If the guest is active but paused, then we can't talk to the agent to issue the request. Having the common guest agent code check for this condition will prevent the scenario of: guest is paused issue the pm suspend, but it times out guest is resumed guest finally acts on command although it is always possible that a guest will suspend itself even without action on our part. At least this isn't as bad as a stale quiesce leaving the guest in a bad state, since on suspend, the host will receive a qemu event about the change in guest state (there's no event for freeze/thaw, since that is an aspect internal to the guest's use of disks and not something inherently visible to qemu to generate an event from). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 03/16/2012 07:11 AM, Eric Blake wrote:
On 01/26/2012 12:59 PM, Michal Privoznik wrote:
This makes use of QEMU guest agent to implement the virDomainSuspendForDuration API. --- src/qemu/qemu_driver.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 93 insertions(+), 0 deletions(-)
+ + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + }
Same question as for quiesce: putting the guest into S3 will only work if the agent can respond, so checking merely for active is not enough. If the guest is active but paused, then we can't talk to the agent to issue the request. Having the common guest agent code check for this condition will prevent the scenario of:
guest is paused issue the pm suspend, but it times out
Actually I encounted the problem when playing dompmsuspend, the dompmsuspend command hangs forever, so I tried to suspend the guest with "pm-suspend" inside guest directly, and then dompmwakeup returns quickly with success, however, the guest wasn't waken up actually. So there are at least two problems here: 1) dompmsuspend hangs 2) dompmwakeup returns success, while the guest wasn't waken up actally. For 1), I tried to create a patch with using guest agent command "guest-ping" to ping the guest agent first before executing the guest-suspend-* commands, and hope it could return quickly if the guest agent isn't available or something else which cause the guest agent doesn't response, and thus we could quit quickly, but no luck, the "guest-ping" hangs too. 1) could be the problem of guest-agent, while 2) might be problem of guest kenrel. It can't be problem of guest agent, as dompmwakeup actually uses the qemu monitor command, not guest agent command. I didn't dig into the problems further more though, it's possible caused by my guest agent is not setup correctly, or it's not that newer, and has bugs. But in any case, we should handle the hanging problem. For anyone who wanted to reproduce the problem, the guest is FC16.
guest is resumed guest finally acts on command
although it is always possible that a guest will suspend itself even without action on our part. At least this isn't as bad as a stale quiesce leaving the guest in a bad state, since on suspend, the host will receive a qemu event about the change in guest state (there's no event for freeze/thaw, since that is an aspect internal to the guest's use of disks and not something inherently visible to qemu to generate an event from).
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 16.03.2012 07:52, Osier Yang wrote:
On 03/16/2012 07:11 AM, Eric Blake wrote:
On 01/26/2012 12:59 PM, Michal Privoznik wrote:
This makes use of QEMU guest agent to implement the virDomainSuspendForDuration API. --- src/qemu/qemu_driver.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 93 insertions(+), 0 deletions(-)
+ + if (!virDomainObjIsActive(vm)) { + qemuReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + goto cleanup; + }
Same question as for quiesce: putting the guest into S3 will only work if the agent can respond, so checking merely for active is not enough. If the guest is active but paused, then we can't talk to the agent to issue the request. Having the common guest agent code check for this condition will prevent the scenario of:
guest is paused issue the pm suspend, but it times out
Actually I encounted the problem when playing dompmsuspend, the dompmsuspend command hangs forever, so I tried to suspend the guest with "pm-suspend" inside guest directly, and then dompmwakeup returns quickly with success, however, the guest wasn't waken up actually.
So there are at least two problems here: 1) dompmsuspend hangs 2) dompmwakeup returns success, while the guest wasn't waken up actally.
For 1), I tried to create a patch with using guest agent command "guest-ping" to ping the guest agent first before executing the guest-suspend-* commands, and hope it could return quickly if the guest agent isn't available or something else which cause the guest agent doesn't response, and thus we could quit quickly, but no luck, the "guest-ping" hangs too.
This was my approach as well: https://www.redhat.com/archives/libvir-list/2012-February/msg00055.html Although I've used waiting with timeout; therefore making any fail harmless. Because if we timeout on guest-sync which doesn't change the state of guest, we don't issue any subsequent state-changing command. I should reorder my TODO list and post the patch again. The sooner the better.
under new command "suspend-duration" --- tools/virsh.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 8 ++++++ 2 files changed, 75 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 74655c2..c00fe5d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2170,6 +2170,71 @@ cmdSuspend(vshControl *ctl, const vshCmd *cmd) } /* + * "suspend-duration" command + */ +static const vshCmdInfo info_suspend_duration[] = { + {"help", N_("suspend a domain for a given time duration")}, + {"desc", N_("Suspend a running domain for a given time duration.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_suspend_duration[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + /* {"duration", VSH_OT_INT, VSH_OFLAG_REQ, N_("duration in seconds")}, */ + {"target", VSH_OT_STRING, VSH_OFLAG_REQ, N_("mem(Suspend-to-RAM), " + "disk(Suspend-to-Disk), " + "hybrid(Hybrid-Suspend)")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdSuspendDuration(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + const char *name; + bool ret = false; + const char *target = NULL; + unsigned int suspendTarget; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) + return false; + + if (vshCommandOptString(cmd, "target", &target) < 0) { + vshError(ctl, _("Invalig target argument")); + goto cleanup; + } + + if (STREQ(target, "mem")) + suspendTarget = VIR_NODE_SUSPEND_TARGET_MEM; + else if (STREQ(target, "disk")) + suspendTarget = VIR_NODE_SUSPEND_TARGET_DISK; + else if (STREQ(target, "hybrid")) + suspendTarget = VIR_NODE_SUSPEND_TARGET_HYBRID; + else { + vshError(ctl, "%s", _("Invalid target")); + goto cleanup; + } + + if (virDomainSuspendForDuration(dom, suspendTarget, 0, 0) < 0) { + vshError(ctl, _("Domain %s could not be suspended"), + virDomainGetName(dom)); + goto cleanup; + } + + vshPrint(ctl, _("Domain %s successfully suspended"), + virDomainGetName(dom)); + + ret = true; + +cleanup: + virDomainFree(dom); + return ret; +} + +/* * "create" command */ static const vshCmdInfo info_create[] = { @@ -16070,6 +16135,8 @@ static const vshCmdDef domManagementCmds[] = { {"shutdown", cmdShutdown, opts_shutdown, info_shutdown, 0}, {"start", cmdStart, opts_start, info_start, 0}, {"suspend", cmdSuspend, opts_suspend, info_suspend, 0}, + {"suspend-duration", cmdSuspendDuration, + opts_suspend_duration, info_suspend_duration, 0}, {"ttyconsole", cmdTTYConsole, opts_ttyconsole, info_ttyconsole, 0}, {"undefine", cmdUndefine, opts_undefine, info_undefine, 0}, {"update-device", cmdUpdateDevice, opts_update_device, diff --git a/tools/virsh.pod b/tools/virsh.pod index 8599f66..dc6fd01 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1239,6 +1239,14 @@ Moves a domain out of the suspended state. This will allow a previously suspended domain to now be eligible for scheduling by the underlying hypervisor. +=item B<suspend-duration> I<domain-id> I<target> + +Suspend a running domain into one of these states (possible I<target> +values): + mem equivallent of S3 ACPI state + disk equivallent of S4 ACPI state + hybrid RAM is saved to disk but not powered off + =item B<ttyconsole> I<domain-id> Output the device used for the TTY console of the domain. If the information -- 1.7.3.4
On Thu, Jan 26, 2012 at 08:59:47PM +0100, Michal Privoznik wrote:
under new command "suspend-duration" --- tools/virsh.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 8 ++++++ 2 files changed, 75 insertions(+), 0 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 74655c2..c00fe5d 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2170,6 +2170,71 @@ cmdSuspend(vshControl *ctl, const vshCmd *cmd) }
/* + * "suspend-duration" command + */ +static const vshCmdInfo info_suspend_duration[] = { + {"help", N_("suspend a domain for a given time duration")}, + {"desc", N_("Suspend a running domain for a given time duration.")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_suspend_duration[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + /* {"duration", VSH_OT_INT, VSH_OFLAG_REQ, N_("duration in seconds")}, */ + {"target", VSH_OT_STRING, VSH_OFLAG_REQ, N_("mem(Suspend-to-RAM), " + "disk(Suspend-to-Disk), " + "hybrid(Hybrid-Suspend)")}, + {NULL, 0, 0, NULL} +}; + +static bool +cmdSuspendDuration(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom; + const char *name; + bool ret = false; + const char *target = NULL; + unsigned int suspendTarget; + + if (!vshConnectionUsability(ctl, ctl->conn)) + return false; + + if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) + return false; + + if (vshCommandOptString(cmd, "target", &target) < 0) { + vshError(ctl, _("Invalig target argument")); + goto cleanup; + } + + if (STREQ(target, "mem")) + suspendTarget = VIR_NODE_SUSPEND_TARGET_MEM; + else if (STREQ(target, "disk")) + suspendTarget = VIR_NODE_SUSPEND_TARGET_DISK; + else if (STREQ(target, "hybrid")) + suspendTarget = VIR_NODE_SUSPEND_TARGET_HYBRID; + else { + vshError(ctl, "%s", _("Invalid target")); + goto cleanup; + } + + if (virDomainSuspendForDuration(dom, suspendTarget, 0, 0) < 0) { + vshError(ctl, _("Domain %s could not be suspended"), + virDomainGetName(dom)); + goto cleanup; + } + + vshPrint(ctl, _("Domain %s successfully suspended"), + virDomainGetName(dom)); + + ret = true; + +cleanup: + virDomainFree(dom); + return ret; +} + +/* * "create" command */ static const vshCmdInfo info_create[] = { @@ -16070,6 +16135,8 @@ static const vshCmdDef domManagementCmds[] = { {"shutdown", cmdShutdown, opts_shutdown, info_shutdown, 0}, {"start", cmdStart, opts_start, info_start, 0}, {"suspend", cmdSuspend, opts_suspend, info_suspend, 0}, + {"suspend-duration", cmdSuspendDuration, + opts_suspend_duration, info_suspend_duration, 0}, {"ttyconsole", cmdTTYConsole, opts_ttyconsole, info_ttyconsole, 0}, {"undefine", cmdUndefine, opts_undefine, info_undefine, 0}, {"update-device", cmdUpdateDevice, opts_update_device, diff --git a/tools/virsh.pod b/tools/virsh.pod index 8599f66..dc6fd01 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -1239,6 +1239,14 @@ Moves a domain out of the suspended state. This will allow a previously suspended domain to now be eligible for scheduling by the underlying hypervisor.
+=item B<suspend-duration> I<domain-id> I<target> + +Suspend a running domain into one of these states (possible I<target> +values): + mem equivallent of S3 ACPI state + disk equivallent of S4 ACPI state + hybrid RAM is saved to disk but not powered off
As with the API, I think we need a different name prefix, like domnodesuspend or dompmsuspend ACK if we rename it in one of these ways or another suggestion 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 :|
On 01/26/2012 12:59 PM, Michal Privoznik wrote:
under new command "suspend-duration" --- tools/virsh.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh.pod | 8 ++++++ 2 files changed, 75 insertions(+), 0 deletions(-) +static const vshCmdOptDef opts_suspend_duration[] = { + {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")}, + /* {"duration", VSH_OT_INT, VSH_OFLAG_REQ, N_("duration in seconds")}, */
Good idea; no problem omitting this as long as there is no hypervisor that can support non-zero values.
+ + if (vshCommandOptString(cmd, "target", &target) < 0) { + vshError(ctl, _("Invalig target argument"));
s/Invalig/Invalid/
+ goto cleanup; + } + + if (STREQ(target, "mem")) + suspendTarget = VIR_NODE_SUSPEND_TARGET_MEM; + else if (STREQ(target, "disk")) + suspendTarget = VIR_NODE_SUSPEND_TARGET_DISK; + else if (STREQ(target, "hybrid")) + suspendTarget = VIR_NODE_SUSPEND_TARGET_HYBRID; + else {
Style. If the else branch has {}, then _every_ if branch also has to have {}.
+ vshError(ctl, "%s", _("Invalid target")); + goto cleanup; + } + + if (virDomainSuspendForDuration(dom, suspendTarget, 0, 0) < 0) { + vshError(ctl, _("Domain %s could not be suspended"), + virDomainGetName(dom)); + goto cleanup; + } + + vshPrint(ctl, _("Domain %s successfully suspended"),
Is this wording accurate? We might do better stating: Domain %s suspension request successful since not all guests will properly act on suspension requests, and since suspension is somewhat of an asynchronous command (you issue the call, but you have to probe to see if it was acted on).
@@ -16070,6 +16135,8 @@ static const vshCmdDef domManagementCmds[] = { {"shutdown", cmdShutdown, opts_shutdown, info_shutdown, 0}, {"start", cmdStart, opts_start, info_start, 0}, {"suspend", cmdSuspend, opts_suspend, info_suspend, 0}, + {"suspend-duration", cmdSuspendDuration, + opts_suspend_duration, info_suspend_duration, 0},
Sort this into wherever the renamed command fits.
+=item B<suspend-duration> I<domain-id> I<target> + +Suspend a running domain into one of these states (possible I<target> +values): + mem equivallent of S3 ACPI state
s/equivallent/equivalent/, but see below
+ disk equivallent of S4 ACPI state + hybrid RAM is saved to disk but not powered off
I'd borrow wording from B<nodesuspend>, but then improve things to mention the actual target names (and while you are at it, fix nodesuspend to mention target names). Something like: Puts the domain into a sleep state according to the contents of I<target>, recognizing values of B<mem> for Suspend-to-RAM (S3), B<disk> for Suspend-to-Disk (S4), or B<hybrid> (save state to disk, but then suspend to ram). -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Michal Privoznik -
Osier Yang