Add QMP migration events

Signed-off-by Juan Quintela <[email protected]>

---
 migration.c |   16 ++++++++++------
 migration.h |    2 +-
 vl.c        |    7 ++++++-
 3 files changed, 17 insertions(+), 8 deletions(-)

migration.c +10 -6

index 05f6cc5..9c1d4b6 100644
@@ -36,22 +36,26 @@ static uint32_t max_throttle = (32 << 20);
 
 static MigrationState *current_migration;
 
-void qemu_start_incoming_migration(const char *uri)
+int qemu_start_incoming_migration(const char *uri)
 {
     const char *p;
+    int ret;
 
     if (strstart(uri, "tcp:", &p))
-        tcp_start_incoming_migration(p);
+        ret = tcp_start_incoming_migration(p);
 #if !defined(WIN32)
     else if (strstart(uri, "exec:", &p))
-        exec_start_incoming_migration(p);
+        ret =  exec_start_incoming_migration(p);
     else if (strstart(uri, "unix:", &p))
-        unix_start_incoming_migration(p);
+        ret = unix_start_incoming_migration(p);
     else if (strstart(uri, "fd:", &p))
-        fd_start_incoming_migration(p);
+        ret = fd_start_incoming_migration(p);
 #endif
-    else
+    else {
         fprintf(stderr, "unknown migration protocol: %s\n", uri);
+        ret = -EPROTONOSUPPORT;
+    }
+    return ret;
 }
 
 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data)

migration.h +1 -1

index 385423f..dd423a1 100644
@@ -50,7 +50,7 @@ struct FdMigrationState
     void *opaque;
 };
 
-void qemu_start_incoming_migration(const char *uri);
+int qemu_start_incoming_migration(const char *uri);
 
 int do_migrate(Monitor *mon, const QDict *qdict, QObject **ret_data);
 

vl.c +0 -0

index 328395e..d13440d 100644

vl.c +1 -0

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

Quoted 7 lines · On Tue, 25 May 2010 16:21:01 +0200 Juan Quintela <[email protected]> wrote:

Signed-off-by Juan Quintela <[email protected]>

---
 migration.c |   16 ++++++++++------
 migration.h |    2 +-
 vl.c        |    7 ++++++-
 3 files changed, 17 insertions(+), 8 deletions(-)

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

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

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

Quoted 13 lines · Luiz Capitulino

Quoted 8 lines · On Tue, 25 May 2010 16:21:01 +0200 Juan Quintela<[email protected]> wrote:

Signed-off-by Juan Quintela<[email protected]>

---
  migration.c |   16 ++++++++++------
  migration.h |    2 +-
  vl.c        |    7 ++++++-
  3 files changed, 17 insertions(+), 8 deletions(-)

Quoted 6 lines

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

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

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

Quoted 4 lines

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

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

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

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

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

Later, Juan.

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

Signed-off-by Juan Quintela<[email protected]>

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

QMP/qmp-events.txt +0 -0

index 01ec85f..93caa4d 100644
@@ -26,6 +26,56 @@ Example:
  Note: If action is "stop", a STOP event will eventually follow the
  BLOCK_IO_ERROR event.

QMP/qmp-events.txt +3 -0

+Data: None
+
+Example:

QMP/qmp-events.txt +7 -0

