From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55713) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axsTf-0005Dg-2w for qemu-devel@nongnu.org; Wed, 04 May 2016 04:48:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1axsTT-0000XI-7A for qemu-devel@nongnu.org; Wed, 04 May 2016 04:48:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55249) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axsTS-0000U3-Ve for qemu-devel@nongnu.org; Wed, 04 May 2016 04:48:03 -0400 Date: Wed, 4 May 2016 09:47:47 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20160504084747.GA2302@work-vm> References: <1462333259-3237-1-git-send-email-liang.z.li@intel.com> <1462333259-3237-3-git-send-email-liang.z.li@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1462333259-3237-3-git-send-email-liang.z.li@intel.com> Subject: Re: [Qemu-devel] [PATCH 2/5] migration: Fix a potential issue List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Liang Li Cc: qemu-devel@nongnu.org, quintela@redhat.com, amit.shah@redhat.com, berrange@redhat.com * Liang Li (liang.z.li@intel.com) wrote: > At the end of live migration and before vm_start() on the destination > side, we should make sure all the decompression tasks are finished, if > this can not be guaranteed, the VM may get the incorrect memory data, > or the updated memory may be overwritten by the decompression thread. > Add the code to fix this potential issue. > > Suggested-by: David Alan Gilbert > Signed-off-by: Liang Li > --- > include/migration/migration.h | 1 + > migration/migration.c | 2 +- > migration/ram.c | 20 ++++++++++++++++++++ > 3 files changed, 22 insertions(+), 1 deletion(-) > > diff --git a/include/migration/migration.h b/include/migration/migration.h > index ac2c12c..1c9051e 100644 > --- a/include/migration/migration.h > +++ b/include/migration/migration.h > @@ -223,6 +223,7 @@ void migrate_compress_threads_create(void); > void migrate_compress_threads_join(void); > void migrate_decompress_threads_create(void); > void migrate_decompress_threads_join(void); > +void wait_for_decompress_done(void); > uint64_t ram_bytes_remaining(void); > uint64_t ram_bytes_transferred(void); > uint64_t ram_bytes_total(void); > diff --git a/migration/migration.c b/migration/migration.c > index 991313a..5228c28 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -347,7 +347,7 @@ static void process_incoming_migration_bh(void *opaque) > /* If global state section was not received or we are in running > state, we need to obey autostart. Any other state is set with > runstate_set. */ > - > + wait_for_decompress_done(); I wonder if that's early enough; the roder here is that we get: ram_load devices load wait_for_decompress_done() start VM Loading the devices can access guest RAM though (especially virtio); so I think you need: ram_load wait_for_decompress_done() devices load start VM I think you could do that by placing the wait_for_decompress_done() at the end of ram_load(). Dave > if (!global_state_received() || > global_state_get_runstate() == RUN_STATE_RUNNING) { > if (autostart) { > diff --git a/migration/ram.c b/migration/ram.c > index 7ab6ab5..4459b38 100644 > --- a/migration/ram.c > +++ b/migration/ram.c > @@ -2220,6 +2220,26 @@ static void *do_data_decompress(void *opaque) > return NULL; > } > > +void wait_for_decompress_done(void) > +{ > + int idx, thread_count; > + > + if (!migrate_use_compression()) { > + return; > + } > + thread_count = migrate_decompress_threads(); > + for (idx = 0; idx < thread_count; idx++) { > + if (!decomp_param[idx].done) { > + qemu_mutex_lock(&decomp_done_lock); > + while (!decomp_param[idx].done) { > + qemu_cond_wait(&decomp_done_cond, &decomp_done_lock); > + } > + qemu_mutex_unlock(&decomp_done_lock); > + } > + } > + > +} > + > void migrate_decompress_threads_create(void) > { > int i, thread_count; > -- > 1.9.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK