Skip to contentSkip to conversation

Add QMP migration events

PATCH Superseded Juan Quintela 4 participants 36 messages May 25, 2010 – May 27, 2010

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

v2 cover Add QMP migration events

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(-)
#1 lore raw

PATCH v2 1/5 Exit if incoming migration fails

---
 migration.c |   16 ++++++++++------
 migration.h |    2 +-
 vl.c        |    7 ++++++-
 3 files changed, 17 insertions(+), 8 deletions(-)
migration.c +10 -6
index 05f6cc5..9c1d4b6 100644
@@ -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)
migration.h +1 -1
index 385423f..dd423a1 100644
@@ -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);
 
vl.c +6 -1
index 328395e..d13440d 100644
@@ -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();
     }
Signature · 1 line
1.6.6.1
#2 lore raw

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

Quoted 62 lines · On Tue, 25 May 2010 16:21:01 +0200 Juan Quintela <[email protected]> wrote:
---
 migration.c |   16 ++++++++++------
 migration.h |    2 +-
 vl.c        |    7 ++++++-
 3 files changed, 17 insertions(+), 8 deletions(-)
migration.c +10 -6
index 05f6cc5..9c1d4b6 100644
@@ -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)
migration.h +1 -1
index 385423f..dd423a1 100644
@@ -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);
 
vl.c +0 -0
index 328395e..d13440d 100644

vl.c int main(int argc, char **argv, char **envp) · line 3830

Quoted 9 lines
vl.c +5 -1
@@ -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?

Quoted 4 lines
vl.c +1 -0
+        }
     } else if (autostart) {
         vm_start();
     }
#3 lore raw

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

Quoted 11 lines · Luiz Capitulino
Quoted 7 lines · On Tue, 25 May 2010 16:21:01 +0200 Juan Quintela <[email protected]> wrote:
---
 migration.c |   16 ++++++++++------
 migration.h |    2 +-
 vl.c        |    7 ++++++-
 3 files changed, 17 insertions(+), 8 deletions(-)
Quoted 5 lines

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..

Quoted 3 lines

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.

#4 lore raw

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

Quoted 26 lines · Juan Quintela
Quoted 13 lines · Luiz Capitulino
Quoted 8 lines · On Tue, 25 May 2010 16:21:01 +0200 Juan Quintela<[email protected]> wrote:
---
  migration.c |   16 ++++++++++------
  migration.h |    2 +-
  vl.c        |    7 ++++++-
  3 files changed, 17 insertions(+), 8 deletions(-)
Quoted 6 lines

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..

Quoted 9 lines
Quoted 4 lines

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

Quoted 15 lines

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.

#5 lore raw

PATCH v2 2/5 Factorize common migration incoming code

In reply to Juan Quintela
---
 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(-)
migration-exec.c +1 -13
index 5435827..07af11a 100644
@@ -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);
 }
migration-fd.c +1 -13
index 0abd372..6d14505 100644
@@ -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);
 }
migration-tcp.c +2 -13
index 95ce722..20f2e37 100644
@@ -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);
migration-unix.c +2 -13
index 49de1b9..57232c0 100644
@@ -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);
migration.c +13 -0
index 9c1d4b6..c388902 100644
@@ -58,6 +58,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;
migration.h +2 -0
index dd423a1..017e9c3 100644
@@ -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);
Signature · 1 line
1.6.6.1
#6 lore raw

PATCH v2 3/5 QMP: Introduce MIGRATION events

In reply to Juan Quintela

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

---
 QMP/qmp-events.txt |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 monitor.c          |   12 ++++++++++++
 monitor.h          |    4 ++++
 3 files changed, 66 insertions(+), 0 deletions(-)
