| Message ID | 1739563953-227207-1-git-send-email-steven.sistare@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [V2] migration: ram block cpr blockers | expand |
On 14.02.25 21:12, Steve Sistare wrote: > Unlike cpr-reboot mode, cpr-transfer mode cannot save volatile ram blocks > in the migration stream file and recreate them later, because the physical > memory for the blocks is pinned and registered for vfio. Add a blocker > for volatile ram blocks. > > Also add a blocker for RAM_GUEST_MEMFD. Preserving guest_memfd may be > sufficient for CPR, but it has not been tested yet. > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > Reviewed-by: Fabiano Rosas <farosas@suse.de> > --- > include/exec/memory.h | 3 +++ > include/exec/ramblock.h | 1 + > migration/savevm.c | 2 ++ > system/physmem.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 74 insertions(+) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 9f73b59..ea5d33a 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -3184,6 +3184,9 @@ bool ram_block_discard_is_disabled(void); > */ > bool ram_block_discard_is_required(void); > > +void ram_block_add_cpr_blocker(RAMBlock *rb, Error **errp); > +void ram_block_del_cpr_blocker(RAMBlock *rb); > + > #endif > > #endif > diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h > index 0babd10..64484cd 100644 > --- a/include/exec/ramblock.h > +++ b/include/exec/ramblock.h > @@ -39,6 +39,7 @@ struct RAMBlock { > /* RCU-enabled, writes protected by the ramlist lock */ > QLIST_ENTRY(RAMBlock) next; > QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers; > + Error *cpr_blocker; > int fd; > uint64_t fd_offset; > int guest_memfd; > diff --git a/migration/savevm.c b/migration/savevm.c > index bc375db..85a3559 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -3315,12 +3315,14 @@ void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev) > qemu_ram_set_idstr(mr->ram_block, > memory_region_name(mr), dev); > qemu_ram_set_migratable(mr->ram_block); > + ram_block_add_cpr_blocker(mr->ram_block, &error_fatal); > } > > void vmstate_unregister_ram(MemoryRegion *mr, DeviceState *dev) > { > qemu_ram_unset_idstr(mr->ram_block); > qemu_ram_unset_migratable(mr->ram_block); > + ram_block_del_cpr_blocker(mr->ram_block); > } > > void vmstate_register_ram_global(MemoryRegion *mr) > diff --git a/system/physmem.c b/system/physmem.c > index 67c9db9..c416068 100644 > --- a/system/physmem.c > +++ b/system/physmem.c > @@ -70,7 +70,10 @@ > > #include "qemu/pmem.h" > > +#include "qapi/qapi-types-migration.h" > +#include "migration/blocker.h" > #include "migration/cpr.h" > +#include "migration/options.h" > #include "migration/vmstate.h" > > #include "qemu/range.h" > @@ -1899,6 +1902,14 @@ static void ram_block_add(RAMBlock *new_block, Error **errp) > qemu_mutex_unlock_ramlist(); > goto out_free; > } > + > + error_setg(&new_block->cpr_blocker, > + "Memory region %s uses guest_memfd, " > + "which is not supported with CPR.", > + memory_region_name(new_block->mr)); > + migrate_add_blocker_modes(&new_block->cpr_blocker, errp, > + MIG_MODE_CPR_TRANSFER, > + -1); > } > > ram_size = (new_block->offset + new_block->max_length) >> TARGET_PAGE_BITS; > @@ -4059,3 +4070,60 @@ bool ram_block_discard_is_required(void) > return qatomic_read(&ram_block_discard_required_cnt) || > qatomic_read(&ram_block_coordinated_discard_required_cnt); > } > + > +/* > + * Return true if ram contents would be lost during CPR. Do not exclude rom, > + * because the rom file could change in new QEMU. > + */ > +static bool ram_is_volatile(RAMBlock *rb) Can we call this ram_is_cpr_compatible() / ram_is_cpr_incompatible() or sth. instead? Talking about RAM and "volatile" is misleading, and the function is specific to CPR already (e.g., comment :) ).
On 2/14/2025 4:21 PM, David Hildenbrand wrote: > On 14.02.25 21:12, Steve Sistare wrote: >> Unlike cpr-reboot mode, cpr-transfer mode cannot save volatile ram blocks >> in the migration stream file and recreate them later, because the physical >> memory for the blocks is pinned and registered for vfio. Add a blocker >> for volatile ram blocks. >> >> Also add a blocker for RAM_GUEST_MEMFD. Preserving guest_memfd may be >> sufficient for CPR, but it has not been tested yet. >> >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> >> Reviewed-by: Fabiano Rosas <farosas@suse.de> >> --- >> include/exec/memory.h | 3 +++ >> include/exec/ramblock.h | 1 + >> migration/savevm.c | 2 ++ >> system/physmem.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 74 insertions(+) >> >> diff --git a/include/exec/memory.h b/include/exec/memory.h >> index 9f73b59..ea5d33a 100644 >> --- a/include/exec/memory.h >> +++ b/include/exec/memory.h >> @@ -3184,6 +3184,9 @@ bool ram_block_discard_is_disabled(void); >> */ >> bool ram_block_discard_is_required(void); >> +void ram_block_add_cpr_blocker(RAMBlock *rb, Error **errp); >> +void ram_block_del_cpr_blocker(RAMBlock *rb); >> + >> #endif >> #endif >> diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h >> index 0babd10..64484cd 100644 >> --- a/include/exec/ramblock.h >> +++ b/include/exec/ramblock.h >> @@ -39,6 +39,7 @@ struct RAMBlock { >> /* RCU-enabled, writes protected by the ramlist lock */ >> QLIST_ENTRY(RAMBlock) next; >> QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers; >> + Error *cpr_blocker; >> int fd; >> uint64_t fd_offset; >> int guest_memfd; >> diff --git a/migration/savevm.c b/migration/savevm.c >> index bc375db..85a3559 100644 >> --- a/migration/savevm.c >> +++ b/migration/savevm.c >> @@ -3315,12 +3315,14 @@ void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev) >> qemu_ram_set_idstr(mr->ram_block, >> memory_region_name(mr), dev); >> qemu_ram_set_migratable(mr->ram_block); >> + ram_block_add_cpr_blocker(mr->ram_block, &error_fatal); >> } >> void vmstate_unregister_ram(MemoryRegion *mr, DeviceState *dev) >> { >> qemu_ram_unset_idstr(mr->ram_block); >> qemu_ram_unset_migratable(mr->ram_block); >> + ram_block_del_cpr_blocker(mr->ram_block); >> } >> void vmstate_register_ram_global(MemoryRegion *mr) >> diff --git a/system/physmem.c b/system/physmem.c >> index 67c9db9..c416068 100644 >> --- a/system/physmem.c >> +++ b/system/physmem.c >> @@ -70,7 +70,10 @@ >> #include "qemu/pmem.h" >> +#include "qapi/qapi-types-migration.h" >> +#include "migration/blocker.h" >> #include "migration/cpr.h" >> +#include "migration/options.h" >> #include "migration/vmstate.h" >> #include "qemu/range.h" >> @@ -1899,6 +1902,14 @@ static void ram_block_add(RAMBlock *new_block, Error **errp) >> qemu_mutex_unlock_ramlist(); >> goto out_free; >> } >> + >> + error_setg(&new_block->cpr_blocker, >> + "Memory region %s uses guest_memfd, " >> + "which is not supported with CPR.", >> + memory_region_name(new_block->mr)); >> + migrate_add_blocker_modes(&new_block->cpr_blocker, errp, >> + MIG_MODE_CPR_TRANSFER, >> + -1); >> } >> ram_size = (new_block->offset + new_block->max_length) >> TARGET_PAGE_BITS; >> @@ -4059,3 +4070,60 @@ bool ram_block_discard_is_required(void) >> return qatomic_read(&ram_block_discard_required_cnt) || >> qatomic_read(&ram_block_coordinated_discard_required_cnt); >> } >> + >> +/* >> + * Return true if ram contents would be lost during CPR. Do not exclude rom, >> + * because the rom file could change in new QEMU. >> + */ >> +static bool ram_is_volatile(RAMBlock *rb) > > Can we call this > > ram_is_cpr_compatible() / ram_is_cpr_incompatible() or sth. instead? > > Talking about RAM and "volatile" is misleading, and the function is specific to CPR already (e.g., comment :) ). Will do, thanks - steve
On Fri, Feb 14, 2025 at 04:44:40PM -0500, Steven Sistare wrote: > On 2/14/2025 4:21 PM, David Hildenbrand wrote: > > On 14.02.25 21:12, Steve Sistare wrote: > > > Unlike cpr-reboot mode, cpr-transfer mode cannot save volatile ram blocks > > > in the migration stream file and recreate them later, because the physical > > > memory for the blocks is pinned and registered for vfio. Add a blocker > > > for volatile ram blocks. > > > > > > Also add a blocker for RAM_GUEST_MEMFD. Preserving guest_memfd may be > > > sufficient for CPR, but it has not been tested yet. > > > > > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > > > Reviewed-by: Fabiano Rosas <farosas@suse.de> > > > --- > > > include/exec/memory.h | 3 +++ > > > include/exec/ramblock.h | 1 + > > > migration/savevm.c | 2 ++ > > > system/physmem.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ > > > 4 files changed, 74 insertions(+) > > > > > > diff --git a/include/exec/memory.h b/include/exec/memory.h > > > index 9f73b59..ea5d33a 100644 > > > --- a/include/exec/memory.h > > > +++ b/include/exec/memory.h > > > @@ -3184,6 +3184,9 @@ bool ram_block_discard_is_disabled(void); > > > */ > > > bool ram_block_discard_is_required(void); > > > +void ram_block_add_cpr_blocker(RAMBlock *rb, Error **errp); > > > +void ram_block_del_cpr_blocker(RAMBlock *rb); > > > + > > > #endif > > > #endif > > > diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h > > > index 0babd10..64484cd 100644 > > > --- a/include/exec/ramblock.h > > > +++ b/include/exec/ramblock.h > > > @@ -39,6 +39,7 @@ struct RAMBlock { > > > /* RCU-enabled, writes protected by the ramlist lock */ > > > QLIST_ENTRY(RAMBlock) next; > > > QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers; > > > + Error *cpr_blocker; > > > int fd; > > > uint64_t fd_offset; > > > int guest_memfd; > > > diff --git a/migration/savevm.c b/migration/savevm.c > > > index bc375db..85a3559 100644 > > > --- a/migration/savevm.c > > > +++ b/migration/savevm.c > > > @@ -3315,12 +3315,14 @@ void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev) > > > qemu_ram_set_idstr(mr->ram_block, > > > memory_region_name(mr), dev); > > > qemu_ram_set_migratable(mr->ram_block); > > > + ram_block_add_cpr_blocker(mr->ram_block, &error_fatal); > > > } > > > void vmstate_unregister_ram(MemoryRegion *mr, DeviceState *dev) > > > { > > > qemu_ram_unset_idstr(mr->ram_block); > > > qemu_ram_unset_migratable(mr->ram_block); > > > + ram_block_del_cpr_blocker(mr->ram_block); > > > } > > > void vmstate_register_ram_global(MemoryRegion *mr) > > > diff --git a/system/physmem.c b/system/physmem.c > > > index 67c9db9..c416068 100644 > > > --- a/system/physmem.c > > > +++ b/system/physmem.c > > > @@ -70,7 +70,10 @@ > > > #include "qemu/pmem.h" > > > +#include "qapi/qapi-types-migration.h" > > > +#include "migration/blocker.h" > > > #include "migration/cpr.h" > > > +#include "migration/options.h" > > > #include "migration/vmstate.h" > > > #include "qemu/range.h" > > > @@ -1899,6 +1902,14 @@ static void ram_block_add(RAMBlock *new_block, Error **errp) > > > qemu_mutex_unlock_ramlist(); > > > goto out_free; > > > } > > > + > > > + error_setg(&new_block->cpr_blocker, > > > + "Memory region %s uses guest_memfd, " > > > + "which is not supported with CPR.", > > > + memory_region_name(new_block->mr)); > > > + migrate_add_blocker_modes(&new_block->cpr_blocker, errp, > > > + MIG_MODE_CPR_TRANSFER, > > > + -1); > > > } > > > ram_size = (new_block->offset + new_block->max_length) >> TARGET_PAGE_BITS; > > > @@ -4059,3 +4070,60 @@ bool ram_block_discard_is_required(void) > > > return qatomic_read(&ram_block_discard_required_cnt) || > > > qatomic_read(&ram_block_coordinated_discard_required_cnt); > > > } > > > + > > > +/* > > > + * Return true if ram contents would be lost during CPR. Do not exclude rom, > > > + * because the rom file could change in new QEMU. > > > + */ > > > +static bool ram_is_volatile(RAMBlock *rb) > > > > Can we call this > > > > ram_is_cpr_compatible() / ram_is_cpr_incompatible() or sth. instead? > > > > Talking about RAM and "volatile" is misleading, and the function is specific to CPR already (e.g., comment :) ). > > Will do, thanks - steve With that, feel free to take: Reviewed-by: Peter Xu <peterx@redhat.com>
diff --git a/include/exec/memory.h b/include/exec/memory.h index 9f73b59..ea5d33a 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -3184,6 +3184,9 @@ bool ram_block_discard_is_disabled(void); */ bool ram_block_discard_is_required(void); +void ram_block_add_cpr_blocker(RAMBlock *rb, Error **errp); +void ram_block_del_cpr_blocker(RAMBlock *rb); + #endif #endif diff --git a/include/exec/ramblock.h b/include/exec/ramblock.h index 0babd10..64484cd 100644 --- a/include/exec/ramblock.h +++ b/include/exec/ramblock.h @@ -39,6 +39,7 @@ struct RAMBlock { /* RCU-enabled, writes protected by the ramlist lock */ QLIST_ENTRY(RAMBlock) next; QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers; + Error *cpr_blocker; int fd; uint64_t fd_offset; int guest_memfd; diff --git a/migration/savevm.c b/migration/savevm.c index bc375db..85a3559 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -3315,12 +3315,14 @@ void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev) qemu_ram_set_idstr(mr->ram_block, memory_region_name(mr), dev); qemu_ram_set_migratable(mr->ram_block); + ram_block_add_cpr_blocker(mr->ram_block, &error_fatal); } void vmstate_unregister_ram(MemoryRegion *mr, DeviceState *dev) { qemu_ram_unset_idstr(mr->ram_block); qemu_ram_unset_migratable(mr->ram_block); + ram_block_del_cpr_blocker(mr->ram_block); } void vmstate_register_ram_global(MemoryRegion *mr) diff --git a/system/physmem.c b/system/physmem.c index 67c9db9..c416068 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -70,7 +70,10 @@ #include "qemu/pmem.h" +#include "qapi/qapi-types-migration.h" +#include "migration/blocker.h" #include "migration/cpr.h" +#include "migration/options.h" #include "migration/vmstate.h" #include "qemu/range.h" @@ -1899,6 +1902,14 @@ static void ram_block_add(RAMBlock *new_block, Error **errp) qemu_mutex_unlock_ramlist(); goto out_free; } + + error_setg(&new_block->cpr_blocker, + "Memory region %s uses guest_memfd, " + "which is not supported with CPR.", + memory_region_name(new_block->mr)); + migrate_add_blocker_modes(&new_block->cpr_blocker, errp, + MIG_MODE_CPR_TRANSFER, + -1); } ram_size = (new_block->offset + new_block->max_length) >> TARGET_PAGE_BITS; @@ -4059,3 +4070,60 @@ bool ram_block_discard_is_required(void) return qatomic_read(&ram_block_discard_required_cnt) || qatomic_read(&ram_block_coordinated_discard_required_cnt); } + +/* + * Return true if ram contents would be lost during CPR. Do not exclude rom, + * because the rom file could change in new QEMU. + */ +static bool ram_is_volatile(RAMBlock *rb) +{ + MemoryRegion *mr = rb->mr; + + if (!mr || !memory_region_is_ram(mr)) { + return false; + } + + /* Ram device is remapped in new QEMU */ + if (memory_region_is_ram_device(mr)) { + return false; + } + + /* Named files are remapped in new QEMU, same contents if shared (no COW) */ + if (qemu_ram_is_shared(rb) && qemu_ram_is_named_file(rb)) { + return false; + } + + /* A file descriptor is remapped in new QEMU */ + if (rb->fd >= 0) { + return false; + } + + return true; +} + +/* + * Add a blocker for each volatile ram block. This function should only be + * called after we know that the block is migratable. Non-migratable blocks + * are either re-created in new QEMU, or are handled specially, or are covered + * by a device-level CPR blocker. + */ +void ram_block_add_cpr_blocker(RAMBlock *rb, Error **errp) +{ + assert(qemu_ram_is_migratable(rb)); + + if (!ram_is_volatile(rb)) { + return; + } + + error_setg(&rb->cpr_blocker, + "Memory region %s is volatile. share=on is required for " + "memory-backend objects, and aux-ram-share=on is required.", + memory_region_name(rb->mr)); + migrate_add_blocker_modes(&rb->cpr_blocker, errp, MIG_MODE_CPR_TRANSFER, + -1); +} + +void ram_block_del_cpr_blocker(RAMBlock *rb) +{ + migrate_del_blocker(&rb->cpr_blocker); +}