From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-stable@nongnu.org, yama@redhat.com, dgilbert@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: Tue, 11 May 2021 17:49:02 +0100 [thread overview]
Message-ID: <YJq1fq57yQGFjw/E@redhat.com> (raw)
In-Reply-To: <20210511163151.45167-1-kwolf@redhat.com>
On Tue, May 11, 2021 at 06:31:51PM +0200, Kevin Wolf 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.
>
> 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();
> }
> hmp_handle_error(mon, err);
Paolo had sent a different fix here:
https://lists.gnu.org/archive/html/qemu-devel/2021-05/msg01093.html
As with my feedback there, I think we should be adding test coverage
when fixing this. How about this:
diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
index 54e49c8ffa..137c5d0577 100755
--- a/tests/qemu-iotests/068
+++ b/tests/qemu-iotests/068
@@ -77,7 +77,7 @@ for extra_args in \
# Give qemu some time to boot before saving the VM state
{ sleep 1; printf "savevm 0\nquit\n"; } | _qemu $extra_args
# Now try to continue from that VM state (this should just work)
- { sleep 1; printf "loadvm 0\nloadvm 0\nquit\n"; } | _qemu $extra_args -S
+ { sleep 1; printf "info status\nloadvm 0\ninfo status\ncont\ninfo status\nloadvm 0\ninfo status\nquit\n"; } | _qemu $extra_args -S
done
# success, all done
diff --git a/tests/qemu-iotests/068.out b/tests/qemu-iotests/068.out
index f07a938a38..75c0a5df5f 100644
--- a/tests/qemu-iotests/068.out
+++ b/tests/qemu-iotests/068.out
@@ -7,8 +7,17 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) savevm 0
(qemu) quit
QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) info status
+VM status: paused (prelaunch)
(qemu) loadvm 0
+(qemu) info status
+VM status: paused (prelaunch)
+(qemu) cont
+(qemu) info status
+VM status: running
(qemu) loadvm 0
+(qemu) info status
+VM status: running
(qemu) quit
=== Saving and reloading a VM state to/from a qcow2 image (-object iothread,id=iothread0 -set device.hba0.iothread=iothread0) ===
@@ -18,7 +27,16 @@ QEMU X.Y.Z monitor - type 'help' for more information
(qemu) savevm 0
(qemu) quit
QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) info status
+VM status: paused (prelaunch)
(qemu) loadvm 0
+(qemu) info status
+VM status: paused (prelaunch)
+(qemu) cont
+(qemu) info status
+VM status: running
(qemu) loadvm 0
+(qemu) info status
+VM status: running
(qemu) quit
*** done
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
next prev parent reply other threads:[~2021-05-11 17:00 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é [this message]
2021-05-11 16:58 ` Kevin Wolf
2021-05-11 17:05 ` Daniel P. Berrangé
2021-05-12 19:16 ` Dr. David Alan Gilbert
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=YJq1fq57yQGFjw/E@redhat.com \
--to=berrange@redhat.com \
--cc=dgilbert@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.