[Qemu-devel] [PATCH v2 0/3] -incoming pause

[Qemu-devel] [PATCH v2 0/3] -incoming pause

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

This patchset provides a way of setting options on an incoming
migration before the fd/process/socket has been created.

   start qemu with   -incoming pause
   <use the monitor to set whatever you need>
   migrate_incoming theuri

v2:
  Create migrate_incoming/migrate-incoming rather than squashing the -u
  into the existing migrate command.

Dave

Dr. David Alan Gilbert (3):
  Add -incoming pause
  Add migrate_incoming
  Document -incoming options

 hmp-commands.hx       | 16 ++++++++++++++++
 hmp.c                 | 14 ++++++++++++++
 hmp.h                 |  1 +
 migration/migration.c | 48 ++++++++++++++++++++++++++++++++++++++++++------
 qapi-schema.json      | 15 +++++++++++++++
 qemu-options.hx       | 29 ++++++++++++++++++++++++++---
 qmp-commands.hx       | 31 ++++++++++++++++++++++++++++++-
 7 files changed, 144 insertions(+), 10 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v2 1/3] Add -incoming pause

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

-incoming pause causes qemu to wait for an incoming migration
to be specified later.  The monitor can be used to set migration
capabilities that may affect the incoming connection process.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration.c | 29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index b3adbc6..77263a5 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -49,6 +49,8 @@ enum {
 static NotifierList migration_state_notifiers =
     NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
 
+static bool paused_incoming;
+
 /* When we add fault tolerance, we could have several
    migrations at once.  For now we don't need to add
    dynamic creation of migration */
@@ -65,25 +67,40 @@ MigrationState *migrate_get_current(void)
     return &current_migration;
 }
 
+/*
+ * Called on -incoming with a pause: uri.
+ * The migration can be started later after any parameters have been
+ * changed.
+ */
+static void paused_incoming_migration(Error **errp)
+{
+    if (paused_incoming) {
+        error_setg(errp, "Pausing an already paused incoming migration");
+    }
+    paused_incoming = true;
+}
+
 void qemu_start_incoming_migration(const char *uri, Error **errp)
 {
     const char *p;
 
-    if (strstart(uri, "tcp:", &p))
+    if (!strcmp(uri, "pause")) {
+        paused_incoming_migration(errp);
+    } else if (strstart(uri, "tcp:", &p)) {
         tcp_start_incoming_migration(p, errp);
 #ifdef CONFIG_RDMA
-    else if (strstart(uri, "rdma:", &p))
+    } else if (strstart(uri, "rdma:", &p)) {
         rdma_start_incoming_migration(p, errp);
 #endif
 #if !defined(WIN32)
-    else if (strstart(uri, "exec:", &p))
+    } else if (strstart(uri, "exec:", &p)) {
         exec_start_incoming_migration(p, errp);
-    else if (strstart(uri, "unix:", &p))
+    } else if (strstart(uri, "unix:", &p)) {
         unix_start_incoming_migration(p, errp);
-    else if (strstart(uri, "fd:", &p))
+    } else if (strstart(uri, "fd:", &p)) {
         fd_start_incoming_migration(p, errp);
 #endif
-    else {
+    } else {
         error_setg(errp, "unknown migration protocol: %s", uri);
     }
 }
-- 
2.1.0

[Qemu-devel] [PATCH v2 2/3] Add migrate_incoming

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Add migrate_incoming/migrate-incoming to start an incoming
migration.

Once a qemu has been started with
    -incoming pause

the migration can be started by issuing:
    migrate_incoming uri

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hmp-commands.hx       | 16 ++++++++++++++++
 hmp.c                 | 14 ++++++++++++++
 hmp.h                 |  1 +
 migration/migration.c | 19 +++++++++++++++++++
 qapi-schema.json      | 15 +++++++++++++++
 qmp-commands.hx       | 31 ++++++++++++++++++++++++++++++-
 6 files changed, 95 insertions(+), 1 deletion(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e37bc8b..d889b73 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -922,6 +922,22 @@ Cancel the current VM migration.
 ETEXI
 
     {
+        .name       = "migrate_incoming",
+        .args_type  = "uri:s",
+        .params     = "uri",
+        .help       = "Unpause an incoming migration from an -incoming pause",
+        .mhandler.cmd = hmp_migrate_incoming,
+    },
+
+STEXI
+@item migrate_incoming @var{uri}
+@findex migrate_incoming
+Unpause an incoming migration using the @var{uri} (that has the same syntax
+as the -incoming option).
+
+ETEXI
+
+    {
         .name       = "migrate_set_cache_size",
         .args_type  = "value:o",
         .params     = "value",
diff --git a/hmp.c b/hmp.c
index b47f331..f051896 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1083,6 +1083,20 @@ void hmp_migrate_cancel(Monitor *mon, const QDict *qdict)
     qmp_migrate_cancel(NULL);
 }
 
+void hmp_migrate_incoming(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    const char *uri = qdict_get_str(qdict, "uri");
+
+    qmp_migrate_incoming(uri, &err);
+
+    if (err) {
+        monitor_printf(mon, "%s\n", error_get_pretty(err));
+        error_free(err);
+        return;
+    }
+}
+
 void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict)
 {
     double value = qdict_get_double(qdict, "value");
diff --git a/hmp.h b/hmp.h
index 4bb5dca..95efe63 100644
--- a/hmp.h
+++ b/hmp.h
@@ -60,6 +60,7 @@ void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict);
 void hmp_drive_mirror(Monitor *mon, const QDict *qdict);
 void hmp_drive_backup(Monitor *mon, const QDict *qdict);
 void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
+void hmp_migrate_incoming(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict);
diff --git a/migration/migration.c b/migration/migration.c
index 77263a5..c9f482d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -432,6 +432,25 @@ void migrate_del_blocker(Error *reason)
     migration_blockers = g_slist_remove(migration_blockers, reason);
 }
 
