Message ID | 1596122076-341293-10-git-send-email-steven.sistare@oracle.com |
---|---|
State | New |
Headers | show |
Series | [V1,01/32] savevm: add vmstate handler iterators | expand |
* Steve Sistare (steven.sistare@oracle.com) wrote: > cprsave and cprload require that guest ram be backed by an externally > visible shared file. Check that in cprsave. > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > --- > exec.c | 32 ++++++++++++++++++++++++++++++++ > include/exec/memory.h | 2 ++ > migration/savevm.c | 4 ++++ > 3 files changed, 38 insertions(+) > > diff --git a/exec.c b/exec.c > index 6f381f9..02160e0 100644 > --- a/exec.c > +++ b/exec.c > @@ -2726,6 +2726,38 @@ ram_addr_t qemu_ram_addr_from_host(void *ptr) > return block->offset + offset; > } > > +/* > + * Return true if any memory regions are writable and not backed by shared > + * memory. Exclude x86 option rom shadow "pc.rom" by name, even though it is > + * writable. Tell me about 'pc.rom' - this is a very odd hack. Again note the trick done by the existing migration capability x-ignore-shared ; it doesn't special case, it just doesn't migrate the 'shared' blocks. Dave > + */ > +bool qemu_ram_volatile(Error **errp) > +{ > + RAMBlock *block; > + MemoryRegion *mr; > + bool ret = false; > + > + rcu_read_lock(); > + QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { > + mr = block->mr; > + if (mr && > + memory_region_is_ram(mr) && > + !memory_region_is_ram_device(mr) && > + !memory_region_is_rom(mr) && > + (!mr->name || strcmp(mr->name, "pc.rom")) && > + (block->fd == -1 || !qemu_ram_is_shared(block))) { > + > + error_setg(errp, "Memory region %s is volatile", > + memory_region_name(mr)); > + ret = true; > + break; > + } > + } > + > + rcu_read_unlock(); > + return ret; > +} > + > /* Generate a debug exception if a watchpoint has been hit. */ > void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len, > MemTxAttrs attrs, int flags, uintptr_t ra) > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 307e527..6aafbb0 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -2519,6 +2519,8 @@ bool ram_block_discard_is_disabled(void); > */ > bool ram_block_discard_is_required(void); > > +bool qemu_ram_volatile(Error **errp); > + > #endif > > #endif > diff --git a/migration/savevm.c b/migration/savevm.c > index 1509173..f101039 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -2713,6 +2713,10 @@ void save_cpr_snapshot(const char *file, const char *mode, Error **errp) > return; > } > > + if (op == VMS_REBOOT && qemu_ram_volatile(errp)) { > + return; > + } > + > f = qf_file_open(file, O_CREAT | O_WRONLY | O_TRUNC, 0600, errp); > if (!f) { > return; > -- > 1.8.3.1 >
On 9/11/2020 1:35 PM, Dr. David Alan Gilbert wrote: > * Steve Sistare (steven.sistare@oracle.com) wrote: >> cprsave and cprload require that guest ram be backed by an externally >> visible shared file. Check that in cprsave. >> >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> >> --- >> exec.c | 32 ++++++++++++++++++++++++++++++++ >> include/exec/memory.h | 2 ++ >> migration/savevm.c | 4 ++++ >> 3 files changed, 38 insertions(+) >> >> diff --git a/exec.c b/exec.c >> index 6f381f9..02160e0 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -2726,6 +2726,38 @@ ram_addr_t qemu_ram_addr_from_host(void *ptr) >> return block->offset + offset; >> } >> >> +/* >> + * Return true if any memory regions are writable and not backed by shared >> + * memory. Exclude x86 option rom shadow "pc.rom" by name, even though it is >> + * writable. > > Tell me about 'pc.rom' - this is a very odd hack. > Again note the trick done by the existing migration capability > x-ignore-shared ; it doesn't special case, it just doesn't migrate > the 'shared' blocks. pc.rom is indeed a rom. Its contents do not change, and it can be recreated from scratch after a restart. However, the x86 arch code does not mark it readonly, so there is no proper way to tell it is not volatile, and its presence blocks cprsave reboot. Checking for the name "pc.rom" was the only way to recognize this anomaly, short of modifying the x86 code. However, I initially developed the above using an old version of qemu, and a more recent version has fixed it: pc_memory_init() memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE, &error_fatal); if (pcmc->pci_enabled) { memory_region_set_readonly(option_rom_mr, true); } Now memory_region_is_rom() will correctly classify this segment, and I will happily delete the hack. - Steve >> + */ >> +bool qemu_ram_volatile(Error **errp) >> +{ >> + RAMBlock *block; >> + MemoryRegion *mr; >> + bool ret = false; >> + >> + rcu_read_lock(); >> + QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { >> + mr = block->mr; >> + if (mr && >> + memory_region_is_ram(mr) && >> + !memory_region_is_ram_device(mr) && >> + !memory_region_is_rom(mr) && >> + (!mr->name || strcmp(mr->name, "pc.rom")) && >> + (block->fd == -1 || !qemu_ram_is_shared(block))) { >> + >> + error_setg(errp, "Memory region %s is volatile", >> + memory_region_name(mr)); >> + ret = true; >> + break; >> + } >> + } >> + >> + rcu_read_unlock(); >> + return ret; >> +} >> + >> /* Generate a debug exception if a watchpoint has been hit. */ >> void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len, >> MemTxAttrs attrs, int flags, uintptr_t ra) >> diff --git a/include/exec/memory.h b/include/exec/memory.h >> index 307e527..6aafbb0 100644 >> --- a/include/exec/memory.h >> +++ b/include/exec/memory.h >> @@ -2519,6 +2519,8 @@ bool ram_block_discard_is_disabled(void); >> */ >> bool ram_block_discard_is_required(void); >> >> +bool qemu_ram_volatile(Error **errp); >> + >> #endif >> >> #endif >> diff --git a/migration/savevm.c b/migration/savevm.c >> index 1509173..f101039 100644 >> --- a/migration/savevm.c >> +++ b/migration/savevm.c >> @@ -2713,6 +2713,10 @@ void save_cpr_snapshot(const char *file, const char *mode, Error **errp) >> return; >> } >> >> + if (op == VMS_REBOOT && qemu_ram_volatile(errp)) { >> + return; >> + } >> + >> f = qf_file_open(file, O_CREAT | O_WRONLY | O_TRUNC, 0600, errp); >> if (!f) { >> return; >> -- >> 1.8.3.1 >>
diff --git a/exec.c b/exec.c index 6f381f9..02160e0 100644 --- a/exec.c +++ b/exec.c @@ -2726,6 +2726,38 @@ ram_addr_t qemu_ram_addr_from_host(void *ptr) return block->offset + offset; } +/* + * Return true if any memory regions are writable and not backed by shared + * memory. Exclude x86 option rom shadow "pc.rom" by name, even though it is + * writable. + */ +bool qemu_ram_volatile(Error **errp) +{ + RAMBlock *block; + MemoryRegion *mr; + bool ret = false; + + rcu_read_lock(); + QLIST_FOREACH_RCU(block, &ram_list.blocks, next) { + mr = block->mr; + if (mr && + memory_region_is_ram(mr) && + !memory_region_is_ram_device(mr) && + !memory_region_is_rom(mr) && + (!mr->name || strcmp(mr->name, "pc.rom")) && + (block->fd == -1 || !qemu_ram_is_shared(block))) { + + error_setg(errp, "Memory region %s is volatile", + memory_region_name(mr)); + ret = true; + break; + } + } + + rcu_read_unlock(); + return ret; +} + /* Generate a debug exception if a watchpoint has been hit. */ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, vaddr len, MemTxAttrs attrs, int flags, uintptr_t ra) diff --git a/include/exec/memory.h b/include/exec/memory.h index 307e527..6aafbb0 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -2519,6 +2519,8 @@ bool ram_block_discard_is_disabled(void); */ bool ram_block_discard_is_required(void); +bool qemu_ram_volatile(Error **errp); + #endif #endif diff --git a/migration/savevm.c b/migration/savevm.c index 1509173..f101039 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -2713,6 +2713,10 @@ void save_cpr_snapshot(const char *file, const char *mode, Error **errp) return; } + if (op == VMS_REBOOT && qemu_ram_volatile(errp)) { + return; + } + f = qf_file_open(file, O_CREAT | O_WRONLY | O_TRUNC, 0600, errp); if (!f) { return;
cprsave and cprload require that guest ram be backed by an externally visible shared file. Check that in cprsave. Signed-off-by: Steve Sistare <steven.sistare@oracle.com> --- exec.c | 32 ++++++++++++++++++++++++++++++++ include/exec/memory.h | 2 ++ migration/savevm.c | 4 ++++ 3 files changed, 38 insertions(+)