+Data: None
+
+Example:
+
+{ "event": "MIGRATION_STARTED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
  RESET
  -----

monitor.c +12 -0

index ad50f12..5158780 100644
@@ -444,6 +444,18 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
          case QEVENT_WATCHDOG:
              event_name = "WATCHDOG";
              break;
+        case QEVENT_MIGRATION_STARTED:
+            event_name = "MIGRATION_STARTED";
+            break;
+        case QEVENT_MIGRATION_ENDED:
+            event_name = "MIGRATION_ENDED";
+            break;
+        case QEVENT_MIGRATION_FAILED:
+            event_name = "MIGRATION_FAILED";
+            break;
+        case QEVENT_MIGRATION_CANCELED:
+            event_name = "MIGRATION_CANCELED";
+            break;
          default:
              abort();
              break;

monitor.h +4 -0

index ea15469..34bcd38 100644
@@ -28,6 +28,10 @@ typedef enum MonitorEvent {
      QEVENT_BLOCK_IO_ERROR,
      QEVENT_RTC_CHANGE,
      QEVENT_WATCHDOG,
+    QEVENT_MIGRATION_STARTED,
+    QEVENT_MIGRATION_ENDED,
+    QEVENT_MIGRATION_FAILED,
+    QEVENT_MIGRATION_CANCELED,
      QEVENT_MAX,
  } MonitorEvent;
 

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

Quoted 7 lines · Juan Quintela

QMP/qmp-events.txt +6 -0

+MIGRATION_CANCELED
+------------------
+
+Emitted when migration is canceled.  This is emitted in the source.
+Target will emit MIGRATION_FAILED (no way to differentiate a FAILED
+and CANCELED migration for target).

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

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

Quoted 13 lines

QMP/qmp-events.txt +11 -0

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

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

Quoted 9 lines

QMP/qmp-events.txt +9 -0

+Data: None
+
+Example:
+
+{ "event": "MIGRATION_ENDED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_FAILED
+----------------

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

Quoted 5 lines

QMP/qmp-events.txt +4 -0

+
+Emitted when migration fails (both is source and target).
+
+Data: None

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

Quoted 5 lines

QMP/qmp-events.txt +5 -0

+Example:
+
+{ "event": "MIGRATION_FAILED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+

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

Quoted 5 lines

QMP/qmp-events.txt +4 -0

+MIGRATION_STARTED
+-----------------
+
+Emitted when migration starts (both in source and target).

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

Quoted 5 lines · Anthony Liguori

Quoted 5 lines

QMP/qmp-events.txt +5 -0

+Example:
+
+{ "event": "MIGRATION_FAILED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+

Quoted 8 lines

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

Quoted 5 lines

QMP/qmp-events.txt +4 -0

+MIGRATION_STARTED
+-----------------
+
+Emitted when migration starts (both in source and target).

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

What kind of peer information?

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

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

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

Quoted 14 lines · Anthony Liguori

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

Quoted 8 lines · Juan Quintela

QMP/qmp-events.txt +6 -0

+MIGRATION_CANCELED
+------------------
+
+Emitted when migration is canceled.  This is emitted in the source.
+Target will emit MIGRATION_FAILED (no way to differentiate a FAILED
+and CANCELED migration for target).

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

ok, then that one is ok.

Quoted 16 lines

Quoted 4 lines

QMP/qmp-events.txt +1 -0

+Data: None

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

Quoted 10 lines

QMP/qmp-events.txt +10 -0

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

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

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

Quoted 9 lines

Quoted 9 lines

QMP/qmp-events.txt +9 -0

+Data: None
+
+Example:
+
+{ "event": "MIGRATION_ENDED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_FAILED
+----------------

Quoted 9 lines

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

Quoted 6 lines

QMP/qmp-events.txt +4 -0

+
+Emitted when migration fails (both is source and target).
+
+Data: None

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

At this point, we have basically -1 :(

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

Quoted 3 lines

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

What kind of peer information?

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

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

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

Later, Juan.

Quoted 19 lines · Juan Quintela

Quoted 16 lines · Anthony Liguori

Quoted 4 lines

QMP/qmp-events.txt +1 -0

+Data: None

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

Quoted 10 lines

QMP/qmp-events.txt +10 -0

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

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

Quoted 4 lines

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

Why does it do that??

That sound like a terrible idea.

Quoted 9 lines

Quoted 3 lines

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

What kind of peer information?

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

Quoted 3 lines

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

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

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

Quoted 4 lines · Juan Quintela

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

Why does it do that??

That sound like a terrible idea.

Quoted 8 lines

Quoted 3 lines

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

At this point, we have basically -1 :(

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

We need a better solution for reporting errors via notifications.

Quoted 9 lines

Quoted 3 lines

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

What kind of peer information?

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

Quoted 4 lines

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

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

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

Quoted 8 lines · Anthony Liguori

Quoted 5 lines · Juan Quintela

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

Quoted 4 lines

Why does it do that??

That sound like a terrible idea.

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

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

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

danp?

Quoted 10 lines

Quoted 10 lines

Quoted 4 lines

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

At this point, we have basically -1 :(

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

Quoted 2 lines

We need a better solution for reporting errors via notifications.

Suggestions?

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

Quoted 10 lines

Quoted 10 lines

Quoted 4 lines

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

What kind of peer information?

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

Quoted 8 lines

Quoted 5 lines

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

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

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

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

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

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

Later, Juan.

Quoted 12 lines

Quoted 12 lines

Quoted 10 lines

Quoted 4 lines

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

At this point, we have basically -1 :(

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

We need a better solution for reporting errors via notifications.

Quoted 5 lines

Suggestions?

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

Markus/Luiz?

Quoted 4 lines · Juan Quintela

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

Why does it do that??

That sound like a terrible idea.

Quoted 8 lines

Quoted 3 lines

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

At this point, we have basically -1 :(

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

We need a better solution for reporting errors via notifications.

Quoted 9 lines

Quoted 3 lines

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

What kind of peer information?

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

Quoted 4 lines

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

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

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

Quoted 10 lines · Anthony Liguori

Quoted 4 lines · Juan Quintela

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

Why does it do that??

That sound like a terrible idea.

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

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

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

Quoted 20 lines · Juan Quintela

Quoted 12 lines · Anthony Liguori

Quoted 5 lines · Juan Quintela

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

Why does it do that??

That sound like a terrible idea.

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

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

Quoted 3 lines

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

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

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

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

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

Regards,
Daniel

Quoted 3 lines · Daniel P. Berrange

Quoted 1 line · Juan Quintela

Anthony Liguori<[email protected]> wrote:

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

Quoted 4 lines · Anthony Liguori

Quoted 3 lines · Daniel P. Berrange

Quoted 1 line · Juan Quintela

Anthony Liguori<[email protected]> wrote:

Quoted 2 lines

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

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

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

Quoted 5 lines

Quoted 3 lines

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

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

Quoted 3 lines

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

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

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

Regards,
Daniel

Quoted 5 lines · Daniel P. Berrange

Quoted 5 lines

Quoted 3 lines

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

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

Quoted 8 lines

Quoted 3 lines

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

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

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

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

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

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

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

Quoted 18 lines · Anthony Liguori

Quoted 6 lines · Daniel P. Berrange

Quoted 6 lines

Quoted 4 lines

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

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

Quoted 10 lines

Quoted 4 lines

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

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

Quoted 7 lines

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

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

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

Quoted 5 lines

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

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

Quoted 6 lines

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

Quoted 8 lines

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

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

Daniel

Quoted 27 lines · Daniel P. Berrange

Quoted 18 lines · Anthony Liguori

Quoted 6 lines · Daniel P. Berrange

Quoted 6 lines

Quoted 4 lines

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

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

Quoted 10 lines

Quoted 4 lines

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

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

Quoted 7 lines

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

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

Quoted 10 lines

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

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

This is how the info balloon command works, BTW.

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

Quoted 14 lines

Quoted 14 lines

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

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

Quoted 2 lines

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

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

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

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

Yes and it already has 'id' support:

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

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

Quoted 1 line

This is how the info balloon command works, BTW.

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

Quoted 4 lines

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

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

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

[...]

Quoted 14 lines

Quoted 14 lines

Quoted 14 lines

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

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

Quoted 5 lines

Quoted 2 lines

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

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

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

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

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

Quoted 25 lines · Luiz Capitulino

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

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

Yes and it already has 'id' support:

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

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

Quoted 1 line

This is how the info balloon command works, BTW.

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

Quoted 4 lines

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

Quoted 6 lines

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

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

How difficult is that?

Quoted 6 lines

Quoted 2 lines

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

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

Quoted 7 lines

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

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

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

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

Quoted 28 lines · Luiz Capitulino

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

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

Yes and it already has 'id' support:

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

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

Quoted 2 lines

This is how the info balloon command works, BTW.

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

Quoted 5 lines

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

Quoted 7 lines

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

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

How difficult is that?

Quoted 27 lines

[...]

Quoted 15 lines

Quoted 15 lines

Quoted 15 lines

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

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

Quoted 6 lines

Quoted 3 lines

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

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

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

Quoted 8 lines

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

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

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

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

Later, Juan.

Quoted 3 lines · Daniel P. Berrange

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

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

Quoted 6 lines

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

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

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

Quoted 12 lines · Anthony Liguori

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

Quoted 7 lines · Juan Quintela

QMP/qmp-events.txt +6 -0

+MIGRATION_CANCELED
+------------------
+
+Emitted when migration is canceled.  This is emitted in the source.
+Target will emit MIGRATION_FAILED (no way to differentiate a FAILED
+and CANCELED migration for target).

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

ok, then that one is ok.

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

Quoted 12 lines · Anthony Liguori

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

Quoted 7 lines · Juan Quintela

QMP/qmp-events.txt +6 -0

+MIGRATION_CANCELED
+------------------
+
+Emitted when migration is canceled.  This is emitted in the source.
+Target will emit MIGRATION_FAILED (no way to differentiate a FAILED
+and CANCELED migration for target).

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

Quoted 1 line

ok, then that one is ok.

Isn't this one important for the destination instead?

From management prespective, management application knows that it has

Quoted 4 lines · Juan Quintela

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

QMP/qmp-events.txt +1 -0

+Data: None

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

Quoted 10 lines

QMP/qmp-events.txt +10 -0

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

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

Quoted 4 lines

QMP/qmp-events.txt +3 -0

+Data: None
+
+Example:

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

Quoted 10 lines

QMP/qmp-events.txt +10 -0

+
+{ "event": "MIGRATION_ENDED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_FAILED
+----------------
+
+Emitted when migration fails (both is source and target).
+
+Data: None

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

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

Quoted 10 lines

QMP/qmp-events.txt +9 -0

+Example:
+
+{ "event": "MIGRATION_FAILED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
+
+MIGRATION_STARTED
+-----------------
+
+Emitted when migration starts (both in source and target).

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

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

Signed-off-by Juan Quintela <[email protected]>

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

QMP/qmp-events.txt +0 -0

index 01ec85f..93caa4d 100644

QMP/qmp-events.txt +23 -0

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

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

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

Signed-off-by Juan Quintela<[email protected]>

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

QMP/qmp-events.txt +0 -0

index 01ec85f..93caa4d 100644
@@ -26,6 +26,56 @@ Example:
  Note: If action is "stop", a STOP event will eventually follow the
  BLOCK_IO_ERROR event.
 

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

Quoted 5 lines

QMP/qmp-events.txt +4 -0

+MIGRATION_CANCELED
+------------------
+
+Emitted when migration is canceled.  This is emitted in the source.
 

Shouldn't this one be emitted in the destination?

Quoted 24 lines

QMP/qmp-events.txt +24 -0

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

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

Quoted 10 lines

QMP/qmp-events.txt +9 -0

+
+Emitted when migration fails (both is source and target).
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_FAILED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
 

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

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

Anyway, the counterpart of MIGRATION_FAILED is MIGRATION_SUCCEEDED.

Quoted 21 lines · Luiz Capitulino

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

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

Signed-off-by Juan Quintela<[email protected]>

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

QMP/qmp-events.txt +0 -0

index 01ec85f..93caa4d 100644
@@ -26,6 +26,56 @@ Example:
  Note: If action is "stop", a STOP event will eventually follow the
  BLOCK_IO_ERROR event.
 

Quoted 7 lines

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

Quoted 5 lines

QMP/qmp-events.txt +4 -0

+MIGRATION_CANCELED
+------------------
+
+Emitted when migration is canceled.  This is emitted in the source.
 

Shouldn't this one be emitted in the destination?

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

Quoted 40 lines

Quoted 24 lines

QMP/qmp-events.txt +24 -0

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

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

Quoted 10 lines

QMP/qmp-events.txt +9 -0

+
+Emitted when migration fails (both is source and target).
+
+Data: None
+
+Example:
+
+{ "event": "MIGRATION_FAILED",
+    "timestamp": {"seconds": 1274687575, "microseconds": 592483} }
 

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

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

Quoted 2 lines

Anyway, the counterpart of MIGRATION_FAILED is MIGRATION_SUCCEEDED.

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