On 03/28/2018 05:25 PM, Peter Xu wrote:
On Tue, Mar 27, 2018 at 05:10:35PM +0800, guangrong.xiao@xxxxxxxxx wrote: [...]@@ -357,10 +358,20 @@ static void compress_threads_save_cleanup(void) terminate_compression_threads(); thread_count = migrate_compress_threads(); for (i = 0; i < thread_count; i++) { + /* + * stream.opaque can be used to store private data, we use it + * as a indicator which shows if the thread is properly init'd + * or not + */ + if (!comp_param[i].stream.opaque) { + break; + }How about using comp_param[i].file? The opaque seems to be hiding deeper, and...
Yes, indeed, good suggestion.
qemu_thread_join(compress_threads + i); qemu_fclose(comp_param[i].file); qemu_mutex_destroy(&comp_param[i].mutex); qemu_cond_destroy(&comp_param[i].cond); + deflateEnd(&comp_param[i].stream); + comp_param[i].stream.opaque = NULL; } qemu_mutex_destroy(&comp_done_lock); qemu_cond_destroy(&comp_done_cond); @@ -370,12 +381,12 @@ static void compress_threads_save_cleanup(void) comp_param = NULL; }-static void compress_threads_save_setup(void)+static int compress_threads_save_setup(void) { int i, thread_count;if (!migrate_use_compression()) {- return; + return 0; } thread_count = migrate_compress_threads(); compress_threads = g_new0(QemuThread, thread_count); @@ -383,6 +394,12 @@ static void compress_threads_save_setup(void) qemu_cond_init(&comp_done_cond); qemu_mutex_init(&comp_done_lock); for (i = 0; i < thread_count; i++) { + if (deflateInit(&comp_param[i].stream, + migrate_compress_level()) != Z_OK) {(indent issue)
Will fix.
+ goto exit; + } + comp_param[i].stream.opaque = &comp_param[i];...here from document: ZEXTERN int ZEXPORT deflateInit OF((z_streamp strm, int level)); Initializes the internal stream state for compression. The fields zalloc, zfree and opaque must be initialized before by the caller. If zalloc and zfree are set to Z_NULL, deflateInit updates them to use default allocation functions. So shall we init opaque first? Otherwise looks good to me.
No, opaque need to be init-ed only if zalloc and zfree are specified, it is not the case in this patch.
