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

[Qemu-devel] [PATCH 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 -u theuri 

Dave

Dr. David Alan Gilbert (3):
  Add -incoming pause
  Add migrate -u option for -incoming pause
  Document -incoming options

 hmp-commands.hx       | 11 +++++++----
 hmp.c                 |  6 ++++--
 migration/migration.c | 50 +++++++++++++++++++++++++++++++++++++++++++-------
 qapi-schema.json      |  5 ++++-
 qemu-options.hx       | 29 ++++++++++++++++++++++++++---
 qmp-commands.hx       |  3 ++-
 6 files changed, 86 insertions(+), 18 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH 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 migraiton
capabilities that may affect the incoming connection process.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@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 2/3] Add migrate -u option for -incoming pause

[Dr. David Alan Gilbert (git)]

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

Once a qemu has been started with -incoming pause   the
migration can be started by issuing:

migrate -u uri

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hmp-commands.hx       | 11 +++++++----
 hmp.c                 |  6 ++++--
 migration/migration.c | 21 ++++++++++++++++++++-
 qapi-schema.json      |  5 ++++-
 qmp-commands.hx       |  3 ++-
 5 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e37bc8b..45f293a 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -887,23 +887,26 @@ ETEXI
 
     {
         .name       = "migrate",
-        .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
-        .params     = "[-d] [-b] [-i] uri",
+        .args_type  = "detach:-d,blk:-b,inc:-i,unpause:-u,uri:s",
+        .params     = "[-d] [-b] [-i] [-u] uri",
         .help       = "migrate to URI (using -d to not wait for completion)"
 		      "\n\t\t\t -b for migration without shared storage with"
 		      " full copy of disk\n\t\t\t -i for migration without "
 		      "shared storage with incremental copy of disk "
-		      "(base image shared between src and destination)",
+		      "(base image shared between src and destination)"
+                      "\n\t\t\t -u unpauses an incoming migration started with "
+                      "-incoming pause using the given uri.",
         .mhandler.cmd = hmp_migrate,
     },
 
 
 STEXI
-@item migrate [-d] [-b] [-i] @var{uri}
+@item migrate [-d] [-b] [-i] [-u] @var{uri}
 @findex migrate
 Migrate to @var{uri} (using -d to not wait for completion).
 	-b for migration with full copy of disk
 	-i for migration with incremental copy of disk (base image is shared)
+        -u to unpause an incoming migration started with -incoming pause
 ETEXI
 
     {
diff --git a/hmp.c b/hmp.c
index b47f331..fb0cde1 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1344,17 +1344,19 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
     int detach = qdict_get_try_bool(qdict, "detach", 0);
     int blk = qdict_get_try_bool(qdict, "blk", 0);
     int inc = qdict_get_try_bool(qdict, "inc", 0);
+    int unpause = qdict_get_try_bool(qdict, "unpause", 0);
     const char *uri = qdict_get_str(qdict, "uri");
     Error *err = NULL;
 
-    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err);
+    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, !!unpause, unpause,
+                &err);
     if (err) {
         monitor_printf(mon, "migrate: %s\n", error_get_pretty(err));
         error_free(err);
         return;
     }
 
