All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-stable@nongnu.org, berrange@redhat.com, yama@redhat.com,
	qemu-block@nongnu.org, qemu-devel@nongnu.org
Subject: Re: [PATCH] hmp: Fix loadvm to resume the VM on success instead of failure
Date: Wed, 12 May 2021 20:16:47 +0100	[thread overview]
Message-ID: <YJwpn+WbXx6A+Dad@work-vm> (raw)
In-Reply-To: <20210511163151.45167-1-kwolf@redhat.com>

* Kevin Wolf (kwolf@redhat.com) wrote:
> Commit f61fe11aa6f broke hmp_loadvm() by adding an incorrect negation
> when converting from 0/-errno return values to a bool value. The result
> is that loadvm resumes the VM now if it failed and keeps it stopped if
> it failed. Fix it to restore the old behaviour and do it the other way
> around.

I think your wording is wrong there - you have two 'if it failed'

> Fixes: f61fe11aa6f7f8f0ffe4ddaa56a8108f3ab57854
> Cc: qemu-stable@nongnu.org
> Reported-by: Yanhui Ma <yama@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  monitor/hmp-cmds.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 0ad5b77477..cc15d9b6ee 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1133,7 +1133,7 @@ void hmp_loadvm(Monitor *mon, const QDict *qdict)
>  
>      vm_stop(RUN_STATE_RESTORE_VM);
>  
> -    if (!load_snapshot(name, NULL, false, NULL, &err) && saved_vm_running) {
> +    if (load_snapshot(name, NULL, false, NULL, &err) && saved_vm_running) {
>          vm_start();

So if I'm reading htat right, that gets you it restarting it if
load_snapshot returns true, and it returns true if the load_snapshot
worked; i.e. if we were running and we succesfully load a snasphot
then we carry on running.

Other than the commit message, it makes sense tome:


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

>      }

>      hmp_handle_error(mon, err);
> -- 
> 2.30.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



  parent reply	other threads:[~2021-05-12 19:52 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-11 16:31 [PATCH] hmp: Fix loadvm to resume the VM on success instead of failure Kevin Wolf
2021-05-11 16:49 ` Daniel P. Berrangé
2021-05-11 16:58   ` Kevin Wolf
2021-05-11 17:05     ` Daniel P. Berrangé
2021-05-12 19:16 ` Dr. David Alan Gilbert [this message]
2021-05-25 17:43 ` Dr. David Alan Gilbert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YJwpn+WbXx6A+Dad@work-vm \
    --to=dgilbert@redhat.com \
    --cc=berrange@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=yama@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.