[Qemu-devel] [PATCH 0/5] Add QMP migration events

[Qemu-devel] [PATCH 0/5] Add QMP migration events

[Juan Quintela]

Hi

This series does:

- exit incoming migration on failure.  For exec/fd migrations, once
  there was a failure, there was nothing useful to do.  And for tcp
  migration, not exiting created interesting bugs when trying to
  migrate again to a process with a faild migration.

- Factorize common migration code, no more duplication, makes easier to do
  "global" migration things, like QMP events.

- Introduce QMP events, both for incoming and outgoing migration.


Now, the million dollar question: Why I didn't refactorize outgoing
migration?  I tried, and have it partially done on my local tree.  But
it depends (too much) of current_migration global variable -> Libvirt
folks will also want "info migrate" to work on the incoming side,
i.e. current_migraition has to also be updated on incoming side.  Done
until here, but then I hit the wall "incoming migration is synchronous".

To make the monitor work on incoming migration, we need to change
buffered_file.c abstraction to also work for incoming fd's, or another
similar solution.  I am open to suggestions about what to do here.

This series are quite simple (the unfinished part is more complex),
will send the other part as an RFC later.

Please review and consider to apply it.

Later, Juan.

Juan Quintela (5):
  Exit if incoming migration fails
  Factorize common migration incoming code
  QMP: Introduce MIGRATION events
  QMP: Emit migration events on incoming migration
  QMP: Emit migration events on outgoing migration

 QMP/qmp-events.txt |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 migration-exec.c   |   17 +++--------------
 migration-fd.c     |   15 ++-------------
 migration-tcp.c    |   17 ++++-------------
 migration-unix.c   |   17 ++++-------------
 migration.c        |   35 ++++++++++++++++++++++++++++-------
 migration.h        |    4 +++-
 monitor.c          |   12 ++++++++++++
 monitor.h          |    4 ++++
 vl.c               |    5 ++++-
 10 files changed, 114 insertions(+), 62 deletions(-)

[Qemu-devel] [PATCH 1/5] Exit if incoming migration fails

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |   14 +++++++-------
 migration.h |    2 +-
 vl.c        |    5 ++++-
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/migration.c b/migration.c
index 05f6cc5..cf30a8e 100644
--- a/migration.c
+++ b/migration.c
@@ -36,22 +36,22 @@ static uint32_t max_throttle = (32 << 20);

 static MigrationState *current_migration;

-void qemu_start_incoming_migration(const char *uri)
+int qemu_start_incoming_migration(const char *uri)
 {
     const char *p;
+    int ret = -1;

     if (strstart(uri, "tcp:", &p))
-        tcp_start_incoming_migration(p);
+        ret = tcp_start_incoming_migration(p);
 #if !defined(WIN32)
     else if (strstart(uri, "exec:", &p))
-        exec_start_incoming_migration(p);
+        ret =  exec_start_incoming_migration(p);
     else if (strstart(uri, "unix:", &p))
-        unix_start_incoming_migration(p);
+        ret = unix_start_incoming_migration(p);
     else if (strstart(uri, "fd:", &p))
-        fd_start_incoming_migration(p);
+        ret = fd_start_incoming_migration(p);
 #endif
-    else
-        fprintf(stderr, "unknown migration protocol: %s\n", uri);
+    return ret;
 }

 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
diff --git a/migration.h b/migration.h
index 385423f..dd423a1 100644
--- a/migration.h
+++ b/migration.h
@@ -50,7 +50,7 @@ struct FdMigrationState
     void *opaque;
 };

-void qemu_start_incoming_migration(const char *uri);
+int qemu_start_incoming_migration(const char *uri);

 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data);

diff --git a/vl.c b/vl.c
index d77b47c..3dfab9e 100644
--- a/vl.c
+++ b/vl.c
@@ -3841,7 +3841,10 @@ int main(int argc, char **argv, char **envp)
     }

     if (incoming) {
-        qemu_start_incoming_migration(incoming);
+        if (qemu_start_incoming_migration(incoming) < 0) {
+            fprintf(stderr, "unknown migration protocol: %s\n", incoming);
+            exit(1);
+        }
     } else if (autostart) {
         vm_start();
     }