-    if (!detach) {
+    if (!detach && !unpause) {
         MigrationStatus *status;
 
         if (monitor_suspend(mon) < 0) {
diff --git a/migration/migration.c b/migration/migration.c
index 77263a5..2308067 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -434,7 +434,7 @@ void migrate_del_blocker(Error *reason)
 
 void qmp_migrate(const char *uri, bool has_blk, bool blk,
                  bool has_inc, bool inc, bool has_detach, bool detach,
-                 Error **errp)
+                 bool has_unpause, bool unpause, Error **errp)
 {
     Error *local_err = NULL;
     MigrationState *s = migrate_get_current();
@@ -450,6 +450,25 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
         return;
     }
 
+    if (unpause) {
+        /* Unpause is starting up an *incoming* migration */
+        if (detach || inc || blk) {
+            error_setg(errp, "Other flags are not meaningful for unpause");
+            return;
+        }
+
+        if (!paused_incoming) {
+            error_setg(errp, "Unpause can only be used with -incoming pause");
+            return;
+        }
+
+        qemu_start_incoming_migration(uri, errp);
+        if (!errp || !*errp) {
+            paused_incoming = false;
+        }
+        return;
+    }
+
     if (runstate_check(RUN_STATE_INMIGRATE)) {
         error_setg(errp, "Guest is waiting for an incoming migration");
         return;
diff --git a/qapi-schema.json b/qapi-schema.json
index e16f8eb..8dd0b88 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1731,12 +1731,15 @@
 # @detach: this argument exists only for compatibility reasons and
 #          is ignored by QEMU
 #
+# @unpause: Start an inward migration initiated with -incoming pause
+#
 # Returns: nothing on success
 #
 # Since: 0.14.0
 ##
 { 'command': 'migrate',
-  'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool' } }
+  'data': {'uri': 'str', '*blk': 'bool', '*inc': 'bool', '*detach': 'bool',
+           '*unpause': 'bool' } }
 
 # @xen-save-devices-state:
 #
diff --git a/qmp-commands.hx b/qmp-commands.hx
index a85d847..76673c5 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -610,7 +610,7 @@ EQMP
 
     {
         .name       = "migrate",
-        .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
+        .args_type  = "detach:-d,blk:-b,inc:-i,unpause:-u,uri:s",
         .mhandler.cmd_new = qmp_marshal_input_migrate,
     },
 
@@ -624,6 +624,7 @@ Arguments:
 
 - "blk": block migration, full disk copy (json-bool, optional)
 - "inc": incremental disk copy (json-bool, optional)
+- "unpause": Unpause an incoming migration (json-bool, optional)
 - "uri": Destination URI (json-string)
 
 Example:
-- 
2.1.0

[Qemu-devel] [PATCH 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>
---
 qemu-options.hx | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 85ca3ad..aae9a99 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 -u)\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 -u)
+@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 1/3] Add -incoming pause

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> 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 migraiton
> capabilities that may affect the incoming connection process.
>

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

Re: [Qemu-devel] [PATCH 2/3] Add migrate -u option for -incoming pause

[Daniel P. Berrange]

