Re: [PATCH 1/1] migration/cpr: Add HMP support for cpr-transfer

["Dr. David Alan Gilbert" <dave@treblig.org>]

* Dongli Zhang (dongli.zhang@oracle.com) wrote:
> Currently the cpr-transfer source QEMU instance cannot be driven entirely
> via HMP.  The source must use QMP in order to specify both the
> main migration channel and the CPR channel.
> 
> Extend the HMP migrate command with an optional CPR channel URI. When the
> migration mode is cpr-transfer, HMP uses this URI to build a
> CPR MigrationChannel in addition to the main migration channel. The new
> argument is rejected unless the migration mode is cpr-transfer, so existing
> HMP migrate usage is unchanged.
> 
> For example, source QEMU HMP commands can be something like below. The
> "unix:/tmp/cpr.sock" is for CPR URI.
> 
> (qemu) migrate_set_parameter mode cpr-transfer
> (qemu) migrate tcp:0:50002 unix:/tmp/cpr.sock
> 
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>

Yeh this looks OK to me; I don't see a check for if you do:
(qemu) migrate_set_parameter mode cpr-transfer
(qemu) migrate tcp:0:50002

Does that get caught by the qmp_migrate or should you add a check here?

I do have a slight preference for using flag optional parameters rather 
than positional, e.g.

   migrate -c unix:/tmp/cpr.sock tcp:0:50002

but I wouldn't insist on it.

Dave


> ---
>  hmp-commands.hx                | 11 +++++++----
>  migration/migration-hmp-cmds.c | 18 ++++++++++++++++++
>  2 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 7ae2468a3d..021e51b142 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -928,16 +928,17 @@ ERST
>  
>      {
>          .name       = "migrate",
> -        .args_type  = "detach:-d,resume:-r,uri:s",
> -        .params     = "[-d] [-r] uri",
> +        .args_type  = "detach:-d,resume:-r,uri:s,uri-cpr:s?",
> +        .params     = "[-d] [-r] uri [uri-cpr]",
>          .help       = "migrate to URI (using -d to not wait for completion)"
> -		      "\n\t\t\t -r to resume a paused postcopy migration",
> +		      "\n\t\t\t -r to resume a paused postcopy migration"
> +		      "\n\t\t\t uri-cpr specifies CPR URI for cpr-transfer mode",
>          .cmd        = hmp_migrate,
>      },
>  
>  
>  SRST
> -``migrate [-d] [-r]`` *uri*
> +``migrate [-d] [-r]`` *uri* [*uri-cpr*]
>    Migrate the VM to *uri*.
>  
>    ``-d``
> @@ -945,6 +946,8 @@ SRST
>      query an ongoing migration process, use "info migrate".
>    ``-r``
>      Resume a paused postcopy migration.
> +  ``uri-cpr``
> +    Specify CPR URI for cpr-transfer mode. It must be a UNIX domain socket.
>  ERST
>  
>      {
> diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
> index 8b385f560e..42ca652bb3 100644
> --- a/migration/migration-hmp-cmds.c
> +++ b/migration/migration-hmp-cmds.c
> @@ -836,9 +836,11 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
>      bool detach = qdict_get_try_bool(qdict, "detach", false);
>      bool resume = qdict_get_try_bool(qdict, "resume", false);
>      const char *uri = qdict_get_str(qdict, "uri");
> +    const char *uri_cpr = qdict_get_try_str(qdict, "uri-cpr");
>      Error *err = NULL;
>      g_autoptr(MigrationChannelList) caps = NULL;
>      g_autoptr(MigrationChannel) channel = NULL;
> +    g_autoptr(MigrationChannel) channel_cpr = NULL;
>  
>      if (!migrate_uri_parse(uri, &channel, &err)) {
>          hmp_handle_error(mon, err);
> @@ -846,6 +848,22 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
>      }
>      QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel));
>  
> +    if (uri_cpr) {
> +        if (migrate_mode() != MIG_MODE_CPR_TRANSFER) {
> +            error_setg(&err, "uri-cpr requires cpr-transfer mode");
> +            hmp_handle_error(mon, err);
> +            return;
> +        }
> +
> +        if (!migrate_uri_parse(uri_cpr, &channel_cpr, &err)) {
> +            hmp_handle_error(mon, err);
> +            return;
> +        }
> +
> +        channel_cpr->channel_type = MIGRATION_CHANNEL_TYPE_CPR;
> +        QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel_cpr));
> +    }
> +
>      qmp_migrate(NULL, true, caps, true, resume, &err);
>      if (hmp_handle_error(mon, err)) {
>          return;
> -- 
> 2.43.7
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/