+void qmp_migrate_incoming(const char *uri, Error **errp)
+{
+    Error *local_err = NULL;
+
+    if (!paused_incoming) {
+        error_setg(errp, "-incoming pause is required for migrate_incoming");
+        return;
+    }
+
+    qemu_start_incoming_migration(uri, &local_err);
+
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    paused_incoming = false;
+}
+
 void qmp_migrate(const char *uri, bool has_blk, bool blk,
                  bool has_inc, bool inc, bool has_detach, bool detach,
                  Error **errp)
diff --git a/qapi-schema.json b/qapi-schema.json
index e16f8eb..6093696 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1738,6 +1738,21 @@
 { 'command': 'migrate',
   'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool' } }
 
+##
+# @migrate-incoming
+#
+# Start an incoming migration, the qemu must have been started
+# with -incoming pause
+#
+# @uri: The Uniform Resource Identifier identifying the source or
+#       address to listen on
+#
+# Returns: nothing on success
+#
+# Since: 2.3
+##
+{ 'command': 'migrate-incoming', 'data': {'uri': 'str' } }
+
 # @xen-save-devices-state:
 #
 # Save the state of all devices to file. The RAM and the block devices
diff --git a/qmp-commands.hx b/qmp-commands.hx
index a85d847..79593fe 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -661,7 +661,36 @@ Example:
 <- { "return": {} }
 
 EQMP
