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

[Peter Lieven <pl@kamp.de>]

Am 12.05.2014 12:02, schrieb Juan Quintela:
> 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?

Yes, that would be great.

I was also wondering if we write anything into pc.ram or *.ram segment or just into
the other regions?

Peter

>
> Thanks, Juan.