qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 17/23] migration: make sure we always have a mig


From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH 17/23] migration: make sure we always have a migration state
Date: Tue, 04 Oct 2011 09:29:01 -0500
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.21) Gecko/20110831 Lightning/1.0b2 Thunderbird/3.1.13

On 09/23/2011 07:57 AM, Juan Quintela wrote:
This cleans up a lot the code as we don't have to check anymore if
the variable is NULL or not.

We don't make it static, because when we integrate fault tolerance, we
can have several migrations at once.

Signed-off-by: Juan Quintela<address@hidden>
---
  migration.c |  126 +++++++++++++++++++++++++---------------------------------
  1 files changed, 54 insertions(+), 72 deletions(-)

diff --git a/migration.c b/migration.c
index 1cc21f0..c382383 100644
--- a/migration.c
+++ b/migration.c
@@ -34,7 +34,13 @@
  /* Migration speed throttling */
  static int64_t max_throttle = (32<<  20);

-static MigrationState *current_migration;
+/* When we add fault tolerance, we could have several
+   migrations at once.  For now we don't need to add
+   dynamic creation of migration */
+static MigrationState current_migration_static = {
+    .state = MIG_STATE_NONE,
+};
+static MigrationState *current_migration =&current_migration_static;

This doesn't make sense to me. We can have two migration states that are both in the MIG_STATE_NONE? What does that actually mean?

I don't see the point in making migration state nullable via the state member other than avoiding the if (s == NULL) check.

Regards,

Anthony Liguori


  static NotifierList migration_state_notifiers =
      NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
