Re: [Qemu-devel] [PATCH v6 03/12] migration: avoid concurrent invoke channel_close by different threads

2018-08-06 Thread 858585 jemmy
This patch causes compile error when make check.

  LINKtests/test-qdist
migration/qemu-file.o: In function `qemu_fclose':
/tmp/qemu-test/src/migration/qemu-file.c:331: undefined reference to
`migrate_get_current'
/tmp/qemu-test/src/migration/qemu-file.c:333: undefined reference to
`migrate_get_current'
collect2: error: ld returned 1 exit status
make: *** [tests/test-vmstate] Error 1

but I don't find an efficient way to fix it.
so I prefer to remove this patch from this series.
maybe it's not the best way to protect by
migrate_get_current()->qemu_file_close_lock.


On Fri, Aug 3, 2018 at 5:13 PM, Lidong Chen  wrote:
> From: Lidong Chen 
>
> The channel_close maybe invoked by different threads. For example, source
> qemu invokes qemu_fclose in main thread, migration thread and return path
> thread. Destination qemu invokes qemu_fclose in main thread, listen thread
> and COLO incoming thread.
>
> Signed-off-by: Lidong Chen 
> Reviewed-by: Daniel P. Berrangé 
> ---
>  migration/migration.c | 2 ++
>  migration/migration.h | 7 +++
>  migration/qemu-file.c | 6 --
>  3 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index b7d9854..a3a0756 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3200,6 +3200,7 @@ static void migration_instance_finalize(Object *obj)
>  qemu_sem_destroy(>postcopy_pause_sem);
>  qemu_sem_destroy(>postcopy_pause_rp_sem);
>  qemu_sem_destroy(>rp_state.rp_sem);
> +qemu_mutex_destroy(>qemu_file_close_lock);
>  error_free(ms->error);
>  }
>
> @@ -3236,6 +3237,7 @@ static void migration_instance_init(Object *obj)
>  qemu_sem_init(>rp_state.rp_sem, 0);
>  qemu_sem_init(>rate_limit_sem, 0);
>  qemu_mutex_init(>qemu_file_lock);
> +qemu_mutex_init(>qemu_file_close_lock);
>  }
>
>  /*
> diff --git a/migration/migration.h b/migration/migration.h
> index 64a7b33..a50c2de 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -122,6 +122,13 @@ struct MigrationState
>  QemuMutex qemu_file_lock;
>
>  /*
> + * The to_src_file and from_dst_file point to one QIOChannelRDMA,
> + * And qemu_fclose maybe invoked by different threads. use this lock
> + * to avoid concurrent invoke channel_close by different threads.
> + */
> +QemuMutex qemu_file_close_lock;
> +
> +/*
>   * Used to allow urgent requests to override rate limiting.
>   */
>  QemuSemaphore rate_limit_sem;
> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
> index 977b9ae..74c48e0 100644
> --- a/migration/qemu-file.c
> +++ b/migration/qemu-file.c
> @@ -323,12 +323,14 @@ void qemu_update_position(QEMUFile *f, size_t size)
>   */
>  int qemu_fclose(QEMUFile *f)
>  {
> -int ret;
> +int ret, ret2;
>  qemu_fflush(f);
>  ret = qemu_file_get_error(f);
>
>  if (f->ops->close) {
> -int ret2 = f->ops->close(f->opaque);
> +qemu_mutex_lock(_get_current()->qemu_file_close_lock);
> +ret2 = f->ops->close(f->opaque);
> +qemu_mutex_unlock(_get_current()->qemu_file_close_lock);
>  if (ret >= 0) {
>  ret = ret2;
>  }
> --
> 1.8.3.1
>



[Qemu-devel] [PATCH v6 03/12] migration: avoid concurrent invoke channel_close by different threads

2018-08-03 Thread Lidong Chen
From: Lidong Chen 

The channel_close maybe invoked by different threads. For example, source
qemu invokes qemu_fclose in main thread, migration thread and return path
thread. Destination qemu invokes qemu_fclose in main thread, listen thread
and COLO incoming thread.

Signed-off-by: Lidong Chen 
Reviewed-by: Daniel P. Berrangé 
---
 migration/migration.c | 2 ++
 migration/migration.h | 7 +++
 migration/qemu-file.c | 6 --
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index b7d9854..a3a0756 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3200,6 +3200,7 @@ static void migration_instance_finalize(Object *obj)
 qemu_sem_destroy(>postcopy_pause_sem);
 qemu_sem_destroy(>postcopy_pause_rp_sem);
 qemu_sem_destroy(>rp_state.rp_sem);
+qemu_mutex_destroy(>qemu_file_close_lock);
 error_free(ms->error);
 }
 
@@ -3236,6 +3237,7 @@ static void migration_instance_init(Object *obj)
 qemu_sem_init(>rp_state.rp_sem, 0);
 qemu_sem_init(>rate_limit_sem, 0);
 qemu_mutex_init(>qemu_file_lock);
+qemu_mutex_init(>qemu_file_close_lock);
 }
 
 /*
diff --git a/migration/migration.h b/migration/migration.h
index 64a7b33..a50c2de 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -122,6 +122,13 @@ struct MigrationState
 QemuMutex qemu_file_lock;
 
 /*
+ * The to_src_file and from_dst_file point to one QIOChannelRDMA,
+ * And qemu_fclose maybe invoked by different threads. use this lock
+ * to avoid concurrent invoke channel_close by different threads.
+ */
+QemuMutex qemu_file_close_lock;
+
+/*
  * Used to allow urgent requests to override rate limiting.
  */
 QemuSemaphore rate_limit_sem;
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index 977b9ae..74c48e0 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -323,12 +323,14 @@ void qemu_update_position(QEMUFile *f, size_t size)
  */
 int qemu_fclose(QEMUFile *f)
 {
-int ret;
+int ret, ret2;
 qemu_fflush(f);
 ret = qemu_file_get_error(f);
 
 if (f->ops->close) {
-int ret2 = f->ops->close(f->opaque);
+qemu_mutex_lock(_get_current()->qemu_file_close_lock);
+ret2 = f->ops->close(f->opaque);
+qemu_mutex_unlock(_get_current()->qemu_file_close_lock);
 if (ret >= 0) {
 ret = ret2;
 }
-- 
1.8.3.1