-- 
1.6.6.1

[Qemu-devel] Re: [PATCH 1/5] Exit if incoming migration fails

[Paolo Bonzini]

> -void qemu_start_incoming_migration(const char *uri)
> +int qemu_start_incoming_migration(const char *uri)
>   {
>       const char *p;
> +    int ret = -1;

Maybe -ENOSYS or -EPROTONOSUPPORT, since the *_start_incoming_migration 
functions return a negative errno value?

>       if (incoming) {
> -        qemu_start_incoming_migration(incoming);
> +        if (qemu_start_incoming_migration(incoming)<  0) {
> +            fprintf(stderr, "unknown migration protocol: %s\n", incoming);
> +            exit(1);
> +        }

Dually, if you have a failure from *_start_incoming_migration, this 
fprintf is spurious.

Paolo

[Qemu-devel] Re: [PATCH 1/5] Exit if incoming migration fails

[Michael S. Tsirkin]

On Mon, May 24, 2010 at 10:25:25AM +0200, Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration.c |   14 +++++++-------
>  migration.h |    2 +-
>  vl.c        |    5 ++++-
>  3 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/migration.c b/migration.c
> index 05f6cc5..cf30a8e 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -36,22 +36,22 @@ static uint32_t max_throttle = (32 << 20);
> 
>  static MigrationState *current_migration;
> 
> -void qemu_start_incoming_migration(const char *uri)
> +int qemu_start_incoming_migration(const char *uri)
>  {
>      const char *p;
> +    int ret = -1;
> 
>      if (strstart(uri, "tcp:", &p))
> -        tcp_start_incoming_migration(p);
> +        ret = tcp_start_incoming_migration(p);
>  #if !defined(WIN32)
>      else if (strstart(uri, "exec:", &p))
> -        exec_start_incoming_migration(p);
> +        ret =  exec_start_incoming_migration(p);
>      else if (strstart(uri, "unix:", &p))
> -        unix_start_incoming_migration(p);
> +        ret = unix_start_incoming_migration(p);
>      else if (strstart(uri, "fd:", &p))
> -        fd_start_incoming_migration(p);
> +        ret = fd_start_incoming_migration(p);
>  #endif
> -    else
> -        fprintf(stderr, "unknown migration protocol: %s\n", uri);

I think we need this message, this is where we know
reason for failure is bad protocol.

> +    return ret;
>  }
> 
>  int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
> diff --git a/migration.h b/migration.h
> index 385423f..dd423a1 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -50,7 +50,7 @@ struct FdMigrationState
>      void *opaque;
>  };
> 
> -void qemu_start_incoming_migration(const char *uri);
> +int qemu_start_incoming_migration(const char *uri);
> 
>  int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data);
> 
> diff --git a/vl.c b/vl.c
> index d77b47c..3dfab9e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3841,7 +3841,10 @@ int main(int argc, char **argv, char **envp)
>      }
> 
>      if (incoming) {
> -        qemu_start_incoming_migration(incoming);
> +        if (qemu_start_incoming_migration(incoming) < 0) {
> +            fprintf(stderr, "unknown migration protocol: %s\n", incoming);
> +            exit(1);
> +        }

Let's put here: 'Migration failed. Exit code %s(%d), exiting.\n'
and then exit(ret);

>      } else if (autostart) {
>          vm_start();
>      }
> -- 
> 1.6.6.1
> 

[Qemu-devel] [PATCH 2/5] Factorize common migration incoming code

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration-exec.c |   14 +-------------
 migration-fd.c   |   14 +-------------
 migration-tcp.c  |   15 ++-------------
 migration-unix.c |   15 ++-------------
 migration.c      |   13 +++++++++++++
 migration.h      |    2 ++
 6 files changed, 21 insertions(+), 52 deletions(-)