@@ -136,37 +142,34 @@ void do_info_migrate(Monitor *mon, QObject **ret_data)
  {
      QDict *qdict;

-    if (current_migration) {
-
-        switch (current_migration->state) {
-        case MIG_STATE_NONE:
-            /* no migration has happened ever */
-            break;
-        case MIG_STATE_ACTIVE:
-            qdict = qdict_new();
-            qdict_put(qdict, "status", qstring_from_str("active"));
-
-            migrate_put_status(qdict, "ram", ram_bytes_transferred(),
-                               ram_bytes_remaining(), ram_bytes_total());
-
-            if (blk_mig_active()) {
-                migrate_put_status(qdict, "disk", blk_mig_bytes_transferred(),
-                                   blk_mig_bytes_remaining(),
-                                   blk_mig_bytes_total());
-            }
-
-            *ret_data = QOBJECT(qdict);
-            break;
-        case MIG_STATE_COMPLETED:
-            *ret_data = qobject_from_jsonf("{ 'status': 'completed' }");
-            break;
-        case MIG_STATE_ERROR:
-            *ret_data = qobject_from_jsonf("{ 'status': 'failed' }");
-            break;
-        case MIG_STATE_CANCELLED:
-            *ret_data = qobject_from_jsonf("{ 'status': 'cancelled' }");
-            break;
+    switch (current_migration->state) {
+    case MIG_STATE_NONE:
+        /* no migration has happened ever */
+        break;
+    case MIG_STATE_ACTIVE:
+        qdict = qdict_new();
+        qdict_put(qdict, "status", qstring_from_str("active"));
+
+        migrate_put_status(qdict, "ram", ram_bytes_transferred(),
+                           ram_bytes_remaining(), ram_bytes_total());
+
+        if (blk_mig_active()) {
+            migrate_put_status(qdict, "disk", blk_mig_bytes_transferred(),
+                               blk_mig_bytes_remaining(),
+                               blk_mig_bytes_total());
          }
+
+        *ret_data = QOBJECT(qdict);
+        break;
+    case MIG_STATE_COMPLETED:
+        *ret_data = qobject_from_jsonf("{ 'status': 'completed' }");
+        break;
+    case MIG_STATE_ERROR:
+        *ret_data = qobject_from_jsonf("{ 'status': 'failed' }");
+        break;
+    case MIG_STATE_CANCELLED:
+        *ret_data = qobject_from_jsonf("{ 'status': 'cancelled' }");
+        break;
      }
  }

@@ -357,11 +360,7 @@ void remove_migration_state_change_notifier(Notifier 
*notify)

  int get_migration_state(void)
  {
-    if (current_migration) {
-        return current_migration->state;
-    } else {
-        return MIG_STATE_ERROR;
-    }
+    return current_migration->state;
  }

  void migrate_fd_connect(MigrationState *s)
@@ -386,28 +385,23 @@ void migrate_fd_connect(MigrationState *s)
      migrate_fd_put_ready(s);
  }

-static MigrationState *migrate_create_state(Monitor *mon,
-                                            int64_t bandwidth_limit,
-                                            int detach, int blk, int inc)
+static void migrate_init_state(Monitor *mon, int64_t bandwidth_limit,
+                               int detach, int blk, int inc)
  {
-    MigrationState *s = g_malloc0(sizeof(*s));
-
-    s->blk = blk;
-    s->shared = inc;
-    s->mon = NULL;
-    s->bandwidth_limit = bandwidth_limit;
-    s->state = MIG_STATE_NONE;
+    memset(current_migration, 0, sizeof(current_migration));
+    current_migration->blk = blk;
+    current_migration->shared = inc;
+    current_migration->mon = NULL;
+    current_migration->bandwidth_limit = bandwidth_limit;
+    current_migration->state = MIG_STATE_NONE;

      if (!detach) {
-        migrate_fd_monitor_suspend(s, mon);
+        migrate_fd_monitor_suspend(current_migration, mon);
      }
-
-    return s;
  }

  int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
  {
-    MigrationState *s = NULL;
      const char *p;
      int detach = qdict_get_try_bool(qdict, "detach", 0);
      int blk = qdict_get_try_bool(qdict, "blk", 0);
@@ -415,8 +409,7 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
      const char *uri = qdict_get_str(qdict, "uri");
      int ret;

-    if (current_migration&&
-        current_migration->state == MIG_STATE_ACTIVE) {
+    if (current_migration->state == MIG_STATE_ACTIVE) {
          monitor_printf(mon, "migration already in progress\n");
          return -1;
      }
@@ -425,43 +418,35 @@ int do_migrate(Monitor *mon, const QDict *qdict, QObject 
**ret_data)
          return -1;
      }

-    s = migrate_create_state(mon, max_throttle, detach, blk, inc);
+    migrate_init_state(mon, max_throttle, detach, blk, inc);

      if (strstart(uri, "tcp:",&p)) {
-        ret = tcp_start_outgoing_migration(s, p);
+        ret = tcp_start_outgoing_migration(current_migration, p);
  #if !defined(WIN32)
      } else if (strstart(uri, "exec:",&p)) {
-        ret = exec_start_outgoing_migration(s, p);
+        ret = exec_start_outgoing_migration(current_migration, p);
      } else if (strstart(uri, "unix:",&p)) {
-        ret = unix_start_outgoing_migration(s, p);
+        ret = unix_start_outgoing_migration(current_migration, p);
      } else if (strstart(uri, "fd:",&p)) {
-        ret = fd_start_outgoing_migration(s, p);
+        ret = fd_start_outgoing_migration(current_migration, p);
  #endif
      } else {
          monitor_printf(mon, "unknown migration protocol: %s\n", uri);
-        ret  = -EINVAL;
-        goto free_migrate_state;
+        return -EINVAL;
      }

      if (ret<  0) {
          monitor_printf(mon, "migration failed\n");
-        goto free_migrate_state;
+        return ret;
      }

-    g_free(current_migration);
-    current_migration = s;
      notifier_list_notify(&migration_state_notifiers, NULL);
      return 0;
-free_migrate_state:
-    g_free(s);
-    return -1;
  }

  int do_migrate_cancel(Monitor *mon, const QDict *qdict, QObject **ret_data)
  {
-    if (current_migration) {
-        migrate_fd_cancel(current_migration);
-    }
+    migrate_fd_cancel(current_migration);
      return 0;
  }

@@ -475,10 +460,7 @@ int do_migrate_set_speed(Monitor *mon, const QDict *qdict, 
QObject **ret_data)
      }
      max_throttle = d;

-    if (current_migration) {
-        qemu_file_set_rate_limit(current_migration->file, max_throttle);
-    }
-
+    qemu_file_set_rate_limit(current_migration->file, max_throttle);
      return 0;
  }





reply via email to

[Prev in Thread] Current Thread [Next in Thread]