On Tue, Feb 10, 2015 at 04:16:38PM +0000, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Once a qemu has been started with -incoming pause   the
> migration can be started by issuing:
> 
> migrate -u uri
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hmp-commands.hx       | 11 +++++++----
>  hmp.c                 |  6 ++++--
>  migration/migration.c | 21 ++++++++++++++++++++-
>  qapi-schema.json      |  5 ++++-
>  qmp-commands.hx       |  3 ++-
>  5 files changed, 37 insertions(+), 9 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e37bc8b..45f293a 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -887,23 +887,26 @@ ETEXI
>  
>      {
>          .name       = "migrate",
> -        .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
> -        .params     = "[-d] [-b] [-i] uri",
> +        .args_type  = "detach:-d,blk:-b,inc:-i,unpause:-u,uri:s",
> +        .params     = "[-d] [-b] [-i] [-u] uri",
>          .help       = "migrate to URI (using -d to not wait for completion)"
>  		      "\n\t\t\t -b for migration without shared storage with"
>  		      " full copy of disk\n\t\t\t -i for migration without "
>  		      "shared storage with incremental copy of disk "
> -		      "(base image shared between src and destination)",
> +		      "(base image shared between src and destination)"
> +                      "\n\t\t\t -u unpauses an incoming migration started with "
> +                      "-incoming pause using the given uri.",
>          .mhandler.cmd = hmp_migrate,
>      },
>  
>  
>  STEXI
> -@item migrate [-d] [-b] [-i] @var{uri}
> +@item migrate [-d] [-b] [-i] [-u] @var{uri}
>  @findex migrate
>  Migrate to @var{uri} (using -d to not wait for completion).
>  	-b for migration with full copy of disk
>  	-i for migration with incremental copy of disk (base image is shared)
> +        -u to unpause an incoming migration started with -incoming pause
>  ETEXI
>  
>      {
> diff --git a/hmp.c b/hmp.c
> index b47f331..fb0cde1 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1344,17 +1344,19 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
>      int detach = qdict_get_try_bool(qdict, "detach", 0);
>      int blk = qdict_get_try_bool(qdict, "blk", 0);
>      int inc = qdict_get_try_bool(qdict, "inc", 0);
> +    int unpause = qdict_get_try_bool(qdict, "unpause", 0);
>      const char *uri = qdict_get_str(qdict, "uri");
>      Error *err = NULL;
>  
> -    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err);
> +    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, !!unpause, unpause,
> +                &err);
>      if (err) {
>          monitor_printf(mon, "migrate: %s\n", error_get_pretty(err));
>          error_free(err);
>          return;
>      }
>  
> -    if (!detach) {
> +    if (!detach && !unpause) {
>          MigrationStatus *status;
>  
>          if (monitor_suspend(mon) < 0) {
> diff --git a/migration/migration.c b/migration/migration.c
> index 77263a5..2308067 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -434,7 +434,7 @@ void migrate_del_blocker(Error *reason)
>  
>  void qmp_migrate(const char *uri, bool has_blk, bool blk,
>                   bool has_inc, bool inc, bool has_detach, bool detach,
> -                 Error **errp)
> +                 bool has_unpause, bool unpause, Error **errp)
>  {
>      Error *local_err = NULL;
>      MigrationState *s = migrate_get_current();
> @@ -450,6 +450,25 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>          return;
>      }
>  
> +    if (unpause) {
> +        /* Unpause is starting up an *incoming* migration */
> +        if (detach || inc || blk) {
> +            error_setg(errp, "Other flags are not meaningful for unpause");
> +            return;
> +        }
> +
> +        if (!paused_incoming) {
> +            error_setg(errp, "Unpause can only be used with -incoming pause");
> +            return;
> +        }
> +
> +        qemu_start_incoming_migration(uri, errp);
> +        if (!errp || !*errp) {
> +            paused_incoming = false;
> +        }
> +        return;
> +    }
> +
>      if (runstate_check(RUN_STATE_INMIGRATE)) {
>          error_setg(errp, "Guest is waiting for an incoming migration");
>          return;

Hmm, the 'unpause' codepath doesn't really share anything with the existing
codepath. Also the URIs for the existing migrate command are not quite the
same as the URIs for the incoming migrate side. This would suggest to me
that it might be better to have a separate 'migrate-incoming' command in
the monitor rather than overload the existing 'migrate' command.

Also having a separate command will make it possible to detect that this
feature is supported from libvirt, since I don't think QMP introspection
provides enough info to detect it based on the new arg to existing
commands.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 2/3] Add migrate -u option for -incoming pause

[Eric Blake]

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

On 02/10/2015 09:47 AM, Daniel P. Berrange wrote:
> On Tue, Feb 10, 2015 at 04:16:38PM +0000, Dr. David Alan Gilbert (git) wrote:
>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>>
>> Once a qemu has been started with -incoming pause   the

s/pause   the/pause, the/

>> migration can be started by issuing:
>>
>> migrate -u uri
>>
>> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> ---
> 
> Hmm, the 'unpause' codepath doesn't really share anything with the existing
> codepath. Also the URIs for the existing migrate command are not quite the
> same as the URIs for the incoming migrate side. This would suggest to me
> that it might be better to have a separate 'migrate-incoming' command in
> the monitor rather than overload the existing 'migrate' command.
> 
> Also having a separate command will make it possible to detect that this
> feature is supported from libvirt, since I don't think QMP introspection
> provides enough info to detect it based on the new arg to existing
> commands.

Agree, a new command for QMP would be better (it serves as both the new
command to use, and the witness that the '-incoming pause:' command line
works).  The HMP 'migrate -u' is just fine, though (it's fine to have a
single HMP command smart enough to call out to two different QMP commands).

-- 
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 2/3] Add migrate -u option for -incoming pause

[Dr. David Alan Gilbert]

* Daniel P. Berrange (berrange@redhat.com) wrote:
> On Tue, Feb 10, 2015 at 04:16:38PM +0000, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> > 
> > Once a qemu has been started with -incoming pause   the
> > migration can be started by issuing:
> > 
> > migrate -u uri
> > 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  hmp-commands.hx       | 11 +++++++----
> >  hmp.c                 |  6 ++++--
> >  migration/migration.c | 21 ++++++++++++++++++++-
> >  qapi-schema.json      |  5 ++++-
> >  qmp-commands.hx       |  3 ++-
> >  5 files changed, 37 insertions(+), 9 deletions(-)
> > 
> > diff --git a/hmp-commands.hx b/hmp-commands.hx
> > index e37bc8b..45f293a 100644
> > --- a/hmp-commands.hx
> > +++ b/hmp-commands.hx
> > @@ -887,23 +887,26 @@ ETEXI
> >  
> >      {
> >          .name       = "migrate",
> > -        .args_type  = "detach:-d,blk:-b,inc:-i,uri:s",
> > -        .params     = "[-d] [-b] [-i] uri",
> > +        .args_type  = "detach:-d,blk:-b,inc:-i,unpause:-u,uri:s",
> > +        .params     = "[-d] [-b] [-i] [-u] uri",
> >          .help       = "migrate to URI (using -d to not wait for completion)"
> >  		      "\n\t\t\t -b for migration without shared storage with"
> >  		      " full copy of disk\n\t\t\t -i for migration without "
> >  		      "shared storage with incremental copy of disk "
> > -		      "(base image shared between src and destination)",
> > +		      "(base image shared between src and destination)"
> > +                      "\n\t\t\t -u unpauses an incoming migration started with "
> > +                      "-incoming pause using the given uri.",
> >          .mhandler.cmd = hmp_migrate,
> >      },
> >  
> >  
> >  STEXI
> > -@item migrate [-d] [-b] [-i] @var{uri}
> > +@item migrate [-d] [-b] [-i] [-u] @var{uri}
> >  @findex migrate
> >  Migrate to @var{uri} (using -d to not wait for completion).
> >  	-b for migration with full copy of disk
> >  	-i for migration with incremental copy of disk (base image is shared)
> > +        -u to unpause an incoming migration started with -incoming pause
> >  ETEXI
> >  
> >      {
> > diff --git a/hmp.c b/hmp.c
> > index b47f331..fb0cde1 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -1344,17 +1344,19 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
> >      int detach = qdict_get_try_bool(qdict, "detach", 0);
> >      int blk = qdict_get_try_bool(qdict, "blk", 0);
> >      int inc = qdict_get_try_bool(qdict, "inc", 0);
> > +    int unpause = qdict_get_try_bool(qdict, "unpause", 0);
> >      const char *uri = qdict_get_str(qdict, "uri");
> >      Error *err = NULL;
> >  
> > -    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err);
> > +    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, !!unpause, unpause,
> > +                &err);
> >      if (err) {
> >          monitor_printf(mon, "migrate: %s\n", error_get_pretty(err));
> >          error_free(err);
> >          return;
> >      }
> >  
> > -    if (!detach) {
> > +    if (!detach && !unpause) {
> >          MigrationStatus *status;
> >  
> >          if (monitor_suspend(mon) < 0) {
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 77263a5..2308067 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -434,7 +434,7 @@ void migrate_del_blocker(Error *reason)
> >  
> >  void qmp_migrate(const char *uri, bool has_blk, bool blk,
> >                   bool has_inc, bool inc, bool has_detach, bool detach,
> > -                 Error **errp)
> > +                 bool has_unpause, bool unpause, Error **errp)
> >  {
> >      Error *local_err = NULL;
> >      MigrationState *s = migrate_get_current();
> > @@ -450,6 +450,25 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> >          return;
> >      }
> >  
> > +    if (unpause) {
> > +        /* Unpause is starting up an *incoming* migration */
> > +        if (detach || inc || blk) {
> > +            error_setg(errp, "Other flags are not meaningful for unpause");
> > +            return;
> > +        }
> > +
> > +        if (!paused_incoming) {
> > +            error_setg(errp, "Unpause can only be used with -incoming pause");
> > +            return;
> > +        }
> > +
> > +        qemu_start_incoming_migration(uri, errp);
> > +        if (!errp || !*errp) {
> > +            paused_incoming = false;
> > +        }
> > +        return;
> > +    }
> > +
> >      if (runstate_check(RUN_STATE_INMIGRATE)) {
> >          error_setg(errp, "Guest is waiting for an incoming migration");
> >          return;
> 
> Hmm, the 'unpause' codepath doesn't really share anything with the existing
> codepath. Also the URIs for the existing migrate command are not quite the
> same as the URIs for the incoming migrate side. This would suggest to me
> that it might be better to have a separate 'migrate-incoming' command in
> the monitor rather than overload the existing 'migrate' command.
> 
> Also having a separate command will make it possible to detect that this
> feature is supported from libvirt, since I don't think QMP introspection
> provides enough info to detect it based on the new arg to existing
> commands.

OK, that would be easy enough for me to split out.

I'm not completely convinced that the URI parsing is currently different between
incoming and migrate; for example the 'migrate' command seems to blindly
take ,to=4445  which I think it then ignores.  But that's more of a bug.

Dave

> 
> 
> Regards,
> Daniel
> -- 
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 3/3] Document -incoming options

[Juan Quintela]

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote:
> 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>

My good, this was old.  I think that patch is needed without the paused
part, independenly  of what we do with pause.

For what is worth, I don't rememeber that "documented syntax".

Later, Juan.

Re: [Qemu-devel] [PATCH 2/3] Add migrate -u option for -incoming pause

[Juan Quintela]

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

> -		      "(base image shared between src and destination)",
> +		      "(base image shared between src and destination)"
> +                      "\n\t\t\t -u unpauses an incoming migration started with "
> +                      "-incoming pause using the given uri.",

Spaces vs tabs.

> +        -u to unpause an incoming migration started with -incoming pause

more spaces

> -    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err);
> +    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, !!unpause, unpause,
> +                &err);

