[Qemu-devel] [PATCH 1/3] move state and mon_resume to struct MigrationState

[Paolo Bonzini]

The handling of suspending/resuming the monitor, and the
small active/completed/cancelled "state machine" should be the same
across all "subclasses" of MigrationState.  If any differences in
behavior are required, more methods should be added that subclasses
can customize.  This is what I do here with migrate_fd_cleanup.

I also extracted the initialization of a FdMigrationState into
a new function, migrate_fd_init.

---
 migration-exec.c |    8 +-----
 migration-tcp.c  |    8 +-----
 migration.c      |   55 +++++++++++++++++++++++++++++++----------------------
 migration.h      |   17 +++++++++------
 4 files changed, 46 insertions(+), 42 deletions(-)

diff --git a/migration-exec.c b/migration-exec.c
index 0dd5aff..cfa1304 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -83,16 +83,12 @@ MigrationState *exec_start_outgoing_migration(const char 
*command,
     s->close = exec_close;
     s->get_error = file_errno;
     s->write = file_write;
-    s->mig_state.cancel = migrate_fd_cancel;
-    s->mig_state.get_status = migrate_fd_get_status;
-    s->mig_state.release = migrate_fd_release;
 
-    s->state = MIG_STATE_ACTIVE;
-    s->mon_resume = NULL;
     s->bandwidth_limit = bandwidth_limit;
 
+    migrate_fd_init(s);
     if (!detach)
-        migrate_fd_monitor_suspend(s);
+        migrate_monitor_suspend(&s->mig_state);
 
     migrate_fd_connect(s);
     return &s->mig_state;
diff --git a/migration-tcp.c b/migration-tcp.c
index 1f4358e..ef48145 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -92,12 +92,8 @@ MigrationState *tcp_start_outgoing_migration(const char 
*host_port,
     s->get_error = socket_errno;
     s->write = socket_write;
     s->close = tcp_close;
-    s->mig_state.cancel = migrate_fd_cancel;
-    s->mig_state.get_status = migrate_fd_get_status;
-    s->mig_state.release = migrate_fd_release;
+    migrate_fd_init(s);
 
-    s->state = MIG_STATE_ACTIVE;
-    s->mon_resume = NULL;
     s->bandwidth_limit = bandwidth_limit;
     s->fd = socket(PF_INET, SOCK_STREAM, 0);
     if (s->fd == -1) {
@@ -108,7 +104,7 @@ MigrationState *tcp_start_outgoing_migration(const char 
*host_port,
     socket_set_nonblock(s->fd);
 
     if (!detach)
-        migrate_fd_monitor_suspend(s);
+        migrate_monitor_suspend(&s->mig_state);
 
     do {
         ret = connect(s->fd, (struct sockaddr *)&addr, sizeof(addr));
diff --git a/migration.c b/migration.c
index 190b37e..5abce0c 100644
--- a/migration.c
+++ b/migration.c
@@ -143,7 +143,7 @@ void do_info_migrate(Monitor *mon)
 
     if (s) {
         monitor_printf(mon, "Migration status: ");
-        switch (s->get_status(s)) {
+        switch (s->state) {
         case MIG_STATE_ACTIVE:
             monitor_printf(mon, "active\n");
             monitor_printf(mon, "transferred ram: %" PRIu64 " kbytes\n", 
ram_bytes_transferred() >> 10);
@@ -163,9 +163,19 @@ void do_info_migrate(Monitor *mon)
     }
 }
 
+void migrate_fd_init(FdMigrationState *s)
+{
+    s->mig_state.cleanup = migrate_fd_cleanup;
+    s->mig_state.cancel = migrate_fd_cancel;
+    s->mig_state.release = migrate_fd_release;
+
+    s->mig_state.state = MIG_STATE_ACTIVE;
+    s->mig_state.mon_resume = NULL;
+}
+
 /* shared migration helpers */
 
-void migrate_fd_monitor_suspend(FdMigrationState *s)
+void migrate_monitor_suspend(MigrationState *s)
 {
     s->mon_resume = cur_mon;
     if (monitor_suspend(cur_mon) == 0)
@@ -178,12 +188,13 @@ void migrate_fd_monitor_suspend(FdMigrationState *s)
 void migrate_fd_error(FdMigrationState *s)
 {
     dprintf("setting error state\n");
-    s->state = MIG_STATE_ERROR;
-    migrate_fd_cleanup(s);
+    migrate_set_state(&s->mig_state, MIG_STATE_ERROR);
 }
 
-void migrate_fd_cleanup(FdMigrationState *s)
+void migrate_fd_cleanup(MigrationState *mig_state)
 {
+    FdMigrationState *s = migrate_to_fms(mig_state);
+
     qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
 
     if (s->file) {
@@ -194,10 +205,6 @@ void migrate_fd_cleanup(FdMigrationState *s)
     if (s->fd != -1)
         close(s->fd);
 
-    /* Don't resume monitor until we've flushed all of the buffers */
-    if (s->mon_resume)
-        monitor_resume(s->mon_resume);
-
     s->fd = -1;
 }
 
@@ -253,7 +260,7 @@ void migrate_fd_put_ready(void *opaque)
 {
     FdMigrationState *s = opaque;
 
-    if (s->state != MIG_STATE_ACTIVE) {
+    if (s->mig_state.state != MIG_STATE_ACTIVE) {
         dprintf("put_ready returning because of non-active state\n");
         return;
     }
@@ -271,29 +278,32 @@ void migrate_fd_put_ready(void *opaque)
         } else {
             state = MIG_STATE_COMPLETED;
         }
-        migrate_fd_cleanup(s);
-        s->state = state;
+        migrate_set_state(&s->mig_state, state);
     }
 }
 
-int migrate_fd_get_status(MigrationState *mig_state)
+void migrate_set_state(MigrationState *s, int state)
 {
-    FdMigrationState *s = migrate_to_fms(mig_state);
-    return s->state;
+    s->state = state;
+
+    if (state != MIG_STATE_ACTIVE) {
+       s->cleanup (s);
+        /* Don't resume monitor until we've flushed all of the buffers */
+       if (s->mon_resume)
+            monitor_resume(s->mon_resume);
+    }
 }
 
 void migrate_fd_cancel(MigrationState *mig_state)
 {
     FdMigrationState *s = migrate_to_fms(mig_state);
 
-    if (s->state != MIG_STATE_ACTIVE)
+    if (s->mig_state.state != MIG_STATE_ACTIVE)
         return;
 
     dprintf("cancelling migration\n");
 
-    s->state = MIG_STATE_CANCELLED;
-
-    migrate_fd_cleanup(s);
+    migrate_set_state(&s->mig_state, MIG_STATE_CANCELLED);
 }
 
 void migrate_fd_release(MigrationState *mig_state)
@@ -302,9 +312,8 @@ void migrate_fd_release(MigrationState *mig_state)
 
     dprintf("releasing state\n");
    
-    if (s->state == MIG_STATE_ACTIVE) {
-        s->state = MIG_STATE_CANCELLED;
-        migrate_fd_cleanup(s);
+    if (s->mig_state.state == MIG_STATE_ACTIVE) {
+        migrate_set_state(&s->mig_state, MIG_STATE_CANCELLED);
     }
     free(s);
 }
@@ -315,7 +324,7 @@ void migrate_fd_wait_for_unfreeze(void *opaque)
     int ret;
 
     dprintf("wait for unfreeze\n");
-    if (s->state != MIG_STATE_ACTIVE)
+    if (s->mig_state.state != MIG_STATE_ACTIVE)
         return;
 
     do {
diff --git a/migration.h b/migration.h
index 37c7f8e..81ac361 100644
--- a/migration.h
+++ b/migration.h
@@ -25,9 +25,12 @@ typedef struct MigrationState MigrationState;
 
 struct MigrationState
 {
+    Monitor *mon_resume;
+    int state;
+
     /* FIXME: add more accessors to print migration info */
+    void (*cleanup)(MigrationState *s);
     void (*cancel)(MigrationState *s);
-    int (*get_status)(MigrationState *s);
     void (*release)(MigrationState *s);
 };
 
@@ -39,8 +42,6 @@ struct FdMigrationState
     int64_t bandwidth_limit;
     QEMUFile *file;
     int fd;
-    Monitor *mon_resume;
-    int state;
     int (*get_error)(struct FdMigrationState*);
     int (*close)(struct FdMigrationState*);
     int (*write)(struct FdMigrationState*, const void *, size_t);
@@ -73,11 +74,13 @@ MigrationState *tcp_start_outgoing_migration(const char 
*host_port,
                                             int64_t bandwidth_limit,
                                             int detach);
 
-void migrate_fd_monitor_suspend(FdMigrationState *s);
+void migrate_monitor_suspend(MigrationState *s);
 
-void migrate_fd_error(FdMigrationState *s);
+void migrate_set_state(MigrationState *s, int state);
 
-void migrate_fd_cleanup(FdMigrationState *s);
+void migrate_fd_init(FdMigrationState *s);
+
+void migrate_fd_error(FdMigrationState *s);
 
 void migrate_fd_put_notify(void *opaque);
 
@@ -87,7 +90,7 @@ void migrate_fd_connect(FdMigrationState *s);
 
 void migrate_fd_put_ready(void *opaque);
 
-int migrate_fd_get_status(MigrationState *mig_state);
+void migrate_fd_cleanup(MigrationState *mig_state);
 
 void migrate_fd_cancel(MigrationState *mig_state);
 
-- 
1.5.5.6