Re: [PATCH 2/6] migration: Remove 'inc' option from migrate command

2024-04-26 Thread Markus Armbruster
Markus Armbruster  writes:

> Fabiano Rosas  writes:
>
>> The block incremental option for block migration has been deprecated
>> in 8.2 in favor of using the block-mirror feature. Remove it now.
>>
>> Deprecation commit 40101f320d ("migration: migrate 'inc' command
>> option is deprecated.").
>>
>> Signed-off-by: Fabiano Rosas 

I think you missed the update to hmp-commands.hx.  I almost missed it,
too :)




Re: [PATCH 2/6] migration: Remove 'inc' option from migrate command

2024-04-26 Thread Markus Armbruster
Fabiano Rosas  writes:

> The block incremental option for block migration has been deprecated
> in 8.2 in favor of using the block-mirror feature. Remove it now.
>
> Deprecation commit 40101f320d ("migration: migrate 'inc' command
> option is deprecated.").
>
> Signed-off-by: Fabiano Rosas 
> ---
>  docs/about/deprecated.rst   |  9 --
>  docs/about/removed-features.rst | 14 +
>  migration/block.c   |  1 -
>  migration/migration-hmp-cmds.c  | 18 ++-
>  migration/migration.c   | 24 +--
>  migration/options.c | 30 +-
>  qapi/migration.json | 54 +++--
>  7 files changed, 35 insertions(+), 115 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 4d9d6bf2da..25b309dde4 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -488,15 +488,6 @@ option).
>  Migration
>  -
>  
> -``inc`` migrate command option (since 8.2)
> -''
> -
> -Use blockdev-mirror with NBD instead.
> -
> -As an intermediate step the ``inc`` functionality can be achieved by
> -setting the ``block-incremental`` migration parameter to ``true``.
> -But this parameter is also deprecated.
> -
>  ``blk`` migrate command option (since 8.2)
>  ''
>  
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index 9873f59bee..e62fc760f1 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -620,6 +620,13 @@ was superseded by ``sections``.
>  Member ``skipped`` of the ``MigrationStats`` struct hasn't been used
>  for more than 10 years. Removed with no replacement.
>  
> +``migrate`` command option ``inc`` (removed in 9.1)
> +'''
> +
> +Use blockdev-mirror with NBD instead. See "QMP invocation for live
> +storage migration with ``blockdev-mirror`` + NBD" in
> +docs/interop/live-block-operations.rst for a detailed explanation.

You didn't just copy the text from deprecated.rst, you made it more
useful.  Appreciated!

> +
>  Human Monitor Protocol (HMP) commands
>  -
>  
> @@ -680,6 +687,13 @@ This command didn't produce any output already. Removed 
> with no replacement.
>  The ``singlestep`` command has been replaced by the ``one-insn-per-tb``
>  command, which has the same behaviour but a less misleading name.
>  
> +``migrate`` command ``-i`` option (removed in 9.1)
> +''
> +
> +Use blockdev-mirror with NBD instead. See "QMP invocation for live
> +storage migration with ``blockdev-mirror`` + NBD" in
> +docs/interop/live-block-operations.rst for a detailed explanation.
> +
>  Host Architectures
>  --
>  

[...]

Reviewed-by: Markus Armbruster 




[PATCH 2/6] migration: Remove 'inc' option from migrate command

2024-04-25 Thread Fabiano Rosas
The block incremental option for block migration has been deprecated
in 8.2 in favor of using the block-mirror feature. Remove it now.

Deprecation commit 40101f320d ("migration: migrate 'inc' command
option is deprecated.").

Signed-off-by: Fabiano Rosas 
---
 docs/about/deprecated.rst   |  9 --
 docs/about/removed-features.rst | 14 +
 migration/block.c   |  1 -
 migration/migration-hmp-cmds.c  | 18 ++-
 migration/migration.c   | 24 +--
 migration/options.c | 30 +-
 qapi/migration.json | 54 +++--
 7 files changed, 35 insertions(+), 115 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 4d9d6bf2da..25b309dde4 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -488,15 +488,6 @@ option).
 Migration
 -
 
-``inc`` migrate command option (since 8.2)
-''
-
-Use blockdev-mirror with NBD instead.
-
-As an intermediate step the ``inc`` functionality can be achieved by
-setting the ``block-incremental`` migration parameter to ``true``.
-But this parameter is also deprecated.
-
 ``blk`` migrate command option (since 8.2)
 ''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 9873f59bee..e62fc760f1 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -620,6 +620,13 @@ was superseded by ``sections``.
 Member ``skipped`` of the ``MigrationStats`` struct hasn't been used
 for more than 10 years. Removed with no replacement.
 
+``migrate`` command option ``inc`` (removed in 9.1)
+'''
+
+Use blockdev-mirror with NBD instead. See "QMP invocation for live
+storage migration with ``blockdev-mirror`` + NBD" in
+docs/interop/live-block-operations.rst for a detailed explanation.
+
 Human Monitor Protocol (HMP) commands
 -
 