I don't claim to understand QMP, but this whole bussines of !!foo, foo
is getting confusing, no?

No, this is not relaced to this patch.

>  {
>      Error *local_err = NULL;
>      MigrationState *s = migrate_get_current();
> @@ -450,6 +450,25 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>          return;
>      }
>  

I would preffer something like:

    if (runstate_check(RUN_STATE_INMIGRATE)) {
        if (unpause) {
            ... unpause code
        }
    } else {
        error_setg(errp, "Guest is waiting for an incoming migration");
        return;
    }

    if (unpause) {
        error_setg(errp, "Guest is waiting for an incoming migration");
        return;
    }

    if (s->state == MIG_STATE_ACTIVE || s->state == MIG_STATE_SETUP ||
        s->state == MIG_STATE_CANCELLING) {
        error_set(errp, QERR_MIGRATION_ACTIVE);
        return;
    }

    if (qemu_savevm_state_blocked(errp)) {
        return;
    }

.... and now continue with the rest ...




Thinking more about this problem, I am not sure this is the "cleanest
approach".   What do you think of:

- create RUN_STATE_INMIGRATE_PAUSED
  bonus: no need of paused_incoming variable

- create a new migrate_incoming command

And then we have cleaner separation of what we are doing?

Later, Juan.

Re: [Qemu-devel] [PATCH 2/3] Add migrate -u option for -incoming pause

