Re: [Qemu-devel] [PATCH v11 for-4.0 09/11] qemu_thread: supplement error handling for migration
在 2019/2/5 上午12:34, Dr. David Alan Gilbert 写道:
* Markus Armbruster (arm...@redhat.com) wrote:
Dave, I tried to review the error paths, in particular resource cleanup,
but there's a lot going on, and I'm not feeling confident. Please have
a close look.
Fei Li writes:
From: Fei Li
Update qemu_thread_create()'s callers by
- setting an error on qemu_thread_create() failure for callers that
set an error on failure;
- reporting the error and returning failure for callers that return
an error code on failure;
- reporting the error and setting some state for callers that just
report errors and choose not to continue on.
Besides, make compress_threads_save_cleanup() cope with partially
initialized comp_param[i] to adapt to the new qemu_thread_create()
failure case.
Cc: Markus Armbruster
Cc: Dr. David Alan Gilbert
Signed-off-by: Fei Li
Reviewed-by: Dr. David Alan Gilbert
---
migration/migration.c| 35 +---
migration/postcopy-ram.c | 16 ++---
migration/ram.c | 70 ++--
migration/savevm.c | 12 ---
4 files changed, 89 insertions(+), 44 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 1da71211c8..0034ca1334 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -447,10 +447,13 @@ static void process_incoming_migration_co(void *opaque)
goto fail;
}
-/* TODO: let the further caller handle the error instead of abort() */
-qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
- colo_process_incoming_thread, mis,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
+ colo_process_incoming_thread, mis,
+ QEMU_THREAD_JOINABLE, &local_err) < 0) {
+error_reportf_err(local_err, "failed to create "
+ "colo_process_incoming_thread: ");
+goto fail;
+}
mis->have_colo_incoming_thread = true;
qemu_coroutine_yield();
@@ -2349,6 +2352,7 @@ out:
static int open_return_path_on_source(MigrationState *ms,
bool create_thread)
{
+Error *local_err = NULL;
ms->rp_state.from_dst_file = qemu_file_get_return_path(ms->to_dst_file);
if (!ms->rp_state.from_dst_file) {
@@ -2362,10 +2366,15 @@ static int open_return_path_on_source(MigrationState
*ms,
return 0;
}
-/* TODO: let the further caller handle the error instead of abort() here */
-qemu_thread_create(&ms->rp_state.rp_thread, "return path",
- source_return_path_thread, ms,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&ms->rp_state.rp_thread, "return path",
+ source_return_path_thread, ms,
+ QEMU_THREAD_JOINABLE, &local_err) < 0) {
+error_reportf_err(local_err,
+ "failed to create source_return_path_thread: ");
+qemu_fclose(ms->rp_state.from_dst_file);
+ms->rp_state.from_dst_file = NULL;
+return -1;
+ }
trace_open_return_path_on_source_continue();
@@ -3201,9 +3210,13 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
if (multifd_save_setup() != 0) {
migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
MIGRATION_STATUS_FAILED);
migrate_fd_cleanup(s);
return;
}
-/* TODO: let the further caller handle the error instead of abort() here */
-qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
+ QEMU_THREAD_JOINABLE, &error_in) < 0) {
+error_reportf_err(error_in, "failed to create migration_thread: ");
+migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
+migrate_fd_cleanup(s);
Is there anything to clean up for multifd_save_setup()? Dave?
I need to bounce that one to Juan; he knows the multifd stuff; cc'd
Hi all,
I check the code again, the migrate_fd_cleanup() will call
multifd_save_setup() to clean
the multifd-stuff. Thus the current code is just fine. :)
Sorry for the late reply...
BTW, I will send the updated v12 later.
Have a nice day, thanks
Fei
+return;
+}
s->migration_thread_running = true;
}
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 221ea24919..0934a1403a 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1083,6 +1083,8 @@ retry:
int postcopy_ram_enable_notify(MigrationIncomingState *mis)
{
+Error *local_err = NULL;
+
/* Open the fd for the
Re: [Qemu-devel] [PATCH v11 for-4.0 09/11] qemu_thread: supplement error handling for migration
在 2019/2/5 上午12:34, Dr. David Alan Gilbert 写道:
* Markus Armbruster (arm...@redhat.com) wrote:
Dave, I tried to review the error paths, in particular resource cleanup,
but there's a lot going on, and I'm not feeling confident. Please have
a close look.
Fei Li writes:
From: Fei Li
Update qemu_thread_create()'s callers by
- setting an error on qemu_thread_create() failure for callers that
set an error on failure;
- reporting the error and returning failure for callers that return
an error code on failure;
- reporting the error and setting some state for callers that just
report errors and choose not to continue on.
Besides, make compress_threads_save_cleanup() cope with partially
initialized comp_param[i] to adapt to the new qemu_thread_create()
failure case.
Cc: Markus Armbruster
Cc: Dr. David Alan Gilbert
Signed-off-by: Fei Li
Reviewed-by: Dr. David Alan Gilbert
---
migration/migration.c| 35 +---
migration/postcopy-ram.c | 16 ++---
migration/ram.c | 70 ++--
migration/savevm.c | 12 ---
4 files changed, 89 insertions(+), 44 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 1da71211c8..0034ca1334 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -447,10 +447,13 @@ static void process_incoming_migration_co(void *opaque)
goto fail;
}
-/* TODO: let the further caller handle the error instead of abort() */
-qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
- colo_process_incoming_thread, mis,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
+ colo_process_incoming_thread, mis,
+ QEMU_THREAD_JOINABLE, &local_err) < 0) {
+error_reportf_err(local_err, "failed to create "
+ "colo_process_incoming_thread: ");
+goto fail;
+}
mis->have_colo_incoming_thread = true;
qemu_coroutine_yield();
@@ -2349,6 +2352,7 @@ out:
static int open_return_path_on_source(MigrationState *ms,
bool create_thread)
{
+Error *local_err = NULL;
ms->rp_state.from_dst_file = qemu_file_get_return_path(ms->to_dst_file);
if (!ms->rp_state.from_dst_file) {
@@ -2362,10 +2366,15 @@ static int open_return_path_on_source(MigrationState
*ms,
return 0;
}
-/* TODO: let the further caller handle the error instead of abort() here */
-qemu_thread_create(&ms->rp_state.rp_thread, "return path",
- source_return_path_thread, ms,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&ms->rp_state.rp_thread, "return path",
+ source_return_path_thread, ms,
+ QEMU_THREAD_JOINABLE, &local_err) < 0) {
+error_reportf_err(local_err,
+ "failed to create source_return_path_thread: ");
+qemu_fclose(ms->rp_state.from_dst_file);
+ms->rp_state.from_dst_file = NULL;
+return -1;
+ }
trace_open_return_path_on_source_continue();
@@ -3201,9 +3210,13 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
if (multifd_save_setup() != 0) {
migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
MIGRATION_STATUS_FAILED);
migrate_fd_cleanup(s);
return;
}
-/* TODO: let the further caller handle the error instead of abort() here */
-qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
+ QEMU_THREAD_JOINABLE, &error_in) < 0) {
+error_reportf_err(error_in, "failed to create migration_thread: ");
+migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
+migrate_fd_cleanup(s);
Is there anything to clean up for multifd_save_setup()? Dave?
I need to bounce that one to Juan; he knows the multifd stuff; cc'd
Hi Juan, Peter,
I think this may involve the sequential order between migration_thread() and
multifd_save_setup() we talked earlier :
https://www.mail-archive.com/qemu-devel@nongnu.org/msg571178.html
To be specific, the current code can not tell whether all channels have
been sent via multifd_new_send_channel_async() before/during/after the
migration_thread() runs.
What do you think?
Have a nice day, thanks :)
Fei
+return;
+}
s->migration_thread_running = true;
}
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 221ea24919..0934a1403a 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postc
Re: [Qemu-devel] [PATCH v11 for-4.0 09/11] qemu_thread: supplement error handling for migration
* Markus Armbruster (arm...@redhat.com) wrote:
> Dave, I tried to review the error paths, in particular resource cleanup,
> but there's a lot going on, and I'm not feeling confident. Please have
> a close look.
>
> Fei Li writes:
>
> > From: Fei Li
> >
> > Update qemu_thread_create()'s callers by
> > - setting an error on qemu_thread_create() failure for callers that
> > set an error on failure;
> > - reporting the error and returning failure for callers that return
> > an error code on failure;
> > - reporting the error and setting some state for callers that just
> > report errors and choose not to continue on.
> >
> > Besides, make compress_threads_save_cleanup() cope with partially
> > initialized comp_param[i] to adapt to the new qemu_thread_create()
> > failure case.
> >
> > Cc: Markus Armbruster
> > Cc: Dr. David Alan Gilbert
> > Signed-off-by: Fei Li
> > Reviewed-by: Dr. David Alan Gilbert
> > ---
> > migration/migration.c| 35 +---
> > migration/postcopy-ram.c | 16 ++---
> > migration/ram.c | 70 ++--
> > migration/savevm.c | 12 ---
> > 4 files changed, 89 insertions(+), 44 deletions(-)
> >
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 1da71211c8..0034ca1334 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -447,10 +447,13 @@ static void process_incoming_migration_co(void
> > *opaque)
> > goto fail;
> > }
> >
> > -/* TODO: let the further caller handle the error instead of
> > abort() */
> > -qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
> > - colo_process_incoming_thread, mis,
> > - QEMU_THREAD_JOINABLE, &error_abort);
> > +if (qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
> > + colo_process_incoming_thread, mis,
> > + QEMU_THREAD_JOINABLE, &local_err) < 0) {
> > +error_reportf_err(local_err, "failed to create "
> > + "colo_process_incoming_thread: ");
> > +goto fail;
> > +}
> > mis->have_colo_incoming_thread = true;
> > qemu_coroutine_yield();
> >
> > @@ -2349,6 +2352,7 @@ out:
> > static int open_return_path_on_source(MigrationState *ms,
> >bool create_thread)
> > {
> > +Error *local_err = NULL;
> >
> > ms->rp_state.from_dst_file =
> > qemu_file_get_return_path(ms->to_dst_file);
> > if (!ms->rp_state.from_dst_file) {
> > @@ -2362,10 +2366,15 @@ static int
> > open_return_path_on_source(MigrationState *ms,
> > return 0;
> > }
> >
> > -/* TODO: let the further caller handle the error instead of abort()
> > here */
> > -qemu_thread_create(&ms->rp_state.rp_thread, "return path",
> > - source_return_path_thread, ms,
> > - QEMU_THREAD_JOINABLE, &error_abort);
> > +if (qemu_thread_create(&ms->rp_state.rp_thread, "return path",
> > + source_return_path_thread, ms,
> > + QEMU_THREAD_JOINABLE, &local_err) < 0) {
> > +error_reportf_err(local_err,
> > + "failed to create source_return_path_thread: ");
> > +qemu_fclose(ms->rp_state.from_dst_file);
> > +ms->rp_state.from_dst_file = NULL;
> > +return -1;
> > + }
> >
> > trace_open_return_path_on_source_continue();
> >
> > @@ -3201,9 +3210,13 @@ void migrate_fd_connect(MigrationState *s, Error
> > *error_in)
>if (multifd_save_setup() != 0) {
>migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
> MIGRATION_STATUS_FAILED);
> > migrate_fd_cleanup(s);
> > return;
> > }
> > -/* TODO: let the further caller handle the error instead of abort()
> > here */
> > -qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
> > - QEMU_THREAD_JOINABLE, &error_abort);
> > +if (qemu_thread_create(&s->thread, "live_migration", migration_thread,
> > s,
> > + QEMU_THREAD_JOINABLE, &error_in) < 0) {
> > +error_reportf_err(error_in, "failed to create migration_thread: ");
> > +migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
> > +migrate_fd_cleanup(s);
>
> Is there anything to clean up for multifd_save_setup()? Dave?
I need to bounce that one to Juan; he knows the multifd stuff; cc'd
> > +return;
> > +}
> > s->migration_thread_running = true;
> > }
> >
> > diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> > index 221ea24919..0934a1403a 100644
> > --- a/migration/postcopy-ram.c
> > +++ b/migration/postcopy-ram.c
> > @@ -1083,6 +1083,8 @@ retry:
> >
> > int postcopy_ram_enable_notify
Re: [Qemu-devel] [PATCH v11 for-4.0 09/11] qemu_thread: supplement error handling for migration
在 2019/2/1 下午11:34, Markus Armbruster 写道:
Dave, I tried to review the error paths, in particular resource cleanup,
but there's a lot going on, and I'm not feeling confident. Please have
a close look.
Fei Li writes:
From: Fei Li
Update qemu_thread_create()'s callers by
- setting an error on qemu_thread_create() failure for callers that
set an error on failure;
- reporting the error and returning failure for callers that return
an error code on failure;
- reporting the error and setting some state for callers that just
report errors and choose not to continue on.
Besides, make compress_threads_save_cleanup() cope with partially
initialized comp_param[i] to adapt to the new qemu_thread_create()
failure case.
Cc: Markus Armbruster
Cc: Dr. David Alan Gilbert
Signed-off-by: Fei Li
Reviewed-by: Dr. David Alan Gilbert
---
migration/migration.c| 35 +---
migration/postcopy-ram.c | 16 ++---
migration/ram.c | 70 ++--
migration/savevm.c | 12 ---
4 files changed, 89 insertions(+), 44 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 1da71211c8..0034ca1334 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -447,10 +447,13 @@ static void process_incoming_migration_co(void *opaque)
goto fail;
}
-/* TODO: let the further caller handle the error instead of abort() */
-qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
- colo_process_incoming_thread, mis,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
+ colo_process_incoming_thread, mis,
+ QEMU_THREAD_JOINABLE, &local_err) < 0) {
+error_reportf_err(local_err, "failed to create "
+ "colo_process_incoming_thread: ");
+goto fail;
+}
mis->have_colo_incoming_thread = true;
qemu_coroutine_yield();
@@ -2349,6 +2352,7 @@ out:
static int open_return_path_on_source(MigrationState *ms,
bool create_thread)
{
+Error *local_err = NULL;
ms->rp_state.from_dst_file = qemu_file_get_return_path(ms->to_dst_file);
if (!ms->rp_state.from_dst_file) {
@@ -2362,10 +2366,15 @@ static int open_return_path_on_source(MigrationState
*ms,
return 0;
}
-/* TODO: let the further caller handle the error instead of abort() here */
-qemu_thread_create(&ms->rp_state.rp_thread, "return path",
- source_return_path_thread, ms,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&ms->rp_state.rp_thread, "return path",
+ source_return_path_thread, ms,
+ QEMU_THREAD_JOINABLE, &local_err) < 0) {
+error_reportf_err(local_err,
+ "failed to create source_return_path_thread: ");
+qemu_fclose(ms->rp_state.from_dst_file);
+ms->rp_state.from_dst_file = NULL;
+return -1;
+ }
trace_open_return_path_on_source_continue();
@@ -3201,9 +3210,13 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
if (multifd_save_setup() != 0) {
migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
MIGRATION_STATUS_FAILED);
migrate_fd_cleanup(s);
return;
}
-/* TODO: let the further caller handle the error instead of abort() here */
-qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
+ QEMU_THREAD_JOINABLE, &error_in) < 0) {
+error_reportf_err(error_in, "failed to create migration_thread: ");
+migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
+migrate_fd_cleanup(s);
Is there anything to clean up for multifd_save_setup()? Dave?
+return;
+}
s->migration_thread_running = true;
}
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 221ea24919..0934a1403a 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1083,6 +1083,8 @@ retry:
int postcopy_ram_enable_notify(MigrationIncomingState *mis)
{
+Error *local_err = NULL;
+
/* Open the fd for the kernel to give us userfaults */
mis->userfault_fd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
if (mis->userfault_fd == -1) {
@@ -1109,10 +,16 @@ int postcopy_ram_enable_notify(MigrationIncomingState
*mis)
}
qemu_sem_init(&mis->fault_thread_sem, 0);
-/* TODO: let the further caller handle the error instead of abort() here */
-qemu
Re: [Qemu-devel] [PATCH v11 for-4.0 09/11] qemu_thread: supplement error handling for migration
Dave, I tried to review the error paths, in particular resource cleanup,
but there's a lot going on, and I'm not feeling confident. Please have
a close look.
Fei Li writes:
> From: Fei Li
>
> Update qemu_thread_create()'s callers by
> - setting an error on qemu_thread_create() failure for callers that
> set an error on failure;
> - reporting the error and returning failure for callers that return
> an error code on failure;
> - reporting the error and setting some state for callers that just
> report errors and choose not to continue on.
>
> Besides, make compress_threads_save_cleanup() cope with partially
> initialized comp_param[i] to adapt to the new qemu_thread_create()
> failure case.
>
> Cc: Markus Armbruster
> Cc: Dr. David Alan Gilbert
> Signed-off-by: Fei Li
> Reviewed-by: Dr. David Alan Gilbert
> ---
> migration/migration.c| 35 +---
> migration/postcopy-ram.c | 16 ++---
> migration/ram.c | 70 ++--
> migration/savevm.c | 12 ---
> 4 files changed, 89 insertions(+), 44 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 1da71211c8..0034ca1334 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -447,10 +447,13 @@ static void process_incoming_migration_co(void *opaque)
> goto fail;
> }
>
> -/* TODO: let the further caller handle the error instead of abort()
> */
> -qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
> - colo_process_incoming_thread, mis,
> - QEMU_THREAD_JOINABLE, &error_abort);
> +if (qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
> + colo_process_incoming_thread, mis,
> + QEMU_THREAD_JOINABLE, &local_err) < 0) {
> +error_reportf_err(local_err, "failed to create "
> + "colo_process_incoming_thread: ");
> +goto fail;
> +}
> mis->have_colo_incoming_thread = true;
> qemu_coroutine_yield();
>
> @@ -2349,6 +2352,7 @@ out:
> static int open_return_path_on_source(MigrationState *ms,
>bool create_thread)
> {
> +Error *local_err = NULL;
>
> ms->rp_state.from_dst_file = qemu_file_get_return_path(ms->to_dst_file);
> if (!ms->rp_state.from_dst_file) {
> @@ -2362,10 +2366,15 @@ static int open_return_path_on_source(MigrationState
> *ms,
> return 0;
> }
>
> -/* TODO: let the further caller handle the error instead of abort() here
> */
> -qemu_thread_create(&ms->rp_state.rp_thread, "return path",
> - source_return_path_thread, ms,
> - QEMU_THREAD_JOINABLE, &error_abort);
> +if (qemu_thread_create(&ms->rp_state.rp_thread, "return path",
> + source_return_path_thread, ms,
> + QEMU_THREAD_JOINABLE, &local_err) < 0) {
> +error_reportf_err(local_err,
> + "failed to create source_return_path_thread: ");
> +qemu_fclose(ms->rp_state.from_dst_file);
> +ms->rp_state.from_dst_file = NULL;
> +return -1;
> + }
>
> trace_open_return_path_on_source_continue();
>
> @@ -3201,9 +3210,13 @@ void migrate_fd_connect(MigrationState *s, Error
> *error_in)
if (multifd_save_setup() != 0) {
migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
MIGRATION_STATUS_FAILED);
> migrate_fd_cleanup(s);
> return;
> }
> -/* TODO: let the further caller handle the error instead of abort() here
> */
> -qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
> - QEMU_THREAD_JOINABLE, &error_abort);
> +if (qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
> + QEMU_THREAD_JOINABLE, &error_in) < 0) {
> +error_reportf_err(error_in, "failed to create migration_thread: ");
> +migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
> +migrate_fd_cleanup(s);
Is there anything to clean up for multifd_save_setup()? Dave?
> +return;
> +}
> s->migration_thread_running = true;
> }
>
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 221ea24919..0934a1403a 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -1083,6 +1083,8 @@ retry:
>
> int postcopy_ram_enable_notify(MigrationIncomingState *mis)
> {
> +Error *local_err = NULL;
> +
> /* Open the fd for the kernel to give us userfaults */
> mis->userfault_fd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
> if (mis->userfault_fd == -1) {
> @@ -1109,10 +,16 @@ int postcopy_ram_enable_notify(MigrationIncomingState
> *mis)
> }
>
> qem
Re: [Qemu-devel] [PATCH v11 for-4.0 09/11] qemu_thread: supplement error handling for migration
在 2019/2/1 下午1:18, Fei Li 写道:
From: Fei Li
Update qemu_thread_create()'s callers by
- setting an error on qemu_thread_create() failure for callers that
set an error on failure;
- reporting the error and returning failure for callers that return
an error code on failure;
- reporting the error and setting some state for callers that just
report errors and choose not to continue on.
Besides, make compress_threads_save_cleanup() cope with partially
initialized comp_param[i] to adapt to the new qemu_thread_create()
failure case.
Hi David,
I got your Reviewed-by in last version at an early stage, but later one
issue is raised
by Markus due to the improper “[patch v9 16/16] qemu_thread_join: fix
segmentation fault”.
Thus in this v11, I do an update for the following two functions:
compress_threads_save_cleanup() & compress_threads_save_setup()
Please help to review again, thanks :)
Have a nice day
Fei
Cc: Markus Armbruster
Cc: Dr. David Alan Gilbert
Signed-off-by: Fei Li
Reviewed-by: Dr. David Alan Gilbert
---
migration/migration.c| 35 +---
migration/postcopy-ram.c | 16 ++---
migration/ram.c | 70 ++--
migration/savevm.c | 12 ---
4 files changed, 89 insertions(+), 44 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 1da71211c8..0034ca1334 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -447,10 +447,13 @@ static void process_incoming_migration_co(void *opaque)
goto fail;
}
-/* TODO: let the further caller handle the error instead of abort() */
-qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
- colo_process_incoming_thread, mis,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
+ colo_process_incoming_thread, mis,
+ QEMU_THREAD_JOINABLE, &local_err) < 0) {
+error_reportf_err(local_err, "failed to create "
+ "colo_process_incoming_thread: ");
+goto fail;
+}
mis->have_colo_incoming_thread = true;
qemu_coroutine_yield();
@@ -2349,6 +2352,7 @@ out:
static int open_return_path_on_source(MigrationState *ms,
bool create_thread)
{
+Error *local_err = NULL;
ms->rp_state.from_dst_file = qemu_file_get_return_path(ms->to_dst_file);
if (!ms->rp_state.from_dst_file) {
@@ -2362,10 +2366,15 @@ static int open_return_path_on_source(MigrationState
*ms,
return 0;
}
-/* TODO: let the further caller handle the error instead of abort() here */
-qemu_thread_create(&ms->rp_state.rp_thread, "return path",
- source_return_path_thread, ms,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&ms->rp_state.rp_thread, "return path",
+ source_return_path_thread, ms,
+ QEMU_THREAD_JOINABLE, &local_err) < 0) {
+error_reportf_err(local_err,
+ "failed to create source_return_path_thread: ");
+qemu_fclose(ms->rp_state.from_dst_file);
+ms->rp_state.from_dst_file = NULL;
+return -1;
+ }
trace_open_return_path_on_source_continue();
@@ -3201,9 +3210,13 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
migrate_fd_cleanup(s);
return;
}
-/* TODO: let the further caller handle the error instead of abort() here */
-qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
+ QEMU_THREAD_JOINABLE, &error_in) < 0) {
+error_reportf_err(error_in, "failed to create migration_thread: ");
+migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
+migrate_fd_cleanup(s);
+return;
+}
s->migration_thread_running = true;
}
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 221ea24919..0934a1403a 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1083,6 +1083,8 @@ retry:
int postcopy_ram_enable_notify(MigrationIncomingState *mis)
{
+Error *local_err = NULL;
+
/* Open the fd for the kernel to give us userfaults */
mis->userfault_fd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
if (mis->userfault_fd == -1) {
@@ -1109,10 +,16 @@ int postcopy_ram_enable_notify(MigrationIncomingState
*mis)
}
qemu_sem_init(&mis->fault_thread_sem, 0);
-/* TODO: let the further caller handle the error instead of abort() here */
-qemu_thread_create(&mis->fault_thre
[Qemu-devel] [PATCH v11 for-4.0 09/11] qemu_thread: supplement error handling for migration
From: Fei Li
Update qemu_thread_create()'s callers by
- setting an error on qemu_thread_create() failure for callers that
set an error on failure;
- reporting the error and returning failure for callers that return
an error code on failure;
- reporting the error and setting some state for callers that just
report errors and choose not to continue on.
Besides, make compress_threads_save_cleanup() cope with partially
initialized comp_param[i] to adapt to the new qemu_thread_create()
failure case.
Cc: Markus Armbruster
Cc: Dr. David Alan Gilbert
Signed-off-by: Fei Li
Reviewed-by: Dr. David Alan Gilbert
---
migration/migration.c| 35 +---
migration/postcopy-ram.c | 16 ++---
migration/ram.c | 70 ++--
migration/savevm.c | 12 ---
4 files changed, 89 insertions(+), 44 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 1da71211c8..0034ca1334 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -447,10 +447,13 @@ static void process_incoming_migration_co(void *opaque)
goto fail;
}
-/* TODO: let the further caller handle the error instead of abort() */
-qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
- colo_process_incoming_thread, mis,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
+ colo_process_incoming_thread, mis,
+ QEMU_THREAD_JOINABLE, &local_err) < 0) {
+error_reportf_err(local_err, "failed to create "
+ "colo_process_incoming_thread: ");
+goto fail;
+}
mis->have_colo_incoming_thread = true;
qemu_coroutine_yield();
@@ -2349,6 +2352,7 @@ out:
static int open_return_path_on_source(MigrationState *ms,
bool create_thread)
{
+Error *local_err = NULL;
ms->rp_state.from_dst_file = qemu_file_get_return_path(ms->to_dst_file);
if (!ms->rp_state.from_dst_file) {
@@ -2362,10 +2366,15 @@ static int open_return_path_on_source(MigrationState
*ms,
return 0;
}
-/* TODO: let the further caller handle the error instead of abort() here */
-qemu_thread_create(&ms->rp_state.rp_thread, "return path",
- source_return_path_thread, ms,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&ms->rp_state.rp_thread, "return path",
+ source_return_path_thread, ms,
+ QEMU_THREAD_JOINABLE, &local_err) < 0) {
+error_reportf_err(local_err,
+ "failed to create source_return_path_thread: ");
+qemu_fclose(ms->rp_state.from_dst_file);
+ms->rp_state.from_dst_file = NULL;
+return -1;
+ }
trace_open_return_path_on_source_continue();
@@ -3201,9 +3210,13 @@ void migrate_fd_connect(MigrationState *s, Error
*error_in)
migrate_fd_cleanup(s);
return;
}
-/* TODO: let the further caller handle the error instead of abort() here */
-qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&s->thread, "live_migration", migration_thread, s,
+ QEMU_THREAD_JOINABLE, &error_in) < 0) {
+error_reportf_err(error_in, "failed to create migration_thread: ");
+migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
+migrate_fd_cleanup(s);
+return;
+}
s->migration_thread_running = true;
}
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 221ea24919..0934a1403a 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1083,6 +1083,8 @@ retry:
int postcopy_ram_enable_notify(MigrationIncomingState *mis)
{
+Error *local_err = NULL;
+
/* Open the fd for the kernel to give us userfaults */
mis->userfault_fd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
if (mis->userfault_fd == -1) {
@@ -1109,10 +,16 @@ int postcopy_ram_enable_notify(MigrationIncomingState
*mis)
}
qemu_sem_init(&mis->fault_thread_sem, 0);
-/* TODO: let the further caller handle the error instead of abort() here */
-qemu_thread_create(&mis->fault_thread, "postcopy/fault",
- postcopy_ram_fault_thread, mis,
- QEMU_THREAD_JOINABLE, &error_abort);
+if (qemu_thread_create(&mis->fault_thread, "postcopy/fault",
+ postcopy_ram_fault_thread, mis,
+ QEMU_THREAD_JOINABLE, &local_err) < 0) {
+error_reportf_err(local_err,
+ "failed to create postcopy_ram_fault_thread: ");
+close(mis->us
