[Qemu-devel] [PATCH] spapr: fix spapr-nvram migration

[Qemu-devel] [PATCH] spapr: fix spapr-nvram migration

[Laurent Vivier]

When spapr-nvram is backed by a file using pflash interface,
migration fails on the destination guest with assert:

    bdrv_co_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.

This avoids the problem by delaying the pflash update until after
the device loads complete.

This fix is similar to the one for the pflash_cfi01 migration:

    90c647d Fix pflash migration

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/nvram/spapr_nvram.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c
index 019f25d..4de5f70 100644
--- a/hw/nvram/spapr_nvram.c
+++ b/hw/nvram/spapr_nvram.c
@@ -39,6 +39,7 @@ typedef struct sPAPRNVRAM {
     uint32_t size;
     uint8_t *buf;
     BlockBackend *blk;
+    VMChangeStateEntry *vmstate;
 } sPAPRNVRAM;
 
 #define TYPE_VIO_SPAPR_NVRAM "spapr-nvram"
@@ -185,19 +186,25 @@ static int spapr_nvram_pre_load(void *opaque)
     return 0;
 }
 
+static void postload_update_cb(void *opaque, int running, RunState state)
+{
+    sPAPRNVRAM *nvram = opaque;
+
+    /* This is called after bdrv_invalidate_cache_all.  */
+
+    qemu_del_vm_change_state_handler(nvram->vmstate);
+    nvram->vmstate = NULL;
+
+    blk_pwrite(nvram->blk, 0, nvram->buf, nvram->size, 0);
+}
+
 static int spapr_nvram_post_load(void *opaque, int version_id)
 {
     sPAPRNVRAM *nvram = VIO_SPAPR_NVRAM(opaque);
 
     if (nvram->blk) {
-        int alen = blk_pwrite(nvram->blk, 0, nvram->buf, nvram->size, 0);
-
-        if (alen < 0) {
-            return alen;
-        }
-        if (alen != nvram->size) {
-            return -1;
-        }
+        nvram->vmstate = qemu_add_vm_change_state_handler(postload_update_cb,
+                                                          nvram);
     }
 
     return 0;
-- 
2.5.5

Re: [Qemu-devel] [PATCH] spapr: fix spapr-nvram migration

[Paolo Bonzini]

> When spapr-nvram is backed by a file using pflash interface,
> migration fails on the destination guest with assert:
> 
>     bdrv_co_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.
> 
> This avoids the problem by delaying the pflash update until after
> the device loads complete.
> 
> This fix is similar to the one for the pflash_cfi01 migration:
> 
>     90c647d Fix pflash migration
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  hw/nvram/spapr_nvram.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c
> index 019f25d..4de5f70 100644
> --- a/hw/nvram/spapr_nvram.c
> +++ b/hw/nvram/spapr_nvram.c
> @@ -39,6 +39,7 @@ typedef struct sPAPRNVRAM {
>      uint32_t size;
>      uint8_t *buf;
>      BlockBackend *blk;
> +    VMChangeStateEntry *vmstate;
>  } sPAPRNVRAM;
>  
>  #define TYPE_VIO_SPAPR_NVRAM "spapr-nvram"

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Perhaps spapr_nvram.c should have been in hw/block, or pflash should be
moved to hw/nvram.

Paolo

> @@ -185,19 +186,25 @@ static int spapr_nvram_pre_load(void *opaque)
>      return 0;
>  }
>  
> +static void postload_update_cb(void *opaque, int running, RunState state)
> +{
> +    sPAPRNVRAM *nvram = opaque;
> +
> +    /* This is called after bdrv_invalidate_cache_all.  */
> +
> +    qemu_del_vm_change_state_handler(nvram->vmstate);
> +    nvram->vmstate = NULL;
> +
> +    blk_pwrite(nvram->blk, 0, nvram->buf, nvram->size, 0);
> +}
> +
>  static int spapr_nvram_post_load(void *opaque, int version_id)
>  {
>      sPAPRNVRAM *nvram = VIO_SPAPR_NVRAM(opaque);
>  
>      if (nvram->blk) {
> -        int alen = blk_pwrite(nvram->blk, 0, nvram->buf, nvram->size, 0);
> -
> -        if (alen < 0) {
> -            return alen;
> -        }
> -        if (alen != nvram->size) {
> -            return -1;
> -        }
> +        nvram->vmstate =
> qemu_add_vm_change_state_handler(postload_update_cb,
> +                                                          nvram);
>      }
>  
>      return 0;
> --
> 2.5.5
> 
> 

Re: [Qemu-devel] [PATCH] spapr: fix spapr-nvram migration

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2397 bytes --]

On Thu, Jul 21, 2016 at 02:05:46PM +0200, Laurent Vivier wrote:
> When spapr-nvram is backed by a file using pflash interface,
> migration fails on the destination guest with assert:
> 
>     bdrv_co_pwritev: Assertion `!(bs->open_flags & 0x0800)' failed.
> 
> This avoids the problem by delaying the pflash update until after
> the device loads complete.
> 
> This fix is similar to the one for the pflash_cfi01 migration:
> 
>     90c647d Fix pflash migration
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

It's a bit sad we lose the error checking, but I can't see any easy
way around that, so I've applied to ppc-for-2.7.

> ---
>  hw/nvram/spapr_nvram.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/nvram/spapr_nvram.c b/hw/nvram/spapr_nvram.c
> index 019f25d..4de5f70 100644
> --- a/hw/nvram/spapr_nvram.c
> +++ b/hw/nvram/spapr_nvram.c
> @@ -39,6 +39,7 @@ typedef struct sPAPRNVRAM {
>      uint32_t size;
>      uint8_t *buf;
>      BlockBackend *blk;
> +    VMChangeStateEntry *vmstate;
>  } sPAPRNVRAM;
>  
>  #define TYPE_VIO_SPAPR_NVRAM "spapr-nvram"
> @@ -185,19 +186,25 @@ static int spapr_nvram_pre_load(void *opaque)
>      return 0;
>  }
>  
> +static void postload_update_cb(void *opaque, int running, RunState state)
> +{
> +    sPAPRNVRAM *nvram = opaque;
> +
> +    /* This is called after bdrv_invalidate_cache_all.  */
> +
> +    qemu_del_vm_change_state_handler(nvram->vmstate);
> +    nvram->vmstate = NULL;
> +
> +    blk_pwrite(nvram->blk, 0, nvram->buf, nvram->size, 0);
> +}
> +
>  static int spapr_nvram_post_load(void *opaque, int version_id)
>  {
>      sPAPRNVRAM *nvram = VIO_SPAPR_NVRAM(opaque);
>  
>      if (nvram->blk) {
> -        int alen = blk_pwrite(nvram->blk, 0, nvram->buf, nvram->size, 0);
> -
> -        if (alen < 0) {
> -            return alen;
> -        }
> -        if (alen != nvram->size) {
> -            return -1;
> -        }
> +        nvram->vmstate = qemu_add_vm_change_state_handler(postload_update_cb,
> +                                                          nvram);
>      }
>  
>      return 0;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]