diff --git a/migration-exec.c b/migration-exec.c
index 5435827..07af11a 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -111,20 +111,8 @@ err_after_alloc:
 static void exec_accept_incoming_migration(void *opaque)
 {
     QEMUFile *f = opaque;
-    int ret;

-    ret = qemu_loadvm_state(f);
-    if (ret < 0) {
-        fprintf(stderr, "load of migration failed\n");
-        goto err;
-    }
-    qemu_announce_self();
-    DPRINTF("successfully loaded vm state\n");
-
-    if (autostart)
-        vm_start();
-
-err:
+    process_incoming_migration(f);
     qemu_set_fd_handler2(qemu_stdio_fd(f), NULL, NULL, NULL, NULL);
     qemu_fclose(f);
 }
diff --git a/migration-fd.c b/migration-fd.c
index 0abd372..6d14505 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -104,20 +104,8 @@ err_after_alloc:
 static void fd_accept_incoming_migration(void *opaque)
 {
     QEMUFile *f = opaque;
-    int ret;

-    ret = qemu_loadvm_state(f);
-    if (ret < 0) {
-        fprintf(stderr, "load of migration failed\n");
-        goto err;
-    }
-    qemu_announce_self();
-    DPRINTF("successfully loaded vm state\n");
-
-    if (autostart)
-        vm_start();
-
-err:
+    process_incoming_migration(f);
     qemu_set_fd_handler2(qemu_stdio_fd(f), NULL, NULL, NULL, NULL);
     qemu_fclose(f);
 }
diff --git a/migration-tcp.c b/migration-tcp.c
index 95ce722..20f2e37 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -143,7 +143,7 @@ static void tcp_accept_incoming_migration(void *opaque)
     socklen_t addrlen = sizeof(addr);
     int s = (unsigned long)opaque;
     QEMUFile *f;
-    int c, ret;
+    int c;

     do {
         c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
@@ -162,18 +162,7 @@ static void tcp_accept_incoming_migration(void *opaque)
         goto out;
     }

-    ret = qemu_loadvm_state(f);
-    if (ret < 0) {
-        fprintf(stderr, "load of migration failed\n");
-        goto out_fopen;
-    }
-    qemu_announce_self();
-    DPRINTF("successfully loaded vm state\n");
-
-    if (autostart)
-        vm_start();
-
-out_fopen:
+    process_incoming_migration(f);
     qemu_fclose(f);
 out:
     qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
diff --git a/migration-unix.c b/migration-unix.c
index 49de1b9..57232c0 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -149,7 +149,7 @@ static void unix_accept_incoming_migration(void *opaque)
     socklen_t addrlen = sizeof(addr);
     int s = (unsigned long)opaque;
     QEMUFile *f;
-    int c, ret;
+    int c;

     do {
         c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
@@ -168,18 +168,7 @@ static void unix_accept_incoming_migration(void *opaque)
         goto out;
     }

-    ret = qemu_loadvm_state(f);
-    if (ret < 0) {
-        fprintf(stderr, "load of migration failed\n");
-        goto out_fopen;
-    }
-    qemu_announce_self();
-    DPRINTF("successfully loaded vm state\n");
-
-    if (autostart)
-        vm_start();
-
-out_fopen:
+    process_incoming_migration(f);
     qemu_fclose(f);
 out:
     qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
diff --git a/migration.c b/migration.c
index cf30a8e..6ab5d90 100644
--- a/migration.c
+++ b/migration.c
@@ -54,6 +54,19 @@ int qemu_start_incoming_migration(const char *uri)
     return ret;
 }

+void process_incoming_migration(QEMUFile *f)
+{
+    if (qemu_loadvm_state(f) < 0) {
+        fprintf(stderr, "load of migration failed\n");
+        exit(0);
+    }
+    qemu_announce_self();
+    DPRINTF("successfully loaded vm state\n");
+
+    if (autostart)
+        vm_start();
+}
+
 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
     MigrationState *s = NULL;
