Re: [Qemu-devel] [PATCH] migration: avoid copying ignore-shared ramblock when in incoming migration

[Jia He]

On 2019/3/19 23:49, Catherine Ho wrote:
> Commit 18269069c310 ("migration: Introduce ignore-shared capability")
> addes a ignore-shared capability to bypass the shared ramblock (e,g,
> membackend + numa node). It does good to live migration.
>
> This commit expected that QEMU doesn't write to guest RAM until
> VM starts, but it does on aarch64 qemu:
> Backtrace:
> 1  0x000055f4a296dd84 in address_space_write_rom_internal () at exec.c:3458
> 2  0x000055f4a296de3a in address_space_write_rom () at exec.c:3479
> 3  0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101
> 4  0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
> 5  0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675
> 6  0x000055f4a2c9851d in main () at vl.c:4552
>
> In address_space_write_rom_internal, if the ramblock is RAM of
> ramblock_is_ignored(), memcpy this ramblock will cause the memory-backend
> file corruption.
> But in normal case (!in_incoming_migration), this logic should be reserved.
>
> Fixes: commit 18269069c310 ("migration: Introduce ignore-shared capability")
> Signed-off-by: Catherine Ho <address@hidden>
> Suggested-by: Yury Kotov <address@hidden>

thanks, Catherine

Tested on aarch64 kvm/tcg mode

---
Cheers,
Justin (Jia He)

> ---
>   exec.c                    | 4 ++++
>   include/exec/cpu-common.h | 1 +
>   include/qemu/option.h     | 1 +
>   migration/ram.c           | 2 +-
>   vl.c                      | 2 ++
>   5 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/exec.c b/exec.c
> index 86a38d3b3b..f08321fb7a 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -47,6 +47,7 @@
>   #include "exec/address-spaces.h"
>   #include "sysemu/xen-mapcache.h"
>   #include "trace-root.h"
> +#include "qemu/option.h"
>   
>   #ifdef CONFIG_FALLOCATE_PUNCH_HOLE
>   #include <linux/falloc.h>
> @@ -3455,6 +3456,9 @@ static inline MemTxResult 
> address_space_write_rom_internal(AddressSpace *as,
>               l = memory_access_size(mr, l, addr1);
>           } else {
>               /* ROM/RAM case */
> +            if (in_incoming_migration && ramblock_is_ignored(mr->ram_block)) {
> +                break;
> +            }
>               ptr = qemu_map_ram_ptr(mr->ram_block, addr1);
>               switch (type) {
>               case WRITE_DATA:
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index cef8b88a2a..c80b7248a6 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -76,6 +76,7 @@ void *qemu_ram_get_host_addr(RAMBlock *rb);
>   ram_addr_t qemu_ram_get_offset(RAMBlock *rb);
>   ram_addr_t qemu_ram_get_used_length(RAMBlock *rb);
>   bool qemu_ram_is_shared(RAMBlock *rb);
> +bool ramblock_is_ignored(RAMBlock *block);
>   bool qemu_ram_is_uf_zeroable(RAMBlock *rb);
>   void qemu_ram_set_uf_zeroable(RAMBlock *rb);
>   bool qemu_ram_is_migratable(RAMBlock *rb);
> diff --git a/include/qemu/option.h b/include/qemu/option.h
> index 844587cab3..7509435164 100644
> --- a/include/qemu/option.h
> +++ b/include/qemu/option.h
> @@ -139,4 +139,5 @@ QemuOptsList *qemu_opts_append(QemuOptsList *dst, 
> QemuOptsList *list);
>   QDict *keyval_parse(const char *params, const char *implied_key,
>                       Error **errp);
>   
> +extern bool in_incoming_migration;
>   #endif
> diff --git a/migration/ram.c b/migration/ram.c
> index 35bd6213e9..d6de9d335d 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -159,7 +159,7 @@ out:
>       return ret;
>   }
>   
> -static bool ramblock_is_ignored(RAMBlock *block)
> +bool ramblock_is_ignored(RAMBlock *block)
>   {
>       return !qemu_ram_is_migratable(block) ||
>              (migrate_ignore_shared() && qemu_ram_is_shared(block));
> diff --git a/vl.c b/vl.c
> index c1d5484e12..b79aec9ac3 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -145,6 +145,7 @@ bool enable_cpu_pm = false;
>   int nb_nics;
>   NICInfo nd_table[MAX_NICS];
>   int autostart;
> +bool in_incoming_migration;
>   static enum {
>       RTC_BASE_UTC,
>       RTC_BASE_LOCALTIME,
> @@ -3797,6 +3798,7 @@ int main(int argc, char **argv, char **envp)
>                       runstate_set(RUN_STATE_INMIGRATE);
>                   }
>                   incoming = optarg;
> +                in_incoming_migration = true;
>                   break;
>               case QEMU_OPTION_only_migratable:
>                   /*

--