On 11/05/14 13:35, Michal Privoznik wrote:
> This is done by introducing '--sync-time' to already existing
> 'resume' command. If the option is used, the newer Flags() API
> is called.
>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> tools/virsh-domain.c | 13 ++++++++++++-
> tools/virsh.pod | 10 ++++++----
> 2 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index dfc3a8c..c02f0a2 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -5144,11 +5148,18 @@ cmdResume(vshControl *ctl, const vshCmd *cmd)
> virDomainPtr dom;
> bool ret = true;
> const char *name;
> + unsigned int flags = 0;
>
> if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
> return false;
>
> - if (virDomainResume(dom) == 0) {
> + if (vshCommandOptBool(cmd, "sync-time"))
> + flags |= VIR_DOMAIN_RESUME_SYNC_TIME;
> +
> + if ((flags &&
> + virDomainResumeFlags(dom, flags) == 0) ||
> + (!flags &&
> + virDomainResume(dom) == 0)) {
> vshPrint(ctl, _("Domain %s resumed\n"), name);
> } else {
> vshError(ctl, _("Failed to resume domain %s"), name);
Here you exactly hit the problem I've described in 4/5:
If time sync fails then virsh will output "Failed to resume domain
blah", but the domain was actually resumed.
You either need to check the domain state or switch to the two-call
approach even in virsh so that you can report a proper error.
In case when switching to the two call approach is better for most
clients, having the flag doesn't make much sense though.
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index daddb48..001f61a 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -2101,11 +2101,13 @@ is only supported with container based virtualization.
> Suspend a running domain. It is kept in memory but won't be scheduled
> anymore.
>
> -=item B<resume> I<domain>
> +=item B<resume> I<domain> [I<--sync-time>]
>
> -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.
> +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. Moreover, if I<--sync-time> is specified the
> +domain's time is synced right after the domain CPUs are started. Note,
"right away" may suggest that nothing happening after the resume will
use the old time stamps.
> +that this may require guest agent on some hypervisors.
>
> =item B<dompmsuspend> I<domain> I<target> [I<--duration>]
>
>
My stance on this patch depends on what happens in 4/5
Peter
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list