[v5] migration: propagate vTPM errors using Error objects

[PATCH v5 15/23] migration: push Error **errp into loadvm_handle_recv_bitmap()

Posted by Arun Menon 4 months 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 loadvm_handle_recv_bitmap() must report an error
in errp, in case of failure.

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

diff --git a/migration/savevm.c b/migration/savevm.c
index 20dce4b43a181d2f47b6b22e7a15aa4fc45cac5c..1cbc44a5314043a403d983511066cf137681a18d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2471,32 +2471,35 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error **errp)
  * len (1 byte) + ramblock_name (<255 bytes)
  */
 static int loadvm_handle_recv_bitmap(MigrationIncomingState *mis,
-                                     uint16_t len)
+                                     uint16_t len, Error **errp)
 {
     QEMUFile *file = mis->from_src_file;
     RAMBlock *rb;
     char block_name[256];
     size_t cnt;
+    int ret;
 
     cnt = qemu_get_counted_string(file, block_name);
     if (!cnt) {
-        error_report("%s: failed to read block name", __func__);
+        error_setg(errp, "%s: failed to read block name", __func__);
         return -EINVAL;
     }
 
     /* Validate before using the data */
-    if (qemu_file_get_error(file)) {
-        return qemu_file_get_error(file);
+    ret = qemu_file_get_error(file);
+    if (ret < 0) {
+        error_setg(errp, "migration stream has error: %d", ret);
+        return ret;
     }
 
     if (len != cnt + 1) {
-        error_report("%s: invalid payload length (%d)", __func__, len);
+        error_setg(errp, "%s: invalid payload length (%d)", __func__, len);
         return -EINVAL;
     }
 
     rb = qemu_ram_block_by_name(block_name);
     if (!rb) {
-        error_report("%s: block '%s' not found", __func__, block_name);
+        error_setg(errp, "%s: block '%s' not found", __func__, block_name);
         return -EINVAL;
     }
 
@@ -2635,12 +2638,7 @@ static int loadvm_process_command(QEMUFile *f, Error **errp)
         return 0;
 
     case MIG_CMD_RECV_BITMAP:
-        ret = loadvm_handle_recv_bitmap(mis, len);
-        if (ret < 0) {
-            error_setg(errp, "Failed to load device state command: %d", ret);
-            return -1;
-        }
-        return ret;
+        return loadvm_handle_recv_bitmap(mis, len, errp);
 
     case MIG_CMD_ENABLE_COLO:
         ret = loadvm_process_enable_colo(mis);

-- 
2.50.0

Re: [PATCH v5 15/23] migration: push Error **errp into loadvm_handle_recv_bitmap()

Posted by Daniel P. Berrangé 4 months ago

On Thu, Jul 17, 2025 at 06:07:38AM +0530, Arun Menon 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 loadvm_handle_recv_bitmap() must report an error
> in errp, in case of failure.
> 
> Signed-off-by: Arun Menon <armenon@redhat.com>
> ---
>  migration/savevm.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 20dce4b43a181d2f47b6b22e7a15aa4fc45cac5c..1cbc44a5314043a403d983511066cf137681a18d 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2471,32 +2471,35 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis, Error **errp)
>   * len (1 byte) + ramblock_name (<255 bytes)
>   */
>  static int loadvm_handle_recv_bitmap(MigrationIncomingState *mis,
> -                                     uint16_t len)
> +                                     uint16_t len, Error **errp)
>  {
>      QEMUFile *file = mis->from_src_file;
>      RAMBlock *rb;
>      char block_name[256];
>      size_t cnt;
> +    int ret;
>  
>      cnt = qemu_get_counted_string(file, block_name);
>      if (!cnt) {
> -        error_report("%s: failed to read block name", __func__);
> +        error_setg(errp, "%s: failed to read block name", __func__);
>          return -EINVAL;
>      }
>  
>      /* Validate before using the data */
> -    if (qemu_file_get_error(file)) {
> -        return qemu_file_get_error(file);
> +    ret = qemu_file_get_error(file);
> +    if (ret < 0) {
> +        error_setg(errp, "migration stream has error: %d", ret);
> +        return ret;
>      }
>  
>      if (len != cnt + 1) {
> -        error_report("%s: invalid payload length (%d)", __func__, len);
> +        error_setg(errp, "%s: invalid payload length (%d)", __func__, len);
>          return -EINVAL;
>      }
>  
>      rb = qemu_ram_block_by_name(block_name);
>      if (!rb) {
> -        error_report("%s: block '%s' not found", __func__, block_name);
> +        error_setg(errp, "%s: block '%s' not found", __func__, block_name);
>          return -EINVAL;
>      }

(Pre-existing) redundant use of __func__ that can be removed.

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With 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 :|