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

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

[Juan Quintela]

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(-)

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

[Juan Quintela]

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

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

 static MigrationState *current_migration;

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

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

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

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

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

diff --git a/vl.c b/vl.c
index 7121cd0..c35b46e 100644
--- a/vl.c
+++ b/vl.c
@@ -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();
     }
-- 
1.6.6.1

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

[Anthony Liguori]

On 06/09/2010 07:10 AM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela<quintela@redhat.com>
>    

Applied 1&2 as we discussed.  Thanks.

Regards,

Anthony Liguori
> ---
>   migration.c |   16 ++++++++++------
>   migration.h |    2 +-
>   vl.c        |    7 ++++++-
>   3 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index fbf2339..ecc67f1 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -36,22 +36,26 @@ static uint32_t max_throttle = (32<<  20);
>
>   static MigrationState *current_migration;
>
> -void qemu_start_incoming_migration(const char *uri)
> +int qemu_start_incoming_migration(const char *uri)
>   {
>       const char *p;
> +    int ret;
>
>       if (strstart(uri, "tcp:",&p))
> -        tcp_start_incoming_migration(p);
> +        ret = tcp_start_incoming_migration(p);
>   #if !defined(WIN32)
>       else if (strstart(uri, "exec:",&p))
> -        exec_start_incoming_migration(p);
> +        ret =  exec_start_incoming_migration(p);
>       else if (strstart(uri, "unix:",&p))
> -        unix_start_incoming_migration(p);
> +        ret = unix_start_incoming_migration(p);
>       else if (strstart(uri, "fd:",&p))
> -        fd_start_incoming_migration(p);
> +        ret = fd_start_incoming_migration(p);
>   #endif
> -    else
> +    else {
>           fprintf(stderr, "unknown migration protocol: %s\n", uri);
> +        ret = -EPROTONOSUPPORT;
> +    }
> +    return ret;
>   }
>
>   int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)
> diff --git a/migration.h b/migration.h
> index 97eef4a..e048bb2 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -50,7 +50,7 @@ struct FdMigrationState
>       void *opaque;
>   };
>
> -void qemu_start_incoming_migration(const char *uri);
> +int qemu_start_incoming_migration(const char *uri);
>
>   int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data);
>
> diff --git a/vl.c b/vl.c
> index 7121cd0..c35b46e 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -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();
>       }
>    

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

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

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 qemu-os-win32.h |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/qemu-os-win32.h b/qemu-os-win32.h
index 6323f7f..2ff9f45 100644
--- a/qemu-os-win32.h
+++ b/qemu-os-win32.h
@@ -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

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

[Blue Swirl]

Thanks, applied.

On Thu, Jun 24, 2010 at 8:41 PM, Stefan Weil <weil@mail.berlios.de> wrote:
> 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.
>
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
> ---
>  qemu-os-win32.h |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/qemu-os-win32.h b/qemu-os-win32.h
> index 6323f7f..2ff9f45 100644
> --- a/qemu-os-win32.h
> +++ b/qemu-os-win32.h
> @@ -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
>
>
>

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

[Juan Quintela]

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

diff --git a/migration-exec.c b/migration-exec.c
index a8813b4..14718dd 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -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);
 }
diff --git a/migration-fd.c b/migration-fd.c
index 0abd372..6d14505 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -104,20 +104,8 @@ err_after_alloc:
 static void fd_accept_incoming_migration(void *opaque)
 {
     QEMUFile *f = opaque;
-    int ret;

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

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

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

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

-    ret = qemu_loadvm_state(f);
-    if (ret < 0) {
-        fprintf(stderr, "load of migration failed\n");
-        goto out_fopen;
-    }
-    qemu_announce_self();
-    DPRINTF("successfully loaded vm state\n");
-
-    if (autostart)
-        vm_start();
-
-out_fopen:
+    process_incoming_migration(f);
     qemu_fclose(f);
 out:
     qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
diff --git a/migration.c b/migration.c
index ecc67f1..4ce19ff 100644
--- a/migration.c
+++ b/migration.c
@@ -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;
diff --git a/migration.h b/migration.h
index e048bb2..d13ed4f 100644
--- a/migration.h
+++ b/migration.h
@@ -50,6 +50,8 @@ struct FdMigrationState
     void *opaque;
 };

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

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

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

[Juan Quintela]

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

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

diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
index 01ec85f..cafc4e6 100644
--- a/QMP/qmp-events.txt
+++ b/QMP/qmp-events.txt
@@ -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
 -----

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