diff --git a/migration.h b/migration.h
index dd423a1..017e9c3 100644
--- a/migration.h
+++ b/migration.h
@@ -50,6 +50,8 @@ struct FdMigrationState
     void *opaque;
 };

+void process_incoming_migration(QEMUFile *f);
+
 int qemu_start_incoming_migration(const char *uri);

 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data);
-- 
1.6.6.1

[Qemu-devel] [PATCH 3/5] QMP: Introduce MIGRATION events

[Juan Quintela]

They are emitted when migration starts, ends, has a failure or is canceled.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 QMP/qmp-events.txt |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 monitor.c          |   12 ++++++++++++
 monitor.h          |    4 ++++
 3 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 01ec85f..234360f 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -26,6 +26,56 @@ Example:
 Note: If action is "stop", a STOP event will eventually follow the
 BLOCK_IO_ERROR event.

+MIGRATION_CANCELED
+------------------
+
+Emitted when migration is canceled.  This is emitted in the source.
+Target will emit MIGRATION_CANCELED (no way to differentiate a FAILED
+and CANCELED migration).
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_CANCELED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_ENDED
+---------------
+
+Emitted when migration starts (both in source and target)
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_ENDED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_FAILED
+----------------
+
+Emitted when migration fails (both is source and target).
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_FAILED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_STARTED
+-----------------
+
+Emitted when migration starts (both in source and target).
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_STARTED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
 RESET
 -----

diff --git a/monitor.c b/monitor.c
index a1ebc5d..723ca73 100644
--- a/monitor.c
+++ b/monitor.c
@@ -444,6 +444,18 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
         case QEVENT_WATCHDOG:
             event_name = "WATCHDOG";
             break;
+        case QEVENT_MIGRATION_STARTED:
+            event_name = "MIGRATION_STARTED";
+            break;
+        case QEVENT_MIGRATION_ENDED:
+            event_name = "MIGRATION_ENDED";
+            break;
+        case QEVENT_MIGRATION_FAILED:
+            event_name = "MIGRATION_FAILED";
+            break;
+        case QEVENT_MIGRATION_CANCELED:
+            event_name = "MIGRATION_CANCELED";
+            break;
         default:
             abort();
             break;
diff --git a/monitor.h b/monitor.h
index ea15469..34bcd38 100644
--- a/monitor.h
+++ b/monitor.h
@@ -28,6 +28,10 @@ typedef enum MonitorEvent {
     QEVENT_BLOCK_IO_ERROR,
     QEVENT_RTC_CHANGE,
     QEVENT_WATCHDOG,
+    QEVENT_MIGRATION_STARTED,
+    QEVENT_MIGRATION_ENDED,
+    QEVENT_MIGRATION_FAILED,
+    QEVENT_MIGRATION_CANCELED,
     QEVENT_MAX,
 } MonitorEvent;

-- 
1.6.6.1

[Qemu-devel] Re: [PATCH 3/5] QMP: Introduce MIGRATION events

[Paolo Bonzini]

On 05/24/2010 10:25 AM, Juan Quintela wrote:
> +MIGRATION_CANCELED
> +------------------
> +
> +Emitted when migration is canceled.  This is emitted in the source.
> +Target will emit MIGRATION_CANCELED (no way to differentiate a FAILED
> +and CANCELED migration).

Copy-paste error? (or if not, parse error on my side).

