diff mbox series

[06/33] migration: push Error **errp into qemu_loadvm_section_start_full()

Message ID 20210204171907.901471-7-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.

This is particularly useful for loading snapshots as this is a likely
error scenario to hit when the source and dest VM configs do not
match. This is illustrated by the improved error reporting in the
QMP load snapshot test.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 migration/savevm.c                            | 49 +++++++++----------
 .../tests/internal-snapshots-qapi.out         |  3 +-
 2 files changed, 25 insertions(+), 27 deletions(-)

Comments

Philippe Mathieu-Daudé Feb. 4, 2021, 10:04 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.
> 
> This is particularly useful for loading snapshots as this is a likely
> error scenario to hit when the source and dest VM configs do not
> match. This is illustrated by the improved error reporting in the
> QMP load snapshot test.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  migration/savevm.c                            | 49 +++++++++----------
>  .../tests/internal-snapshots-qapi.out         |  3 +-
>  2 files changed, 25 insertions(+), 27 deletions(-)
...

>      instance_id = qemu_get_be32(f);
>      version_id = qemu_get_be32(f);
>  
>      ret = qemu_file_get_error(f);
>      if (ret) {
> -        error_report("%s: Failed to read instance/version ID: %d",
> -                     __func__, ret);
> -        return ret;
> +        error_setg(errp, "Failed to read instance/version ID: %d",
> +                   ret);

error_setg_errno()

> +        return -1;
>      }
...

> @@ -2601,11 +2603,8 @@ retry:
>          switch (section_type) {
>          case QEMU_VM_SECTION_START:
>          case QEMU_VM_SECTION_FULL:
> -            ret = qemu_loadvm_section_start_full(f, mis);
> +            ret = qemu_loadvm_section_start_full(f, mis, errp);
>              if (ret < 0) {
> -                error_setg(errp,
> -                           "Failed to load device state section start: %d",
> -                           ret);

Ditto.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

>                  goto out;
>              }
>              break;
diff mbox series

Patch

diff --git a/migration/savevm.c b/migration/savevm.c
index e47aec435c..f2eee0a4a7 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2350,7 +2350,8 @@  static bool check_section_footer(QEMUFile *f, SaveStateEntry *se)
 }
 
 static int
-qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
+qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis,
+                               Error **errp)
 {
     uint32_t instance_id, version_id, section_id;
     SaveStateEntry *se;
@@ -2360,18 +2361,18 @@  qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
     /* Read section start */
     section_id = qemu_get_be32(f);
     if (!qemu_get_counted_string(f, idstr)) {
-        error_report("Unable to read ID string for section %u",
-                     section_id);
-        return -EINVAL;
+        error_setg(errp, "Unable to read ID string for section %u",
+                   section_id);
+        return -1;
     }
     instance_id = qemu_get_be32(f);
     version_id = qemu_get_be32(f);
 
     ret = qemu_file_get_error(f);
     if (ret) {
-        error_report("%s: Failed to read instance/version ID: %d",
-                     __func__, ret);
-        return ret;
+        error_setg(errp, "Failed to read instance/version ID: %d",
+                   ret);
+        return -1;
     }
 
     trace_qemu_loadvm_state_section_startfull(section_id, idstr,
@@ -2379,36 +2380,37 @@  qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
     /* Find savevm section */
     se = find_se(idstr, instance_id);
     if (se == NULL) {
-        error_report("Unknown savevm section or instance '%s' %"PRIu32". "
-                     "Make sure that your current VM setup matches your "
-                     "saved VM setup, including any hotplugged devices",
-                     idstr, instance_id);
-        return -EINVAL;
+        error_setg(errp, "Unknown savevm section or instance '%s' %"PRIu32". "
+                   "Make sure that your current VM setup matches your "
+                   "saved VM setup, including any hotplugged devices",
+                   idstr, instance_id);
+        return -1;
     }
 
     /* Validate version */
     if (version_id > se->version_id) {
-        error_report("savevm: unsupported version %d for '%s' v%d",
-                     version_id, idstr, se->version_id);
-        return -EINVAL;
+        error_setg(errp, "savevm: unsupported version %d for '%s' v%d",
+                   version_id, idstr, se->version_id);
+        return -1;
     }
     se->load_version_id = version_id;
     se->load_section_id = section_id;
 
     /* Validate if it is a device's state */
     if (xen_enabled() && se->is_ram) {
-        error_report("loadvm: %s RAM loading not allowed on Xen", idstr);
-        return -EINVAL;
+        error_setg(errp, "loadvm: %s RAM loading not allowed on Xen", idstr);
+        return -1;
     }
 
     ret = vmstate_load(f, se);
     if (ret < 0) {
-        error_report("error while loading state for instance 0x%"PRIx32" of"
-                     " device '%s'", instance_id, idstr);
-        return ret;
+        error_setg(errp, "error while loading state for instance 0x%"PRIx32" of"
+                   " device '%s'", instance_id, idstr);
+        return -1;
     }
     if (!check_section_footer(f, se)) {
-        return -EINVAL;
+        error_setg(errp, "failed check for device state section footer");
+        return -1;
     }
 
     return 0;
@@ -2601,11 +2603,8 @@  retry:
         switch (section_type) {
         case QEMU_VM_SECTION_START:
         case QEMU_VM_SECTION_FULL:
-            ret = qemu_loadvm_section_start_full(f, mis);
+            ret = qemu_loadvm_section_start_full(f, mis, errp);
             if (ret < 0) {
-                error_setg(errp,
-                           "Failed to load device state section start: %d",
-                           ret);
                 goto out;
             }
             break;
diff --git a/tests/qemu-iotests/tests/internal-snapshots-qapi.out b/tests/qemu-iotests/tests/internal-snapshots-qapi.out
index 26ff4a838c..fd3e2a9ed0 100644
--- a/tests/qemu-iotests/tests/internal-snapshots-qapi.out
+++ b/tests/qemu-iotests/tests/internal-snapshots-qapi.out
@@ -345,13 +345,12 @@  Formatting 'TEST_DIR/t.qcow2.alt2', fmt=IMGFMT size=134217728
                                      "devices": ["diskfmt0"]}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "load-err-stderr"}}
-qemu-system-x86_64: Unknown savevm section or instance '0000:00:02.0/virtio-rng' 0. Make sure that your current VM setup matches your saved VM setup, including any hotplugged devices
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "load-err-stderr"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "STOP"}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "aborting", "id": "load-err-stderr"}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "load-err-stderr"}}
 {"execute": "query-jobs"}
-{"return": [{"current-progress": 1, "status": "concluded", "total-progress": 1, "type": "snapshot-load", "id": "load-err-stderr", "error": "Error -22 while loading VM state"}]}
+{"return": [{"current-progress": 1, "status": "concluded", "total-progress": 1, "type": "snapshot-load", "id": "load-err-stderr", "error": "Unknown savevm section or instance '0000:00:02.0/virtio-rng' 0. Make sure that your current VM setup matches your saved VM setup, including any hotplugged devices"}]}
 {"execute": "job-dismiss", "arguments": {"id": "load-err-stderr"}}
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "load-err-stderr"}}