Re: [Qemu-devel] [PATCH 1/2] Revert "migration: do not sent zero pages in bulk stage"

[Paolo Bonzini <pbonzini@redhat.com>]

Il 06/06/2013 10:36, Peter Lieven ha scritto:
> 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 reverts commit f1c72795af573b24a7da5eb52375c9aba8a37972.
> 
> Conflicts:
> 
> 	arch_init.c
> 	qmp-commands.hx
> 
> Signed-off-by: Peter Lieven <pl@kamp.de>

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

> ---
>  arch_init.c                   |   24 ++++--------------------
>  hmp.c                         |    2 --
>  include/migration/migration.h |    2 --
>  migration.c                   |    3 +--
>  qapi-schema.json              |    8 +++-----
>  qmp-commands.hx               |    1 -
>  6 files changed, 8 insertions(+), 32 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 5d32ecf..283081d 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -183,7 +183,6 @@ int64_t xbzrle_cache_resize(int64_t new_size)
>  /* accounting for migration statistics */
>  typedef struct AccountingInfo {
>      uint64_t dup_pages;
> -    uint64_t skipped_pages;
>      uint64_t norm_pages;
>      uint64_t iterations;
>      uint64_t xbzrle_bytes;
> @@ -209,16 +208,6 @@ uint64_t dup_mig_pages_transferred(void)
>      return acct_info.dup_pages;
>  }
>  
> -uint64_t skipped_mig_bytes_transferred(void)
> -{
> -    return acct_info.skipped_pages * TARGET_PAGE_SIZE;
> -}
> -
> -uint64_t skipped_mig_pages_transferred(void)
> -{
> -    return acct_info.skipped_pages;
> -}
> -
>  uint64_t norm_mig_bytes_transferred(void)
>  {
>      return acct_info.norm_pages * TARGET_PAGE_SIZE;
> @@ -457,15 +446,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 --git a/hmp.c b/hmp.c
> index 4fb76ec..55c4423 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -173,8 +173,6 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
>                         info->ram->total >> 10);
>          monitor_printf(mon, "duplicate: %" PRIu64 " pages\n",
>                         info->ram->duplicate);
> -        monitor_printf(mon, "skipped: %" PRIu64 " pages\n",
> -                       info->ram->skipped);
>          monitor_printf(mon, "normal: %" PRIu64 " pages\n",
>                         info->ram->normal);
>          monitor_printf(mon, "normal bytes: %" PRIu64 " kbytes\n",
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index e2acec6..bb617fd 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -96,8 +96,6 @@ extern SaveVMHandlers savevm_ram_handlers;
>  
>  uint64_t dup_mig_bytes_transferred(void);
>  uint64_t dup_mig_pages_transferred(void);
> -uint64_t skipped_mig_bytes_transferred(void);
> -uint64_t skipped_mig_pages_transferred(void);
>  uint64_t norm_mig_bytes_transferred(void);
>  uint64_t norm_mig_pages_transferred(void);
>  uint64_t xbzrle_mig_bytes_transferred(void);
> diff --git a/migration.c b/migration.c
> index 058f9e6..c305d3e 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -197,11 +197,11 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>          info->ram->remaining = ram_bytes_remaining();
>          info->ram->total = ram_bytes_total();
>          info->ram->duplicate = dup_mig_pages_transferred();
> -        info->ram->skipped = skipped_mig_pages_transferred();
>          info->ram->normal = norm_mig_pages_transferred();
>          info->ram->normal_bytes = norm_mig_bytes_transferred();
>          info->ram->dirty_pages_rate = s->dirty_pages_rate;
>  
> +
>          if (blk_mig_active()) {
>              info->has_disk = true;
>              info->disk = g_malloc0(sizeof(*info->disk));
> @@ -227,7 +227,6 @@ MigrationInfo *qmp_query_migrate(Error **errp)
>          info->ram->remaining = 0;
>          info->ram->total = ram_bytes_total();
>          info->ram->duplicate = dup_mig_pages_transferred();
> -        info->ram->skipped = skipped_mig_pages_transferred();
>          info->ram->normal = norm_mig_pages_transferred();
>          info->ram->normal_bytes = norm_mig_bytes_transferred();
>          break;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index ef1f657..db0986c 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -499,9 +499,7 @@
>  #
>  # @total: total amount of bytes involved in the migration process
>  #
> -# @duplicate: number of duplicate (zero) pages (since 1.2)
> -#
> -# @skipped: number of skipped zero pages (since 1.5)
> +# @duplicate: number of duplicate pages (since 1.2)
>  #
>  # @normal : number of normal pages (since 1.2)
>  #
> @@ -514,8 +512,8 @@
>  ##
>  { 'type': 'MigrationStats',
>    'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
> -           'duplicate': 'int', 'skipped': 'int', 'normal': 'int',
> -           'normal-bytes': 'int', 'dirty-pages-rate' : 'int' } }
> +           'duplicate': 'int', 'normal': 'int', 'normal-bytes': 'int',
> +           'dirty-pages-rate' : 'int' } }
>  
>  ##
>  # @XBZRLECacheStats
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index ffd130e..fa5986f 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2515,7 +2515,6 @@ The main json-object contains the following:
>           - "duplicate": number of pages filled entirely with the same
>              byte (json-int)
>              These are sent over the wire much more efficiently.
> -         - "skipped": number of skipped zero pages (json-int)
>           - "normal" : number of whole pages transferred.  I.e. they
>              were not sent as duplicate or xbzrle pages (json-int)
>           - "normal-bytes" : number of bytes transferred in whole
>