[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 10/16] migration: Handle migration_incoming_setup() errors co
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [PATCH 10/16] migration: Handle migration_incoming_setup() errors consistently |
Date: |
Mon, 2 Aug 2021 18:53:20 +0100 |
User-agent: |
Mutt/2.0.7 (2021-05-04) |
* Markus Armbruster (armbru@redhat.com) wrote:
> Commit b673eab4e2 "multifd: Make multifd_load_setup() get an Error
> parameter" changed migration_incoming_setup() to take an Error **
> argument, and adjusted the callers accordingly. It neglected to
> change adjust multifd_load_setup(): it still exit()s on error. Clean
> that up.
>
> The error now gets propagated up two call chains: via
> migration_fd_process_incoming() to rdma_accept_incoming_migration(),
> and via migration_ioc_process_incoming() to
> migration_channel_process_incoming(). Both chain ends report the
> error with error_report_err(), but otherwise ignore it. Behavioral
> change: we no longer exit() on this error.
>
> This is consistent with how we handle other errors here, e.g. from
> multifd_recv_new_channel() via migration_ioc_process_incoming() to
> migration_channel_process_incoming(). Wether it's consistently right
> or consistently wrong I can't tell.
>
> Also clean up the return value from the unusual 0 on success, 1 on
> error to the more common true on success, false on error.
>
> Cc: Juan Quintela <quintela@redhat.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> migration/migration.c | 27 +++++++++------------------
> 1 file changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 231dc24414..c1c0a48647 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -609,30 +609,25 @@ fail:
> }
>
> /**
> - * @migration_incoming_setup: Setup incoming migration
> - *
> - * Returns 0 for no error or 1 for error
> - *
> + * migration_incoming_setup: Setup incoming migration
> * @f: file for main migration channel
> * @errp: where to put errors
> + *
> + * Returns: %true on success, %false on error.
> */
> -static int migration_incoming_setup(QEMUFile *f, Error **errp)
> +static bool migration_incoming_setup(QEMUFile *f, Error **errp)
> {
> MigrationIncomingState *mis = migration_incoming_get_current();
> - Error *local_err = NULL;
>
> - if (multifd_load_setup(&local_err) != 0) {
> - /* We haven't been able to create multifd threads
> - nothing better to do */
> - error_report_err(local_err);
> - exit(EXIT_FAILURE);
> + if (multifd_load_setup(errp) != 0) {
> + return false;
> }
What I don't know is how well that will survive; for example in
multifd_load_setup it creates multiple threads and calls the recv_setup
member on each thread; now if one of those fails what happens - if we
don't exit, is it cleaned up enough so you can try another
migrate_incoming or is it just going to fall over?
Dave
>
> if (!mis->from_src_file) {
> mis->from_src_file = f;
> }
> qemu_file_set_blocking(f, false);
> - return 0;
> + return true;
> }
>
> void migration_incoming_process(void)
> @@ -675,14 +670,11 @@ static bool postcopy_try_recover(QEMUFile *f)
>
> void migration_fd_process_incoming(QEMUFile *f, Error **errp)
> {
> - Error *local_err = NULL;
> -
> if (postcopy_try_recover(f)) {
> return;
> }
>
> - if (migration_incoming_setup(f, &local_err)) {
> - error_propagate(errp, local_err);
> + if (!migration_incoming_setup(f, errp)) {
> return;
> }
> migration_incoming_process();
> @@ -703,8 +695,7 @@ void migration_ioc_process_incoming(QIOChannel *ioc,
> Error **errp)
> return;
> }
>
> - if (migration_incoming_setup(f, &local_err)) {
> - error_propagate(errp, local_err);
> + if (!migration_incoming_setup(f, errp)) {
> return;
> }
>
> --
> 2.31.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
- Re: [PATCH 10/16] migration: Handle migration_incoming_setup() errors consistently,
Dr. David Alan Gilbert <=