@@ -680,6 +687,13 @@ This command didn't produce any output already. Removed 
with no replacement.
 The ``singlestep`` command has been replaced by the ``one-insn-per-tb``
 command, which has the same behaviour but a less misleading name.
 
+``migrate`` command ``-i`` option (removed in 9.1)
+''
+
+Use blockdev-mirror with NBD instead. See "QMP invocation for live
+storage migration with ``blockdev-mirror`` + NBD" in
+docs/interop/live-block-operations.rst for a detailed explanation.
+
 Host Architectures
 --
 
diff --git a/migration/block.c b/migration/block.c
index bae6e94891..87ec1a7e68 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -419,7 +419,6 @@ static int init_blk_migration(QEMUFile *f, Error **errp)
 bmds->bulk_completed = 0;
 bmds->total_sectors = sectors;
 bmds->completed_sectors = 0;
-bmds->shared_base = migrate_block_incremental();
 
 assert(i < num_bs);
 bmds_bs[i].bmds = bmds;
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 28f776d06d..f49f061be1 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -332,10 +332,6 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict 
*qdict)
 monitor_printf(mon, "%s: %u ms\n",
 MigrationParameter_str(MIGRATION_PARAMETER_X_CHECKPOINT_DELAY),
 params->x_checkpoint_delay);
-assert(params->has_block_incremental);
-monitor_printf(mon, "%s: %s\n",
-MigrationParameter_str(MIGRATION_PARAMETER_BLOCK_INCREMENTAL),
-params->block_incremental ? "on" : "off");
 monitor_printf(mon, "%s: %u\n",
 MigrationParameter_str(MIGRATION_PARAMETER_MULTIFD_CHANNELS),
 params->multifd_channels);
@@ -616,10 +612,6 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict 
*qdict)
 p->has_x_checkpoint_delay = true;
 visit_type_uint32(v, param, &p->x_checkpoint_delay, &err);
 break;
-case MIGRATION_PARAMETER_BLOCK_INCREMENTAL:
-p->has_block_incremental = true;
-visit_type_bool(v, param, &p->block_incremental, &err);
-break;
 case MIGRATION_PARAMETER_MULTIFD_CHANNELS:
 p->has_multifd_channels = true;
 visit_type_uint8(v, param, &p->multifd_channels, &err);
@@ -767,18 +759,12 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
 {
 bool detach = qdict_get_try_bool(qdict, "detach", false);
 bool blk = qdict_get_try_bool(qdict, "blk", false);
-bool inc = qdict_get_try_bool(qdict, "inc", false);
 bool resume = qdict_get_try_bool(qdict, "resume", false);
 const char *uri = qdict_get_str(qdict, "uri");
 Error *err = NULL;
 g_autoptr(MigrationChannelList) caps = NULL;
 g_autoptr(MigrationChannel) channel = NULL;
 
-if (inc) {
-warn_report("option '-i' is deprecated;"
-