[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/5] migration: Fix a potential issue
From: |
Li, Liang Z |
Subject: |
Re: [Qemu-devel] [PATCH 2/5] migration: Fix a potential issue |
Date: |
Wed, 4 May 2016 14:13:54 +0000 |
> -----Original Message-----
> From: Qemu-devel [mailto:qemu-devel-
> address@hidden On Behalf Of Juan Quintela
> Sent: Wednesday, May 04, 2016 5:18 PM
> To: Li, Liang Z
> Cc: address@hidden; address@hidden; address@hidden
> Subject: Re: [Qemu-devel] [PATCH 2/5] migration: Fix a potential issue
>
> Liang Li <address@hidden> 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 <address@hidden>
> > Signed-off-by: Liang Li <address@hidden>
> > ---
> > 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();
> > 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;
> > }
> >
>
> why?
>
> > +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);
> > + }
> > + }
> > +
> > +}
> > +
>
> thread_count = migrate_decompress_threads();
> qemu_mutex_lock(&decomp_done_lock);
> for (idx = 0; idx < thread_count; idx++) {
> while (!decomp_param[idx].done) {
> qemu_cond_wait(&decomp_done_cond, &decomp_done_lock);
> }
> }
> qemu_mutex_unlock(&decomp_done_lock);
>
> Simpler and correct, no?
>
> Later, Juan.
Yes, thanks!
Again, I think I should clean up the multiple thread compression code.
Liang
- [Qemu-devel] [PATCH 0/5] live migration bug fix and refine, Liang Li, 2016/05/03
- [Qemu-devel] [PATCH 1/5] migration: Fix multi-thread compression bug, Liang Li, 2016/05/03
- [Qemu-devel] [PATCH 3/5] migration: remove useless code, Liang Li, 2016/05/03
- [Qemu-devel] [PATCH 5/5] migration: refine ram_save_compressed_page, Liang Li, 2016/05/03
- [Qemu-devel] [PATCH 2/5] migration: Fix a potential issue, Liang Li, 2016/05/03
- [Qemu-devel] [PATCH 4/5] qemu-file: Fix qemu_put_compression_data flaw, Liang Li, 2016/05/03