[PATCH] hmp: Fix loadvm to resume the VM on success instead of failure

Kevin Wolf posted 1 patch 2 years, 12 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210511163151.45167-1-kwolf@redhat.com
Maintainers: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
monitor/hmp-cmds.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] hmp: Fix loadvm to resume the VM on success instead of failure
Posted by Kevin Wolf 2 years, 12 months ago
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);
-- 
2.30.2


Re: [PATCH] hmp: Fix loadvm to resume the VM on success instead of failure
Posted by Dr. David Alan Gilbert 2 years, 11 months ago
* 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


Re: [PATCH] hmp: Fix loadvm to resume the VM on success instead of failure
Posted by Dr. David Alan Gilbert 2 years, 11 months ago
* 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.
> 
> Fixes: f61fe11aa6f7f8f0ffe4ddaa56a8108f3ab57854
> Cc: qemu-stable@nongnu.org
> Reported-by: Yanhui Ma <yama@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

queued

> ---
>  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);
> -- 
> 2.30.2
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK