Re: [Qemu-devel] [PATCH RFC v2 7/8] migration: add dirty parameter

[John Snow <jsnow@redhat.com>]

On 01/27/2015 05:56 AM, Vladimir Sementsov-Ogievskiy wrote:
> Add dirty parameter to qmp-migrate command. If this parameter is true,
> migration/block.c will migrate dirty bitmaps. This parameter can be used
> without "blk" parameter to migrate only dirty bitmaps, skipping block
> migration.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com>
> ---
>   hmp-commands.hx               | 12 +++++++-----
>   hmp.c                         |  4 +++-
>   include/migration/migration.h |  1 +
>   migration/migration.c         |  4 +++-
>   qapi-schema.json              |  7 ++++++-
>   qmp-commands.hx               |  5 ++++-
>   savevm.c                      |  3 ++-
>   7 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index a9be506..c16f93c 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -902,23 +902,25 @@ 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,dirty:-D,uri:s",
> +        .params     = "[-d] [-b] [-i] [-D] 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)",
> +		      "shared storage with incremental copy of disk\n\t\t\t"
> +		      " -D for migration of named dirty bitmaps as well\n\t\t\t"
> +		      " (base image shared between src and destination)",
>           .mhandler.cmd = hmp_migrate,
>       },
>
>
>   STEXI
> -@item migrate [-d] [-b] [-i] @var{uri}
> +@item migrate [-d] [-b] [-i] [-D] @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)
> +	-D for migration of named dirty bitmaps
>   ETEXI
>
>       {
> diff --git a/hmp.c b/hmp.c
> index a269145..0b89ee8 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1347,10 +1347,12 @@ 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 dirty = qdict_get_try_bool(qdict, "dirty", 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, !!dirty, dirty,
> +                false, false, &err);
>       if (err) {
>           monitor_printf(mon, "migrate: %s\n", error_get_pretty(err));
>           error_free(err);
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 3cb5ba8..48d71d3 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -37,6 +37,7 @@
>   struct MigrationParams {
>       bool blk;
>       bool shared;
> +    bool dirty;
>   };
>
>   typedef struct MigrationState MigrationState;
> diff --git a/migration/migration.c b/migration/migration.c
> index c49a05a..e7bb7f3 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -404,7 +404,8 @@ 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,
> +                 bool has_inc, bool inc, bool has_dirty, bool dirty,
> +                 bool has_detach, bool detach,
>                    Error **errp)
>   {
>       Error *local_err = NULL;
> @@ -414,6 +415,7 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
>
>       params.blk = has_blk && blk;
>       params.shared = has_inc && inc;
> +    params.dirty = has_dirty && dirty;
>
>       if (s->state == MIG_STATE_ACTIVE || s->state == MIG_STATE_SETUP ||
>           s->state == MIG_STATE_CANCELLING) {
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 1475f69..1d10d6b 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1656,12 +1656,17 @@
>   # @detach: this argument exists only for compatibility reasons and
>   #          is ignored by QEMU
>   #
> +# @dirty: #optional do dirty-bitmaps migration (can be used with or without
> +#         @blk parameter)
> +#         (since 2.3)
> +#
>   # 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', '*dirty': 'bool',
> +            '*detach': 'bool' } }
>
>   # @xen-save-devices-state:
>   #
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 8065715..f8e50ac 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,dirty:-D,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)
> +- "dirty": migrate named dirty bitmaps (json-bool, optional)
>   - "uri": Destination URI (json-string)
>
>   Example:
> @@ -638,6 +639,8 @@ Notes:
>   (2) All boolean arguments default to false
>   (3) The user Monitor's "detach" argument is invalid in QMP and should not
>       be used
> +(4) The "dirty" argument may be used without "blk", to migrate only dirty
> +    bitmaps
>
>   EQMP
>
> diff --git a/savevm.c b/savevm.c
> index 08ec678..a598d1d 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -784,7 +784,8 @@ static int qemu_savevm_state(QEMUFile *f)
>       int ret;
>       MigrationParams params = {
>           .blk = 0,
> -        .shared = 0
> +        .shared = 0,
> +        .dirty = 0
>       };
>
>       if (qemu_savevm_state_blocked(NULL)) {
>

I'll defer to Eric's judgment here; It looks like you have already 
re-spun but were waiting on my lethargy to send out to the ML again.