Re: [PATCH-for-9.1] rdma: Remove RDMA subsystem and pvrdma device

2024-03-27 Thread Fabiano Rosas
Philippe Mathieu-Daudé  writes:

> The whole RDMA subsystem was deprecated in commit e9a54265f5
> ("hw/rdma: Deprecate the pvrdma device and the rdma subsystem")
> released in v8.2. Time to remove it.
>
> Keep the RAM_SAVE_FLAG_HOOK definition since it might appears
> in old migration streams.
>
> Remove the dependencies on libibumad and libibverbs.
>
> Remove the generated vmw_pvrdma/ directory from linux-headers.
>
> Remove RDMA handling from migration.
>
> Remove RDMA handling in GlusterFS block driver.
>
> Remove rdmacm-mux tool from contrib/.
>
> Remove PVRDMA device.
>
> Cc: Peter Xu 
> Cc: Li Zhijian 
> Cc: Yuval Shaia 
> Cc: Marcel Apfelbaum 
> Signed-off-by: Philippe Mathieu-Daudé 

Acked-by: Fabiano Rosas 
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Re: [PATCH-for-9.1 v2 2/3] migration: Remove RDMA protocol handling

2024-03-28 Thread Fabiano Rosas
Philippe Mathieu-Daudé  writes:

> The whole RDMA subsystem was deprecated in commit e9a54265f5
> ("hw/rdma: Deprecate the pvrdma device and the rdma subsystem")
> released in v8.2.
>
> Remove:
>  - RDMA handling from migration
>  - dependencies on libibumad, libibverbs and librdmacm
>
> Keep the RAM_SAVE_FLAG_HOOK definition since it might appears
> in old migration streams.
>
> Cc: Peter Xu 
> Cc: Li Zhijian 
> Acked-by: Fabiano Rosas 
> Signed-off-by: Philippe Mathieu-Daudé 

Just to be clear, because people raised the point in the last version,
the first link in the deprecation commit links to a thread comprising
entirely of rdma migration patches. I don't see any ambiguity on whether
the deprecation was intended to include migration. There's even an ack
from Juan.

So on the basis of not reverting the previous maintainer's decision, my
Ack stands here.

We also had pretty obvious bugs ([1], [2]) in the past that would have
been caught if we had any kind of testing for the feature, so I can't
even say this thing works currently.

@Peter Xu, @Li Zhijian, what are your thoughts on this?

1- https://lore.kernel.org/r/20230920090412.726725-1-lizhij...@fujitsu.com
2- 
https://lore.kernel.org/r/cahecvy7hxswn4ow_kog+q+tn6f_kmeichevz1qgm-fbxbpp...@mail.gmail.com
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 0/6] migration removals & deprecations

2024-04-25 Thread Fabiano Rosas
Hi everyone,

Here's some cleaning up of deprecated code. It removes the old block
migration and compression code. Both have suitable replacements in the
form of the blockdev-mirror driver and multifd compression,
respectively.

There's also a deprecation for fd: + file to cope with the fact that
the new MigrationAddress API defines transports instead of protocols
(loose terms) like the old string API did. So we cannot map 1:1 from
fd: to any transport because fd: allows *both* file migration and
socket migration.

CI run: https://gitlab.com/farosas/qemu/-/pipelines/1267859704

Fabiano Rosas (6):
  migration: Remove 'skipped' field from MigrationStats
  migration: Remove 'inc' option from migrate command
  migration: Remove 'blk/-b' option from migrate commands
  migration: Remove block migration
  migration: Remove non-multifd compression
  migration: Deprecate fd: for file migration

 .gitlab-ci.d/buildtest.yml   |2 +-
 MAINTAINERS  |1 -
 docs/about/deprecated.rst|   51 +-
 docs/about/removed-features.rst  |  104 ++-
 docs/devel/migration/main.rst|2 +-
 hw/core/machine.c|1 -
 include/migration/misc.h |6 -
 meson.build  |2 -
 meson_options.txt|2 -
 migration/block.c| 1019 --
 migration/block.h|   52 --
 migration/colo.c |1 -
 migration/meson.build|4 -
 migration/migration-hmp-cmds.c   |   97 +--
 migration/migration.c|   70 +-
 migration/migration.h|7 -
 migration/options.c  |  229 ---
 migration/ram-compress.c |  564 -
 migration/ram.c  |  166 +
 migration/savevm.c   |5 -
 qapi/migration.json  |  205 +-
 scripts/meson-buildoptions.sh|4 -
 tests/qemu-iotests/183   |  147 -
 tests/qemu-iotests/183.out   |   66 --
 tests/qemu-iotests/common.filter |7 -
 tests/qtest/migration-test.c |  139 
 26 files changed, 130 insertions(+), 2823 deletions(-)
 delete mode 100644 migration/block.c
 delete mode 100644 migration/block.h
 delete mode 100644 migration/ram-compress.c
 delete mode 100755 tests/qemu-iotests/183
 delete mode 100644 tests/qemu-iotests/183.out

-- 
2.35.3
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 1/6] migration: Remove 'skipped' field from MigrationStats

2024-04-25 Thread Fabiano Rosas
The 'skipped' field of the MigrationStats struct has been deprecated
in 8.1. Time to remove it.

Deprecation commit 7b24d32634 ("migration: skipped field is really
obsolete.").

Signed-off-by: Fabiano Rosas 
---
 docs/about/deprecated.rst   | 6 --
 docs/about/removed-features.rst | 6 ++
 migration/migration-hmp-cmds.c  | 2 --
 migration/migration.c   | 2 --
 qapi/migration.json | 8 
 5 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 7b548519b5..4d9d6bf2da 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -488,12 +488,6 @@ option).
 Migration
 -
 
-``skipped`` MigrationStats field (since 8.1)
-''''''''''''''''''''''''''''''''''''''''''''
-
-``skipped`` field in Migration stats has been deprecated.  It hasn't
-been used for more than 10 years.
-
 ``inc`` migrate command option (since 8.2)
 ''''''''''''''''''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index f9cf874f7b..9873f59bee 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -614,6 +614,12 @@ was superseded by ``sections``.
 Member ``section-size`` in the return value of ``query-sgx-capabilities``
 was superseded by ``sections``.
 
