diff mbox

check_section_footers: Check the correct section_id

Message ID 1435825323-2806-1-git-send-email-dgilbert@redhat.com
State New
Headers show

Commit Message

Dr. David Alan Gilbert July 2, 2015, 8:22 a.m. UTC
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The section footers check was incorrectly checking the section_id
in the SaveStateEntry not the LoadStateEntry.  These can validly be different
if the two QEMU instances have instantiated their devices in a
different order.  The test only cares that we're finishing the same
section we started, and hence it's the LoadStateEntry that we care about.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 migration/savevm.c | 74 +++++++++++++++++++++++++++---------------------------
 1 file changed, 37 insertions(+), 37 deletions(-)

Comments

Christian Borntraeger July 2, 2015, 8:58 a.m. UTC | #1
Am 02.07.2015 um 10:22 schrieb Dr. David Alan Gilbert (git):
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> The section footers check was incorrectly checking the section_id
> in the SaveStateEntry not the LoadStateEntry.  These can validly be different
> if the two QEMU instances have instantiated their devices in a
> different order.  The test only cares that we're finishing the same
> section we started, and hence it's the LoadStateEntry that we care about.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>

Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>


> ---
>  migration/savevm.c | 74 +++++++++++++++++++++++++++---------------------------
>  1 file changed, 37 insertions(+), 37 deletions(-)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 1a9b00b..acbad9c 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -653,41 +653,6 @@ static void save_section_footer(QEMUFile *f, SaveStateEntry *se)
>      }
>  }
> 
> -/*
> - * Read a footer off the wire and check that it matches the expected section
> - *
> - * Returns: true if the footer was good
> - *          false if there is a problem (and calls error_report to say why)
> - */
> -static bool check_section_footer(QEMUFile *f, SaveStateEntry *se)
> -{
> -    uint8_t read_mark;
> -    uint32_t read_section_id;
> -
> -    if (skip_section_footers) {
> -        /* No footer to check */
> -        return true;
> -    }
> -
> -    read_mark = qemu_get_byte(f);
> -
> -    if (read_mark != QEMU_VM_SECTION_FOOTER) {
> -        error_report("Missing section footer for %s", se->idstr);
> -        return false;
> -    }
> -
> -    read_section_id = qemu_get_be32(f);
> -    if (read_section_id != se->section_id) {
> -        error_report("Mismatched section id in footer for %s -"
> -                     " read 0x%x expected 0x%x",
> -                     se->idstr, read_section_id, se->section_id);
> -        return false;
> -    }
> -
> -    /* All good */
> -    return true;
> -}
> -
>  bool qemu_savevm_state_blocked(Error **errp)
>  {
>      SaveStateEntry *se;
> @@ -989,6 +954,41 @@ struct LoadStateEntry {
>      int version_id;
>  };
> 
> +/*
> + * Read a footer off the wire and check that it matches the expected section
> + *
> + * Returns: true if the footer was good
> + *          false if there is a problem (and calls error_report to say why)
> + */
> +static bool check_section_footer(QEMUFile *f, LoadStateEntry *le)
> +{
> +    uint8_t read_mark;
> +    uint32_t read_section_id;
> +
> +    if (skip_section_footers) {
> +        /* No footer to check */
> +        return true;
> +    }
> +
> +    read_mark = qemu_get_byte(f);
> +
> +    if (read_mark != QEMU_VM_SECTION_FOOTER) {
> +        error_report("Missing section footer for %s", le->se->idstr);
> +        return false;
> +    }
> +
> +    read_section_id = qemu_get_be32(f);
> +    if (read_section_id != le->section_id) {
> +        error_report("Mismatched section id in footer for %s -"
> +                     " read 0x%x expected 0x%x",
> +                     le->se->idstr, read_section_id, le->section_id);
> +        return false;
> +    }
> +
> +    /* All good */
> +    return true;
> +}
> +
>  void loadvm_free_handlers(MigrationIncomingState *mis)
>  {
>      LoadStateEntry *le, *new_le;
> @@ -1082,7 +1082,7 @@ int qemu_loadvm_state(QEMUFile *f)
>                               " device '%s'", instance_id, idstr);
>                  goto out;
>              }
> -            if (!check_section_footer(f, le->se)) {
> +            if (!check_section_footer(f, le)) {
>                  ret = -EINVAL;
>                  goto out;
>              }
> @@ -1109,7 +1109,7 @@ int qemu_loadvm_state(QEMUFile *f)
>                               section_id, le->se->idstr);
>                  goto out;
>              }
> -            if (!check_section_footer(f, le->se)) {
> +            if (!check_section_footer(f, le)) {
>                  ret = -EINVAL;
>                  goto out;
>              }
>
diff mbox

Patch

diff --git a/migration/savevm.c b/migration/savevm.c
index 1a9b00b..acbad9c 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -653,41 +653,6 @@  static void save_section_footer(QEMUFile *f, SaveStateEntry *se)
     }
 }
 
