Re: [Qemu-devel] [PATCH 23/36] migration: Refactor and simplify error checking in migrate_fd_put_ready

2011-10-17 Thread Anthony Liguori

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);
  }
  }






[Qemu-devel] [PATCH 23/36] migration: Refactor and simplify error checking in migrate_fd_put_ready

2011-10-11 Thread Juan Quintela
Signed-off-by: Juan Quintela 
---
 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);
 }
 }

-- 
1.7.6.4