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

["Daniel P. Berrange" <berrange@redhat.com>]

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