[PATCH v9 10/27] migration: push Error **errp into qemu_loadvm_section_start_full()

Arun Menon posted 27 patches 4 months, 1 week ago
Maintainers: Stefan Berger <stefanb@linux.vnet.ibm.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, "Michael S. Tsirkin" <mst@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Cornelia Huck <cohuck@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Richard Henderson <richard.henderson@linaro.org>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, Nicholas Piggin <npiggin@gmail.com>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Paolo Bonzini <pbonzini@redhat.com>, Fam Zheng <fam@euphon.net>, Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>, Hailiang Zhang <zhanghailiang@xfusion.com>, Steve Sistare <steven.sistare@oracle.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>
There is a newer version of this series
[PATCH v9 10/27] migration: push Error **errp into qemu_loadvm_section_start_full()
Posted by Arun Menon 4 months, 1 week ago
This is an incremental step in converting vmstate loading
code to report error via Error objects instead of directly
printing it to console/monitor.
It is ensured that qemu_loadvm_section_start_full() must report an error
in errp, in case of failure.

Signed-off-by: Arun Menon <armenon@redhat.com>
---
 migration/savevm.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index ad3dd9b172afc541f104d2187a79bafa8e380466..e337e3c61e7627f09b853bf5e9b845c38cb5f082 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2722,8 +2722,9 @@ static bool check_section_footer(QEMUFile *f, SaveStateEntry *se)
 }
 
 static int
-qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
+qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type, Error **errp)
 {
+    ERRP_GUARD();
     bool trace_downtime = (type == QEMU_VM_SECTION_FULL);
     uint32_t instance_id, version_id, section_id;
     int64_t start_ts, end_ts;
@@ -2734,8 +2735,8 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
     /* 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);
+        error_setg(errp, "Unable to read ID string for section %u",
+                   section_id);
         return -EINVAL;
     }
     instance_id = qemu_get_be32(f);
@@ -2743,8 +2744,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
 
     ret = qemu_file_get_error(f);
     if (ret) {
-        error_report("%s: Failed to read instance/version ID: %d",
-                     __func__, ret);
+        error_setg(errp, "Failed to read instance/version ID: %d", ret);
         return ret;
     }
 
@@ -2753,17 +2753,17 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
     /* 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);
+        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 -EINVAL;
     }
 
     /* Validate version */
     if (version_id > se->version_id) {
-        error_report("savevm: unsupported version %d for '%s' v%d",
-                     version_id, idstr, se->version_id);
+        error_setg(errp, "savevm: unsupported version %d for '%s' v%d",
+                   version_id, idstr, se->version_id);
         return -EINVAL;
     }
     se->load_version_id = version_id;
@@ -2771,7 +2771,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
 
     /* 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);
+        error_setg(errp, "loadvm: %s RAM loading not allowed on Xen", idstr);
         return -EINVAL;
     }
 
@@ -2779,10 +2779,11 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
         start_ts = qemu_clock_get_us(QEMU_CLOCK_REALTIME);
     }
 
-    ret = vmstate_load(f, se, NULL);
+    ret = vmstate_load(f, se, errp);
     if (ret < 0) {
-        error_report("error while loading state for instance 0x%"PRIx32" of"
-                     " device '%s'", instance_id, idstr);
+        error_prepend(errp,
+                      "error while loading state for instance 0x%"PRIx32" of"
+                      " device '%s': ", instance_id, idstr);
         return ret;
     }
 
@@ -2793,6 +2794,9 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
     }
 
     if (!check_section_footer(f, se)) {
+        error_setg(errp, "Reading footer section of instance "
+                   "0x%"PRIx32" of device '%s' for version_id: %d failed",
+                   instance_id, idstr, version_id);
         return -EINVAL;
     }
 
@@ -3097,7 +3101,7 @@ retry:
         switch (section_type) {
         case QEMU_VM_SECTION_START:
         case QEMU_VM_SECTION_FULL:
-            ret = qemu_loadvm_section_start_full(f, section_type);
+            ret = qemu_loadvm_section_start_full(f, section_type, errp);
             if (ret < 0) {
                 goto out;
             }

-- 
2.50.1
Re: [PATCH v9 10/27] migration: push Error **errp into qemu_loadvm_section_start_full()
Posted by Marc-André Lureau 4 months, 1 week ago
On Tue, Aug 5, 2025 at 11:03 PM Arun Menon <armenon@redhat.com> wrote:
>
> This is an incremental step in converting vmstate loading
> code to report error via Error objects instead of directly
> printing it to console/monitor.
> It is ensured that qemu_loadvm_section_start_full() must report an error
> in errp, in case of failure.
>
> Signed-off-by: Arun Menon <armenon@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  migration/savevm.c | 36 ++++++++++++++++++++----------------
>  1 file changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index ad3dd9b172afc541f104d2187a79bafa8e380466..e337e3c61e7627f09b853bf5e9b845c38cb5f082 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2722,8 +2722,9 @@ static bool check_section_footer(QEMUFile *f, SaveStateEntry *se)
>  }
>
>  static int
> -qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
> +qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type, Error **errp)
>  {
> +    ERRP_GUARD();
>      bool trace_downtime = (type == QEMU_VM_SECTION_FULL);
>      uint32_t instance_id, version_id, section_id;
>      int64_t start_ts, end_ts;
> @@ -2734,8 +2735,8 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
>      /* 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);
> +        error_setg(errp, "Unable to read ID string for section %u",
> +                   section_id);
>          return -EINVAL;
>      }
>      instance_id = qemu_get_be32(f);
> @@ -2743,8 +2744,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
>
>      ret = qemu_file_get_error(f);
>      if (ret) {
> -        error_report("%s: Failed to read instance/version ID: %d",
> -                     __func__, ret);
> +        error_setg(errp, "Failed to read instance/version ID: %d", ret);
>          return ret;
>      }
>
> @@ -2753,17 +2753,17 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
>      /* 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);
> +        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 -EINVAL;
>      }
>
>      /* Validate version */
>      if (version_id > se->version_id) {
> -        error_report("savevm: unsupported version %d for '%s' v%d",
> -                     version_id, idstr, se->version_id);
> +        error_setg(errp, "savevm: unsupported version %d for '%s' v%d",
> +                   version_id, idstr, se->version_id);
>          return -EINVAL;
>      }
>      se->load_version_id = version_id;
> @@ -2771,7 +2771,7 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
>
>      /* 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);
> +        error_setg(errp, "loadvm: %s RAM loading not allowed on Xen", idstr);
>          return -EINVAL;
>      }
>
> @@ -2779,10 +2779,11 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
>          start_ts = qemu_clock_get_us(QEMU_CLOCK_REALTIME);
>      }
>
> -    ret = vmstate_load(f, se, NULL);
> +    ret = vmstate_load(f, se, errp);
>      if (ret < 0) {
> -        error_report("error while loading state for instance 0x%"PRIx32" of"
> -                     " device '%s'", instance_id, idstr);
> +        error_prepend(errp,
> +                      "error while loading state for instance 0x%"PRIx32" of"
> +                      " device '%s': ", instance_id, idstr);
>          return ret;
>      }
>
> @@ -2793,6 +2794,9 @@ qemu_loadvm_section_start_full(QEMUFile *f, uint8_t type)
>      }
>
>      if (!check_section_footer(f, se)) {
> +        error_setg(errp, "Reading footer section of instance "
> +                   "0x%"PRIx32" of device '%s' for version_id: %d failed",
> +                   instance_id, idstr, version_id);
>          return -EINVAL;
>      }
>
> @@ -3097,7 +3101,7 @@ retry:
>          switch (section_type) {
>          case QEMU_VM_SECTION_START:
>          case QEMU_VM_SECTION_FULL:
> -            ret = qemu_loadvm_section_start_full(f, section_type);
> +            ret = qemu_loadvm_section_start_full(f, section_type, errp);
>              if (ret < 0) {
>                  goto out;
>              }
>
> --
> 2.50.1
>
>


-- 
Marc-André Lureau