From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:54619) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RFnpL-0006ec-Ir for qemu-devel@nongnu.org; Mon, 17 Oct 2011 10:06:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RFnpB-0003tE-WE for qemu-devel@nongnu.org; Mon, 17 Oct 2011 10:06:03 -0400 Received: from mail-gx0-f173.google.com ([209.85.161.173]:55191) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RFnpB-0003sj-Rp for qemu-devel@nongnu.org; Mon, 17 Oct 2011 10:05:53 -0400 Received: by ggnr5 with SMTP id r5so3760166ggn.4 for ; Mon, 17 Oct 2011 07:05:53 -0700 (PDT) Message-ID: <4E9C363E.5020400@codemonkey.ws> Date: Mon, 17 Oct 2011 09:05:50 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <7241665c71aea94834aa9402777a52765f42281a.1318326684.git.quintela@redhat.com> In-Reply-To: <7241665c71aea94834aa9402777a52765f42281a.1318326684.git.quintela@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 23/36] migration: Refactor and simplify error checking in migrate_fd_put_ready List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu-devel@nongnu.org On 10/11/2011 05:00 AM, Juan Quintela wrote: > Signed-off-by: Juan Quintela Just a general comment. This series is fairly touch to review because you're repeated refactoring the same function with no other comments beyond "refactor and simplify". As a reviewer, I really have no help in understanding your motiviation for making this changes, why it can't be done all at once, etc. I'm not rejecting this patch, but in the future, please put a little more care into better rationale in commit messages to simplify reviewing. Reviewed-by: Anthony Liguori Regards, Anthony Liguori > --- > migration.c | 21 ++++++++++----------- > 1 files changed, 10 insertions(+), 11 deletions(-) > > diff --git a/migration.c b/migration.c > index a01bf4f..432afe6 100644 > --- a/migration.c > +++ b/migration.c > @@ -372,23 +372,22 @@ static void migrate_fd_put_ready(void *opaque) > DPRINTF("done iterating\n"); > vm_stop(RUN_STATE_FINISH_MIGRATE); > > - if ((qemu_savevm_state_complete(s->mon, s->file))< 0) { > - if (old_vm_running) { > - vm_start(); > + if (qemu_savevm_state_complete(s->mon, s->file)< 0) { > + migrate_fd_error(s); > + } else { > + if (migrate_fd_cleanup(s)< 0) { > + migrate_fd_error(s); > + } else { > + s->state = MIG_STATE_COMPLETED; > + runstate_set(RUN_STATE_POSTMIGRATE); > + notifier_list_notify(&migration_state_notifiers, NULL); > } > - s->state = MIG_STATE_ERROR; > } > - if (migrate_fd_cleanup(s)< 0) { > + if (s->get_status(s) != MIG_STATE_COMPLETED) { > if (old_vm_running) { > vm_start(); > } > - s->state = MIG_STATE_ERROR; > } > - if (s->state == MIG_STATE_ACTIVE) { > - s->state = MIG_STATE_COMPLETED; > - runstate_set(RUN_STATE_POSTMIGRATE); > - } > - notifier_list_notify(&migration_state_notifiers, NULL); > } > } >