[Dr. David Alan Gilbert]

* Eric Blake (eblake@redhat.com) wrote:
> On 02/10/2015 09:47 AM, Daniel P. Berrange wrote:
> > On Tue, Feb 10, 2015 at 04:16:38PM +0000, Dr. David Alan Gilbert (git) wrote:
> >> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >>
> >> Once a qemu has been started with -incoming pause   the
> 
> s/pause   the/pause, the/
> 
> >> migration can be started by issuing:
> >>
> >> migrate -u uri
> >>
> >> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >> ---
> > 
> > Hmm, the 'unpause' codepath doesn't really share anything with the existing
> > codepath. Also the URIs for the existing migrate command are not quite the
> > same as the URIs for the incoming migrate side. This would suggest to me
> > that it might be better to have a separate 'migrate-incoming' command in
> > the monitor rather than overload the existing 'migrate' command.
> > 
> > Also having a separate command will make it possible to detect that this
> > feature is supported from libvirt, since I don't think QMP introspection
> > provides enough info to detect it based on the new arg to existing
> > commands.
> 
> Agree, a new command for QMP would be better (it serves as both the new
> command to use, and the witness that the '-incoming pause:' command line
> works).  The HMP 'migrate -u' is just fine, though (it's fine to have a
> single HMP command smart enough to call out to two different QMP commands).