-/*
- * Read a footer off the wire and check that it matches the expected section
- *
- * Returns: true if the footer was good
- *          false if there is a problem (and calls error_report to say why)
- */
-static bool check_section_footer(QEMUFile *f, SaveStateEntry *se)
-{
-    uint8_t read_mark;
-    uint32_t read_section_id;
-
-    if (skip_section_footers) {
-        /* No footer to check */
-        return true;
-    }
-
-    read_mark = qemu_get_byte(f);
-
-    if (read_mark != QEMU_VM_SECTION_FOOTER) {
-        error_report("Missing section footer for %s", se->idstr);
-        return false;
-    }
-
-    read_section_id = qemu_get_be32(f);
-    if (read_section_id != se->section_id) {
-        error_report("Mismatched section id in footer for %s -"
-                     " read 0x%x expected 0x%x",
-                     se->idstr, read_section_id, se->section_id);
-        return false;
-    }
-
-    /* All good */
-    return true;
-}
-
 bool qemu_savevm_state_blocked(Error **errp)
 {
     SaveStateEntry *se;
@@ -989,6 +954,41 @@  struct LoadStateEntry {
     int version_id;
 };
 
+/*
+ * Read a footer off the wire and check that it matches the expected section
+ *
+ * Returns: true if the footer was good
+ *          false if there is a problem (and calls error_report to say why)
+ */
+static bool check_section_footer(QEMUFile *f, LoadStateEntry *le)
+{
+    uint8_t read_mark;
+    uint32_t read_section_id;
+
+    if (skip_section_footers) {
+        /* No footer to check */
+        return true;
+    }
+
+    read_mark = qemu_get_byte(f);
+
+    if (read_mark != QEMU_VM_SECTION_FOOTER) {
+        error_report("Missing section footer for %s", le->se->idstr);
+        return false;
+    }
+
+    read_section_id = qemu_get_be32(f);
+    if (read_section_id != le->section_id) {
+        error_report("Mismatched section id in footer for %s -"
+                     " read 0x%x expected 0x%x",
+                     le->se->idstr, read_section_id, le->section_id);
+        return false;
+    }
+
+    /* All good */
+    return true;
+}
+
 void loadvm_free_handlers(MigrationIncomingState *mis)
 {
     LoadStateEntry *le, *new_le;
@@ -1082,7 +1082,7 @@  int qemu_loadvm_state(QEMUFile *f)
                              " device '%s'", instance_id, idstr);
                 goto out;
             }
-            if (!check_section_footer(f, le->se)) {
+            if (!check_section_footer(f, le)) {
                 ret = -EINVAL;
                 goto out;
             }
@@ -1109,7 +1109,7 @@  int qemu_loadvm_state(QEMUFile *f)
                              section_id, le->se->idstr);
                 goto out;
             }
-            if (!check_section_footer(f, le->se)) {
+            if (!check_section_footer(f, le)) {
                 ret = -EINVAL;
                 goto out;
             }