Patchwork [v2,12/17] savevm: add error parameter to qemu_loadvm_state()

login
register
mail settings
Submitter Pavel Hrdina
Date Dec. 13, 2012, 3:40 p.m.
Message ID <c87d35594310986dd711976fddfd538f15c3546e.1355404685.git.phrdina@redhat.com>
Download mbox | patch
Permalink /patch/206147/
State New
Headers show

Comments

Pavel Hrdina - Dec. 13, 2012, 3:40 p.m.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 migration.c |  8 ++++----
 savevm.c    | 39 +++++++++++++++++++++++----------------
 sysemu.h    |  3 ++-
 3 files changed, 29 insertions(+), 21 deletions(-)
Luiz Capitulino - Dec. 14, 2012, 4:52 p.m.
On Thu, 13 Dec 2012 16:40:46 +0100
Pavel Hrdina <phrdina@redhat.com> wrote:

> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  migration.c |  8 ++++----
>  savevm.c    | 39 +++++++++++++++++++++++----------------
>  sysemu.h    |  3 ++-
>  3 files changed, 29 insertions(+), 21 deletions(-)
> 
> diff --git a/migration.c b/migration.c
> index 3ae1db0..4c2d2d3 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -86,13 +86,13 @@ void qemu_start_incoming_migration(const char *uri, Error **errp)
>  static void process_incoming_migration_co(void *opaque)
>  {
>      QEMUFile *f = opaque;
> -    int ret;
> +    Error **errp = NULL;
>  
> -    ret = qemu_loadvm_state(f);
> +    qemu_loadvm_state(f, errp);
>      qemu_set_fd_handler(qemu_get_fd(f), NULL, NULL, NULL);
>      qemu_fclose(f);
> -    if (ret < 0) {
> -        fprintf(stderr, "load of migration failed\n");
> +    if (error_is_set(errp)) {
> +        handle_error(errp);

You can just print to stderr here, as qemu is going to exit.

>          exit(0);
>      }
>      qemu_announce_self();
> diff --git a/savevm.c b/savevm.c
> index 71c7df8..2c63aad 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -1962,7 +1962,8 @@ typedef struct LoadStateEntry {
>      int version_id;
>  } LoadStateEntry;
>  
> -int qemu_loadvm_state(QEMUFile *f)
> +int qemu_loadvm_state(QEMUFile *f,
> +                      Error **errp)
>  {
>      QLIST_HEAD(, LoadStateEntry) loadvm_handlers =
>          QLIST_HEAD_INITIALIZER(loadvm_handlers);
> @@ -1971,21 +1972,25 @@ int qemu_loadvm_state(QEMUFile *f)
>      unsigned int v;
>      int ret;
>  
> -    if (qemu_savevm_state_blocked(NULL)) {
> -        return -EINVAL;
> +    if (qemu_savevm_state_blocked(errp)) {
> +        return -ENOTSUP;

Why?

>      }
>  
>      v = qemu_get_be32(f);
> -    if (v != QEMU_VM_FILE_MAGIC)
> +    if (v != QEMU_VM_FILE_MAGIC) {
> +        error_setg(errp, "Unknown vmstate file magic");
>          return -EINVAL;
> +    }
>  
>      v = qemu_get_be32(f);
>      if (v == QEMU_VM_FILE_VERSION_COMPAT) {
> -        fprintf(stderr, "SaveVM v2 format is obsolete and don't work anymore\n");
> +        error_setg(errp, "Not supported.");

Will this new error message reach the terminal as fprintf() does? Also,
please keep the good error message.

>          return -ENOTSUP;
>      }
> -    if (v != QEMU_VM_FILE_VERSION)
> +    if (v != QEMU_VM_FILE_VERSION) {
> +        error_setg(errp, "Not supported.");

Please, improve the error message.

>          return -ENOTSUP;
> +    }
>  
>      while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
>          uint32_t instance_id, version_id, section_id;
> @@ -2007,15 +2012,16 @@ int qemu_loadvm_state(QEMUFile *f)
>              /* Find savevm section */
>              se = find_se(idstr, instance_id);
>              if (se == NULL) {
> -                fprintf(stderr, "Unknown savevm section or instance '%s' %d\n", idstr, instance_id);
> +                error_setg(errp, "Unknown savevm section or instance '%s' %d",
> +                           idstr, instance_id);
>                  ret = -EINVAL;
>                  goto out;
>              }
>  
>              /* Validate version */
>              if (version_id > se->version_id) {
> -                fprintf(stderr, "savevm: unsupported version %d for '%s' v%d\n",
> -                        version_id, idstr, se->version_id);
> +                error_setg(errp, "savevm: unsupported version %d for '%s' v%d",
> +                           version_id, idstr, se->version_id);
>                  ret = -EINVAL;
>                  goto out;
>              }
> @@ -2030,8 +2036,7 @@ int qemu_loadvm_state(QEMUFile *f)
>  
>              ret = vmstate_load(f, le->se, le->version_id);
>              if (ret < 0) {
> -                fprintf(stderr, "qemu: warning: error while loading state for instance 0x%x of device '%s'\n",
> -                        instance_id, idstr);
> +                error_setg(errp, "Failed to load vmstate.");
>                  goto out;
>              }
>              break;
> @@ -2045,20 +2050,19 @@ int qemu_loadvm_state(QEMUFile *f)
>                  }
>              }
>              if (le == NULL) {
> -                fprintf(stderr, "Unknown savevm section %d\n", section_id);
> +                error_setg(errp, "Unknown savevm section %d", section_id);
>                  ret = -EINVAL;
>                  goto out;
>              }
>  
>              ret = vmstate_load(f, le->se, le->version_id);
>              if (ret < 0) {
> -                fprintf(stderr, "qemu: warning: error while loading state section id %d\n",
> -                        section_id);
> +                error_setg(errp, "Failed to load vmstate.");
>                  goto out;
>              }
>              break;
>          default:
> -            fprintf(stderr, "Unknown savevm section type %d\n", section_type);
> +            error_setg(errp, "Unknown savevm section type %d", section_type);
>              ret = -EINVAL;
>              goto out;
>          }
> @@ -2076,6 +2080,9 @@ out:
>  
>      if (ret == 0) {
>          ret = qemu_file_get_error(f);
> +        if (ret < 0) {
> +            error_set(errp, QERR_GENERIC_ERROR, ret);

No reason to sue QERR_ macros here.

> +        }
>      }
>  
>      return ret;
> @@ -2342,7 +2349,7 @@ int load_vmstate(const char *name)
>      }
>  
>      qemu_system_reset(VMRESET_SILENT);
> -    ret = qemu_loadvm_state(f);
> +    ret = qemu_loadvm_state(f, NULL);
>  
>      qemu_fclose(f);
>      if (ret < 0) {
> diff --git a/sysemu.h b/sysemu.h
> index 11a4560..c08f7a3 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -81,7 +81,8 @@ int qemu_savevm_state_iterate(QEMUFile *f,
>  int qemu_savevm_state_complete(QEMUFile *f,
>                                 Error **errp);
>  void qemu_savevm_state_cancel(QEMUFile *f);
> -int qemu_loadvm_state(QEMUFile *f);
> +int qemu_loadvm_state(QEMUFile *f,
> +                      Error **errp);
>  
>  /* SLIRP */
>  void do_info_slirp(Monitor *mon);

Patch

diff --git a/migration.c b/migration.c
index 3ae1db0..4c2d2d3 100644
--- a/migration.c
+++ b/migration.c
@@ -86,13 +86,13 @@  void qemu_start_incoming_migration(const char *uri, Error **errp)
 static void process_incoming_migration_co(void *opaque)
 {
     QEMUFile *f = opaque;
-    int ret;
+    Error **errp = NULL;
 
-    ret = qemu_loadvm_state(f);
+    qemu_loadvm_state(f, errp);
     qemu_set_fd_handler(qemu_get_fd(f), NULL, NULL, NULL);
     qemu_fclose(f);
-    if (ret < 0) {
-        fprintf(stderr, "load of migration failed\n");
+    if (error_is_set(errp)) {
+        handle_error(errp);
         exit(0);
     }
     qemu_announce_self();
diff --git a/savevm.c b/savevm.c
index 71c7df8..2c63aad 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1962,7 +1962,8 @@  typedef struct LoadStateEntry {
     int version_id;
 } LoadStateEntry;
 
-int qemu_loadvm_state(QEMUFile *f)
+int qemu_loadvm_state(QEMUFile *f,
+                      Error **errp)
 {
     QLIST_HEAD(, LoadStateEntry) loadvm_handlers =
         QLIST_HEAD_INITIALIZER(loadvm_handlers);
@@ -1971,21 +1972,25 @@  int qemu_loadvm_state(QEMUFile *f)
     unsigned int v;
     int ret;
 
-    if (qemu_savevm_state_blocked(NULL)) {
-        return -EINVAL;
+    if (qemu_savevm_state_blocked(errp)) {
+        return -ENOTSUP;
     }
 
     v = qemu_get_be32(f);
-    if (v != QEMU_VM_FILE_MAGIC)
+    if (v != QEMU_VM_FILE_MAGIC) {
+        error_setg(errp, "Unknown vmstate file magic");
         return -EINVAL;
+    }
 
     v = qemu_get_be32(f);
     if (v == QEMU_VM_FILE_VERSION_COMPAT) {
-        fprintf(stderr, "SaveVM v2 format is obsolete and don't work anymore\n");
+        error_setg(errp, "Not supported.");
         return -ENOTSUP;
     }
-    if (v != QEMU_VM_FILE_VERSION)
+    if (v != QEMU_VM_FILE_VERSION) {
+        error_setg(errp, "Not supported.");
         return -ENOTSUP;
+    }
 
     while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
         uint32_t instance_id, version_id, section_id;
@@ -2007,15 +2012,16 @@  int qemu_loadvm_state(QEMUFile *f)
             /* Find savevm section */
             se = find_se(idstr, instance_id);
             if (se == NULL) {
-                fprintf(stderr, "Unknown savevm section or instance '%s' %d\n", idstr, instance_id);
+                error_setg(errp, "Unknown savevm section or instance '%s' %d",
+                           idstr, instance_id);
                 ret = -EINVAL;
                 goto out;
             }
 
             /* Validate version */
             if (version_id > se->version_id) {
-                fprintf(stderr, "savevm: unsupported version %d for '%s' v%d\n",
-                        version_id, idstr, se->version_id);
+                error_setg(errp, "savevm: unsupported version %d for '%s' v%d",
+                           version_id, idstr, se->version_id);
                 ret = -EINVAL;
                 goto out;
             }
@@ -2030,8 +2036,7 @@  int qemu_loadvm_state(QEMUFile *f)
 
             ret = vmstate_load(f, le->se, le->version_id);
             if (ret < 0) {
-                fprintf(stderr, "qemu: warning: error while loading state for instance 0x%x of device '%s'\n",
-                        instance_id, idstr);
+                error_setg(errp, "Failed to load vmstate.");
                 goto out;
             }
             break;
@@ -2045,20 +2050,19 @@  int qemu_loadvm_state(QEMUFile *f)
                 }
             }
             if (le == NULL) {
-                fprintf(stderr, "Unknown savevm section %d\n", section_id);
+                error_setg(errp, "Unknown savevm section %d", section_id);
                 ret = -EINVAL;
                 goto out;
             }
 
             ret = vmstate_load(f, le->se, le->version_id);
             if (ret < 0) {
-                fprintf(stderr, "qemu: warning: error while loading state section id %d\n",
-                        section_id);
+                error_setg(errp, "Failed to load vmstate.");
                 goto out;
             }
             break;
         default:
-            fprintf(stderr, "Unknown savevm section type %d\n", section_type);
+            error_setg(errp, "Unknown savevm section type %d", section_type);
             ret = -EINVAL;
             goto out;
         }
@@ -2076,6 +2080,9 @@  out:
 
     if (ret == 0) {
         ret = qemu_file_get_error(f);
+        if (ret < 0) {
+            error_set(errp, QERR_GENERIC_ERROR, ret);
+        }
     }
 
     return ret;
@@ -2342,7 +2349,7 @@  int load_vmstate(const char *name)
     }
 
     qemu_system_reset(VMRESET_SILENT);
-    ret = qemu_loadvm_state(f);
+    ret = qemu_loadvm_state(f, NULL);
 
     qemu_fclose(f);
     if (ret < 0) {
diff --git a/sysemu.h b/sysemu.h
index 11a4560..c08f7a3 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -81,7 +81,8 @@  int qemu_savevm_state_iterate(QEMUFile *f,
 int qemu_savevm_state_complete(QEMUFile *f,
                                Error **errp);
 void qemu_savevm_state_cancel(QEMUFile *f);
-int qemu_loadvm_state(QEMUFile *f);
+int qemu_loadvm_state(QEMUFile *f,
+                      Error **errp);
 
 /* SLIRP */
 void do_info_slirp(Monitor *mon);