Re: [Qemu-devel] [PATCH 1/4] migration: add RAMBlock's offset validation

[Dr. David Alan Gilbert]

* Yury Kotov (address@hidden) wrote:
> RAM migration has a RAMBlock validation stage (flag RAM_SAVE_FLAG_MEM_SIZE).
> In this stage QEMU checks further information about RAMBlock:
> 1. Presence (by idstr),
> 2. Length (trying to resize, when differs),
> 3. Optional page size.
> 
> This patch adds a check for RAMBlock's offset. Currently we check it during
> RAM pages loading - every RAM page has an offset in its header. But there is a
> case when we don't send RAM pages (see below).
> 
> The following commits introduce a capability (ignore-external) to skip some
> RAM blocks from migration. In such case the migration stream contains only
> meta information about RAM blocks to validate them. So, the only way to check
> block's offset is to send it explicitly.
> 
> Signed-off-by: Yury Kotov <address@hidden>

But why check that offsets match?  THey aren't supposed to!
Offset's are entirely private to each qemu and they're allowed to be
different; the only requirement is that the length and name of each
RAMBlock matches, then all the operations we do over the migration
stream are relative to the start of the block.

One example where they are validly different is where you hotplug some
RAM, so for example:


  source qemu
      -M 4G
      hotplug PCI card
      hotplug 2G

  destination qemu
      -M 4G
      PCI card declared on the command line
      extra 2G declared on the command line

The offsets are different but we can migrate that case fine.

Dave

> ---
>  migration/ram.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index 7e7deec4d8..39629254e1 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -3171,6 +3171,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>          if (migrate_postcopy_ram() && block->page_size != 
> qemu_host_page_size) {
>              qemu_put_be64(f, block->page_size);
>          }
> +        qemu_put_be64(f, block->offset);
>      }
>  
>      rcu_read_unlock();
> @@ -4031,7 +4032,7 @@ static int ram_load(QEMUFile *f, void *opaque, int 
> version_id)
>  
>      seq_iter++;
>  
> -    if (version_id != 4) {
> +    if (version_id < 4) {
>          ret = -EINVAL;
>      }
>  
> @@ -4132,6 +4133,16 @@ static int ram_load(QEMUFile *f, void *opaque, int 
> version_id)
>                              ret = -EINVAL;
>                          }
>                      }
> +                    if (version_id >= 5) {
> +                        ram_addr_t offset;
> +                        offset = qemu_get_be64(f);
> +                        if (block->offset != offset) {
> +                            error_report("Mismatched RAM block offset %s "
> +                                         "%" PRId64 "!= %" PRId64,
> +                                         id, offset, 
> (uint64_t)block->offset);
> +                            ret = -EINVAL;
> +                        }
> +                    }
>                      ram_control_load_hook(f, RAM_CONTROL_BLOCK_REG,
>                                            block->idstr);
>                  } else {
> @@ -4363,5 +4374,5 @@ static SaveVMHandlers savevm_ram_handlers = {
>  void ram_mig_init(void)
>  {
>      qemu_mutex_init(&XBZRLE.lock);
> -    register_savevm_live(NULL, "ram", 0, 4, &savevm_ram_handlers, 
> &ram_state);
> +    register_savevm_live(NULL, "ram", 0, 5, &savevm_ram_handlers, 
> &ram_state);
>  }
> -- 
> 2.20.1
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK