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...
> 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)
> + 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.
> +
> /* comp_param[i].file is just used as a dummy buffer to save data,
> * set its ops to empty.
> */
Thanks,
--
Peter Xu
