[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] migration: fix the compression code
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH] migration: fix the compression code |
Date: |
Wed, 26 Sep 2018 16:00:58 +0100 |
User-agent: |
Mutt/1.10.1 (2018-07-13) |
* Fei Li (address@hidden) wrote:
> Add judgement in compress_threads_save_cleanup() to check whether the
> static CompressParam *comp_param has been allocated. If not, just
> return; or else segmentation fault will occur when using the NULL
> comp_param's parameters. One test case can reproduce this is: set
> the compression on and migrate to a wrong nonexistent host IP address.
>
> Our current code does not judge before handling comp_param[idx]'s quit
> and cond that whether they have been initialized. If not initialized,
> "qemu_mutex_lock_impl: Assertion `mutex->initialized' failed." will
> occur. Fix this by squashing the terminate_compression_threads() into
> compress_threads_save_cleanup() and employing the existing judgement
> condition. One test case can reproduce this error is: set the
> compression on and fail to fully setup the default eight compression
> thread in compress_threads_save_setup().
>
> Signed-off-by: Fei Li <address@hidden>
Queued.
> ---
> migration/ram.c | 24 ++++++++----------------
> 1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index f6fd8e5e09..050d535068 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -420,28 +420,14 @@ static void *do_data_compress(void *opaque)
> return NULL;
> }
>
> -static inline void terminate_compression_threads(void)
> -{
> - int idx, thread_count;
> -
> - thread_count = migrate_compress_threads();
> -
> - for (idx = 0; idx < thread_count; idx++) {
> - qemu_mutex_lock(&comp_param[idx].mutex);
> - comp_param[idx].quit = true;
> - qemu_cond_signal(&comp_param[idx].cond);
> - qemu_mutex_unlock(&comp_param[idx].mutex);
> - }
> -}
> -
> static void compress_threads_save_cleanup(void)
> {
> int i, thread_count;
>
> - if (!migrate_use_compression()) {
> + if (!migrate_use_compression() || !comp_param) {
> return;
> }
> - terminate_compression_threads();
> +
> thread_count = migrate_compress_threads();
> for (i = 0; i < thread_count; i++) {
> /*
> @@ -451,6 +437,12 @@ static void compress_threads_save_cleanup(void)
> if (!comp_param[i].file) {
> break;
> }
> +
> + qemu_mutex_lock(&comp_param[i].mutex);
> + comp_param[i].quit = true;
> + qemu_cond_signal(&comp_param[i].cond);
> + qemu_mutex_unlock(&comp_param[i].mutex);
> +
> qemu_thread_join(compress_threads + i);
> qemu_mutex_destroy(&comp_param[i].mutex);
> qemu_cond_destroy(&comp_param[i].cond);
> --
> 2.13.7
>
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK