diff mbox series

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

Message ID 20240425150939.19268-3-farosas@suse.de (mailing list archive)
State New, archived
Headers show
Series migration removals & deprecations | expand

Commit Message

Fabiano Rosas April 25, 2024, 3:09 p.m. UTC
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 <farosas@suse.de>
---
 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(-)

Comments

Markus Armbruster April 26, 2024, 1:25 p.m. UTC | #1
Fabiano Rosas <farosas@suse.de> 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 <farosas@suse.de>
> ---
>  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 <armbru@redhat.com>
Markus Armbruster April 26, 2024, 2:07 p.m. UTC | #2
Markus Armbruster <armbru@redhat.com> writes:

> Fabiano Rosas <farosas@suse.de> 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 <farosas@suse.de>

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

Patch

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;"
-                    " use blockdev-mirror with NBD instead");
-    }
-
     if (blk) {
         warn_report("option '-b' is deprecated;"
                     " use blockdev-mirror with NBD instead");
@@ -790,8 +776,8 @@  void hmp_migrate(Monitor *mon, const QDict *qdict)
     }
     QAPI_LIST_PREPEND(caps, g_steal_pointer(&channel));
 
-    qmp_migrate(NULL, true, caps, !!blk, blk, !!inc, inc,
-                 false, false, true, resume, &err);
+    qmp_migrate(NULL, true, caps, !!blk, blk, false, false,
+                true, resume, &err);
     if (hmp_handle_error(mon, err)) {
         return;
     }
diff --git a/migration/migration.c b/migration/migration.c
index 3b433fdb31..be7a66c23c 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1911,14 +1911,9 @@  bool migration_is_blocked(Error **errp)
 }
 
 /* Returns true if continue to migrate, or false if error detected */
-static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
-                            bool resume, Error **errp)
+static bool migrate_prepare(MigrationState *s, bool blk, bool resume,
+                            Error **errp)
 {
-    if (blk_inc) {
-        warn_report("parameter 'inc' is deprecated;"
-                    " use blockdev-mirror with NBD instead");
-    }
-
     if (blk) {
         warn_report("parameter 'blk' is deprecated;"
                     " use blockdev-mirror with NBD instead");
@@ -2008,12 +2003,12 @@  static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
         }
     }
 
-    if (blk || blk_inc) {
+    if (blk) {
         if (migrate_colo()) {
             error_setg(errp, "No disk migration is required in COLO mode");
             return false;
         }
-        if (migrate_block() || migrate_block_incremental()) {
+        if (migrate_block()) {
             error_setg(errp, "Command options are incompatible with "
                        "current migration capabilities");
             return false;
@@ -2024,10 +2019,6 @@  static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
         s->must_remove_block_options = true;
     }
 
-    if (blk_inc) {
-        migrate_set_block_incremental(true);
-    }
-
     if (migrate_init(s, errp)) {
         return false;
     }
@@ -2037,8 +2028,8 @@  static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc,
 
 void qmp_migrate(const char *uri, bool has_channels,
                  MigrationChannelList *channels, bool has_blk, bool blk,
-                 bool has_inc, bool inc, bool has_detach, bool detach,
-                 bool has_resume, bool resume, Error **errp)
+                 bool has_detach, bool detach, bool has_resume, bool resume,
+                 Error **errp)
 {
     bool resume_requested;
     Error *local_err = NULL;
@@ -2077,8 +2068,7 @@  void qmp_migrate(const char *uri, bool has_channels,
     }
 
     resume_requested = has_resume && resume;
-    if (!migrate_prepare(s, has_blk && blk, has_inc && inc,
-                         resume_requested, errp)) {
+    if (!migrate_prepare(s, has_blk && blk, resume_requested, errp)) {
         /* Error detected, put into errp */
         return;
     }
diff --git a/migration/options.c b/migration/options.c
index bfd7753b69..25d28ada60 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -486,7 +486,7 @@  bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
 
 #ifndef CONFIG_LIVE_BLOCK_MIGRATION
     if (new_caps[MIGRATION_CAPABILITY_BLOCK]) {
-        error_setg(errp, "QEMU compiled without old-style (blk/-b, inc/-i) "
+        error_setg(errp, "QEMU compiled without old-style (blk/-b) "
                    "block migration");
         error_append_hint(errp, "Use blockdev-mirror with NBD instead.\n");
         return false;
@@ -763,13 +763,6 @@  bool migrate_has_block_bitmap_mapping(void)
     return s->parameters.has_block_bitmap_mapping;
 }
 
-bool migrate_block_incremental(void)
-{
-    MigrationState *s = migrate_get_current();
-
-    return s->parameters.block_incremental;
-}
-
 uint32_t migrate_checkpoint_delay(void)
 {
     MigrationState *s = migrate_get_current();
@@ -948,15 +941,6 @@  ZeroPageDetection migrate_zero_page_detection(void)
     return s->parameters.zero_page_detection;
 }
 
-/* parameter setters */
-
-void migrate_set_block_incremental(bool value)
-{
-    MigrationState *s = migrate_get_current();
-
-    s->parameters.block_incremental = value;
-}
-
 /* parameters helpers */
 
 void block_cleanup_parameters(void)
@@ -966,7 +950,6 @@  void block_cleanup_parameters(void)
     if (s->must_remove_block_options) {
         /* setting to false can never fail */
         migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, false, &error_abort);
-        migrate_set_block_incremental(false);
         s->must_remove_block_options = false;
     }
 }
@@ -1020,8 +1003,6 @@  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;
     params->has_multifd_channels = true;
     params->multifd_channels = s->parameters.multifd_channels;
     params->has_multifd_compression = true;
@@ -1081,7 +1062,6 @@  void migrate_params_init(MigrationParameters *params)
     params->has_max_bandwidth = true;
     params->has_downtime_limit = true;
     params->has_x_checkpoint_delay = true;
-    params->has_block_incremental = true;
     params->has_multifd_channels = true;
     params->has_multifd_compression = true;
     params->has_multifd_zlib_level = true;
@@ -1360,9 +1340,6 @@  static void migrate_params_test_apply(MigrateSetParameters *params,
         dest->x_checkpoint_delay = params->x_checkpoint_delay;
     }
 
-    if (params->has_block_incremental) {
-        dest->block_incremental = params->block_incremental;
-    }
     if (params->has_multifd_channels) {
         dest->multifd_channels = params->multifd_channels;
     }
@@ -1503,11 +1480,6 @@  static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
         colo_checkpoint_delay_set();
     }
 
-    if (params->has_block_incremental) {
-        warn_report("block migration is deprecated;"
-                    " use blockdev-mirror with NBD instead");
-        s->parameters.block_incremental = params->block_incremental;
-    }
     if (params->has_multifd_channels) {
         s->parameters.multifd_channels = params->multifd_channels;
     }
diff --git a/qapi/migration.json b/qapi/migration.json
index 401b8e24ac..a23c162ad8 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -839,13 +839,6 @@ 
 # @x-checkpoint-delay: The delay time (in ms) between two COLO
 #     checkpoints in periodic mode.  (Since 2.8)
 #
-# @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)
-#
 # @multifd-channels: Number of channels used to migrate data in
 #     parallel.  This is the same number that the number of sockets
 #     used for migration.  The default value is 2 (since 4.0)
@@ -908,10 +901,9 @@ 
 #
 # Features:
 #
-# @deprecated: Member @block-incremental is deprecated.  Use
-#     blockdev-mirror with NBD instead.  Members @compress-level,
-#     @compress-threads, @decompress-threads and @compress-wait-thread
-#     are deprecated because @compression is deprecated.
+# @deprecated: Members @compress-level, @compress-threads,
+#     @decompress-threads and @compress-wait-thread are deprecated
+#     because @compression is deprecated.
 #
 # @unstable: Members @x-checkpoint-delay and
 #     @x-vcpu-dirty-limit-period are experimental.
@@ -931,7 +923,6 @@ 
            'tls-creds', 'tls-hostname', 'tls-authz', 'max-bandwidth',
            'avail-switchover-bandwidth', 'downtime-limit',
            { 'name': 'x-checkpoint-delay', 'features': [ 'unstable' ] },
-           { 'name': 'block-incremental', 'features': [ 'deprecated' ] },
            'multifd-channels',
            'xbzrle-cache-size', 'max-postcopy-bandwidth',
            'max-cpu-throttle', 'multifd-compression',
@@ -1047,13 +1038,6 @@ 
 # @x-checkpoint-delay: The delay time (in ms) between two COLO
 #     checkpoints in periodic mode.  (Since 2.8)
 #
-# @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)
-#
 # @multifd-channels: Number of channels used to migrate data in
 #     parallel.  This is the same number that the number of sockets
 #     used for migration.  The default value is 2 (since 4.0)
@@ -1116,10 +1100,9 @@ 
 #
 # Features:
 #
-# @deprecated: Member @block-incremental is deprecated.  Use
-#     blockdev-mirror with NBD instead.  Members @compress-level,
-#     @compress-threads, @decompress-threads and @compress-wait-thread
-#     are deprecated because @compression is deprecated.
+# @deprecated: Members @compress-level, @compress-threads,
+#     @decompress-threads and @compress-wait-thread are deprecated
+#     because @compression is deprecated.
 #
 # @unstable: Members @x-checkpoint-delay and
 #     @x-vcpu-dirty-limit-period are experimental.
@@ -1154,8 +1137,6 @@ 
             '*downtime-limit': 'uint64',
             '*x-checkpoint-delay': { 'type': 'uint32',
                                      'features': [ 'unstable' ] },
-            '*block-incremental': { 'type': 'bool',
-                                    'features': [ 'deprecated' ] },
             '*multifd-channels': 'uint8',
             '*xbzrle-cache-size': 'size',
             '*max-postcopy-bandwidth': 'size',
@@ -1279,13 +1260,6 @@ 
 # @x-checkpoint-delay: the delay time between two COLO checkpoints.
 #     (Since 2.8)
 #
-# @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)
-#
 # @multifd-channels: Number of channels used to migrate data in
 #     parallel.  This is the same number that the number of sockets
 #     used for migration.  The default value is 2 (since 4.0)
@@ -1348,10 +1322,9 @@ 
 #
 # Features:
 #
-# @deprecated: Member @block-incremental is deprecated.  Use
-#     blockdev-mirror with NBD instead.  Members @compress-level,
-#     @compress-threads, @decompress-threads and @compress-wait-thread
-#     are deprecated because @compression is deprecated.
+# @deprecated: Members @compress-level, @compress-threads,
+#     @decompress-threads and @compress-wait-thread are deprecated
+#     because @compression is deprecated.
 #
 # @unstable: Members @x-checkpoint-delay and
 #     @x-vcpu-dirty-limit-period are experimental.
@@ -1383,8 +1356,6 @@ 
             '*downtime-limit': 'uint64',
             '*x-checkpoint-delay': { 'type': 'uint32',
                                      'features': [ 'unstable' ] },
-            '*block-incremental': { 'type': 'bool',
-                                    'features': [ 'deprecated' ] },
             '*multifd-channels': 'uint8',
             '*xbzrle-cache-size': 'size',
             '*max-postcopy-bandwidth': 'size',
@@ -1736,8 +1707,6 @@ 
 #
 # @blk: do block migration (full disk copy)
 #
-# @inc: incremental disk copy migration
-#
 # @detach: this argument exists only for compatibility reasons and is
 #     ignored by QEMU
 #
@@ -1745,8 +1714,8 @@ 
 #
 # Features:
 #
-# @deprecated: Members @inc and @blk are deprecated.  Use
-#     blockdev-mirror with NBD instead.
+# @deprecated: Member @blk is deprecated.  Use blockdev-mirror with
+#     NBD instead.
 #
 # Since: 0.14
 #
@@ -1814,7 +1783,6 @@ 
   'data': {'*uri': 'str',
            '*channels': [ 'MigrationChannel' ],
            '*blk': { 'type': 'bool', 'features': [ 'deprecated' ] },
-           '*inc': { 'type': 'bool', 'features': [ 'deprecated' ] },
            '*detach': 'bool', '*resume': 'bool' } }
 
 ##