diff mbox

[v5,1/2] spapr: Add a "no HPT" encoding to HTAB migration stream

Message ID 1496822093-5700-2-git-send-email-bharata@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bharata B Rao June 7, 2017, 7:54 a.m. UTC
Add a "no HPT" encoding (using value -1) to the HTAB migration
stream (in the place of HPT size) when the guest doesn't allocate HPT.
This will help the target side to match target HPT with the source HPT
and thus enable successful migration.

Suggested-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

Comments

David Gibson June 8, 2017, 4:28 a.m. UTC | #1
On Wed, Jun 07, 2017 at 01:24:52PM +0530, Bharata B Rao wrote:
> Add a "no HPT" encoding (using value -1) to the HTAB migration
> stream (in the place of HPT size) when the guest doesn't allocate HPT.
> This will help the target side to match target HPT with the source HPT
> and thus enable successful migration.
> 
> Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

As dicussed in the thread on the previous version, I still think you
can use section_hdr == 0 as the no-HPT encoding (matching htab_shift
== 0.

> ---
>  hw/ppc/spapr.c | 28 ++++++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 86e6228..df27c5c 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1557,13 +1557,19 @@ static int htab_save_setup(QEMUFile *f, void *opaque)
>      sPAPRMachineState *spapr = opaque;
>  
>      /* "Iteration" header */
> -    qemu_put_be32(f, spapr->htab_shift);
> +    if (!spapr->htab_shift) {
> +        qemu_put_be32(f, -1);
> +    } else {
> +        qemu_put_be32(f, spapr->htab_shift);
> +    }
>  
>      if (spapr->htab) {
>          spapr->htab_save_index = 0;
>          spapr->htab_first_pass = true;
>      } else {
> -        assert(kvm_enabled());
> +        if (spapr->htab_shift) {
> +            assert(kvm_enabled());
> +        }
>      }
>  
>  
> @@ -1709,7 +1715,12 @@ static int htab_save_iterate(QEMUFile *f, void *opaque)
>      int rc = 0;
>  
>      /* Iteration header */
> -    qemu_put_be32(f, 0);
> +    if (!spapr->htab_shift) {
> +        qemu_put_be32(f, -1);
> +        return 0;
> +    } else {
> +        qemu_put_be32(f, 0);
> +    }
>  
>      if (!spapr->htab) {
>          assert(kvm_enabled());
> @@ -1743,7 +1754,12 @@ static int htab_save_complete(QEMUFile *f, void *opaque)
>      int fd;
>  
>      /* Iteration header */
> -    qemu_put_be32(f, 0);
> +    if (!spapr->htab_shift) {
> +        qemu_put_be32(f, -1);
> +        return 0;
> +    } else {
> +        qemu_put_be32(f, 0);
> +    }
>  
>      if (!spapr->htab) {
>          int rc;
> @@ -1787,6 +1803,10 @@ static int htab_load(QEMUFile *f, void *opaque, int version_id)
>  
>      section_hdr = qemu_get_be32(f);
>  
> +    if (section_hdr == -1) {

You should free the existing HPT (if any) here.

> +        return 0;
> +    }
> +
>      if (section_hdr) {
>          Error *local_err = NULL;
>
David Gibson June 8, 2017, 6:01 a.m. UTC | #2
On Thu, Jun 08, 2017 at 02:28:48PM +1000, David Gibson wrote:
> On Wed, Jun 07, 2017 at 01:24:52PM +0530, Bharata B Rao wrote:
> > Add a "no HPT" encoding (using value -1) to the HTAB migration
> > stream (in the place of HPT size) when the guest doesn't allocate HPT.
> > This will help the target side to match target HPT with the source HPT
> > and thus enable successful migration.
> > 
> > Suggested-by: David Gibson <david@gibson.dropbear.id.au>
> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> 
> As dicussed in the thread on the previous version, I still think you
> can use section_hdr == 0 as the no-HPT encoding (matching htab_shift
> == 0.

Bharata discussed this with me on IRC and pointed out I was mistaken.
It thought the overall stream header was distinct from the incremental
content, but it's not.  The zero value in the header marks this
"chunk" of data as being continuing incremental content (the "middle"
of the stream) rather than the initial stream header which gives the
HPT size.

So, yes, we do need a new and different encoding for "no HPT".
diff mbox

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 86e6228..df27c5c 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1557,13 +1557,19 @@  static int htab_save_setup(QEMUFile *f, void *opaque)
     sPAPRMachineState *spapr = opaque;
 
     /* "Iteration" header */
-    qemu_put_be32(f, spapr->htab_shift);
+    if (!spapr->htab_shift) {
+        qemu_put_be32(f, -1);
+    } else {
+        qemu_put_be32(f, spapr->htab_shift);
+    }
 
     if (spapr->htab) {
         spapr->htab_save_index = 0;
         spapr->htab_first_pass = true;
     } else {
-        assert(kvm_enabled());
+        if (spapr->htab_shift) {
+            assert(kvm_enabled());
+        }
     }
 
 
@@ -1709,7 +1715,12 @@  static int htab_save_iterate(QEMUFile *f, void *opaque)
     int rc = 0;
 
     /* Iteration header */
-    qemu_put_be32(f, 0);
+    if (!spapr->htab_shift) {
+        qemu_put_be32(f, -1);
+        return 0;
+    } else {
+        qemu_put_be32(f, 0);
+    }
 
     if (!spapr->htab) {
         assert(kvm_enabled());
@@ -1743,7 +1754,12 @@  static int htab_save_complete(QEMUFile *f, void *opaque)
     int fd;
 
     /* Iteration header */
-    qemu_put_be32(f, 0);
+    if (!spapr->htab_shift) {
+        qemu_put_be32(f, -1);
+        return 0;
+    } else {
+        qemu_put_be32(f, 0);
+    }
 
     if (!spapr->htab) {
         int rc;
@@ -1787,6 +1803,10 @@  static int htab_load(QEMUFile *f, void *opaque, int version_id)
 
     section_hdr = qemu_get_be32(f);
 
+    if (section_hdr == -1) {
+        return 0;
+    }
+
     if (section_hdr) {
         Error *local_err = NULL;