-{
+
+    {
+        .name       = "migrate-incoming",
+        .args_type  = "uri:s",
+        .mhandler.cmd_new = qmp_marshal_input_migrate_incoming,
+    },
+
+SQMP
+migrate-incoming
+----------------
+
+Unpause an incoming migration
+
+Arguments:
+
+- "uri": Source/listening URI (json-string)
+
+Example:
+
+-> { "execute": "migrate-incoming", "arguments": { "uri": "tcp::4446" } }
+<- { "return": {} }
+
+Notes:
+
+(1) QEMU must be started with -incoming pause to allow migrate-incoming to
+    be used
+(2) The uri format is the same as to -incoming
+
+EQMP
+    {
         .name       = "migrate-set-cache-size",
         .args_type  = "value:o",
         .mhandler.cmd_new = qmp_marshal_input_migrate_set_cache_size,
-- 
2.1.0

Re: [Qemu-devel] [PATCH v2 2/3] Add migrate_incoming

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1392 bytes --]

On 02/11/2015 09:46 AM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Add migrate_incoming/migrate-incoming to start an incoming
> migration.
> 
> Once a qemu has been started with
>     -incoming pause
> 
> the migration can be started by issuing:
>     migrate_incoming uri
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hmp-commands.hx       | 16 ++++++++++++++++
>  hmp.c                 | 14 ++++++++++++++
>  hmp.h                 |  1 +
>  migration/migration.c | 19 +++++++++++++++++++
>  qapi-schema.json      | 15 +++++++++++++++
>  qmp-commands.hx       | 31 ++++++++++++++++++++++++++++++-
>  6 files changed, 95 insertions(+), 1 deletion(-)
> 

> +++ b/migration/migration.c
> @@ -432,6 +432,25 @@ void migrate_del_blocker(Error *reason)
>      migration_blockers = g_slist_remove(migration_blockers, reason);
>  }
>  
> +void qmp_migrate_incoming(const char *uri, Error **errp)
> +{
> +    Error *local_err = NULL;
> +
> +    if (!paused_incoming) {
> +        error_setg(errp, "-incoming pause is required for migrate_incoming");

Might read easier as "'-incoming pause' is required...", but that's minor.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v2 2/3] Add migrate_incoming

[Juan Quintela]

Eric Blake <eblake@redhat.com> wrote:
> On 02/11/2015 09:46 AM, Dr. David Alan Gilbert (git) wrote:
>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>> 
>> Add migrate_incoming/migrate-incoming to start an incoming
>> migration.
>> 
>> Once a qemu has been started with
>>     -incoming pause
>> 
>> the migration can be started by issuing:
>>     migrate_incoming uri
>> 
>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
>>  hmp-commands.hx       | 16 ++++++++++++++++
>>  hmp.c                 | 14 ++++++++++++++
>>  hmp.h                 |  1 +
>>  migration/migration.c | 19 +++++++++++++++++++
>>  qapi-schema.json      | 15 +++++++++++++++
>>  qmp-commands.hx       | 31 ++++++++++++++++++++++++++++++-
>>  6 files changed, 95 insertions(+), 1 deletion(-)
>> 
>
>> +++ b/migration/migration.c
>> @@ -432,6 +432,25 @@ void migrate_del_blocker(Error *reason)
>>      migration_blockers = g_slist_remove(migration_blockers, reason);
>>  }
>>  
>> +void qmp_migrate_incoming(const char *uri, Error **errp)
>> +{
>> +    Error *local_err = NULL;
>> +
>> +    if (!paused_incoming) {
>> +        error_setg(errp, "-incoming pause is required for migrate_incoming");
>
> Might read easier as "'-incoming pause' is required...", but that's minor.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

As this is a very minor fix, I apply it by hand on my tree.

Thanks to all reviewers.

Applied, Juan.

Re: [Qemu-devel] [PATCH v2 2/3] Add migrate_incoming

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Add migrate_incoming/migrate-incoming to start an incoming
> migration.
>
> Once a qemu has been started with
>     -incoming pause
>
> the migration can be started by issuing:
>     migrate_incoming uri
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

> +        .name       = "migrate_incoming",
> +        .args_type  = "uri:s",
> +        .params     = "uri",
> +        .help       = "Unpause an incoming migration from an -incoming pause",
> +        .mhandler.cmd = hmp_migrate_incoming,
> +    },

As you know, I am a funny man, so .....

(qemu) migrate_incoming paused

funny, right?

Not that --incomining in general is well prepared for abuse O:-)

Re: [Qemu-devel] [PATCH v2 2/3] Add migrate_incoming

[Dr. David Alan Gilbert]

* Juan Quintela (quintela@redhat.com) wrote:
> "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > Add migrate_incoming/migrate-incoming to start an incoming
> > migration.
> >
> > Once a qemu has been started with
> >     -incoming pause
> >
> > the migration can be started by issuing:
> >     migrate_incoming uri
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> 
> > +        .name       = "migrate_incoming",
> > +        .args_type  = "uri:s",
> > +        .params     = "uri",
> > +        .help       = "Unpause an incoming migration from an -incoming pause",
> > +        .mhandler.cmd = hmp_migrate_incoming,
> > +    },
> 
> As you know, I am a funny man, so .....
> 
> (qemu) migrate_incoming paused
> 
> funny, right?