QMP/qmp-events.txt +50 -0
index 01ec85f..93caa4d 100644
@@ -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_FAILED (no way to differentiate a FAILED
+and CANCELED migration for target).
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_CANCELED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_ENDED
+---------------
+
+Emitted when migration ends (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
 -----
 
monitor.c +12 -0
index ad50f12..5158780 100644
@@ -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;
monitor.h +4 -0
index ea15469..34bcd38 100644
@@ -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;
 
Signature · 1 line
1.6.6.1
#7 lore raw

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

Quoted 16 lines · Juan Quintela

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

---
  QMP/qmp-events.txt |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
  monitor.c          |   12 ++++++++++++
  monitor.h          |    4 ++++
  3 files changed, 66 insertions(+), 0 deletions(-)
QMP/qmp-events.txt +0 -0
index 01ec85f..93caa4d 100644
@@ -26,6 +26,56 @@ Example:
  Note: If action is "stop", a STOP event will eventually follow the
  BLOCK_IO_ERROR event.

QMP/qmp-events.txt Example: · line 37

Quoted 10 lines
QMP/qmp-events.txt +6 -0
 
 
 
+MIGRATION_CANCELED
+------------------
+
+Emitted when migration is canceled.  This is emitted in the source.
+Target will emit MIGRATION_FAILED (no way to differentiate a FAILED
+and CANCELED migration for target).
 

But the management tool is the one that cancels so surely, it knows why already.

QMP/qmp-events.txt Example: · line 46

Quoted 12 lines
QMP/qmp-events.txt +11 -0
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_CANCELED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_ENDED
+---------------
+
+Emitted when migration ends (both in source and target)

A start event is going to be generated already, no?

Quoted 4 lines
QMP/qmp-events.txt +3 -0
+Data: None
+
+Example:

QMP/qmp-events.txt Example: · line 60

Quoted 10 lines
QMP/qmp-events.txt +10 -0
+
+{ "event": "MIGRATION_ENDED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_FAILED
+----------------
+
+Emitted when migration fails (both is source and target).
+
+Data: None

There should be some information about why it failed, no? Preferrably in a QError format.

QMP/qmp-events.txt Example: · line 70

Quoted 10 lines
QMP/qmp-events.txt +9 -0
+Example:
+
+{ "event": "MIGRATION_FAILED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_STARTED
+-----------------
+
+Emitted when migration starts (both in source and target).

I think this makes more sense as a MIGRATION_CONNECTED event. It probably should carry peer information too.

Regards,

Anthony Liguori

Quoted 49 lines
QMP/qmp-events.txt +7 -0
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_STARTED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
  RESET
  -----
monitor.c +12 -0
index ad50f12..5158780 100644
@@ -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;
monitor.h +4 -0
index ea15469..34bcd38 100644
@@ -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;
 
#8 lore raw

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

Quoted 12 lines · Anthony Liguori

QMP/qmp-events.txt Example: · line 34

Quoted 7 lines · Juan Quintela
QMP/qmp-events.txt +6 -0
+MIGRATION_CANCELED
+------------------
+
+Emitted when migration is canceled.  This is emitted in the source.
+Target will emit MIGRATION_FAILED (no way to differentiate a FAILED
+and CANCELED migration for target).

But the management tool is the one that cancels so surely, it knows why already.

ok, then that one is ok.

Quoted 15 lines

QMP/qmp-events.txt Example: · line 46

Quoted 13 lines
QMP/qmp-events.txt +11 -0
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_CANCELED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_ENDED
+---------------
+
+Emitted when migration ends (both in source and target)

A start event is going to be generated already, no?

problem here is that libvirt start target with -S, and waits to do the "cont" as soon as possible. As of know, only way to do it is to poll info migrate on source faster.

Quoted 9 lines
Quoted 9 lines
QMP/qmp-events.txt +9 -0
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_ENDED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_FAILED
+----------------
Quoted 8 lines

QMP/qmp-events.txt Example: · line 60

Quoted 5 lines
QMP/qmp-events.txt +4 -0
+
+Emitted when migration fails (both is source and target).
+
+Data: None

There should be some information about why it failed, no? Preferrably in a QError format.

At this point, we have basically -1 :(

I can add a field with an error number, but we are very bad at the moment about moving errno's upstack.

Quoted 5 lines
Quoted 5 lines
QMP/qmp-events.txt +5 -0
+Example:
+
+{ "event": "MIGRATION_FAILED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
Quoted 8 lines

QMP/qmp-events.txt Example: · line 70

Quoted 5 lines
QMP/qmp-events.txt +4 -0
+MIGRATION_STARTED
+-----------------
+
+Emitted when migration starts (both in source and target).

I think this makes more sense as a MIGRATION_CONNECTED event. It probably should carry peer information too.

What kind of peer information?

We have tcp/fd/exec/unix migrations. calling it CONNECTED vs STARTED, I don't care. But adding information? Notice that the management application knows what it did, I can put the:

"exec: gzip -d < /tmp/foo"

string, but not much more that I can put here.

Later, Juan.

#9 lore raw

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

Quoted 18 lines · Juan Quintela
Quoted 5 lines · Anthony Liguori
Quoted 5 lines
QMP/qmp-events.txt +5 -0
+Example:
+
+{ "event": "MIGRATION_FAILED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
Quoted 8 lines

QMP/qmp-events.txt Example: · line 70

Quoted 5 lines
QMP/qmp-events.txt +4 -0
+MIGRATION_STARTED
+-----------------
+
+Emitted when migration starts (both in source and target).

I think this makes more sense as a MIGRATION_CONNECTED event. It probably should carry peer information too.

What kind of peer information?

Quoted 7 lines

We have tcp/fd/exec/unix migrations. calling it CONNECTED vs STARTED, I don't care. But adding information? Notice that the management application knows what it did, I can put the:

"exec: gzip -d < /tmp/foo"

string, but not much more that I can put here.

This is why I think network client CONNECT/DISCONNECT events should be separate from MIGRATION START/END events. They happen to occur at roughly the same time if using a TCP / UNIX socket based migration transport, but CONNECT/DISCONNECT + peer info is meaningless for exec or fd based migration.

Daniel

Signature · 4 lines
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
#10 lore raw

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

In reply to Juan Quintela
Quoted 35 lines · Juan Quintela
Quoted 14 lines · Anthony Liguori

QMP/qmp-events.txt Example: · line 34

Quoted 8 lines · Juan Quintela
QMP/qmp-events.txt +6 -0
+MIGRATION_CANCELED
+------------------
+
+Emitted when migration is canceled.  This is emitted in the source.
+Target will emit MIGRATION_FAILED (no way to differentiate a FAILED
+and CANCELED migration for target).

But the management tool is the one that cancels so surely, it knows why already.

ok, then that one is ok.

Quoted 16 lines
Quoted 4 lines
QMP/qmp-events.txt +1 -0
+Data: None

QMP/qmp-events.txt Example: · line 46

Quoted 10 lines
QMP/qmp-events.txt +10 -0
+
+Example:
+
+{ "event": "MIGRATION_CANCELED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_ENDED
+---------------
+
+Emitted when migration ends (both in source and target)

A start event is going to be generated already, no?

Quoted 4 lines

problem here is that libvirt start target with -S, and waits to do the "cont" as soon as possible. As of know, only way to do it is to poll info migrate on source faster.

Why does it do that??

That sound like a terrible idea.

Quoted 18 lines
Quoted 9 lines
Quoted 9 lines
QMP/qmp-events.txt +9 -0
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_ENDED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_FAILED
+----------------
Quoted 9 lines

QMP/qmp-events.txt Example: · line 60

Quoted 6 lines
QMP/qmp-events.txt +4 -0
+
+Emitted when migration fails (both is source and target).
+
+Data: None

There should be some information about why it failed, no? Preferrably in a QError format.

Quoted 5 lines

At this point, we have basically -1 :(

I can add a field with an error number, but we are very bad at the moment about moving errno's upstack.

We need a better solution for reporting errors via notifications.

Quoted 5 lines
Quoted 3 lines

I think this makes more sense as a MIGRATION_CONNECTED event. It probably should carry peer information too.

What kind of peer information?

Quoted 8 lines

We have tcp/fd/exec/unix migrations. calling it CONNECTED vs STARTED, I don't care. But adding information? Notice that the management application knows what it did, I can put the:

"exec: gzip -d< /tmp/foo"

string, but not much more that I can put here.

Basically, do we have any useful information in info migrate that we can include?

Regards,

Anthony Liguori

Quoted 2 lines

Later, Juan.

#11 lore raw

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

Quoted 20 lines · Anthony Liguori
Quoted 19 lines · Juan Quintela
Quoted 16 lines · Anthony Liguori
Quoted 4 lines
QMP/qmp-events.txt +1 -0
+Data: None

QMP/qmp-events.txt Example: · line 46

Quoted 10 lines
QMP/qmp-events.txt +10 -0
+
+Example:
+
+{ "event": "MIGRATION_CANCELED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_ENDED
+---------------
+
+Emitted when migration ends (both in source and target)

A start event is going to be generated already, no?

Quoted 8 lines
Quoted 4 lines

problem here is that libvirt start target with -S, and waits to do the "cont" as soon as possible. As of know, only way to do it is to poll info migrate on source faster.

Why does it do that??

That sound like a terrible idea.

Historically QEMU gave no alternative. Adding these STARTED/ENDED events is to allow libvirt to detect start + end of migration reliably, avoiding the previous hacks QEMU forced us todo on the dest, and avoid the high rate polling we had no choice but todo on the source.

Quoted 9 lines
Quoted 9 lines
Quoted 3 lines

I think this makes more sense as a MIGRATION_CONNECTED event. It probably should carry peer information too.

What kind of peer information?

We have tcp/fd/exec/unix migrations. calling it CONNECTED vs STARTED, I don't care. But adding information? Notice that the management application knows what it did, I can put the:

Quoted 6 lines
Quoted 3 lines

"exec: gzip -d< /tmp/foo"

string, but not much more that I can put here.

Basically, do we have any useful information in info migrate that we can include?

info migrate just includes the progress info + state (running, finished, cancelled, failed). The event itself replicates state. I don't see a hugely compelling need to include the progress info in the FINISHED/CANCELLED events. If really needed, the app can still call 'info migrate' to get it.

Regards,
Daniel

Signature · 4 lines
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
#12 lore raw

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

In reply to Anthony Liguori
Quoted 10 lines · Anthony Liguori
Quoted 4 lines · Juan Quintela

problem here is that libvirt start target with -S, and waits to do the "cont" as soon as possible. As of know, only way to do it is to poll info migrate on source faster.

Why does it do that??

That sound like a terrible idea.

Becaues migration is not reliable, and they don't have a way to issue cont only in one of the sides :(

We make migration protocol reliable, or management application have to decide when migration suceeded or not.

This new events help then a lot. But they issue the cont really fast (before migration ends). I don't remember why they did that.

danp?

Quoted 10 lines
Quoted 8 lines
Quoted 3 lines

There should be some information about why it failed, no? Preferrably in a QError format.

At this point, we have basically -1 :(

I can add a field with an error number, but we are very bad at the moment about moving errno's upstack.

We need a better solution for reporting errors via notifications.

Suggestions?

Notice that what we need now is a way to know if migration ended with success or in any other way, as soon as possible.

Quoted 9 lines
Quoted 9 lines
Quoted 3 lines

I think this makes more sense as a MIGRATION_CONNECTED event. It probably should carry peer information too.

What kind of peer information?

We have tcp/fd/exec/unix migrations. calling it CONNECTED vs STARTED, I don't care. But adding information? Notice that the management application knows what it did, I can put the:

Quoted 7 lines
Quoted 4 lines

"exec: gzip -d< /tmp/foo"

string, but not much more that I can put here.

Basically, do we have any useful information in info migrate that we can include?

(qemu) info migrate
Migration status: active
transferred ram: 874808 kbytes
remaining ram: 227912 kbytes
total ram: 1065344 kbytes
(qemu)

I can't see anything interesting to put here :(

About the CONNECTED/STARTED distintion, I fully agree with danp. We just want STARTED event for migration, CONNECTION should be generated (or not) for all sockets/char devices. it don't make sense for fd/exec for instance.

Later, Juan.

#13 lore raw

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

Quoted 10 lines · Juan Quintela
Quoted 8 lines · Anthony Liguori
Quoted 5 lines · Juan Quintela

problem here is that libvirt start target with -S, and waits to do the "cont" as soon as possible. As of know, only way to do it is to poll info migrate on source faster.

Quoted 7 lines
Quoted 4 lines

Why does it do that??

That sound like a terrible idea.

Becaues migration is not reliable, and they don't have a way to issue cont only in one of the sides :(

I don't know what you mean by reliable.

When the migration completes on the destination, it will start automatically.

The source will not start unless explicitly invoked. If you successfully cancel a migration on the source, it's guaranteed that it won't start on the destination. So the sequence looks like:

src) // decide we want to give up migration
src) migrate_cancel
src) // check migration status
src) cont // if migration cancelled
src) //if migration succeeded, check destination for completion
dst) // if not responsive and not completed in appropriate amount of time, kill guest src) cont // if killed destination

I don't see what the problem is.

Quoted 3 lines

We make migration protocol reliable, or management application have to decide when migration suceeded or not.

Reliability has nothing to do with the protocol and everything to do with the presence of the third node.

Quoted 3 lines

This new events help then a lot. But they issue the cont really fast (before migration ends). I don't remember why they did that.

If libvirt is launching the destination with -S, it's doing the wrong thing and we ought make sure the proper fix gets implemented.

Quoted 13 lines

danp?

Quoted 10 lines
Quoted 10 lines
Quoted 4 lines

There should be some information about why it failed, no? Preferrably in a QError format.

At this point, we have basically -1 :(

I can add a field with an error number, but we are very bad at the moment about moving errno's upstack.

Quoted 7 lines
Quoted 2 lines

We need a better solution for reporting errors via notifications.

Suggestions?

Notice that what we need now is a way to know if migration ended with success or in any other way, as soon as possible.

Markus/Luiz?

Quoted 18 lines
Quoted 10 lines
Quoted 10 lines
Quoted 4 lines

I think this makes more sense as a MIGRATION_CONNECTED event. It probably should carry peer information too.

What kind of peer information?

We have tcp/fd/exec/unix migrations. calling it CONNECTED vs STARTED, I don't care. But adding information? Notice that the management application knows what it did, I can put the:

Quoted 8 lines
Quoted 5 lines
   "exec: gzip -d<   /tmp/foo"

string, but not much more that I can put here.

Basically, do we have any useful information in info migrate that we can include?

Quoted 9 lines

(qemu) info migrate
Migration status: active
transferred ram: 874808 kbytes
remaining ram: 227912 kbytes
total ram: 1065344 kbytes
(qemu)

I can't see anything interesting to put here :(

Ugh.

Quoted 5 lines

About the CONNECTED/STARTED distintion, I fully agree with danp. We just want STARTED event for migration, CONNECTION should be generated (or not) for all sockets/char devices. it don't make sense for fd/exec for instance.

That makes sense to me.

Regards,

Anthony Liguori

Quoted 2 lines

Later, Juan.

#14 lore raw

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

Quoted 12 lines · On Tue, 25 May 2010 11:10:23 -0500 Anthony Liguori <[email protected]>…
Quoted 12 lines
Quoted 12 lines
Quoted 10 lines
Quoted 4 lines

There should be some information about why it failed, no? Preferrably in a QError format.

At this point, we have basically -1 :(

I can add a field with an error number, but we are very bad at the moment about moving errno's upstack.

We need a better solution for reporting errors via notifications.

Quoted 7 lines
Quoted 5 lines

Suggestions?

Notice that what we need now is a way to know if migration ended with success or in any other way, as soon as possible.

Markus/Luiz?

We need to redesign QError. I could give it a try, but quite frankly, I
don't know how do it good enough..

Markus has worked more on error handling than me though and I think he's
the best person to do it, but he's busy at other things atm.

Note that major work is design, not code churn (could turn out into an issue,
but I doubt it).

#15 lore raw

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

In reply to Anthony Liguori
Quoted 10 lines · Anthony Liguori
Quoted 4 lines · Juan Quintela

problem here is that libvirt start target with -S, and waits to do the "cont" as soon as possible. As of know, only way to do it is to poll info migrate on source faster.

Why does it do that??

That sound like a terrible idea.

Becaues migration is not reliable, and they don't have a way to issue cont only in one of the sides :(

We make migration protocol reliable, or management application have to decide when migration suceeded or not.

This new events help then a lot. But they issue the cont really fast (before migration ends). I don't remember why they did that.

danp?

Quoted 10 lines
Quoted 8 lines
Quoted 3 lines

There should be some information about why it failed, no? Preferrably in a QError format.

At this point, we have basically -1 :(

I can add a field with an error number, but we are very bad at the moment about moving errno's upstack.

We need a better solution for reporting errors via notifications.

Suggestions?

Notice that what we need now is a way to know if migration ended with success or in any other way, as soon as possible.

Quoted 9 lines
Quoted 9 lines
Quoted 3 lines

I think this makes more sense as a MIGRATION_CONNECTED event. It probably should carry peer information too.

What kind of peer information?

We have tcp/fd/exec/unix migrations. calling it CONNECTED vs STARTED, I don't care. But adding information? Notice that the management application knows what it did, I can put the:

Quoted 7 lines
Quoted 4 lines

"exec: gzip -d< /tmp/foo"

string, but not much more that I can put here.

Basically, do we have any useful information in info migrate that we can include?

(qemu) info migrate
Migration status: active
transferred ram: 874808 kbytes
remaining ram: 227912 kbytes
total ram: 1065344 kbytes
(qemu)

I can't see anything interesting to put here :(

About the CONNECTED/STARTED distintion, I fully agree with danp. We just want STARTED event for migration, CONNECTION should be generated (or not) for all sockets/char devices. it don't make sense for fd/exec for instance.

Later, Juan.

#16 lore raw

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

Quoted 12 lines · Juan Quintela
Quoted 10 lines · Anthony Liguori
Quoted 4 lines · Juan Quintela

problem here is that libvirt start target with -S, and waits to do the "cont" as soon as possible. As of know, only way to do it is to poll info migrate on source faster.

Why does it do that??

That sound like a terrible idea.

Quoted 8 lines

Becaues migration is not reliable, and they don't have a way to issue cont only in one of the sides :(

We make migration protocol reliable, or management application have to decide when migration suceeded or not.

This new events help then a lot. But they issue the cont really fast (before migration ends). I don't remember why they did that.

The use of '-S / cont' isn't really because of reliability. There are several scenarios though. There's a migrate API option to leave the guest paused upon completion, hence we need to start it with -S to stop it auto-running upon completion. With some disk locking approaches we need todo a lock transfer before allowing the dest to continue running. It could be optimized to avoid the -S /cont in cases where those two scenarios aren't relevant, but only if we can get a separate async notification of when migration starts and completes on the destination, so we can notify mgmt apps that need this lifecycle event.

So in summary these lifecycle events on source + dest for start, complete, fail, cancel are all focused on allowing libvirt to remove its existing hacks in migration support for current QEMU.

Regards,
Daniel

Signature · 4 lines
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
#17 lore raw

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

Quoted 22 lines · Daniel P. Berrange
Quoted 20 lines · Juan Quintela
Quoted 12 lines · Anthony Liguori
Quoted 5 lines · Juan Quintela

problem here is that libvirt start target with -S, and waits to do the "cont" as soon as possible. As of know, only way to do it is to poll info migrate on source faster.

Why does it do that??

That sound like a terrible idea.

Becaues migration is not reliable, and they don't have a way to issue cont only in one of the sides :(

We make migration protocol reliable, or management application have to decide when migration suceeded or not.

Quoted 7 lines
Quoted 3 lines

This new events help then a lot. But they issue the cont really fast (before migration ends). I don't remember why they did that.

The use of '-S / cont' isn't really because of reliability. There are several scenarios though. There's a migrate API option to leave the guest paused upon completion, hence we need to start it with -S to stop it auto-running upon completion.

That's a strange API. Why would you want to do that? Why not just stop and then migrate? You're just wasting bandwidth doing a live migration and then leaving it stopped. This is a critical period of time for the guest and generally speaking, you don't want to involve many layers of management tooling in these decisions because the result is going to be that you break the migration downtime contract.

Quoted 3 lines

With some disk locking
approaches we need todo a lock transfer before allowing the dest to continue running.

QEMU is going to read the disk before the migration completes so the lock transfer is not going to work with the current implementation (it needs to read the disk to do probing). I assume this is not something that's actually been implemented.

Quoted 6 lines

It could be optimized to avoid the -S /cont
in cases where those two scenarios aren't relevant, but only if we can get a separate async notification of when migration starts and completes on the destination, so we can notify mgmt apps that need this lifecycle event.

Migration completes == guest starts running. You'll get a notification of that but you're not getting that now because you're doing -S which I'd argue is a functional problem on the part of libvirt (you're breaking the downtime contract).

I'm not sure why you would need a notification of when migration starts (since you know when you've started migration).

Regards,

Anthony Liguori

Quoted 7 lines

So in summary these lifecycle events on source + dest for start, complete, fail, cancel are all focused on allowing libvirt to remove its existing hacks in migration support for current QEMU.

Regards,
Daniel

#18 lore raw

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

Quoted 7 lines · Anthony Liguori
Quoted 3 lines · Daniel P. Berrange
Quoted 1 line · Juan Quintela

Anthony Liguori<[email protected]> wrote:

I'm not sure why you would need a notification of when migration starts (since you know when you've started migration).

But you don't know if the other end "knows" that it has also started.

started is needed only in incoming part, because .... we don't have a monitor to ask if migration has started.

Later, Juan.

#19 lore raw

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

Quoted 6 lines · Juan Quintela
Quoted 4 lines · Anthony Liguori
Quoted 3 lines · Daniel P. Berrange
Quoted 1 line · Juan Quintela

Anthony Liguori<[email protected]> wrote:

Quoted 7 lines
Quoted 2 lines

I'm not sure why you would need a notification of when migration starts (since you know when you've started migration).

But you don't know if the other end "knows" that it has also started.

started is needed only in incoming part, because .... we don't have a monitor to ask if migration has started.

If we ever want to get closer to allowing multiple monitors, or allowing apps to issue QMP commands directly via libvirt, then we still need the 'migration started' event on the source, because something else can have issued the 'migrate' command without the mgmt app knowing.

MIGRATED_STARTED+STOPPED really *is* needed if we're to make QMP cope with all possible use cases. If we rely on inferring it from STOP+RESUME events, it is going to exclude a significant set of use cases, and likely result in this being proposed all over again in 12 months time :-(

Regards,
Daniel

Signature · 4 lines
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
#20 lore raw

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

Quoted 5 lines · Daniel P. Berrange
Quoted 5 lines
Quoted 3 lines

I'm not sure why you would need a notification of when migration starts (since you know when you've started migration).

But you don't know if the other end "knows" that it has also started.

Quoted 8 lines
Quoted 3 lines

started is needed only in incoming part, because .... we don't have a monitor to ask if migration has started.

If we ever want to get closer to allowing multiple monitors, or allowing apps to issue QMP commands directly via libvirt, then we still need the 'migration started' event on the source, because something else can have issued the 'migrate' command without the mgmt app knowing.

Migration started doesn't help multiple monitors. You need locking of some sort.

Part of the problem is the QMP migrate command is implemented as a synchronous command. It really ought to be an asynchronous command. That tells you when the migration has actually completed without polling.

On the source end, I can't think of any events that would be useful. The migrate command can complete with a failure so that gives you failure notifications.

On the destination side, we're really limited by the fact that we don't do live incoming migrations. The monitor doesn't get a chance to run at all with exec: migration, for instance.

For tcp: and unix:, a CONNECTED event absolutely makes sense (every socket server should emit a CONNECTED event). Unfortunately, after CONNECTED you lose the monitor until migration is complete. If something bad happens, you have to exit qemu so once the monitor returns, migration has completed successfully.

If we introduce live incoming migration, we'll need to rethink things. I would actually suggest that we deprecate the incoming command if we do that and make incoming migration a monitor command. I would think it should have the same semantics as migrate (as an asynchronous command). A CONNECTED event still makes sense for tcp and unix protocols but I don't think events make sense for start stop vs. an asynchronous command completion.

Regards,

Anthony Liguori

Quoted 8 lines

MIGRATED_STARTED+STOPPED really *is* needed if we're to make QMP cope with all possible use cases. If we rely on inferring it from STOP+RESUME events, it is going to exclude a significant set of use cases, and likely result in this being proposed all over again in 12 months time :-(

Regards,
Daniel

#21 lore raw

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

Quoted 15 lines · Anthony Liguori
Quoted 5 lines · Daniel P. Berrange
Quoted 5 lines
Quoted 3 lines

I'm not sure why you would need a notification of when migration starts (since you know when you've started migration).

But you don't know if the other end "knows" that it has also started.

Quoted 8 lines
Quoted 3 lines

started is needed only in incoming part, because .... we don't have a monitor to ask if migration has started.

If we ever want to get closer to allowing multiple monitors, or allowing apps to issue QMP commands directly via libvirt, then we still need the 'migration started' event on the source, because something else can have issued the 'migrate' command without the mgmt app knowing.

Quoted 6 lines

Migration started doesn't help multiple monitors. You need locking of some sort.

Part of the problem is the QMP migrate command is implemented as a synchronous command. It really ought to be an asynchronous command. That tells you when the migration has actually completed without polling.

Handling asynchronous commands is alot more complicated and error prone for client apps, than providing a asynchronous event notification of the lifecycle stages. If you want to also query status while waiting for the completion, it means you can have to deal with overlapping command execute+return pairs within a single monitor connection. AFAICT this requires a change to QMP to require a unique ID to be sent with the {'execute'..} command and be sent back with the later corresponding {'return'...} data, so you can actually correlate reliably.

Quoted 3 lines

On the destination side, we're really limited by the fact that we don't do live incoming migrations. The monitor doesn't get a chance to run at all with exec: migration, for instance.

If QEMU let you issue a monitor command for starting incoming migration, instead of using -incoming this wouldn't such a bad problem. eg you can launch QEMU in the desired config, with CPUs stopped, do the normal QMP handshake + whatever else is required then issue 'migrate_incoming URI' which blocked the caller for the duration, to allow completion to be detected.

Quoted 6 lines

For tcp: and unix:, a CONNECTED event absolutely makes sense (every socket server should emit a CONNECTED event). Unfortunately, after CONNECTED you lose the monitor until migration is complete. If something bad happens, you have to exit qemu so once the monitor returns, migration has completed successfully.

Quoted 7 lines

If we introduce live incoming migration, we'll need to rethink things. I would actually suggest that we deprecate the incoming command if we do that and make incoming migration a monitor command. I would think it should have the same semantics as migrate (as an asynchronous command). A CONNECTED event still makes sense for tcp and unix protocols but I don't think events make sense for start stop vs. an asynchronous command completion.

Do you actually mean 'deprecate -incoming arg' here ?

Daniel

Signature · 4 lines
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
#22 lore raw

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

Quoted 27 lines · Daniel P. Berrange
Quoted 18 lines · Anthony Liguori
Quoted 6 lines · Daniel P. Berrange
Quoted 6 lines
Quoted 4 lines

I'm not sure why you would need a notification of when migration starts (since you know when you've started migration).

But you don't know if the other end "knows" that it has also started.

Quoted 10 lines
Quoted 4 lines

started is needed only in incoming part, because .... we don't have a monitor to ask if migration has started.

If we ever want to get closer to allowing multiple monitors, or allowing apps to issue QMP commands directly via libvirt, then we still need the 'migration started' event on the source, because something else can have issued the 'migrate' command without the mgmt app knowing.

Quoted 7 lines

Migration started doesn't help multiple monitors. You need locking of some sort.

Part of the problem is the QMP migrate command is implemented as a synchronous command. It really ought to be an asynchronous command. That tells you when the migration has actually completed without polling.

Quoted 10 lines

Handling asynchronous commands is alot more complicated and error prone for client apps, than providing a asynchronous event notification of the lifecycle stages. If you want to also query status while waiting for the completion, it means you can have to deal with overlapping command execute+return pairs within a single monitor connection. AFAICT this requires a change to QMP to require a unique ID to be sent with the {'execute'..} command and be sent back with the later corresponding {'return'...} data, so you can actually correlate reliably.

That's exactly how the protocol is designed. That was one of the major improvements of QMP over the human monior.

This is how the info balloon command works, BTW.

Since there's a clear correlation between the request and the result of the request, an asynchronous command is what makes the most sense. It eliminates the problem of how to pass QErrors via an event which is one of the problems with the current event proposal.

Events are meant for when there is no clear request that the event is associated with. A VNC CONNECT event is a good example of this.

Quoted 27 lines
Quoted 5 lines

On the destination side, we're really limited by the fact that we don't do live incoming migrations. The monitor doesn't get a chance to run at all with exec: migration, for instance.

If QEMU let you issue a monitor command for starting incoming migration, instead of using -incoming this wouldn't such a bad problem. eg you can launch QEMU in the desired config, with CPUs stopped, do the normal QMP handshake + whatever else is required then issue 'migrate_incoming URI' which blocked the caller for the duration, to allow completion to be detected.

Quoted 6 lines

For tcp: and unix:, a CONNECTED event absolutely makes sense (every socket server should emit a CONNECTED event). Unfortunately, after CONNECTED you lose the monitor until migration is complete. If something bad happens, you have to exit qemu so once the monitor returns, migration has completed successfully.

Quoted 8 lines

If we introduce live incoming migration, we'll need to rethink things. I would actually suggest that we deprecate the incoming command if we do that and make incoming migration a monitor command. I would think it should have the same semantics as migrate (as an asynchronous command). A CONNECTED event still makes sense for tcp and unix protocols but I don't think events make sense for start stop vs. an asynchronous command completion.

Quoted 2 lines

Do you actually mean 'deprecate -incoming arg' here ?

Yes. And by deprecate, I really mean that -incoming just becomes syntactic sugar for executing a monitor command immediately.

Regards,

Anthony Liguori

Quoted 2 lines

Daniel

#23 lore raw

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

Quoted 39 lines · On Wed, 26 May 2010 11:55:31 -0500 Anthony Liguori <[email protected]>…
Quoted 27 lines · Daniel P. Berrange
Quoted 18 lines · Anthony Liguori
Quoted 6 lines · Daniel P. Berrange
Quoted 6 lines
Quoted 4 lines

I'm not sure why you would need a notification of when migration starts (since you know when you've started migration).

But you don't know if the other end "knows" that it has also started.

Quoted 10 lines
Quoted 4 lines

started is needed only in incoming part, because .... we don't have a monitor to ask if migration has started.

If we ever want to get closer to allowing multiple monitors, or allowing apps to issue QMP commands directly via libvirt, then we still need the 'migration started' event on the source, because something else can have issued the 'migrate' command without the mgmt app knowing.

Quoted 7 lines

Migration started doesn't help multiple monitors. You need locking of some sort.

Part of the problem is the QMP migrate command is implemented as a synchronous command. It really ought to be an asynchronous command. That tells you when the migration has actually completed without polling.

Quoted 10 lines

Handling asynchronous commands is alot more complicated and error prone for client apps, than providing a asynchronous event notification of the lifecycle stages. If you want to also query status while waiting for the completion, it means you can have to deal with overlapping command execute+return pairs within a single monitor connection. AFAICT this requires a change to QMP to require a unique ID to be sent with the {'execute'..} command and be sent back with the later corresponding {'return'...} data, so you can actually correlate reliably.

Quoted 2 lines

That's exactly how the protocol is designed. That was one of the major improvements of QMP over the human monior.

Yes and it already has 'id' support:

{ "execute": "cont", "id": "luiz" }
{"timestamp": {"seconds": 1274966635, "microseconds": 776813}, "event": "RESUME"} {"return": {}, "id": "luiz"}

But it doesn't detect duplicates, this is something I think it's up
to the client to do, do you agree?

Quoted 1 line

This is how the info balloon command works, BTW.

I won't remember the details now, but that interface has some issues and it
has to be reviewed.

Quoted 4 lines

Since there's a clear correlation between the request and the result of the request, an asynchronous command is what makes the most sense. It eliminates the problem of how to pass QErrors via an event which is one of the problems with the current event proposal.

Not exactly, this is a problem with QError not the event proposal. We'll
have the same issue if we decide to include errno in the migrate errors and the problem still exists with the BLOCK_IO_ERROR event.

That said, I do agree that migrate should be asynchronous. This yet another
thing we may want to fix before 0.13.

[...]

Quoted 14 lines
Quoted 14 lines
Quoted 14 lines

For tcp: and unix:, a CONNECTED event absolutely makes sense (every socket server should emit a CONNECTED event). Unfortunately, after CONNECTED you lose the monitor until migration is complete. If something bad happens, you have to exit qemu so once the monitor returns, migration has completed successfully.

If we introduce live incoming migration, we'll need to rethink things. I would actually suggest that we deprecate the incoming command if we do that and make incoming migration a monitor command. I would think it should have the same semantics as migrate (as an asynchronous command). A CONNECTED event still makes sense for tcp and unix protocols but I don't think events make sense for start stop vs. an asynchronous command completion.

Quoted 5 lines
Quoted 2 lines

Do you actually mean 'deprecate -incoming arg' here ?

Yes. And by deprecate, I really mean that -incoming just becomes syntactic sugar for executing a monitor command immediately.

But we can't change -incoming itself, since our command-line is supposed
to be stable, right?

Also, Juan has said that replacing that arg with a monitor command
doesn't work, as qemu would have to be started in paused monitor for this to work.

So, what about introducing a -incoming-monitor command, which puts qemu
in the right state for migration, but requires a migrate_incoming command to actually start migration?

#24 lore raw

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

Quoted 25 lines · Luiz Capitulino
Quoted 2 lines · On Wed, 26 May 2010 11:55:31 -0500 Anthony Liguori <[email protected]>…

That's exactly how the protocol is designed. That was one of the major improvements of QMP over the human monior.

Yes and it already has 'id' support:

{ "execute": "cont", "id": "luiz" }
{"timestamp": {"seconds": 1274966635, "microseconds": 776813}, "event": "RESUME"} {"return": {}, "id": "luiz"}

But it doesn't detect duplicates, this is something I think it's up
to the client to do, do you agree?

Quoted 1 line

This is how the info balloon command works, BTW.

I won't remember the details now, but that interface has some issues and it
has to be reviewed.

Quoted 4 lines

Since there's a clear correlation between the request and the result of the request, an asynchronous command is what makes the most sense. It eliminates the problem of how to pass QErrors via an event which is one of the problems with the current event proposal.

Quoted 6 lines

Not exactly, this is a problem with QError not the event proposal. We'll
have the same issue if we decide to include errno in the migrate errors and the problem still exists with the BLOCK_IO_ERROR event.

That said, I do agree that migrate should be asynchronous. This yet another
thing we may want to fix before 0.13.

How difficult is that?

Quoted 25 lines

[...]

Quoted 14 lines
Quoted 14 lines
Quoted 14 lines

For tcp: and unix:, a CONNECTED event absolutely makes sense (every socket server should emit a CONNECTED event). Unfortunately, after CONNECTED you lose the monitor until migration is complete. If something bad happens, you have to exit qemu so once the monitor returns, migration has completed successfully.

If we introduce live incoming migration, we'll need to rethink things. I would actually suggest that we deprecate the incoming command if we do that and make incoming migration a monitor command. I would think it should have the same semantics as migrate (as an asynchronous command). A CONNECTED event still makes sense for tcp and unix protocols but I don't think events make sense for start stop vs. an asynchronous command completion.

Quoted 5 lines
Quoted 2 lines

Do you actually mean 'deprecate -incoming arg' here ?

Yes. And by deprecate, I really mean that -incoming just becomes syntactic sugar for executing a monitor command immediately.

But we can't change -incoming itself, since our command-line is supposed
to be stable, right?

Quoted 7 lines

Also, Juan has said that replacing that arg with a monitor command
doesn't work, as qemu would have to be started in paused monitor for this to work.

So, what about introducing a -incoming-monitor command, which puts qemu
in the right state for migration, but requires a migrate_incoming command to actually start migration?

this -incoming-monitor is called -S, that should have a long name of -no-autostart

that is what it does, and what we need for incoming migration as monitor command. Nothing new to see here.

Later, Juan.

#25 lore raw

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

Quoted 26 lines · On Thu, 27 May 2010 17:58:03 +0200 Juan Quintela <[email protected]> wrote:
Quoted 25 lines · Luiz Capitulino
Quoted 2 lines · On Wed, 26 May 2010 11:55:31 -0500 Anthony Liguori <[email protected]>…

That's exactly how the protocol is designed. That was one of the major improvements of QMP over the human monior.

Yes and it already has 'id' support:

{ "execute": "cont", "id": "luiz" }
{"timestamp": {"seconds": 1274966635, "microseconds": 776813}, "event": "RESUME"} {"return": {}, "id": "luiz"}

But it doesn't detect duplicates, this is something I think it's up
to the client to do, do you agree?

Quoted 1 line

This is how the info balloon command works, BTW.

I won't remember the details now, but that interface has some issues and it
has to be reviewed.

Quoted 4 lines

Since there's a clear correlation between the request and the result of the request, an asynchronous command is what makes the most sense. It eliminates the problem of how to pass QErrors via an event which is one of the problems with the current event proposal.

Quoted 8 lines
Quoted 6 lines

Not exactly, this is a problem with QError not the event proposal. We'll
have the same issue if we decide to include errno in the migrate errors and the problem still exists with the BLOCK_IO_ERROR event.

That said, I do agree that migrate should be asynchronous. This yet another
thing we may want to fix before 0.13.

How difficult is that?

Anthony is working on this and should have patches soon.

[...]

Quoted 14 lines
Quoted 6 lines
Quoted 2 lines

Yes. And by deprecate, I really mean that -incoming just becomes syntactic sugar for executing a monitor command immediately.

But we can't change -incoming itself, since our command-line is supposed
to be stable, right?

Quoted 7 lines

Also, Juan has said that replacing that arg with a monitor command
doesn't work, as qemu would have to be started in paused monitor for this to work.

So, what about introducing a -incoming-monitor command, which puts qemu
in the right state for migration, but requires a migrate_incoming command to actually start migration?

Quoted 5 lines

this -incoming-monitor is called -S, that should have a long name of -no-autostart

that is what it does, and what we need for incoming migration as monitor command. Nothing new to see here.

Ok, I thought -S alone wasn't enough but if it's, let's go for it then.

#26 lore raw

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

In reply to Juan Quintela
Quoted 30 lines · Juan Quintela
Quoted 28 lines · Luiz Capitulino
Quoted 3 lines · On Wed, 26 May 2010 11:55:31 -0500 Anthony Liguori<[email protected]>…

That's exactly how the protocol is designed. That was one of the major improvements of QMP over the human monior.

Yes and it already has 'id' support:

{ "execute": "cont", "id": "luiz" }
{"timestamp": {"seconds": 1274966635, "microseconds": 776813}, "event": "RESUME"} {"return": {}, "id": "luiz"}

But it doesn't detect duplicates, this is something I think it's up
to the client to do, do you agree?

Quoted 2 lines

This is how the info balloon command works, BTW.

I won't remember the details now, but that interface has some issues and it
has to be reviewed.

Quoted 5 lines

Since there's a clear correlation between the request and the result of the request, an asynchronous command is what makes the most sense. It eliminates the problem of how to pass QErrors via an event which is one of the problems with the current event proposal.

Quoted 9 lines
Quoted 7 lines

Not exactly, this is a problem with QError not the event proposal. We'll
have the same issue if we decide to include errno in the migrate errors and the problem still exists with the BLOCK_IO_ERROR event.

That said, I do agree that migrate should be asynchronous. This yet another
thing we may want to fix before 0.13.

How difficult is that?

Easy. I've got a patch locally and will submit an RFC this afternoon. I'm currently converting the migration error's to QError to make sure than when an error occurs, we can report it in a meaningful way.

We'll still need the MIGRATION_CONNECTED event though and I'm not currently planning on working on that.

Quoted 35 lines
Quoted 27 lines

[...]

Quoted 15 lines
Quoted 15 lines
Quoted 15 lines

For tcp: and unix:, a CONNECTED event absolutely makes sense (every socket server should emit a CONNECTED event). Unfortunately, after CONNECTED you lose the monitor until migration is complete. If something bad happens, you have to exit qemu so once the monitor returns, migration has completed successfully.

If we introduce live incoming migration, we'll need to rethink things. I would actually suggest that we deprecate the incoming command if we do that and make incoming migration a monitor command. I would think it should have the same semantics as migrate (as an asynchronous command). A CONNECTED event still makes sense for tcp and unix protocols but I don't think events make sense for start stop vs. an asynchronous command completion.

Quoted 6 lines
Quoted 3 lines

Do you actually mean 'deprecate -incoming arg' here ?

Yes. And by deprecate, I really mean that -incoming just becomes syntactic sugar for executing a monitor command immediately.

But we can't change -incoming itself, since our command-line is supposed
to be stable, right?

Quoted 8 lines

Also, Juan has said that replacing that arg with a monitor command
doesn't work, as qemu would have to be started in paused monitor for this to work.

So, what about introducing a -incoming-monitor command, which puts qemu
in the right state for migration, but requires a migrate_incoming command to actually start migration?

Quoted 6 lines

this -incoming-monitor is called -S, that should have a long name of -no-autostart

that is what it does, and what we need for incoming migration as monitor command. Nothing new to see here.

Agreed. We could introduce a migrate_incoming command but it wouldn't be possible to poll it's results while the command executed. I'd rather not do that though because that's potentially ugly for clients to deal with.

Regards,

Anthony Liguori

Quoted 2 lines

Later, Juan.

#27 lore raw

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

In reply to Anthony Liguori
Quoted 9 lines · Anthony Liguori
Quoted 3 lines · Daniel P. Berrange

With some disk locking
approaches we need todo a lock transfer before allowing the dest to continue running.

QEMU is going to read the disk before the migration completes so the lock transfer is not going to work with the current implementation (it needs to read the disk to do probing). I assume this is not something that's actually been implemented.

Lock transfer upon migration is a two-phase process. When the QEMU process for incoming migration starts a shared lock is held. Upon completion of migration an attempt is made to upgrade this to an exclusive lock. Only if this upgrade succeeds the dest is allowed to start running.

Quoted 11 lines
Quoted 6 lines

It could be optimized to avoid the -S /cont
in cases where those two scenarios aren't relevant, but only if we can get a separate async notification of when migration starts and completes on the destination, so we can notify mgmt apps that need this lifecycle event.

Migration completes == guest starts running. You'll get a notification of that but you're not getting that now because you're doing -S which I'd argue is a functional problem on the part of libvirt (you're breaking the downtime contract).

If doing non-live migration, or restoring a saved image which was originally in the paused state, there is no 'guest starts running' event. You cannot reliably infer that migration completed by looking for a RESUME event. Hence the need for an explicit notification for the end of migration events.

Quoted 2 lines

I'm not sure why you would need a notification of when migration starts (since you know when you've started migration).

The destination only knows that it has started a QEMU instance ready for incoming migration. It doesn't know when / if an incoming migration has actually been accepted & started processing. Hence the need for an explicit notification for the start of migration.

Daniel

Signature · 4 lines
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
#28 lore raw

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

In reply to Juan Quintela
Quoted 14 lines · On Tue, 25 May 2010 17:35:53 +0200 Juan Quintela <[email protected]> wrote:
Quoted 12 lines · Anthony Liguori

QMP/qmp-events.txt Example: · line 34

Quoted 7 lines · Juan Quintela
QMP/qmp-events.txt +6 -0
+MIGRATION_CANCELED
+------------------
+
+Emitted when migration is canceled.  This is emitted in the source.
+Target will emit MIGRATION_FAILED (no way to differentiate a FAILED
+and CANCELED migration for target).

But the management tool is the one that cancels so surely, it knows why already.

Quoted 1 line

ok, then that one is ok.

Isn't this one important for the destination instead?

#29 lore raw

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

Quoted 17 lines · Luiz Capitulino
Quoted 14 lines · On Tue, 25 May 2010 17:35:53 +0200 Juan Quintela <[email protected]> wrote:
Quoted 12 lines · Anthony Liguori

QMP/qmp-events.txt Example: · line 34

Quoted 7 lines · Juan Quintela
QMP/qmp-events.txt +6 -0
+MIGRATION_CANCELED
+------------------
+
+Emitted when migration is canceled.  This is emitted in the source.
+Target will emit MIGRATION_FAILED (no way to differentiate a FAILED
+and CANCELED migration for target).

But the management tool is the one that cancels so surely, it knows why already.

Quoted 3 lines
Quoted 1 line

ok, then that one is ok.

Isn't this one important for the destination instead?

Destination don't know what happened, only that conection/data is not coming anymore.

Quoted 1 line

From management prespective, management application knows that it has

canceled the migration, so no need to be told.

Later, Juan.

#30 lore raw

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

In reply to Anthony Liguori
Quoted 36 lines · Anthony Liguori
Quoted 4 lines · Juan Quintela

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

QMP/qmp-events.txt +1 -0
+Data: None

QMP/qmp-events.txt Example: · line 46

Quoted 10 lines
QMP/qmp-events.txt +10 -0
+
+Example:
+
+{ "event": "MIGRATION_CANCELED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_ENDED
+---------------
+
+Emitted when migration ends (both in source and target)

A start event is going to be generated already, no?

Quoted 4 lines
QMP/qmp-events.txt +3 -0
+Data: None
+
+Example:

QMP/qmp-events.txt Example: · line 60

Quoted 10 lines
QMP/qmp-events.txt +10 -0
+
+{ "event": "MIGRATION_ENDED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_FAILED
+----------------
+
+Emitted when migration fails (both is source and target).
+
+Data: None

There should be some information about why it failed, no? Preferrably in a QError format.

Quoted 13 lines

QMP/qmp-events.txt Example: · line 70

Quoted 10 lines
QMP/qmp-events.txt +9 -0
+Example:
+
+{ "event": "MIGRATION_FAILED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_STARTED
+-----------------
+
+Emitted when migration starts (both in source and target).

I think this makes more sense as a MIGRATION_CONNECTED event. It probably should carry peer information too.

FYI the original request for these events from a libvirt POV for in terms of identifying the lifecycle transitions.

Currently we issue a migration commend on source, and then have to poll very frequently on 'info migrate' to get progress stats, and to determine completion. We want to poll much less frequently for stats, and get async notification of completion/errors on the source.

Simiarly on the destination, we need to know when any migration operation is taking place, so we can avoid issuing monitor commands to the QEMU process during that time, and also track success/failure + eventually get progress information via an equivalent of 'info migrate' on destination.

So this is really focused on lifecycle transitions, rather than network client connections. I'm not convinced that we should mix up the two sorts of data. If we want to track network client connections IMHO they ought to be separate events. Perhaps there should be a generic QEMU network CONNECT/DISCONNECT event that works for all QEMU network sockets (migration, chardevs, netdev sockets, vnc, spice, and whatever we invent in future using sockets).

Daniel

Signature · 4 lines
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
#31 lore raw

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

In reply to Juan Quintela
Quoted 14 lines · On Tue, 25 May 2010 16:21:03 +0200 Juan Quintela <[email protected]> wrote:

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

---
 QMP/qmp-events.txt |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 monitor.c          |   12 ++++++++++++
 monitor.h          |    4 ++++
 3 files changed, 66 insertions(+), 0 deletions(-)
QMP/qmp-events.txt +0 -0
index 01ec85f..93caa4d 100644

QMP/qmp-events.txt Example: · line 32

Quoted 7 lines
QMP/qmp-events.txt +4 -0
@@ -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.

Shouldn't this one be emitted in the destination?

Quoted 23 lines
QMP/qmp-events.txt +23 -0
+Target will emit MIGRATION_FAILED (no way to differentiate a FAILED
+and CANCELED migration for target).
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_CANCELED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_ENDED
+---------------
+
+Emitted when migration ends (both in source and target)
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_ENDED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_FAILED

QMP/qmp-events.txt Example: · line 65

Quoted 10 lines
QMP/qmp-events.txt +10 -0
+----------------
+
+Emitted when migration fails (both is source and target).
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_FAILED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }

What about a MIGRATION_FINISHED event, which contains a 'success'
key which is a bool?

The only disadvantage of this is if we decide to add more information
to the event (say, stats) then it'd get ugly. Otherwise, one event is enough.

Anyway, the counterpart of MIGRATION_FAILED is MIGRATION_SUCCEEDED.

QMP/qmp-events.txt Example: · line 70

Quoted 5 lines
QMP/qmp-events.txt +5 -0
+
+MIGRATION_STARTED
+-----------------
+
+Emitted when migration starts (both in source and target).

Don't you need this only on the destination?

#32 lore raw

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

Quoted 21 lines · Luiz Capitulino
Quoted 17 lines · On Tue, 25 May 2010 16:21:03 +0200 Juan Quintela<[email protected]> wrote:

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

---
  QMP/qmp-events.txt |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
  monitor.c          |   12 ++++++++++++
  monitor.h          |    4 ++++
  3 files changed, 66 insertions(+), 0 deletions(-)
QMP/qmp-events.txt +0 -0
index 01ec85f..93caa4d 100644
@@ -26,6 +26,56 @@ Example:
  Note: If action is "stop", a STOP event will eventually follow the
  BLOCK_IO_ERROR event.
 
Quoted 7 lines

QMP/qmp-events.txt Example: · line 81

Quoted 5 lines
QMP/qmp-events.txt +4 -0
+MIGRATION_CANCELED
+------------------
+
+Emitted when migration is canceled.  This is emitted in the source.
 

Shouldn't this one be emitted in the destination?

Destination can't distinguish a cancelled from a closed pipe. But the idea is that a third party is talking to both source and destination so it knows if it's cancelled the migration.

Quoted 37 lines
Quoted 24 lines
QMP/qmp-events.txt +24 -0
+Target will emit MIGRATION_FAILED (no way to differentiate a FAILED
+and CANCELED migration for target).
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_CANCELED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_ENDED
+---------------
+
+Emitted when migration ends (both in source and target)
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_ENDED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_FAILED
+----------------

QMP/qmp-events.txt Example: · line 81

Quoted 10 lines
QMP/qmp-events.txt +9 -0
+
+Emitted when migration fails (both is source and target).
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_FAILED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
 

What about a MIGRATION_FINISHED event, which contains a 'success'
key which is a bool?

Quoted 5 lines

The only disadvantage of this is if we decide to add more information
to the event (say, stats) then it'd get ugly. Otherwise, one event is enough.

Anyway, the counterpart of MIGRATION_FAILED is MIGRATION_SUCCEEDED.

I see MIGRATION_FAILED as being very similar to block I/O error events. I think we'll need a very similar solution for both. It boils down to, how do we raise asynchronous events when something fails?

Regards,

Anthony Liguori

#33 lore raw

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

Quoted 30 lines · On Tue, 25 May 2010 13:51:01 -0500 Anthony Liguori <[email protected]>…
Quoted 21 lines · Luiz Capitulino
Quoted 17 lines · On Tue, 25 May 2010 16:21:03 +0200 Juan Quintela<[email protected]> wrote:

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

---
  QMP/qmp-events.txt |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
  monitor.c          |   12 ++++++++++++
  monitor.h          |    4 ++++
  3 files changed, 66 insertions(+), 0 deletions(-)
QMP/qmp-events.txt +0 -0
index 01ec85f..93caa4d 100644
@@ -26,6 +26,56 @@ Example:
  Note: If action is "stop", a STOP event will eventually follow the
  BLOCK_IO_ERROR event.
 
Quoted 7 lines

QMP/qmp-events.txt Example: · line 81

Quoted 5 lines
QMP/qmp-events.txt +4 -0
+MIGRATION_CANCELED
+------------------
+
+Emitted when migration is canceled.  This is emitted in the source.
 

Shouldn't this one be emitted in the destination?

Quoted 3 lines

Destination can't distinguish a cancelled from a closed pipe. But the idea is that a third party is talking to both source and destination so it knows if it's cancelled the migration.

Yeah, got it.

Quoted 40 lines
Quoted 40 lines
Quoted 24 lines
QMP/qmp-events.txt +24 -0
+Target will emit MIGRATION_FAILED (no way to differentiate a FAILED
+and CANCELED migration for target).
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_CANCELED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_ENDED
+---------------
+
+Emitted when migration ends (both in source and target)
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_ENDED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_FAILED
+----------------

QMP/qmp-events.txt Example: · line 81

Quoted 10 lines
QMP/qmp-events.txt +9 -0
+
+Emitted when migration fails (both is source and target).
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_FAILED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
 

What about a MIGRATION_FINISHED event, which contains a 'success'
key which is a bool?

The only disadvantage of this is if we decide to add more information
to the event (say, stats) then it'd get ugly. Otherwise, one event is enough.

Quoted 6 lines
Quoted 2 lines

Anyway, the counterpart of MIGRATION_FAILED is MIGRATION_SUCCEEDED.

I see MIGRATION_FAILED as being very similar to block I/O error events. I think we'll need a very similar solution for both. It boils down to, how do we raise asynchronous events when something fails?

You mean we should have a sort of error specific events?

#34 lore raw

PATCH v2 4/5 QMP: Emit migration events on incoming migration

In reply to Juan Quintela
---
 migration.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
migration.c +3 -0
index c388902..32470d5 100644
@@ -60,10 +60,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");
 
Signature · 1 line
1.6.6.1
#35 lore raw

PATCH v2 5/5 QMP: Emit migration events on outgoing migration

In reply to Juan Quintela
---
 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(-)
migration-exec.c +2 -1
index 07af11a..ebc9256 100644
@@ -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);
     }
migration-fd.c +1 -0
index 6d14505..9c4c7ae 100644
@@ -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;
migration-tcp.c +2 -0
index 20f2e37..11a1203 100644
@@ -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;
migration-unix.c +2 -0
index 57232c0..08f29a3 100644
@@ -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;
migration.c +5 -0
index 32470d5..86535a9 100644
@@ -306,6 +306,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);
 }
@@ -403,8 +404,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);
@@ -427,6 +430,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);
 
@@ -440,6 +444,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);
     }
Signature · 1 line
1.6.6.1