On Wed, 20 Mar 2019 at 13:07, Peter Xu <pet...@redhat.com> wrote:
>
> On Tue, Mar 19, 2019 at 11:49:22AM -0400, 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.
>
> (I am sorry if these questions are silly...)
>
> Could I ask when a ROM will be changed during execution of the guest,
> and why?
Sure :), as told by Peter
"as part of reset, we write the contents of ROM blobs, ELF files loaded through
-kernel, etc, to the guest memory."
>
> I can understand that a rom_reset() should feed the real ROM buffer
> with the ROM data that provided, but I don't understand why it needs
> to be changed later on and why data could corrupt (in that case it's
> not only changed but shared with other processes with reasons).
I guess (sorry about that), rom data should be restore on reset. I watched about
several mega bytes passed to memcpy in my debuggin.
But if the ram is memory-backend-file and shared, all the neccessry data had
been reserved and unchanged in memory-backend file.
Best regard,
Catherine
>
> >
> > Fixes: commit 18269069c310 ("migration: Introduce ignore-shared capability")
> > Signed-off-by: Catherine Ho <catherine.h...@gmail.com>
> > Suggested-by: Yury Kotov <yury-ko...@yandex-team.ru>
> > ---
> > 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)) {
>
> It's hackish IMHO... and this will be true not only during the
> incoming migration, but also for the rest lifecycle of the VM after
> migration finished. Is that really what you want (since after
> in_incoming_migration is set, it's never cleared)? Even if the answer
> is yes, you can also consider to reuse the "incoming" variable right
> above it and export that? Though maybe it needs a rename.
Thanks, I am considerring clear it after
process_incoming_migration_co->qemu_loadvm_state
>
> Thanks,
>
> > @@ -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:
> > /*
> > --
> > 2.17.1
> >
> >
>
> Regards,
>
> --
> Peter Xu