(qemu) migrate_incoming paused
unknown migration protocol: paused
(qemu) migrate_incoming pause
Pausing an already paused incoming migration

Didn't want to let you get away with that :-)

> Not that --incomining in general is well prepared for abuse O:-)

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

[Qemu-devel] [PATCH v2 3/3] Document -incoming options

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Document the various URI formats for -incoming, the previous
manpage and help text was wrong (out of date?)

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
---
 qemu-options.hx | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 85ca3ad..fd5fbea 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3169,12 +3169,35 @@ Set TB size.
 ETEXI
 
 DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
-    "-incoming p     prepare for incoming migration, listen on port p\n",
+    "-incoming uri   prepare for incoming migration, specifying source:\n" \
+    "                exec:command    Execute 'command' use the stdout as\n" \
+    "                                the migration stream\n" \
+    "                fd:num          listen on the given fd\n" \
+    "                pause           wait for the URI to be specified by\n" \
+    "                                the monitor (migrate_incoming)\n" \
+    "                rdma:addr:port  Listen on RDMA port on given address\n" \
+    "                tcp:addr:port   listen on TCP port (optional address)\n" \
+    "                unix:path       listen on the UNIX socket 'path'\n", \
     QEMU_ARCH_ALL)
 STEXI
-@item -incoming @var{port}
+@item -incoming @var{uri}
 @findex -incoming
-Prepare for incoming migration, listen on @var{port}.
+Prepare for incoming migration, specifying the source of the migration stream
+@table @option
+@item exec:@var{command}
+Execute 'command' and use the stdout as the migration stream.
+@item fd:@var{num}
+listen on the given fd
+@item pause
+wait for the URI to be specified by the monitor (migrate_incoming)
+@item rdma:@var{addr}:@var{port}
+Listen on RDMA port on given address
+@item tcp:@var{addr}:@var{port}[,ipv4][,ipv6][,to=to]
+Listen on TCP port @var{port} (optional @var{addr} to specify address to listen on).
+The options ,ipv4, ipv6 and ,to are used in the same manner as chardev TCP options.
+@item unix:@var{path}
+listen on the UNIX socket @var{path}
+@end table
 ETEXI
 
 DEF("nodefaults", 0, QEMU_OPTION_nodefaults, \
-- 
2.1.0

Re: [Qemu-devel] [PATCH v2 0/3] -incoming pause

[Amit Shah]

On (Wed) 11 Feb 2015 [16:46:21], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> This patchset provides a way of setting options on an incoming
> migration before the fd/process/socket has been created.
> 
>    start qemu with   -incoming pause
>    <use the monitor to set whatever you need>
>    migrate_incoming theuri
> 
> v2:
>   Create migrate_incoming/migrate-incoming rather than squashing the -u
>   into the existing migrate command.

So the only comment I have is with the 'pause' name.

'pause' to me means there's a time period for which a pause has to be
done; or pausing an activity which is already in progress, both of
which are not true for this case.

My suggestions are 'wait' or 'hold' -- wait signifies we wait for a
connection; the uri for which we give later; similar for hold.


		Amit

Re: [Qemu-devel] [PATCH v2 0/3] -incoming pause

[Dr. David Alan Gilbert]

* Amit Shah (amit.shah@redhat.com) wrote:
> On (Wed) 11 Feb 2015 [16:46:21], Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > This patchset provides a way of setting options on an incoming
> > migration before the fd/process/socket has been created.
> > 
> >    start qemu with   -incoming pause
> >    <use the monitor to set whatever you need>
> >    migrate_incoming theuri
> > 
> > v2:
> >   Create migrate_incoming/migrate-incoming rather than squashing the -u
> >   into the existing migrate command.
> 
> So the only comment I have is with the 'pause' name.
> 
> 'pause' to me means there's a time period for which a pause has to be
> done; or pausing an activity which is already in progress, both of
> which are not true for this case.

I think that pause and wait are both pretty similar; wait generally
does mean wait for something specific though.

The 'pause' button on your video player doesn't have an expectation
of how long you will pause it.

> My suggestions are 'wait' or 'hold' -- wait signifies we wait for a
> connection; the uri for which we give later; similar for hold.

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 0/3] -incoming pause

