diff mbox series

[03/33] migration: push Error **errp into qemu_loadvm_state_setup()

Message ID 20210204171907.901471-4-berrange@redhat.com (mailing list archive)
State New, archived
Headers show
Series migration: capture error reports into Error object | expand

Commit Message

Daniel P. Berrangé Feb. 4, 2021, 5:18 p.m. UTC
This is an incremental step in converting vmstate loading code to report
via Error objects instead of printing directly to the console/monitor.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 migration/savevm.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Philippe Mathieu-Daudé Feb. 4, 2021, 9:59 p.m. UTC | #1
On 2/4/21 6:18 PM, Daniel P. Berrangé wrote:
> This is an incremental step in converting vmstate loading code to report
> via Error objects instead of printing directly to the console/monitor.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  migration/savevm.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Markus Armbruster Feb. 5, 2021, 7:50 a.m. UTC | #2
Daniel P. Berrangé <berrange@redhat.com> writes:

> This is an incremental step in converting vmstate loading code to report
> via Error objects instead of printing directly to the console/monitor.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  migration/savevm.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 870199b629..f4ed14a230 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2490,7 +2490,7 @@ static int qemu_loadvm_state_header(QEMUFile *f, Error **errp)
>      return 0;
>  }
>  
> -static int qemu_loadvm_state_setup(QEMUFile *f)
> +static int qemu_loadvm_state_setup(QEMUFile *f, Error **errp)
>  {
>      SaveStateEntry *se;
>      int ret;
> @@ -2509,7 +2509,7 @@ static int qemu_loadvm_state_setup(QEMUFile *f)
>          ret = se->ops->load_setup(f, se->opaque);
>          if (ret < 0) {
>              qemu_file_set_error(f, ret);
> -            error_report("Load state of device %s failed", se->idstr);
> +            error_setg(errp, "Load state of device %s failed", se->idstr);
>              return ret;
>          }
>      }
> @@ -2656,8 +2656,7 @@ int qemu_loadvm_state(QEMUFile *f, Error **errp)
>          return -1;
>      }
>  
> -    if (qemu_loadvm_state_setup(f) != 0) {
> -        error_setg(errp, "Error %d while loading VM state", -EINVAL);
> +    if (qemu_loadvm_state_setup(f, errp) < 0) {
>          return -1;
>      }

Drive-by remark, *not* a demand: I don't like "0 on success, -1 on
failure".  When we return just one value on success and one on failure,
I prefer true and false.  Negative value on failure is of course fine
for returning error codes, and were we want to return arbitrary
non-negative values on success.
diff mbox series

Patch

diff --git a/migration/savevm.c b/migration/savevm.c
index 870199b629..f4ed14a230 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2490,7 +2490,7 @@  static int qemu_loadvm_state_header(QEMUFile *f, Error **errp)
     return 0;
 }
 
-static int qemu_loadvm_state_setup(QEMUFile *f)
+static int qemu_loadvm_state_setup(QEMUFile *f, Error **errp)
 {
     SaveStateEntry *se;
     int ret;
@@ -2509,7 +2509,7 @@  static int qemu_loadvm_state_setup(QEMUFile *f)
         ret = se->ops->load_setup(f, se->opaque);
         if (ret < 0) {
             qemu_file_set_error(f, ret);
-            error_report("Load state of device %s failed", se->idstr);
+            error_setg(errp, "Load state of device %s failed", se->idstr);
             return ret;
         }
     }
@@ -2656,8 +2656,7 @@  int qemu_loadvm_state(QEMUFile *f, Error **errp)
         return -1;
     }
 
-    if (qemu_loadvm_state_setup(f) != 0) {
-        error_setg(errp, "Error %d while loading VM state", -EINVAL);
+    if (qemu_loadvm_state_setup(f, errp) < 0) {
         return -1;
     }