-- 
1.6.6.1

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

[Luiz Capitulino]

On Wed,  9 Jun 2010 14:10:56 +0200
Juan Quintela <quintela@redhat.com> wrote:

> They are emitted when migration starts, ends, has a failure or is canceled.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  QMP/qmp-events.txt |   52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  monitor.c          |   12 ++++++++++++
>  monitor.h          |    4 ++++
>  3 files changed, 68 insertions(+), 0 deletions(-)
> 
> diff --git a/QMP/qmp-events.txt b/QMP/qmp-events.txt
> index 01ec85f..cafc4e6 100644
> --- a/QMP/qmp-events.txt
> +++ b/QMP/qmp-events.txt
> @@ -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.

 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.

> +
> +Data: None
> +
> +Example:
> +
> +{ "event": "MIGRATION_FAILED",
> +    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
> +
> +MIGRATION_STARTED
> +-----------------
> +
> +Emitted when migration starts (both in source and target).
> +
> +Data: None
> +
> +Example:
> +
> +{ "event": "MIGRATION_STARTED",
> +    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
> +
>  RESET
>  -----
> 
> diff --git a/monitor.c b/monitor.c
> index 15b53b9..a5c5388 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -444,6 +444,18 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
>          case QEVENT_WATCHDOG:
>              event_name = "WATCHDOG";
>              break;
> +        case QEVENT_MIGRATION_STARTED:
> +            event_name = "MIGRATION_STARTED";
> +            break;
> +        case QEVENT_MIGRATION_ENDED:
> +            event_name = "MIGRATION_ENDED";
> +            break;
> +        case QEVENT_MIGRATION_FAILED:
> +            event_name = "MIGRATION_FAILED";
> +            break;
> +        case QEVENT_MIGRATION_CANCELED:
> +            event_name = "MIGRATION_CANCELED";
> +            break;
>          default:
>              abort();
>              break;
> diff --git a/monitor.h b/monitor.h
> index ea15469..34bcd38 100644
> --- a/monitor.h
> +++ b/monitor.h
> @@ -28,6 +28,10 @@ typedef enum MonitorEvent {
>      QEVENT_BLOCK_IO_ERROR,
>      QEVENT_RTC_CHANGE,
>      QEVENT_WATCHDOG,
> +    QEVENT_MIGRATION_STARTED,
> +    QEVENT_MIGRATION_ENDED,
> +    QEVENT_MIGRATION_FAILED,
> +    QEVENT_MIGRATION_CANCELED,
>      QEVENT_MAX,
>  } MonitorEvent;
> 

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

[Juan Quintela]

Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Wed,  9 Jun 2010 14:10:56 +0200
> Juan Quintela <quintela@redhat.com> wrote:

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

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

[Luiz Capitulino]

On Thu, 10 Jun 2010 12:33:42 +0200
Juan Quintela <quintela@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> wrote:
> > On Wed,  9 Jun 2010 14:10:56 +0200
> > Juan Quintela <quintela@redhat.com> wrote:
> 
> >> +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?

 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.

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

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

[Juan Quintela]

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

diff --git a/migration.c b/migration.c
index 4ce19ff..2a44b46 100644
--- a/migration.c
+++ b/migration.c
@@ -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");

-- 
1.6.6.1

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

[Juan Quintela]

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

diff --git a/migration-exec.c b/migration-exec.c
index 14718dd..9052cd2 100644
--- a/migration-exec.c
+++ b/migration-exec.c
@@ -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);
     }
diff --git a/migration-fd.c b/migration-fd.c
index 6d14505..9c4c7ae 100644
--- a/migration-fd.c
+++ b/migration-fd.c
@@ -83,6 +83,7 @@ MigrationState *fd_start_outgoing_migration(Monitor *mon,
     s->mig_state.blk = blk;
     s->mig_state.shared = inc;

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

 //#define DEBUG_MIGRATION_TCP

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

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

 //#define DEBUG_MIGRATION_UNIX

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

+    monitor_protocol_event(QEVENT_MIGRATION_STARTED, NULL);
     s->state = MIG_STATE_ACTIVE;
     s->mon = NULL;
     s->bandwidth_limit = bandwidth_limit;
diff --git a/migration.c b/migration.c
index 2a44b46..256a679 100644
--- a/migration.c
+++ b/migration.c
@@ -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);
     }
-- 
1.6.6.1

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

