Skip to contentSkip to conversation

Add QMP migration events

PATCH In review Juan Quintela 7 participants 46 messages Jun 9, 2010 – Jun 27, 2010

lore
Share & tools
lore
#0 lore raw

v3 cover Add QMP migration events

This is a resent with what we agreed on yesterday call. Migration events would be there for 0.13 until we get proper async command support.

Later, Juan.

v3:
- Add comment that MIGRATION_FAILURE will add a QError for 0.14 (when we get internal support for that)
rebase against today tree

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 |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 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, 121 insertions(+), 61 deletions(-)
#1 lore raw

PATCH v3 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 fbf2339..ecc67f1 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 97eef4a..e048bb2 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 7121cd0..c35b46e 100644
@@ -3826,7 +3826,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 v3 1/5] Exit if incoming migration fails

Quoted 2 lines · Juan Quintela

Applied 1&2 as we discussed. Thanks.

Regards,

Anthony Liguori

Quoted 75 lines
---
  migration.c |   16 ++++++++++------
  migration.h |    2 +-
  vl.c        |    7 ++++++-
  3 files changed, 17 insertions(+), 8 deletions(-)
migration.c +10 -6
index fbf2339..ecc67f1 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 97eef4a..e048bb2 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 7121cd0..c35b46e 100644
@@ -3826,7 +3826,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();
      }
#3 lore raw

PATCH 1/1 win32: Add define for missing EPROTONOSUPPORT

mingw32 does not define EPROTONOSUPPORT (which is used by migration.c and maybe future patches), so add a definition which uses a supported errno value.

---
 qemu-os-win32.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
qemu-os-win32.h +4 -0
index 6323f7f..2ff9f45 100644
@@ -49,4 +49,8 @@ static inline void os_setup_post(void) {}
 static inline void os_set_line_buffering(void) {}
 static inline void os_set_proc_name(const char *dummy) {}
 
+#if !defined(EPROTONOSUPPORT)
+# define EPROTONOSUPPORT EINVAL
+#endif
+
 #endif
Signature · 1 line
1.7.1
Applied
#4 lore raw

Re: [Qemu-devel] [PATCH] win32: Add define for missing EPROTONOSUPPORT

Thanks, applied.

Quoted 27 lines · Stefan Weil

mingw32 does not define EPROTONOSUPPORT (which is used by migration.c and maybe future patches), so add a definition which uses a supported errno value.

---
 qemu-os-win32.h |    4 ++++

 1 files changed, 4 insertions(+), 0 deletions(-)
qemu-os-win32.h +4 -1
index 6323f7f..2ff9f45 100644
@@ -49,4 +49,8 @@ static inline void os_setup_post(void) {}
  static inline void os_set_line_buffering(void) {}
  static inline void os_set_proc_name(const char *dummy) {}
 
+#if !defined(EPROTONOSUPPORT)
+# define EPROTONOSUPPORT EINVAL
+#endif
+
  #endif
--

1.7.1

#5 lore raw

PATCH v3 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 a8813b4..14718dd 100644
@@ -121,20 +121,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 ecc67f1..4ce19ff 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 e048bb2..d13ed4f 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 v3 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 |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 monitor.c          |   12 ++++++++++++
 monitor.h          |    4 ++++
 3 files changed, 68 insertions(+), 0 deletions(-)
QMP/qmp-events.txt +52 -0
index 01ec85f..cafc4e6 100644
@@ -26,6 +26,58 @@ 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).  Notice
+that this event will be changed for 0.14 when we have infrastructure
+to emit a QError when things fail.
+
+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 15b53b9..a5c5388 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 v3 3/5] QMP: Introduce MIGRATION events

Quoted 39 lines · On Wed, 9 Jun 2010 14:10:56 +0200 Juan Quintela <[email protected]> wrote:

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

---
 QMP/qmp-events.txt |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 monitor.c          |   12 ++++++++++++
 monitor.h          |    4 ++++
 3 files changed, 68 insertions(+), 0 deletions(-)
QMP/qmp-events.txt +22 -0
index 01ec85f..cafc4e6 100644
@@ -26,6 +26,58 @@ 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:

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).  Notice
+that this event will be changed for 0.14 when we have infrastructure
+to emit a QError when things fail.

This is not the kind of information this file should have, compatible
changes should be noted when time comes and incompatible ones are just forbidden after 0.13.

Quoted 61 lines
QMP/qmp-events.txt +20 -0
+
+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 15b53b9..a5c5388 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 v3 3/5] QMP: Introduce MIGRATION events

Quoted 13 lines · Luiz Capitulino

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

Quoted 6 lines · On Wed, 9 Jun 2010 14:10:56 +0200 Juan Quintela <[email protected]> wrote:
QMP/qmp-events.txt +6 -0
+MIGRATION_FAILED
+----------------
+
+Emitted when migration fails (both is source and target).  Notice
+that this event will be changed for 0.14 when we have infrastructure
+to emit a QError when things fail.

This is not the kind of information this file should have, compatible
changes should be noted when time comes and incompatible ones are just forbidden after 0.13.

Then how you express that this value is going to have a QError in it on the future?

Adding a Default QError that puts 'This QError is going to be refined' or what?

Later, Juan.

#9 lore raw

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

Quoted 15 lines · On Thu, 10 Jun 2010 12:33:42 +0200 Juan Quintela <[email protected]> wrote:
Quoted 13 lines · Luiz Capitulino

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

Quoted 6 lines · On Wed, 9 Jun 2010 14:10:56 +0200 Juan Quintela <[email protected]> wrote:
QMP/qmp-events.txt +6 -0
+MIGRATION_FAILED
+----------------
+
+Emitted when migration fails (both is source and target).  Notice
+that this event will be changed for 0.14 when we have infrastructure
+to emit a QError when things fail.

This is not the kind of information this file should have, compatible
changes should be noted when time comes and incompatible ones are just forbidden after 0.13.

Quoted 2 lines

Then how you express that this value is going to have a QError in it on the future?

We don't have to, the doc's purpose is to describe the current state of
the protocol. There might be exceptions, but in this case the change is compatible and extending the protocol is something that is going to happen at every release.

The doc will be updated when the change is introduced.

Quoted 2 lines

Adding a Default QError that puts 'This QError is going to be refined' or what?

We only have to do something like that if having the error information is
something needed right now, which is the case of the BLOCK_IO_ERROR event.

#10 lore raw

PATCH v3 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 4ce19ff..2a44b46 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
#11 lore raw

PATCH v3 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 14718dd..9052cd2 100644
@@ -22,6 +22,7 @@
 #include "block.h"
 #include <sys/types.h>
 #include <sys/wait.h>
+#include "monitor.h"
 
 //#define DEBUG_MIGRATION_EXEC
 
@@ -101,9 +102,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 2a44b46..256a679 100644
@@ -268,6 +268,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);
 }
@@ -371,8 +372,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;
         }
         if (migrate_fd_cleanup(s) < 0) {
@@ -400,6 +403,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);
 
@@ -413,6 +417,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
#12 lore raw
In reply to Juan Quintela

Hi Juan,

2010/6/9 Juan Quintela <[email protected]>:

Quoted 37 lines

This is a resent with what we agreed on yesterday call. Migration events would be there for 0.13 until we get proper async command support.

Later, Juan.

v3:
- Add comment that MIGRATION_FAILURE will add a QError for 0.14 (when we get internal support for that)
 rebase against today tree

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

Quoted 3 lines

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.

I don't know I have addressed the problem correctly, but here is my try to get "info migrate" on incoming side.

http://lists.gnu.org/archive/html/qemu-devel/2010-06/msg00417.html

Apart from that I have a concern on relation between newly introduced QMP Migration events and existing MIG_STATE_*. Are they supposed to be 1 to 1 mapping? If so, instead of calling monitor_protocol_event() everywhere, how about introducing a common function in migration that sets s->mig_state and emits QMP Migration events at once?

Thanks,

Yoshi

Quoted 28 lines

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 |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 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, 121 insertions(+), 61 deletions(-)
#13 lore raw
Quoted 7 lines · Yoshiaki Tamura

Hi Juan,

I don't know I have addressed the problem correctly, but here is my try to get "info migrate" on incoming side.

http://lists.gnu.org/archive/html/qemu-devel/2010-06/msg00417.html

I saw it, haven't had the time to look at it yet.

Quoted 3 lines

Apart from that I have a concern on relation between newly introduced QMP Migration events and existing MIG_STATE_*. Are they supposed to be 1 to 1 mapping?

Good question. In my tree I had info migrate being the same on source and destination, but I didn't have async migration. I have to check with yours.

Quoted 4 lines

If so, instead of calling
monitor_protocol_event() everywhere, how about introducing a common function in migration that sets s->mig_state and emits QMP Migration events at once?

I did that locally, just that this is in the middle of a big cleanup.

I am not sure that is still enough. We have:

- qemu status: running/stopped
- we have another state "incoming" migration that needs to be same level than running/stopped
- I think MIG_STATE_* should also be promotted to this level, but haven't fully thought how to do it.

Later, Juan.

#14 lore raw

2010/6/10 Juan Quintela <[email protected]>:

Quoted 26 lines
Quoted 7 lines · Yoshiaki Tamura

Hi Juan,

I don't know I have addressed the problem correctly, but here is my try to get "info migrate" on incoming side.

http://lists.gnu.org/archive/html/qemu-devel/2010-06/msg00417.html

I saw it, haven't had the time to look at it yet.

Quoted 3 lines

Apart from that I have a concern on relation between newly introduced QMP Migration events and existing MIG_STATE_*.  Are they supposed to be 1 to 1 mapping?

Good question.  In my tree I had info migrate being the same on source and destination, but I didn't have async migration.  I have to check with yours.

Quoted 4 lines

If so, instead of calling
monitor_protocol_event() everywhere, how about introducing a common function in migration that sets s->mig_state and emits QMP Migration events at once?

I did that locally, just that this is in the middle of a big cleanup.

Quoted 7 lines

I am not sure that is still enough.  We have:

- qemu status: running/stopped
- we have another state "incoming" migration that needs to be same level than running/stopped
- I think MIG_STATE_* should also be promotted to this level, but haven't fully thought how to do it.

