Re: [Qemu-devel] [PATCH 2/5] migration: Create block capability
Juan Quintela writes:
> Create one capability for block migration and one parameter for
> incremental block migration.
>
> Signed-off-by: Juan Quintela
[...]
> diff --git a/include/migration/block.h b/include/migration/block.h
> index 41a1ac8..5225af9 100644
> --- a/include/migration/block.h
> +++ b/include/migration/block.h
> @@ -20,4 +20,6 @@ uint64_t blk_mig_bytes_transferred(void);
> uint64_t blk_mig_bytes_remaining(void);
> uint64_t blk_mig_bytes_total(void);
>
> +void migrate_set_block_enabled(bool value, Error **errp);
> +
> #endif /* MIGRATION_BLOCK_H */
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 49ec501..024a048 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -153,6 +153,9 @@ struct MigrationState
>
> /* The last error that occurred */
> Error *error;
> +/* Do we have to clean up -b/-i from old migrate parameters */
The sentence is a question, so it should end with a '?'.
> +/* This feature is deprecated and will be removed */
> +bool must_remove_block_options;
> };
>
> void migrate_set_state(int *state, int old_state, int new_state);
> @@ -265,6 +268,9 @@ bool migrate_colo_enabled(void);
>
> int64_t xbzrle_cache_resize(int64_t new_size);
>
> +bool migrate_use_block(void);
> +bool migrate_use_block_incremental(void);
> +
> bool migrate_use_compression(void);
> int migrate_compress_level(void);
> int migrate_compress_threads(void);
> diff --git a/migration/migration.c b/migration/migration.c
> index 0304c01..c13c0a2 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
[...]
> @@ -1207,6 +1242,24 @@ void qmp_migrate(const char *uri, bool has_blk, bool
> blk,
> return;
> }
>
> +if ((has_blk && blk) || (has_inc && inc)) {
> +if (migrate_use_block() || migrate_use_block_incremental()) {
> +error_setg(errp, "Command options are incompatible with "
> + "current migration capabilities");
> +return;
> +}
> +migrate_set_block_enabled(true, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +s->must_remove_block_options = true;
> +}
> +
> +if (has_inc && inc) {
> +migrate_set_block_incremental(s, true);
> +}
> +
Putting this within the previous conditional might be clearer. Your
choice.
> s = migrate_init(¶ms);
>
> if (strstart(uri, "tcp:", &p)) {
[...]
Only nitpicks, so
Reviewed-by: Markus Armbruster
Re: [Qemu-devel] [PATCH 2/5] migration: Create block capability
On 05/18/2017 06:18 AM, Juan Quintela wrote: > Create one capability for block migration and one parameter for > incremental block migration. > > Signed-off-by: Juan Quintela > > --- > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 2/5] migration: Create block capability
Eric Blake wrote:
> On 05/17/2017 10:38 AM, Juan Quintela wrote:
>> Create one capability for block migration and one parameter for
>> incremental block migration.
>>
>> Signed-off-by: Juan Quintela
>>
>> ---
>>
>> @@ -1207,6 +1242,26 @@ void qmp_migrate(const char *uri, bool has_blk, bool
>> blk,
>> return;
>> }
>>
>> +if (((has_blk && blk) || (has_inc && inc))
>> +&& (migrate_use_block() || migrate_use_block_incremental())) {
>> +error_setg(errp, "Command options are incompatible with current "
>> + "migration capabilities");
>> +return;
>> +}
>> +
>> +if ((has_blk && blk) || (has_inc & inc)) {
>> +migrate_set_block_enabled(true, &local_err);
>> +if (local_err) {
>> +error_propagate(errp, local_err);
>> +return;
>> +}
>> +s->must_remove_block_options = true;
>> +}
>
> You wrote:
> if (A && B) {
> error;
> }
> if (A) {
> stuff;
> }
>
> I might have done:
>
> if (A) {
> if (B) {
> error;
> }
> stuff;
> }
>
> but it's the same either way.
One less line, and as I have to respin due to the last patch.
>> +++ b/qapi-schema.json
>> @@ -894,11 +894,18 @@
>> # @release-ram: if enabled, qemu will free the migrated ram pages on the
>> source
>> #during postcopy-ram migration. (since 2.9)
>> #
>> +# @block: If enabled, QEMU will also migrate the contents of all block
>> +# devices. Default is disabled. A possible alternative are
>
> s/are/uses/
done
>> +# mirror jobs to a builtin NBD server on the destination, which
>> +# are more flexible.
>
> s/are more flexible/offers more flexibility/
done
>> +# (Since 2.10)
>> +#
>> # Since: 1.2
>> ##
>> { 'enum': 'MigrationCapability',
>>'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
>> - 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram'] }
>> + 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram',
>> + 'block' ] }
>>
>
> The grammar touchups can be done in preparing the pull request, if there
> is no other reason for a respin.
>
> Reviewed-by: Eric Blake
Thanks, Juan.
Re: [Qemu-devel] [PATCH 2/5] migration: Create block capability
On 05/17/2017 10:38 AM, Juan Quintela wrote:
> Create one capability for block migration and one parameter for
> incremental block migration.
>
> Signed-off-by: Juan Quintela
>
> ---
>
> @@ -1207,6 +1242,26 @@ void qmp_migrate(const char *uri, bool has_blk, bool
> blk,
> return;
> }
>
> +if (((has_blk && blk) || (has_inc && inc))
> +&& (migrate_use_block() || migrate_use_block_incremental())) {
> +error_setg(errp, "Command options are incompatible with current "
> + "migration capabilities");
> +return;
> +}
> +
> +if ((has_blk && blk) || (has_inc & inc)) {
> +migrate_set_block_enabled(true, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
> +s->must_remove_block_options = true;
> +}
You wrote:
if (A && B) {
error;
}
if (A) {
stuff;
}
I might have done:
if (A) {
if (B) {
error;
}
stuff;
}
but it's the same either way.
> +++ b/qapi-schema.json
> @@ -894,11 +894,18 @@
> # @release-ram: if enabled, qemu will free the migrated ram pages on the
> source
> #during postcopy-ram migration. (since 2.9)
> #
> +# @block: If enabled, QEMU will also migrate the contents of all block
> +# devices. Default is disabled. A possible alternative are
s/are/uses/
> +# mirror jobs to a builtin NBD server on the destination, which
> +# are more flexible.
s/are more flexible/offers more flexibility/
> +# (Since 2.10)
> +#
> # Since: 1.2
> ##
> { 'enum': 'MigrationCapability',
>'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
> - 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram'] }
> + 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram',
> + 'block' ] }
>
The grammar touchups can be done in preparing the pull request, if there
is no other reason for a respin.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 2/5] migration: Create block capability
Eric Blake wrote: > On 05/16/2017 11:42 AM, Markus Armbruster wrote: > Well, to suggest something, I'd first have to figure out WTF incremental block migration does. Your text helps me some, but not enough. What exactly is being migrated, and what exactly is assumed to be shared between source and destination? Block migration is scandalously underdocumented. >>> > >> Can you draft a documentation comment for @block-incremental? > > How about: > > @block-incremental: Affects how much storage is migrated when the block > migration capability is enabled. When false, the entire storage backing > chain is migrated into a flattened image at the destination; when true, > only the active qcow2 layer is migrated and the destination must already > have access to the same backing chain as was used on the source. > (since 2.10) Changed. Thanks.
Re: [Qemu-devel] [PATCH 2/5] migration: Create block capability
Eric Blake writes: > On 05/16/2017 11:42 AM, Markus Armbruster wrote: > Well, to suggest something, I'd first have to figure out WTF incremental block migration does. Your text helps me some, but not enough. What exactly is being migrated, and what exactly is assumed to be shared between source and destination? Block migration is scandalously underdocumented. >>> > >> Can you draft a documentation comment for @block-incremental? > > How about: > > @block-incremental: Affects how much storage is migrated when the block > migration capability is enabled. When false, the entire storage backing > chain is migrated into a flattened image at the destination; when true, > only the active qcow2 layer is migrated and the destination must already > have access to the same backing chain as was used on the source. > (since 2.10) Works for me, thanks!
Re: [Qemu-devel] [PATCH 2/5] migration: Create block capability
On 05/16/2017 11:42 AM, Markus Armbruster wrote: >>> Well, to suggest something, I'd first have to figure out WTF incremental >>> block migration does. Your text helps me some, but not enough. What >>> exactly is being migrated, and what exactly is assumed to be shared >>> between source and destination? >>> >>> Block migration is scandalously underdocumented. >> > Can you draft a documentation comment for @block-incremental? How about: @block-incremental: Affects how much storage is migrated when the block migration capability is enabled. When false, the entire storage backing chain is migrated into a flattened image at the destination; when true, only the active qcow2 layer is migrated and the destination must already have access to the same backing chain as was used on the source. (since 2.10) -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 2/5] migration: Create block capability
Eric Blake writes: > On 05/16/2017 11:00 AM, Markus Armbruster wrote: > > # periodic mode. (Since 2.8) > # > +# @block-incremental: enable block incremental migration (Since 2.10) > +# What's "block incremental migration" and why should I care? >>> >>> This is good, I will try. >>> >>> "block incremental migration assumes that we have a base image in both >>> sides, and then we continue writting in one of the sides. This way we >>> need to only migrate the changes since the previous state where it was >>> the same in both sides". >>> >>> I am not sure what to put there, really. >> >> Well, to suggest something, I'd first have to figure out WTF incremental >> block migration does. Your text helps me some, but not enough. What >> exactly is being migrated, and what exactly is assumed to be shared >> between source and destination? >> >> Block migration is scandalously underdocumented. > > If I have: > > base <- active > > on the source, then: > > block migration without incremental creates: > > active > > on the destination (the entire disk contents are migrated). Conversely, > block migration WITH incremental assumes that I have pre-created 'base' > on the destination (easy to do, since base is read-only so it can be > copied prior to starting the migration), and the migration results in: > > base <- active > > on the destination, where only the contents of active were transferred > by qemu. Can you draft a documentation comment for @block-incremental?
Re: [Qemu-devel] [PATCH 2/5] migration: Create block capability
On 05/16/2017 11:00 AM, Markus Armbruster wrote: # periodic mode. (Since 2.8) # +# @block-incremental: enable block incremental migration (Since 2.10) +# >>> >>> What's "block incremental migration" and why should I care? >> >> This is good, I will try. >> >> "block incremental migration assumes that we have a base image in both >> sides, and then we continue writting in one of the sides. This way we >> need to only migrate the changes since the previous state where it was >> the same in both sides". >> >> I am not sure what to put there, really. > > Well, to suggest something, I'd first have to figure out WTF incremental > block migration does. Your text helps me some, but not enough. What > exactly is being migrated, and what exactly is assumed to be shared > between source and destination? > > Block migration is scandalously underdocumented. If I have: base <- active on the source, then: block migration without incremental creates: active on the destination (the entire disk contents are migrated). Conversely, block migration WITH incremental assumes that I have pre-created 'base' on the destination (easy to do, since base is read-only so it can be copied prior to starting the migration), and the migration results in: base <- active on the destination, where only the contents of active were transferred by qemu. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 2/5] migration: Create block capability
Juan Quintela writes:
> Markus Armbruster wrote:
>> Juan Quintela writes:
>>
>>> Create one capability for block migration and one parameter for
>>> incremental block migration.
>>>
>>> Signed-off-by: Juan Quintela
>
>>>
>>> /* The last error that occurred */
>>> Error *error;
>>> +/* Do we have to clean up -b/-i from old migrate paramteres */
>>
>> parameters
>
> ok.
>
>
>>> +void migrate_set_block_enabled(bool value, Error **errp)
>>> +{
>>> +MigrationCapabilityStatusList *cap;
>>> +
>>> +cap = g_malloc0(sizeof(*cap));
>>> +cap->value = g_malloc(sizeof(*cap->value));
>>
>> I prefer g_new() for extra type checking. Your choice.
>
> ok.
>
>
>>> +cap->value->capability = MIGRATION_CAPABILITY_BLOCK;
>>> +cap->value->state = value;
>>> +qmp_migrate_set_capabilities(cap, errp);
>>> +qapi_free_MigrationCapabilityStatusList(cap);
>>> +}
>>
>> The objective is to set one bit. The means is building a
>> MigrationCapabilityStatusList. When you have to build an elaborate data
>> structure just so you can set one bit, your interfaces are likely
>> deficient. Observation, not change request.
>
> This was Dave suggestion. The reason for doing this is in the following
> patch when we enable compile disable of block migration to put the ifdef
> in a single place. Otherwise, I have to put it in two places.
>
>>> +static void migrate_set_block_incremental(MigrationState *s, bool value)
>>> +{
>>> +s->parameters.block_incremental = value;
>>> +}
>>
>> This is how setting one bit should look like.
>
> See previous comment.
>
>>
>>> +
>>> +static void block_cleanup_parameters(MigrationState *s)
>>> +{
>>> +if (s->must_remove_block) {
>>> +migrate_set_block_enabled(false, NULL);
>>
>> NULL ignores errors. If an error happens, and we ignore it, we're
>> screwed, aren't we? I suspect we want &error_abort here.
>
> setting to false can't never fail. It is when we set it to true that it
> can fail because it is compiled out (that will be in next patch).
Sounds like a job for &error_abort to me.
>>> @@ -1229,6 +1267,38 @@ void qmp_migrate(const char *uri, bool has_blk, bool
>>> blk,
>>> return;
>>> }
>>>
>>> +if (has_blk && blk) {
>>> +if (migrate_use_block() || migrate_use_block_incremental()) {
>>> +error_setg(errp, "You can't use block capability and -b/i");
>>
>> Error message assumes HMP. In QMP, the thing is called 'blk', not -b.
>> Perhaps we can sidestep the issue like this: "Command options are
>> incompatible with current migration capabilities".
>
> ok.
>
>>
>>> +return;
>>> +}
>>> +migrate_set_block_enabled(true, &local_err);
>>> +if (local_err) {
>>> +error_propagate(errp, local_err);
>>> +return;
>>> +}
>>> +block_enabled = true;
>>> +s->must_remove_block = true;
>>> +}
>>> +
>>> +if (has_inc && inc) {
>>> +if ((migrate_use_block() && !block_enabled)
>>> +|| migrate_use_block_incremental()) {
>>> +error_setg(errp, "You can't use block capability and -b/i");
>>
>> Likewise.
>>
>> The condition would be slightly simpler if you completed error checking
>> before changing global state. Matter of taste.
>
> Being bug compatible has this drawbacks. I also hate that -i implies -b.
>
>>> --- a/qapi-schema.json
>>> +++ b/qapi-schema.json
>>> @@ -894,11 +894,14 @@
>>> # @release-ram: if enabled, qemu will free the migrated ram pages on the
>>> source
>>> #during postcopy-ram migration. (since 2.9)
>>> #
>>> +# @block: enable block migration (Since 2.10)
>>> +#
>>
>> What's "block migration" and why should I care?
>
> "enable migration of block devices. The granularity is all devices or
> no devices, nothing in the middle."
>
> I will be happy with suggestions.
# @block: If enabled, QEMU will also migrate the contents of all block
# devices. Default is disabled. A possible alternative are
# mirror jobs to a builtin NBD server on the destination, which
# are more flexible.
# (Since 2.10)
>>> # Since: 1.2
>>> ##
>>> { 'enum': 'MigrationCapability',
>>>'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
>>> - 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram'] }
>>> + 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram',
>>> + 'block' ] }
>>>
>>> ##
>>> # @MigrationCapabilityStatus:
>>> @@ -1009,13 +1012,15 @@
>>> # @x-checkpoint-delay: The delay time (in ms) between two COLO checkpoints
>>> in
>>> # periodic mode. (Since 2.8)
>>> #
>>> +# @block-incremental: enable block incremental migration (Since 2.10)
>>> +#
>>
>> What's "block incremental migration" and why should I care?
>
> This is good, I will try.
>
> "block incremental migration assumes that we have a base image in both
> sides, and then we continue writting in one of the sides. This way we
Re: [Qemu-devel] [PATCH 2/5] migration: Create block capability
Markus Armbruster wrote:
> Juan Quintela writes:
>
>> Create one capability for block migration and one parameter for
>> incremental block migration.
>>
>> Signed-off-by: Juan Quintela
>>
>> /* The last error that occurred */
>> Error *error;
>> +/* Do we have to clean up -b/-i from old migrate paramteres */
>
> parameters
ok.
>> +void migrate_set_block_enabled(bool value, Error **errp)
>> +{
>> +MigrationCapabilityStatusList *cap;
>> +
>> +cap = g_malloc0(sizeof(*cap));
>> +cap->value = g_malloc(sizeof(*cap->value));
>
> I prefer g_new() for extra type checking. Your choice.
ok.
>> +cap->value->capability = MIGRATION_CAPABILITY_BLOCK;
>> +cap->value->state = value;
>> +qmp_migrate_set_capabilities(cap, errp);
>> +qapi_free_MigrationCapabilityStatusList(cap);
>> +}
>
> The objective is to set one bit. The means is building a
> MigrationCapabilityStatusList. When you have to build an elaborate data
> structure just so you can set one bit, your interfaces are likely
> deficient. Observation, not change request.
This was Dave suggestion. The reason for doing this is in the following
patch when we enable compile disable of block migration to put the ifdef
in a single place. Otherwise, I have to put it in two places.
>> +static void migrate_set_block_incremental(MigrationState *s, bool value)
>> +{
>> +s->parameters.block_incremental = value;
>> +}
>
> This is how setting one bit should look like.
See previous comment.
>
>> +
>> +static void block_cleanup_parameters(MigrationState *s)
>> +{
>> +if (s->must_remove_block) {
>> +migrate_set_block_enabled(false, NULL);
>
> NULL ignores errors. If an error happens, and we ignore it, we're
> screwed, aren't we? I suspect we want &error_abort here.
setting to false can't never fail. It is when we set it to true that it
can fail because it is compiled out (that will be in next patch).
>> @@ -1229,6 +1267,38 @@ void qmp_migrate(const char *uri, bool has_blk, bool
>> blk,
>> return;
>> }
>>
>> +if (has_blk && blk) {
>> +if (migrate_use_block() || migrate_use_block_incremental()) {
>> +error_setg(errp, "You can't use block capability and -b/i");
>
> Error message assumes HMP. In QMP, the thing is called 'blk', not -b.
> Perhaps we can sidestep the issue like this: "Command options are
> incompatible with current migration capabilities".
ok.
>
>> +return;
>> +}
>> +migrate_set_block_enabled(true, &local_err);
>> +if (local_err) {
>> +error_propagate(errp, local_err);
>> +return;
>> +}
>> +block_enabled = true;
>> +s->must_remove_block = true;
>> +}
>> +
>> +if (has_inc && inc) {
>> +if ((migrate_use_block() && !block_enabled)
>> +|| migrate_use_block_incremental()) {
>> +error_setg(errp, "You can't use block capability and -b/i");
>
> Likewise.
>
> The condition would be slightly simpler if you completed error checking
> before changing global state. Matter of taste.
Being bug compatible has this drawbacks. I also hate that -i implies -b.
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -894,11 +894,14 @@
>> # @release-ram: if enabled, qemu will free the migrated ram pages on the
>> source
>> #during postcopy-ram migration. (since 2.9)
>> #
>> +# @block: enable block migration (Since 2.10)
>> +#
>
> What's "block migration" and why should I care?
"enable migration of block devices. The granularity is all devices or
no devices, nothing in the middle."
I will be happy with suggestions.
>
>> # Since: 1.2
>> ##
>> { 'enum': 'MigrationCapability',
>>'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
>> - 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram'] }
>> + 'compress', 'events', 'postcopy-ram', 'x-colo', 'release-ram',
>> + 'block' ] }
>>
>> ##
>> # @MigrationCapabilityStatus:
>> @@ -1009,13 +1012,15 @@
>> # @x-checkpoint-delay: The delay time (in ms) between two COLO checkpoints
>> in
>> # periodic mode. (Since 2.8)
>> #
>> +# @block-incremental: enable block incremental migration (Since 2.10)
>> +#
>
> What's "block incremental migration" and why should I care?
This is good, I will try.
"block incremental migration assumes that we have a base image in both
sides, and then we continue writting in one of the sides. This way we
need to only migrate the changes since the previous state where it was
the same in both sides".
I am not sure what to put there, really.
>
>> # Since: 2.4
>> ##
>> { 'enum': 'MigrationParameter',
>>'data': ['compress-level', 'compress-threads', 'decompress-threads',
>> 'cpu-throttle-initial', 'cpu-throttle-increment',
>> 'tls-creds', 'tls-hostname', 'max-bandwidth',
>> - 'downtime-limit', 'x-checkpoint-delay' ] }
>> + 'downtime-limit', '
Re: [Qemu-devel] [PATCH 2/5] migration: Create block capability
Juan Quintela writes:
> Create one capability for block migration and one parameter for
> incremental block migration.
>
> Signed-off-by: Juan Quintela
> ---
> hmp.c | 13 +++
> include/migration/block.h | 2 +
> include/migration/migration.h | 7
> migration/migration.c | 88
> +++
> qapi-schema.json | 14 +--
> 5 files changed, 121 insertions(+), 3 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index acbbc5c..208154c 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -326,6 +326,10 @@ void hmp_info_migrate_parameters(Monitor *mon, const
> QDict *qdict)
> monitor_printf(mon, "%s: %" PRId64 "\n",
>
> MigrationParameter_lookup[MIGRATION_PARAMETER_X_CHECKPOINT_DELAY],
> params->x_checkpoint_delay);
> +assert(params->has_block_incremental);
> +monitor_printf(mon, "%s: %s\n",
> +MigrationParameter_lookup[MIGRATION_PARAMETER_BLOCK_INCREMENTAL],
> + params->block_incremental ? "on" : "off");
> }
>
> qapi_free_MigrationParameters(params);
> @@ -1527,6 +1531,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const
> QDict *qdict)
> Visitor *v = string_input_visitor_new(valuestr);
> uint64_t valuebw = 0;
> int64_t valueint = 0;
> +bool valuebool = false;
> Error *err = NULL;
> bool use_int_value = false;
> int i, ret;
> @@ -1581,6 +1586,14 @@ void hmp_migrate_set_parameter(Monitor *mon, const
> QDict *qdict)
> p.has_x_checkpoint_delay = true;
> use_int_value = true;
> break;
> +case MIGRATION_PARAMETER_BLOCK_INCREMENTAL:
> +p.has_block_incremental = true;
> +visit_type_bool(v, param, &valuebool, &err);
> +if (err) {
> +goto cleanup;
> +}
> +p.block_incremental = valuebool;
> +break;
> }
>
> if (use_int_value) {
> diff --git a/include/migration/block.h b/include/migration/block.h
> index 41a1ac8..5225af9 100644
> --- a/include/migration/block.h
> +++ b/include/migration/block.h
> @@ -20,4 +20,6 @@ uint64_t blk_mig_bytes_transferred(void);
> uint64_t blk_mig_bytes_remaining(void);
> uint64_t blk_mig_bytes_total(void);
>
> +void migrate_set_block_enabled(bool value, Error **errp);
> +
> #endif /* MIGRATION_BLOCK_H */
> diff --git a/include/migration/migration.h b/include/migration/migration.h
> index 47262bd..adcb8f6 100644
> --- a/include/migration/migration.h
> +++ b/include/migration/migration.h
> @@ -179,6 +179,10 @@ struct MigrationState
>
> /* The last error that occurred */
> Error *error;
> +/* Do we have to clean up -b/-i from old migrate paramteres */
parameters
> +/* This feature is deprecated and will be removed */
> +bool must_remove_block;
> +bool must_remove_block_incremental;
> };
>
> void migrate_set_state(int *state, int old_state, int new_state);
> @@ -293,6 +297,9 @@ bool migrate_colo_enabled(void);
>
> int64_t xbzrle_cache_resize(int64_t new_size);
>
> +bool migrate_use_block(void);
> +bool migrate_use_block_incremental(void);
> +
> bool migrate_use_compression(void);
> int migrate_compress_level(void);
> int migrate_compress_threads(void);
> diff --git a/migration/migration.c b/migration/migration.c
> index 5c92851..03f96df 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -599,6 +599,8 @@ MigrationParameters *qmp_query_migrate_parameters(Error
> **errp)
> params->downtime_limit = s->parameters.downtime_limit;
> params->has_x_checkpoint_delay = true;
> params->x_checkpoint_delay = s->parameters.x_checkpoint_delay;
> +params->has_block_incremental = true;
> +params->block_incremental = s->parameters.block_incremental;
>
> return params;
> }
> @@ -907,6 +909,9 @@ void qmp_migrate_set_parameters(MigrationParameters
> *params, Error **errp)
> colo_checkpoint_notify(s);
> }
> }
> +if (params->has_block_incremental) {
> +s->parameters.block_incremental = params->block_incremental;
> +}
> }
>
>
> @@ -942,6 +947,35 @@ void migrate_set_state(int *state, int old_state, int
> new_state)
> }
> }
>
> +void migrate_set_block_enabled(bool value, Error **errp)
> +{
> +MigrationCapabilityStatusList *cap;
> +
> +cap = g_malloc0(sizeof(*cap));
> +cap->value = g_malloc(sizeof(*cap->value));
I prefer g_new() for extra type checking. Your choice.
> +cap->value->capability = MIGRATION_CAPABILITY_BLOCK;
> +cap->value->state = value;
> +qmp_migrate_set_capabilities(cap, errp);
> +qapi_free_MigrationCapabilityStatusList(cap);
> +}
The objective is to set one bit. The means is building a
MigrationCapabilityStatusList. When you have to build an elaborate data
structure just so you can set one bit, your in