[Amit Shah]

On (Tue) 17 Feb 2015 [13:27:03], Dr. David Alan Gilbert wrote:
> * Amit Shah (amit.shah@redhat.com) wrote:
> > On (Wed) 11 Feb 2015 [16:46:21], Dr. David Alan Gilbert (git) wrote:
> > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > > 
> > > This patchset provides a way of setting options on an incoming
> > > migration before the fd/process/socket has been created.
> > > 
> > >    start qemu with   -incoming pause
> > >    <use the monitor to set whatever you need>
> > >    migrate_incoming theuri
> > > 
> > > v2:
> > >   Create migrate_incoming/migrate-incoming rather than squashing the -u
> > >   into the existing migrate command.
> > 
> > So the only comment I have is with the 'pause' name.
> > 
> > 'pause' to me means there's a time period for which a pause has to be
> > done; or pausing an activity which is already in progress, both of
> > which are not true for this case.
> 
> I think that pause and wait are both pretty similar; wait generally
> does mean wait for something specific though.
> 
> The 'pause' button on your video player doesn't have an expectation
> of how long you will pause it.

Right; but it pauses an already-playing video.  We're not pausing an
incoming migration here; we're not even started.

So pause in the migration world will make sense (to me, of course)
when we actually pause an ongoing migration...


		Amit

Re: [Qemu-devel] [PATCH v2 0/3] -incoming pause

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1055 bytes --]

On 02/17/2015 07:59 AM, Amit Shah wrote:
>>>
>>> So the only comment I have is with the 'pause' name.
>>>
>>> 'pause' to me means there's a time period for which a pause has to be
>>> done; or pausing an activity which is already in progress, both of
>>> which are not true for this case.
>>
>> I think that pause and wait are both pretty similar; wait generally
>> does mean wait for something specific though.
>>
>> The 'pause' button on your video player doesn't have an expectation
>> of how long you will pause it.
> 
> Right; but it pauses an already-playing video.  We're not pausing an
> incoming migration here; we're not even started.
> 
> So pause in the migration world will make sense (to me, of course)
> when we actually pause an ongoing migration...

As long as we are bike-shedding, would 'defer' be any nicer, as an
indication that we are deferring the actual migration until the QMP
command starts it?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v2 0/3] -incoming pause

[Dr. David Alan Gilbert]