> +Data: None
> +
> +Example:
> +
> +{ "event": "MIGRATION_CANCELED",
> +    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
> +
> +MIGRATION_ENDED
> +---------------
> +
> +Emitted when migration starts (both in source and target)

Likewise.

Paolo

[Qemu-devel] [PATCH 4/5] QMP: Emit migration events on incoming migration

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/migration.c b/migration.c
index 6ab5d90..7fba993 100644
--- a/migration.c
+++ b/migration.c
@@ -56,10 +56,13 @@ int qemu_start_incoming_migration(const char *uri)

 void process_incoming_migration(QEMUFile *f)
 {
+    monitor_protocol_event(QEVENT_MIGRATION_STARTED, NULL);
     if (qemu_loadvm_state(f) < 0) {
+        monitor_protocol_event(QEVENT_MIGRATION_FAILED, NULL);
         fprintf(stderr, "load of migration failed\n");
         exit(0);
     }
+    monitor_protocol_event(QEVENT_MIGRATION_ENDED, NULL);
     qemu_announce_self();
     DPRINTF("successfully loaded vm state\n");

-- 
1.6.6.1

[Qemu-devel] [PATCH 5/5] QMP: Emit migration events on outgoing migration

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration-exec.c |    3 ++-
 migration-fd.c   |    1 +
 migration-tcp.c  |    2 ++
 migration-unix.c |    2 ++
 migration.c      |    5 +++++
 5 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/migration-exec.c b/migration-exec.c
index 07af11a..ebc9256 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -20,6 +20,7 @@
 #include "sysemu.h"
 #include "buffered_file.h"
 #include "block.h"
+#include "monitor.h"

 //#define DEBUG_MIGRATION_EXEC

@@ -91,9 +92,9 @@ MigrationState *exec_start_outgoing_migration(Monitor *mon,
     s->mig_state.shared = inc;

     s->state = MIG_STATE_ACTIVE;
+    monitor_protocol_event(QEVENT_MIGRATION_STARTED, NULL);
     s->mon = NULL;
     s->bandwidth_limit = bandwidth_limit;
-
     if (!detach) {
         migrate_fd_monitor_suspend(s, mon);
     }
diff --git a/migration-fd.c b/migration-fd.c
index 6d14505..9c4c7ae 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -83,6 +83,7 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
     s->mig_state.blk = blk;
     s->mig_state.shared = inc;

+    monitor_protocol_event(QEVENT_MIGRATION_STARTED, NULL);
     s->state = MIG_STATE_ACTIVE;
     s->mon = NULL;
     s->bandwidth_limit = bandwidth_limit;
diff --git a/migration-tcp.c b/migration-tcp.c
index 20f2e37..11a1203 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -18,6 +18,7 @@
 #include "sysemu.h"
 #include "buffered_file.h"
 #include "block.h"
+#include "monitor.h"

 //#define DEBUG_MIGRATION_TCP

@@ -102,6 +103,7 @@ MigrationState *tcp_start_outgoing_migration(Monitor *mon,
     s->mig_state.blk = blk;
     s->mig_state.shared = inc;

+    monitor_protocol_event(QEVENT_MIGRATION_STARTED, NULL);
     s->state = MIG_STATE_ACTIVE;
     s->mon = NULL;
     s->bandwidth_limit = bandwidth_limit;
diff --git a/migration-unix.c b/migration-unix.c
index 57232c0..08f29a3 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -18,6 +18,7 @@
 #include "sysemu.h"
 #include "buffered_file.h"
 #include "block.h"
+#include "monitor.h"

 //#define DEBUG_MIGRATION_UNIX

@@ -101,6 +102,7 @@ MigrationState *unix_start_outgoing_migration(Monitor *mon,
     s->mig_state.blk = blk;
     s->mig_state.shared = inc;

+    monitor_protocol_event(QEVENT_MIGRATION_STARTED, NULL);
     s->state = MIG_STATE_ACTIVE;
     s->mon = NULL;
     s->bandwidth_limit = bandwidth_limit;
diff --git a/migration.c b/migration.c
index 7fba993..ff84504 100644
--- a/migration.c
+++ b/migration.c
@@ -302,6 +302,7 @@ void migrate_fd_monitor_suspend(FdMigrationState *s, Monitor *mon)
 void migrate_fd_error(FdMigrationState *s)
 {
     DPRINTF("setting error state\n");
+    monitor_protocol_event(QEVENT_MIGRATION_FAILED, NULL);
     s->state = MIG_STATE_ERROR;
     migrate_fd_cleanup(s);
 }
@@ -399,8 +400,10 @@ void migrate_fd_put_ready(void *opaque)
             if (old_vm_running) {
                 vm_start();
             }
+            monitor_protocol_event(QEVENT_MIGRATION_FAILED, NULL);
             state = MIG_STATE_ERROR;
         } else {
+            monitor_protocol_event(QEVENT_MIGRATION_ENDED, NULL);
             state = MIG_STATE_COMPLETED;
         }
         migrate_fd_cleanup(s);
@@ -423,6 +426,7 @@ void migrate_fd_cancel(MigrationState *mig_state)

     DPRINTF("cancelling migration\n");

+    monitor_protocol_event(QEVENT_MIGRATION_CANCELED, NULL);
     s->state = MIG_STATE_CANCELLED;
     qemu_savevm_state_cancel(s->mon, s->file);

@@ -436,6 +440,7 @@ void migrate_fd_release(MigrationState *mig_state)
     DPRINTF("releasing state\n");
    
     if (s->state == MIG_STATE_ACTIVE) {
+        monitor_protocol_event(QEVENT_MIGRATION_CANCELED, NULL);
         s->state = MIG_STATE_CANCELLED;
         migrate_fd_cleanup(s);
     }
-- 
1.6.6.1

[Qemu-devel] [PATCH v2 0/5] Add QMP migration events

[Juan Quintela]

v2:
- Address pbonzini and mst changes
  (error messages and doc fixes)

v1:

This series does:

- exit incoming migration on failure.  For exec/fd migrations, once
  there was a failure, there was nothing useful to do.  And for tcp
  migration, not exiting created interesting bugs when trying to
  migrate again to a process with a faild migration.

- Factorize common migration code, no more duplication, makes easier to do
  "global" migration things, like QMP events.

- Introduce QMP events, both for incoming and outgoing migration.


Now, the million dollar question: Why I didn't refactorize outgoing
migration?  I tried, and have it partially done on my local tree.  But
it depends (too much) of current_migration global variable -> Libvirt
folks will also want "info migrate" to work on the incoming side,
i.e. current_migraition has to also be updated on incoming side.  Done
until here, but then I hit the wall "incoming migration is synchronous".

To make the monitor work on incoming migration, we need to change
buffered_file.c abstraction to also work for incoming fd's, or another
similar solution.  I am open to suggestions about what to do here.

This series are quite simple (the unfinished part is more complex),
will send the other part as an RFC later.

Please review and consider to apply it.

Later, Juan.


Juan Quintela (5):
  Exit if incoming migration fails
  Factorize common migration incoming code
  QMP: Introduce MIGRATION events
  QMP: Emit migration events on incoming migration
  QMP: Emit migration events on outgoing migration

 QMP/qmp-events.txt |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 migration-exec.c   |   17 +++--------------
 migration-fd.c     |   15 ++-------------
 migration-tcp.c    |   17 ++++-------------
 migration-unix.c   |   17 ++++-------------
 migration.c        |   37 +++++++++++++++++++++++++++++++------
 migration.h        |    4 +++-
 monitor.c          |   12 ++++++++++++
 monitor.h          |    4 ++++
 vl.c               |    7 ++++++-
 10 files changed, 119 insertions(+), 61 deletions(-)

[Qemu-devel] [PATCH 1/5] Exit if incoming migration fails

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration.c |   16 ++++++++++------
 migration.h |    2 +-
 vl.c        |    7 ++++++-
 3 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/migration.c b/migration.c
index 05f6cc5..9c1d4b6 100644
--- a/migration.c
+++ b/migration.c
@@ -36,22 +36,26 @@ static uint32_t max_throttle = (32 << 20);

 static MigrationState *current_migration;

-void qemu_start_incoming_migration(const char *uri)
+int qemu_start_incoming_migration(const char *uri)
 {
     const char *p;
+    int ret;

     if (strstart(uri, "tcp:", &p))
-        tcp_start_incoming_migration(p);
+        ret = tcp_start_incoming_migration(p);
 #if !defined(WIN32)
     else if (strstart(uri, "exec:", &p))
-        exec_start_incoming_migration(p);
+        ret =  exec_start_incoming_migration(p);
     else if (strstart(uri, "unix:", &p))
-        unix_start_incoming_migration(p);
+        ret = unix_start_incoming_migration(p);
     else if (strstart(uri, "fd:", &p))
-        fd_start_incoming_migration(p);
+        ret = fd_start_incoming_migration(p);
 #endif
-    else
+    else {
         fprintf(stderr, "unknown migration protocol: %s\n", uri);
+        ret = -EPROTONOSUPPORT;
+    }
+    return ret;
 }

 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
diff --git a/migration.h b/migration.h
index 385423f..dd423a1 100644
--- a/migration.h
+++ b/migration.h
@@ -50,7 +50,7 @@ struct FdMigrationState
     void *opaque;
 };

-void qemu_start_incoming_migration(const char *uri);
+int qemu_start_incoming_migration(const char *uri);

 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data);

diff --git a/vl.c b/vl.c
index 328395e..d13440d 100644
--- a/vl.c
+++ b/vl.c
@@ -3823,7 +3823,12 @@ int main(int argc, char **argv, char **envp)
     }

     if (incoming) {
-        qemu_start_incoming_migration(incoming);
+        int ret = qemu_start_incoming_migration(incoming);
+        if (ret < 0) {
+            fprintf(stderr, "Migration failed. Exit code %s(%d), exiting.\n",
+                    incoming, ret);
+            exit(ret);
+        }
     } else if (autostart) {
         vm_start();
     }
-- 
1.6.6.1

Re: [Qemu-devel] [PATCH 1/5] Exit if incoming migration fails

[Luiz Capitulino]

On Tue, 25 May 2010 16:21:01 +0200
Juan Quintela <quintela@redhat.com> wrote:

> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  migration.c |   16 ++++++++++------
>  migration.h |    2 +-
>  vl.c        |    7 ++++++-
>  3 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/migration.c b/migration.c
> index 05f6cc5..9c1d4b6 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -36,22 +36,26 @@ static uint32_t max_throttle = (32 << 20);
> 
>  static MigrationState *current_migration;
> 
> -void qemu_start_incoming_migration(const char *uri)
> +int qemu_start_incoming_migration(const char *uri)
>  {
>      const char *p;
> +    int ret;
> 
>      if (strstart(uri, "tcp:", &p))
> -        tcp_start_incoming_migration(p);
> +        ret = tcp_start_incoming_migration(p);
>  #if !defined(WIN32)
>      else if (strstart(uri, "exec:", &p))
> -        exec_start_incoming_migration(p);
> +        ret =  exec_start_incoming_migration(p);
>      else if (strstart(uri, "unix:", &p))
> -        unix_start_incoming_migration(p);
> +        ret = unix_start_incoming_migration(p);
>      else if (strstart(uri, "fd:", &p))
> -        fd_start_incoming_migration(p);
> +        ret = fd_start_incoming_migration(p);
>  #endif
> -    else
> +    else {
>          fprintf(stderr, "unknown migration protocol: %s\n", uri);
> +        ret = -EPROTONOSUPPORT;
> +    }
> +    return ret;
>  }
> 
>  int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
> diff --git a/migration.h b/migration.h
> index 385423f..dd423a1 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -50,7 +50,7 @@ struct FdMigrationState
>      void *opaque;
>  };
> 
> -void qemu_start_incoming_migration(const char *uri);
> +int qemu_start_incoming_migration(const char *uri);
> 
>  int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data);
> 
> diff --git a/vl.c b/vl.c
> index 328395e..d13440d 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3823,7 +3823,12 @@ int main(int argc, char **argv, char **envp)
>      }
> 
>      if (incoming) {
> -        qemu_start_incoming_migration(incoming);
> +        int ret = qemu_start_incoming_migration(incoming);
> +        if (ret < 0) {
> +            fprintf(stderr, "Migration failed. Exit code %s(%d), exiting.\n",
> +                    incoming, ret);
> +            exit(ret);

 While I agree on the change, I have two comments:

1. By taking a look at the code I have the impression that most of the
   fun failures will happen on the handler passed to qemu_set_fd_handler2(),
   do you agree? Any plan to address that?

1. Is exit()ing the best thing to be done? I understand it's the easiest
   and maybe better than nothing, but wouldn't it be better to enter in
   paused-forever state so that clients can query and decide what to do?

> +        }
>      } else if (autostart) {
>          vm_start();
>      }

