[libvirt] [PATCH v2 5/5] virsh: Expose new virDomainSuspendForDuration API

[Eric Blake]

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 at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120127/ff084b6d/attachment-0001.sig>