+``query-migrate`` return value member ``skipped`` (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Member ``skipped`` of the ``MigrationStats`` struct hasn't been used
+for more than 10 years. Removed with no replacement.
+
 Human Monitor Protocol (HMP) commands
 -
 
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 7e96ae6ffd..28f776d06d 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -105,8 +105,6 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
info->ram->total >> 10);
 monitor_printf(mon, "duplicate: %" PRIu64 " pages\n",
info->ram->duplicate);
-monitor_printf(mon, "skipped: %" PRIu64 " pages\n",
-   info->ram->skipped);
 monitor_printf(mon, "normal: %" PRIu64 " pages\n",
info->ram->normal);
 monitor_printf(mon, "normal bytes: %" PRIu64 " kbytes\n",
diff --git a/migration/migration.c b/migration/migration.c
index 696762bc64..3b433fdb31 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1149,8 +1149,6 @@ static void populate_ram_info(MigrationInfo *info, 
MigrationState *s)
 info->ram->transferred = migration_transferred_bytes();
 info->ram->total = ram_bytes_total();
 info->ram->duplicate = stat64_get(&mig_stats.zero_pages);
-/* legacy value.  It is not used anymore */
-info->ram->skipped = 0;
 info->ram->normal = stat64_get(&mig_stats.normal_pages);
 info->ram->normal_bytes = info->ram->normal * page_size;
 info->ram->mbps = s->mbps;
diff --git a/qapi/migration.json b/qapi/migration.json
index 8c65b90328..401b8e24ac 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -23,9 +23,6 @@
 #
 # @duplicate: number of duplicate (zero) pages (since 1.2)
 #
-# @skipped: number of skipped zero pages.  Always zero, only provided
-# for compatibility (since 1.5)
-#
 # @normal: number of normal pages (since 1.2)
 #
 # @normal-bytes: number of normal bytes sent (since 1.2)
@@ -63,16 +60,11 @@
 # between 0 and @dirty-sync-count * @multifd-channels.  (since
 # 7.1)
 #
-# Features:
-#
-# @deprecated: Member @skipped is always zero since 1.5.3
-#
 # Since: 0.14
 ##
 { 'struct': 'MigrationStats',
   'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
'duplicate': 'int',
-   'skipped': { 'type': 'int', 'features': [ 'deprecated' ] },
'normal': 'int',
'normal-bytes': 'int', 'dirty-pages-rate': 'int',
'mbps': 'number', 'dirty-sync-count': 'int',
-- 
2.35.3
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[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_pa

[PATCH 3/6] migration: Remove 'blk/-b' option from migrate commands

2024-04-25 Thread Fabiano Rosas
The block migration is considered obsolete and has been deprecated in
8.2. Remove the migrate command option that enables it. This only
affects the QMP and HMP commands, the feature can still be accessed by
setting the migration 'block' capability. The whole feature will be
removed in a future patch.

Deprecation commit 8846b5bfca ("migration: migrate 'blk' command
option is deprecated.").

Signed-off-by: Fabiano Rosas 
---
 .gitlab-ci.d/buildtest.yml   |   2 +-
 docs/about/deprecated.rst|   9 --
 docs/about/removed-features.rst  |  15 +++-
 migration/migration-hmp-cmds.c   |   9 +-
 migration/migration.c|  31 ++-
 migration/options.c  |   3 +-
 qapi/migration.json  |   8 --
 tests/qemu-iotests/183   | 147 ---
 tests/qemu-iotests/183.out   |  66 --
 tests/qemu-iotests/common.filter |   7 --
 10 files changed, 22 insertions(+), 275 deletions(-)
 delete mode 100755 tests/qemu-iotests/183
 delete mode 100644 tests/qemu-iotests/183.out

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index cfdff175c3..b1a536592f 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -342,7 +342,7 @@ build-tcg-disabled:
 - cd tests/qemu-iotests/
 - ./check -raw 001 002 003 004 005 008 009 010 011 012 021 025 032 033 048
 052 063 077 086 101 104 106 113 148 150 151 152 157 159 160 163
-170 171 183 184 192 194 208 221 226 227 236 253 277 image-fleecing
+170 171 184 192 194 208 221 226 227 236 253 277 image-fleecing
 - ./check -qcow2 028 051 056 057 058 065 068 082 085 091 095 096 102 122
 124 132 139 142 144 145 151 152 155 157 165 194 196 200 202
 208 209 216 218 227 234 246 247 248 250 254 255 257 258
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 25b309dde4..83adb763e6 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -488,15 +488,6 @@ option).
 Migration
 -
 
-``blk`` migrate command option (since 8.2)
-''''''''''''''''''''''''''''''''''''''''''
-
-Use blockdev-mirror with NBD instead.
-
-As an intermediate step the ``blk`` functionality can be achieved by
-setting the ``block`` migration capability to ``true``.  But this
-capability is also deprecated.
-
 block migration (since 8.2)
 '''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index e62fc760f1..0ba94c826a 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -627,6 +627,13 @@ 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.
 
+``migrate`` command option ``blk`` (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
 -
 
@@ -694,6 +701,13 @@ 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.
 
+``migrate`` command ``-b`` 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
 --
 
@@ -1025,4 +1039,3 @@ There is a newer Rust implementation of ``virtiofsd`` at
 stable for some time and is now widely used.
 The command line and feature set is very close to the removed
 C implementation.
-
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index f49f061be1..5ab204d91d 100644
--- a/mi

[PATCH 6/6] migration: Deprecate fd: for file migration

2024-04-25 Thread Fabiano Rosas
The fd: URI can currently trigger two different types of migration, a
TCP migration using sockets and a file migration using a plain
file. This is in conflict with the recently introduced (8.2) QMP
migrate API that takes structured data as JSON-like format. We cannot
keep the same backend for both types of migration because with the new
API the code is more tightly coupled to the type of transport. This
means a TCP migration must use the 'socket' transport and a file
migration must use the 'file' transport.

If we keep allowing fd: when using a file, this creates an issue when
the user converts the old-style (fd:) to the new style ("transport":
"socket") invocation because the file descriptor in question has
previously been allowed to be either a plain file or a socket.

To avoid creating too much confusion, we can simply deprecate the fd:
+ file usage, which is thought to be rarely used currently and instead
establish a 1:1 correspondence between fd: URI and socket transport,
and file: URI and file transport.

Signed-off-by: Fabiano Rosas 
---
 docs/about/deprecated.rst | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index fadb85f289..11dce324c8 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -484,3 +484,17 @@ both, older and future versions of QEMU.
 The ``blacklist`` config file option has been renamed to ``block-rpcs``
 (to be in sync with the renaming of the corresponding command line
 option).
+
+Migration
+-
+
+``fd:`` URI when used for file migration (since 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+The ``fd:`` URI can currently provide a file descriptor that
+references either a socket or a plain file. These are two different
+types of migration. In order to reduce ambiguity, the ``fd:`` URI
+usage of providing a file descriptor to a plain file has been
+deprecated in favor of explicitly using the ``file:`` URI with the
+file descriptor being passed as an ``fdset``. Refer to the ``add-fd``
+command documentation for details on the ``fdset`` usage.
-- 
2.35.3
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH 4/6] migration: Remove block migration

2024-04-25 Thread Fabiano Rosas
The block migration has been considered obsolete since QEMU 8.2 in
favor of the more flexible storage migration provided by the
blockdev-mirror driver. Two releases have passed so now it's time to
remove it.

Deprecation commit 66db46ca83 ("migration: Deprecate block
migration").

Signed-off-by: Fabiano Rosas 
---
 MAINTAINERS |1 -
 docs/about/deprecated.rst   |   10 -
 docs/about/removed-features.rst |   14 +
 docs/devel/migration/main.rst   |2 +-
 include/migration/misc.h|6 -
 meson.build |2 -
 meson_options.txt   |2 -
 migration/block.c   | 1018 ---
 migration/block.h   |   52 --
 migration/colo.c|1 -
 migration/meson.build   |3 -
 migration/migration-hmp-cmds.c  |   25 -
 migration/migration.c   |   17 -
 migration/options.c |   36 --
 migration/ram.c |   15 -
 migration/savevm.c  |5 -
 qapi/migration.json |   61 +-
 scripts/meson-buildoptions.sh   |4 -
 18 files changed, 26 insertions(+), 1248 deletions(-)
 delete mode 100644 migration/block.c
 delete mode 100644 migration/block.h

diff --git a/MAINTAINERS b/MAINTAINERS
index f1f6922025..9a53ac9ec2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2877,7 +2877,6 @@ F: util/aio-*.h
 F: util/defer-call.c
 F: util/fdmon-*.c
 F: block/io.c
-F: migration/block*
 F: include/block/aio.h
 F: include/block/aio-wait.h
 F: include/qemu/defer-call.h
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 83adb763e6..409c9e0c4d 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -488,16 +488,6 @@ option).
 Migration
 -
 
-block migration (since 8.2)
-'''''''''''''''''''''''''''
-
-Block migration is too inflexible.  It needs to migrate all block
-devices or none.
-
-Please see "QMP invocation for live storage migration with
-``blockdev-mirror`` + NBD" in docs/interop/live-block-operations.rst
-for a detailed explanation.
-
 old compression method (since 8.2)
 ''''''''''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 0ba94c826a..dd20f37f2c 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -634,6 +634,13 @@ 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.
 
+``migrate-set-capabilities`` ``block`` option (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Block migration has been removed. For a replacement, see "QMP
+invocation for live storage migration with ``blockdev-mirror`` + NBD"
+in docs/interop/live-block-operations.rst.
+
 Human Monitor Protocol (HMP) commands
 -
 
@@ -708,6 +715,13 @@ 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.
 
+``migrate_set_capability`` ``block`` option (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Block migration has been removed. For a replacement, see "QMP
+invocation for live storage migration with ``blockdev-mirror`` + NBD"
+in docs/interop/live-block-operations.rst.
+
 Host Architectures
 --
 
diff --git a/docs/devel/migration/main.rst b/docs/devel/migration/main.rst
index 54385a23e5..495cdcb112 100644
--- a/docs/devel/migration/main.rst
+++ b/docs/devel/migration/main.rst
@@ -454,7 +454,7 @@ Examples of such API functions are:
 Iterative device migration
 --
 
-Some devices, such as RAM, Block storage or certain platform devices,
+Some devices, such as RAM or certain platform devices,
 have large amounts of data that would mean that the CPUs would be
 paused for too lon

[PATCH 5/6] migration: Remove non-multifd compression

2024-04-25 Thread Fabiano Rosas
The 'compress' migration capability enables the old compression code
which has shown issues over the years and is thought to be less stable
and tested than the more recent multifd-based compression. The old
compression code has been deprecated in 8.2 and now is time to remove
it.

Deprecation commit 864128df46 ("migration: Deprecate old compression
method").

Signed-off-by: Fabiano Rosas 
---
 docs/about/deprecated.rst   |  11 -
 docs/about/removed-features.rst |  55 
 hw/core/machine.c   |   1 -
 migration/meson.build   |   1 -
 migration/migration-hmp-cmds.c  |  47 ---
 migration/migration.c   |  14 -
 migration/migration.h   |   7 -
 migration/options.c | 164 --
 migration/ram-compress.c| 564 
 migration/ram.c | 151 +
 qapi/migration.json | 112 ---
 tests/qtest/migration-test.c| 139 
 12 files changed, 63 insertions(+), 1203 deletions(-)
 delete mode 100644 migration/ram-compress.c

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 409c9e0c4d..fadb85f289 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -484,14 +484,3 @@ both, older and future versions of QEMU.
 The ``blacklist`` config file option has been renamed to ``block-rpcs``
 (to be in sync with the renaming of the corresponding command line
 option).
-
-Migration
--
-
-old compression method (since 8.2)
-''''''''''''''''''''''''''''''''''
-
-Compression method fails too much.  Too many races.  We are going to
-remove it if nobody fixes it.  For starters, migration-test
-compression tests are disabled because they fail randomly.  If you need
-compression, use multifd compression methods.
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index dd20f37f2c..783f6b479d 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -505,6 +505,11 @@ configurations (e.g. -smp 8,sockets=0) is removed since 
9.0, users have
 to ensure that all the topology members described with -smp are greater
 than zero.
 
+``-global migration.decompress-error-check`` (removed in 9.1)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Removed along with the ``compression`` migration capability.
+
 User-mode emulator command line arguments
 -
 
@@ -641,6 +646,31 @@ Block migration has been removed. For a replacement, see 
"QMP
 invocation for live storage migration with ``blockdev-mirror`` + NBD"
 in docs/interop/live-block-operations.rst.
 
+``migrate-set-parameter`` ``compress-level`` option (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use ``multifd-zlib-level`` or ``multifd-zstd-level`` instead.
+
+``migrate-set-parameter`` ``compress-threads`` option (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use ``multifd-channels`` instead.
+
+``migrate-set-parameter`` ``compress-wait-thread`` option (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Removed with no replacement.
+
+``migrate-set-parameter`` ``decompress-threads`` option (removed in 9.1)
+'''''''''''''''''''''&#

Re: [PATCH 0/6] migration removals & deprecations

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

> Doesn't apply for me.  What's your base?

88daa112d4 ("Merge tag 'migration-20240423-pull-request' of
https://gitlab.com/peterx/qemu into staging")

Probably clashed with the other removals from Philippe.
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Re: Revisiting parallel save/restore

2024-04-26 Thread Fabiano Rosas
Daniel P. Berrangé  writes:

> On Wed, Apr 17, 2024 at 05:12:27PM -0600, Jim Fehlig via Devel wrote:
>> A good starting point on this journey is supporting the new mapped-ram
>> capability in qemu 9.0 [2]. Since mapped-ram is a new on-disk format, I
>> assume we'll need a new QEMU_SAVE_VERSION 3 when using it? Otherwise I'm not
>> sure how to detect if a saved image is in mapped-ram format vs the existing,
>> sequential stream format.
>
> Yes, we'll need to be supporting 'mapped-ram', so a good first step.
>
> A question is whether we make that feature mandatory for all save images,
> or implied by another feature (parallel save), or an directly controllable
> feature with opt-in.
>
> The former breaks back compat with existnig libvirt, while the latter 2
> options are net new so don't have compat implications.
>
> In terms of actual data blocks written on disk mapped-ram should be be the
> same size, or smaller, than the existing format.
>
> In terms of logical file size, however, mapped-ram will almost always be
> larger.
>
> This is because mapped-ram will result in a file whose logical size matches
> the guest RAM size, plus some header overhead, while being sparse so not
> all blocks are written.
>
> If tools handling save images aren't sparse-aware this could come across
> as a surprise and even be considered a regression.
>
> Mapped ram is needed for parallel saves since it lets each thread write
> to a specific region of the file.
>
> Mapped ram is good for non-parallel saves too though, because the mapping
> of RAM into the file is aligned suitably to allow for O_DIRECT to be used.
> Currently libvirt has to tunnnel over its iohelper to futz alignment
> needed for O_DIRECT. This makes it desirable to use in general, but back
> compat hurts...

Note that QEMU doesn't support O_DIRECT without multifd.

From mapped-ram patch series v4:

- Dropped support for direct-io with fixed-ram _without_ multifd. This
  is something I said I would do for this version, but I had to drop
  it because performance is really bad. I think the single-threaded
  precopy code cannot cope with the extra latency/synchronicity of
  O_DIRECT.
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH v2 0/6] migration removals & deprecations

2024-04-26 Thread Fabiano Rosas
[respinning because master moved and there were conflicts]

Hi everyone,

Here's some cleaning up of deprecated code. It removes the old block
migration and compression code. Both have suitable replacements in the
form of the blockdev-mirror driver and multifd compression,
respectively.

There's also a deprecation for fd: + file to cope with the fact that
the new MigrationAddress API defines transports instead of protocols
(loose terms) like the old string API did. So we cannot map 1:1 from
fd: to any transport because fd: allows *both* file migration and
socket migration.

v1:
https://lore.kernel.org/r/20240425150939.19268-1-faro...@suse.de

Fabiano Rosas (6):
  migration: Remove 'skipped' field from MigrationStats
  migration: Remove 'inc' option from migrate command
  migration: Remove 'blk/-b' option from migrate commands
  migration: Remove block migration
  migration: Remove non-multifd compression
  migration: Deprecate fd: for file migration

 .gitlab-ci.d/buildtest.yml   |2 +-
 MAINTAINERS  |1 -
 docs/about/deprecated.rst|   51 +-
 docs/about/removed-features.rst  |  103 +++
 docs/devel/migration/main.rst|2 +-
 hw/core/machine.c|1 -
 include/migration/misc.h |6 -
 meson.build  |2 -
 meson_options.txt|2 -
 migration/block.c| 1019 --
 migration/block.h|   52 --
 migration/colo.c |1 -
 migration/meson.build|4 -
 migration/migration-hmp-cmds.c   |   97 +--
 migration/migration.c|   70 +-
 migration/migration.h|7 -
 migration/options.c  |  229 ---
 migration/ram-compress.c |  564 -
 migration/ram.c  |  166 +
 migration/savevm.c   |5 -
 qapi/migration.json  |  205 +-
 scripts/meson-buildoptions.sh|4 -
 tests/qemu-iotests/183   |  147 -
 tests/qemu-iotests/183.out   |   66 --
 tests/qemu-iotests/common.filter |7 -
 tests/qtest/migration-test.c |  139 
 26 files changed, 130 insertions(+), 2822 deletions(-)
 delete mode 100644 migration/block.c
 delete mode 100644 migration/block.h
 delete mode 100644 migration/ram-compress.c
 delete mode 100755 tests/qemu-iotests/183
 delete mode 100644 tests/qemu-iotests/183.out


base-commit: a118c4aff4087eafb68f7132b233ad548cf16376
-- 
2.35.3
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH v2 1/6] migration: Remove 'skipped' field from MigrationStats

2024-04-26 Thread Fabiano Rosas
The 'skipped' field of the MigrationStats struct has been deprecated
in 8.1. Time to remove it.

Deprecation commit 7b24d32634 ("migration: skipped field is really
obsolete.").

Reviewed-by: Markus Armbruster 
Signed-off-by: Fabiano Rosas 
---
 docs/about/deprecated.rst   | 6 --
 docs/about/removed-features.rst | 6 ++
 migration/migration-hmp-cmds.c  | 2 --
 migration/migration.c   | 2 --
 qapi/migration.json | 8 
 5 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 7b8aafa15b..1c03a358d1 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -468,12 +468,6 @@ option).
 Migration
 -
 
-``skipped`` MigrationStats field (since 8.1)
-''''''''''''''''''''''''''''''''''''''''''''
-
-``skipped`` field in Migration stats has been deprecated.  It hasn't
-been used for more than 10 years.
-
 ``inc`` migrate command option (since 8.2)
 ''''''''''''''''''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 53ca08aba9..c4cb2692d0 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -614,6 +614,12 @@ was superseded by ``sections``.
 Member ``section-size`` in the return value of ``query-sgx-capabilities``
 was superseded by ``sections``.
 
+``query-migrate`` return value member ``skipped`` (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Member ``skipped`` of the ``MigrationStats`` struct hasn't been used
+for more than 10 years. Removed with no replacement.
+
 Human Monitor Protocol (HMP) commands
 -
 
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 7e96ae6ffd..28f776d06d 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -105,8 +105,6 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
info->ram->total >> 10);
 monitor_printf(mon, "duplicate: %" PRIu64 " pages\n",
info->ram->duplicate);
-monitor_printf(mon, "skipped: %" PRIu64 " pages\n",
-   info->ram->skipped);
 monitor_printf(mon, "normal: %" PRIu64 " pages\n",
info->ram->normal);
 monitor_printf(mon, "normal bytes: %" PRIu64 " kbytes\n",
diff --git a/migration/migration.c b/migration/migration.c
index b5af6b5105..266652668d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1149,8 +1149,6 @@ static void populate_ram_info(MigrationInfo *info, 
MigrationState *s)
 info->ram->transferred = migration_transferred_bytes();
 info->ram->total = ram_bytes_total();
 info->ram->duplicate = stat64_get(&mig_stats.zero_pages);
-/* legacy value.  It is not used anymore */
-info->ram->skipped = 0;
 info->ram->normal = stat64_get(&mig_stats.normal_pages);
 info->ram->normal_bytes = info->ram->normal * page_size;
 info->ram->mbps = s->mbps;
diff --git a/qapi/migration.json b/qapi/migration.json
index 8c65b90328..401b8e24ac 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -23,9 +23,6 @@
 #
 # @duplicate: number of duplicate (zero) pages (since 1.2)
 #
-# @skipped: number of skipped zero pages.  Always zero, only provided
-# for compatibility (since 1.5)
-#
 # @normal: number of normal pages (since 1.2)
 #
 # @normal-bytes: number of normal bytes sent (since 1.2)
@@ -63,16 +60,11 @@
 # between 0 and @dirty-sync-count * @multifd-channels.  (since
 # 7.1)
 #
-# Features:
-#
-# @deprecated: Member @skipped is always zero since 1.5.3
-#
 # Since: 0.14
 ##
 { 'struct': 'MigrationStats',
   'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
'duplicate': 'int',
-   'skipped': { 'type': 'int', 'features': [ 'deprecated' ] },
'normal': 'int',
'normal-bytes': 'int', 'dirty-pages-rate': 'int',
'mbps': 'number', 'dirty-sync-count': 'int',
-- 
2.35.3
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


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

2024-04-26 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 1c03a358d1..ebe53821ed 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -468,15 +468,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 c4cb2692d0..7da4b3df14 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_pa

[PATCH v2 3/6] migration: Remove 'blk/-b' option from migrate commands

2024-04-26 Thread Fabiano Rosas
The block migration is considered obsolete and has been deprecated in
8.2. Remove the migrate command option that enables it. This only
affects the QMP and HMP commands, the feature can still be accessed by
setting the migration 'block' capability. The whole feature will be
removed in a future patch.

Deprecation commit 8846b5bfca ("migration: migrate 'blk' command
option is deprecated.").

Signed-off-by: Fabiano Rosas 
---
 .gitlab-ci.d/buildtest.yml   |   2 +-
 docs/about/deprecated.rst|   9 --
 docs/about/removed-features.rst  |  14 +++
 migration/migration-hmp-cmds.c   |   9 +-
 migration/migration.c|  31 ++-
 migration/options.c  |   3 +-
 qapi/migration.json  |   8 --
 tests/qemu-iotests/183   | 147 ---
 tests/qemu-iotests/183.out   |  66 --
 tests/qemu-iotests/common.filter |   7 --
 10 files changed, 22 insertions(+), 274 deletions(-)
 delete mode 100755 tests/qemu-iotests/183
 delete mode 100644 tests/qemu-iotests/183.out

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 6394b8f41e..3d975f8c34 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -342,7 +342,7 @@ build-tcg-disabled:
 - cd tests/qemu-iotests/
 - ./check -raw 001 002 003 004 005 008 009 010 011 012 021 025 032 033 048
 052 063 077 086 101 104 106 113 148 150 151 152 157 159 160 163
-170 171 183 184 192 194 208 221 226 227 236 253 277 image-fleecing
+170 171 184 192 194 208 221 226 227 236 253 277 image-fleecing
 - ./check -qcow2 028 051 056 057 058 065 068 082 085 091 095 096 102 122
 124 132 139 142 144 145 151 152 155 157 165 194 196 200 202
 208 209 216 218 227 234 246 247 248 250 254 255 257 258
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index ebe53821ed..d739358bb1 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -468,15 +468,6 @@ option).
 Migration
 -
 
-``blk`` migrate command option (since 8.2)
-''''''''''''''''''''''''''''''''''''''''''
-
-Use blockdev-mirror with NBD instead.
-
-As an intermediate step the ``blk`` functionality can be achieved by
-setting the ``block`` migration capability to ``true``.  But this
-capability is also deprecated.
-
 block migration (since 8.2)
 '''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 7da4b3df14..a491c0 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -627,6 +627,13 @@ 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.
 
+``migrate`` command option ``blk`` (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
 -
 
@@ -694,6 +701,13 @@ 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.
 
+``migrate`` command ``-b`` 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/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index f49f061be1..5ab204d91d 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -758,26 +758,19 @@ static void hmp_migrate_status_cb(void *opaque)
 void hmp_migrate(Monitor *mon, const QDict *qdict)
 {
 bool detach =

[PATCH v2 6/6] migration: Deprecate fd: for file migration

2024-04-26 Thread Fabiano Rosas
The fd: URI can currently trigger two different types of migration, a
TCP migration using sockets and a file migration using a plain
file. This is in conflict with the recently introduced (8.2) QMP
migrate API that takes structured data as JSON-like format. We cannot
keep the same backend for both types of migration because with the new
API the code is more tightly coupled to the type of transport. This
means a TCP migration must use the 'socket' transport and a file
migration must use the 'file' transport.

If we keep allowing fd: when using a file, this creates an issue when
the user converts the old-style (fd:) to the new style ("transport":
"socket") invocation because the file descriptor in question has
previously been allowed to be either a plain file or a socket.

To avoid creating too much confusion, we can simply deprecate the fd:
+ file usage, which is thought to be rarely used currently and instead
establish a 1:1 correspondence between fd: URI and socket transport,
and file: URI and file transport.

Signed-off-by: Fabiano Rosas 
---
 docs/about/deprecated.rst | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 0fb5c82640..813f7996fe 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -464,3 +464,17 @@ both, older and future versions of QEMU.
 The ``blacklist`` config file option has been renamed to ``block-rpcs``
 (to be in sync with the renaming of the corresponding command line
 option).
+
+Migration
+-
+
+``fd:`` URI when used for file migration (since 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+The ``fd:`` URI can currently provide a file descriptor that
+references either a socket or a plain file. These are two different
+types of migration. In order to reduce ambiguity, the ``fd:`` URI
+usage of providing a file descriptor to a plain file has been
+deprecated in favor of explicitly using the ``file:`` URI with the
+file descriptor being passed as an ``fdset``. Refer to the ``add-fd``
+command documentation for details on the ``fdset`` usage.
-- 
2.35.3
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH v2 4/6] migration: Remove block migration

2024-04-26 Thread Fabiano Rosas
The block migration has been considered obsolete since QEMU 8.2 in
favor of the more flexible storage migration provided by the
blockdev-mirror driver. Two releases have passed so now it's time to
remove it.

Deprecation commit 66db46ca83 ("migration: Deprecate block
migration").

Signed-off-by: Fabiano Rosas 
---
 MAINTAINERS |1 -
 docs/about/deprecated.rst   |   10 -
 docs/about/removed-features.rst |   14 +
 docs/devel/migration/main.rst   |2 +-
 include/migration/misc.h|6 -
 meson.build |2 -
 meson_options.txt   |2 -
 migration/block.c   | 1018 ---
 migration/block.h   |   52 --
 migration/colo.c|1 -
 migration/meson.build   |3 -
 migration/migration-hmp-cmds.c  |   25 -
 migration/migration.c   |   17 -
 migration/options.c |   36 --
 migration/ram.c |   15 -
 migration/savevm.c  |5 -
 qapi/migration.json |   61 +-
 scripts/meson-buildoptions.sh   |4 -
 18 files changed, 26 insertions(+), 1248 deletions(-)
 delete mode 100644 migration/block.c
 delete mode 100644 migration/block.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 96411e6adf..048d68ad5c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2865,7 +2865,6 @@ F: util/aio-*.h
 F: util/defer-call.c
 F: util/fdmon-*.c
 F: block/io.c
-F: migration/block*
 F: include/block/aio.h
 F: include/block/aio-wait.h
 F: include/qemu/defer-call.h
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index d739358bb1..3b9b5ba6ef 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -468,16 +468,6 @@ option).
 Migration
 -
 
-block migration (since 8.2)
-'''''''''''''''''''''''''''
-
-Block migration is too inflexible.  It needs to migrate all block
-devices or none.
-
-Please see "QMP invocation for live storage migration with
-``blockdev-mirror`` + NBD" in docs/interop/live-block-operations.rst
-for a detailed explanation.
-
 old compression method (since 8.2)
 ''''''''''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index a491c0..9e7d8ee4ff 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -634,6 +634,13 @@ 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.
 
+``migrate-set-capabilities`` ``block`` option (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Block migration has been removed. For a replacement, see "QMP
+invocation for live storage migration with ``blockdev-mirror`` + NBD"
+in docs/interop/live-block-operations.rst.
+
 Human Monitor Protocol (HMP) commands
 -
 
@@ -708,6 +715,13 @@ 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.
 
+``migrate_set_capability`` ``block`` option (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Block migration has been removed. For a replacement, see "QMP
+invocation for live storage migration with ``blockdev-mirror`` + NBD"
+in docs/interop/live-block-operations.rst.
+
 Host Architectures
 --
 
diff --git a/docs/devel/migration/main.rst b/docs/devel/migration/main.rst
index 54385a23e5..495cdcb112 100644
--- a/docs/devel/migration/main.rst
+++ b/docs/devel/migration/main.rst
@@ -454,7 +454,7 @@ Examples of such API functions are:
 Iterative device migration
 --
 
-Some devices, such as RAM, Block storage or certain platform devices,
+Some devices, such as RAM or certain platform devices,
 have large amounts of data that would mean that the CPUs would be
 paused f

[PATCH v2 5/6] migration: Remove non-multifd compression

2024-04-26 Thread Fabiano Rosas
The 'compress' migration capability enables the old compression code
which has shown issues over the years and is thought to be less stable
and tested than the more recent multifd-based compression. The old
compression code has been deprecated in 8.2 and now is time to remove
it.

Deprecation commit 864128df46 ("migration: Deprecate old compression
method").

Signed-off-by: Fabiano Rosas 
---
 docs/about/deprecated.rst   |  11 -
 docs/about/removed-features.rst |  55 
 hw/core/machine.c   |   1 -
 migration/meson.build   |   1 -
 migration/migration-hmp-cmds.c  |  47 ---
 migration/migration.c   |  14 -
 migration/migration.h   |   7 -
 migration/options.c | 164 --
 migration/ram-compress.c| 564 
 migration/ram.c | 151 +
 qapi/migration.json | 112 ---
 tests/qtest/migration-test.c| 139 
 12 files changed, 63 insertions(+), 1203 deletions(-)
 delete mode 100644 migration/ram-compress.c

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 3b9b5ba6ef..0fb5c82640 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -464,14 +464,3 @@ both, older and future versions of QEMU.
 The ``blacklist`` config file option has been renamed to ``block-rpcs``
 (to be in sync with the renaming of the corresponding command line
 option).
-
-Migration
--
-
-old compression method (since 8.2)
-''''''''''''''''''''''''''''''''''
-
-Compression method fails too much.  Too many races.  We are going to
-remove it if nobody fixes it.  For starters, migration-test
-compression tests are disabled because they fail randomly.  If you need
-compression, use multifd compression methods.
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 9e7d8ee4ff..fba0cfb0b0 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -505,6 +505,11 @@ configurations (e.g. -smp 8,sockets=0) is removed since 
9.0, users have
 to ensure that all the topology members described with -smp are greater
 than zero.
 
+``-global migration.decompress-error-check`` (removed in 9.1)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Removed along with the ``compression`` migration capability.
+
 User-mode emulator command line arguments
 -
 
@@ -641,6 +646,31 @@ Block migration has been removed. For a replacement, see 
"QMP
 invocation for live storage migration with ``blockdev-mirror`` + NBD"
 in docs/interop/live-block-operations.rst.
 
+``migrate-set-parameter`` ``compress-level`` option (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use ``multifd-zlib-level`` or ``multifd-zstd-level`` instead.
+
+``migrate-set-parameter`` ``compress-threads`` option (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use ``multifd-channels`` instead.
+
+``migrate-set-parameter`` ``compress-wait-thread`` option (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Removed with no replacement.
+
+``migrate-set-parameter`` ``decompress-threads`` option (removed in 9.1)
+'''''''''''''''''''''&#

Re: [PATCH 0/6] migration removals & deprecations

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

> Fabiano Rosas  writes:
>
>> Markus Armbruster  writes:
>>
>>> Doesn't apply for me.  What's your base?
>>
>> 88daa112d4 ("Merge tag 'migration-20240423-pull-request' of
>> https://gitlab.com/peterx/qemu into staging")
>>
>> Probably clashed with the other removals from Philippe.
>
> Thanks!

I sent a respin a moment ago.
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Re: [PATCH 3/6] migration: Remove 'blk/-b' option from migrate commands

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

> Markus Armbruster  writes:
>
>> Fabiano Rosas  writes:
>>
>>> The block migration is considered obsolete and has been deprecated in
>>> 8.2. Remove the migrate command option that enables it. This only
>>> affects the QMP and HMP commands, the feature can still be accessed by
>>> setting the migration 'block' capability. The whole feature will be
>>> removed in a future patch.
>>>
>>> Deprecation commit 8846b5bfca ("migration: migrate 'blk' command
>>> option is deprecated.").
>>>
>>> Signed-off-by: Fabiano Rosas 
>>
>> Reviewed-by: Markus Armbruster 
>
> I think you missed the update to hmp-commands.hx.  I almost missed it,
> too :)

Nice catch, it was bound to happen. There's approximately 9000 instances
of 'blk' in this project.
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Re: Revisiting parallel save/restore

2024-04-26 Thread Fabiano Rosas
Daniel P. Berrangé  writes:

> On Fri, Apr 26, 2024 at 10:03:29AM -0300, Fabiano Rosas wrote:
>> Daniel P. Berrangé  writes:
>> 
>> > On Wed, Apr 17, 2024 at 05:12:27PM -0600, Jim Fehlig via Devel wrote:
>> >> A good starting point on this journey is supporting the new mapped-ram
>> >> capability in qemu 9.0 [2]. Since mapped-ram is a new on-disk format, I
>> >> assume we'll need a new QEMU_SAVE_VERSION 3 when using it? Otherwise I'm 
>> >> not
>> >> sure how to detect if a saved image is in mapped-ram format vs the 
>> >> existing,
>> >> sequential stream format.
>> >
>> > Yes, we'll need to be supporting 'mapped-ram', so a good first step.
>> >
>> > A question is whether we make that feature mandatory for all save images,
>> > or implied by another feature (parallel save), or an directly controllable
>> > feature with opt-in.
>> >
>> > The former breaks back compat with existnig libvirt, while the latter 2
>> > options are net new so don't have compat implications.
>> >
>> > In terms of actual data blocks written on disk mapped-ram should be be the
>> > same size, or smaller, than the existing format.
>> >
>> > In terms of logical file size, however, mapped-ram will almost always be
>> > larger.
>> >
>> > This is because mapped-ram will result in a file whose logical size matches
>> > the guest RAM size, plus some header overhead, while being sparse so not
>> > all blocks are written.
>> >
>> > If tools handling save images aren't sparse-aware this could come across
>> > as a surprise and even be considered a regression.
>> >
>> > Mapped ram is needed for parallel saves since it lets each thread write
>> > to a specific region of the file.
>> >
>> > Mapped ram is good for non-parallel saves too though, because the mapping
>> > of RAM into the file is aligned suitably to allow for O_DIRECT to be used.
>> > Currently libvirt has to tunnnel over its iohelper to futz alignment
>> > needed for O_DIRECT. This makes it desirable to use in general, but back
>> > compat hurts...
>> 
>> Note that QEMU doesn't support O_DIRECT without multifd.
>> 
>> From mapped-ram patch series v4:
>> 
>> - Dropped support for direct-io with fixed-ram _without_ multifd. This
>>   is something I said I would do for this version, but I had to drop
>>   it because performance is really bad. I think the single-threaded
>>   precopy code cannot cope with the extra latency/synchronicity of
>>   O_DIRECT.
>
> Note the reason for using O_DIRECT is *not* to make saving / restoring
> the guest VM faster. Rather it is to ensure that saving/restoring a VM
> does not trash the host I/O / buffer cache, which will negatively impact
> performance of all the *other* concurrently running VMs.

Well, there's surely a performance degradation threshold that negates
the benefits of perserving the caches. But maybe it's not as low as I
initially thought then. 

The direct-io enablement is now posted to the qemu mailing list, please
take a look when you get the chance. I'll revisit the direct-io
no-parallel approach in the meantime, let's keep that option open for
now.

>
> With regards,
> Daniel
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Re: [PATCH v2 3/6] migration: Remove 'blk/-b' option from migrate commands

2024-04-29 Thread Fabiano Rosas
Peter Xu  writes:

> On Fri, Apr 26, 2024 at 10:14:05AM -0300, Fabiano Rosas wrote:
>> @@ -2003,21 +1997,7 @@ static bool migrate_prepare(MigrationState *s, bool 
>> blk, bool resume,
>>  }
>>  }
>>  
>> -if (blk) {
>> -if (migrate_colo()) {
>> -error_setg(errp, "No disk migration is required in COLO mode");
>> -return false;
>> -}
>> -if (migrate_block()) {
>> -error_setg(errp, "Command options are incompatible with "
>> -   "current migration capabilities");
>> -return false;
>> -}
>> -if (!migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, true, errp)) {
>> -return false;
>> -}
>> -s->must_remove_block_options = true;
>> -}
>> +s->must_remove_block_options = true;
>
> Can we drop this var too?  Perhaps with block_cleanup_parameters()?

Yes, Markus mentioned it in v1 already. Take a look there. There's
several other declarations I missed. v3 is coming soon.
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Re: [PATCH v2 3/6] migration: Remove 'blk/-b' option from migrate commands

2024-04-29 Thread Fabiano Rosas
Peter Xu  writes:

> On Mon, Apr 29, 2024 at 02:18:57PM -0300, Fabiano Rosas wrote:
>> Peter Xu  writes:
>> 
>> > On Fri, Apr 26, 2024 at 10:14:05AM -0300, Fabiano Rosas wrote:
>> >> @@ -2003,21 +1997,7 @@ static bool migrate_prepare(MigrationState *s, 
>> >> bool blk, bool resume,
>> >>  }
>> >>  }
>> >>  
>> >> -if (blk) {
>> >> -if (migrate_colo()) {
>> >> -error_setg(errp, "No disk migration is required in COLO 
>> >> mode");
>> >> -return false;
>> >> -}
>> >> -if (migrate_block()) {
>> >> -error_setg(errp, "Command options are incompatible with "
>> >> -   "current migration capabilities");
>> >> -return false;
>> >> -}
>> >> -if (!migrate_cap_set(MIGRATION_CAPABILITY_BLOCK, true, errp)) {
>> >> -return false;
>> >> -}
>> >> -s->must_remove_block_options = true;
>> >> -}
>> >> +s->must_remove_block_options = true;
>> >
>> > Can we drop this var too?  Perhaps with block_cleanup_parameters()?
>> 
>> Yes, Markus mentioned it in v1 already. Take a look there. There's
>> several other declarations I missed. v3 is coming soon.
>
> Right, noticed that it's removed actually in the next patch.
>
> But iiuc it can already been removed in this patch.  If we want to remove
> it in the next, logically we should set must_remove_block_options=false
> here, though..  So maybe easier to just drop it here.

Ah I see what you mean. I thought you're just asking for the removal
overall.

But block_cleanup_parameters sets the block capability to false and the
whole block migration only goes away in the next patch. I think we need
to keep this as true to preserve behavior. In theory, after this patch
people could still use the block migration just fine by setting the cap.
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


Re: [PATCH v2 6/6] migration: Deprecate fd: for file migration

2024-04-29 Thread Fabiano Rosas
Peter Xu  writes:

> On Fri, Apr 26, 2024 at 10:14:08AM -0300, Fabiano Rosas wrote:
>> The fd: URI can currently trigger two different types of migration, a
>> TCP migration using sockets and a file migration using a plain
>> file. This is in conflict with the recently introduced (8.2) QMP
>> migrate API that takes structured data as JSON-like format. We cannot
>> keep the same backend for both types of migration because with the new
>> API the code is more tightly coupled to the type of transport. This
>> means a TCP migration must use the 'socket' transport and a file
>> migration must use the 'file' transport.
>> 
>> If we keep allowing fd: when using a file, this creates an issue when
>> the user converts the old-style (fd:) to the new style ("transport":
>> "socket") invocation because the file descriptor in question has
>> previously been allowed to be either a plain file or a socket.
>> 
>> To avoid creating too much confusion, we can simply deprecate the fd:
>> + file usage, which is thought to be rarely used currently and instead
>> establish a 1:1 correspondence between fd: URI and socket transport,
>> and file: URI and file transport.
>> 
>> Signed-off-by: Fabiano Rosas 
>> ---
>>  docs/about/deprecated.rst | 14 ++
>>  1 file changed, 14 insertions(+)
>> 
>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>> index 0fb5c82640..813f7996fe 100644
>> --- a/docs/about/deprecated.rst
>> +++ b/docs/about/deprecated.rst
>> @@ -464,3 +464,17 @@ both, older and future versions of QEMU.
>>  The ``blacklist`` config file option has been renamed to ``block-rpcs``
>>  (to be in sync with the renaming of the corresponding command line
>>  option).
>> +
>> +Migration
>> +-
>> +
>> +``fd:`` URI when used for file migration (since 9.1)
>> +''''''''''''''''''''''''''''''''''''''''''''''''''''
>> +
>> +The ``fd:`` URI can currently provide a file descriptor that
>> +references either a socket or a plain file. These are two different
>> +types of migration. In order to reduce ambiguity, the ``fd:`` URI
>> +usage of providing a file descriptor to a plain file has been
>> +deprecated in favor of explicitly using the ``file:`` URI with the
>> +file descriptor being passed as an ``fdset``. Refer to the ``add-fd``
>> +command documentation for details on the ``fdset`` usage.
>
> Wanna do some warn_report() when detected non-socket fds alongside?  Looks
> like we previously do this for all deprecations.

Yes, good point.

>
> What's the plan when it's support removed?  I'm imaginging that we sanity
> check fstat() + S_ISSOCK on the fd and fail otherwise?  In that case we can
> have the code there, dump warn_report(), then switch to failing qmp migrate
> (and incoming side) later on?

Something along those lines. We currently use fd_is_socket():

bool fd_is_socket(int fd)
{
int optval;
socklen_t optlen = sizeof(optval);
return !getsockopt(fd, SOL_SOCKET, SO_TYPE, &optval, &optlen);
}

I'm thinking of this in fd_start_outgoing_migation():

if (!fd_is_socket(fd)) {
warn_report("fd: migration to a file is deprecated."
" Use file: instead.");
}
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH v3 0/6] migration removals & deprecations

2024-04-30 Thread Fabiano Rosas
since v2:

- removed some more stuff which I missed:
   blk/inc options from hmp-commands.hx
   the entire ram-compress.h
   unused declarations from options.h
   unused compression functions from qemu-file.c

- removed must_remove_block_options earlier in the 'blk' patch

- added a deprecation warning to outgoing/incoming fd

CI run: https://gitlab.com/farosas/qemu/-/pipelines/1272385260

v2:
https://lore.kernel.org/r/20240426131408.25410-1-faro...@suse.de
v1:
https://lore.kernel.org/r/20240425150939.19268-1-faro...@suse.de

Hi everyone,

Here's some cleaning up of deprecated code. It removes the old block
migration and compression code. Both have suitable replacements in the
form of the blockdev-mirror driver and multifd compression,
respectively.

There's also a deprecation for fd: + file to cope with the fact that
the new MigrationAddress API defines transports instead of protocols
(loose terms) like the old string API did. So we cannot map 1:1 from
fd: to any transport because fd: allows *both* file migration and
socket migration.

Fabiano Rosas (6):
  migration: Remove 'skipped' field from MigrationStats
  migration: Remove 'inc' option from migrate command
  migration: Remove 'blk/-b' option from migrate commands
  migration: Remove block migration
  migration: Remove non-multifd compression
  migration: Deprecate fd: for file migration

 .gitlab-ci.d/buildtest.yml   |2 +-
 MAINTAINERS  |1 -
 docs/about/deprecated.rst|   51 +-
 docs/about/removed-features.rst  |  103 +++
 docs/devel/migration/main.rst|2 +-
 hmp-commands.hx  |   17 +-
 hw/core/machine.c|1 -
 include/migration/misc.h |6 -
 meson.build  |2 -
 meson_options.txt|2 -
 migration/block.c| 1019 --
 migration/block.h|   52 --
 migration/colo.c |1 -
 migration/fd.c   |   12 +
 migration/meson.build|4 -
 migration/migration-hmp-cmds.c   |   97 +--
 migration/migration.c|   70 +-
 migration/migration.h|   11 -
 migration/options.c  |  229 ---
 migration/options.h  |   13 -
 migration/qemu-file.c|   78 ---
 migration/qemu-file.h|4 -
 migration/ram-compress.c |  564 -
 migration/ram-compress.h |   77 ---
 migration/ram.c  |  169 +
 migration/savevm.c   |5 -
 qapi/migration.json  |  205 +-
 scripts/meson-buildoptions.sh|4 -
 tests/qemu-iotests/183   |  147 -
 tests/qemu-iotests/183.out   |   66 --
 tests/qemu-iotests/common.filter |7 -
 tests/qtest/migration-test.c |  139 
 32 files changed, 147 insertions(+), 3013 deletions(-)
 delete mode 100644 migration/block.c
 delete mode 100644 migration/block.h
 delete mode 100644 migration/ram-compress.c
 delete mode 100644 migration/ram-compress.h
 delete mode 100755 tests/qemu-iotests/183
 delete mode 100644 tests/qemu-iotests/183.out


base-commit: fd87be1dada5672f877e03c2ca8504458292c479
-- 
2.35.3
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH v3 1/6] migration: Remove 'skipped' field from MigrationStats

2024-04-30 Thread Fabiano Rosas
The 'skipped' field of the MigrationStats struct has been deprecated
in 8.1. Time to remove it.

Deprecation commit 7b24d32634 ("migration: skipped field is really
obsolete.").

Reviewed-by: Markus Armbruster 
Reviewed-by: Peter Xu 
Signed-off-by: Fabiano Rosas 
---
 docs/about/deprecated.rst   | 6 --
 docs/about/removed-features.rst | 6 ++
 migration/migration-hmp-cmds.c  | 2 --
 migration/migration.c   | 2 --
 qapi/migration.json | 8 
 5 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 7b8aafa15b..1c03a358d1 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -468,12 +468,6 @@ option).
 Migration
 -
 
-``skipped`` MigrationStats field (since 8.1)
-''''''''''''''''''''''''''''''''''''''''''''
-
-``skipped`` field in Migration stats has been deprecated.  It hasn't
-been used for more than 10 years.
-
 ``inc`` migrate command option (since 8.2)
 ''''''''''''''''''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 53ca08aba9..c4cb2692d0 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -614,6 +614,12 @@ was superseded by ``sections``.
 Member ``section-size`` in the return value of ``query-sgx-capabilities``
 was superseded by ``sections``.
 
+``query-migrate`` return value member ``skipped`` (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Member ``skipped`` of the ``MigrationStats`` struct hasn't been used
+for more than 10 years. Removed with no replacement.
+
 Human Monitor Protocol (HMP) commands
 -
 
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 7e96ae6ffd..28f776d06d 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -105,8 +105,6 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict)
info->ram->total >> 10);
 monitor_printf(mon, "duplicate: %" PRIu64 " pages\n",
info->ram->duplicate);
-monitor_printf(mon, "skipped: %" PRIu64 " pages\n",
-   info->ram->skipped);
 monitor_printf(mon, "normal: %" PRIu64 " pages\n",
info->ram->normal);
 monitor_printf(mon, "normal bytes: %" PRIu64 " kbytes\n",
diff --git a/migration/migration.c b/migration/migration.c
index b5af6b5105..266652668d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1149,8 +1149,6 @@ static void populate_ram_info(MigrationInfo *info, 
MigrationState *s)
 info->ram->transferred = migration_transferred_bytes();
 info->ram->total = ram_bytes_total();
 info->ram->duplicate = stat64_get(&mig_stats.zero_pages);
-/* legacy value.  It is not used anymore */
-info->ram->skipped = 0;
 info->ram->normal = stat64_get(&mig_stats.normal_pages);
 info->ram->normal_bytes = info->ram->normal * page_size;
 info->ram->mbps = s->mbps;
diff --git a/qapi/migration.json b/qapi/migration.json
index 8c65b90328..401b8e24ac 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -23,9 +23,6 @@
 #
 # @duplicate: number of duplicate (zero) pages (since 1.2)
 #
-# @skipped: number of skipped zero pages.  Always zero, only provided
-# for compatibility (since 1.5)
-#
 # @normal: number of normal pages (since 1.2)
 #
 # @normal-bytes: number of normal bytes sent (since 1.2)
@@ -63,16 +60,11 @@
 # between 0 and @dirty-sync-count * @multifd-channels.  (since
 # 7.1)
 #
-# Features:
-#
-# @deprecated: Member @skipped is always zero since 1.5.3
-#
 # Since: 0.14
 ##
 { 'struct': 'MigrationStats',
   'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' ,
'duplicate': 'int',
-   'skipped': { 'type': 'int', 'features': [ 'deprecated' ] },
'normal': 'int',
'normal-bytes': 'int', 'dirty-pages-rate': 'int',
'mbps': 'number', 'dirty-sync-count': 'int',
-- 
2.35.3
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


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

2024-04-30 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.").

Reviewed-by: Markus Armbruster 
Signed-off-by: Fabiano Rosas 
---
 docs/about/deprecated.rst   |  9 --
 docs/about/removed-features.rst | 14 +
 hmp-commands.hx | 13 +++-
 migration/block.c   |  1 -
 migration/migration-hmp-cmds.c  | 18 ++-
 migration/migration.c   | 24 +--
 migration/options.c | 30 +-
 migration/options.h |  5 ---
 qapi/migration.json | 54 +++--
 9 files changed, 39 insertions(+), 129 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 1c03a358d1..ebe53821ed 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -468,15 +468,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 c4cb2692d0..7da4b3df14 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/hmp-commands.hx b/hmp-commands.hx
index 2e2a3bcf98..7978302949 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -909,26 +909,21 @@ ERST
 
 {
 .name   = "migrate",
-.args_type  = "detach:-d,blk:-b,inc:-i,resume:-r,uri:s",
-.params = "[-d] [-b] [-i] [-r] uri",
+.args_type  = "detach:-d,blk:-b,resume:-r,uri:s",
+.params = "[-d] [-b] [-r] 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)"
-  "\n\t\t\t -r to resume a paused migration",
+ " full copy of disk\n\t\t\t -r to resume a paused 
migration",
 .cmd= hmp_migrate,
 },
 
 
 SRST
-``migrate [-d] [-b] [-i]`` *uri*
+``migrate [-d] [-b]`` *uri*
   Migrate to *uri* (using -d to not wait for completion).
 
   ``-b``
 for migrati

[PATCH v3 3/6] migration: Remove 'blk/-b' option from migrate commands

2024-04-30 Thread Fabiano Rosas
The block migration is considered obsolete and has been deprecated in
8.2. Remove the migrate command option that enables it. This only
affects the QMP and HMP commands, the feature can still be accessed by
setting the migration 'block' capability. The whole feature will be
removed in a future patch.

Deprecation commit 8846b5bfca ("migration: migrate 'blk' command
option is deprecated.").

Reviewed-by: Markus Armbruster 
Signed-off-by: Fabiano Rosas 
---
 .gitlab-ci.d/buildtest.yml   |   2 +-
 docs/about/deprecated.rst|   9 --
 docs/about/removed-features.rst  |  14 +++
 hmp-commands.hx  |  12 +--
 migration/migration-hmp-cmds.c   |   9 +-
 migration/migration.c|  31 +--
 migration/migration.h|   4 -
 migration/options.c  |  14 +--
 migration/options.h  |   2 -
 qapi/migration.json  |   8 --
 tests/qemu-iotests/183   | 147 ---
 tests/qemu-iotests/183.out   |  66 --
 tests/qemu-iotests/common.filter |   7 --
 13 files changed, 25 insertions(+), 300 deletions(-)
 delete mode 100755 tests/qemu-iotests/183
 delete mode 100644 tests/qemu-iotests/183.out

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 6394b8f41e..3d975f8c34 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -342,7 +342,7 @@ build-tcg-disabled:
 - cd tests/qemu-iotests/
 - ./check -raw 001 002 003 004 005 008 009 010 011 012 021 025 032 033 048
 052 063 077 086 101 104 106 113 148 150 151 152 157 159 160 163
-170 171 183 184 192 194 208 221 226 227 236 253 277 image-fleecing
+170 171 184 192 194 208 221 226 227 236 253 277 image-fleecing
 - ./check -qcow2 028 051 056 057 058 065 068 082 085 091 095 096 102 122
 124 132 139 142 144 145 151 152 155 157 165 194 196 200 202
 208 209 216 218 227 234 246 247 248 250 254 255 257 258
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index ebe53821ed..d739358bb1 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -468,15 +468,6 @@ option).
 Migration
 -
 
-``blk`` migrate command option (since 8.2)
-''''''''''''''''''''''''''''''''''''''''''
-
-Use blockdev-mirror with NBD instead.
-
-As an intermediate step the ``blk`` functionality can be achieved by
-setting the ``block`` migration capability to ``true``.  But this
-capability is also deprecated.
-
 block migration (since 8.2)
 '''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 7da4b3df14..a491c0 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -627,6 +627,13 @@ 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.
 
+``migrate`` command option ``blk`` (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
 -
 
@@ -694,6 +701,13 @@ 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.
 
+``migrate`` command ``-b`` 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/hmp-commands.hx b/hmp-commands.hx
index 7978302949..ebca2cdced 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -909,21 +909,17 @@ ERST
 
 {
   

[PATCH v3 6/6] migration: Deprecate fd: for file migration

2024-04-30 Thread Fabiano Rosas
The fd: URI can currently trigger two different types of migration, a
TCP migration using sockets and a file migration using a plain
file. This is in conflict with the recently introduced (8.2) QMP
migrate API that takes structured data as JSON-like format. We cannot
keep the same backend for both types of migration because with the new
API the code is more tightly coupled to the type of transport. This
means a TCP migration must use the 'socket' transport and a file
migration must use the 'file' transport.

If we keep allowing fd: when using a file, this creates an issue when
the user converts the old-style (fd:) to the new style ("transport":
"socket") invocation because the file descriptor in question has
previously been allowed to be either a plain file or a socket.

To avoid creating too much confusion, we can simply deprecate the fd:
+ file usage, which is thought to be rarely used currently and instead
establish a 1:1 correspondence between fd: URI and socket transport,
and file: URI and file transport.

Signed-off-by: Fabiano Rosas 
---
 docs/about/deprecated.rst | 14 ++
 migration/fd.c| 12 
 2 files changed, 26 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 0fb5c82640..813f7996fe 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -464,3 +464,17 @@ both, older and future versions of QEMU.
 The ``blacklist`` config file option has been renamed to ``block-rpcs``
 (to be in sync with the renaming of the corresponding command line
 option).
+
+Migration
+-
+
+``fd:`` URI when used for file migration (since 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+The ``fd:`` URI can currently provide a file descriptor that
+references either a socket or a plain file. These are two different
+types of migration. In order to reduce ambiguity, the ``fd:`` URI
+usage of providing a file descriptor to a plain file has been
+deprecated in favor of explicitly using the ``file:`` URI with the
+file descriptor being passed as an ``fdset``. Refer to the ``add-fd``
+command documentation for details on the ``fdset`` usage.
diff --git a/migration/fd.c b/migration/fd.c
index 449adaa2de..aab5189eac 100644
--- a/migration/fd.c
+++ b/migration/fd.c
@@ -20,6 +20,8 @@
 #include "file.h"
 #include "migration.h"
 #include "monitor/monitor.h"
+#include "qemu/error-report.h"
+#include "qemu/sockets.h"
 #include "io/channel-util.h"
 #include "trace.h"
 
@@ -32,6 +34,11 @@ void fd_start_outgoing_migration(MigrationState *s, const 
char *fdname, Error **
 return;
 }
 
+if (!fd_is_socket(fd)) {
+warn_report("fd: migration to a file is deprecated."
+" Use file: instead.");
+}
+
 trace_migration_fd_outgoing(fd);
 ioc = qio_channel_new_fd(fd, errp);
 if (!ioc) {
@@ -61,6 +68,11 @@ void fd_start_incoming_migration(const char *fdname, Error 
**errp)
 return;
 }
 
+if (!fd_is_socket(fd)) {
+warn_report("fd: migration to a file is deprecated."
+" Use file: instead.");
+}
+
 trace_migration_fd_incoming(fd);
 
 ioc = qio_channel_new_fd(fd, errp);
-- 
2.35.3
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org


[PATCH v3 4/6] migration: Remove block migration

2024-04-30 Thread Fabiano Rosas
The block migration has been considered obsolete since QEMU 8.2 in
favor of the more flexible storage migration provided by the
blockdev-mirror driver. Two releases have passed so now it's time to
remove it.

Deprecation commit 66db46ca83 ("migration: Deprecate block
migration").

Reviewed-by: Markus Armbruster 
Signed-off-by: Fabiano Rosas 
---
 MAINTAINERS |1 -
 docs/about/deprecated.rst   |   10 -
 docs/about/removed-features.rst |   14 +
 docs/devel/migration/main.rst   |2 +-
 include/migration/misc.h|6 -
 meson.build |2 -
 meson_options.txt   |2 -
 migration/block.c   | 1018 ---
 migration/block.h   |   52 --
 migration/colo.c|1 -
 migration/meson.build   |3 -
 migration/migration-hmp-cmds.c  |   25 -
 migration/migration.c   |   15 -
 migration/options.c |   25 -
 migration/options.h |1 -
 migration/ram.c |   15 -
 migration/savevm.c  |5 -
 qapi/migration.json |   61 +-
 scripts/meson-buildoptions.sh   |4 -
 19 files changed, 26 insertions(+), 1236 deletions(-)
 delete mode 100644 migration/block.c
 delete mode 100644 migration/block.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 302b6fd00c..b7eaee942e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2865,7 +2865,6 @@ F: util/aio-*.h
 F: util/defer-call.c
 F: util/fdmon-*.c
 F: block/io.c
-F: migration/block*
 F: include/block/aio.h
 F: include/block/aio-wait.h
 F: include/qemu/defer-call.h
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index d739358bb1..3b9b5ba6ef 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -468,16 +468,6 @@ option).
 Migration
 -
 
-block migration (since 8.2)
-'''''''''''''''''''''''''''
-
-Block migration is too inflexible.  It needs to migrate all block
-devices or none.
-
-Please see "QMP invocation for live storage migration with
-``blockdev-mirror`` + NBD" in docs/interop/live-block-operations.rst
-for a detailed explanation.
-
 old compression method (since 8.2)
 ''''''''''''''''''''''''''''''''''
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index a491c0..9e7d8ee4ff 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -634,6 +634,13 @@ 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.
 
+``migrate-set-capabilities`` ``block`` option (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Block migration has been removed. For a replacement, see "QMP
+invocation for live storage migration with ``blockdev-mirror`` + NBD"
+in docs/interop/live-block-operations.rst.
+
 Human Monitor Protocol (HMP) commands
 -
 
@@ -708,6 +715,13 @@ 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.
 
+``migrate_set_capability`` ``block`` option (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Block migration has been removed. For a replacement, see "QMP
+invocation for live storage migration with ``blockdev-mirror`` + NBD"
+in docs/interop/live-block-operations.rst.
+
 Host Architectures
 --
 
diff --git a/docs/devel/migration/main.rst b/docs/devel/migration/main.rst
index 54385a23e5..495cdcb112 100644
--- a/docs/devel/migration/main.rst
+++ b/docs/devel/migration/main.rst
@@ -454,7 +454,7 @@ Examples of such API functions are:
 Iterative device migration
 --
 
-Some devices, such as RAM, Block storage or certain platform devices,
+Some devices, such as RAM or certain platform devices,
 have

[PATCH v3 5/6] migration: Remove non-multifd compression

2024-04-30 Thread Fabiano Rosas
The 'compress' migration capability enables the old compression code
which has shown issues over the years and is thought to be less stable
and tested than the more recent multifd-based compression. The old
compression code has been deprecated in 8.2 and now is time to remove
it.

Deprecation commit 864128df46 ("migration: Deprecate old compression
method").

Acked-by: Markus Armbruster 
Signed-off-by: Fabiano Rosas 
---
 docs/about/deprecated.rst   |  11 -
 docs/about/removed-features.rst |  55 
 hw/core/machine.c   |   1 -
 migration/meson.build   |   1 -
 migration/migration-hmp-cmds.c  |  47 ---
 migration/migration.c   |  14 -
 migration/migration.h   |   7 -
 migration/options.c | 164 --
 migration/options.h |   5 -
 migration/qemu-file.c   |  78 -
 migration/qemu-file.h   |   4 -
 migration/ram-compress.c| 564 
 migration/ram-compress.h|  77 -
 migration/ram.c | 154 +
 qapi/migration.json | 112 ---
 tests/qtest/migration-test.c| 139 
 16 files changed, 64 insertions(+), 1369 deletions(-)
 delete mode 100644 migration/ram-compress.c
 delete mode 100644 migration/ram-compress.h

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 3b9b5ba6ef..0fb5c82640 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -464,14 +464,3 @@ both, older and future versions of QEMU.
 The ``blacklist`` config file option has been renamed to ``block-rpcs``
 (to be in sync with the renaming of the corresponding command line
 option).
-
-Migration
--
-
-old compression method (since 8.2)
-''''''''''''''''''''''''''''''''''
-
-Compression method fails too much.  Too many races.  We are going to
-remove it if nobody fixes it.  For starters, migration-test
-compression tests are disabled because they fail randomly.  If you need
-compression, use multifd compression methods.
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 9e7d8ee4ff..fba0cfb0b0 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -505,6 +505,11 @@ configurations (e.g. -smp 8,sockets=0) is removed since 
9.0, users have
 to ensure that all the topology members described with -smp are greater
 than zero.
 
+``-global migration.decompress-error-check`` (removed in 9.1)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Removed along with the ``compression`` migration capability.
+
 User-mode emulator command line arguments
 -
 
@@ -641,6 +646,31 @@ Block migration has been removed. For a replacement, see 
"QMP
 invocation for live storage migration with ``blockdev-mirror`` + NBD"
 in docs/interop/live-block-operations.rst.
 
+``migrate-set-parameter`` ``compress-level`` option (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use ``multifd-zlib-level`` or ``multifd-zstd-level`` instead.
+
+``migrate-set-parameter`` ``compress-threads`` option (removed in 9.1)
+''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Use ``multifd-channels`` instead.
+
+``migrate-set-parameter`` ``compress-wait-thread`` option (removed in 9.1)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''&#

Re: [PATCH v3 06/17] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)

2024-07-09 Thread Fabiano Rosas
Philippe Mathieu-Daudé  writes:

> "General command" (GEN_CMD, CMD56) is described as:
>
>   GEN_CMD is the same as the single block read or write
>   commands (CMD24 or CMD17). The difference is that [...]
>   the data block is not a memory payload data but has a
>   vendor specific format and meaning.
>
> Thus this block must not be stored overwriting data block
> on underlying storage drive. Keep it in a dedicated
> 'vendor_data[]' array.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> Tested-by: Cédric Le Goater 
> ---
> RFC: Is it safe to reuse VMSTATE_UNUSED_V() (which happens
> to be the same size)?

Hi, sorry it took some time to get to this, I had just left for vacation
when you first posted.

I think it's ok:

{
  "field": "unused",
  "version_id": 1,
  "field_exists": false,
  "size": 512
},

vs.

{
  "field": "vendor_data",
  "version_id": 0,
  "field_exists": false,
  "num": 512,
  "size": 1
},

The unused field was introduced in 2016 so there's no chance of
migrating a QEMU that old to/from 9.1.


Re: [PATCH v3 06/17] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)

2024-07-10 Thread Fabiano Rosas
Peter Xu  writes:

> On Tue, Jul 09, 2024 at 05:38:54PM -0300, Fabiano Rosas wrote:
>> Philippe Mathieu-Daudé  writes:
>> 
>> > "General command" (GEN_CMD, CMD56) is described as:
>> >
>> >   GEN_CMD is the same as the single block read or write
>> >   commands (CMD24 or CMD17). The difference is that [...]
>> >   the data block is not a memory payload data but has a
>> >   vendor specific format and meaning.
>> >
>> > Thus this block must not be stored overwriting data block
>> > on underlying storage drive. Keep it in a dedicated
>> > 'vendor_data[]' array.
>> >
>> > Signed-off-by: Philippe Mathieu-Daudé 
>> > Tested-by: Cédric Le Goater 
>> > ---
>> > RFC: Is it safe to reuse VMSTATE_UNUSED_V() (which happens
>> > to be the same size)?
>> 
>> Hi, sorry it took some time to get to this, I had just left for vacation
>> when you first posted.
>
> And I totally overlooked there's the email.. until you replied.  Welcome
> back.

Thanks!

>
>> 
>> I think it's ok:
>> 
>> {
>>   "field": "unused",
>>   "version_id": 1,
>>   "field_exists": false,
>>   "size": 512
>> },
>> 
>> vs.
>> 
>> {
>>   "field": "vendor_data",
>>   "version_id": 0,
>>   "field_exists": false,
>>   "num": 512,
>>   "size": 1
>> },
>> 
>> The unused field was introduced in 2016 so there's no chance of
>> migrating a QEMU that old to/from 9.1.
>
> What happens if an old qemu 9.0 sends rubbish here to a new QEMU, while the
> new QEMU would consider it meaningful data?

It will send zeros, no? The code will have to cope with that. The
alternative is to put the vendor_data in a subsection and the code will
also have to cope with the lack of data when the old QEMU doesn't send
it.


Re: [PATCH v3 06/17] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)

2024-07-10 Thread Fabiano Rosas
Peter Xu  writes:

> On Wed, Jul 10, 2024 at 11:08:20AM -0300, Fabiano Rosas wrote:
>> >> I think it's ok:
>> >> 
>> >> {
>> >>   "field": "unused",
>> >>   "version_id": 1,
>> >>   "field_exists": false,
>> >>   "size": 512
>> >> },
>> >> 
>> >> vs.
>> >> 
>> >> {
>> >>   "field": "vendor_data",
>> >>   "version_id": 0,
>> >>   "field_exists": false,
>> >>   "num": 512,
>> >>   "size": 1
>> >> },
>> >> 
>> >> The unused field was introduced in 2016 so there's no chance of
>> >> migrating a QEMU that old to/from 9.1.
>> >
>> > What happens if an old qemu 9.0 sends rubbish here to a new QEMU, while the
>> > new QEMU would consider it meaningful data?
>> 
>> It will send zeros, no? The code will have to cope with that. The
>> alternative is to put the vendor_data in a subsection and the code will
>> also have to cope with the lack of data when the old QEMU doesn't send
>> it.
>
> Ah indeed, that "static const uint8_t buf[1024]" is there at least since
> 2017.  So yes, probably always sending zeros.

@Philippe, can vendor_data be 0 after migration? Otherwise 9.0 -> 9.1
migration might crash.

>
> Nothing I can think of otherwise indeed, if we want to trust that nothing
> will migrate before 2016.  It's just that we may want to know how that
> "2016" is justified to be safe if we would like to allow that in the
> future.

It's not about trust, we simply don't support migrations other than
n->n+1 and (maybe) n->n-1. So QEMU from 2016 is certainly not included.

>
> One thing _could_ be that "rule of thumb" is we plan to obsolete machines
> with 6 years, so anything "UNUSED" older than 6 years can be over-written?


Re: [PATCH v3 06/17] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)

2024-07-10 Thread Fabiano Rosas
Peter Xu  writes:

> On Wed, Jul 10, 2024 at 01:21:51PM -0300, Fabiano Rosas wrote:
>> It's not about trust, we simply don't support migrations other than
>> n->n+1 and (maybe) n->n-1. So QEMU from 2016 is certainly not included.
>
> Where does it come from?  I thought we suppport that..

I'm taking that from:

docs/devel/migration/main.rst:
  "In general QEMU tries to maintain forward migration compatibility
  (i.e. migrating from QEMU n->n+1) and there are users who benefit from
  backward compatibility as well."

But of course it doesn't say whether that comes with a transitive rule
allowing n->n+2 migrations.

>
> The same question would be: are we requesting an OpenStack cluster to
> always upgrade QEMU with +1 versions, otherwise migration will fail?

Will an OpenStack cluster be using upstream QEMU? If not, then that's a
question for the distro. In a very practical sense, we're not requesting
anything. We barely test n->n+1/n->n-1, even if we had a strong support
statement I wouldn't be confident saying migration from QEMU 2.7 -> QEMU
9.1 should succeed.


Re: [PATCH v3 06/17] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)

2024-07-10 Thread Fabiano Rosas
Peter Xu  writes:

> On Wed, Jul 10, 2024 at 04:48:23PM -0300, Fabiano Rosas wrote:
>> Peter Xu  writes:
>> 
>> > On Wed, Jul 10, 2024 at 01:21:51PM -0300, Fabiano Rosas wrote:
>> >> It's not about trust, we simply don't support migrations other than
>> >> n->n+1 and (maybe) n->n-1. So QEMU from 2016 is certainly not included.
>> >
>> > Where does it come from?  I thought we suppport that..
>> 
>> I'm taking that from:
>> 
>> docs/devel/migration/main.rst:
>>   "In general QEMU tries to maintain forward migration compatibility
>>   (i.e. migrating from QEMU n->n+1) and there are users who benefit from
>>   backward compatibility as well."
>> 
>> But of course it doesn't say whether that comes with a transitive rule
>> allowing n->n+2 migrations.
>
> I'd say that "i.e." implies n->n+1 is not the only forward migration we
> would support.
>
> I _think_ we should support all forward migration as long as the machine
> type matches.
>
>> 
>> >
>> > The same question would be: are we requesting an OpenStack cluster to
>> > always upgrade QEMU with +1 versions, otherwise migration will fail?
>> 
>> Will an OpenStack cluster be using upstream QEMU? If not, then that's a
>
> It's an example to show what I meant! :) Nothing else. Definitely not
> saying that everyone should use an upstream released QEMU (but in reality,
> it's not a problem, I think, and I do feel like people use them, perhaps
> more with the stable releases).
>
>> question for the distro. In a very practical sense, we're not requesting
>> anything. We barely test n->n+1/n->n-1, even if we had a strong support
>> statement I wouldn't be confident saying migration from QEMU 2.7 -> QEMU
>> 9.1 should succeed.
>
> No matter what we test in CI, I don't think we should break that for >1
> versions..  I hope 2.7->9.1 keeps working, otherwise I think it's legal to
> file a bug by anyone.
>
> For example, I randomly fetched a bug report:
>
> https://gitlab.com/qemu-project/qemu/-/issues/1937
>
> QEMU version:6.2 and 7.2.5
>
> And I believe that's the common case even for upstream.  If we don't do
> that right for upstream, it can be impossible tasks for downstream and for
> all of us to maintain.

But do we do that right currently? I have no idea. Have we ever done
it? And we're here discussing a hypothetical 2.7->9.1 ...

So we cannot reuse the UNUSED field because QEMU from 2016 might send
their data and QEMU from 2024 would interpret it wrong.

How do we proceed? Add a subsection. And make the code survive when
receiving 0.

@Peter is that it? What about backwards-compat? We'll need a property as
well it seems.


Re: [PATCH v3 06/17] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)

2024-07-11 Thread Fabiano Rosas
Peter Xu  writes:

> On Wed, Jul 10, 2024 at 06:38:26PM -0300, Fabiano Rosas wrote:
>> Peter Xu  writes:
>> 
>> > On Wed, Jul 10, 2024 at 04:48:23PM -0300, Fabiano Rosas wrote:
>> >> Peter Xu  writes:
>> >> 
>> >> > On Wed, Jul 10, 2024 at 01:21:51PM -0300, Fabiano Rosas wrote:
>> >> >> It's not about trust, we simply don't support migrations other than
>> >> >> n->n+1 and (maybe) n->n-1. So QEMU from 2016 is certainly not included.
>> >> >
>> >> > Where does it come from?  I thought we suppport that..
>> >> 
>> >> I'm taking that from:
>> >> 
>> >> docs/devel/migration/main.rst:
>> >>   "In general QEMU tries to maintain forward migration compatibility
>> >>   (i.e. migrating from QEMU n->n+1) and there are users who benefit from
>> >>   backward compatibility as well."
>> >> 
>> >> But of course it doesn't say whether that comes with a transitive rule
>> >> allowing n->n+2 migrations.
>> >
>> > I'd say that "i.e." implies n->n+1 is not the only forward migration we
>> > would support.
>> >
>> > I _think_ we should support all forward migration as long as the machine
>> > type matches.
>> >
>> >> 
>> >> >
>> >> > The same question would be: are we requesting an OpenStack cluster to
>> >> > always upgrade QEMU with +1 versions, otherwise migration will fail?
>> >> 
>> >> Will an OpenStack cluster be using upstream QEMU? If not, then that's a
>> >
>> > It's an example to show what I meant! :) Nothing else. Definitely not
>> > saying that everyone should use an upstream released QEMU (but in reality,
>> > it's not a problem, I think, and I do feel like people use them, perhaps
>> > more with the stable releases).
>> >
>> >> question for the distro. In a very practical sense, we're not requesting
>> >> anything. We barely test n->n+1/n->n-1, even if we had a strong support
>> >> statement I wouldn't be confident saying migration from QEMU 2.7 -> QEMU
>> >> 9.1 should succeed.
>> >
>> > No matter what we test in CI, I don't think we should break that for >1
>> > versions..  I hope 2.7->9.1 keeps working, otherwise I think it's legal to
>> > file a bug by anyone.
>> >
>> > For example, I randomly fetched a bug report:
>> >
>> > https://gitlab.com/qemu-project/qemu/-/issues/1937
>> >
>> > QEMU version:6.2 and 7.2.5
>> >
>> > And I believe that's the common case even for upstream.  If we don't do
>> > that right for upstream, it can be impossible tasks for downstream and for
>> > all of us to maintain.
>> 
>> But do we do that right currently? I have no idea. Have we ever done
>> it? And we're here discussing a hypothetical 2.7->9.1 ...
>> 
>> So we cannot reuse the UNUSED field because QEMU from 2016 might send
>> their data and QEMU from 2024 would interpret it wrong.
>> 
>> How do we proceed? Add a subsection. And make the code survive when
>> receiving 0.
>> 
>> @Peter is that it? What about backwards-compat? We'll need a property as
>> well it seems.
>
> Compat property is definitely one way to go, but I think it's you who more
> or less persuaded me that reusing it seems possible! At least I can't yet
> think of anything bad if it's ancient unused buffers.

Since we're allowing any old QEMU version to migrate to the most recent
one, we need to think of the data that was there before the introduction
of the UNUSED field. If that QEMU version is used, then it's not going
to be zeroes on the stream, but whatever data was there before. The new
QEMU will be expecting the vendor_data introduced in this patch.

> And that's why I was asking about a sane way to describe the "magic
> year".. And I was very serious when I said "6 years" to follow the
> deprecation of machine types, because it'll be something we can follow
> to say when an unused buffer can be obsolete and it could make some
> sense: if we will start to deprecate machine types, then it may not
> make sense to keep any UNUSED compatible forever, after all.
>

Is there an easy way to look at a field and tell in which machine type's
timeframe it was introduced? If the machine type of that era has been
removed, then the field is free to go as well. I'd prefer if we had a
hard link instead of just counting years. Maybe we should to that
mapping at the machine deprecation time? As in, "look at the unused
fields introduced in that timeframe and mark them free".

> And we need that ruler to be as accurate as "always 6 years to follow
> machine type deprecation procedure", in case someone else tomorrow asks us
> something that was only UNUSED since 2018.  We need a rule of thumb if we
> want to reuse it, and if all of you agree we can start with this one,
> perhaps with a comment above the field (before we think all through and
> make it a rule on deprecating UNUSED)?


Re: [PATCH v3 06/17] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)

2024-07-11 Thread Fabiano Rosas
Peter Xu  writes:

> On Thu, Jul 11, 2024 at 10:34:12AM -0300, Fabiano Rosas wrote:
>> Is there an easy way to look at a field and tell in which machine type's
>> timeframe it was introduced?
>
> I am not aware of any.
>
>> If the machine type of that era has been removed, then the field is free
>> to go as well. I'd prefer if we had a hard link instead of just counting
>> years. Maybe we should to that mapping at the machine deprecation time?
>> As in, "look at the unused fields introduced in that timeframe and mark
>> them free".
>
> We can do that, but depending on how easy it would be. That can be an
> overkill to me if it's non-trivial.  When it becomes complicated, I'd
> rather make machine compat property easier to use so we always stick with
> that.  Currently it's not as easy to use.
>
> Maybe we shouldn't make it a common rule to let people reuse the UNUSED
> fields, even if in this case it's probably fine?
>
> E.g. I don't think it's a huge deal to keep all UNUSED fields forever -
> sending 512B zeros for only one specific device isn't an issue even if kept
> forever.
>
> If "over 6 years" would be okay and simple enough, then maybe we can stick
> with that (and only if people would like to reuse a field and ask; that's
> after all not required..).  If more than that I doubt whether we should
> spend time working on covering all the fields.

I'm fine with a simple rule.

But of course, that means we cannot claim to support all kinds of
forward migrations anymore. Only those in the 6 year period.


Re: [PATCH v3 06/17] hw/sd/sdcard: Do not store vendor data on block drive (CMD56)

2024-07-11 Thread Fabiano Rosas
Peter Xu  writes:

> On Thu, Jul 11, 2024 at 11:44:08AM -0300, Fabiano Rosas wrote:
>> But of course, that means we cannot claim to support all kinds of
>> forward migrations anymore. Only those in the 6 year period.
>
> That "6 years" comes from machine type deprecation period, and migration
> compatibility is mostly only attached to machine types, and we only ever
> allowed migration with the same machine type.
>
> It means, >6 years migration will never work anyway as soon as we start to
> deprecate machine types (irrelevant of any reuse of UNUSED), because the >6
> years machine types will simply be gone.. See configuration_post_load(),
> where it'll throw an error upfront when machine type mismatched.

Yes, duh! What am I talking about...


Re: [PATCH v2 4/4] virtio-net: Add support for USO features

2024-08-09 Thread Fabiano Rosas
Peter Xu  writes:

> On Thu, Aug 08, 2024 at 10:47:28AM -0400, Michael S. Tsirkin wrote:
>> On Thu, Aug 08, 2024 at 10:15:36AM -0400, Peter Xu wrote:
>> > On Thu, Aug 08, 2024 at 07:12:14AM -0400, Michael S. Tsirkin wrote:
>> > > This is too big of a hammer. People already use what you call "cross
>> > > migrate" and have for years. We are not going to stop developing
>> > > features just because someone suddenly became aware of some such bit.
>> > > If you care, you will have to work to solve the problem properly -
>> > > nacking half baked hacks is the only tool maintainers have to make
>> > > people work on hard problems.
>> > 
>> > IMHO this is totally different thing.  It's not about proposing a new
>> > feature yet so far, it's about how we should fix a breakage first.
>> > 
>> > And that's why I think we should fix it even in the simple way first, then
>> > we consider anything more benefitial from perf side without breaking
>> > anything, which should be on top of that.
>> > 
>> > Thanks,
>> 
>> As I said, once the quick hack is merged people stop caring.
>
> IMHO it's not a hack. It's a proper fix to me to disable it by default for
> now.
>
> OTOH, having it ON always even knowing it can break migration is a hack to
> me, when we don't have anything else to guard the migration.
>
>> Mixing different kernel versions in migration is esoteric enough for
>> this not to matter to most people. There's no rush I think, address
>> it properly.
>
> Exactly mixing kernel versions will be tricky to users to identify, but
> that's, AFAICT, exactly happening everywhere.  We can't urge user to always
> use the exact same kernels when we're talking about a VM cluster.  That's
> why I think allowing migration to work across those kernels matter.

I also worry a bit about the scenario where the cluster changes slightly
and now all VMs are already restricted by some option that requires the
exact same kernel. Specifically, kernel changes in a cloud environment
also happen due to factors completely unrelated to migration. I'm not
sure the people managing the infra (who care about migration) will be
gating kernel changes just because QEMU has been configured in a
specific manner.


Re: [PATCH 10/20] qemu: Add support for mapped-ram on save

2024-10-10 Thread Fabiano Rosas
Daniel P. Berrangé  writes:

> On Thu, Aug 08, 2024 at 05:38:03PM -0600, Jim Fehlig via Devel wrote:
>> Introduce support for QEMU's new mapped-ram stream format [1].
>> mapped-ram is enabled by default if the underlying QEMU advertises
>> the mapped-ram migration capability. It can be disabled by changing
>> the 'save_image_version' setting in qemu.conf to version '2'.
>> 
>> To use mapped-ram with QEMU:
>> - The 'mapped-ram' migration capability must be set to true
>> - The 'multifd' migration capability must be set to true and
>>   the 'multifd-channels' migration parameter must set to 1
>> - QEMU must be provided an fdset containing the migration fd
>> - The 'migrate' qmp command is invoked with a URI referencing the
>>   fdset and an offset where to start writing the data stream, e.g.
>> 
>>   {"execute":"migrate",
>>"arguments":{"detach":true,"resume":false,
>> "uri":"file:/dev/fdset/0,offset=0x11921"}}
>> 
>> The mapped-ram stream, in conjunction with direct IO and multifd
>> support provided by subsequent patches, can significantly improve
>> the time required to save VM memory state. The following tables
>> compare mapped-ram with the existing, sequential save stream. In
>> all cases, the save and restore operations are to/from a block
>> device comprised of two NVMe disks in RAID0 configuration with
>> xfs (~8600MiB/s). The values in the 'save time' and 'restore time'
>> columns were scraped from the 'real' time reported by time(1). The
>> 'Size' and 'Blocks' columns were provided by the corresponding
>> outputs of stat(1).
>> 
>> VM: 32G RAM, 1 vcpu, idle (shortly after boot)
>> 
>>| save| restore |
>> | time| time| Size | Blocks
>> ---+-+-+--+
>> legacy | 6.193s  | 4.399s  | 985744812| 1925288
>> ---+-+-+--+
>> mapped-ram | 5.109s  | 1.176s  | 34368554354  | 1774472
>
> I'm surprised by the restore time speed up, as I didn't think
> mapped-ram should make any perf difference without direct IO
> and multifd.
>
>> ---+-+-+--+
>> legacy + direct IO | 5.725s  | 4.512s  | 985765251| 1925328
>> ---+-+-+--+
>> mapped-ram + direct IO | 4.627s  | 1.490s  | 34368554354  | 1774304
>
> Still somewhat surprised by the speed up on restore here too

Hmm, I'm thinking this might be caused by zero page handling. The non
mapped-ram path has an extra buffer_is_zero() and memset() of the hva
page.

Now, is it an issue that mapped-ram skips that memset? I assume guest
memory will always be clear at the start of migration. There won't be a
situation where the destination VM starts with memory already
dirty... *and* the save file is also different, otherwise it wouldn't
make any difference.

>
>> ---+-+-+--+
>> mapped-ram + direct IO | | |  |
>>  + multifd-channels=8  | 4.421s  | 0.845s  | 34368554318  | 1774312
>> ---
>> 
>> VM: 32G RAM, 30G dirty, 1 vcpu in tight loop dirtying memory
>> 
>>| save| restore |
>> | time| time| Size | Blocks
>> ---+-+-+--+-
>> legacy | 25.800s | 14.332s | 33154309983  | 64754512
>> ---+-+-+--+-
>> mapped-ram | 18.742s | 15.027s | 34368559228  | 64617160
>> ---+-+-+--+-
>> legacy + direct IO | 13.115s | 18.050s | 33154310496  | 64754520
>> ---+-+-+--+-
>> mapped-ram + direct IO | 13.623s | 15.959s | 34368557392  | 64662040
>
> These figures make more sense with restore time matching save time
> more or less.
>
>> ---+ +-+--+-
>> mapped-ram + direct IO | | |  |
>>  + multifd-channels=8  | 6.994s  | 6.470s  | 34368554980  | 64665776
>> 
>> 
>> As can be seen from the tables, one caveat of mapped-ram is the logical
>> file size of a saved image is basically equivalent to the VM memory size.
>> Note however that mapped-ram typically uses fewer blocks on disk.
>> 
>> Another caveat of mapped-ram is the requirement for a seekable file
>> descriptor, which currently makes it incompatible with libvirt's
>> support for save image compression. Also note the mapped-ram stream
>> is incompatible with the existing stream format, hence mapped-ram
>> cannot be used to restore an image saved with the existing format
>> and vice versa.
>> 
>> [1] 
>> htt

Re: [PATCH 10/20] qemu: Add support for mapped-ram on save

2024-10-10 Thread Fabiano Rosas
Daniel P. Berrangé  writes:

> On Thu, Oct 10, 2024 at 12:06:51PM -0300, Fabiano Rosas wrote:
>> Daniel P. Berrangé  writes:
>> 
>> > On Thu, Aug 08, 2024 at 05:38:03PM -0600, Jim Fehlig via Devel wrote:
>> >> Introduce support for QEMU's new mapped-ram stream format [1].
>> >> mapped-ram is enabled by default if the underlying QEMU advertises
>> >> the mapped-ram migration capability. It can be disabled by changing
>> >> the 'save_image_version' setting in qemu.conf to version '2'.
>> >> 
>> >> To use mapped-ram with QEMU:
>> >> - The 'mapped-ram' migration capability must be set to true
>> >> - The 'multifd' migration capability must be set to true and
>> >>   the 'multifd-channels' migration parameter must set to 1
>> >> - QEMU must be provided an fdset containing the migration fd
>> >> - The 'migrate' qmp command is invoked with a URI referencing the
>> >>   fdset and an offset where to start writing the data stream, e.g.
>> >> 
>> >>   {"execute":"migrate",
>> >>"arguments":{"detach":true,"resume":false,
>> >> "uri":"file:/dev/fdset/0,offset=0x11921"}}
>> >> 
>> >> The mapped-ram stream, in conjunction with direct IO and multifd
>> >> support provided by subsequent patches, can significantly improve
>> >> the time required to save VM memory state. The following tables
>> >> compare mapped-ram with the existing, sequential save stream. In
>> >> all cases, the save and restore operations are to/from a block
>> >> device comprised of two NVMe disks in RAID0 configuration with
>> >> xfs (~8600MiB/s). The values in the 'save time' and 'restore time'
>> >> columns were scraped from the 'real' time reported by time(1). The
>> >> 'Size' and 'Blocks' columns were provided by the corresponding
>> >> outputs of stat(1).
>> >> 
>> >> VM: 32G RAM, 1 vcpu, idle (shortly after boot)
>> >> 
>> >>| save| restore |
>> >>  | time| time| Size | Blocks
>> >> ---+-+-+--+
>> >> legacy | 6.193s  | 4.399s  | 985744812| 1925288
>> >> ---+-+-+--+
>> >> mapped-ram | 5.109s  | 1.176s  | 34368554354  | 1774472
>> >
>> > I'm surprised by the restore time speed up, as I didn't think
>> > mapped-ram should make any perf difference without direct IO
>> > and multifd.
>> >
>> >> ---+-+-+--+
>> >> legacy + direct IO | 5.725s  | 4.512s  | 985765251| 1925328
>> >> ---+-+-+--+
>> >> mapped-ram + direct IO | 4.627s  | 1.490s  | 34368554354  | 1774304
>> >
>> > Still somewhat surprised by the speed up on restore here too
>> 
>> Hmm, I'm thinking this might be caused by zero page handling. The non
>> mapped-ram path has an extra buffer_is_zero() and memset() of the hva
>> page.
>> 
>> Now, is it an issue that mapped-ram skips that memset? I assume guest
>> memory will always be clear at the start of migration. There won't be a
>> situation where the destination VM starts with memory already
>> dirty... *and* the save file is also different, otherwise it wouldn't
>> make any difference.
>
> Consider the snapshot use case. You're running the VM, so memory
> has arbitrary contents, now you restore to a saved snapshot. QEMU
> remains running this whole time and you can't assume initial
> memory is zeroed. Surely we need the memset ?

Hmm, I probably have a big gap on my knowledge here, but savevm doesn't
hook into file migration, so there's no way to load a snapshot with
mapped-ram that I know of. Is this something that libvirt enables
somehow? There would be no -incoming on the cmdline.

>
> With regards,
> Daniel


Re: [PATCH 10/20] qemu: Add support for mapped-ram on save

2024-10-14 Thread Fabiano Rosas
Daniel P. Berrangé  writes:

> On Thu, Oct 10, 2024 at 02:52:56PM -0300, Fabiano Rosas wrote:
>> Daniel P. Berrangé  writes:
>> 
>> > On Thu, Oct 10, 2024 at 12:06:51PM -0300, Fabiano Rosas wrote:
>> >> Daniel P. Berrangé  writes:
>> >> 
>> >> > On Thu, Aug 08, 2024 at 05:38:03PM -0600, Jim Fehlig via Devel wrote:
>> >> >> Introduce support for QEMU's new mapped-ram stream format [1].
>> >> >> mapped-ram is enabled by default if the underlying QEMU advertises
>> >> >> the mapped-ram migration capability. It can be disabled by changing
>> >> >> the 'save_image_version' setting in qemu.conf to version '2'.
>> >> >> 
>> >> >> To use mapped-ram with QEMU:
>> >> >> - The 'mapped-ram' migration capability must be set to true
>> >> >> - The 'multifd' migration capability must be set to true and
>> >> >>   the 'multifd-channels' migration parameter must set to 1
>> >> >> - QEMU must be provided an fdset containing the migration fd
>> >> >> - The 'migrate' qmp command is invoked with a URI referencing the
>> >> >>   fdset and an offset where to start writing the data stream, e.g.
>> >> >> 
>> >> >>   {"execute":"migrate",
>> >> >>"arguments":{"detach":true,"resume":false,
>> >> >> "uri":"file:/dev/fdset/0,offset=0x11921"}}
>> >> >> 
>> >> >> The mapped-ram stream, in conjunction with direct IO and multifd
>> >> >> support provided by subsequent patches, can significantly improve
>> >> >> the time required to save VM memory state. The following tables
>> >> >> compare mapped-ram with the existing, sequential save stream. In
>> >> >> all cases, the save and restore operations are to/from a block
>> >> >> device comprised of two NVMe disks in RAID0 configuration with
>> >> >> xfs (~8600MiB/s). The values in the 'save time' and 'restore time'
>> >> >> columns were scraped from the 'real' time reported by time(1). The
>> >> >> 'Size' and 'Blocks' columns were provided by the corresponding
>> >> >> outputs of stat(1).
>> >> >> 
>> >> >> VM: 32G RAM, 1 vcpu, idle (shortly after boot)
>> >> >> 
>> >> >>| save| restore |
>> >> >>   | time| time| Size | Blocks
>> >> >> ---+-+-+--+
>> >> >> legacy | 6.193s  | 4.399s  | 985744812| 1925288
>> >> >> ---+-+-+--+
>> >> >> mapped-ram | 5.109s  | 1.176s  | 34368554354  | 1774472
>> >> >
>> >> > I'm surprised by the restore time speed up, as I didn't think
>> >> > mapped-ram should make any perf difference without direct IO
>> >> > and multifd.
>> >> >
>> >> >> ---+-+-+--+
>> >> >> legacy + direct IO | 5.725s  | 4.512s  | 985765251| 1925328
>> >> >> ---+-+-+--+
>> >> >> mapped-ram + direct IO | 4.627s  | 1.490s  | 34368554354  | 1774304
>> >> >
>> >> > Still somewhat surprised by the speed up on restore here too
>> >> 
>> >> Hmm, I'm thinking this might be caused by zero page handling. The non
>> >> mapped-ram path has an extra buffer_is_zero() and memset() of the hva
>> >> page.
>> >> 
>> >> Now, is it an issue that mapped-ram skips that memset? I assume guest
>> >> memory will always be clear at the start of migration. There won't be a
>> >> situation where the destination VM starts with memory already
>> >> dirty... *and* the save file is also different, otherwise it wouldn't
>> >> make any difference.
>> >
>> > Consider the snapshot use case. You're running the VM, so memory
>> > has arbitrary contents, now you restore to a saved snapshot. QEMU
>> > remains running this whole time and you can't assume initial
>> > memory is zeroed. Surely we need the memset ?
>> 
>> Hmm, I probably have a big gap on my knowledge here, but savevm doesn't
>> hook into file migration, so there's no way to load a snapshot with
>> mapped-ram that I know of. Is this something that libvirt enables
>> somehow? There would be no -incoming on the cmdline.
>
> Opps, yes, i always forget savevm is off in its own little world.
>
> Upstream we've talking about making savevm be a facade around the
> 'migrate' command, but no one has ever made a PoC.

Yeah, that would be nice. Once I learn how the data ends up in the qcow2
image, maybe I can look into adding a new 'snapshot' migration mode to
QEMU.

>
> With regards,
> Daniel


Re: [PATCH 1/3] tests/qtest/test-x86-cpuid-compat: Remove tests related to pc-i440fx-2.3

2025-01-17 Thread Fabiano Rosas
Thomas Huth  writes:

> The pc-i440fx-2.3 machine type has been removed in commit 46a2bd5257
> ("hw/i386/pc: Remove deprecated pc-i440fx-2.3 machine") already, so
> these tests are just dead code by now.
>
> Signed-off-by: Thomas Huth 

Reviewed-by: Fabiano Rosas 

I'm queuing this one individually for now.


[PATCH v2 18/24] qapi/migration: Deprecate capabilities commands

2025-06-30 Thread Fabiano Rosas
The concept of capabilities is being merged into the concept of
parameters. From now on, the commands that handle capabilities are
deprecated in favor of the commands that handle parameters.

Affected commands:

- migrate-set-capabilities
- query-migrate-capabilities

Signed-off-by: Fabiano Rosas 
---
 docs/about/deprecated.rst  | 12 
 migration/migration-hmp-cmds.c |  6 ++
 qapi/migration.json| 16 ++--
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 42037131de..15474833ea 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -605,3 +605,15 @@ command documentation for details on the ``fdset`` usage.
 
 The ``zero-blocks`` capability was part of the block migration which
 doesn't exist anymore since it was removed in QEMU v9.1.
+
+``migrate-set-capabilities`` command (since 10.1)
+'''''''''''''''''''''''''''''''''''''''''''''''''
+
+This command was deprecated. Use ``migrate-set-parameters`` instead
+which now supports setting capabilities.
+
+``query-migrate-capabilities`` command (since 10.1)
+'''''''''''''''''''''''''''''''''''''''''''''''''''
+
+This command was deprecated. Use ``query-migrate-parameters`` instead
+which now supports querying capabilities.
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 8615340a6b..7f234d5aa8 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -229,6 +229,9 @@ void hmp_info_migrate_capabilities(Monitor *mon, const 
QDict *qdict)
 {
 MigrationCapabilityStatusList *caps, *cap;
 
+warn_report("info migrate_capabilities is deprecated;"
+" use info migrate_parameters instead");
+
 caps = qmp_query_migrate_capabilities(NULL);
 
 if (caps) {
@@ -616,6 +619,9 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict 
*qdict)
 MigrationCapabilityStatus *value;
 int val;
 
+warn_report("migrate_set_capability is deprecated;"
+" use migrate_set_parameter instead");
+
 val = qapi_enum_parse(&MigrationCapability_lookup, cap, -1, &err);
 if (val < 0) {
 goto end;
diff --git a/qapi/migration.json b/qapi/migration.json
index 3d3f5624c5..c5e6ea1a2d 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -521,6 +521,11 @@
 #
 # @capabilities: json array of capability modifications to make
 #
+# Features:
+#
+# @deprecated: This command is deprecated in favor of
+# migrate-set-parameters.
+#
 # Since: 1.2
 #
 # .. qmp-example::
@@ -530,7 +535,8 @@
 # <- { "return": {} }
 ##
 { 'command': 'migrate-set-capabilities',
-  'data': { 'capabilities': ['MigrationCapabilityStatus'] } }
+  'data': { 'capabilities': ['MigrationCapabilityStatus'] },
+  'features': ['deprecated'] }
 
 ##
 # @query-migrate-capabilities:
@@ -539,6 +545,11 @@
 #
 # Returns: @MigrationCapabilityStatus
 #
+# Features:
+#
+# @deprecated: This command is deprecated in favor of
+# query-migrate-parameters.
+#
 # Since: 1.2
 #
 # .. qmp-example::
@@ -554,7 +565,8 @@
 #   {"state": false, "capability": "x-colo"}
 #]}
 ##
-{ 'command': 'query-migrate-capabilities', 'returns':   
['MigrationCapabilityStatus']}
+{ 'command': 'query-migrate-capabilities', 'returns':   
['MigrationCapabilityStatus'],
+  'features': ['deprecated'] }
 
 ##
 # @MultiFDCompression:
-- 
2.35.3


Re: [PATCH 2/4] qapi/migration: Deprecate migrate argument @detach

2025-05-21 Thread Fabiano Rosas
Markus Armbruster  writes:

> Argument @detach has always been ignored.  Start the clock to get rid
> of it.
>
> Cc: Peter Xu 
> Cc: Fabiano Rosas 
> Signed-off-by: Markus Armbruster 

Reviewed-by: Fabiano Rosas