On Wed, Sep 19, 2018 at 09:35:20PM +0800, Fei Li 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 in terminate_compression_threads().
One test case can reproduce this error is: set the compression on
and migrate to a wrong nonexistent host IP address.
Add judgement before handling comp_param[idx]'s quit and cond in
terminate_compression_threads(), in case they are not initialized.
Or else "qemu_mutex_lock_impl: Assertion `mutex->initialized' failed."
will occur. One test case can reproduce this error is: set the
compression on and fail to fully setup the eight compression thread
in compress_threads_save_setup().
Signed-off-by: Fei Li <address@hidden>
---
migration/ram.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/migration/ram.c b/migration/ram.c
index 79c89425a3..522a5550b4 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -427,6 +427,9 @@ static inline void terminate_compression_threads(void)
thread_count = migrate_compress_threads();
for (idx = 0; idx < thread_count; idx++) {
+ if (!comp_param[idx].mutex.initialized) {
+ break;
+ }
Instead of accessing the mutex internal variable "initialized", how
about we just squash the terminate_compression_threads() into existing
compress_threads_save_cleanup()? Note that we have this in
compress_threads_save_cleanup() already:
for (i = 0; i < thread_count; i++) {
/*
* we use it as a indicator which shows if the thread is
* properly init'd or not
*/
if (!comp_param[i].file) {
break;
}
qemu_thread_join(compress_threads + i);
qemu_mutex_destroy(&comp_param[i].mutex);
qemu_cond_destroy(&comp_param[i].cond);
deflateEnd(&comp_param[i].stream);
g_free(comp_param[i].originbuf);
qemu_fclose(comp_param[i].file);
comp_param[i].file = NULL;
}
So we already try to detect this using the comp_param[i].file, which
IMO is better (the exposure of the mutex.init var is just "an
accident" - logically we could hide that from mutex impl).