Patchwork [03/22] migration: Fold MigrationState into FdMigrationState

login
register
mail settings
Submitter Juan Quintela
Date Feb. 23, 2011, 12:44 a.m.
Message ID <cbcfdae0dd8935f17b24fb97916d2db7a2d016f9.1298421307.git.quintela@redhat.com>
Download mbox | patch
Permalink /patch/84020/
State New
Headers show

Comments

Juan Quintela - Feb. 23, 2011, 12:44 a.m.
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration-exec.c |   10 +++++-----
 migration-fd.c   |   10 +++++-----
 migration-tcp.c  |   10 +++++-----
 migration-unix.c |   10 +++++-----
 migration.c      |   11 +++++------
 migration.h      |   23 +++++------------------
 6 files changed, 30 insertions(+), 44 deletions(-)
Yoshiaki Tamura - Feb. 23, 2011, 8:07 a.m.
2011/2/23 Juan Quintela <quintela@redhat.com>:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration-exec.c |   10 +++++-----
>  migration-fd.c   |   10 +++++-----
>  migration-tcp.c  |   10 +++++-----
>  migration-unix.c |   10 +++++-----
>  migration.c      |   11 +++++------
>  migration.h      |   23 +++++------------------
>  6 files changed, 30 insertions(+), 44 deletions(-)
>
> diff --git a/migration-exec.c b/migration-exec.c
> index b49a475..02b0667 100644
> --- a/migration-exec.c
> +++ b/migration-exec.c
> @@ -93,12 +93,12 @@ FdMigrationState *exec_start_outgoing_migration(Monitor *mon,
>     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->cancel = migrate_fd_cancel;
> +    s->get_status = migrate_fd_get_status;
> +    s->release = migrate_fd_release;
>
> -    s->mig_state.blk = blk;
> -    s->mig_state.shared = inc;
> +    s->blk = blk;
> +    s->shared = inc;
>
>     s->state = MIG_STATE_ACTIVE;
>     s->mon = NULL;
> diff --git a/migration-fd.c b/migration-fd.c
> index bd5e8a9..ccba86b 100644
> --- a/migration-fd.c
> +++ b/migration-fd.c
> @@ -76,12 +76,12 @@ FdMigrationState *fd_start_outgoing_migration(Monitor *mon,
>     s->get_error = fd_errno;
>     s->write = fd_write;
>     s->close = fd_close;
> -    s->mig_state.cancel = migrate_fd_cancel;
> -    s->mig_state.get_status = migrate_fd_get_status;
> -    s->mig_state.release = migrate_fd_release;
> +    s->cancel = migrate_fd_cancel;
> +    s->get_status = migrate_fd_get_status;
> +    s->release = migrate_fd_release;
>
> -    s->mig_state.blk = blk;
> -    s->mig_state.shared = inc;
> +    s->blk = blk;
> +    s->shared = inc;
>
>     s->state = MIG_STATE_ACTIVE;
>     s->mon = NULL;
> diff --git a/migration-tcp.c b/migration-tcp.c
> index 355bc37..02b01ed 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -95,12 +95,12 @@ FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
>     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;
> +    s->cancel = migrate_fd_cancel;
> +    s->get_status = migrate_fd_get_status;
> +    s->release = migrate_fd_release;
>
> -    s->mig_state.blk = blk;
> -    s->mig_state.shared = inc;
> +    s->blk = blk;
> +    s->shared = inc;
>
>     s->state = MIG_STATE_ACTIVE;
>     s->mon = NULL;
> diff --git a/migration-unix.c b/migration-unix.c
> index b9b0dbf..fb73f46 100644
> --- a/migration-unix.c
> +++ b/migration-unix.c
> @@ -94,12 +94,12 @@ FdMigrationState *unix_start_outgoing_migration(Monitor *mon,
>     s->get_error = unix_errno;
>     s->write = unix_write;
>     s->close = unix_close;
> -    s->mig_state.cancel = migrate_fd_cancel;
> -    s->mig_state.get_status = migrate_fd_get_status;
> -    s->mig_state.release = migrate_fd_release;
> +    s->cancel = migrate_fd_cancel;
> +    s->get_status = migrate_fd_get_status;
> +    s->release = migrate_fd_release;
>
> -    s->mig_state.blk = blk;
> -    s->mig_state.shared = inc;
> +    s->blk = blk;
> +    s->shared = inc;
>
>     s->state = MIG_STATE_ACTIVE;
>     s->mon = NULL;
> diff --git a/migration.c b/migration.c
> index 3a371a3..dd4cdab 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -86,7 +86,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>     const char *uri = qdict_get_str(qdict, "uri");
>
>     if (current_migration &&
> -        current_migration->mig_state.get_status(current_migration) == MIG_STATE_ACTIVE) {
> +        current_migration->get_status(current_migration) == MIG_STATE_ACTIVE) {
>         monitor_printf(mon, "migration already in progress\n");
>         return -1;
>     }
> @@ -120,7 +120,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
>     }
>
>     if (current_migration) {
> -        current_migration->mig_state.release(current_migration);
> +        current_migration->release(current_migration);
>     }
>
>     current_migration = s;
> @@ -133,7 +133,7 @@ int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
>     FdMigrationState *s = current_migration;
>
>     if (s)
> -        s->mig_state.cancel(s);
> +        s->cancel(s);
>
>     return 0;
>  }
> @@ -229,7 +229,7 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
>     QDict *qdict;
>
>     if (current_migration) {
> -        MigrationState *s = &current_migration->mig_state;
> +        FdMigrationState *s = current_migration;
>
>         switch (s->get_status(current_migration)) {
>         case MIG_STATE_ACTIVE:
> @@ -353,8 +353,7 @@ void migrate_fd_connect(FdMigrationState *s)
>                                       migrate_fd_close);
>
>     DPRINTF("beginning savevm\n");
> -    ret = qemu_savevm_state_begin(s->mon, s->file, s->mig_state.blk,
> -                                  s->mig_state.shared);
> +    ret = qemu_savevm_state_begin(s->mon, s->file, s->blk, s->shared);
>     if (ret < 0) {
>         DPRINTF("failed, %d\n", ret);
>         migrate_fd_error(s);
> diff --git a/migration.h b/migration.h
> index f49a9e2..93441e4 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -23,23 +23,10 @@
>  #define MIG_STATE_CANCELLED    1
>  #define MIG_STATE_ACTIVE       2
>
> -typedef struct MigrationState MigrationState;
> -
>  typedef struct FdMigrationState FdMigrationState;
>
> -struct MigrationState
> -{
> -    /* FIXME: add more accessors to print migration info */
> -    void (*cancel)(FdMigrationState *s);
> -    int (*get_status)(FdMigrationState *s);
> -    void (*release)(FdMigrationState *s);
> -    int blk;
> -    int shared;
> -};
> -
>  struct FdMigrationState
>  {
> -    MigrationState mig_state;
>     int64_t bandwidth_limit;
>     QEMUFile *file;
>     int fd;
> @@ -48,7 +35,12 @@ struct FdMigrationState
>     int (*get_error)(struct FdMigrationState*);
>     int (*close)(struct FdMigrationState*);
>     int (*write)(struct FdMigrationState*, const void *, size_t);
> +    void (*cancel)(FdMigrationState *s);
> +    int (*get_status)(FdMigrationState *s);
> +    void (*release)(FdMigrationState *s);
>     void *opaque;
> +    int blk;
> +    int shared;
>  };

Although I don't have objections for folding MigrationState into
FdMigrationState, it would be good to ask why the original author
split it intentionally.

Thanks,

Yoshi

>
>  void process_incoming_migration(QEMUFile *f);
> @@ -130,11 +122,6 @@ void migrate_fd_wait_for_unfreeze(void *opaque);
>
>  int migrate_fd_close(void *opaque);
>
> -static inline FdMigrationState *migrate_to_fms(MigrationState *mig_state)
> -{
> -    return container_of(mig_state, FdMigrationState, mig_state);
> -}
> -
>  void add_migration_state_change_notifier(Notifier *notify);
>  void remove_migration_state_change_notifier(Notifier *notify);
>  int get_migration_state(void);
> --
> 1.7.4
>
>
>
Juan Quintela - Feb. 23, 2011, 9:13 a.m.
Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> wrote:
> 2011/2/23 Juan Quintela <quintela@redhat.com>:

>>  struct FdMigrationState
>>  {
>> -    MigrationState mig_state;
>>     int64_t bandwidth_limit;
>>     QEMUFile *file;
>>     int fd;
>> @@ -48,7 +35,12 @@ struct FdMigrationState
>>     int (*get_error)(struct FdMigrationState*);
>>     int (*close)(struct FdMigrationState*);
>>     int (*write)(struct FdMigrationState*, const void *, size_t);
>> +    void (*cancel)(FdMigrationState *s);
>> +    int (*get_status)(FdMigrationState *s);
>> +    void (*release)(FdMigrationState *s);
>>     void *opaque;
>> +    int blk;
>> +    int shared;
>>  };
>
> Although I don't have objections for folding MigrationState into
> FdMigrationState, it would be good to ask why the original author
> split it intentionally.

I asked, Anthony answer was that it is a "historical" artifact.

Later, Juan.

Patch

diff --git a/migration-exec.c b/migration-exec.c
index b49a475..02b0667 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -93,12 +93,12 @@  FdMigrationState *exec_start_outgoing_migration(Monitor *mon,
     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->cancel = migrate_fd_cancel;
+    s->get_status = migrate_fd_get_status;
+    s->release = migrate_fd_release;

-    s->mig_state.blk = blk;
-    s->mig_state.shared = inc;
+    s->blk = blk;
+    s->shared = inc;

     s->state = MIG_STATE_ACTIVE;
     s->mon = NULL;
diff --git a/migration-fd.c b/migration-fd.c
index bd5e8a9..ccba86b 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -76,12 +76,12 @@  FdMigrationState *fd_start_outgoing_migration(Monitor *mon,
     s->get_error = fd_errno;
     s->write = fd_write;
     s->close = fd_close;
-    s->mig_state.cancel = migrate_fd_cancel;
-    s->mig_state.get_status = migrate_fd_get_status;
-    s->mig_state.release = migrate_fd_release;
+    s->cancel = migrate_fd_cancel;
+    s->get_status = migrate_fd_get_status;
+    s->release = migrate_fd_release;

-    s->mig_state.blk = blk;
-    s->mig_state.shared = inc;
+    s->blk = blk;
+    s->shared = inc;

     s->state = MIG_STATE_ACTIVE;
     s->mon = NULL;
diff --git a/migration-tcp.c b/migration-tcp.c
index 355bc37..02b01ed 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -95,12 +95,12 @@  FdMigrationState *tcp_start_outgoing_migration(Monitor *mon,
     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;
+    s->cancel = migrate_fd_cancel;
+    s->get_status = migrate_fd_get_status;
+    s->release = migrate_fd_release;

-    s->mig_state.blk = blk;
-    s->mig_state.shared = inc;
+    s->blk = blk;
+    s->shared = inc;

     s->state = MIG_STATE_ACTIVE;
     s->mon = NULL;
diff --git a/migration-unix.c b/migration-unix.c
index b9b0dbf..fb73f46 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -94,12 +94,12 @@  FdMigrationState *unix_start_outgoing_migration(Monitor *mon,
     s->get_error = unix_errno;
     s->write = unix_write;
     s->close = unix_close;
-    s->mig_state.cancel = migrate_fd_cancel;
-    s->mig_state.get_status = migrate_fd_get_status;
-    s->mig_state.release = migrate_fd_release;
+    s->cancel = migrate_fd_cancel;
+    s->get_status = migrate_fd_get_status;
+    s->release = migrate_fd_release;

-    s->mig_state.blk = blk;
-    s->mig_state.shared = inc;
+    s->blk = blk;
+    s->shared = inc;

     s->state = MIG_STATE_ACTIVE;
     s->mon = NULL;
diff --git a/migration.c b/migration.c
index 3a371a3..dd4cdab 100644
--- a/migration.c
+++ b/migration.c
@@ -86,7 +86,7 @@  int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
     const char *uri = qdict_get_str(qdict, "uri");

     if (current_migration &&
-        current_migration->mig_state.get_status(current_migration) == MIG_STATE_ACTIVE) {
+        current_migration->get_status(current_migration) == MIG_STATE_ACTIVE) {
         monitor_printf(mon, "migration already in progress\n");
         return -1;
     }
@@ -120,7 +120,7 @@  int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
     }

     if (current_migration) {
-        current_migration->mig_state.release(current_migration);
+        current_migration->release(current_migration);
     }

     current_migration = s;
@@ -133,7 +133,7 @@  int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
     FdMigrationState *s = current_migration;

     if (s)
-        s->mig_state.cancel(s);
+        s->cancel(s);

     return 0;
 }
@@ -229,7 +229,7 @@  void do_info_migrate(Monitor *mon, QObject **ret_data)
     QDict *qdict;

     if (current_migration) {
-        MigrationState *s = &current_migration->mig_state;
+        FdMigrationState *s = current_migration;

         switch (s->get_status(current_migration)) {
         case MIG_STATE_ACTIVE:
@@ -353,8 +353,7 @@  void migrate_fd_connect(FdMigrationState *s)
                                       migrate_fd_close);

     DPRINTF("beginning savevm\n");
-    ret = qemu_savevm_state_begin(s->mon, s->file, s->mig_state.blk,
-                                  s->mig_state.shared);
+    ret = qemu_savevm_state_begin(s->mon, s->file, s->blk, s->shared);
     if (ret < 0) {
         DPRINTF("failed, %d\n", ret);
         migrate_fd_error(s);
diff --git a/migration.h b/migration.h
index f49a9e2..93441e4 100644
--- a/migration.h
+++ b/migration.h
@@ -23,23 +23,10 @@ 
 #define MIG_STATE_CANCELLED	1
 #define MIG_STATE_ACTIVE	2

-typedef struct MigrationState MigrationState;
-
 typedef struct FdMigrationState FdMigrationState;

-struct MigrationState
-{
-    /* FIXME: add more accessors to print migration info */
-    void (*cancel)(FdMigrationState *s);
-    int (*get_status)(FdMigrationState *s);
-    void (*release)(FdMigrationState *s);
-    int blk;
-    int shared;
-};
-
 struct FdMigrationState
 {
-    MigrationState mig_state;
     int64_t bandwidth_limit;
     QEMUFile *file;
     int fd;
@@ -48,7 +35,12 @@  struct FdMigrationState
     int (*get_error)(struct FdMigrationState*);
     int (*close)(struct FdMigrationState*);
     int (*write)(struct FdMigrationState*, const void *, size_t);
+    void (*cancel)(FdMigrationState *s);
+    int (*get_status)(FdMigrationState *s);
+    void (*release)(FdMigrationState *s);
     void *opaque;
+    int blk;
+    int shared;
 };

 void process_incoming_migration(QEMUFile *f);
@@ -130,11 +122,6 @@  void migrate_fd_wait_for_unfreeze(void *opaque);

 int migrate_fd_close(void *opaque);

-static inline FdMigrationState *migrate_to_fms(MigrationState *mig_state)
-{
-    return container_of(mig_state, FdMigrationState, mig_state);
-}
-
 void add_migration_state_change_notifier(Notifier *notify);
 void remove_migration_state_change_notifier(Notifier *notify);
 int get_migration_state(void);