Background snapshots
cover Background snapshots
The workflow to make a snapshot is the following:
1. Pause the vm
2. Make a snapshot of block devices using the scheme of your choice
3. Turn on background-snapshot migration capability
4. Start the migration using the destination (migration stream) of your choice. The migration will resume the vm execution by itself
when it has the devices' states saved and is ready to start ram writing to the migration stream.
5. Listen to the migration finish event
The bakground snapshot works with support of KVM patch:
"x86: mmu: report failed memory access to the userspace" (not applied to the mainstream, it's in the kvm mailing list)
--
Change log:
v0 => v1:
============
the patch series has been split in smaller chunks
Denis Plotnikov (17):
migration: add background snapshot capability
bitops: add some atomic versions of bitmap operations
threads: add infrastructure to process sigsegv
background snapshot: make a dedicated type for ram block list
ram: extend the data structures for background snapshotting background snapshot: add helpers to manage a copy of ram block list background snapshot: introduce page buffer
migration: add helpers to change VM memory protection rights background snapshot: extend RAM request for holding a page copy
pointer
background snapshots: adapt the page queueing code for using page
copies
background snapshot: add a memory page copying function
ram: add background snapshot support in ram page saving part of
migration
background snapshot: add write-protected page access handler function
kvm: add failed memeory access exit reason
kvm: add vCPU failed memeory access processing
migration: move the device state saving logic to a separate function background snapshot: enable background snapshot
include/exec/ram_addr.h | 7 + include/exec/ramlist.h | 4 +- include/qemu/bitops.h | 25 +++ include/qemu/thread.h | 5 + linux-headers/linux/kvm.h | 5 + migration/migration.c | 140 +++++++++++++- migration/migration.h | 1 + migration/ram.c | 374 ++++++++++++++++++++++++++++++++++++-- migration/ram.h | 17 +- migration/savevm.c | 91 +++++----- migration/savevm.h | 2 + qapi/migration.json | 6 +- target/i386/kvm.c | 17 ++ util/qemu-thread-posix.c | 52 ++++++ 14 files changed, 684 insertions(+), 62 deletions(-)
Signature · 1 line
2.17.0
PATCH 1/17 migration: add background snapshot capability
The capability is used for the background vmstate saving using the migration infrastructure.
Background vmstate saving means that the majority of vmstate (RAM) is saved in the background when VM's vCPUS are running. This helps to reduce the VM downtime on VM snapshotting.
--- migration/migration.c | 35 +++++++++++++++++++++++++++++++++++ migration/migration.h | 1 + qapi/migration.json | 6 +++++- 3 files changed, 41 insertions(+), 1 deletion(-)
migration/migration.c +35 -0
@@ -697,6 +697,12 @@ static bool migrate_caps_check(bool *cap_list,return false; } +if (cap_list[MIGRATION_CAPABILITY_BACKGROUND_SNAPSHOT ]) { +error_setg (errp,"Postcopy is not compatible " +"with background snapshot" ); +return false; + } +/* This check is reasonably expensive, so only when it's being * set the first time, also it's only the destination that needs * special support.
@@ -711,6 +717,26 @@ static bool migrate_caps_check(bool *cap_list, } } +if (cap_list[MIGRATION_CAPABILITY_BACKGROUND_SNAPSHOT ]) { +if (cap_list[MIGRATION_CAPABILITY_RELEASE_RAM ]) { +error_setg (errp,"Background snapshot is not compatible " +"with release ram capability" ); +return false; + } + +if (cap_list[MIGRATION_CAPABILITY_COMPRESS ]) { +error_setg (errp,"Background snapshot is not " +"currently compatible with compression" ); +return false; + } + +if (cap_list[MIGRATION_CAPABILITY_XBZRLE ]) { +error_setg (errp,"Background snapshot is not " +"currently compatible with XBZLRE" ); +return false; + } + } +return true; }
@@ -1635,6 +1661,15 @@ bool migrate_use_block_incremental(void)return s->parameters.block_incremental; } +bool migrate_background_snapshot (void ) +{ +MigrationState *s; + + s =migrate_get_current (); + +return s->enabled_capabilities[MIGRATION_CAPABILITY_BACKGROUND_SNAPSHOT ]; +} +/* migration thread support */ /* * Something bad happened to the RP stream, mark an error
migration/migration.h +1 -0
@@ -201,6 +201,7 @@ int migrate_compress_level(void);int migrate_compress_threads (void );int migrate_decompress_threads (void );bool migrate_use_events (void ); +bool migrate_background_snapshot (void );/* Sending on the return path - generic and then for each message type */ void migrate_send_rp_shut(MigrationIncomingState *mis,
qapi/migration.json +5 -1
@@ -352,12 +352,16 @@ # # @x-multifd: Use more than one fd for migration (since 2.11) # +# @background-snapshot: Using migration infrastructure makes VM snapshot +# saving its RAM in background. This reduces VM downtime. (since 2.12) +# # Since: 1.2 ## { 'enum': 'MigrationCapability', 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks', 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram', - 'block', 'return-path', 'pause-before-switchover', 'x-multifd' ] } + 'block', 'return-path', 'pause-before-switchover', 'x-multifd', + 'background-snapshot' ] } ## # @MigrationCapabilityStatus:
Signature · 1 line
2.17.0
Re: [Qemu-devel] [PATCH v1 01/17] migration: add background snapshot capability
Quoted 90 lines · Denis Plotnikov
The capability is used for the background vmstate saving using the migration infrastructure.
Background vmstate saving means that the majority of vmstate (RAM) is saved in the background when VM's vCPUS are running. This helps to reduce the VM downtime on VM snapshotting.Signed-off-by Denis Plotnikov <[email protected]>--- migration/migration.c | 35 +++++++++++++++++++++++++++++++++++ migration/migration.h | 1 + qapi/migration.json | 6 +++++- 3 files changed, 41 insertions(+), 1 deletion(-)migration/migration.c +35
-0@@ -697,6 +697,12 @@ static bool migrate_caps_check(bool *cap_list,return false; } +if (cap_list[MIGRATION_CAPABILITY_BACKGROUND_SNAPSHOT ]) { +error_setg (errp,"Postcopy is not compatible " +"with background snapshot" ); +return false; + } +/* This check is reasonably expensive, so only when it's being * set the first time, also it's only the destination that needs * special support. @@ -711,6 +717,26 @@ static bool migrate_caps_check(bool *cap_list, } } +if (cap_list[MIGRATION_CAPABILITY_BACKGROUND_SNAPSHOT ]) { +if (cap_list[MIGRATION_CAPABILITY_RELEASE_RAM ]) { +error_setg (errp,"Background snapshot is not compatible " +"with release ram capability" ); +return false; + } + +if (cap_list[MIGRATION_CAPABILITY_COMPRESS ]) { +error_setg (errp,"Background snapshot is not " +"currently compatible with compression" ); +return false; + } + +if (cap_list[MIGRATION_CAPABILITY_XBZRLE ]) { +error_setg (errp,"Background snapshot is not " +"currently compatible with XBZLRE" ); +return false; + } + } +return true; }@@ -1635,6 +1661,15 @@ bool migrate_use_block_incremental(void)return s->parameters.block_incremental; } +bool migrate_background_snapshot (void ) +{ +MigrationState *s; + + s =migrate_get_current (); + +return s->enabled_capabilities[MIGRATION_CAPABILITY_BACKGROUND_SNAPSHOT ]; +} +/* migration thread support */ /* * Something bad happened to the RP stream, mark an error migration/migration.h +1
-0@@ -201,6 +201,7 @@ int migrate_compress_level(void);int migrate_compress_threads (void );int migrate_decompress_threads (void );bool migrate_use_events (void ); +bool migrate_background_snapshot (void );/* Sending on the return path - generic and then for each message type */ void migrate_send_rp_shut(MigrationIncomingState *mis,qapi/migration.json +0
-0
qapi/migration.json line 357
Quoted 6 lines
qapi/migration.json +3
-0@@ -352,12 +352,16 @@ # # @x-multifd: Use more than one fd for migration (since 2.11) # +# @background-snapshot: Using migration infrastructure makes VM snapshot +# saving its RAM in background. This reduces VM downtime. (since 2.12) +#
Please specify "(since 3.1)" in your next post.
The sentence is not that clear to me. For example, "saving its RAM in background" is done in every migration, not only for the live snapshot. The words "reduces VM downtime" sounds also like for a live migration rather than snapshots. My try:
If enabled, the migration stream will be a snapshot of the VM exactly at the point when the migration procedure starts.
Quoted 15 lines
qapi/migration.json +2
-1# Since: 1.2 ## { 'enum': 'MigrationCapability', 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks', 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram', - 'block', 'return-path', 'pause-before-switchover', 'x-multifd' ] } + 'block', 'return-path', 'pause-before-switchover', 'x-multifd', + 'background-snapshot' ] } ## # @MigrationCapabilityStatus:Signature · 1 line
2.17.0
Regards,
Signature · 1 line
Peter Xu
PATCH 2/17 bitops: add some atomic versions of bitmap operations
In reply to Denis Plotnikov1. test bit
2. test and set bit
--- include/qemu/bitops.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
include/qemu/bitops.h +25 -0
@@ -94,6 +94,21 @@ static inline int test_and_set_bit(long nr, unsigned long *addr)return (old & mask) !=0 ; } +/** +* test_and_set_bit_atomic - Set a bit atomically and return its old value +* @nr: Bit to set +* @addr: Address to count from +*/ +static inline int test_and_set_bit_atomic (long nr,unsigned long *addr) +{ +unsigned long mask =BIT_MASK (nr); +unsigned long *p = addr +BIT_WORD (nr); +unsigned long old; + + old =atomic_fetch_or (p, mask); +return (old & mask) !=0 ; +} +/** * test_and_clear_bit - Clear a bit and return its old value * @nr: Bit to clear
@@ -134,6 +149,16 @@ static inline int test_bit(long nr, const unsigned long *addr)return 1UL & (addr[BIT_WORD (nr)] >> (nr & (BITS_PER_LONG -1 ))); } +/** +* test_bit_atomic - Determine whether a bit is set atomicallly +* @nr: bit number to test +* @addr: Address to start counting from +*/ +static inline int test_bit_atomic (long nr,const unsigned long *addr) +{ +long valid_nr = nr & (BITS_PER_LONG -1 ); +return 1UL & (atomic_read (&addr[BIT_WORD (nr)]) >> valid_nr); +}/** * find_last_bit - find the last set bit in a memory region * @addr: The address to start the search at
Signature · 1 line
2.17.0
Re: [Qemu-devel] [PATCH v1 02/17] bitops: add some atomic versions of bitmap operations
Quoted 4 lines · Denis Plotnikov
1. test bit
2. test and set bitSigned-off-by Denis Plotnikov <[email protected]>
Regards,
Signature · 1 line
Peter Xu
PATCH 3/17 threads: add infrastructure to process sigsegv
In reply to Denis PlotnikovAllows to define sigsegv handler temporary for all threads. This is useful to implement copy-on-write logic while linux usefaultfd doesn't support write-protected faults. In the future, switch to using WP userfaultfd when it's available.
--- include/qemu/thread.h | 5 ++++ util/qemu-thread-posix.c | 52 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+)
include/qemu/thread.h +5 -0
@@ -210,4 +210,9 @@ void qemu_lockcnt_inc_and_unlock(QemuLockCnt *lockcnt);*/ unsigned qemu_lockcnt_count (QemuLockCnt *lockcnt); + +typedef void (*sigsegv_handler )(int signum,siginfo_t *siginfo,void *sigctx); +void sigsegv_user_handler_set (sigsegv_handler handler); +void sigsegv_user_handler_reset (void ); +#endif
util/qemu-thread-posix.c +52 -0
72 lines not on this page · raw message
Signature · 1 line
2.17.0
Re: [Qemu-devel] [PATCH v1 03/17] threads: add infrastructure to process sigsegv
Quoted 31 lines · Denis Plotnikov
Allows to define sigsegv handler temporary for all threads. This is useful to implement copy-on-write logic while linux usefaultfd doesn't support write-protected faults. In the future, switch to using WP userfaultfd when it's available.
Signed-off-by Denis Plotnikov <[email protected]>--- include/qemu/thread.h | 5 ++++ util/qemu-thread-posix.c | 52 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+)include/qemu/thread.h +5
-0@@ -210,4 +210,9 @@ void qemu_lockcnt_inc_and_unlock(QemuLockCnt *lockcnt);*/ unsigned qemu_lockcnt_count (QemuLockCnt *lockcnt); + +typedef void (*sigsegv_handler )(int signum,siginfo_t *siginfo,void *sigctx); +void sigsegv_user_handler_set (sigsegv_handler handler); +void sigsegv_user_handler_reset (void ); +#endif util/qemu-thread-posix.c +0
-0
util/qemu-thread-posix.c static void qemu_thread_set_name(QemuThread *thread, const char *name) · line 496
Quoted 8 lines
util/qemu-thread-posix.c +5
-0@@ -489,6 +489,47 @@ static void qemu_thread_set_name(QemuThread *thread, const char *name)#endif } +static sigsegv_handler sigsegv_user_handler; + +void sigsegv_user_handler_set (sigsegv_handler handler) +{ +assert (handler);
We can also assert !sigsegv_user_handler here to make sure there won't accidentally be two users that want to specify a SIGSEGV handler in parallel.
Quoted 17 lines
util/qemu-thread-posix.c +17
-0+atomic_set (&sigsegv_user_handler, handler); +} + +static sigsegv_handler sigsegv_user_handler_get (void ) +{ +return atomic_read (&sigsegv_user_handler); +} + +void sigsegv_user_handler_reset (void ) +{ +atomic_set (&sigsegv_user_handler,NULL ); +} + +static void sigsegv_default_handler (int signum,siginfo_t *siginfo,void *sigctx) +{ +sigsegv_handler handler =sigsegv_user_handler_get (); +
util/qemu-thread-posix.c static void qemu_thread_set_name(QemuThread *thread, const char *name) · line 523
Quoted 10 lines
util/qemu-thread-posix.c +10
-0+if (!handler) { +/* +* remove the sigsegv handler if it's not set by user +* this will lead to re-raising the error without a handler +* and exiting from the program with "Segmentation fault" +*/ +int err; +struct sigaction act; +memset (&act,0 ,sizeof (act)); + act.sa_flags =SA_RESETHAND ;
I'm not sure whether this is the correct way to use SA_RESETHAND.
SA_RESETHAND
Restore the signal action to the default upon entry to the signal handler. This flag is meaningful only when establishing a signal han‐ dler. SA_ONESHOT is an obsolete, nonstandard synonym for this flag.
My understanding is that it's used as a "oneshot" pattern, but here you didn't really specify any handler. My wild guess is that the handler is actually specified to SIG_DFL (which is, 0) after you bzero-ed the sigaction, hence got what we want.
I'm not sure whether we need this complexity - could we just unset the handler outside itself when reset?
util/qemu-thread-posix.c static void qemu_thread_set_name(QemuThread *thread, const char *name) · line 529
Quoted 6 lines
util/qemu-thread-posix.c +6
-0+ err =sigaction (SIGSEGV , &act,NULL ); +if (err) { +error_exit (err, __func__); + } + }else { +handler (signum, siginfo, sigctx);
Here not sure whether we can return something from the user handler showing that whether it correctly handled the SIGSEGV signal, otherwise how could we identify it's an expected event or a real SIGSEGV fault?
Quoted 19 lines
util/qemu-thread-posix.c +6
-0+ } +} +void qemu_thread_create (QemuThread *thread,const char *name,void *(*start_routine)(void *),void *arg,int mode)@@ -496,14 +537,25 @@ void qemu_thread_create(QemuThread *thread, const char *name,sigset_t set, oldset;int err;pthread_attr_t attr; +struct sigaction act; err =pthread_attr_init (&attr);if (err) {error_exit (err, __func__); } +memset (&act,0 ,sizeof (act)); + act.sa_flags =SA_SIGINFO ;
util/qemu-thread-posix.c void qemu_thread_create(QemuThread *thread, const char *name, · line 558
Quoted 10 lines
util/qemu-thread-posix.c +8
-0+ act.sa_sigaction = sigsegv_default_handler; + err =sigaction (SIGSEGV , &act,NULL ); +if (err) { +error_exit (err, __func__); + } +/* Leave signal handling to the iothread. */ sigfillset (&set); +/* ...all but SIGSEGV */ +sigdelset (&set,SIGSEGV );
Why we need to enable SIGSEGV on all threads explicitly? Could we just keep the old behavior just like what the other signals do?
Quoted 7 lines
util/qemu-thread-posix.c +0
-0pthread_sigmask (SIG_SETMASK , &set, &oldset); err =pthread_create (&thread->thread, &attr, start_routine, arg);if (err)Signature · 1 line
2.17.0
Regards,
Signature · 1 line
Peter Xu
PATCH 4/17 background snapshot: make a dedicated type for ram block list
In reply to Denis PlotnikovThe type will be used later to hold a list of ram blocks which memory content to be used for the vm state saving when making a background snapshot.
--- include/exec/ramlist.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
include/exec/ramlist.h +3 -1
@@ -44,11 +44,13 @@ typedef struct {unsigned long *blocks[]; } DirtyMemoryBlocks; +typedef QLIST_HEAD(, RAMBlock) RamBlockList; + typedef struct RAMList { QemuMutex mutex; RAMBlock *mru_block; /* RCU-enabled, writes protected by the ramlist lock. */ - QLIST_HEAD(, RAMBlock) blocks; + RamBlockList blocks; DirtyMemoryBlocks *dirty_memory[DIRTY_MEMORY_NUM]; uint32_t version; QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers;
Signature · 1 line
2.17.0
PATCH 5/17 ram: extend the data structures for background snapshotting
In reply to Denis Plotnikov--- include/exec/ram_addr.h | 7 +++++++ 1 file changed, 7 insertions(+)
include/exec/ram_addr.h +7 -0
@@ -36,6 +36,8 @@ struct RAMBlock {char idstr[256 ];/* RCU-enabled, writes protected by the ramlist lock */ QLIST_ENTRY (RAMBlock) next; +/* blocks used for background snapshot */ +QLIST_ENTRY (RAMBlock) bgs_next; QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers;int fd;size_t page_size;
@@ -49,6 +51,11 @@ struct RAMBlock {unsigned long *unsentmap;/* bitmap of already received pages in postcopy */ unsigned long *receivedmap; +/* The following 2 are for background snapshot */ +/* Pages currently being copied */ +unsigned long *touched_map; +/* Pages has been copied already */ +unsigned long *copied_map; };static inline bool offset_in_ramblock (RAMBlock *b,ram_addr_t offset)
Signature · 1 line
2.17.0
Re: [Qemu-devel] [PATCH v1 05/17] ram: extend the data structures for background snapshotting
Quoted 34 lines · Denis Plotnikov
Signed-off-by Denis Plotnikov <[email protected]>--- include/exec/ram_addr.h | 7 +++++++ 1 file changed, 7 insertions(+)include/exec/ram_addr.h +7
-0@@ -36,6 +36,8 @@ struct RAMBlock {char idstr[256 ];/* RCU-enabled, writes protected by the ramlist lock */ QLIST_ENTRY (RAMBlock) next; +/* blocks used for background snapshot */ +QLIST_ENTRY (RAMBlock) bgs_next; QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers;int fd;size_t page_size;@@ -49,6 +51,11 @@ struct RAMBlock {unsigned long *unsentmap;/* bitmap of already received pages in postcopy */ unsigned long *receivedmap; +/* The following 2 are for background snapshot */ +/* Pages currently being copied */ +unsigned long *touched_map; +/* Pages has been copied already */ +unsigned long *copied_map; };static inline bool offset_in_ramblock (RAMBlock *b,ram_addr_t offset)Signature · 1 line
2.17.0
IMHO these new fields can be put into the patches where they are firstly used. Isolating them do not really help much for anything AFAICT...
Regards,
Signature · 1 line
Peter Xu
PATCH 6/17 background snapshot: add helpers to manage a copy of ram block list
In reply to Denis PlotnikovThe VM ramblock list may be changed during the snapshotting. We want to make sure that we have the same ramblock list as it was at the time of snapshot beginning.
So, we create a copy of the list at the beginning of the snapshotting and use it during the process.
--- migration/ram.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ migration/ram.h | 6 ++++++ 2 files changed, 57 insertions(+)
migration/ram.c +51 -0
@@ -1508,6 +1508,57 @@ static int ram_save_host_page(RAMState *rs, PageSearchStatus *pss,return pages; } +/* BackGround Snapshot Blocks */ +static RamBlockList bgs_blocks; + +static RamBlockList *ram_bgs_block_list_get (void ) +{ +return &bgs_blocks; +} + +void ram_block_list_create (void ) +{ +RAMBlock *block =NULL ; +RamBlockList *block_list =ram_bgs_block_list_get (); + +qemu_mutex_lock_ramlist (); + QLIST_FOREACH(block, &ram_list.blocks, next) { +memory_region_ref (block->mr); +QLIST_INSERT_HEAD (block_list, block, bgs_next); + } +qemu_mutex_unlock_ramlist (); +} + +void ram_block_list_destroy (void ) +{ +RAMBlock *next, *block; +RamBlockList *block_list =ram_bgs_block_list_get (); + + QLIST_FOREACH_SAFE(block, block_list, bgs_next, next) { +QLIST_REMOVE (block, bgs_next); +memory_region_unref (block->mr); + } +} + +RAMBlock *ram_bgs_block_find (uint8_t *address,ram_addr_t *page_offset) +{ +RAMBlock *block =NULL ; +RamBlockList *block_list =ram_bgs_block_list_get (); + + QLIST_FOREACH(block, block_list, bgs_next) { +/* This case append when the block is not mapped. */ +if (block->host ==NULL ) { +continue ; + } + +if (address - block->host < block->max_length) { + *page_offset = (address - block->host) &TARGET_PAGE_MASK ; +return block; + } + } + +return NULL ; +}/** * ram_find_and_save_block: finds a dirty page and sends it to f *
migration/ram.h +6 -0
@@ -31,6 +31,7 @@#include "qemu-common.h" #include "exec/cpu-common.h" +#include "exec/ramlist.h" extern MigrationStats ram_counters;extern XBZRLECacheStats xbzrle_counters;
@@ -61,5 +62,10 @@ void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);int ramblock_recv_bitmap_test (RAMBlock *rb,void *host_addr);void ramblock_recv_bitmap_set (RAMBlock *rb,void *host_addr);void ramblock_recv_bitmap_set_range (RAMBlock *rb,void *host_addr,size_t nr); +/* For background snapshot */ +void ram_block_list_create (void ); +void ram_block_list_destroy (void ); + +RAMBlock *ram_bgs_block_find (uint8_t *address,ram_addr_t *page_offset);#endif
Signature · 1 line
2.17.0
Re: [Qemu-devel] [PATCH v1 06/17] background snapshot: add helpers to manage a copy of ram block list
Quoted 5 lines · Denis Plotnikov
The VM ramblock list may be changed during the snapshotting. We want to make sure that we have the same ramblock list as it was at the time of snapshot beginning.
So, we create a copy of the list at the beginning of the snapshotting and use it during the process.
Could I ask why the list might change?
Quoted 20 lines
Signed-off-by Denis Plotnikov <[email protected]>--- migration/ram.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ migration/ram.h | 6 ++++++ 2 files changed, 57 insertions(+)migration/ram.c +5
-0@@ -1508,6 +1508,57 @@ static int ram_save_host_page(RAMState *rs, PageSearchStatus *pss,return pages; } +/* BackGround Snapshot Blocks */ +static RamBlockList bgs_blocks; + +static RamBlockList *ram_bgs_block_list_get (void ) +{
migration/ram.c static int ram_save_host_page(RAMState *rs, PageSearchStatus *pss, · line 1525
Quoted 10 lines
migration/ram.c +10
-0+return &bgs_blocks; +} + +void ram_block_list_create (void ) +{ +RAMBlock *block =NULL ; +RamBlockList *block_list =ram_bgs_block_list_get (); + +qemu_mutex_lock_ramlist (); + QLIST_FOREACH(block, &ram_list.blocks, next) {
Even if we want to copy a list of ramblocks, we possibly don't need the ones that aren't migratable? Please refer to RAMBLOCK_FOREACH_MIGRATABLE(). Then I think...
Quoted 15 lines
migration/ram.c +15
-0+memory_region_ref (block->mr); +QLIST_INSERT_HEAD (block_list, block, bgs_next); + } +qemu_mutex_unlock_ramlist (); +} + +void ram_block_list_destroy (void ) +{ +RAMBlock *next, *block; +RamBlockList *block_list =ram_bgs_block_list_get (); + + QLIST_FOREACH_SAFE(block, block_list, bgs_next, next) { +QLIST_REMOVE (block, bgs_next); +memory_region_unref (block->mr); + }
migration/ram.c static int ram_save_host_page(RAMState *rs, PageSearchStatus *pss, · line 1550
Quoted 10 lines
migration/ram.c +10
-0+} + +RAMBlock *ram_bgs_block_find (uint8_t *address,ram_addr_t *page_offset) +{ +RAMBlock *block =NULL ; +RamBlockList *block_list =ram_bgs_block_list_get (); + + QLIST_FOREACH(block, block_list, bgs_next) { +/* This case append when the block is not mapped. */ +if (block->host ==NULL ) {
... things like this should not happen.
Thanks,
Quoted 41 lines
migration/ram.c +11
-0+continue ; + } + +if (address - block->host < block->max_length) { + *page_offset = (address - block->host) &TARGET_PAGE_MASK ; +return block; + } + } + +return NULL ; +}/** * ram_find_and_save_block: finds a dirty page and sends it to f * migration/ram.h +6
-0@@ -31,6 +31,7 @@#include "qemu-common.h" #include "exec/cpu-common.h" +#include "exec/ramlist.h" extern MigrationStats ram_counters;extern XBZRLECacheStats xbzrle_counters;@@ -61,5 +62,10 @@ void ram_handle_compressed(void *host, uint8_t ch, uint64_t size);int ramblock_recv_bitmap_test (RAMBlock *rb,void *host_addr);void ramblock_recv_bitmap_set (RAMBlock *rb,void *host_addr);void ramblock_recv_bitmap_set_range (RAMBlock *rb,void *host_addr,size_t nr); +/* For background snapshot */ +void ram_block_list_create (void ); +void ram_block_list_destroy (void ); + +RAMBlock *ram_bgs_block_find (uint8_t *address,ram_addr_t *page_offset);#endif Signature · 1 line
2.17.0
Signature · 1 line
Peter Xu
PATCH 7/17 background snapshot: introduce page buffer
In reply to Denis PlotnikovTo limit the amount of memory used by the background snapshot a memory limiter is used which called "page buffer".
In fact, it's a memory page counter limiting the page allocation. Currently, the limit of pages is hardcoded but its setter is the matter of the future work.
The background snapshot makes a copy of the page before writing it to the snapshot destination, but it doesn't use a preallocated buffer, because the background snapshot employs the migration infrastructure which, in turn, uses madvice to release the buffer.
The advice issuing is hard to track, so here we rely on anonymous memory mapping and releasing it by the existing code.
--- migration/ram.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++ migration/ram.h | 3 ++ 2 files changed, 94 insertions(+)
migration/ram.c +91 -0
@@ -192,6 +192,16 @@ struct RAMSrcPageRequest {QSIMPLEQ_ENTRY (RAMSrcPageRequest) next_req; }; +/* Page buffer used for background snapshot */ +typedef struct RAMPageBuffer { +/* Page buffer capacity in host pages */ +int capacity; +/* Current number of pages in the buffer */ +int used; +/* Event to notify that buffer usage is under capacity */ +QemuEvent used_decreased; +}RAMPageBuffer ; +/* State of RAM for migration */ struct RAMState {/* QEMUFile used for this migration */
@@ -230,6 +240,11 @@ struct RAMState {/* Queue of outstanding page requests from the destination */ QemuMutex src_page_req_mutex; QSIMPLEQ_HEAD(src_page_requests, RAMSrcPageRequest) src_page_requests; +/* The following 2 are for background snapshot */ +/* Buffer data to store copies of ram pages while async vm saving */ +RAMPageBuffer page_buffer; +/* Event to notify that a page copying just has finished*/ +QemuEvent page_copying_done; };typedef struct RAMState RAMState ;
@@ -1540,6 +1555,52 @@ void ram_block_list_destroy(void) } } +static void ram_page_buffer_decrease_used (void ) +{ +qemu_event_reset (&ram_state->page_buffer.used_decreased); +atomic_dec (&ram_state->page_buffer.used); +qemu_event_set (&ram_state->page_buffer.used_decreased); +} + +static void ram_page_buffer_increase_used_wait (void ) +{ +int used, *used_ptr; +RAMState *rs = ram_state; + used_ptr = &rs->page_buffer.used; +do { + used =atomic_read (used_ptr); +if (rs->page_buffer.capacity > used) { +if (atomic_cmpxchg (used_ptr, used, used +1 ) == used) { +return ; + }else { +continue ; + } + }else { +qemu_event_wait (&rs->page_buffer.used_decreased); + } + }while (true); +} + +void *ram_page_buffer_get (void ) +{ +void *page; +ram_page_buffer_increase_used_wait (); + page =mmap (0 ,TARGET_PAGE_SIZE ,PROT_READ |PROT_WRITE , +MAP_PRIVATE |MAP_ANONYMOUS ,-1 ,0 ); + +if (page ==MAP_FAILED ) { +ram_page_buffer_decrease_used (); + page =NULL ; + } +return page; +} + +int ram_page_buffer_free (void *buffer) +{ +ram_page_buffer_decrease_used (); +return qemu_madvise (buffer,TARGET_PAGE_SIZE ,MADV_DONTNEED ); +} +RAMBlock *ram_bgs_block_find (uint8_t *address,ram_addr_t *page_offset) {RAMBlock *block =NULL ;
@@ -1559,6 +1620,7 @@ RAMBlock *ram_bgs_block_find(uint8_t *address, ram_addr_t *page_offset)return NULL ; } +/** * ram_find_and_save_block: finds a dirty page and sends it to f *
@@ -1651,6 +1713,13 @@ static void xbzrle_load_cleanup(void)static void ram_state_cleanup (RAMState **rsp) { +if (migrate_background_snapshot ()) { +qemu_event_destroy (&(*rsp)->page_buffer.used_decreased); +/* In case somebody is still waiting for the event */ +qemu_event_set (&(*rsp)->page_copying_done); +qemu_event_destroy (&(*rsp)->page_copying_done); + } +migration_page_queue_free (*rsp);qemu_mutex_destroy (&(*rsp)->bitmap_mutex);qemu_mutex_destroy (&(*rsp)->src_page_req_mutex);
@@ -1689,6 +1758,13 @@ static void ram_save_cleanup(void *opaque) block->bmap =NULL ;g_free (block->unsentmap); block->unsentmap =NULL ; + +if (migrate_background_snapshot ()) { +g_free (block->touched_map); + block->touched_map =NULL ; +g_free (block->copied_map); + block->copied_map =NULL ; + } }xbzrle_cleanup ();
@@ -1703,6 +1779,10 @@ static void ram_state_reset(RAMState *rs) rs->last_page =0 ; rs->last_version = ram_list.version; rs->ram_bulk_stage = true; + +/* page buffer capacity in number of pages */ + rs->page_buffer.capacity =1000 ; + rs->page_buffer.used =0 ; }#define MAX_WAIT 50/* ms, half buffered_file limit */
@@ -2180,6 +2260,11 @@ static int ram_state_init(RAMState **rsp)*/ (*rsp)->migration_dirty_pages =ram_bytes_total () >>TARGET_PAGE_BITS ; +if (migrate_background_snapshot ()) { +qemu_event_init (&ram_state->page_buffer.used_decreased, false); +qemu_event_init (&ram_state->page_copying_done, false); + } +ram_state_reset (*rsp);return 0 ;
@@ -2196,10 +2281,16 @@ static void ram_list_init_bitmaps(void) pages = block->max_length >>TARGET_PAGE_BITS ; block->bmap =bitmap_new (pages);bitmap_set (block->bmap,0 , pages); +if (migrate_postcopy_ram ()) { block->unsentmap =bitmap_new (pages);bitmap_set (block->unsentmap,0 , pages); } + +if (migrate_background_snapshot ()) { + block->touched_map =bitmap_new (pages); + block->copied_map =bitmap_new (pages); + } } } }
migration/ram.h +3 -0
@@ -68,4 +68,7 @@ void ram_block_list_destroy(void);RAMBlock *ram_bgs_block_find (uint8_t *address,ram_addr_t *page_offset); +void *ram_page_buffer_get (void ); +int ram_page_buffer_free (void *buffer); +#endif
Signature · 1 line
2.17.0
Re: [Qemu-devel] [PATCH v1 07/17] background snapshot: introduce page buffer
Quoted 53 lines · Denis Plotnikov
To limit the amount of memory used by the background snapshot a memory limiter is used which called "page buffer".
In fact, it's a memory page counter limiting the page allocation. Currently, the limit of pages is hardcoded but its setter is the matter of the future work.The background snapshot makes a copy of the page before writing it to the snapshot destination, but it doesn't use a preallocated buffer, because the background snapshot employs the migration infrastructure which, in turn, uses madvice to release the buffer.
The advice issuing is hard to track, so here we rely on anonymous memory mapping and releasing it by the existing code.Signed-off-by Denis Plotnikov <[email protected]>--- migration/ram.c | 91 +++++++++++++++++++++++++++++++++++++++++++++++++ migration/ram.h | 3 ++ 2 files changed, 94 insertions(+)migration/ram.c +15
-0@@ -192,6 +192,16 @@ struct RAMSrcPageRequest {QSIMPLEQ_ENTRY (RAMSrcPageRequest) next_req; }; +/* Page buffer used for background snapshot */ +typedef struct RAMPageBuffer { +/* Page buffer capacity in host pages */ +int capacity; +/* Current number of pages in the buffer */ +int used; +/* Event to notify that buffer usage is under capacity */ +QemuEvent used_decreased; +}RAMPageBuffer ; +/* State of RAM for migration */ struct RAMState {/* QEMUFile used for this migration */ @@ -230,6 +240,11 @@ struct RAMState {/* Queue of outstanding page requests from the destination */ QemuMutex src_page_req_mutex; QSIMPLEQ_HEAD(src_page_requests, RAMSrcPageRequest) src_page_requests; +/* The following 2 are for background snapshot */ +/* Buffer data to store copies of ram pages while async vm saving */ +RAMPageBuffer page_buffer; +/* Event to notify that a page copying just has finished*/ +QemuEvent page_copying_done; };typedef struct RAMState RAMState ;
migration/ram.c void ram_block_list_destroy(void) · line 1560
Quoted 6 lines
migration/ram.c +3
-0@@ -1540,6 +1555,52 @@ void ram_block_list_destroy(void) } } +static void ram_page_buffer_decrease_used (void ) +{ +qemu_event_reset (&ram_state->page_buffer.used_decreased);
Could I ask why do we need to reset it before set?
Quoted 11 lines
migration/ram.c +11
-0+atomic_dec (&ram_state->page_buffer.used); +qemu_event_set (&ram_state->page_buffer.used_decreased); +} + +static void ram_page_buffer_increase_used_wait (void ) +{ +int used, *used_ptr; +RAMState *rs = ram_state; + used_ptr = &rs->page_buffer.used; + do { + used = atomic_read(used_ptr);
migration/ram.c void ram_block_list_destroy(void) · line 1581
Quoted 10 lines
migration/ram.c +10
-0+if (rs->page_buffer.capacity > used) { +if (atomic_cmpxchg (used_ptr, used, used +1 ) == used) { +return ; + }else { +continue ; + } + }else { +qemu_event_wait (&rs->page_buffer.used_decreased); + } + }while (true);
AFAIU this whole function tries to serialize the usage of page_buffer.used. Have you ever tried to simply use e.g. a mutex to protect it, and just implement the function in the simplest (but complete) way first? I believe this is for performance's sake but I'm not sure if it will run faster if the critical section of the mutex is very small (here, only the update of page_buffer_used). It seems that there are multiple ongoing works on the list that have tried to implement their own data structures (possibly lockless), including this one. For example, Guangrong has proposed a lockless fifo just in ~1.5 month:
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg00526.html
Lots of discussions there, and IIUC the conclusion is that we can consider to just backport an existing implementation from the kernel:
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg08807.html
I'm just hoping that these "data structure" works might not block the function itself from being merged. Same thing applies to this series. I never meant that you should drop existing work and do it again using mutex, but I just spoke this out in case it makes some sense to move the work forward faster.
Quoted 10 lines
migration/ram.c +10
-0+} + +void *ram_page_buffer_get (void ) +{ +void *page; +ram_page_buffer_increase_used_wait (); + page =mmap (0 ,TARGET_PAGE_SIZE ,PROT_READ |PROT_WRITE , +MAP_PRIVATE |MAP_ANONYMOUS ,-1 ,0 ); + +if (page ==MAP_FAILED ) {
migration/ram.c void ram_block_list_destroy(void) · line 1601
Quoted 10 lines
migration/ram.c +10
-0+ram_page_buffer_decrease_used (); + page =NULL ; + } +return page; +} + +int ram_page_buffer_free (void *buffer) +{ +ram_page_buffer_decrease_used (); +return qemu_madvise (buffer,TARGET_PAGE_SIZE ,MADV_DONTNEED );
Just to confirm: is this really exactly the same as munmap()?
Quoted 14 lines
migration/ram.c +3
-0+} +RAMBlock *ram_bgs_block_find (uint8_t *address,ram_addr_t *page_offset) {RAMBlock *block =NULL ;@@ -1559,6 +1620,7 @@ RAMBlock *ram_bgs_block_find(uint8_t *address, ram_addr_t *page_offset)return NULL ; } +/** * ram_find_and_save_block: finds a dirty page and sends it to f *
migration/ram.c static void xbzrle_load_cleanup(void) · line 1720
Quoted 8 lines
migration/ram.c +5
-0@@ -1651,6 +1713,13 @@ static void xbzrle_load_cleanup(void)static void ram_state_cleanup (RAMState **rsp) { +if (migrate_background_snapshot ()) { +qemu_event_destroy (&(*rsp)->page_buffer.used_decreased); +/* In case somebody is still waiting for the event */ +qemu_event_set (&(*rsp)->page_copying_done); +qemu_event_destroy (&(*rsp)->page_copying_done);
We set this since "there might be someone waiting", however immediately we destroy it... Then what if someone reads it after we destroy it? Don't we need to sync somehow before destruction happens?
Quoted 75 lines
migration/ram.c +24
-0+ } +migration_page_queue_free (*rsp);qemu_mutex_destroy (&(*rsp)->bitmap_mutex);qemu_mutex_destroy (&(*rsp)->src_page_req_mutex);@@ -1689,6 +1758,13 @@ static void ram_save_cleanup(void *opaque) block->bmap =NULL ;g_free (block->unsentmap); block->unsentmap =NULL ; + +if (migrate_background_snapshot ()) { +g_free (block->touched_map); + block->touched_map =NULL ; +g_free (block->copied_map); + block->copied_map =NULL ; + } }xbzrle_cleanup ();@@ -1703,6 +1779,10 @@ static void ram_state_reset(RAMState *rs) rs->last_page =0 ; rs->last_version = ram_list.version; rs->ram_bulk_stage = true; + +/* page buffer capacity in number of pages */ + rs->page_buffer.capacity =1000 ; + rs->page_buffer.used =0 ; }#define MAX_WAIT 50/* ms, half buffered_file limit */ @@ -2180,6 +2260,11 @@ static int ram_state_init(RAMState **rsp)*/ (*rsp)->migration_dirty_pages =ram_bytes_total () >>TARGET_PAGE_BITS ; +if (migrate_background_snapshot ()) { +qemu_event_init (&ram_state->page_buffer.used_decreased, false); +qemu_event_init (&ram_state->page_copying_done, false); + } +ram_state_reset (*rsp);return 0 ;@@ -2196,10 +2281,16 @@ static void ram_list_init_bitmaps(void) pages = block->max_length >>TARGET_PAGE_BITS ; block->bmap =bitmap_new (pages);bitmap_set (block->bmap,0 , pages); +if (migrate_postcopy_ram ()) { block->unsentmap =bitmap_new (pages);bitmap_set (block->unsentmap,0 , pages); } + +if (migrate_background_snapshot ()) { + block->touched_map =bitmap_new (pages); + block->copied_map =bitmap_new (pages); + } } } }migration/ram.h +3
-0@@ -68,4 +68,7 @@ void ram_block_list_destroy(void);RAMBlock *ram_bgs_block_find (uint8_t *address,ram_addr_t *page_offset); +void *ram_page_buffer_get (void ); +int ram_page_buffer_free (void *buffer); +#endif Signature · 1 line
2.17.0
Regards,
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [PATCH v1 07/17] background snapshot: introduce page buffer
In reply to Denis Plotnikovmigration/ram.c void ram_block_list_destroy(void) · line 1562
Quoted 5 lines · Denis Plotnikov
migration/ram.c +5
-0+static void ram_page_buffer_decrease_used (void ) +{ +qemu_event_reset (&ram_state->page_buffer.used_decreased); +atomic_dec (&ram_state->page_buffer.used); +qemu_event_set (&ram_state->page_buffer.used_decreased);
As Peter noted, only the qemu_event_set is needed here, while...
Quoted 9 lines
migration/ram.c +9
-0+} + +static void ram_page_buffer_increase_used_wait (void ) +{ +int used, *used_ptr; +RAMState *rs = ram_state; + used_ptr = &rs->page_buffer.used; + do { + used = atomic_read(used_ptr);
migration/ram.c void ram_block_list_destroy(void) · line 1581
Quoted 10 lines
migration/ram.c +10
-0+if (rs->page_buffer.capacity > used) { +if (atomic_cmpxchg (used_ptr, used, used +1 ) == used) { +return ; + }else { +continue ; + } + }else { +qemu_event_wait (&rs->page_buffer.used_decreased); + } + }while (true);
... the right idiom for using qemu_event_wait is this:
if test condition
reset
if test condition
wait
So something like
for(;;) {
used = atomic_read(used_ptr);
if (rs->page_buffer.capacity <= used) {
qemu_event_reset(&rs->page_buffer.used_decreased);
used = atomic_read(used_ptr);
if (rs->page_buffer.capacity <= used) {
qemu_event_wait(&rs->page_buffer.used_decreased);
continue;
}
}
if (atomic_cmpxchg(used_ptr, used, used + 1) == used) {
return;
}
}Note that there must be a single thread doing the above. Otherwise, you have to use mutex and condvar.
Paolo
PATCH 8/17 migration: add helpers to change VM memory protection rights
In reply to Denis Plotnikov--- migration/ram.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++ migration/ram.h | 3 +++ 2 files changed, 57 insertions(+)
migration/ram.c +54 -0
@@ -1555,6 +1555,60 @@ void ram_block_list_destroy(void) } } +static int mem_protect (void *addr,uint64_t length,int prot) +{ +int ret =mprotect (addr, length, prot); + +if (ret <0 ) { +error_report ("%s: Can't change protection on ram block at %p" , + __func__, addr); + } + +return ret; +} + +static int ram_set_ro (void *addr,uint64_t length) +{ +return mem_protect (addr, length,PROT_READ ); +} + +static int ram_set_rw (void *addr,uint64_t length) +{ +return mem_protect (addr, length,PROT_READ |PROT_WRITE ); +} + +int ram_block_list_set_readonly (void ) +{ +RAMBlock *block =NULL ; +RamBlockList *block_list =ram_bgs_block_list_get (); +int ret =0 ; + + QLIST_FOREACH(block, block_list, bgs_next) { + ret =ram_set_ro (block->host, block->used_length); +if (ret) { +break ; + } + } + +return ret; +} + +int ram_block_list_set_writable (void ) +{ +RAMBlock *block =NULL ; +RamBlockList *block_list =ram_bgs_block_list_get (); +int ret =0 ; + + QLIST_FOREACH(block, block_list, bgs_next) { + ret =ram_set_rw (block->host, block->used_length); +if (ret) { +break ; + } + } + +return ret; +} +static void ram_page_buffer_decrease_used (void ) {qemu_event_reset (&ram_state->page_buffer.used_decreased);
migration/ram.h +3 -0
@@ -71,4 +71,7 @@ RAMBlock *ram_bgs_block_find(uint8_t *address, ram_addr_t *page_offset);void *ram_page_buffer_get (void );int ram_page_buffer_free (void *buffer); +int ram_block_list_set_readonly (void ); +int ram_block_list_set_writable (void ); +#endif
Signature · 1 line
2.17.0
Re: [Qemu-devel] [PATCH v1 08/17] migration: add helpers to change VM memory protection rights
Quoted 54 lines · * Denis Plotnikov ([email protected]) wrote:
Signed-off-by Denis Plotnikov <[email protected]>--- migration/ram.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++ migration/ram.h | 3 +++ 2 files changed, 57 insertions(+)migration/ram.c +40
-0@@ -1555,6 +1555,60 @@ void ram_block_list_destroy(void) } } +static int mem_protect (void *addr,uint64_t length,int prot) +{ +int ret =mprotect (addr, length, prot); + +if (ret <0 ) { +error_report ("%s: Can't change protection on ram block at %p" , + __func__, addr); + } + +return ret; +} + +static int ram_set_ro (void *addr,uint64_t length) +{ +return mem_protect (addr, length,PROT_READ ); +} + +static int ram_set_rw (void *addr,uint64_t length) +{ +return mem_protect (addr, length,PROT_READ |PROT_WRITE ); +} + +int ram_block_list_set_readonly (void ) +{ +RAMBlock *block =NULL ; +RamBlockList *block_list =ram_bgs_block_list_get (); +int ret =0 ; + + QLIST_FOREACH(block, block_list, bgs_next) { + ret =ram_set_ro (block->host, block->used_length); +if (ret) { +break ; + } + } + +return ret; +} + +int ram_block_list_set_writable (void ) +{
migration/ram.c void ram_block_list_destroy(void) · line 1607
Quoted 10 lines
migration/ram.c +10
-0+RAMBlock *block =NULL ; +RamBlockList *block_list =ram_bgs_block_list_get (); +int ret =0 ; + + QLIST_FOREACH(block, block_list, bgs_next) { + ret =ram_set_rw (block->host, block->used_length); +if (ret) { +break ; + } + }
Do you need to keep track of who was writeable previously to avoid changing blocks that should be read-only, Or are all RAMBlocks currently mapped rw?
Dave
Quoted 21 lines
migration/ram.c +3
-0+return ret; +} +static void ram_page_buffer_decrease_used (void ) {qemu_event_reset (&ram_state->page_buffer.used_decreased);migration/ram.h +3
-0@@ -71,4 +71,7 @@ RAMBlock *ram_bgs_block_find(uint8_t *address, ram_addr_t *page_offset);void *ram_page_buffer_get (void );int ram_page_buffer_free (void *buffer); +int ram_block_list_set_readonly (void ); +int ram_block_list_set_writable (void ); +#endif Signature · 1 line
2.17.0
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK
PATCH 9/17 background snapshot: extend RAM request for holding a page copy pointer
In reply to Denis PlotnikovThis pointer is going to be used to transfer a memory.
Once the memory page is copied the content the snapshot interested in is saved for writing and we can make the page writable again.
--- migration/ram.c | 3 +++ 1 file changed, 3 insertions(+)
migration/ram.c +3 -0
@@ -188,6 +188,7 @@ struct RAMSrcPageRequest {RAMBlock *rb;hwaddr offset;hwaddr len; +void *page_copy;QSIMPLEQ_ENTRY (RAMSrcPageRequest) next_req; };
@@ -265,6 +266,8 @@ struct PageSearchStatus {unsigned long page;/* Set once we wrap around */ bool complete_round; +/* Pointer to the cached page */ +void *page_copy; };typedef struct PageSearchStatus PageSearchStatus ;
Signature · 1 line
2.17.0
PATCH 10/17 background snapshots: adapt the page queueing code for using page copies
In reply to Denis PlotnikovThe background snapshot uses memeory page copying to seal the page memory content. The patch adapts the migration infrastructure to save copies of the pages.
--- migration/migration.c | 2 +- migration/ram.c | 59 ++++++++++++++++++++++++++++++++----------- migration/ram.h | 3 ++- 3 files changed, 47 insertions(+), 17 deletions(-)
migration/migration.c +1 -1
@@ -1716,7 +1716,7 @@ static void migrate_handle_rp_req_pages(MigrationState *ms, const char* rbname,return ; } -if (ram_save_queue_pages (rbname, start, len)) { +if (ram_save_queue_pages (NULL , rbname, start, len,NULL )) {mark_source_rp_bad (ms); } }
migration/ram.c +44 -15
141 lines not on this page · raw message
migration/ram.h +2 -1
@@ -46,7 +46,8 @@ int multifd_load_setup(void);int multifd_load_cleanup (Error **errp);uint64_t ram_pagesize_summary (void ); -int ram_save_queue_pages (const char *rbname,ram_addr_t start,ram_addr_t len); +int ram_save_queue_pages (RAMBlock *block,const char *rbname, +ram_addr_t start,ram_addr_t len,void *page_copy);void acct_update_position (QEMUFile *f,size_t size,bool zero);void ram_debug_dump_bitmap (unsigned long *todump,bool expected,unsigned long pages);
Signature · 1 line
2.17.0
Re: [Qemu-devel] [PATCH v1 10/17] background snapshots: adapt the page queueing code for using page copies
Quoted 3 lines · Denis Plotnikov
The background snapshot uses memeory page copying to seal the page memory content. The patch adapts the migration infrastructure to save copies of the pages.
Again, since previous page only defined some fields that are firstly used in this patch, I think you can squash that into this one.
Quoted 124 lines
Signed-off-by Denis Plotnikov <[email protected]>--- migration/migration.c | 2 +- migration/ram.c | 59 ++++++++++++++++++++++++++++++++----------- migration/ram.h | 3 ++- 3 files changed, 47 insertions(+), 17 deletions(-)migration/migration.c +1
-1@@ -1716,7 +1716,7 @@ static void migrate_handle_rp_req_pages(MigrationState *ms, const char* rbname,return ; } -if (ram_save_queue_pages (rbname, start, len)) { +if (ram_save_queue_pages (NULL , rbname, start, len,NULL )) {mark_source_rp_bad (ms); } }migration/ram.c +30
-12@@ -976,7 +976,12 @@ static int ram_save_page(RAMState *rs, PageSearchStatus *pss, bool last_stage)RAMBlock *block = pss->block;ram_addr_t offset = pss->page <<TARGET_PAGE_BITS ; - p = block->host + offset; +if (pss->page_copy) { + p = pss->page_copy; + }else { + p = block->host + offset; + } +trace_ram_save_page (block->idstr, (uint64_t )offset, p);/* In doubt sent page as normal */ @@ -1003,13 +1008,18 @@ static int ram_save_page(RAMState *rs, PageSearchStatus *pss, bool last_stage) }else { pages =save_zero_page (rs, block, offset, p);if (pages >0 ) { -/* Must let xbzrle know, otherwise a previous (now 0'd) cached -* page would be stale -*/ -xbzrle_cache_zero_page (rs, current_addr); -ram_release_pages (block->idstr, offset, pages); +if (pss->page_copy) { +qemu_madvise (p,TARGET_PAGE_SIZE ,MADV_DONTNEED ); + }else { +/* Must let xbzrle know, otherwise a previous (now 0'd) cached +* page would be stale +*/ +xbzrle_cache_zero_page (rs, current_addr); +ram_release_pages (block->idstr, offset, pages); + } }else if (!rs->ram_bulk_stage && - !migration_in_postcopy () &&migrate_use_xbzrle ()) { + !migration_in_postcopy () &&migrate_use_xbzrle () && + !migrate_background_snapshot ()) { pages =save_xbzrle_page (rs, &p, current_addr, block, offset, last_stage);if (!last_stage) {@@ -1026,9 +1036,10 @@ static int ram_save_page(RAMState *rs, PageSearchStatus *pss, bool last_stage) ram_counters.transferred +=save_page_header (rs, rs->f, block, offset |RAM_SAVE_FLAG_PAGE );if (send_async) { -qemu_put_buffer_async (rs->f, p,TARGET_PAGE_SIZE , -migrate_release_ram () & -migration_in_postcopy ()); +bool may_free =migrate_background_snapshot () || + (migrate_release_ram () && +migration_in_postcopy ()); +qemu_put_buffer_async (rs->f, p,TARGET_PAGE_SIZE , may_free); }else {qemu_put_buffer (rs->f, p,TARGET_PAGE_SIZE ); }@@ -1269,7 +1280,8 @@ static bool find_dirty_block(RAMState *rs, PageSearchStatus *pss, bool *again)* @rs: current RAM state * @offset: used to return the offset within the RAMBlock */ -static RAMBlock *unqueue_page (RAMState *rs,ram_addr_t *offset) +static RAMBlock *unqueue_page (RAMState *rs,ram_addr_t *offset, +void **page_copy) {RAMBlock *block =NULL ;@@ -1279,10 +1291,14 @@ static RAMBlock *unqueue_page(RAMState *rs, ram_addr_t *offset)QSIMPLEQ_FIRST (&rs->src_page_requests); block = entry->rb; *offset = entry->offset; + *page_copy = entry->page_copy;if (entry->len >TARGET_PAGE_SIZE ) { entry->len -=TARGET_PAGE_SIZE ; entry->offset +=TARGET_PAGE_SIZE ; +if (entry->page_copy) { + entry->page_copy +=TARGET_PAGE_SIZE /sizeof (void *); + } }else {memory_region_unref (block->mr);QSIMPLEQ_REMOVE_HEAD (&rs->src_page_requests, next_req);@@ -1309,9 +1325,10 @@ static bool get_queued_page(RAMState *rs, PageSearchStatus *pss)RAMBlock *block;ram_addr_t offset;bool dirty; +void *page_copy; do { - block = unqueue_page(rs, &offset); + block = unqueue_page(rs, &offset, &page_copy); /* * We're sending this page, and since it's postcopy nothing else * will dirty it, and we must make sure it doesn't get sent again@@ -1349,6 +1366,7 @@ static bool get_queued_page(RAMState *rs, PageSearchStatus *pss)*/ pss->block = block; pss->page = offset >>TARGET_PAGE_BITS ; + pss->page_copy = page_copy; }return !!block;
migration/ram.c static void migration_page_queue_free(RAMState *rs) · line 1408
Quoted 5 lines
migration/ram.c +2
-0@@ -1386,17 +1404,25 @@ static void migration_page_queue_free(RAMState *rs)* * @rbname: Name of the RAMBLock of the request. NULL means the * same that last one. +* @block: RAMBlock to use. block and rbname have mutualy exclusive +* semantic with higher priority of the block.
If "mutual exclusive" then no priority at all? Maybe simply:
@block: RAMBlock to use. When @block is provided, then @rbname is ignored.
migration/ram.c static void migration_page_queue_free(RAMState *rs) · line 1411
Quoted 3 lines
migration/ram.c +1
-0* @start: starting address from the start of the RAMBlock * @len: length (in bytes) to send +* @page_copy: the address the page should be written from
Maybe:
@page_copy: the page to copy to destination. If not specified, will
use the page data specified by @start and @len.
Quoted 28 lines
migration/ram.c +8
-2*/ -int ram_save_queue_pages (const char *rbname,ram_addr_t start,ram_addr_t len) +int ram_save_queue_pages (RAMBlock *block,const char *rbname, +ram_addr_t start,ram_addr_t len,void *page_copy) {RAMBlock *ramblock;RAMState *rs = ram_state; ram_counters.postcopy_requests++; +rcu_read_lock (); -if (!rbname) { + +if (block) { + ramblock = block; + }else if (!rbname) {/* Reuse last RAMBlock */ ramblock = rs->last_req_rb;@@ -1431,6 +1457,7 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len) new_entry->rb = ramblock; new_entry->offset = start; new_entry->len = len; + new_entry->page_copy = page_copy;memory_region_ref (ramblock->mr);qemu_mutex_lock (&rs->src_page_req_mutex);
migration/ram.c static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss, · line 1499
Quoted 6 lines
migration/ram.c +2
-1@@ -1468,7 +1495,8 @@ static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss,* xbzrle can do better than compression. */ if (migrate_use_compression () && - (rs->ram_bulk_stage || !migrate_use_xbzrle ())) { + (rs->ram_bulk_stage || !migrate_use_xbzrle ()) && + !migrate_background_snapshot ()) {
This seems unecessary - in the first patch you have already made sure that compression is not enabled during a live snapshot, so we should be fine since we check migrate_use_compression() first.
Quoted 29 lines
migration/ram.c +1
-0res =ram_save_compressed_page (rs, pss, last_stage); }else { res =ram_save_page (rs, pss, last_stage);@@ -1706,6 +1734,7 @@ static int ram_find_and_save_block(RAMState *rs, bool last_stage) pss.block = rs->last_seen_block; pss.page = rs->last_page; pss.complete_round = false; + pss.page_copy =NULL ;if (!pss.block) { pss.block =QLIST_FIRST_RCU (&ram_list.blocks);migration/ram.h +2
-1@@ -46,7 +46,8 @@ int multifd_load_setup(void);int multifd_load_cleanup (Error **errp);uint64_t ram_pagesize_summary (void ); -int ram_save_queue_pages (const char *rbname,ram_addr_t start,ram_addr_t len); +int ram_save_queue_pages (RAMBlock *block,const char *rbname, +ram_addr_t start,ram_addr_t len,void *page_copy);void acct_update_position (QEMUFile *f,size_t size,bool zero);void ram_debug_dump_bitmap (unsigned long *todump,bool expected,unsigned long pages);Signature · 1 line
2.17.0
Regards,
Signature · 1 line
Peter Xu
Re: [Qemu-devel] [PATCH v1 10/17] background snapshots: adapt the page queueing code for using page copies
In reply to Denis PlotnikovOn Wed, Jul 18, 2018 at 06:41:53PM +0300, Denis Plotnikov wrote:
[...]
migration/ram.c static int ram_save_page(RAMState *rs, PageSearchStatus *pss, bool last_stage) · line 1009
Quoted 3 lines
migration/ram.c +0
-0@@ -1003,13 +1008,18 @@ static int ram_save_page(RAMState *rs, PageSearchStatus *pss, bool last_stage) }else { pages =save_zero_page (rs, block, offset, p);
Now save_zero_page() is not called by ram_save_page(). I'd suggest that you rebase to the latest master in your next post since the code base looks old.
Thanks,
Signature · 1 line
Peter Xu
PATCH 11/17 background snapshot: add a memory page copying function
In reply to Denis PlotnikovIt's the only function making a memory page copy. It supports multithreading semantics ensuring that the page is copied by one thread only and releasing the copied page from write protection.
--- migration/ram.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++ migration/ram.h | 1 + 2 files changed, 57 insertions(+)
migration/ram.c +56 -0
@@ -1706,6 +1706,62 @@ RAMBlock *ram_bgs_block_find(uint8_t *address, ram_addr_t *page_offset)return NULL ; } +/** +* ram_copy_page: make a page copy +* +* Used in the background snapshot to make a copy of a memeory page. +* Ensures that the memeory page is copied only once. +* When a page copy is done, restores read/write access to the memory +* page. +* If a page is being copied by another thread, wait until the copying +* ends and exit. +* +* Returns: +* -1 - on error +* 0 - the page wasn't copied by the function call +* 1 - the page has been copied +* +* @block: RAM block to use +* @page_nr: the page number to copy +* @page_copy: the pointer to return a page copy +* +*/ +int ram_copy_page (RAMBlock *block,unsigned long page_nr, +void **page_copy) +{ +void *host_page; + +if (test_and_set_bit_atomic (page_nr, block->touched_map)) { +while (!test_bit_atomic (page_nr, block->copied_map)) { +/* the page is being copied -- wait for the end of the copying +* and check once again +*/ +qemu_event_wait (&ram_state->page_copying_done); + } +return 0 ; + } + + *page_copy =ram_page_buffer_get (); +if (!*page_copy) { +return -1 ; + } + + host_page = block->host + (page_nr <<TARGET_PAGE_BITS ); +memcpy (*page_copy, host_page,TARGET_PAGE_SIZE ); + +if (ram_set_rw (host_page,TARGET_PAGE_SIZE )) { +ram_page_buffer_free (*page_copy); + *page_copy =NULL ; +return -1 ; + } + +set_bit_atomic (page_nr, block->copied_map); +qemu_event_set (&ram_state->page_copying_done); +qemu_event_reset (&ram_state->page_copying_done); + +return 1 ; +} +/** * ram_find_and_save_block: finds a dirty page and sends it to f *
migration/ram.h +1 -0
@@ -75,4 +75,5 @@ int ram_page_buffer_free(void *buffer);int ram_block_list_set_readonly (void );int ram_block_list_set_writable (void ); +int ram_copy_page (RAMBlock *block,unsigned long page_nr,void **page_copy);#endif
Signature · 1 line
2.17.0
PATCH 12/17 ram: add background snapshot support in ram page saving part of migration
In reply to Denis PlotnikovUnify the function saving the ram pages for using in both the migration and the background snapshots.
--- migration/ram.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
migration/ram.c +17 -0
@@ -1803,11 +1803,28 @@ static int ram_find_and_save_block(RAMState *rs, bool last_stage)if (!found) {/* priority queue empty, so just search for something dirty */ found =find_dirty_block (rs, &pss, &again); + +if (found &&migrate_background_snapshot ()) { +/* make a copy of the page and +* pass it to the page search status */ +int ret; + ret =ram_copy_page (pss.block, pss.page, &pss.page_copy); +if (ret ==0 ) { + found = false; + pages =0 ; + }else if (ret <0 ) { +return ret; + } + } }if (found) { pages =ram_save_host_page (rs, &pss, last_stage); } + +if (pss.page_copy) { +ram_page_buffer_decrease_used (); + } }while (!pages && again); rs->last_seen_block = pss.block;
Signature · 1 line
2.17.0
PATCH 13/17 background snapshot: add write-protected page access handler function
In reply to Denis PlotnikovThe handler does all the necessary operations to save the page being accessed to the snapshot file(stream).
--- migration/ram.c | 43 +++++++++++++++++++++++++++++++++++++++++++ migration/ram.h | 1 + 2 files changed, 44 insertions(+)
migration/ram.c +43 -0
@@ -1762,6 +1762,49 @@ int ram_copy_page(RAMBlock *block, unsigned long page_nr,return 1 ; } +/** +* ram_process_page_fault +* +* Used in the background snapshot to queue the copy of the memory +* page for writing. +* +* Returns: +* 0 > - on error +* 0 - success +* +* @address: guest address +* +*/ +int ram_process_page_fault (void *address) +{ +int ret; +void *page_copy =NULL ; +unsigned long page_nr; +ram_addr_t offset; + +RAMBlock *block =ram_bgs_block_find (address, &offset); + +if (!block) { +return -1 ; + } + + page_nr = offset >>TARGET_PAGE_BITS ; + + ret =ram_copy_page (block, page_nr, &page_copy); + +if (ret <0 ) { +return ret; + }else if (ret >0 ) { +if (ram_save_queue_pages (block,NULL , offset, +TARGET_PAGE_SIZE , page_copy)) { +ram_page_buffer_free (page_copy); +return -1 ; + } + } + +return 0 ; +} +/** * ram_find_and_save_block: finds a dirty page and sends it to f *
migration/ram.h +1 -0
@@ -76,4 +76,5 @@ int ram_block_list_set_readonly(void);int ram_block_list_set_writable (void );int ram_copy_page (RAMBlock *block,unsigned long page_nr,void **page_copy); +int ram_process_page_fault (void *address);#endif
Signature · 1 line
2.17.0
PATCH 14/17 kvm: add failed memeory access exit reason
In reply to Denis PlotnikovThe patch allows qemu to be aware of how to read kvm failed memeory access.
This is a temporary patch. It sould be removed on importing the kvm failed memeory access exit from the linux branch.
--- linux-headers/linux/kvm.h | 5 +++++ 1 file changed, 5 insertions(+)
linux-headers/linux/kvm.h +5 -0
@@ -235,6 +235,7 @@ struct kvm_hyperv_exit {#define KVM_EXIT_S390_STSI 25#define KVM_EXIT_IOAPIC_EOI 26#define KVM_EXIT_HYPERV 27 +#define KVM_EXIT_FAIL_MEM_ACCESS 28/* For KVM_EXIT_INTERNAL_ERROR */ /* Emulate instruction failed. */
@@ -392,6 +393,10 @@ struct kvm_run { } eoi;/* KVM_EXIT_HYPERV */ struct kvm_hyperv_exit hyperv; +/* KVM_EXIT_FAIL_MEM_ACCESS */ +struct { +__u64 hva; + } fail_mem_access;/* Fix the size of the union. */ char padding[256 ]; };
Signature · 1 line
2.17.0
PATCH 15/17 kvm: add vCPU failed memeory access processing
In reply to Denis PlotnikovIs done with support of the KVM patch returning the faulting address.
--- target/i386/kvm.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
target/i386/kvm.c +17 -0
@@ -45,6 +45,8 @@#include "hw/pci/msi.h" #include "hw/pci/msix.h" #include "migration/blocker.h" +#include "migration/savevm.h" +#include "migration/ram.h" #include "exec/memattrs.h" #include "trace.h"
@@ -3130,6 +3132,18 @@ static bool host_supports_vmx(void)return ecx &CPUID_EXT_VMX ; } +static int kvm_handle_fail_mem_access (CPUState *cpu) +{ +struct kvm_run *run = cpu->kvm_run; +int ret =ram_process_page_fault ((void *)run->fail_mem_access.hva); + +if (ret >=0 ) { +cpu_resume (cpu); + } + +return ret; +} +#define VMX_INVALID_GUEST_STATE 0x80000021int kvm_arch_handle_exit (CPUState *cs,struct kvm_run *run)
@@ -3188,6 +3202,9 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)ioapic_eoi_broadcast (run->eoi.vector); ret =0 ;break ; +case KVM_EXIT_FAIL_MEM_ACCESS : + ret =kvm_handle_fail_mem_access (cs); +break ;default :fprintf (stderr,"KVM: unknown exit reason %d\n" , run->exit_reason); ret =-1 ;
Signature · 1 line
2.17.0
Re: [Qemu-devel] [PATCH v1 15/17] kvm: add vCPU failed memeory access processing
Quoted 3 lines · Denis Plotnikov
Is done with support of the KVM patch returning the faulting address.
Signed-off-by Denis Plotnikov <[email protected]>
I feel like these two kvm-related patches can be put at the end of the series as an extension to kvm support. E.g., without these kvm patches one should still be able to do live snapshot with TCG. Then even if your KVM patches are not settled (along with the header update) there's still chance that your framework can be merged first.
Quoted 50 lines
--- target/i386/kvm.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)target/i386/kvm.c +17
-0@@ -45,6 +45,8 @@#include "hw/pci/msi.h" #include "hw/pci/msix.h" #include "migration/blocker.h" +#include "migration/savevm.h" +#include "migration/ram.h" #include "exec/memattrs.h" #include "trace.h" @@ -3130,6 +3132,18 @@ static bool host_supports_vmx(void)return ecx &CPUID_EXT_VMX ; } +static int kvm_handle_fail_mem_access (CPUState *cpu) +{ +struct kvm_run *run = cpu->kvm_run; +int ret =ram_process_page_fault ((void *)run->fail_mem_access.hva); + +if (ret >=0 ) { +cpu_resume (cpu); + } + +return ret; +} +#define VMX_INVALID_GUEST_STATE 0x80000021int kvm_arch_handle_exit (CPUState *cs,struct kvm_run *run)@@ -3188,6 +3202,9 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)ioapic_eoi_broadcast (run->eoi.vector); ret =0 ;break ; +case KVM_EXIT_FAIL_MEM_ACCESS : + ret =kvm_handle_fail_mem_access (cs); +break ;default :fprintf (stderr,"KVM: unknown exit reason %d\n" , run->exit_reason); ret =-1 ;Signature · 1 line
2.17.0
Regards,
Signature · 1 line
Peter Xu
PATCH 16/17 migration: move the device state saving logic to a separate function
In reply to Denis PlotnikovThe logic being split will be reused by the background snapshot.
--- migration/savevm.c | 91 +++++++++++++++++++++++++--------------------- migration/savevm.h | 2 + 2 files changed, 52 insertions(+), 41 deletions(-)
migration/savevm.c +50 -41
@@ -1107,51 +1107,15 @@ void qemu_savevm_state_complete_postcopy(QEMUFile *f)qemu_fflush (f); } -int qemu_savevm_state_complete_precopy (QEMUFile *f,bool iterable_only, -bool inactivate_disks) +int qemu_savevm_state_save (QEMUFile *f,bool inactivate_disks, +bool send_eof) { -QJSON *vmdesc;int vmdesc_len;SaveStateEntry *se;int ret; -bool in_postcopy =migration_in_postcopy (); - -trace_savevm_state_complete_precopy (); +QJSON *vmdesc =qjson_new ();cpu_synchronize_all_states (); - - QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { -if (!se->ops || - (in_postcopy && se->ops->has_postcopy && - se->ops->has_postcopy (se->opaque)) || - (in_postcopy && !iterable_only) || - !se->ops->save_live_complete_precopy) { -continue ; - } - -if (se->ops && se->ops->is_active) { -if (!se->ops->is_active (se->opaque)) { -continue ; - } - } -trace_savevm_section_start (se->idstr, se->section_id); - -save_section_header (f, se,QEMU_VM_SECTION_END ); - - ret = se->ops->save_live_complete_precopy (f, se->opaque); -trace_savevm_section_end (se->idstr, se->section_id, ret); -save_section_footer (f, se); -if (ret <0 ) { -qemu_file_set_error (f, ret); -return -1 ; - } - } - -if (iterable_only) { -return 0 ; - } - - vmdesc =qjson_new ();json_prop_int (vmdesc,"page_size" ,qemu_target_page_size ());json_start_array (vmdesc,"devices" ); QTAILQ_FOREACH(se, &savevm_state.handlers, entry) {
@@ -1193,8 +1157,8 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only,return ret; } } -if (!in_postcopy) { -/* Postcopy stream will still be going */ + +if (send_eof) {qemu_put_byte (f,QEMU_VM_EOF ); }
@@ -1213,6 +1177,51 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only,return 0 ; } +int qemu_savevm_state_complete_precopy (QEMUFile *f,bool iterable_only, +bool inactivate_disks) +{ +SaveStateEntry *se; +int ret; +bool in_postcopy =migration_in_postcopy (); + +trace_savevm_state_complete_precopy (); + +cpu_synchronize_all_states (); + + QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { +if (!se->ops || + (in_postcopy && se->ops->has_postcopy && + se->ops->has_postcopy (se->opaque)) || + (in_postcopy && !iterable_only) || + !se->ops->save_live_complete_precopy) { +continue ; + } + +if (se->ops && se->ops->is_active) { +if (!se->ops->is_active (se->opaque)) { +continue ; + } + } +trace_savevm_section_start (se->idstr, se->section_id); + +save_section_header (f, se,QEMU_VM_SECTION_END ); + + ret = se->ops->save_live_complete_precopy (f, se->opaque); +trace_savevm_section_end (se->idstr, se->section_id, ret); +save_section_footer (f, se); +if (ret <0 ) { +qemu_file_set_error (f, ret); +return -1 ; + } + } + +if (iterable_only) { +return 0 ; + } + +return qemu_savevm_state_save (f, inactivate_disks, !in_postcopy); +} +/* Give an estimate of the amount left to be transferred, * the result is split into the amount for units that can and * for units that can't do postcopy.
migration/savevm.h +2 -0
@@ -40,6 +40,8 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f, bool iterable_only,void qemu_savevm_state_pending (QEMUFile *f,uint64_t max_size,uint64_t *res_non_postcopiable,uint64_t *res_postcopiable); +int qemu_savevm_state_save (QEMUFile *f,bool inactivate_disks, +bool send_eof);void qemu_savevm_send_ping (QEMUFile *f,uint32_t value);void qemu_savevm_send_open_return_path (QEMUFile *f);int qemu_savevm_send_packaged (QEMUFile *f,const uint8_t *buf,size_t len);
Signature · 1 line
2.17.0
PATCH 17/17 background snapshot: enable background snapshot
In reply to Denis PlotnikovThe patch enables to save vmstate to a migration thread in the background: ram is being saved while vCPUs are running. This is done to reduce downtime on vm snapshotting: the majority of vmstate is ram, the rest of devices consumes only a few MB of memory on a typical vm.
By this moment, there were no means to make a snapshot without full vm stopping. From now, one can save a vm state having the vm stopped only for a couple of seconds to save devices' states. Then the vm resumed and ram is written without the vm full stopping.
To use async vm state it's needed to enable "background-snapshot" migration capability and start the migration
--- migration/migration.c | 103 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 101 insertions(+), 2 deletions(-)
migration/migration.c +101 -2
@@ -2188,6 +2188,99 @@ bool migrate_colo_enabled(void)return s->enabled_capabilities[MIGRATION_CAPABILITY_X_COLO ]; } +static void background_snapshot_sigsegv_handler (int unused0,siginfo_t *info, +void *unused1) +{ +if (ram_process_page_fault (info->si_addr)) { +assert (false); + } +} + +static void *background_snapshot_thread (void *opaque) +{ +MigrationState *m = opaque; +QIOChannelBuffer *bioc; +QEMUFile *fb; +int res =0 ; + +rcu_register_thread (); + +qemu_file_set_rate_limit (m->to_dst_file,INT64_MAX ); + +qemu_mutex_lock_iothread (); +vm_stop (RUN_STATE_PAUSED ); + +qemu_savevm_state_header (m->to_dst_file); +qemu_mutex_unlock_iothread (); +qemu_savevm_state_setup (m->to_dst_file); +qemu_mutex_lock_iothread (); + +migrate_set_state (&m->state,MIGRATION_STATUS_SETUP , +MIGRATION_STATUS_ACTIVE ); + +/* save the device state to the temporary buffer */ + bioc =qio_channel_buffer_new (4096 ); +qio_channel_set_name (QIO_CHANNEL (bioc),"vmstate-buffer" ); + fb =qemu_fopen_channel_output (QIO_CHANNEL (bioc)); +object_unref (OBJECT (bioc)); + +ram_block_list_create (); +ram_block_list_set_readonly (); + +if (global_state_store ()) { + goto exit; + } + +if (qemu_savevm_state_save (fb, false, true) <0 ) { +migrate_set_state (&m->state,MIGRATION_STATUS_ACTIVE , +MIGRATION_STATUS_FAILED ); + goto exit; + } + +sigsegv_user_handler_set (background_snapshot_sigsegv_handler); + +vm_start (); +qemu_mutex_unlock_iothread (); + +while (!res) { + res =qemu_savevm_state_iterate (m->to_dst_file, false); + +if (res <0 ||qemu_file_get_error (m->to_dst_file)) { +migrate_set_state (&m->state,MIGRATION_STATUS_ACTIVE , +MIGRATION_STATUS_FAILED ); + goto exit; + } + } + +/* +* By this moment we have RAM saved into the stream +* The next step is to flush the device state right after the +* RAM saved. The rest of device state is stored in +* the temporary buffer. Do flush the buffer into the stream. +*/ +qemu_put_buffer (m->to_dst_file, bioc->data, bioc->usage); +qemu_fflush (m->to_dst_file); + +if (qemu_file_get_error (m->to_dst_file)) { +migrate_set_state (&m->state,MIGRATION_STATUS_ACTIVE , +MIGRATION_STATUS_FAILED ); + goto exit; + } + +migrate_set_state (&m->state,MIGRATION_STATUS_ACTIVE , +MIGRATION_STATUS_COMPLETED ); +exit: +ram_block_list_set_writable (); +ram_block_list_destroy (); +sigsegv_user_handler_reset (); +qemu_fclose (fb); +qemu_mutex_lock_iothread (); +qemu_savevm_state_cleanup (); +qemu_mutex_unlock_iothread (); +rcu_unregister_thread (); +return NULL ; +} +/* * Master migration thread on the source VM. * It drives the migration and pumps the data down the outgoing channel.
@@ -2402,8 +2495,14 @@ void migrate_fd_connect(MigrationState *s)migrate_fd_cleanup (s);return ; } -qemu_thread_create (&s->thread,"live_migration" , migration_thread, s, -QEMU_THREAD_JOINABLE ); +if (migrate_background_snapshot ()) { +qemu_thread_create (&s->thread,"bg_snapshot" , + background_snapshot_thread, s, +QEMU_THREAD_JOINABLE ); + }else { +qemu_thread_create (&s->thread,"live_migration" , migration_thread, s, +QEMU_THREAD_JOINABLE ); + } s->migration_thread_running = true; }
Signature · 1 line
2.17.0
Quoted 10 lines · Denis Plotnikov
The workflow to make a snapshot is the following:
1. Pause the vm
2. Make a snapshot of block devices using the scheme of your choice
3. Turn on background-snapshot migration capability
4. Start the migration using the destination (migration stream) of your choice. The migration will resume the vm execution by itself
when it has the devices' states saved and is ready to start ram writing to the migration stream.
5. Listen to the migration finish event
Quoted 3 lines
The bakground snapshot works with support of KVM patch:
"x86: mmu: report failed memory access to the userspace" (not applied to the mainstream, it's in the kvm mailing list)
Hello, Denis,
Do you mind to push your tree to an online repository in case to make review easier?
Thanks,
Signature · 1 line
Peter Xu
Hi Peter
I moved the code to the repository https://github.com/denis-plotnikov/qemu/tree/background-snapshot-kvm.
the current version includes fixes with respect to your comments for version 1.
I moved KVM related patches to the end of the branch (formerly patch series).
Since, the KVM patches and the other parts to modify (vhost an others) are needless in favor of upcoming userfaltfd, I would ask you to review the general framework which is able to work with tcg.
Thanks in advance!
Denis
Quoted 22 lines · Peter Xu
Quoted 10 lines · Denis Plotnikov
The workflow to make a snapshot is the following:
1. Pause the vm
2. Make a snapshot of block devices using the scheme of your choice
3. Turn on background-snapshot migration capability
4. Start the migration using the destination (migration stream) of your choice. The migration will resume the vm execution by itself
when it has the devices' states saved and is ready to start ram writing to the migration stream.
5. Listen to the migration finish eventQuoted 3 lines
The bakground snapshot works with support of KVM patch:
"x86: mmu: report failed memory access to the userspace" (not applied to the mainstream, it's in the kvm mailing list)Hello, Denis,
Do you mind to push your tree to an online repository in case to make review easier?
Thanks,
Signature · 2 lines
Best, Denis
Quoted 1 line · Denis Plotnikov
Hi Peter
Hi, Denis,
Quoted 12 lines
I moved the code to the repository
https://github.com/denis-plotnikov/qemu/tree/background-snapshot-kvm.
the current version includes fixes with respect to your comments for version 1.
I moved KVM related patches to the end of the branch (formerly patch series).
Since, the KVM patches and the other parts to modify (vhost an others) are needless in favor of upcoming userfaltfd,
I would ask you to review the general framework which is able to work with tcg.
Quoted 1 line
Thanks in advance!
Thank you for pushing the tree.
I might have made a mistake before that I thought this work is at least working for TCG, but I think I was wrong. The problem is (I'm trying to repeat Dave's question that you seems haven't yet answered):
even for TCG there could be use cases where the process might access guest memory from the kernel space (e.g., vhost, or any system calls that with a guest memory buffer passed in). I'm afraid mprotect() and the whole signal-based mechanism cannot be able to address these page faults, then we'll encounter adhoc errors and we'll need to fix all these places up. Userfaultfd-wp should not have this problem.
I think the general idea of the work is good, but I'm not sure whether we can merge the work if we don't settle these issues.
Regards,
Signature · 1 line
Peter Xu
Hi Peter,
Thanks for the reply.
Ok, I understand about tcg.
So my only option is to wait for userfaultfd-wp.
Do you know if anyone is currently working on this? And if so, then is there any estimations when the userfaultfd is ready?
Denis
Quoted 36 lines · Peter Xu
Quoted 1 line · Denis Plotnikov
Hi Peter
Hi, Denis,
Quoted 12 lines
I moved the code to the repository
https://github.com/denis-plotnikov/qemu/tree/background-snapshot-kvm.
the current version includes fixes with respect to your comments for version 1.
I moved KVM related patches to the end of the branch (formerly patch series).
Since, the KVM patches and the other parts to modify (vhost an others) are needless in favor of upcoming userfaltfd,
I would ask you to review the general framework which is able to work with tcg.Quoted 1 line
Thanks in advance!
Thank you for pushing the tree.
I might have made a mistake before that I thought this work is at least working for TCG, but I think I was wrong. The problem is (I'm trying to repeat Dave's question that you seems haven't yet answered):
even for TCG there could be use cases where the process might access guest memory from the kernel space (e.g., vhost, or any system calls that with a guest memory buffer passed in). I'm afraidmprotect()and the whole signal-based mechanism cannot be able to address these page faults, then we'll encounter adhoc errors and we'll need to fix all these places up. Userfaultfd-wp should not have this problem.I think the general idea of the work is good, but I'm not sure whether we can merge the work if we don't settle these issues.
Regards,
Signature · 2 lines
Best, Denis
Quoted 8 lines · Denis Plotnikov
Hi Peter,
Thanks for the reply.
Ok, I understand about tcg.
So my only option is to wait for userfaultfd-wp.
Do you know if anyone is currently working on this? And if so, then is there any estimations when the userfaultfd is ready?
I am working on it, but it's only progressing slowly. The latest status is that I can run with QEMU/tcg now with Andrea's tree (will need two more patches to be applied upon from me) however all the changes are still unmature (e.g., one of the patch is discussing on the linux-mm list, the other one I haven't discussed with anyone; but these patches worked for me).
Let me know if want to play with the tree, at least I can share with you on what I got (so maybe at least you can try to run your live snapshot with QEMU/tcg with my userfaultfd tree; if error occurred we can investigate). Again, it is always welcomed if you want to participate in the kernel work too.
Let me know what you think about it.
Regards,
Signature · 1 line
Peter Xu