[Yoshiaki Tamura]

Hi Juan,

2010/6/9 Juan Quintela <quintela@redhat.com>:
> 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.

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

> 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(-)
>
>
>

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

[Juan Quintela]

Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> wrote:
> 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.

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

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

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

[Yoshiaki Tamura]

2010/6/10 Juan Quintela <quintela@redhat.com>:
> Yoshiaki Tamura <tamura.yoshiaki@lab.ntt.co.jp> wrote:
>> 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.
>
>> 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.
>
>> 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.

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

>
> Later, Juan.
>
>

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

[Luiz Capitulino]

On Wed,  9 Jun 2010 14:10:53 +0200
Juan Quintela <quintela@redhat.com> 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?

> 
> 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(-)
> 
> 

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

[Yoshiaki Tamura]

2010/6/10 Luiz Capitulino <lcapitulino@redhat.com>:
> On Wed,  9 Jun 2010 14:10:53 +0200
> Juan Quintela <quintela@redhat.com> 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?

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

>
>>
>> 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(-)
>>
>>
>
>
>

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

[Juan Quintela]

Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Wed,  9 Jun 2010 14:10:53 +0200
> Juan Quintela <quintela@redhat.com> 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?
>

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.

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

[Luiz Capitulino]

On Thu, 10 Jun 2010 12:44:55 +0200
Juan Quintela <quintela@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> wrote:
> > On Wed,  9 Jun 2010 14:10:53 +0200
> > Juan Quintela <quintela@redhat.com> 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?
> >
> 
> 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.

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

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

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

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

[Anthony Liguori]

On 06/11/2010 09:30 AM, Luiz Capitulino wrote:
> On Thu, 10 Jun 2010 12:44:55 +0200
> Juan Quintela<quintela@redhat.com>  wrote:
>
>    
>> Luiz Capitulino<lcapitulino@redhat.com>  wrote:
>>      
>>> On Wed,  9 Jun 2010 14:10:53 +0200
>>> Juan Quintela<quintela@redhat.com>  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?
>>>
>>>        
>> 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" }
>    

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

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

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

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

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

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

MIGRATION_DONE gets deprecated for 0.14.

Regards,

Anthony Liguori

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

[Luiz Capitulino]

On Fri, 11 Jun 2010 09:38:42 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

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

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

> >> - 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?
> >    
> 
> Yes.  Today, we should just generate a MIGRATION_DONE event and let a 
> client poll for failure status.

[...]

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

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

[Juan Quintela]

Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Fri, 11 Jun 2010 09:38:42 -0500
> Anthony Liguori <anthony@codemonkey.ws> wrote:
>
>> >   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.
>
>> >> - 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?

Both.

>> >> - 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?
>> >    
>> 
>> Yes.  Today, we should just generate a MIGRATION_DONE event and let a 
>> client poll for failure status.
>
> [...]
>
>> 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.

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

[Luiz Capitulino]

On Sat, 12 Jun 2010 13:20:54 +0200
Juan Quintela <quintela@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> wrote:
> > On Fri, 11 Jun 2010 09:38:42 -0500
> > Anthony Liguori <anthony@codemonkey.ws> wrote:
> >
> >> >   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.
> >
> >> >> - 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?
> 
> Both.

 What does it report in the source?

> >> >> - 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?
> >> >    
> >> 
> >> Yes.  Today, we should just generate a MIGRATION_DONE event and let a 
> >> client poll for failure status.
> >
> > [...]
> >
> >> 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, .....

 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.

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

[Juan Quintela]

Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Sat, 12 Jun 2010 13:20:54 +0200
> Juan Quintela <quintela@redhat.com> wrote:

>> Both.
>
>  What does it report in the source?

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

Think again multiple monitors and/or audit.

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

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.

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

[Juan Quintela]

Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 06/11/2010 09:30 AM, Luiz Capitulino wrote:
>> On Thu, 10 Jun 2010 12:44:55 +0200
>> Juan Quintela<quintela@redhat.com>  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.

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

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

[Anthony Liguori]

On 06/12/2010 06:14 AM, Juan Quintela wrote:
> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>    
>> On 06/11/2010 09:30 AM, Luiz Capitulino wrote:
>>      
>>> On Thu, 10 Jun 2010 12:44:55 +0200
>>> Juan Quintela<quintela@redhat.com>   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.

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

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

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

> Later, Juan.
>    

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

[Luiz Capitulino]

On Mon, 14 Jun 2010 08:58:19 -0500
Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:

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

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

[Anthony Liguori]

On 06/14/2010 09:24 AM, Luiz Capitulino wrote:
> On Mon, 14 Jun 2010 08:58:19 -0500
> Anthony Liguori<aliguori@linux.vnet.ibm.com>  wrote:
>
>    
>> 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.
>    

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

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

[Luiz Capitulino]

On Mon, 14 Jun 2010 09:35:52 -0500
Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:

> On 06/14/2010 09:24 AM, Luiz Capitulino wrote:
> > On Mon, 14 Jun 2010 08:58:19 -0500
> > Anthony Liguori<aliguori@linux.vnet.ibm.com>  wrote:
> >
> >    
> >> 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.
> >    
> 
> 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.

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

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

[Juan Quintela]

Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Thu, 10 Jun 2010 12:44:55 +0200
> Juan Quintela <quintela@redhat.com> wrote:
>
>> Luiz Capitulino <lcapitulino@redhat.com> wrote:
>> > On Wed,  9 Jun 2010 14:10:53 +0200
>> > Juan Quintela <quintela@redhat.com> 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?
>> >
>> 
>> 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.

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.


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

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

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.

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

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

Later, Juan.

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

[Anthony Liguori]

On 06/12/2010 06:05 AM, Juan Quintela wrote:
> Luiz Capitulino<lcapitulino@redhat.com>  wrote:
>    
>> On Thu, 10 Jun 2010 12:44:55 +0200
>> Juan Quintela<quintela@redhat.com>  wrote:
>>
>>      
>>> Luiz Capitulino<lcapitulino@redhat.com>  wrote:
>>>        
>>>> On Wed,  9 Jun 2010 14:10:53 +0200
>>>> Juan Quintela<quintela@redhat.com>  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?
>>>>
>>>>          
>>> 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.
>>      
> 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.
>
>
>    
>>> - 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.

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

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

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

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

[Juan Quintela]

Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:
> On 06/12/2010 06:05 AM, Juan Quintela wrote:
>> Luiz Capitulino<lcapitulino@redhat.com>  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.

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

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

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

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

[Anthony Liguori]

On 06/14/2010 11:02 AM, Juan Quintela wrote:
> Anthony Liguori<aliguori@linux.vnet.ibm.com>  wrote:
>    
>> On 06/12/2010 06:05 AM, Juan Quintela wrote:
>>      
>>> Luiz Capitulino<lcapitulino@redhat.com>   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.
>    

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

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

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

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

> Later, Juan.
>    

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

[Juan Quintela]

Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:
> On 06/14/2010 11:02 AM, Juan Quintela wrote:
>> Anthony Liguori<aliguori@linux.vnet.ibm.com>  wrote:
>>    
>>> On 06/12/2010 06:05 AM, Juan Quintela wrote:
>>>      
>>>> Luiz Capitulino<lcapitulino@redhat.com>   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.
>>    
>
> 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.

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

Fully agree.

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

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

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

[Anthony Liguori]

On 06/14/2010 01:35 PM, Juan Quintela wrote:
> Anthony Liguori<aliguori@linux.vnet.ibm.com>  wrote:
>    
>> On 06/14/2010 11:02 AM, Juan Quintela wrote:
>>      
>>> Anthony Liguori<aliguori@linux.vnet.ibm.com>   wrote:
>>>
>>>        
>>>> On 06/12/2010 06:05 AM, Juan Quintela wrote:
>>>>
>>>>          
>>>>> Luiz Capitulino<lcapitulino@redhat.com>    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.
>>>
>>>        
>> 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.
>
>    
>>> 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.
>>      
> Fully agree.
>
>    
>>> 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).
>    

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.

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

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

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

[Juan Quintela]

Anthony Liguori <aliguori@linux.vnet.ibm.com> wrote:
> On 06/14/2010 01:35 PM, Juan Quintela wrote:

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

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

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

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

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

[Anthony Liguori]

On 06/14/2010 02:54 PM, Juan Quintela wrote:
> Anthony Liguori<aliguori@linux.vnet.ibm.com>  wrote:
>    
>> On 06/14/2010 01:35 PM, Juan Quintela wrote:
>>      
>    
>>> 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.
>>      
> In this case, I also want to know when migration ended.
>
>    
>>> 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.
>>>
>>>        
>> 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.

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

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

[Juan Quintela]

Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 06/14/2010 02:54 PM, Juan Quintela wrote:
>> Anthony Liguori<aliguori@linux.vnet.ibm.com>  wrote:

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

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.

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