I think this problem isn't limited to migration. We need an central table that contains the relation between states in qemu and corresponding QMP events that must be emitted. Otherwise, we can get into troubles like missing to emit necessary QMP events, or duplicated QMP events.

Thanks,

Yoshi

Quoted 4 lines

Later, Juan.

#15 lore raw
In reply to Juan Quintela
Quoted 3 lines · On Wed, 9 Jun 2010 14:10:53 +0200 Juan Quintela <[email protected]> wrote:

This is a resent with what we agreed on yesterday call. Migration events would be there for 0.13 until we get proper async command support.

Something which is not clear to me is the set of events we'd have if migrate
was an async command.

Ie, do we really need MIGRATION_FAILED in this case? Don't we expect to get
this information from the async response?

Quoted 65 lines

Later, Juan.

v3:
- Add comment that MIGRATION_FAILURE will add a QError for 0.14 (when we get internal support for that)
rebase against today tree

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 |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 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, 121 insertions(+), 61 deletions(-)
#16 lore raw

2010/6/10 Luiz Capitulino <[email protected]>:

Quoted 7 lines
Quoted 3 lines · On Wed,  9 Jun 2010 14:10:53 +0200 Juan Quintela <[email protected]> wrote:

This is a resent with what we agreed on yesterday call. Migration events would be there for 0.13 until we get proper async command support.

Quoted 5 lines

 Something which is not clear to me is the set of events we'd have if migrate was an async command.

 Ie, do we really need MIGRATION_FAILED in this case? Don't we expect to get this information from the async response?