[Qemu-devel] Re: [PATCH 1/5] Exit if incoming migration fails

[Juan Quintela]

Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Tue, 25 May 2010 16:21:01 +0200
> Juan Quintela <quintela@redhat.com> wrote:
>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  migration.c |   16 ++++++++++------
>>  migration.h |    2 +-
>>  vl.c        |    7 ++++++-
>>  3 files changed, 17 insertions(+), 8 deletions(-)
>> 

>  While I agree on the change, I have two comments:
>
> 1. By taking a look at the code I have the impression that most of the
>    fun failures will happen on the handler passed to qemu_set_fd_handler2(),
>    do you agree? Any plan to address that?

That is outgoing migration, not incoming migration.
Incoming migration in synchronous..


> 1. Is exit()ing the best thing to be done? I understand it's the easiest
>    and maybe better than nothing, but wouldn't it be better to enter in
>    paused-forever state so that clients can query and decide what to do?

For incoming migration, if it fails in the middle, every bet is off.
You are in a really inconsistent state, not sure which one, and if
migration was live, with the other host possibly retaking the disks to
continue.

In some cases, you can't do anything:
- you got passed an fd, and fd got closed/image corrupted/...
- you got passed an exec command like "exec: gzip -d < foo.gz"
  If gzip failed once, it will fail forever.

