diff mbox

[PATCHv2,1/2] Revert "migration: do not sent zero pages in bulk stage"

Message ID 1370859260-8183-2-git-send-email-pl@kamp.de
State New
Headers show

Commit Message

Peter Lieven June 10, 2013, 10:14 a.m. UTC
Not sending zero pages breaks migration if a page is zero
at the source but not at the destination. This can e.g. happen
if different BIOS versions are used at source and destination.
It has also been reported that migration on pseries is completely
broken with this patch.

This effectively reverts commit f1c72795af573b24a7da5eb52375c9aba8a37972.

Conflicts:

	arch_init.c

Signed-off-by: Peter Lieven <pl@kamp.de>
---
 arch_init.c |   13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

Comments

Michael Roth June 10, 2013, 3:42 p.m. UTC | #1
On Mon, Jun 10, 2013 at 12:14:19PM +0200, Peter Lieven wrote:
> Not sending zero pages breaks migration if a page is zero
> at the source but not at the destination. This can e.g. happen
> if different BIOS versions are used at source and destination.
> It has also been reported that migration on pseries is completely
> broken with this patch.
> 
> This effectively reverts commit f1c72795af573b24a7da5eb52375c9aba8a37972.
> 
> Conflicts:
> 
> 	arch_init.c
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

Also CC'ing qemu-stable

> ---
>  arch_init.c |   13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 5d32ecf..08fccf6 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -457,15 +457,10 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
>              bytes_sent = -1;
>              if (is_zero_page(p)) {
>                  acct_info.dup_pages++;
> -                if (!ram_bulk_stage) {
> -                    bytes_sent = save_block_hdr(f, block, offset, cont,
> -                                                RAM_SAVE_FLAG_COMPRESS);
> -                    qemu_put_byte(f, 0);
> -                    bytes_sent++;
> -                } else {
> -                    acct_info.skipped_pages++;
> -                    bytes_sent = 0;
> -                }
> +                bytes_sent = save_block_hdr(f, block, offset, cont,
> +                                            RAM_SAVE_FLAG_COMPRESS);
> +                qemu_put_byte(f, 0);
> +                bytes_sent++;
>              } else if (!ram_bulk_stage && migrate_use_xbzrle()) {
>                  current_addr = block->offset + offset;
>                  bytes_sent = save_xbzrle_page(f, p, current_addr, block,
> -- 
> 1.7.9.5
> 
>
Orit Wasserman June 10, 2013, 5:08 p.m. UTC | #2
On 06/10/2013 01:14 PM, Peter Lieven wrote:
> Not sending zero pages breaks migration if a page is zero
> at the source but not at the destination. This can e.g. happen
> if different BIOS versions are used at source and destination.
> It has also been reported that migration on pseries is completely
> broken with this patch.
> 
> This effectively reverts commit f1c72795af573b24a7da5eb52375c9aba8a37972.
> 
> Conflicts:
> 
> 	arch_init.c
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  arch_init.c |   13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 5d32ecf..08fccf6 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -457,15 +457,10 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
>              bytes_sent = -1;
>              if (is_zero_page(p)) {
>                  acct_info.dup_pages++;
> -                if (!ram_bulk_stage) {
> -                    bytes_sent = save_block_hdr(f, block, offset, cont,
> -                                                RAM_SAVE_FLAG_COMPRESS);
> -                    qemu_put_byte(f, 0);
> -                    bytes_sent++;
> -                } else {
> -                    acct_info.skipped_pages++;
> -                    bytes_sent = 0;
> -                }
> +                bytes_sent = save_block_hdr(f, block, offset, cont,
> +                                            RAM_SAVE_FLAG_COMPRESS);
> +                qemu_put_byte(f, 0);
> +                bytes_sent++;
>              } else if (!ram_bulk_stage && migrate_use_xbzrle()) {
>                  current_addr = block->offset + offset;
>                  bytes_sent = save_xbzrle_page(f, p, current_addr, block,
> 

Reviewed-by: Orit Wasserman <owasserm@redhat.com>
Wayne Xia June 13, 2013, 2:45 a.m. UTC | #3
clear patch, I don't see any issue.

Reviewed-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>

> Not sending zero pages breaks migration if a page is zero
> at the source but not at the destination. This can e.g. happen
> if different BIOS versions are used at source and destination.
> It has also been reported that migration on pseries is completely
> broken with this patch.
> 
> This effectively reverts commit f1c72795af573b24a7da5eb52375c9aba8a37972.
> 
> Conflicts:
> 
> 	arch_init.c
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>   arch_init.c |   13 ++++---------
>   1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 5d32ecf..08fccf6 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -457,15 +457,10 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
>               bytes_sent = -1;
>               if (is_zero_page(p)) {
>                   acct_info.dup_pages++;
> -                if (!ram_bulk_stage) {
> -                    bytes_sent = save_block_hdr(f, block, offset, cont,
> -                                                RAM_SAVE_FLAG_COMPRESS);
> -                    qemu_put_byte(f, 0);
> -                    bytes_sent++;
> -                } else {
> -                    acct_info.skipped_pages++;
> -                    bytes_sent = 0;
> -                }
> +                bytes_sent = save_block_hdr(f, block, offset, cont,
> +                                            RAM_SAVE_FLAG_COMPRESS);
> +                qemu_put_byte(f, 0);
> +                bytes_sent++;
>               } else if (!ram_bulk_stage && migrate_use_xbzrle()) {
>                   current_addr = block->offset + offset;
>                   bytes_sent = save_xbzrle_page(f, p, current_addr, block,
>
diff mbox

Patch

diff --git a/arch_init.c b/arch_init.c
index 5d32ecf..08fccf6 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -457,15 +457,10 @@  static int ram_save_block(QEMUFile *f, bool last_stage)
             bytes_sent = -1;
             if (is_zero_page(p)) {
                 acct_info.dup_pages++;
-                if (!ram_bulk_stage) {
-                    bytes_sent = save_block_hdr(f, block, offset, cont,
-                                                RAM_SAVE_FLAG_COMPRESS);
-                    qemu_put_byte(f, 0);
-                    bytes_sent++;
-                } else {
-                    acct_info.skipped_pages++;
-                    bytes_sent = 0;
-                }
+                bytes_sent = save_block_hdr(f, block, offset, cont,
+                                            RAM_SAVE_FLAG_COMPRESS);
+                qemu_put_byte(f, 0);
+                bytes_sent++;
             } else if (!ram_bulk_stage && migrate_use_xbzrle()) {
                 current_addr = block->offset + offset;
                 bytes_sent = save_xbzrle_page(f, p, current_addr, block,