* Eric Blake (eblake@redhat.com) wrote:
> On 02/17/2015 07:59 AM, Amit Shah wrote:
> >>>
> >>> So the only comment I have is with the 'pause' name.
> >>>
> >>> 'pause' to me means there's a time period for which a pause has to be
> >>> done; or pausing an activity which is already in progress, both of
> >>> which are not true for this case.
> >>
> >> I think that pause and wait are both pretty similar; wait generally
> >> does mean wait for something specific though.
> >>
> >> The 'pause' button on your video player doesn't have an expectation
> >> of how long you will pause it.
> > 
> > Right; but it pauses an already-playing video.  We're not pausing an
> > incoming migration here; we're not even started.
> > 
> > So pause in the migration world will make sense (to me, of course)
> > when we actually pause an ongoing migration...
> 
> As long as we are bike-shedding, would 'defer' be any nicer, as an
> indication that we are deferring the actual migration until the QMP
> command starts it?

I'm happy with any of them (as I assume were those who R-b'd the set).

If people really think it needs to change then fine, I'm happy
to recut and retest it.  But if it's just bike-shedding, then I'd
rather not.

Dave

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 


--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2 0/3] -incoming pause

[Amit Shah]

On (Tue) 17 Feb 2015 [20:13:25], Dr. David Alan Gilbert wrote:
> * Eric Blake (eblake@redhat.com) wrote:
> > On 02/17/2015 07:59 AM, Amit Shah wrote:
> > >>>
> > >>> So the only comment I have is with the 'pause' name.
> > >>>
> > >>> 'pause' to me means there's a time period for which a pause has to be
> > >>> done; or pausing an activity which is already in progress, both of
> > >>> which are not true for this case.
> > >>
> > >> I think that pause and wait are both pretty similar; wait generally
> > >> does mean wait for something specific though.
> > >>
> > >> The 'pause' button on your video player doesn't have an expectation
> > >> of how long you will pause it.
> > > 
> > > Right; but it pauses an already-playing video.  We're not pausing an
> > > incoming migration here; we're not even started.
> > > 
> > > So pause in the migration world will make sense (to me, of course)
> > > when we actually pause an ongoing migration...
> > 
> > As long as we are bike-shedding, would 'defer' be any nicer, as an
> > indication that we are deferring the actual migration until the QMP
> > command starts it?
> 
> I'm happy with any of them (as I assume were those who R-b'd the set).
> 
> If people really think it needs to change then fine, I'm happy
> to recut and retest it.  But if it's just bike-shedding, then I'd
> rather not.

One of the reasons for me raising this is the line:

'Pausing an already paused guest'

comparing that to:

'Incoming migration already deferred'

or some construct like that is much clearer.

The former just adds to the confusion and the mess we have with
migration error messages.  You've already done a good job fixing many
of our existing error and debug messages, so it'll be great to avoid
introducing one.

Thanks,

		Amit

Re: [Qemu-devel] [PATCH v2 0/3] -incoming pause

[Amit Shah]

On (Tue) 17 Feb 2015 [09:14:27], Eric Blake wrote:
> On 02/17/2015 07:59 AM, Amit Shah wrote:
> >>>
> >>> So the only comment I have is with the 'pause' name.
> >>>
> >>> 'pause' to me means there's a time period for which a pause has to be
> >>> done; or pausing an activity which is already in progress, both of
> >>> which are not true for this case.
> >>
> >> I think that pause and wait are both pretty similar; wait generally
> >> does mean wait for something specific though.
> >>
> >> The 'pause' button on your video player doesn't have an expectation
> >> of how long you will pause it.
> > 
> > Right; but it pauses an already-playing video.  We're not pausing an
> > incoming migration here; we're not even started.
> > 
> > So pause in the migration world will make sense (to me, of course)
> > when we actually pause an ongoing migration...
> 
> As long as we are bike-shedding, would 'defer' be any nicer, as an
> indication that we are deferring the actual migration until the QMP
> command starts it?

I like 'defer'; I wasn't too sure about wait/hold as well, so thanks :-)


		Amit