If you are running it by hand, cursor up + enter, and you are back
If you are using a management application, it is going to be easier to
restart the process that trying to cleanup everything.

Experience shows that people really tries to do weird things when
machine is in this state.

Later, Juan.

Re: [Qemu-devel] Re: [PATCH 1/5] Exit if incoming migration fails

[Anthony Liguori]

On 05/25/2010 01:37 PM, Juan Quintela wrote:
> Luiz Capitulino<lcapitulino@redhat.com>  wrote:
>    
>> On Tue, 25 May 2010 16:21:01 +0200
>> Juan Quintela<quintela@redhat.com>  wrote:
>>
>>      
>>> Signed-off-by: Juan Quintela<quintela@redhat.com>
>>> ---
>>>   migration.c |   16 ++++++++++------
>>>   migration.h |    2 +-
>>>   vl.c        |    7 ++++++-
>>>   3 files changed, 17 insertions(+), 8 deletions(-)
>>>
>>>        
>    
>>   While I agree on the change, I have two comments:
>>
>> 1. By taking a look at the code I have the impression that most of the
>>     fun failures will happen on the handler passed to qemu_set_fd_handler2(),
>>     do you agree? Any plan to address that?
>>      
> That is outgoing migration, not incoming migration.
> Incoming migration in synchronous..
>
>
>    
>> 1. Is exit()ing the best thing to be done? I understand it's the easiest
>>     and maybe better than nothing, but wouldn't it be better to enter in
>>     paused-forever state so that clients can query and decide what to do?
>>      
> For incoming migration, if it fails in the middle, every bet is off.
> You are in a really inconsistent state, not sure which one, and if
> migration was live, with the other host possibly retaking the disks to
> continue.
>    

I agree that exiting is the only sane behavior for the destination.

Regards,

Anthony Liguori

> In some cases, you can't do anything:
> - you got passed an fd, and fd got closed/image corrupted/...
> - you got passed an exec command like "exec: gzip -d<  foo.gz"
>    If gzip failed once, it will fail forever.
>
> If you are running it by hand, cursor up + enter, and you are back
> If you are using a management application, it is going to be easier to
> restart the process that trying to cleanup everything.
>
> Experience shows that people really tries to do weird things when
> machine is in this state.
>
> Later, Juan.
>
>