[Luiz Capitulino]

On Tue, 15 Jun 2010 12:30:57 +0200
Juan Quintela <quintela@redhat.com> wrote:

> Anthony Liguori <anthony@codemonkey.ws> wrote:
> > On 06/14/2010 02:54 PM, Juan Quintela wrote:
> >> Anthony Liguori<aliguori@linux.vnet.ibm.com>  wrote:
> 
> >>> 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.
> 
> 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.

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

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

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

[Juan Quintela]

Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Tue, 15 Jun 2010 12:30:57 +0200
> Juan Quintela <quintela@redhat.com> wrote:
>
>> Anthony Liguori <anthony@codemonkey.ws> wrote:
>> > On 06/14/2010 02:54 PM, Juan Quintela wrote:
>> >> Anthony Liguori<aliguori@linux.vnet.ibm.com>  wrote:
>> 
>> >>> 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.
>> 
>> 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.
>
>> 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.

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.

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

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.

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

[Luiz Capitulino]

On Tue, 15 Jun 2010 17:24:59 +0200
Juan Quintela <quintela@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> wrote:
> > On Tue, 15 Jun 2010 12:30:57 +0200
> > Juan Quintela <quintela@redhat.com> wrote:
> >
> >> Anthony Liguori <anthony@codemonkey.ws> wrote:
> >> > On 06/14/2010 02:54 PM, Juan Quintela wrote:
> >> >> Anthony Liguori<aliguori@linux.vnet.ibm.com>  wrote:
> >> 
> >> >>> 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.
> >> 
> >> 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.
> >
> >> 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.
> 
> 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.

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

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

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

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

[Juan Quintela]

Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Tue, 15 Jun 2010 17:24:59 +0200
> Juan Quintela <quintela@redhat.com> wrote:

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

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

> (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:-)

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

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

[Luiz Capitulino]

On Wed, 16 Jun 2010 21:10:04 +0200
Juan Quintela <quintela@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> wrote:
> > On Tue, 15 Jun 2010 17:24:59 +0200
> > Juan Quintela <quintela@redhat.com> wrote:
> 
> >> >
> >> >  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.

 [...]

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

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

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

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

[Juan Quintela]

Luiz Capitulino <lcapitulino@redhat.com> wrote:
> On Wed, 16 Jun 2010 21:10:04 +0200
> Juan Quintela <quintela@redhat.com> wrote:
>
>> Luiz Capitulino <lcapitulino@redhat.com> wrote:
>> > On Tue, 15 Jun 2010 17:24:59 +0200
>> > Juan Quintela <quintela@redhat.com> wrote:
>> 
>> >> >
>> >> >  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.
>
>  [...]
>
>> 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.

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

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

[Luiz Capitulino]

On Thu, 17 Jun 2010 18:34:00 +0200
Juan Quintela <quintela@redhat.com> wrote:

> Luiz Capitulino <lcapitulino@redhat.com> wrote:
> > On Wed, 16 Jun 2010 21:10:04 +0200
> > Juan Quintela <quintela@redhat.com> wrote:
> >
> >> Luiz Capitulino <lcapitulino@redhat.com> wrote:
> >> > On Tue, 15 Jun 2010 17:24:59 +0200
> >> > Juan Quintela <quintela@redhat.com> wrote:
> >> 
> >> >> >
> >> >> >  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.
> >
> >  [...]
> >
> >> 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.

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

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

[Anthony Liguori]

On 06/17/2010 11:45 AM, Luiz Capitulino wrote:
> On Thu, 17 Jun 2010 18:34:00 +0200
> Juan Quintela<quintela@redhat.com>  wrote:
>
>    
>> Luiz Capitulino<lcapitulino@redhat.com>  wrote:
>>      
>>> On Wed, 16 Jun 2010 21:10:04 +0200
>>> Juan Quintela<quintela@redhat.com>  wrote:
>>>
>>>        
>>>> Luiz Capitulino<lcapitulino@redhat.com>  wrote:
>>>>          
>>>>> On Tue, 15 Jun 2010 17:24:59 +0200
>>>>> Juan Quintela<quintela@redhat.com>  wrote:
>>>>>            
>>>>          
>>>>>>>   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.
>>>>          
>>>   [...]
>>>
>>>        
>>>> 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.
>
>    
>>>   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.
>>>
>>>        
>>>> 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?
>    

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