I've split it out as you all suggested;  I made it migrate-incoming and
also made an HMP migrate_incoming; although as you say you can merge them,
I just find it easier when they match - if you're used to using an HMP
command it's easier when you need to find the matching QMP command.

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 2/3] Add migrate -u option for -incoming pause

[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>
> >
> > Once a qemu has been started with -incoming pause   the
> > migration can be started by issuing:
> >
> > migrate -u uri
> >
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> > -		      "(base image shared between src and destination)",
> > +		      "(base image shared between src and destination)"
> > +                      "\n\t\t\t -u unpauses an incoming migration started with "
> > +                      "-incoming pause using the given uri.",
> 
> Spaces vs tabs.
> 
> > +        -u to unpause an incoming migration started with -incoming pause
> 
> more spaces

All this is reworked anyway since I split it out.

> > -    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err);
> > +    qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, !!unpause, unpause,
> > +                &err);
> 
> I don't claim to understand QMP, but this whole bussines of !!foo, foo
> is getting confusing, no?

Yes, and it's very very easy to screw up and get them in the wrong order.

> No, this is not relaced to this patch.
> 
> >  {
> >      Error *local_err = NULL;
> >      MigrationState *s = migrate_get_current();
> > @@ -450,6 +450,25 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
> >          return;
> >      }
> >  
> 
> I would preffer something like:
> 
>     if (runstate_check(RUN_STATE_INMIGRATE)) {
>         if (unpause) {
>             ... unpause code
>         }
>     } else {
>         error_setg(errp, "Guest is waiting for an incoming migration");
>         return;
>     }
> 
>     if (unpause) {
>         error_setg(errp, "Guest is waiting for an incoming migration");
>         return;
>     }
> 
>     if (s->state == MIG_STATE_ACTIVE || s->state == MIG_STATE_SETUP ||
>         s->state == MIG_STATE_CANCELLING) {
>         error_set(errp, QERR_MIGRATION_ACTIVE);
>         return;
>     }
> 
>     if (qemu_savevm_state_blocked(errp)) {
>         return;
>     }
> 
> .... and now continue with the rest ...

Again, all gone in the new version.

> Thinking more about this problem, I am not sure this is the "cleanest
> approach".   What do you think of:
> 
> - create RUN_STATE_INMIGRATE_PAUSED
>   bonus: no need of paused_incoming variable

That we could do separately; doing that means carefully looking at
the existing users of RUN_STATE_INMIGRATE, and since it's a
visible state that includes anything that uses the interface.

> - create a new migrate_incoming command
> 
> And then we have cleaner separation of what we are doing?

Done.

Dave

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

Re: [Qemu-devel] [PATCH 2/3] Add migrate -u option for -incoming pause

[Eric Blake]

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

On 02/11/2015 09:48 AM, Dr. David Alan Gilbert wrote:

>> Agree, a new command for QMP would be better (it serves as both the new
>> command to use, and the witness that the '-incoming pause:' command line
>> works).  The HMP 'migrate -u' is just fine, though (it's fine to have a
>> single HMP command smart enough to call out to two different QMP commands).
> 
> I've split it out as you all suggested;  I made it migrate-incoming and
> also made an HMP migrate_incoming; although as you say you can merge them,
> I just find it easier when they match - if you're used to using an HMP
> command it's easier when you need to find the matching QMP command.

Fair enough; HMP exists for ease-of-use, so whichever way you found
easier to code is sufficient :)

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