There would be two types of MIGRATION_FAILED.
One for errors happen during setup of live migration, and one for during the process of live migration. The former is a synchronous event, and later would be an asynchronous event (if you didn't use -d option, this should also be a synchronous event).

I'm not sure whether we should distinguish these failures or not.

Yoshi

Quoted 69 lines
Quoted 66 lines

Later, Juan.

v3:
- Add comment that MIGRATION_FAILURE will add a QError for 0.14 (when we get internal support for that)
  rebase against today tree

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 |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 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, 121 insertions(+), 61 deletions(-)
#17 lore raw
In reply to Luiz Capitulino
Quoted 7 lines · Luiz Capitulino
Quoted 3 lines · On Wed, 9 Jun 2010 14:10:53 +0200 Juan Quintela <[email protected]> wrote:

This is a resent with what we agreed on yesterday call. Migration events would be there for 0.13 until we get proper async command support.

Quoted 6 lines

Something which is not clear to me is the set of events we'd have if migrate
was an async command.

Ie, do we really need MIGRATION_FAILED in this case? Don't we expect to get
this information from the async response?

I am not able to define simpler semantics for this events:

- MIGRATION_STARTED: somebody started a migration, it is emited on source and target, all monitors receive this event.
- MIGRATION_ENDED: migration ended with sucess, all needed data is in target machine. Also emitted in all monitors on source and target.
- MIGRATION_CANCELED: in one of the source monitors somebody typed:
migrate_cancel. It is only emmited on the source monitors, target monitors will receive a MIGRATION_FAILED event.

- MIGRATION_FAILED (with this error). At this point we don't have neither the QMP infraestructure for sending (with this error) nor migration infrastructure to put there anything different than -1.

This event is emmited on all source and target monitors.
- For 0.13: Event don't have a QError.
- For 0.14: It will gain a QError.

About migration becoming an async command. Really it is independent of what events we emit. If migration becomes async command, only difference is for the monitor that emitted the command, rest of monitors see nothing. If we want to be able to see that informantion in the other monitors, we need the events anyways.

Why do we want this? It makes things like audit simpler (we already know when a machine starts/stops, knowing when it migrates is also a good idea. Same for things like the storage management examples that danp did. With events, it becomes trivial, without events, it can be done with workarounds, sending messages left, rigth and center.

Spice also wanted the equivalent of this events to reconnect to the new server machine. I can't see why people have so much trouble with this events, they are of the simpler class. The only real problem that we have is what to put on the MIGRATE_FAILED event, and the problem is an infrastructure one, that we don't have neither what or how to put something useful there.

At this point, management applications only want to know if migration ended with success or with failure, but they will like to know at some point what kind of failure they had.

Later, Juan.

#18 lore raw
Quoted 8 lines · On Thu, 10 Jun 2010 12:44:55 +0200 Juan Quintela <[email protected]> wrote:
Quoted 7 lines · Luiz Capitulino
Quoted 3 lines · On Wed, 9 Jun 2010 14:10:53 +0200 Juan Quintela <[email protected]> wrote:

This is a resent with what we agreed on yesterday call. Migration events would be there for 0.13 until we get proper async command support.

Quoted 8 lines
Quoted 6 lines

Something which is not clear to me is the set of events we'd have if migrate
was an async command.

Ie, do we really need MIGRATION_FAILED in this case? Don't we expect to get
this information from the async response?

I am not able to define simpler semantics for this events:

Ok, I must be missing something here.

First, let's talk about how async commands work today, better yet, how they
should work in 0.14.

I see two possible interfaces (off the top of my head):

1. QMP only returns the response when the command is finished, eg:

    C: { "execute": "migrate", "id": "foo" ... }
    /* nothing is returned, other commands are issued, after several hours... */
    S: { "return": ... "id": "foo" }

2. QMP returns a response right away just to signal that the command

    has been accepted:

    C: { "execute": "migrate", "id": "foo" ... }
    S: { "return": {}, "id": "foo" ... }

    However, the actual response is emitted as an event:

    S: { "event": "ASYNC_RESPONSE", "return": ..., "id": "foo" }

That's what I have in mind, that's why I'm confused about what we're
trying to accomplish here.

Quoted 2 lines

- MIGRATION_STARTED: somebody started a migration, it is emited on source and target, all monitors receive this event.

The client has started the migration, it knows it. Why is the event needed?

Quoted 10 lines

- MIGRATION_ENDED: migration ended with sucess, all needed data is in target machine. Also emitted in all monitors on source and target.

- MIGRATION_CANCELED: in one of the source monitors somebody typed:
migrate_cancel. It is only emmited on the source monitors, target monitors will receive a MIGRATION_FAILED event.

- MIGRATION_FAILED (with this error). At this point we don't have neither the QMP infraestructure for sending (with this error) nor migration infrastructure to put there anything different than -1.

Aren't all the three events above duplicating the async response?

Quoted 9 lines

This event is emmited on all source and target monitors.
- For 0.13: Event don't have a QError.
- For 0.14: It will gain a QError.

About migration becoming an async command. Really it is independent of what events we emit. If migration becomes async command, only difference is for the monitor that emitted the command, rest of monitors see nothing. If we want to be able to see that informantion in the other monitors, we need the events anyways.

Somewhere else in this discussion someone has said that we should assume
that the client talking to the source is the same one which is going to talk to the target, in this case all the communication is done through the source qemu instance.

#19 lore raw
Quoted 28 lines · Luiz Capitulino
Quoted 10 lines · On Thu, 10 Jun 2010 12:44:55 +0200 Juan Quintela<[email protected]> wrote:
Quoted 8 lines · Luiz Capitulino
Quoted 4 lines · On Wed, 9 Jun 2010 14:10:53 +0200 Juan Quintela<[email protected]> wrote:

This is a resent with what we agreed on yesterday call. Migration events would be there for 0.13 until we get proper async command support.

Quoted 9 lines
Quoted 7 lines

Something which is not clear to me is the set of events we'd have if migrate
was an async command.

Ie, do we really need MIGRATION_FAILED in this case? Don't we expect to get
this information from the async response?

I am not able to define simpler semantics for this events:

Ok, I must be missing something here.

First, let's talk about how async commands work today, better yet, how they
should work in 0.14.

Quoted 8 lines

I see two possible interfaces (off the top of my head):

1. QMP only returns the response when the command is finished, eg:

     C: { "execute": "migrate", "id": "foo" ... }
     /* nothing is returned, other commands are issued, after several hours... */
     S: { "return": ... "id": "foo" }

This is how just about every RPC mechanism works...

Quoted 10 lines

2. QMP returns a response right away just to signal that the command

     has been accepted:

     C: { "execute": "migrate", "id": "foo" ... }
     S: { "return": {}, "id": "foo" ... }

     However, the actual response is emitted as an event:

     S: { "event": "ASYNC_RESPONSE", "return": ..., "id": "foo" }

That's quite unusual and I don't see why it would be necessary. It adds quite a bit of complexity because now clients have to handle to distinct error conditions instead of one.

Quoted 5 lines
Quoted 3 lines

- MIGRATION_STARTED: somebody started a migration, it is emited on source and target, all monitors receive this event.

The client has started the migration, it knows it. Why is the event needed?

I think we've more or less agreed that MIGRATION_CONNECTED is really the event we want.

Quoted 7 lines
Quoted 7 lines

- MIGRATION_ENDED: migration ended with sucess, all needed data is in target machine. Also emitted in all monitors on source and target.

- MIGRATION_CANCELED: in one of the source monitors somebody typed:
migrate_cancel. It is only emmited on the source monitors, target monitors will receive a MIGRATION_FAILED event.

Quoted 6 lines
Quoted 4 lines

- MIGRATION_FAILED (with this error). At this point we don't have neither the QMP infraestructure for sending (with this error) nor migration infrastructure to put there anything different than -1.

Aren't all the three events above duplicating the async response?

Yes. Today, we should just generate a MIGRATION_DONE event and let a client poll for failure status.

Quoted 10 lines
Quoted 10 lines

This event is emmited on all source and target monitors.
- For 0.13: Event don't have a QError.
- For 0.14: It will gain a QError.

About migration becoming an async command. Really it is independent of what events we emit. If migration becomes async command, only difference is for the monitor that emitted the command, rest of monitors see nothing. If we want to be able to see that informantion in the other monitors, we need the events anyways.

Quoted 5 lines

Somewhere else in this discussion someone has said that we should assume
that the client talking to the source is the same one which is going to talk to the target, in this case all the communication is done through the source qemu instance.

MIGRATION_DONE gets deprecated for 0.14.

Regards,

Anthony Liguori

#20 lore raw
Quoted 8 lines · On Fri, 11 Jun 2010 09:38:42 -0500 Anthony Liguori <[email protected]>…
Quoted 6 lines

1. QMP only returns the response when the command is finished, eg:

     C: { "execute": "migrate", "id": "foo" ... }
     /* nothing is returned, other commands are issued, after several hours... */
     S: { "return": ... "id": "foo" }

This is how just about every RPC mechanism works...

Let's go for it then.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines

- MIGRATION_STARTED: somebody started a migration, it is emited on source and target, all monitors receive this event.

The client has started the migration, it knows it. Why is the event needed?

I think we've more or less agreed that MIGRATION_CONNECTED is really the event we want.

Is it useful in the source or in the target?

Quoted 16 lines
Quoted 7 lines
Quoted 7 lines

- MIGRATION_ENDED: migration ended with sucess, all needed data is in target machine. Also emitted in all monitors on source and target.

- MIGRATION_CANCELED: in one of the source monitors somebody typed:
migrate_cancel. It is only emmited on the source monitors, target monitors will receive a MIGRATION_FAILED event.

Quoted 6 lines
Quoted 4 lines

- MIGRATION_FAILED (with this error). At this point we don't have neither the QMP infraestructure for sending (with this error) nor migration infrastructure to put there anything different than -1.

Aren't all the three events above duplicating the async response?

Yes. Today, we should just generate a MIGRATION_DONE event and let a client poll for failure status.

[...]

Quoted 1 line

MIGRATION_DONE gets deprecated for 0.14.

Yeah, this removes the need for polling in 0.13, but I was wondering if
it's worth it. If I'm not mistaken, libvirt does the polling when working with the text Monitor and I believe it's not a big deal to keep it until 0.14.

#21 lore raw
Quoted 17 lines · Luiz Capitulino
Quoted 8 lines · On Fri, 11 Jun 2010 09:38:42 -0500 Anthony Liguori <[email protected]>…
Quoted 6 lines

1. QMP only returns the response when the command is finished, eg:

     C: { "execute": "migrate", "id": "foo" ... }
     /* nothing is returned, other commands are issued, after several hours... */
     S: { "return": ... "id": "foo" }

This is how just about every RPC mechanism works...

Let's go for it then.

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines

- MIGRATION_STARTED: somebody started a migration, it is emited on source and target, all monitors receive this event.

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

The client has started the migration, it knows it. Why is the event needed?

I think we've more or less agreed that MIGRATION_CONNECTED is really the event we want.

Is it useful in the source or in the target?

Both.

Quoted 17 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

- MIGRATION_ENDED: migration ended with sucess, all needed data is in target machine. Also emitted in all monitors on source and target.

- MIGRATION_CANCELED: in one of the source monitors somebody typed:
migrate_cancel. It is only emmited on the source monitors, target monitors will receive a MIGRATION_FAILED event.

- MIGRATION_FAILED (with this error). At this point we don't have neither the QMP infraestructure for sending (with this error) nor migration infrastructure to put there anything different than -1.

Quoted 5 lines
Quoted 2 lines

Aren't all the three events above duplicating the async response?

Yes. Today, we should just generate a MIGRATION_DONE event and let a client poll for failure status.

Quoted 7 lines

[...]

Quoted 1 line

MIGRATION_DONE gets deprecated for 0.14.

Yeah, this removes the need for polling in 0.13, but I was wondering if
it's worth it. If I'm not mistaken, libvirt does the polling when working with the text Monitor and I believe it's not a big deal to keep it until 0.14.

It makes things slower for no good reason.

This reasoning is sneaky at least:
- qemu didn't give interfaces to libvirt for do what libvirt wanted
- libvirt uses workarounds
- qemu tells libvirt that they are using workarounds that they shouldn't
- libvirt tells qemu why they need the new interface
- qemu tells libvirt that they could continue to use its workarounds.

I am losing something? The whole point of live migration is that they need to be as fast as possible. For some scenaries (shared storage with funny locking) libvirt needs to move from shared locks to normal locks as far as migration ends on target. We are telling them to do workarounds becauese qemu don't want to tell libvirt on destination when migration has ended.

Why can't we just tell them that migration has ended with success as fast as possible?

I can't understand what I am missing here. I can't believe that libvirt(management app in general) could came with a simple request that would make its live better. And to make things worse, it is _trivial_ to implement, semantics are clear, has other uses, .....

Later, Juan.

#22 lore raw
Quoted 21 lines · On Sat, 12 Jun 2010 13:20:54 +0200 Juan Quintela <[email protected]> wrote:
Quoted 20 lines · Luiz Capitulino
Quoted 8 lines · On Fri, 11 Jun 2010 09:38:42 -0500 Anthony Liguori <[email protected]>…
Quoted 6 lines

1. QMP only returns the response when the command is finished, eg:

     C: { "execute": "migrate", "id": "foo" ... }
     /* nothing is returned, other commands are issued, after several hours... */
     S: { "return": ... "id": "foo" }

This is how just about every RPC mechanism works...

Let's go for it then.

Quoted 6 lines
Quoted 5 lines
Quoted 3 lines

- MIGRATION_STARTED: somebody started a migration, it is emited on source and target, all monitors receive this event.

The client has started the migration, it knows it. Why is the event needed?

Quoted 6 lines
Quoted 4 lines
Quoted 2 lines

I think we've more or less agreed that MIGRATION_CONNECTED is really the event we want.

Is it useful in the source or in the target?

Both.

What does it report in the source?

Quoted 41 lines
Quoted 17 lines
Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

- MIGRATION_ENDED: migration ended with sucess, all needed data is in target machine. Also emitted in all monitors on source and target.

- MIGRATION_CANCELED: in one of the source monitors somebody typed:
migrate_cancel. It is only emmited on the source monitors, target monitors will receive a MIGRATION_FAILED event.

- MIGRATION_FAILED (with this error). At this point we don't have neither the QMP infraestructure for sending (with this error) nor migration infrastructure to put there anything different than -1.

Quoted 5 lines
Quoted 2 lines

Aren't all the three events above duplicating the async response?

Yes. Today, we should just generate a MIGRATION_DONE event and let a client poll for failure status.

Quoted 7 lines

[...]

Quoted 1 line

MIGRATION_DONE gets deprecated for 0.14.

Yeah, this removes the need for polling in 0.13, but I was wondering if
it's worth it. If I'm not mistaken, libvirt does the polling when working with the text Monitor and I believe it's not a big deal to keep it until 0.14.

It makes things slower for no good reason.

This reasoning is sneaky at least:
- qemu didn't give interfaces to libvirt for do what libvirt wanted
- libvirt uses workarounds
- qemu tells libvirt that they are using workarounds that they shouldn't
- libvirt tells qemu why they need the new interface
- qemu tells libvirt that they could continue to use its workarounds.

I am losing something? The whole point of live migration is that they need to be as fast as possible. For some scenaries (shared storage with funny locking) libvirt needs to move from shared locks to normal locks as far as migration ends on target. We are telling them to do workarounds becauese qemu don't want to tell libvirt on destination when migration has ended.

Quoted 7 lines

Why can't we just tell them that migration has ended with success as fast as possible?

I can't understand what I am missing here. I can't believe that libvirt(management app in general) could came with a simple request that would make its live better. And to make things worse, it is _trivial_ to implement, semantics are clear, has other uses, .....

I'd be ok in having the done event if the polling is shown to be that
problematic, but from some private talks I had with libvirt guys it seemed to me that it's more like a wish have.

For us, having this event means that we'll have to carry it for (at least)
several releases. I really would like to avoid adding one-time workarounds in a stable interface like QMP.

#23 lore raw
Quoted 6 lines · Luiz Capitulino
Quoted 1 line · On Sat, 12 Jun 2010 13:20:54 +0200 Juan Quintela <[email protected]> wrote:

Both.

What does it report in the source?

That migration has started :) Nothing else, nothing less.

Think again multiple monitors and/or audit.

Quoted 8 lines
Quoted 7 lines

Why can't we just tell them that migration has ended with success as fast as possible?

I can't understand what I am missing here. I can't believe that libvirt(management app in general) could came with a simple request that would make its live better. And to make things worse, it is _trivial_ to implement, semantics are clear, has other uses, .....

Quoted 7 lines

I'd be ok in having the done event if the polling is shown to be that
problematic, but from some private talks I had with libvirt guys it seemed to me that it's more like a wish have.

For us, having this event means that we'll have to carry it for (at least)
several releases. I really would like to avoid adding one-time workarounds in a stable interface like QMP.

For me, this is going to be there forever. It has clear semantics, it is useful and all other alternatives given to date are convoluted at least.

Later, Juan.

#24 lore raw
In reply to Anthony Liguori
Quoted 7 lines · Anthony Liguori
Quoted 2 lines · Luiz Capitulino

On Thu, 10 Jun 2010 12:44:55 +0200
Juan Quintela<[email protected]> wrote:

I think we've more or less agreed that MIGRATION_CONNECTED is really the event we want.

This had the problem of migrating to a file/whatever.

MIGRATION_CONECTED only makes sense when you have TCP or similar. MIGRATION_STARTED it just means that migration has started, independently of whatever method you have. For TCP it is possible that we want another event each time that somebody connected to a port (not only for migration), but that is something different.

Quoted 11 lines
Quoted 11 lines
Quoted 11 lines

- MIGRATION_ENDED: migration ended with sucess, all needed data is in target machine. Also emitted in all monitors on source and target.

- MIGRATION_CANCELED: in one of the source monitors somebody typed:
migrate_cancel. It is only emmited on the source monitors, target monitors will receive a MIGRATION_FAILED event.

- MIGRATION_FAILED (with this error). At this point we don't have neither the QMP infraestructure for sending (with this error) nor migration infrastructure to put there anything different than -1.

Quoted 5 lines
Quoted 2 lines

Aren't all the three events above duplicating the async response?

Yes. Today, we should just generate a MIGRATION_DONE event and let a client poll for failure status.

I disagree completely. It just defeat the reason for this.

MIGRATION_ENDED on destination machine: go ahead, everything is ok.
MIGRATION_FAILED: Uh, oh, something got wrong, we are in the slow path.

With MIGRATION_DONE + polling, we are delaying the "success" case just to avoid having a new event. I don't buy it.

Quoted 10 lines
Quoted 10 lines
Quoted 10 lines

This event is emmited on all source and target monitors.
- For 0.13: Event don't have a QError.
- For 0.14: It will gain a QError.

About migration becoming an async command. Really it is independent of what events we emit. If migration becomes async command, only difference is for the monitor that emitted the command, rest of monitors see nothing. If we want to be able to see that informantion in the other monitors, we need the events anyways.

Quoted 7 lines
Quoted 5 lines

Somewhere else in this discussion someone has said that we should assume
that the client talking to the source is the same one which is going to talk to the target, in this case all the communication is done through the source qemu instance.

MIGRATION_DONE gets deprecated for 0.14.

I still think that we want the 4 events that I described. My understanding is that libvirt people also would love to have that 4 events.

Answer here is that: you can do this workaround and this other workaround and you can get that information.

About semantics of messages, I don't see anytime soon that migration are going to change from:

Start migration and then end with success or failure.

The only one that we could change/remove is MIGRATION_CANCEL to MIGRATION_FAILURE(User canceled) it. But that is it.

Why have to do a polling when none is needed?
If you preffer to change the MIGRATION_ENDED + MIGRATION_FAILURE(error) to MIGRATION_ENDED(result code), and you have to check the error code, I can also live with that. But that is it.

Later, Juan.

#25 lore raw
Quoted 4 lines · Juan Quintela
Quoted 2 lines · Anthony Liguori

On 06/11/2010 09:30 AM, Luiz Capitulino wrote:

Quoted 9 lines
Quoted 7 lines
Quoted 3 lines

On Thu, 10 Jun 2010 12:44:55 +0200
Juan Quintela<[email protected]> wrote:

I think we've more or less agreed that MIGRATION_CONNECTED is really the event we want.

This had the problem of migrating to a file/whatever.

With migrating to exec, the MIGRATION_STARTED event fires as soon as you launch qemu which is also very unlikely to actually be when a QMP client is connected. IOW, I'm fairly certain you'll never see a MIGRATION_STARTED event with QMP for exec migration anyway.

It's not useful information. MIGRATION_CONNECTED is useful information and something we'll want to support long term.

Quoted 11 lines
Quoted 3 lines

Yes. Today, we should just generate a MIGRATION_DONE event and let a client poll for failure status.

I disagree completely. It just defeat the reason for this.

MIGRATION_ENDED on destination machine: go ahead, everything is ok.
MIGRATION_FAILED: Uh, oh, something got wrong, we are in the slow path.

With MIGRATION_DONE + polling, we are delaying the "success" case just to avoid having a new event. I don't buy it.

This is an event that we plan on deprecating immediately. Why introduce two events that are going to be immediately deprecated when we can just introduce one?

Quoted 12 lines

I still think that we want the 4 events that I described. My understanding is that libvirt people also would love to have that 4 events.

Answer here is that: you can do this workaround and this other workaround and you can get that information.

About semantics of messages, I don't see anytime soon that migration are going to change from:

Start migration and then end with success or failure.

Quoted 8 lines

The only one that we could change/remove is MIGRATION_CANCEL to MIGRATION_FAILURE(User canceled) it. But that is it.

Why have to do a polling when none is needed?
If you preffer to change the MIGRATION_ENDED + MIGRATION_FAILURE(error) to MIGRATION_ENDED(result code), and you have to check the error code, I can also live with that. But that is it.

We will have a MIGRATION_ENDED(result code) event for 0.14. It'll be a proper async migrate command. But we have no way to do this sanely because we can't generate rich errors during migrate and we can't pass QErrors over async events.

For 0.13, we need to focus on introducing the least disruptive change that addresses the fundamental requirement--allow clients to avoid a polling loop for determining when migration ends. Having a single event with no payload is an extremely simple change.

Regards,

Anthony Liguori

Quoted 2 lines

Later, Juan.

#26 lore raw
Quoted 4 lines · On Mon, 14 Jun 2010 08:58:19 -0500 Anthony Liguori…

For 0.13, we need to focus on introducing the least disruptive change that addresses the fundamental requirement--allow clients to avoid a polling loop for determining when migration ends. Having a single event with no payload is an extremely simple change.

Agreed.

Actually, I'm slightly against introducing the done event too. While the
polling is undesired, I don't think that having an event only for a release is worth it.

#27 lore raw
Quoted 9 lines · Luiz Capitulino
Quoted 5 lines · On Mon, 14 Jun 2010 08:58:19 -0500 Anthony…

For 0.13, we need to focus on introducing the least disruptive change that addresses the fundamental requirement--allow clients to avoid a polling loop for determining when migration ends. Having a single event with no payload is an extremely simple change.

Quoted 6 lines

Agreed.

Actually, I'm slightly against introducing the done event too. While the
polling is undesired, I don't think that having an event only for a release is worth it.

I don't think we can realistically remove it. In my mind, what deprecate means with respect to QMP is that we never add additional features to the deprecated commands.

Right now, 'migrate', 'info migrate', and MIGRATE_DONE do not provide very rich error reporting. They really just provide a boolean failed/succeeded.

For 0.14, we'll have to introduce a 'migrate2' command and deprecate the old 'migrate' command. 'migrate2' will provide rich error reporting and all new features will be added to 'migrate2'.

BTW, I lack any kind of naming creativity so 'migrate2' is not necessarily what it ought to be called.

Regards,

Anthony Liguori

#28 lore raw
Quoted 12 lines · On Mon, 14 Jun 2010 09:35:52 -0500 Anthony Liguori…
Quoted 11 lines · Luiz Capitulino
Quoted 5 lines · On Mon, 14 Jun 2010 08:58:19 -0500 Anthony…

For 0.13, we need to focus on introducing the least disruptive change that addresses the fundamental requirement--allow clients to avoid a polling loop for determining when migration ends. Having a single event with no payload is an extremely simple change.

Agreed.

Quoted 8 lines
Quoted 4 lines

Actually, I'm slightly against introducing the done event too. While the
polling is undesired, I don't think that having an event only for a release is worth it.

I don't think we can realistically remove it. In my mind, what deprecate means with respect to QMP is that we never add additional features to the deprecated commands.

Oh, by 'only for a release' I meant 'useful only for a release', ie. I
didn't mean we would remove it but that it will only be useful for 0.13.

Quoted 10 lines

Right now, 'migrate', 'info migrate', and MIGRATE_DONE do not provide very rich error reporting. They really just provide a boolean failed/succeeded.

For 0.14, we'll have to introduce a 'migrate2' command and deprecate the old 'migrate' command. 'migrate2' will provide rich error reporting and all new features will be added to 'migrate2'.

BTW, I lack any kind of naming creativity so 'migrate2' is not necessarily what it ought to be called.

We can have a contest.

#29 lore raw
In reply to Luiz Capitulino
Quoted 44 lines · Luiz Capitulino
Quoted 8 lines · On Thu, 10 Jun 2010 12:44:55 +0200 Juan Quintela <[email protected]> wrote:
Quoted 7 lines · Luiz Capitulino
Quoted 3 lines · On Wed, 9 Jun 2010 14:10:53 +0200 Juan Quintela <[email protected]> wrote:

This is a resent with what we agreed on yesterday call. Migration events would be there for 0.13 until we get proper async command support.

Quoted 8 lines
Quoted 6 lines

Something which is not clear to me is the set of events we'd have if migrate
was an async command.

Ie, do we really need MIGRATION_FAILED in this case? Don't we expect to get
this information from the async response?

I am not able to define simpler semantics for this events:

Ok, I must be missing something here.

First, let's talk about how async commands work today, better yet, how they
should work in 0.14.

I see two possible interfaces (off the top of my head):

1. QMP only returns the response when the command is finished, eg:

    C: { "execute": "migrate", "id": "foo" ... }
    /* nothing is returned, other commands are issued, after several hours... */
    S: { "return": ... "id": "foo" }

2. QMP returns a response right away just to signal that the command

    has been accepted:

    C: { "execute": "migrate", "id": "foo" ... }
    S: { "return": {}, "id": "foo" ... }

    However, the actual response is emitted as an event:

    S: { "event": "ASYNC_RESPONSE", "return": ..., "id": "foo" }
Quoted 2 lines

That's what I have in mind, that's why I'm confused about what we're
trying to accomplish here.

You continue forgeting the case that you have more than one monitor conected. The other monitor will not receive _ethire_ of that responses. Will not know what is happening.

Quoted 4 lines
Quoted 2 lines

- MIGRATION_STARTED: somebody started a migration, it is emited on source and target, all monitors receive this event.

The client has started the migration, it knows it. Why is the event needed?

The monitor that did it knows it, nobody else knows it. At destination time, I guess you agree this is important, i.e. the management app knows that migration has started.

I have been needinng this for audit, knowing when guest start/stop/migrates. And just now the only way to get that information is to "hack" qemu source code. With migration_events it will be "trivial" to know when that happens.

In libvirt case. First thing that I would do if I receive a MIGRATION_START command that I didn't started: I release control of that VM, something fishy happened. At this point, it is imposible to know what happens.

Quoted 7 lines
Quoted 7 lines

- MIGRATION_ENDED: migration ended with sucess, all needed data is in target machine. Also emitted in all monitors on source and target.

- MIGRATION_CANCELED: in one of the source monitors somebody typed:
migrate_cancel. It is only emmited on the source monitors, target monitors will receive a MIGRATION_FAILED event.

Quoted 5 lines
Quoted 3 lines

- MIGRATION_FAILED (with this error). At this point we don't have neither the QMP infraestructure for sending (with this error) nor migration infrastructure to put there anything different than -1.

Aren't all the three events above duplicating the async response?

Again, no. Think that you have more than one monitor. And indeed in the case of a single monitor. We are delaying the information to the target management app.

MIGRATION_ENDED on target machine: We can do whatever is needed when migration has ended. Async (or sync) answer to the source management app, it needs to receive that information + send that information to destination machine + receive information in destination machine + do whatever is needed on destination vm.

Just because we refuse to give Information that we have, ready. I am not asking for something that is difficult to do in qemu, it is information that qemu knows (when migration has started/ended). And we are telling management apps that they need to guess when things happens and use polling to know it.

Quoted 10 lines
Quoted 9 lines

This event is emmited on all source and target monitors.
- For 0.13: Event don't have a QError.
- For 0.14: It will gain a QError.

About migration becoming an async command. Really it is independent of what events we emit. If migration becomes async command, only difference is for the monitor that emitted the command, rest of monitors see nothing. If we want to be able to see that informantion in the other monitors, we need the events anyways.

Quoted 4 lines

Somewhere else in this discussion someone has said that we should assume
that the client talking to the source is the same one which is going to talk to the target, in this case all the communication is done through the source qemu instance.

That is another problem, that we don't have a monitor in the destination target during migration.

Later, Juan.

#30 lore raw
Quoted 58 lines · Juan Quintela
Quoted 47 lines · Luiz Capitulino
Quoted 10 lines · On Thu, 10 Jun 2010 12:44:55 +0200 Juan Quintela<[email protected]> wrote:
Quoted 8 lines · Luiz Capitulino
Quoted 4 lines · On Wed, 9 Jun 2010 14:10:53 +0200 Juan Quintela<[email protected]> wrote:

This is a resent with what we agreed on yesterday call. Migration events would be there for 0.13 until we get proper async command support.

Quoted 9 lines
Quoted 7 lines

Something which is not clear to me is the set of events we'd have if migrate
was an async command.

Ie, do we really need MIGRATION_FAILED in this case? Don't we expect to get
this information from the async response?

I am not able to define simpler semantics for this events:

Ok, I must be missing something here.

First, let's talk about how async commands work today, better yet, how they
should work in 0.14.

I see two possible interfaces (off the top of my head):

1. QMP only returns the response when the command is finished, eg:

     C: { "execute": "migrate", "id": "foo" ... }
     /* nothing is returned, other commands are issued, after several hours... */
     S: { "return": ... "id": "foo" }

2. QMP returns a response right away just to signal that the command

     has been accepted:

     C: { "execute": "migrate", "id": "foo" ... }
     S: { "return": {}, "id": "foo" ... }

     However, the actual response is emitted as an event:

     S: { "event": "ASYNC_RESPONSE", "return": ..., "id": "foo" }
Quoted 3 lines

That's what I have in mind, that's why I'm confused about what we're
trying to accomplish here.

You continue forgeting the case that you have more than one monitor conected. The other monitor will not receive _ethire_ of that responses. Will not know what is happening.

Quoted 9 lines
Quoted 5 lines
Quoted 3 lines

- MIGRATION_STARTED: somebody started a migration, it is emited on source and target, all monitors receive this event.

The client has started the migration, it knows it. Why is the event needed?

The monitor that did it knows it, nobody else knows it. At destination time, I guess you agree this is important, i.e. the management app knows that migration has started.

Dual monitors is a slippery slope argument because even if you had these events, it doesn't give you nearly enough information to implement anything safely.

If you truly want to support dual uncooperative monitors, you basically need to mirror every monitor session so that each monitor can see what the other monitors are doing. You also need a transaction mechanism to allow a client to do operations in a non-racy way.

Since we're not likely to ever implement these things, dual monitor support is really not a viable argument for such a change.

Quoted 5 lines

I have been needinng this for audit, knowing when guest start/stop/migrates. And just now the only way to get that information is to "hack" qemu source code. With migration_events it will be "trivial" to know when that happens.

QMP is the wrong mechanism for this. Merging the tracing code and then adding trace points to migration is the right solution for this problem.

Quoted 23 lines
Quoted 7 lines
Quoted 7 lines

- MIGRATION_ENDED: migration ended with sucess, all needed data is in target machine. Also emitted in all monitors on source and target.

- MIGRATION_CANCELED: in one of the source monitors somebody typed:
migrate_cancel. It is only emmited on the source monitors, target monitors will receive a MIGRATION_FAILED event.

Quoted 6 lines
Quoted 4 lines

- MIGRATION_FAILED (with this error). At this point we don't have neither the QMP infraestructure for sending (with this error) nor migration infrastructure to put there anything different than -1.

Aren't all the three events above duplicating the async response?

Again, no. Think that you have more than one monitor. And indeed in the case of a single monitor. We are delaying the information to the target management app.

MIGRATION_ENDED on target machine: We can do whatever is needed when migration has ended. Async (or sync) answer to the source management app, it needs to receive that information + send that information to destination machine + receive information in destination machine + do whatever is needed on destination vm.

Quoted 6 lines

Just because we refuse to give Information that we have, ready. I am not asking for something that is difficult to do in qemu, it is information that qemu knows (when migration has started/ended). And we are telling management apps that they need to guess when things happens and use polling to know it.

The problem is, all of the arguments you're using to justify this for the migrate command is applicable for every other command we have. Why do this for migrate and not for commit or savevm?

Do we really want to introduce 4 events for every command that we support?

Regards,

Anthony Liguori

#31 lore raw
Quoted 11 lines · Anthony Liguori
Quoted 6 lines · Juan Quintela

Luiz Capitulino<[email protected]> wrote:

The monitor that did it knows it, nobody else knows it. At destination time, I guess you agree this is important, i.e. the management app knows that migration has started.

Dual monitors is a slippery slope argument because even if you had these events, it doesn't give you nearly enough information to implement anything safely.

Security folks here needed to do logging of qemu events, here. Guest what ones: vm_start, vm_stop, vm_continue, and vm_migrate.

Insteod of a nice: write this "small qmp client, and listen for this four events, I just had to point them where to hack their logging.

About libvirt, I would rreally, really like to be able to use libvirt to launch a guest, and then let im me launch another monitor and stoup libvirt for continuing with it. There is no way for a monitor that there has been doing "write" operations in other monitors. I see this as a useful feature for all "write" (i.e. not query) commands.

Quoted 5 lines

If you truly want to support dual uncooperative monitors, you basically need to mirror every monitor session so that each monitor can see what the other monitors are doing. You also need a transaction mechanism to allow a client to do operations in a non-racy way.

For now, I will set with just knowing that other "write" operations has happened.

Quoted 2 lines

Since we're not likely to ever implement these things, dual monitor support is really not a viable argument for such a change.

As showed before for the audit logging, A "read only" monitor would have been useful for me "now", i.e. not I wish, whatever.

Quoted 9 lines

QMP is the wrong mechanism for this. Merging the tracing code and then adding trace points to migration is the right solution for this problem.

The problem is, all of the arguments you're using to justify this for the migrate command is applicable for every other command we have. Why do this for migrate and not for commit or savevm?

Do we really want to introduce 4 events for every command that we support?

Migration commands have a "feature" that dont' have other commands: they invosve two machines.

And I would also liked to have that events for all the "write" commands. Migration is more "interesting" becaues it needs synchronization.

Later, Juan.

#32 lore raw
Quoted 12 lines · Juan Quintela
Quoted 10 lines · Anthony Liguori
Quoted 8 lines · Juan Quintela

Luiz Capitulino<[email protected]> wrote:

The monitor that did it knows it, nobody else knows it. At destination time, I guess you agree this is important, i.e. the management app knows that migration has started.

Quoted 7 lines
Quoted 4 lines

Dual monitors is a slippery slope argument because even if you had these events, it doesn't give you nearly enough information to implement anything safely.

Security folks here needed to do logging of qemu events, here. Guest what ones: vm_start, vm_stop, vm_continue, and vm_migrate.

Why do security folks need this? Why are they not interested in other things like savevm? Why are they talkign to qemu and not libvirt (they shouldn't trust qemu).

Quoted 9 lines

Insteod of a nice: write this "small qmp client, and listen for this four events, I just had to point them where to hack their logging.

About libvirt, I would rreally, really like to be able to use libvirt to launch a guest, and then let im me launch another monitor and stoup libvirt for continuing with it. There is no way for a monitor that there has been doing "write" operations in other monitors. I see this as a useful feature for all "write" (i.e. not query) commands.

Yeah, but if we want to do this, then we need to discuss this with the libvirt folks and come up with a proposal that works for all commands. Sneaking in a few migration events is not going to help.

Quoted 9 lines
Quoted 9 lines

QMP is the wrong mechanism for this. Merging the tracing code and then adding trace points to migration is the right solution for this problem.

The problem is, all of the arguments you're using to justify this for the migrate command is applicable for every other command we have. Why do this for migrate and not for commit or savevm?

Quoted 8 lines
Quoted 2 lines

Do we really want to introduce 4 events for every command that we support?

Migration commands have a "feature" that dont' have other commands: they invosve two machines.

And I would also liked to have that events for all the "write" commands. Migration is more "interesting" becaues it needs synchronization.

I'm still fundamentally confused about what you think you can do with these events. But do you really intend on introducing events for every non-query QMP command? Does that seem a bit unrealistic?

Wouldn't it be better to have a mechanism to snoop on monitor traffic in a more proper way?

Regards,

Anthony Liguori

Quoted 2 lines

Later, Juan.

#33 lore raw
Quoted 17 lines · Anthony Liguori
Quoted 16 lines · Juan Quintela
Quoted 14 lines · Anthony Liguori
Quoted 8 lines · Juan Quintela

Luiz Capitulino<[email protected]> wrote:

The monitor that did it knows it, nobody else knows it. At destination time, I guess you agree this is important, i.e. the management app knows that migration has started.

Dual monitors is a slippery slope argument because even if you had these events, it doesn't give you nearly enough information to implement anything safely.

Quoted 7 lines
Quoted 3 lines

Security folks here needed to do logging of qemu events, here. Guest what ones: vm_start, vm_stop, vm_continue, and vm_migrate.

Why do security folks need this? Why are they not interested in other things like savevm? Why are they talkign to qemu and not libvirt (they shouldn't trust qemu).

No clue about last one. I just was asked to provide that list of events. will ask back.

Quoted 10 lines
Quoted 9 lines

Insteod of a nice: write this "small qmp client, and listen for this four events, I just had to point them where to hack their logging.

About libvirt, I would rreally, really like to be able to use libvirt to launch a guest, and then let im me launch another monitor and stoup libvirt for continuing with it. There is no way for a monitor that there has been doing "write" operations in other monitors. I see this as a useful feature for all "write" (i.e. not query) commands.

Quoted 3 lines

Yeah, but if we want to do this, then we need to discuss this with the libvirt folks and come up with a proposal that works for all commands. Sneaking in a few migration events is not going to help.

Fully agree.

Quoted 10 lines
Quoted 6 lines

Migration commands have a "feature" that dont' have other commands: they invosve two machines.

And I would also liked to have that events for all the "write" commands. Migration is more "interesting" becaues it needs synchronization.

I'm still fundamentally confused about what you think you can do with these events. But do you really intend on introducing events for every non-query QMP command? Does that seem a bit unrealistic?

Ok. lets stop here. My definitions:

Event: this important thing happened (important has several meanings).

Migration events fully enter in this definition. Furthermore, migration events happens from actions that are issued in machine A and event happens in machine A and machine B. (I.e. they are so special as they can get).

Now convenience. I "think" it would be convenient to also know in the other monitors when any "write" command happens. About how to implement this, if there are more uses or no, .... that is clearly open to discussion. I think that this enter fully in the politics vs mechanism discussions, events allow you to notify when things happen, and management app can do anything that it sees fit.

As principle, I think that "important happenings" (to not repeat the "event" word) should be published in a very clear way. Migration start/end are a basic example of that. It is not as if Migration is going to stop having a "start" or an "end" any time soon. Making the app polling to know that is too cumbersome for the "normal" good case. This kind of things should be plublished "somehow". The same that happens when a machine start/stops. That are improntant events.

Quoted 2 lines

Wouldn't it be better to have a mechanism to snoop on monitor traffic in a more proper way?

I haven't looked at the trace commands that you pointed (so no comment there), but I see it "interesting" to be able to know what commands have been issued from other monitor.

In my ideal world, I would get a monitor from libvirt, where I could play. And libvirt will look at the commands that I issue and decided if he can continue or just leave the guest for me.

Notice the "ideal world" part O:-)

Adiso, Juan.

#34 lore raw
Quoted 66 lines · Juan Quintela
Quoted 24 lines · Anthony Liguori
Quoted 22 lines · Juan Quintela
Quoted 19 lines · Anthony Liguori
Quoted 11 lines · Juan Quintela

Luiz Capitulino<[email protected]> wrote:

The monitor that did it knows it, nobody else knows it. At destination time, I guess you agree this is important, i.e. the management app knows that migration has started.

Dual monitors is a slippery slope argument because even if you had these events, it doesn't give you nearly enough information to implement anything safely.

Quoted 8 lines
Quoted 4 lines

Security folks here needed to do logging of qemu events, here. Guest what ones: vm_start, vm_stop, vm_continue, and vm_migrate.

Why do security folks need this? Why are they not interested in other things like savevm? Why are they talkign to qemu and not libvirt (they shouldn't trust qemu).

No clue about last one. I just was asked to provide that list of events. will ask back.

Quoted 10 lines
Quoted 10 lines

Insteod of a nice: write this "small qmp client, and listen for this four events, I just had to point them where to hack their logging.

About libvirt, I would rreally, really like to be able to use libvirt to launch a guest, and then let im me launch another monitor and stoup libvirt for continuing with it. There is no way for a monitor that there has been doing "write" operations in other monitors. I see this as a useful feature for all "write" (i.e. not query) commands.

Quoted 4 lines

Yeah, but if we want to do this, then we need to discuss this with the libvirt folks and come up with a proposal that works for all commands. Sneaking in a few migration events is not going to help.

Fully agree.

Quoted 11 lines
Quoted 7 lines

Migration commands have a "feature" that dont' have other commands: they invosve two machines.

And I would also liked to have that events for all the "write" commands. Migration is more "interesting" becaues it needs synchronization.

I'm still fundamentally confused about what you think you can do with these events. But do you really intend on introducing events for every non-query QMP command? Does that seem a bit unrealistic?

Quoted 9 lines

Ok. lets stop here. My definitions:

Event: this important thing happened (important has several meanings).

Migration events fully enter in this definition. Furthermore, migration events happens from actions that are issued in machine A and event happens in machine A and machine B. (I.e. they are so special as they can get).

I think you've got too narrow a view. Migration doesn't always involve two machines. Migration can involve just the source writing via "exec:dd of=foo.img" and this is in fact an important use case for libvirt.

Quoted 7 lines

Now convenience. I "think" it would be convenient to also know in the other monitors when any "write" command happens. About how to implement this, if there are more uses or no, .... that is clearly open to discussion. I think that this enter fully in the politics vs mechanism discussions, events allow you to notify when things happen, and management app can do anything that it sees fit.

Quoted 8 lines

As principle, I think that "important happenings" (to not repeat the "event" word) should be published in a very clear way. Migration start/end are a basic example of that. It is not as if Migration is going to stop having a "start" or an "end" any time soon. Making the app polling to know that is too cumbersome for the "normal" good case. This kind of things should be plublished "somehow". The same that happens when a machine start/stops. That are improntant events.

What makes migration important and not savevm?

It's not that I don't agree that migration is important and that it's important for tools to be able to know about it. I disagree that migration is *more* important than most of the other things that happen in the monitor and I want to make sure that we come up with a solution that solves the broader problem.

Regards,

Anthony Liguori

#35 lore raw
Quoted 12 lines · Anthony Liguori
Quoted 9 lines · Juan Quintela

Ok. lets stop here. My definitions:

Event: this important thing happened (important has several meanings).

Migration events fully enter in this definition. Furthermore, migration events happens from actions that are issued in machine A and event happens in machine A and machine B. (I.e. they are so special as they can get).

Quoted 4 lines

I think you've got too narrow a view. Migration doesn't always involve two machines. Migration can involve just the source writing via "exec:dd of=foo.img" and this is in fact an important use case for libvirt.

In this case, I also want to know when migration ended.

Quoted 16 lines
Quoted 7 lines

Now convenience. I "think" it would be convenient to also know in the other monitors when any "write" command happens. About how to implement this, if there are more uses or no, .... that is clearly open to discussion. I think that this enter fully in the politics vs mechanism discussions, events allow you to notify when things happen, and management app can do anything that it sees fit.

Quoted 8 lines

As principle, I think that "important happenings" (to not repeat the "event" word) should be published in a very clear way. Migration start/end are a basic example of that. It is not as if Migration is going to stop having a "start" or an "end" any time soon. Making the app polling to know that is too cumbersome for the "normal" good case. This kind of things should be plublished "somehow". The same that happens when a machine start/stops. That are improntant events.

Quoted 1 line

What makes migration important and not savevm?

That is the reason why I insist to have the events "both" in source and destination. About how to integrate savevm on the whole picture ....

VM_SAVE_START/VM_SAVE_END/VM_RESTORE_START/VM_RESTORE_END events?

Quoted 5 lines

It's not that I don't agree that migration is important and that it's important for tools to be able to know about it. I disagree that migration is *more* important than most of the other things that happen in the monitor and I want to make sure that we come up with a solution that solves the broader problem.

Agreed. That was also the reason why I told that the "write" commands are "more interesting" in this regard.

But now (at least in my point of view), we are moving in the right direction. From "we can get this with polling + other workarounds" to "this mechininsm could be useful for other things".

Later, Juan.

#36 lore raw
Quoted 39 lines · Juan Quintela
Quoted 13 lines · Anthony Liguori
Quoted 10 lines · Juan Quintela

Ok. lets stop here. My definitions:

Event: this important thing happened (important has several meanings).

Migration events fully enter in this definition. Furthermore, migration events happens from actions that are issued in machine A and event happens in machine A and machine B. (I.e. they are so special as they can get).

Quoted 5 lines

I think you've got too narrow a view. Migration doesn't always involve two machines. Migration can involve just the source writing via "exec:dd of=foo.img" and this is in fact an important use case for libvirt.

In this case, I also want to know when migration ended.

Quoted 16 lines
Quoted 16 lines

Now convenience. I "think" it would be convenient to also know in the other monitors when any "write" command happens. About how to implement this, if there are more uses or no, .... that is clearly open to discussion. I think that this enter fully in the politics vs mechanism discussions, events allow you to notify when things happen, and management app can do anything that it sees fit.

As principle, I think that "important happenings" (to not repeat the "event" word) should be published in a very clear way. Migration start/end are a basic example of that. It is not as if Migration is going to stop having a "start" or an "end" any time soon. Making the app polling to know that is too cumbersome for the "normal" good case. This kind of things should be plublished "somehow". The same that happens when a machine start/stops. That are improntant events.

Quoted 7 lines
Quoted 2 lines

What makes migration important and not savevm?

That is the reason why I insist to have the events "both" in source and destination. About how to integrate savevm on the whole picture ....

VM_SAVE_START/VM_SAVE_END/VM_RESTORE_START/VM_RESTORE_END events?

If savevm is an asychronous command, then it's already there.

You really want to support turning all command submissions/completions into events. You could do it with two events. The first would be COMMAND_REQUEST and would contain the request data and which monitor it occurred on. The second would be COMMAND_RESPONSE and would contain the response data and which monitor it occurred on.

But honestly, I think it's a stretch to say this functionality is really needed.

Quoted 16 lines
Quoted 6 lines

It's not that I don't agree that migration is important and that it's important for tools to be able to know about it. I disagree that migration is *more* important than most of the other things that happen in the monitor and I want to make sure that we come up with a solution that solves the broader problem.

Agreed. That was also the reason why I told that the "write" commands are "more interesting" in this regard.

But now (at least in my point of view), we are moving in the right direction. From "we can get this with polling + other workarounds" to "this mechininsm could be useful for other things".

Later, Juan.

#37 lore raw
Quoted 13 lines · Anthony Liguori
Quoted 9 lines · Juan Quintela
Quoted 2 lines · Anthony Liguori

What makes migration important and not savevm?

That is the reason why I insist to have the events "both" in source and destination. About how to integrate savevm on the whole picture ....

VM_SAVE_START/VM_SAVE_END/VM_RESTORE_START/VM_RESTORE_END events?

If savevm is an asychronous command, then it's already there.

Quoted 8 lines

You really want to support turning all command submissions/completions
into events. You could do it with two events. The first would be
COMMAND_REQUEST and would contain the request data and which monitor it occurred on. The second would be COMMAND_RESPONSE and would contain the response data and which monitor it occurred on.

But honestly, I think it's a stretch to say this functionality is really needed.

As already told, what I need is the migration ones.

The imporant case is MIGRATION_ENDED on target when migration were sucessful. This is the fast path, and it makes a difference here.

MIGRATION_STARTED on target is also quite "nice" to have. At this point libvirt has an

sleep(250ms): echo "cont"

Due to a race here in incoming migration.

As we only wanted one ending event, can agree on:

MIGRATION_STARTED(both source and target)
MIGRATION_DONE(result) (both source and target)

where result can be ok or -1 (at this point we don't have anything else to put there).

That moves us from 4 events to 2?

Later, Juan.

#38 lore raw
Quoted 20 lines · On Tue, 15 Jun 2010 12:30:57 +0200 Juan Quintela <[email protected]> wrote:
Quoted 19 lines · Anthony Liguori
Quoted 9 lines · Juan Quintela
Quoted 2 lines · Anthony Liguori

What makes migration important and not savevm?

That is the reason why I insist to have the events "both" in source and destination. About how to integrate savevm on the whole picture ....

VM_SAVE_START/VM_SAVE_END/VM_RESTORE_START/VM_RESTORE_END events?

If savevm is an asychronous command, then it's already there.

You really want to support turning all command submissions/completions
into events. You could do it with two events. The first would be
COMMAND_REQUEST and would contain the request data and which monitor it occurred on. The second would be COMMAND_RESPONSE and would contain the response data and which monitor it occurred on.

Quoted 7 lines
Quoted 2 lines

But honestly, I think it's a stretch to say this functionality is really needed.

As already told, what I need is the migration ones.

The imporant case is MIGRATION_ENDED on target when migration were sucessful. This is the fast path, and it makes a difference here.

I think we could avoid this one too, but as it has a clear feature for
0.13, I'm not too opposed either.

Quoted 6 lines

MIGRATION_STARTED on target is also quite "nice" to have. At this point libvirt has an

sleep(250ms): echo "cont"

Due to a race here in incoming migration.

Hm. Did you investigate that race in detail? I hope we're not using events
to hide bugs.

Quoted 9 lines

As we only wanted one ending event, can agree on:

MIGRATION_STARTED(both source and target)
MIGRATION_DONE(result) (both source and target)

where result can be ok or -1 (at this point we don't have anything else to put there).

That moves us from 4 events to 2?

I still don't see the need for MIGRATION_STARTED, it could be useful in
the target but I'd like to understand the use case in more detail.

#39 lore raw
Quoted 37 lines · Luiz Capitulino
Quoted 20 lines · On Tue, 15 Jun 2010 12:30:57 +0200 Juan Quintela <[email protected]> wrote:
Quoted 19 lines · Anthony Liguori
Quoted 9 lines · Juan Quintela
Quoted 2 lines · Anthony Liguori

What makes migration important and not savevm?

That is the reason why I insist to have the events "both" in source and destination. About how to integrate savevm on the whole picture ....

VM_SAVE_START/VM_SAVE_END/VM_RESTORE_START/VM_RESTORE_END events?

If savevm is an asychronous command, then it's already there.

You really want to support turning all command submissions/completions
into events. You could do it with two events. The first would be
COMMAND_REQUEST and would contain the request data and which monitor it occurred on. The second would be COMMAND_RESPONSE and would contain the response data and which monitor it occurred on.

Quoted 7 lines
Quoted 2 lines

But honestly, I think it's a stretch to say this functionality is really needed.

As already told, what I need is the migration ones.

The imporant case is MIGRATION_ENDED on target when migration were sucessful. This is the fast path, and it makes a difference here.

I think we could avoid this one too, but as it has a clear feature for
0.13, I'm not too opposed either.

Quoted 3 lines

MIGRATION_STARTED on target is also quite "nice" to have. At this point libvirt has an

Quoted 6 lines
Quoted 3 lines

sleep(250ms): echo "cont"

Due to a race here in incoming migration.

Hm. Did you investigate that race in detail? I hope we're not using events
to hide bugs.

Yes. we can call "cont" while staying in "waiting for incomming migration" because we dont' have "waiting incoming migraiton" than vm_start/stop understand.

Reviewing all callers to see that I can add a new state.

Quoted 5 lines
Quoted 5 lines

As we only wanted one ending event, can agree on:

MIGRATION_STARTED(both source and target)
MIGRATION_DONE(result) (both source and target)

Quoted 7 lines
Quoted 4 lines

where result can be ok or -1 (at this point we don't have anything else to put there).

That moves us from 4 events to 2?

I still don't see the need for MIGRATION_STARTED, it could be useful in
the target but I'd like to understand the use case in more detail.

At this point, if you are doing migration with tcp, and you are putting the wrong port on source (no path or any other error), you get no info at all of what is happening.

It is important to be sure than migration has started, i.e. something happenend. It happens to me all the time, and users with more complex setup will like it to know what is happening.

As a workaround, we can try something like "info status" or ony read only version to know that migration is not happening (because) monitor is working (as I tell an workaround).

Later, Juan.

#40 lore raw
Quoted 42 lines · On Tue, 15 Jun 2010 17:24:59 +0200 Juan Quintela <[email protected]> wrote:
Quoted 41 lines · Luiz Capitulino
Quoted 20 lines · On Tue, 15 Jun 2010 12:30:57 +0200 Juan Quintela <[email protected]> wrote:
Quoted 19 lines · Anthony Liguori
Quoted 9 lines · Juan Quintela
Quoted 2 lines · Anthony Liguori

What makes migration important and not savevm?

That is the reason why I insist to have the events "both" in source and destination. About how to integrate savevm on the whole picture ....

VM_SAVE_START/VM_SAVE_END/VM_RESTORE_START/VM_RESTORE_END events?

If savevm is an asychronous command, then it's already there.

You really want to support turning all command submissions/completions
into events. You could do it with two events. The first would be
COMMAND_REQUEST and would contain the request data and which monitor it occurred on. The second would be COMMAND_RESPONSE and would contain the response data and which monitor it occurred on.

Quoted 7 lines
Quoted 2 lines

But honestly, I think it's a stretch to say this functionality is really needed.

As already told, what I need is the migration ones.

The imporant case is MIGRATION_ENDED on target when migration were sucessful. This is the fast path, and it makes a difference here.

I think we could avoid this one too, but as it has a clear feature for
0.13, I'm not too opposed either.

Quoted 6 lines

MIGRATION_STARTED on target is also quite "nice" to have. At this point libvirt has an

sleep(250ms): echo "cont"

Due to a race here in incoming migration.

Quoted 8 lines
Quoted 2 lines

Hm. Did you investigate that race in detail? I hope we're not using events
to hide bugs.

Yes. we can call "cont" while staying in "waiting for incomming migration" because we dont' have "waiting incoming migraiton" than vm_start/stop understand.

Reviewing all callers to see that I can add a new state.

Will skip this one because it seems to be being discussed in a different
thread.

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

As we only wanted one ending event, can agree on:

MIGRATION_STARTED(both source and target)
MIGRATION_DONE(result) (both source and target)

where result can be ok or -1 (at this point we don't have anything else to put there).

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

That moves us from 4 events to 2?

I still don't see the need for MIGRATION_STARTED, it could be useful in
the target but I'd like to understand the use case in more detail.

At this point, if you are doing migration with tcp, and you are putting the wrong port on source (no path or any other error), you get no info at all of what is happening.

Shouldn't the migrate command just the return the expected error?

Quoted 3 lines

It is important to be sure than migration has started, i.e. something happenend. It happens to me all the time, and users with more complex setup will like it to know what is happening.

It already happened to me too, but I feel that the event is being used
as a workaround: we should return good error information instead, like this:

(qemu) migrate tcp:foobar:444
migrate: Can't locate host 'foobar'

(qemu) migrate tcp:doriath:1
migrate: Host 'doriath' is not listening for migration in port 1

(qemu) migrate 'exec:asd'
migrate: Can't execute 'asd'

In QMP the client does get an event, it's the completion response of the
async command, with the error information.

Quoted 6 lines

As a workaround, we can try something like "info status" or ony read only version to know that migration is not happening (because) monitor is working (as I tell an workaround).

Later, Juan.

#41 lore raw
Quoted 4 lines · Luiz Capitulino
Quoted 1 line · On Tue, 15 Jun 2010 17:24:59 +0200 Juan Quintela <[email protected]> wrote:
Quoted 1 line
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

I still don't see the need for MIGRATION_STARTED, it could be useful in
the target but I'd like to understand the use case in more detail.

At this point, if you are doing migration with tcp, and you are putting the wrong port on source (no path or any other error), you get no info at all of what is happening.

Shouldn't the migrate command just the return the expected error?

No. Think you are "having troubles". You try to find what happens. launch things by hand. And there is no way to know if anybody has conected to the destination machine. Some notification that migration has started is _very_ useful. expecially when there are networks/firewalls/... in the middle.

Quoted 6 lines
Quoted 3 lines

It is important to be sure than migration has started, i.e. something happenend. It happens to me all the time, and users with more complex setup will like it to know what is happening.

It already happened to me too, but I feel that the event is being used
as a workaround: we should return good error information instead, like this:

You want the notification on the destination side. When I am debugging a problem with migration, the first thing that I do is putting a printf there, because it is very useful to know that this has happened.

Quoted 5 lines

(qemu) migrate tcp:foobar:444
migrate: Can't locate host 'foobar'

(qemu) migrate tcp:doriath:1
migrate: Host 'doriath' is not listening for migration in port 1

You like to wait for timeouts O:-)

Quoted 5 lines

(qemu) migrate 'exec:asd'
migrate: Can't execute 'asd'

In QMP the client does get an event, it's the completion response of the
async command, with the error information.

Ok. I will give one example.

You want to move from a home in Town A to a home in Town B. You are a very rich man, and you have lots of things. The company that is doing the move only have a lorry and have to do several trips.

You: I am going to be on the garden of the new house, could you ring the bell when you arrive? (a.k.a. MigrationStart).

Lorry company: No sir, you have to be waiting on the door until we arrive.

You: And when you are going to arrive?

Them: We don't know.

You: Could you please, pretty please, ring the bell when you arrive so I can be on the Garden.

Them: No. You have to wait in the door.

Note: you can fix the problem, just that you con't do anything else.

But it gets better when the end. remember that you are a very rich man, and you don't remember how many things you have. Then you ask again:

You: could you told me when you end to close the house dor.

Them: Yes, no problem. We would just told you when last lorry leaves old house.

You: But I am in the new house!!!! Couldn't you told me when you have finished "in the new house".

Them: No, we already told you in the old house. That is how we have worked forever.

You: But, you know when you ended here. You know when you have finished. Why don't you told me that you have finished (MIGRATION_ENDED).

Them: because we have already told you in the old house. You can go to the old house and look if there is anything else there. If there is nothing, that means that we have finished.

If you really, really think that this is good service. This is what we are giving to management folks. And they are asking for "so complicated" things as:
- ring a bell when migration starts: one event
- ring a bell when migration ends: another event

That is it. But you continue telling that going to the old house and doing a info migrate is a good interface.

To add insult to injury, the problem is that libvirt people are not collaborative, and expect things that can't be done, are uncooperative,
....

Libvirt folks "also" do lots of things wrong, they are not perfect. But it in this case, who is being completely unreasonable is qemu land.

Later, Juan.

#42 lore raw
Quoted 19 lines · On Wed, 16 Jun 2010 21:10:04 +0200 Juan Quintela <[email protected]> wrote:
Quoted 4 lines · Luiz Capitulino
Quoted 1 line · On Tue, 15 Jun 2010 17:24:59 +0200 Juan Quintela <[email protected]> wrote:
Quoted 1 line
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

I still don't see the need for MIGRATION_STARTED, it could be useful in
the target but I'd like to understand the use case in more detail.

At this point, if you are doing migration with tcp, and you are putting the wrong port on source (no path or any other error), you get no info at all of what is happening.

Shouldn't the migrate command just the return the expected error?

No. Think you are "having troubles". You try to find what happens. launch things by hand. And there is no way to know if anybody has conected to the destination machine. Some notification that migration has started is _very_ useful. expecially when there are networks/firewalls/... in the middle.

[...]

Quoted 2 lines

That is it. But you continue telling that going to the old house and doing a info migrate is a good interface.

I'm sorry? When did I ever claimed such a thing?

First point: all you describe is MIGRATION_CONNECTED, at the end of the day
it would do exactly what you want for MIGRATION_STARTED.

The second, and most important point, is that we're trying not to make
things worse. Adding a number of events to circumvent a bad designed command and having the wrong expectations (ie. help developer debugging) is a clear recipe for disaster.

Anyway, I think it doesn't matter anymore, as QMP is not going to be declared
stable for 0.13. In this case we'll have enough time to design the proper interface.

Quoted 2 lines

To add insult to injury, the problem is that libvirt people are not collaborative, and expect things that can't be done, are uncooperative,

Again, I've never claimed that and I think you're taking this thread to
the wrong direction.

Quoted 7 lines

....

Libvirt folks "also" do lots of things wrong, they are not perfect. But it in this case, who is being completely unreasonable is qemu land.

Later, Juan.

#43 lore raw
Quoted 23 lines · Luiz Capitulino
Quoted 12 lines · On Wed, 16 Jun 2010 21:10:04 +0200 Juan Quintela <[email protected]> wrote:
Quoted 11 lines · Luiz Capitulino
Quoted 7 lines · On Tue, 15 Jun 2010 17:24:59 +0200 Juan Quintela <[email protected]> wrote:
Quoted 3 lines

I still don't see the need for MIGRATION_STARTED, it could be useful in
the target but I'd like to understand the use case in more detail.

At this point, if you are doing migration with tcp, and you are putting the wrong port on source (no path or any other error), you get no info at all of what is happening.

Quoted 7 lines
Quoted 1 line

Shouldn't the migrate command just the return the expected error?

No. Think you are "having troubles". You try to find what happens. launch things by hand. And there is no way to know if anybody has conected to the destination machine. Some notification that migration has started is _very_ useful. expecially when there are networks/firewalls/... in the middle.

Quoted 6 lines

[...]

Quoted 2 lines

That is it. But you continue telling that going to the old house and doing a info migrate is a good interface.

I'm sorry? When did I ever claimed such a thing?

polling is enough. polling has to be done in source machine.

Quoted 12 lines

First point: all you describe is MIGRATION_CONNECTED, at the end of the day
it would do exactly what you want for MIGRATION_STARTED.

The second, and most important point, is that we're trying not to make
things worse. Adding a number of events to circumvent a bad designed command and having the wrong expectations (ie. help developer debugging) is a clear recipe for disaster.

Anyway, I think it doesn't matter anymore, as QMP is not going to be declared
stable for 0.13. In this case we'll have enough time to design the proper interface.

Quoted 5 lines
Quoted 2 lines

To add insult to injury, the problem is that libvirt people are not collaborative, and expect things that can't be done, are uncooperative,

Again, I've never claimed that and I think you're taking this thread to
the wrong direction.

Ok, I stop then.

Later, Juan.

#44 lore raw
Quoted 24 lines · On Thu, 17 Jun 2010 18:34:00 +0200 Juan Quintela <[email protected]> wrote:
Quoted 23 lines · Luiz Capitulino
Quoted 19 lines · On Wed, 16 Jun 2010 21:10:04 +0200 Juan Quintela <[email protected]> wrote:
Quoted 4 lines · Luiz Capitulino
Quoted 1 line · On Tue, 15 Jun 2010 17:24:59 +0200 Juan Quintela <[email protected]> wrote:
Quoted 1 line
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

I still don't see the need for MIGRATION_STARTED, it could be useful in
the target but I'd like to understand the use case in more detail.

At this point, if you are doing migration with tcp, and you are putting the wrong port on source (no path or any other error), you get no info at all of what is happening.

Shouldn't the migrate command just the return the expected error?

No. Think you are "having troubles". You try to find what happens. launch things by hand. And there is no way to know if anybody has conected to the destination machine. Some notification that migration has started is _very_ useful. expecially when there are networks/firewalls/... in the middle.

Quoted 8 lines
Quoted 6 lines

[...]

Quoted 2 lines

That is it. But you continue telling that going to the old house and doing a info migrate is a good interface.

I'm sorry? When did I ever claimed such a thing?

polling is enough. polling has to be done in source machine.

Enough for the meantime, until we have something better to offer. The problem
here is that adding not so good stuff to a protocol is that we will have to maintain it for a quite long time, possibly forever.

That's why I'm being so opposed to a large set of events, a reduced set is a lot
more attractive.

Quoted 12 lines
Quoted 12 lines

First point: all you describe is MIGRATION_CONNECTED, at the end of the day
it would do exactly what you want for MIGRATION_STARTED.

The second, and most important point, is that we're trying not to make
things worse. Adding a number of events to circumvent a bad designed command and having the wrong expectations (ie. help developer debugging) is a clear recipe for disaster.

Anyway, I think it doesn't matter anymore, as QMP is not going to be declared
stable for 0.13. In this case we'll have enough time to design the proper interface.

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

To add insult to injury, the problem is that libvirt people are not collaborative, and expect things that can't be done, are uncooperative,

Again, I've never claimed that and I think you're taking this thread to
the wrong direction.

Ok, I stop then.

I'm not asking you to stop arguing, just to avoid taking the non-technical
route in a bad way.

Now, we have the following situation: MIGRATION_CONNECTED and MIGRATION_DONE
would have possibly been a good fit for 0.13 if QMP was going to be stable.

However, that's not going to happen so the question is: is it interesting
to have those events for an unstable QMP? Do we expect any client to need it? Or can we wait until 0.14?

#45 lore raw
Quoted 73 lines · Luiz Capitulino
Quoted 30 lines · On Thu, 17 Jun 2010 18:34:00 +0200 Juan Quintela<[email protected]> wrote:
Quoted 28 lines · Luiz Capitulino
Quoted 21 lines · On Wed, 16 Jun 2010 21:10:04 +0200 Juan Quintela<[email protected]> wrote:
Quoted 4 lines · Luiz Capitulino

On Tue, 15 Jun 2010 17:24:59 +0200
Juan Quintela<[email protected]> wrote:

Quoted 9 lines
Quoted 7 lines
Quoted 3 lines

I still don't see the need for MIGRATION_STARTED, it could be useful in
the target but I'd like to understand the use case in more detail.

At this point, if you are doing migration with tcp, and you are putting the wrong port on source (no path or any other error), you get no info at all of what is happening.

Shouldn't the migrate command just the return the expected error?

No. Think you are "having troubles". You try to find what happens. launch things by hand. And there is no way to know if anybody has conected to the destination machine. Some notification that migration has started is _very_ useful. expecially when there are networks/firewalls/... in the middle.

[...]

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

That is it. But you continue telling that going to the old house and doing a info migrate is a good interface.

I'm sorry? When did I ever claimed such a thing?

polling is enough. polling has to be done in source machine.

Enough for the meantime, until we have something better to offer. The problem
here is that adding not so good stuff to a protocol is that we will have to maintain it for a quite long time, possibly forever.

That's why I'm being so opposed to a large set of events, a reduced set is a lot
more attractive.

Quoted 13 lines
Quoted 13 lines

First point: all you describe is MIGRATION_CONNECTED, at the end of the day
it would do exactly what you want for MIGRATION_STARTED.

The second, and most important point, is that we're trying not to make
things worse. Adding a number of events to circumvent a bad designed command and having the wrong expectations (ie. help developer debugging) is a clear recipe for disaster.

Anyway, I think it doesn't matter anymore, as QMP is not going to be declared
stable for 0.13. In this case we'll have enough time to design the proper interface.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

To add insult to injury, the problem is that libvirt people are not collaborative, and expect things that can't be done, are uncooperative,

Again, I've never claimed that and I think you're taking this thread to
the wrong direction.

Ok, I stop then.

I'm not asking you to stop arguing, just to avoid taking the non-technical
route in a bad way.

Quoted 7 lines

Now, we have the following situation: MIGRATION_CONNECTED and MIGRATION_DONE
would have possibly been a good fit for 0.13 if QMP was going to be stable.

However, that's not going to happen so the question is: is it interesting
to have those events for an unstable QMP? Do we expect any client to need it? Or can we wait until 0.14?

We need MIGRATION_CONNECTED post 0.13. We won't need MIGRATION_DONE so there's probably no point in introducing it.

Regards,

Anthony Liguori