From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:46885) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gf18K-00022W-B9 for qemu-devel@nongnu.org; Thu, 03 Jan 2019 06:25:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gf18F-0005Uy-EV for qemu-devel@nongnu.org; Thu, 03 Jan 2019 06:25:52 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47202) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gf18F-0005Ue-67 for qemu-devel@nongnu.org; Thu, 03 Jan 2019 06:25:47 -0500 Date: Thu, 3 Jan 2019 11:25:39 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20190103112538.GF2316@work-vm> References: <20181225140449.15786-1-fli@suse.com> <20181225140449.15786-6-fli@suse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181225140449.15786-6-fli@suse.com> Subject: Re: [Qemu-devel] [PATCH for-4.0 v9 05/16] migration: unify error handling for process_incoming_migration_co List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fei Li Cc: qemu-devel@nongnu.org, shirley17fei@gmail.com, lifei1214@126.com, Markus Armbruster , Peter Xu * Fei Li (fli@suse.com) wrote: > In the current code, if process_incoming_migration_co() fails we do > the same error handing: set the error state, close the source file, > do the cleanup for multifd, and then exit(EXIT_FAILURE). To make the > code clearer, add a "goto fail" to unify the error handling. > > Cc: Markus Armbruster > Cc: Dr. David Alan Gilbert > Cc: Peter Xu > Signed-off-by: Fei Li > --- > migration/migration.c | 26 +++++++++++++------------- > 1 file changed, 13 insertions(+), 13 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index 5d322eb9d6..ded151b1bf 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -438,15 +438,13 @@ static void process_incoming_migration_co(void *opaque) > /* Make sure all file formats flush their mutable metadata */ > bdrv_invalidate_cache_all(&local_err); > if (local_err) { > - migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, > - MIGRATION_STATUS_FAILED); > error_report_err(local_err); > - exit(EXIT_FAILURE); > + goto fail; > } > > if (colo_init_ram_cache() < 0) { > error_report("Init ram cache failed"); > - exit(EXIT_FAILURE); > + goto fail; > } > > qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming", > @@ -461,20 +459,22 @@ static void process_incoming_migration_co(void *opaque) > } > > if (ret < 0) { > - Error *local_err = NULL; > - > - migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, > - MIGRATION_STATUS_FAILED); > error_report("load of migration failed: %s", strerror(-ret)); > - qemu_fclose(mis->from_src_file); > - if (multifd_load_cleanup(&local_err) != 0) { > - error_report_err(local_err); > - } > - exit(EXIT_FAILURE); > + goto fail; > } > mis->bh = qemu_bh_new(process_incoming_migration_bh, mis); > qemu_bh_schedule(mis->bh); > mis->migration_incoming_co = NULL; > + return; > +fail: > + local_err = NULL; > + migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE, > + MIGRATION_STATUS_FAILED); > + qemu_fclose(mis->from_src_file); > + if (multifd_load_cleanup(&local_err) != 0) { > + error_report_err(local_err); > + } > + exit(EXIT_FAILURE); > } > > static void migration_incoming_setup(QEMUFile *f) OK, so this is really unifying the normal error case and the two colo-incoming error cases; so I think that's fine. Reviewed-by: Dr. David Alan Gilbert > -- > 2.13.7 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK