Re: [Qemu-devel] [RFC PATCH] migration: reintroduce skipped zero pages

[Juan Quintela <quintela@redhat.com>]

Peter Lieven <pl@kamp.de> wrote:
> commit f1c72795a introduced skipping of all zero pages during
> bulk phase of ram migration. In theory this should have worked,
> however the underlying assumption that the memory of target VM
> is totally empty (zeroed) was wrong. Altough qemu accepts an incoming
> migration BIOS, ROMs or tables are set up. If e.g. a ROM differs
> between source and target we get memory corruption if a page
> is zero at the source and not at the target. Therefore the
> original patch was reverted later on.
>
> This patch now reintroduces the feature to skip zero pages.
> However, this time it has to be explicitely turned on through
> a migration capability which should only be enabled if both
> source and destination support it.
>
> The feature especially makes sense if you expect a significant portion
> of zero pages while bandwidth or disk space is limited.
> Because even if a zero page is compressed we still transfer 9 bytes for
> each page.
>
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
>  arch_init.c                   |   44 +++++++++++++++++++++++++++++++++--------
>  include/migration/migration.h |    2 +-
>  migration.c                   |    9 +++++++++
>  qapi-schema.json              |   11 ++++++++---
>  4 files changed, 54 insertions(+), 12 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 995f56d..2579302 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -123,7 +123,8 @@ static uint64_t bitmap_sync_count;
>  #define RAM_SAVE_FLAG_EOS      0x10
>  #define RAM_SAVE_FLAG_CONTINUE 0x20
>  #define RAM_SAVE_FLAG_XBZRLE   0x40
> -/* 0x80 is reserved in migration.h start with 0x100 next */
> +/* 0x80 is reserved in migration.h */
> +#define RAM_SAVE_FLAG_ZERO_TARGET 0x100
>  
>  static struct defconfig_file {
>      const char *filename;
> @@ -575,8 +576,9 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
>      MemoryRegion *mr;
>      ram_addr_t current_addr;
>  
> -    if (!block)
> +    if (!block) {
>          block = QTAILQ_FIRST(&ram_list.blocks);
> +    }
>  
>      while (true) {
>          mr = block->mr;
> @@ -619,11 +621,16 @@ static int ram_save_block(QEMUFile *f, bool last_stage)
>                      }
>                  }
>              } else if (is_zero_range(p, TARGET_PAGE_SIZE)) {
> -                acct_info.dup_pages++;
> -                bytes_sent = save_block_hdr(f, block, offset, cont,
> -                                            RAM_SAVE_FLAG_COMPRESS);
> -                qemu_put_byte(f, 0);
> -                bytes_sent++;
> +                if (!ram_bulk_stage || !migrate_skip_zero_pages()) {
> +                    acct_info.dup_pages++;
> +                    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;
> +                }
>                  /* Must let xbzrle know, otherwise a previous (now 0'd) cached
>                   * page would be stale
>                   */
> @@ -752,6 +759,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>  {
>      RAMBlock *block;
>      int64_t ram_bitmap_pages; /* Size of bitmap in pages, including gaps */
> +    uint64_t flags = 0;

       flags = RAM_SAVE_FLAG_MEM_SIZE;


>  
>      mig_throttle_on = false;
>      dirty_rate_high_cnt = 0;
> @@ -812,7 +820,11 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>      migration_bitmap_sync();
>      qemu_mutex_unlock_iothread();
>  
> -    qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE);
> +    if (migrate_skip_zero_pages()) {
> +        flags |= RAM_SAVE_FLAG_ZERO_TARGET;
> +    }
> +
> +    qemu_put_be64(f, ram_bytes_total() | RAM_SAVE_FLAG_MEM_SIZE | flags);

       qemu_put_be64(f, ram_bytes_total() | flags);

??


Could someone from pseries take a look?

Thanks, Juan.