* [PULL 01/31] migration: fix parsing snapshots with x-ignore-shared flag
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 02/31] migration: Fix writing mapped_ram + ignore_shared snapshots Peter Xu
` (30 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Pawel Zmarzly
From: Pawel Zmarzly <pzmarzly0@gmail.com>
Snapshots made with mapped-ram and x-ignore-shared flags are
not parsed properly.
The ignore-shared feature adds and extra field in the stream, which
needs to be consumed on the destination side. Even though mapped-ram has
a fixed header format, the ignore-shared is part of the "generic" stream
infomation so the mapped-ram code is currently skipping that be64 read
which incorrectly offsets every subsequent read from the stream.
The current ignore-shared handling can simply be moved earlier in the code
to encompass mapped-ram as well since the ignore-shared doubleword is the
first one read when parsing the ramblock section of the stream.
Co-authored-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Pawel Zmarzly <pzmarzly0@gmail.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251126121233.542473-1-pzmarzly0@gmail.com
[peterx: enhance commit log per fabiano]
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 21 +++++++++++----------
tests/qtest/migration/file-tests.c | 18 ++++++++++++++++++
2 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 29f016cb25..7d024b88b5 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4205,6 +4205,17 @@ static int parse_ramblock(QEMUFile *f, RAMBlock *block, ram_addr_t length)
assert(block);
+ if (migrate_ignore_shared()) {
+ hwaddr addr = qemu_get_be64(f);
+ if (migrate_ram_is_ignored(block) &&
+ block->mr->addr != addr) {
+ error_report("Mismatched GPAs for block %s "
+ "%" PRId64 "!= %" PRId64, block->idstr,
+ (uint64_t)addr, (uint64_t)block->mr->addr);
+ return -EINVAL;
+ }
+ }
+
if (migrate_mapped_ram()) {
parse_ramblock_mapped_ram(f, block, length, &local_err);
if (local_err) {
@@ -4244,16 +4255,6 @@ static int parse_ramblock(QEMUFile *f, RAMBlock *block, ram_addr_t length)
return -EINVAL;
}
}
- if (migrate_ignore_shared()) {
- hwaddr addr = qemu_get_be64(f);
- if (migrate_ram_is_ignored(block) &&
- block->mr->addr != addr) {
- error_report("Mismatched GPAs for block %s "
- "%" PRId64 "!= %" PRId64, block->idstr,
- (uint64_t)addr, (uint64_t)block->mr->addr);
- return -EINVAL;
- }
- }
ret = rdma_block_notification_handle(f, block->idstr);
if (ret < 0) {
qemu_file_set_error(f, ret);
diff --git a/tests/qtest/migration/file-tests.c b/tests/qtest/migration/file-tests.c
index 4d78ce0855..c196a703ff 100644
--- a/tests/qtest/migration/file-tests.c
+++ b/tests/qtest/migration/file-tests.c
@@ -303,6 +303,22 @@ static void migration_test_add_file_smoke(MigrationTestEnv *env)
test_multifd_file_mapped_ram_dio);
}
+static void test_precopy_file_mapped_ram_ignore_shared(void)
+{
+ g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
+ FILE_TEST_FILENAME);
+ MigrateCommon args = {
+ .connect_uri = uri,
+ .listen_uri = "defer",
+ .start = {
+ .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
+ .caps[MIGRATION_CAPABILITY_X_IGNORE_SHARED] = true,
+ },
+ };
+
+ test_file_common(&args, true);
+}
+
void migration_test_add_file(MigrationTestEnv *env)
{
tmpfs = env->tmpfs;
@@ -329,6 +345,8 @@ void migration_test_add_file(MigrationTestEnv *env)
migration_test_add("/migration/multifd/file/mapped-ram",
test_multifd_file_mapped_ram);
+ migration_test_add("/migration/multifd/file/mapped-ram/ignore-shared",
+ test_precopy_file_mapped_ram_ignore_shared);
migration_test_add("/migration/multifd/file/mapped-ram/live",
test_multifd_file_mapped_ram_live);
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 02/31] migration: Fix writing mapped_ram + ignore_shared snapshots
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
2025-12-23 14:29 ` [PULL 01/31] migration: fix parsing snapshots with x-ignore-shared flag Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 03/31] scripts/analyze-migration: Rename RAM_SAVE_FLAG_COMPRESS to RAM_SAVE_FLAG_ZERO Peter Xu
` (29 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Pawel Zmarzly
From: Pawel Zmarzly <pzmarzly0@gmail.com>
Currently if you set these flags and have any shared memory object, saving
a snapshot will fail with:
Failed to write bitmap to file: Unable to write to file: Bad address
We need to skip writing RAMBlocks that are backed by shared objects.
Also, we should mark these RAMBlocks as skipped, so the snapshot format stays
readable to tools that later don't know QEMU's command line (for example
scripts/analyze-migration.py). I used bitmap_offset=0 pages_offset=0 for this.
This minor change to snapshot format should be safe, as offset=0 should not
have ever been possible.
Signed-off-by: Pawel Zmarzly <pzmarzly0@gmail.com>
Link: https://lore.kernel.org/r/20251126154734.940066-1-pzmarzly0@gmail.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 53 ++++++++++++++++++++++++++++++++-----------------
1 file changed, 35 insertions(+), 18 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 7d024b88b5..117957da91 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3042,28 +3042,37 @@ static void mapped_ram_setup_ramblock(QEMUFile *file, RAMBlock *block)
header = g_new0(MappedRamHeader, 1);
header_size = sizeof(MappedRamHeader);
- num_pages = block->used_length >> TARGET_PAGE_BITS;
- bitmap_size = BITS_TO_LONGS(num_pages) * sizeof(unsigned long);
-
- /*
- * Save the file offsets of where the bitmap and the pages should
- * go as they are written at the end of migration and during the
- * iterative phase, respectively.
- */
- block->bitmap_offset = qemu_get_offset(file) + header_size;
- block->pages_offset = ROUND_UP(block->bitmap_offset +
- bitmap_size,
- MAPPED_RAM_FILE_OFFSET_ALIGNMENT);
-
header->version = cpu_to_be32(MAPPED_RAM_HDR_VERSION);
header->page_size = cpu_to_be64(TARGET_PAGE_SIZE);
- header->bitmap_offset = cpu_to_be64(block->bitmap_offset);
- header->pages_offset = cpu_to_be64(block->pages_offset);
+
+ if (migrate_ram_is_ignored(block)) {
+ header->bitmap_offset = 0;
+ header->pages_offset = 0;
+ } else {
+ num_pages = block->used_length >> TARGET_PAGE_BITS;
+ bitmap_size = BITS_TO_LONGS(num_pages) * sizeof(unsigned long);
+
+ /*
+ * Save the file offsets of where the bitmap and the pages should
+ * go as they are written at the end of migration and during the
+ * iterative phase, respectively.
+ */
+ block->bitmap_offset = qemu_get_offset(file) + header_size;
+ block->pages_offset = ROUND_UP(block->bitmap_offset +
+ bitmap_size,
+ MAPPED_RAM_FILE_OFFSET_ALIGNMENT);
+
+ header->bitmap_offset = cpu_to_be64(block->bitmap_offset);
+ header->pages_offset = cpu_to_be64(block->pages_offset);
+ }
qemu_put_buffer(file, (uint8_t *) header, header_size);
- /* prepare offset for next ramblock */
- qemu_set_offset(file, block->pages_offset + block->used_length, SEEK_SET);
+ if (!migrate_ram_is_ignored(block)) {
+ /* leave space for block data */
+ qemu_set_offset(file, block->pages_offset + block->used_length,
+ SEEK_SET);
+ }
}
static bool mapped_ram_read_header(QEMUFile *file, MappedRamHeader *header,
@@ -3146,7 +3155,6 @@ static int ram_save_setup(QEMUFile *f, void *opaque, Error **errp)
if (migrate_ignore_shared()) {
qemu_put_be64(f, block->mr->addr);
}
-
if (migrate_mapped_ram()) {
mapped_ram_setup_ramblock(f, block);
}
@@ -3217,6 +3225,10 @@ static void ram_save_file_bmap(QEMUFile *f)
RAMBlock *block;
RAMBLOCK_FOREACH_MIGRATABLE(block) {
+ if (migrate_ram_is_ignored(block)) {
+ continue;
+ }
+
long num_pages = block->used_length >> TARGET_PAGE_BITS;
long bitmap_size = BITS_TO_LONGS(num_pages) * sizeof(unsigned long);
@@ -4162,6 +4174,11 @@ static void parse_ramblock_mapped_ram(QEMUFile *f, RAMBlock *block,
return;
}
+ if (migrate_ignore_shared() &&
+ header.bitmap_offset == 0 && header.pages_offset == 0) {
+ return;
+ }
+
block->pages_offset = header.pages_offset;
/*
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 03/31] scripts/analyze-migration: Rename RAM_SAVE_FLAG_COMPRESS to RAM_SAVE_FLAG_ZERO
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
2025-12-23 14:29 ` [PULL 01/31] migration: fix parsing snapshots with x-ignore-shared flag Peter Xu
2025-12-23 14:29 ` [PULL 02/31] migration: Fix writing mapped_ram + ignore_shared snapshots Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 04/31] scripts/analyze-migration: Support mapped-ram snapshot format Peter Xu
` (28 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Pawel Zmarzly
From: Pawel Zmarzly <pzmarzly0@gmail.com>
It has been renamed on the C side a few years ago. In modern QEMU versions,
fill_byte must be zero. Updating the Python script to make grepping and
understanding the code easier.
Signed-off-by: Pawel Zmarzly <pzmarzly0@gmail.com>
Link: https://lore.kernel.org/r/20251125173007.245607-1-pzmarzly0@gmail.com
[peterx: fix over-long line]
Signed-off-by: Peter Xu <peterx@redhat.com>
---
scripts/analyze-migration.py | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
index 67631ac43e..a12ea9fc8f 100755
--- a/scripts/analyze-migration.py
+++ b/scripts/analyze-migration.py
@@ -107,7 +107,7 @@ def close(self):
self.file.close()
class RamSection(object):
- RAM_SAVE_FLAG_COMPRESS = 0x02
+ RAM_SAVE_FLAG_ZERO = 0x02
RAM_SAVE_FLAG_MEM_SIZE = 0x04
RAM_SAVE_FLAG_PAGE = 0x08
RAM_SAVE_FLAG_EOS = 0x10
@@ -172,19 +172,16 @@ def read(self):
mr_addr = self.file.read64()
flags &= ~self.RAM_SAVE_FLAG_MEM_SIZE
- if flags & self.RAM_SAVE_FLAG_COMPRESS:
+ if flags & self.RAM_SAVE_FLAG_ZERO:
if flags & self.RAM_SAVE_FLAG_CONTINUE:
flags &= ~self.RAM_SAVE_FLAG_CONTINUE
else:
self.name = self.file.readstr()
- fill_char = self.file.read8()
- # The page in question is filled with fill_char now
- if self.write_memory and fill_char != 0:
- self.files[self.name].seek(addr, os.SEEK_SET)
- self.files[self.name].write(chr(fill_char) * self.TARGET_PAGE_SIZE)
+ _fill_char = self.file.read8()
if self.dump_memory:
- self.memory['%s (0x%016x)' % (self.name, addr)] = 'Filled with 0x%02x' % fill_char
- flags &= ~self.RAM_SAVE_FLAG_COMPRESS
+ self.memory['%s (0x%016x)' %
+ (self.name, addr)] = 'Filled with 0x00'
+ flags &= ~self.RAM_SAVE_FLAG_ZERO
elif flags & self.RAM_SAVE_FLAG_PAGE:
if flags & self.RAM_SAVE_FLAG_CONTINUE:
flags &= ~self.RAM_SAVE_FLAG_CONTINUE
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 04/31] scripts/analyze-migration: Support mapped-ram snapshot format
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (2 preceding siblings ...)
2025-12-23 14:29 ` [PULL 03/31] scripts/analyze-migration: Rename RAM_SAVE_FLAG_COMPRESS to RAM_SAVE_FLAG_ZERO Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 05/31] migration: Use explicit error_free() instead of g_autoptr Peter Xu
` (27 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Pawel Zmarzly
From: Pawel Zmarzly <pzmarzly0@gmail.com>
The script has not been updated to read mapped-ram snapshots and is currently
crashing when trying to read such a file.
With this commit, it can now read a snapshot created with:
(qemu) migrate_set_capability x-ignore-shared on
(qemu) migrate_set_capability mapped-ram on
(qemu) migrate -d file:vm.state
Signed-off-by: Pawel Zmarzly <pzmarzly0@gmail.com>
Link: https://lore.kernel.org/r/20251126155015.941129-1-pzmarzly0@gmail.com
[peterx: space fixes, introduce parseMappedRamBlob(), add comments, etc.]
Signed-off-by: Peter Xu <peterx@redhat.com>
---
scripts/analyze-migration.py | 57 ++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/scripts/analyze-migration.py b/scripts/analyze-migration.py
index a12ea9fc8f..e81deab8f9 100755
--- a/scripts/analyze-migration.py
+++ b/scripts/analyze-migration.py
@@ -19,6 +19,7 @@
import json
import os
+import math
import argparse
import collections
import struct
@@ -127,6 +128,7 @@ def __init__(self, file, version_id, ramargs, section_key):
self.dump_memory = ramargs['dump_memory']
self.write_memory = ramargs['write_memory']
self.ignore_shared = ramargs['ignore_shared']
+ self.mapped_ram = ramargs['mapped_ram']
self.sizeinfo = collections.OrderedDict()
self.data = collections.OrderedDict()
self.data['section sizes'] = self.sizeinfo
@@ -146,6 +148,57 @@ def __str__(self):
def getDict(self):
return self.data
+ def parseMappedRamBlob(self, len):
+ version = self.file.read32()
+ if version != 1:
+ raise Exception("Unsupported MappedRamHeader version %s" % version)
+
+ page_size = self.file.read64()
+ if page_size != self.TARGET_PAGE_SIZE:
+ raise Exception("Page size mismatch in MappedRamHeader")
+
+ bitmap_offset = self.file.read64()
+ pages_offset = self.file.read64()
+
+ if self.ignore_shared and bitmap_offset == 0 and pages_offset == 0:
+ # This is a shared ramblock, x-ignore-share must have been
+ # enabled, and mapped-ram didn't allocate bitmap or page blob
+ # for it.
+ return
+
+ if self.dump_memory or self.write_memory:
+ num_pages = len // page_size
+
+ self.file.seek(bitmap_offset, os.SEEK_SET)
+ bitmap_len = int(math.ceil(num_pages / 8))
+ bitmap = self.file.readvar(size=bitmap_len)
+
+ self.file.seek(pages_offset, os.SEEK_SET)
+ for page_num in range(num_pages):
+ page_addr = page_num * page_size
+
+ is_filled = (bitmap[page_num // 8] >> page_num % 8) & 1
+ if is_filled:
+ data = self.file.readvar(size=self.TARGET_PAGE_SIZE)
+ if self.write_memory:
+ self.files[self.name].seek(page_addr, os.SEEK_SET)
+ self.files[self.name].write(data)
+ if self.dump_memory:
+ hexdata = " ".join("{0:02x}".format(c) for c in data)
+ self.memory['%s (0x%016x)' %
+ (self.name, page_addr)] = hexdata
+ else:
+ self.file.seek(self.TARGET_PAGE_SIZE, os.SEEK_CUR)
+ if self.write_memory:
+ self.files[self.name].seek(page_addr, os.SEEK_SET)
+ self.files[self.name].write(
+ b'\x00' * self.TARGET_PAGE_SIZE)
+ if self.dump_memory:
+ self.memory['%s (0x%016x)' %
+ (self.name, page_addr)] = 'Filled with 0x00'
+
+ self.file.seek(pages_offset + len, os.SEEK_SET)
+
def read(self):
# Read all RAM sections
while True:
@@ -170,6 +223,8 @@ def read(self):
self.files[self.name] = f
if self.ignore_shared:
mr_addr = self.file.read64()
+ if self.mapped_ram:
+ self.parseMappedRamBlob(len)
flags &= ~self.RAM_SAVE_FLAG_MEM_SIZE
if flags & self.RAM_SAVE_FLAG_ZERO:
@@ -660,6 +715,7 @@ def read(self, desc_only = False, dump_memory = False,
ramargs['dump_memory'] = dump_memory
ramargs['write_memory'] = write_memory
ramargs['ignore_shared'] = False
+ ramargs['mapped_ram'] = False
self.section_classes[('ram',0)][1] = ramargs
while True:
@@ -671,6 +727,7 @@ def read(self, desc_only = False, dump_memory = False,
section = ConfigurationSection(file, config_desc)
section.read()
ramargs['ignore_shared'] = section.has_capability('x-ignore-shared')
+ ramargs['mapped_ram'] = section.has_capability('mapped-ram')
elif section_type == self.QEMU_VM_SECTION_START or section_type == self.QEMU_VM_SECTION_FULL:
section_id = file.read32()
name = file.readstr()
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 05/31] migration: Use explicit error_free() instead of g_autoptr
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (3 preceding siblings ...)
2025-12-23 14:29 ` [PULL 04/31] scripts/analyze-migration: Support mapped-ram snapshot format Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 06/31] Revert "error: define g_autoptr() cleanup function for the Error type" Peter Xu
` (26 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Markus Armbruster
There're only two use cases of g_autoptr to free Error objects in migration
code paths.
Due to the nature of how Error should be used (normally ownership will be
passed over to Error APIs, like error_report_err), auto-free functions may
be error prone on its own. The auto cleanup function was merged without
proper review, as pointed out by Dan and Markus:
https://lore.kernel.org/r/aSWSLMi6ZhTCS_p2@redhat.com
Remove the two use cases so that we can remove the auto cleanup function,
hence suggest to not use auto frees for Errors.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Link: https://lore.kernel.org/r/20251201194510.1121221-2-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/multifd-device-state.c | 3 ++-
migration/savevm.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/migration/multifd-device-state.c b/migration/multifd-device-state.c
index fce64f00b0..db3239fef5 100644
--- a/migration/multifd-device-state.c
+++ b/migration/multifd-device-state.c
@@ -140,7 +140,7 @@ static void multifd_device_state_save_thread_data_free(void *opaque)
static int multifd_device_state_save_thread(void *opaque)
{
SaveCompletePrecopyThreadData *data = opaque;
- g_autoptr(Error) local_err = NULL;
+ Error *local_err = NULL;
if (!data->hdlr(data, &local_err)) {
MigrationState *s = migrate_get_current();
@@ -159,6 +159,7 @@ static int multifd_device_state_save_thread(void *opaque)
* return we end setting is purely arbitrary.
*/
migrate_set_error(s, local_err);
+ error_free(local_err);
}
return 0;
diff --git a/migration/savevm.c b/migration/savevm.c
index 62cc2ce25c..638e9b364f 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2823,7 +2823,7 @@ static int qemu_loadvm_load_thread(void *thread_opaque)
{
struct LoadThreadData *data = thread_opaque;
MigrationIncomingState *mis = migration_incoming_get_current();
- g_autoptr(Error) local_err = NULL;
+ Error *local_err = NULL;
if (!data->function(data->opaque, &mis->load_threads_abort, &local_err)) {
MigrationState *s = migrate_get_current();
@@ -2841,6 +2841,7 @@ static int qemu_loadvm_load_thread(void *thread_opaque)
* return we end setting is purely arbitrary.
*/
migrate_set_error(s, local_err);
+ error_free(local_err);
}
return 0;
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 06/31] Revert "error: define g_autoptr() cleanup function for the Error type"
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (4 preceding siblings ...)
2025-12-23 14:29 ` [PULL 05/31] migration: Use explicit error_free() instead of g_autoptr Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 07/31] error: Poison g_autoptr(Error) to prevent its use Peter Xu
` (25 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, Cédric Le Goater,
Maciej S. Szmigiero, Markus Armbruster, Maciej S. Szmigiero
This reverts commit 18eb55546a54e443d94a4c49286348176ad4b00a.
Due to the nature of how Error should be used (normally ownership will be
passed over to Error APIs, like error_report_err), auto-free functions may
be error prone on its own. The auto cleanup function was merged without
proper review as pointed out by Dan and Markus:
https://lore.kernel.org/r/aSWSLMi6ZhTCS_p2@redhat.com
Cc: Cédric Le Goater <clg@redhat.com>
Acked-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Acked-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
Link: https://lore.kernel.org/r/20251201194510.1121221-3-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/qapi/error.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/include/qapi/error.h b/include/qapi/error.h
index b16c6303f8..f3ce4a4a2d 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -437,8 +437,6 @@ Error *error_copy(const Error *err);
*/
void error_free(Error *err);
-G_DEFINE_AUTOPTR_CLEANUP_FUNC(Error, error_free)
-
/*
* Convenience function to assert that *@errp is set, then silently free it.
*/
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 07/31] error: Poison g_autoptr(Error) to prevent its use
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (5 preceding siblings ...)
2025-12-23 14:29 ` [PULL 06/31] Revert "error: define g_autoptr() cleanup function for the Error type" Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 08/31] migration: Make migration_connect_set_error() own the error Peter Xu
` (24 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, Markus Armbruster, Peter Maydell,
Cédric Le Goater
From: Markus Armbruster <armbru@redhat.com>
The previous commit reverted support for g_autoptr(Error). This one
should stop it from coming back.
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Tested-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Link: https://lore.kernel.org/r/20251201194510.1121221-4-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/qapi/error.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/include/qapi/error.h b/include/qapi/error.h
index f3ce4a4a2d..2356b84bb3 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -437,6 +437,26 @@ Error *error_copy(const Error *err);
*/
void error_free(Error *err);
+/*
+ * Poison g_autoptr(Error) to prevent its use.
+ *
+ * Functions that report or propagate an error take ownership of the
+ * Error object. Explicit error_free() is needed when you handle an
+ * error in some other way. This is rare.
+ *
+ * g_autoptr(Error) would call error_free() automatically on return.
+ * To avoid a double-free, we'd have to manually clear the pointer
+ * every time we propagate or report.
+ *
+ * Thus, g_autoptr(Error) would make the rare case easier to get right
+ * (less prone to leaks), and the common case easier to get wrong
+ * (more prone to double-free).
+ */
+extern void
+__attribute__((error("Do not use g_autoptr() to declare Error * variables")))
+error_free_poisoned(Error *err);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(Error, error_free_poisoned)
+
/*
* Convenience function to assert that *@errp is set, then silently free it.
*/
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 08/31] migration: Make migration_connect_set_error() own the error
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (6 preceding siblings ...)
2025-12-23 14:29 ` [PULL 07/31] error: Poison g_autoptr(Error) to prevent its use Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 09/31] migration: Make multifd_send_set_error() " Peter Xu
` (23 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Markus Armbruster
Make migration_connect_set_error() take ownership of the error always.
Paving way for making migrate_set_error() to take ownership.
When at it, renaming it to migration_connect_error_propagate(), following
Error API, to imply the Error object ownership transition.
NOTE: this patch also makes migration_connect() to take ownership of the
Error passed in.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Link: https://lore.kernel.org/r/20251201194510.1121221-5-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/channel.c | 1 -
migration/migration.c | 9 +++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/migration/channel.c b/migration/channel.c
index 462cc183e1..92435fa7f7 100644
--- a/migration/channel.c
+++ b/migration/channel.c
@@ -95,7 +95,6 @@ void migration_channel_connect(MigrationState *s,
}
}
migration_connect(s, error);
- error_free(error);
}
diff --git a/migration/migration.c b/migration/migration.c
index b316ee01ab..0ff8b31a88 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1575,7 +1575,7 @@ static void migrate_error_free(MigrationState *s)
}
}
-static void migration_connect_set_error(MigrationState *s, const Error *error)
+static void migration_connect_error_propagate(MigrationState *s, Error *error)
{
MigrationStatus current = s->state;
MigrationStatus next;
@@ -1602,6 +1602,7 @@ static void migration_connect_set_error(MigrationState *s, const Error *error)
migrate_set_state(&s->state, current, next);
migrate_set_error(s, error);
+ error_free(error);
}
void migration_cancel(void)
@@ -2292,7 +2293,7 @@ void qmp_migrate(const char *uri, bool has_channels,
out:
if (local_err) {
- migration_connect_set_error(s, local_err);
+ migration_connect_error_propagate(s, error_copy(local_err));
error_propagate(errp, local_err);
}
}
@@ -2337,7 +2338,7 @@ static void qmp_migrate_finish(MigrationAddress *addr, bool resume_requested,
if (!resume_requested) {
yank_unregister_instance(MIGRATION_YANK_INSTANCE);
}
- migration_connect_set_error(s, local_err);
+ migration_connect_error_propagate(s, error_copy(local_err));
error_propagate(errp, local_err);
return;
}
@@ -4039,7 +4040,7 @@ void migration_connect(MigrationState *s, Error *error_in)
s->expected_downtime = migrate_downtime_limit();
if (error_in) {
- migration_connect_set_error(s, error_in);
+ migration_connect_error_propagate(s, error_in);
if (resume) {
/*
* Don't do cleanup for resume if channel is invalid, but only dump
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 09/31] migration: Make multifd_send_set_error() own the error
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (7 preceding siblings ...)
2025-12-23 14:29 ` [PULL 08/31] migration: Make migration_connect_set_error() own the error Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 10/31] migration: Make multifd_recv_terminate_threads() " Peter Xu
` (22 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Markus Armbruster
Make multifd_send_set_error() take ownership of the error always. Paving
way for making migrate_set_error() to take ownership.
When at it, rename it to multifd_send_error_propagate() to imply the
ownership transition following Error API's naming style.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Link: https://lore.kernel.org/r/20251201194510.1121221-6-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/multifd.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/migration/multifd.c b/migration/multifd.c
index 3203dc98e1..651ea3d14b 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -414,7 +414,7 @@ bool multifd_send(MultiFDSendData **send_data)
}
/* Multifd send side hit an error; remember it and prepare to quit */
-static void multifd_send_set_error(Error *err)
+static void multifd_send_error_propagate(Error *err)
{
/*
* We don't want to exit each threads twice. Depending on where
@@ -429,6 +429,7 @@ static void multifd_send_set_error(Error *err)
if (err) {
MigrationState *s = migrate_get_current();
migrate_set_error(s, err);
+ error_free(err);
if (s->state == MIGRATION_STATUS_SETUP ||
s->state == MIGRATION_STATUS_PRE_SWITCHOVER ||
s->state == MIGRATION_STATUS_DEVICE ||
@@ -777,9 +778,8 @@ out:
if (ret) {
assert(local_err);
trace_multifd_send_error(p->id);
- multifd_send_set_error(local_err);
+ multifd_send_error_propagate(local_err);
multifd_send_kick_main(p);
- error_free(local_err);
}
rcu_unregister_thread();
@@ -901,14 +901,13 @@ out:
}
trace_multifd_new_send_channel_async_error(p->id, local_err);
- multifd_send_set_error(local_err);
+ multifd_send_error_propagate(local_err);
/*
* For error cases (TLS or non-TLS), IO channel is always freed here
* rather than when cleanup multifd: since p->c is not set, multifd
* cleanup code doesn't even know its existence.
*/
object_unref(OBJECT(ioc));
- error_free(local_err);
}
static bool multifd_new_send_channel_create(gpointer opaque, Error **errp)
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 10/31] migration: Make multifd_recv_terminate_threads() own the error
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (8 preceding siblings ...)
2025-12-23 14:29 ` [PULL 09/31] migration: Make multifd_send_set_error() " Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 11/31] migration: Replace migrate_set_error() with migrate_error_propagate() Peter Xu
` (21 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Markus Armbruster
Make multifd_recv_terminate_threads() take ownership of the error always.
Paving way for making migrate_set_error() to take ownership.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Link: https://lore.kernel.org/r/20251201194510.1121221-7-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/multifd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/migration/multifd.c b/migration/multifd.c
index 651ea3d14b..52e4d25857 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -1068,6 +1068,7 @@ static void multifd_recv_terminate_threads(Error *err)
if (err) {
MigrationState *s = migrate_get_current();
migrate_set_error(s, err);
+ error_free(err);
if (s->state == MIGRATION_STATUS_SETUP ||
s->state == MIGRATION_STATUS_ACTIVE) {
migrate_set_state(&s->state, s->state,
@@ -1434,7 +1435,6 @@ static void *multifd_recv_thread(void *opaque)
if (local_err) {
multifd_recv_terminate_threads(local_err);
- error_free(local_err);
}
rcu_unregister_thread();
@@ -1535,7 +1535,7 @@ void multifd_recv_new_channel(QIOChannel *ioc, Error **errp)
if (use_packets) {
id = multifd_recv_initial_packet(ioc, &local_err);
if (id < 0) {
- multifd_recv_terminate_threads(local_err);
+ multifd_recv_terminate_threads(error_copy(local_err));
error_propagate_prepend(errp, local_err,
"failed to receive packet"
" via multifd channel %d: ",
@@ -1551,7 +1551,7 @@ void multifd_recv_new_channel(QIOChannel *ioc, Error **errp)
if (p->c != NULL) {
error_setg(&local_err, "multifd: received id '%d' already setup'",
id);
- multifd_recv_terminate_threads(local_err);
+ multifd_recv_terminate_threads(error_copy(local_err));
error_propagate(errp, local_err);
return;
}
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 11/31] migration: Replace migrate_set_error() with migrate_error_propagate()
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (9 preceding siblings ...)
2025-12-23 14:29 ` [PULL 10/31] migration: Make multifd_recv_terminate_threads() " Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 12/31] migration: Use error_propagate() in migrate_error_propagate() Peter Xu
` (20 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Markus Armbruster
migrate_set_error() currently doesn't take ownership of the error being
passed in. It's not aligned with the error API and meanwhile it also
makes most of the caller free the error explicitly.
Change the API to take the ownership of the Error object instead. This
should save a lot of error_copy() invocations.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Link: https://lore.kernel.org/r/20251201194510.1121221-8-peterx@redhat.com
[peterx: break line for qemu_savevm_send_packaged, per markus]
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/migration.h | 2 +-
migration/cpr-exec.c | 5 ++--
migration/migration.c | 44 +++++++++++++++-----------------
migration/multifd-device-state.c | 5 +---
migration/multifd.c | 19 +++++++-------
migration/postcopy-ram.c | 5 ++--
migration/ram.c | 4 +--
migration/savevm.c | 17 +++++-------
8 files changed, 44 insertions(+), 57 deletions(-)
diff --git a/migration/migration.h b/migration/migration.h
index 213b33fe6e..e4b4f25deb 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -525,7 +525,7 @@ void migration_incoming_process(void);
bool migration_has_all_channels(void);
-void migrate_set_error(MigrationState *s, const Error *error);
+void migrate_error_propagate(MigrationState *s, Error *error);
bool migrate_has_error(MigrationState *s);
void migration_connect(MigrationState *s, Error *error_in);
diff --git a/migration/cpr-exec.c b/migration/cpr-exec.c
index 0b8344a86f..da287d8031 100644
--- a/migration/cpr-exec.c
+++ b/migration/cpr-exec.c
@@ -158,8 +158,9 @@ static void cpr_exec_cb(void *opaque)
error_report_err(error_copy(err));
migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
- migrate_set_error(s, err);
- error_free(err);
+
+ migrate_error_propagate(s, err);
+ /* We must reset the error because it'll be reused later */
err = NULL;
/* Note, we can go from state COMPLETED to FAILED */
diff --git a/migration/migration.c b/migration/migration.c
index 0ff8b31a88..70813e5006 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -914,9 +914,7 @@ process_incoming_migration_co(void *opaque)
fail:
migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
MIGRATION_STATUS_FAILED);
- migrate_set_error(s, local_err);
- error_free(local_err);
-
+ migrate_error_propagate(s, local_err);
migration_incoming_state_destroy();
if (mis->exit_on_error) {
@@ -1548,14 +1546,20 @@ static void migration_cleanup_bh(void *opaque)
migration_cleanup(opaque);
}
-void migrate_set_error(MigrationState *s, const Error *error)
+/*
+ * Propagate the Error* object to migration core. The caller mustn't
+ * reference the error pointer after the function returned, because the
+ * Error* object might be freed.
+ */
+void migrate_error_propagate(MigrationState *s, Error *error)
{
QEMU_LOCK_GUARD(&s->error_mutex);
-
trace_migrate_error(error_get_pretty(error));
if (!s->error) {
- s->error = error_copy(error);
+ s->error = error;
+ } else {
+ error_free(error);
}
}
@@ -1601,8 +1605,7 @@ static void migration_connect_error_propagate(MigrationState *s, Error *error)
}
migrate_set_state(&s->state, current, next);
- migrate_set_error(s, error);
- error_free(error);
+ migrate_error_propagate(s, error);
}
void migration_cancel(void)
@@ -2014,8 +2017,7 @@ void qmp_migrate_pause(Error **errp)
/* Tell the core migration that we're pausing */
error_setg(&error, "Postcopy migration is paused by the user");
- migrate_set_error(ms, error);
- error_free(error);
+ migrate_error_propagate(ms, error);
qemu_mutex_lock(&ms->qemu_file_lock);
if (ms->to_dst_file) {
@@ -2647,8 +2649,7 @@ static void *source_return_path_thread(void *opaque)
out:
if (err) {
- migrate_set_error(ms, err);
- error_free(err);
+ migrate_error_propagate(ms, err);
trace_source_return_path_thread_bad_end();
}
@@ -3094,12 +3095,10 @@ static void migration_completion(MigrationState *s)
fail:
if (qemu_file_get_error_obj(s->to_dst_file, &local_err)) {
- migrate_set_error(s, local_err);
- error_free(local_err);
+ migrate_error_propagate(s, local_err);
} else if (ret) {
error_setg_errno(&local_err, -ret, "Error in migration completion");
- migrate_set_error(s, local_err);
- error_free(local_err);
+ migrate_error_propagate(s, local_err);
}
if (s->state != MIGRATION_STATUS_CANCELLING) {
@@ -3326,8 +3325,7 @@ static MigThrError migration_detect_error(MigrationState *s)
}
if (local_error) {
- migrate_set_error(s, local_error);
- error_free(local_error);
+ migrate_error_propagate(s, local_error);
}
if (state == MIGRATION_STATUS_POSTCOPY_ACTIVE && ret) {
@@ -3522,7 +3520,7 @@ static MigIterateState migration_iteration_run(MigrationState *s)
if (must_precopy <= s->threshold_size &&
can_switchover && qatomic_read(&s->start_postcopy)) {
if (postcopy_start(s, &local_err)) {
- migrate_set_error(s, local_err);
+ migrate_error_propagate(s, error_copy(local_err));
error_report_err(local_err);
}
return MIG_ITERATE_SKIP;
@@ -3819,8 +3817,7 @@ static void *migration_thread(void *opaque)
* devices to unplug. This to preserve migration state transitions.
*/
if (ret) {
- migrate_set_error(s, local_err);
- error_free(local_err);
+ migrate_error_propagate(s, local_err);
migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
MIGRATION_STATUS_FAILED);
goto out;
@@ -3944,8 +3941,7 @@ static void *bg_migration_thread(void *opaque)
* devices to unplug. This to preserve migration state transitions.
*/
if (ret) {
- migrate_set_error(s, local_err);
- error_free(local_err);
+ migrate_error_propagate(s, local_err);
migrate_set_state(&s->state, MIGRATION_STATUS_ACTIVE,
MIGRATION_STATUS_FAILED);
goto fail_setup;
@@ -4127,7 +4123,7 @@ void migration_connect(MigrationState *s, Error *error_in)
return;
fail:
- migrate_set_error(s, local_err);
+ migrate_error_propagate(s, error_copy(local_err));
if (s->state != MIGRATION_STATUS_CANCELLING) {
migrate_set_state(&s->state, s->state, MIGRATION_STATUS_FAILED);
}
diff --git a/migration/multifd-device-state.c b/migration/multifd-device-state.c
index db3239fef5..91d5d81556 100644
--- a/migration/multifd-device-state.c
+++ b/migration/multifd-device-state.c
@@ -143,8 +143,6 @@ static int multifd_device_state_save_thread(void *opaque)
Error *local_err = NULL;
if (!data->hdlr(data, &local_err)) {
- MigrationState *s = migrate_get_current();
-
/*
* Can't call abort_device_state_save_threads() here since new
* save threads could still be in process of being launched
@@ -158,8 +156,7 @@ static int multifd_device_state_save_thread(void *opaque)
* In case of multiple save threads failing which thread error
* return we end setting is purely arbitrary.
*/
- migrate_set_error(s, local_err);
- error_free(local_err);
+ migrate_error_propagate(migrate_get_current(), local_err);
}
return 0;
diff --git a/migration/multifd.c b/migration/multifd.c
index 52e4d25857..bf6da85af8 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -428,8 +428,9 @@ static void multifd_send_error_propagate(Error *err)
if (err) {
MigrationState *s = migrate_get_current();
- migrate_set_error(s, err);
- error_free(err);
+
+ migrate_error_propagate(s, err);
+
if (s->state == MIGRATION_STATUS_SETUP ||
s->state == MIGRATION_STATUS_PRE_SWITCHOVER ||
s->state == MIGRATION_STATUS_DEVICE ||
@@ -588,8 +589,7 @@ void multifd_send_shutdown(void)
Error *local_err = NULL;
if (!multifd_send_cleanup_channel(p, &local_err)) {
- migrate_set_error(migrate_get_current(), local_err);
- error_free(local_err);
+ migrate_error_propagate(migrate_get_current(), local_err);
}
}
@@ -962,8 +962,7 @@ bool multifd_send_setup(void)
p->write_flags = 0;
if (!multifd_new_send_channel_create(p, &local_err)) {
- migrate_set_error(s, local_err);
- error_free(local_err);
+ migrate_error_propagate(s, local_err);
ret = -1;
}
}
@@ -987,8 +986,7 @@ bool multifd_send_setup(void)
ret = multifd_send_state->ops->send_setup(p, &local_err);
if (ret) {
- migrate_set_error(s, local_err);
- error_free(local_err);
+ migrate_error_propagate(s, local_err);
goto err;
}
assert(p->iov);
@@ -1067,8 +1065,9 @@ static void multifd_recv_terminate_threads(Error *err)
if (err) {
MigrationState *s = migrate_get_current();
- migrate_set_error(s, err);
- error_free(err);
+
+ migrate_error_propagate(s, err);
+
if (s->state == MIGRATION_STATUS_SETUP ||
s->state == MIGRATION_STATUS_ACTIVE) {
migrate_set_state(&s->state, s->state,
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 715ef021a9..3623ab9dab 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1928,8 +1928,7 @@ postcopy_preempt_send_channel_done(MigrationState *s,
QIOChannel *ioc, Error *local_err)
{
if (local_err) {
- migrate_set_error(s, local_err);
- error_free(local_err);
+ migrate_error_propagate(s, local_err);
} else {
migration_ioc_register_yank(ioc);
s->postcopy_qemufile_src = qemu_file_new_output(ioc);
@@ -2163,7 +2162,7 @@ static void *postcopy_listen_thread(void *opaque)
* exit depending on if postcopy-exit-on-error is true, but the
* migration cannot be recovered.
*/
- migrate_set_error(migr, local_err);
+ migrate_error_propagate(migr, error_copy(local_err));
error_report_err(local_err);
migrate_set_state(&mis->state, mis->state, MIGRATION_STATUS_FAILED);
goto out;
diff --git a/migration/ram.c b/migration/ram.c
index 117957da91..ecd81601e2 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4748,9 +4748,7 @@ static void ram_mig_ram_block_resized(RAMBlockNotifier *n, void *host,
* Abort and indicate a proper reason.
*/
error_setg(&err, "RAM block '%s' resized during precopy.", rb->idstr);
- migrate_set_error(migrate_get_current(), err);
- error_free(err);
-
+ migrate_error_propagate(migrate_get_current(), err);
migration_cancel();
}
diff --git a/migration/savevm.c b/migration/savevm.c
index 638e9b364f..470c9ef0f7 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1125,13 +1125,13 @@ void qemu_savevm_send_open_return_path(QEMUFile *f)
int qemu_savevm_send_packaged(QEMUFile *f, const uint8_t *buf, size_t len)
{
uint32_t tmp;
- MigrationState *ms = migrate_get_current();
Error *local_err = NULL;
if (len > MAX_VM_CMD_PACKAGED_SIZE) {
error_setg(&local_err, "%s: Unreasonably large packaged state: %zu",
__func__, len);
- migrate_set_error(ms, local_err);
+ migrate_error_propagate(migrate_get_current(),
+ error_copy(local_err));
error_report_err(local_err);
return -1;
}
@@ -1373,7 +1373,7 @@ int qemu_savevm_state_setup(QEMUFile *f, Error **errp)
if (se->vmsd && se->vmsd->early_setup) {
ret = vmstate_save(f, se, vmdesc, errp);
if (ret) {
- migrate_set_error(ms, *errp);
+ migrate_error_propagate(ms, error_copy(*errp));
qemu_file_set_error(f, ret);
break;
}
@@ -1681,7 +1681,7 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
ret = vmstate_save(f, se, vmdesc, &local_err);
if (ret) {
- migrate_set_error(ms, local_err);
+ migrate_error_propagate(ms, error_copy(local_err));
error_report_err(local_err);
qemu_file_set_error(f, ret);
return ret;
@@ -1858,7 +1858,6 @@ void qemu_savevm_live_state(QEMUFile *f)
int qemu_save_device_state(QEMUFile *f)
{
- MigrationState *ms = migrate_get_current();
Error *local_err = NULL;
SaveStateEntry *se;
@@ -1876,7 +1875,8 @@ int qemu_save_device_state(QEMUFile *f)
}
ret = vmstate_save(f, se, NULL, &local_err);
if (ret) {
- migrate_set_error(ms, local_err);
+ migrate_error_propagate(migrate_get_current(),
+ error_copy(local_err));
error_report_err(local_err);
return ret;
}
@@ -2826,8 +2826,6 @@ static int qemu_loadvm_load_thread(void *thread_opaque)
Error *local_err = NULL;
if (!data->function(data->opaque, &mis->load_threads_abort, &local_err)) {
- MigrationState *s = migrate_get_current();
-
/*
* Can't set load_threads_abort here since processing of main migration
* channel data could still be happening, resulting in launching of new
@@ -2840,8 +2838,7 @@ static int qemu_loadvm_load_thread(void *thread_opaque)
* In case of multiple load threads failing which thread error
* return we end setting is purely arbitrary.
*/
- migrate_set_error(s, local_err);
- error_free(local_err);
+ migrate_error_propagate(migrate_get_current(), local_err);
}
return 0;
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 12/31] migration: Use error_propagate() in migrate_error_propagate()
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (10 preceding siblings ...)
2025-12-23 14:29 ` [PULL 11/31] migration: Replace migrate_set_error() with migrate_error_propagate() Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 13/31] migration/options: Add x-ignore-shared Peter Xu
` (19 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Markus Armbruster
It improves readability, as suggested by Markus.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Link: https://lore.kernel.org/r/20251202175317.1186544-1-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/migration.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 70813e5006..d55fde222a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1555,12 +1555,7 @@ void migrate_error_propagate(MigrationState *s, Error *error)
{
QEMU_LOCK_GUARD(&s->error_mutex);
trace_migrate_error(error_get_pretty(error));
-
- if (!s->error) {
- s->error = error;
- } else {
- error_free(error);
- }
+ error_propagate(&s->error, error);
}
bool migrate_has_error(MigrationState *s)
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 13/31] migration/options: Add x-ignore-shared
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (11 preceding siblings ...)
2025-12-23 14:29 ` [PULL 12/31] migration: Use error_propagate() in migrate_error_propagate() Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 14/31] MAINTAINERS: Update reviewers for CPR Peter Xu
` (18 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu
This aids scriptings only.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251205172054.288909-1-peterx@redhat.com
[peterx: make the property x-ignore-shared to match, per cedric]
[peterx: fix over-80 line]
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/options.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/migration/options.c b/migration/options.c
index e78324b80c..ea19b2c7cd 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -203,6 +203,8 @@ const Property migration_properties[] = {
MIGRATION_CAPABILITY_SWITCHOVER_ACK),
DEFINE_PROP_MIG_CAP("x-dirty-limit", MIGRATION_CAPABILITY_DIRTY_LIMIT),
DEFINE_PROP_MIG_CAP("mapped-ram", MIGRATION_CAPABILITY_MAPPED_RAM),
+ DEFINE_PROP_MIG_CAP("x-ignore-shared",
+ MIGRATION_CAPABILITY_X_IGNORE_SHARED),
};
const size_t migration_properties_count = ARRAY_SIZE(migration_properties);
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 14/31] MAINTAINERS: Update reviewers for CPR
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (12 preceding siblings ...)
2025-12-23 14:29 ` [PULL 13/31] migration/options: Add x-ignore-shared Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 15/31] migration: Fix leak of block_bitmap_mapping Peter Xu
` (17 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Ben Chaney
From: Ben Chaney <bchaney@akamai.com>
Signed-off-by: Ben Chaney <bchaney@akamai.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251210143624.416697-1-bchaney@akamai.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 63e9ba521b..dc6235e174 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3118,6 +3118,8 @@ T: git https://gitlab.com/vsementsov/qemu.git block
CheckPoint and Restart (CPR)
R: Peter Xu <peterx@redhat.com>
R: Fabiano Rosas <farosas@suse.de>
+R: Mark Kanda <mark.kanda@oracle.com>
+R: Ben Chaney <bchaney@akamai.com>
S: Supported
F: hw/vfio/cpr*
F: include/hw/vfio/vfio-cpr.h
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 15/31] migration: Fix leak of block_bitmap_mapping
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (13 preceding siblings ...)
2025-12-23 14:29 ` [PULL 14/31] MAINTAINERS: Update reviewers for CPR Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 16/31] migration: Fix leak of cpr_exec_command Peter Xu
` (16 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Markus Armbruster
From: Fabiano Rosas <farosas@suse.de>
Caught by inspection, but ASAN also reports:
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 in malloc
#1 in g_malloc
#2 in g_memdup
#3 in qapi_clone_start_struct ../qapi/qapi-clone-visitor.c:40:12
#4 in qapi_clone_start_list ../qapi/qapi-clone-visitor.c:59:12
#5 in visit_start_list ../qapi/qapi-visit-core.c:80:10
#6 in visit_type_BitmapMigrationNodeAliasList qapi/qapi-visit-migration.c:639:10
#7 in migrate_params_apply ../migration/options.c:1407:13
#8 in qmp_migrate_set_parameters ../migration/options.c:1463:5
#9 in qmp_marshal_migrate_set_parameters qapi/qapi-commands-migration.c:214:5
#10 in do_qmp_dispatch_bh ../qapi/qmp-dispatch.c:128:5
Note that this is entirely harmless because the migration object which
contains the MigrationParameters structure is kept until the QEMU
process exits.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-2-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/migration.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/migration/migration.c b/migration/migration.c
index d55fde222a..1ff728b6a2 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -4139,6 +4139,7 @@ static void migration_instance_finalize(Object *obj)
{
MigrationState *ms = MIGRATION_OBJ(obj);
+ qapi_free_BitmapMigrationNodeAliasList(ms->parameters.block_bitmap_mapping);
qemu_mutex_destroy(&ms->error_mutex);
qemu_mutex_destroy(&ms->qemu_file_lock);
qemu_sem_destroy(&ms->wait_unplug_sem);
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 16/31] migration: Fix leak of cpr_exec_command
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (14 preceding siblings ...)
2025-12-23 14:29 ` [PULL 15/31] migration: Fix leak of block_bitmap_mapping Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 17/31] migration: Add a qdev property for StrOrNull Peter Xu
` (15 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu
From: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/r/20251215220041.12657-3-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/migration.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/migration/migration.c b/migration/migration.c
index 1ff728b6a2..f1378c3bf4 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -4140,6 +4140,7 @@ static void migration_instance_finalize(Object *obj)
MigrationState *ms = MIGRATION_OBJ(obj);
qapi_free_BitmapMigrationNodeAliasList(ms->parameters.block_bitmap_mapping);
+ qapi_free_strList(ms->parameters.cpr_exec_command);
qemu_mutex_destroy(&ms->error_mutex);
qemu_mutex_destroy(&ms->qemu_file_lock);
qemu_sem_destroy(&ms->wait_unplug_sem);
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 17/31] migration: Add a qdev property for StrOrNull
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (15 preceding siblings ...)
2025-12-23 14:29 ` [PULL 16/31] migration: Fix leak of cpr_exec_command Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2026-01-22 13:21 ` Peter Maydell
2026-01-22 13:23 ` Peter Maydell
2025-12-23 14:29 ` [PULL 18/31] tests/qtest/migration: Add a NULL parameters test for TLS Peter Xu
` (14 subsequent siblings)
31 siblings, 2 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu
From: Fabiano Rosas <farosas@suse.de>
The MigrationState is a QOM object with TYPE_DEVICE as a parent. This
was done about eight years ago so the migration code could make use of
qdev properties to define the defaults for the migration parameters
and to be able to expose migration knobs for debugging via the
'-global migration' command line option.
Due to unrelated historical reasons, three of the migration parameters
(TLS options) received different types when used via the
query-migrate-parameters QMP command than with the
migrate-set-parameters command. This has created a lot of duplication
in the migration code and in the QAPI documentation because the whole
of MigrationParameters had to be duplicated as well.
The migration code is now being fixed to remove the duplication and
for that to happen the offending fields need to be reconciled into a
single type. The StrOrNull type is going to be used.
To keep the command line compatibility, the parameters need to
continue being exposed via qdev properties accessible from the command
line. Introduce a qdev property StrOrNull just for that.
Note that this code is being kept in migration/options.c as this
version of StrOrNull doesn't need to handle QNULL because it was never
a valid option in the previous command line, which took a string.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Acked-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/r/20251215220041.12657-4-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/options.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 67 insertions(+)
diff --git a/migration/options.c b/migration/options.c
index ea19b2c7cd..d55f3104be 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -83,6 +83,11 @@
#define DEFINE_PROP_MIG_CAP(name, x) \
DEFINE_PROP_BOOL(name, MigrationState, capabilities[x], false)
+const PropertyInfo qdev_prop_StrOrNull;
+#define DEFINE_PROP_STR_OR_NULL(_name, _state, _field) \
+ DEFINE_PROP(_name, _state, _field, qdev_prop_StrOrNull, StrOrNull *, \
+ .set_default = true)
+
#define DEFAULT_MIGRATE_VCPU_DIRTY_LIMIT_PERIOD 1000 /* milliseconds */
#define DEFAULT_MIGRATE_VCPU_DIRTY_LIMIT 1 /* MB/s */
@@ -208,6 +213,68 @@ const Property migration_properties[] = {
};
const size_t migration_properties_count = ARRAY_SIZE(migration_properties);
+static void get_StrOrNull(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ const Property *prop = opaque;
+ StrOrNull **ptr = object_field_prop_ptr(obj, prop);
+ StrOrNull *str_or_null = *ptr;
+
+ if (!str_or_null) {
+ str_or_null = g_new0(StrOrNull, 1);
+ str_or_null->type = QTYPE_QSTRING;
+ str_or_null->u.s = g_strdup("");
+ } else {
+ /* the setter doesn't allow QNULL */
+ assert(str_or_null->type != QTYPE_QNULL);
+ }
+ visit_type_str(v, name, &str_or_null->u.s, errp);
+}
+
+static void set_StrOrNull(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ const Property *prop = opaque;
+ StrOrNull **ptr = object_field_prop_ptr(obj, prop);
+ StrOrNull *str_or_null = g_new0(StrOrNull, 1);
+
+ /*
+ * Only str to keep compatibility, QNULL was never used via
+ * command line.
+ */
+ str_or_null->type = QTYPE_QSTRING;
+ if (!visit_type_str(v, name, &str_or_null->u.s, errp)) {
+ return;
+ }
+
+ qapi_free_StrOrNull(*ptr);
+ *ptr = str_or_null;
+}
+
+static void release_StrOrNull(Object *obj, const char *name, void *opaque)
+{
+ const Property *prop = opaque;
+ qapi_free_StrOrNull(*(StrOrNull **)object_field_prop_ptr(obj, prop));
+}
+
+static void set_default_value_tls_opt(ObjectProperty *op, const Property *prop)
+{
+ object_property_set_default_str(op, "");
+}
+
+/*
+ * String property like qdev_prop_string, except it's backed by a
+ * StrOrNull instead of a char *. This is intended for
+ * TYPE_MIGRATION's TLS options.
+ */
+const PropertyInfo qdev_prop_StrOrNull = {
+ .type = "StrOrNull",
+ .get = get_StrOrNull,
+ .set = set_StrOrNull,
+ .release = release_StrOrNull,
+ .set_default_value = set_default_value_tls_opt,
+};
+
bool migrate_auto_converge(void)
{
MigrationState *s = migrate_get_current();
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PULL 17/31] migration: Add a qdev property for StrOrNull
2025-12-23 14:29 ` [PULL 17/31] migration: Add a qdev property for StrOrNull Peter Xu
@ 2026-01-22 13:21 ` Peter Maydell
2026-01-22 13:23 ` Peter Maydell
1 sibling, 0 replies; 39+ messages in thread
From: Peter Maydell @ 2026-01-22 13:21 UTC (permalink / raw)
To: Peter Xu; +Cc: qemu-devel, Fabiano Rosas
On Tue, 23 Dec 2025 at 14:32, Peter Xu <peterx@redhat.com> wrote:
>
> From: Fabiano Rosas <farosas@suse.de>
>
> The MigrationState is a QOM object with TYPE_DEVICE as a parent. This
> was done about eight years ago so the migration code could make use of
> qdev properties to define the defaults for the migration parameters
> and to be able to expose migration knobs for debugging via the
> '-global migration' command line option.
>
> Due to unrelated historical reasons, three of the migration parameters
> (TLS options) received different types when used via the
> query-migrate-parameters QMP command than with the
> migrate-set-parameters command. This has created a lot of duplication
> in the migration code and in the QAPI documentation because the whole
> of MigrationParameters had to be duplicated as well.
>
> The migration code is now being fixed to remove the duplication and
> for that to happen the offending fields need to be reconciled into a
> single type. The StrOrNull type is going to be used.
>
> To keep the command line compatibility, the parameters need to
> continue being exposed via qdev properties accessible from the command
> line. Introduce a qdev property StrOrNull just for that.
>
> Note that this code is being kept in migration/options.c as this
> version of StrOrNull doesn't need to handle QNULL because it was never
> a valid option in the previous command line, which took a string.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> Acked-by: Peter Xu <peterx@redhat.com>
> Link: https://lore.kernel.org/r/20251215220041.12657-4-farosas@suse.de
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> migration/options.c | 67 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 67 insertions(+)
Hi; Coverity thinks there's a memory leak in this code
(CID 1643919). Now I know Coverity gets easily confused about
our visitor patterns, and I don't myself know the way they're
supposed to handle ownership of memory, so this should be
checked by somebody who does before adding any extra calls
to free. That said:
> +static void set_StrOrNull(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + const Property *prop = opaque;
> + StrOrNull **ptr = object_field_prop_ptr(obj, prop);
> + StrOrNull *str_or_null = g_new0(StrOrNull, 1);
> +
> + /*
> + * Only str to keep compatibility, QNULL was never used via
> + * command line.
> + */
> + str_or_null->type = QTYPE_QSTRING;
> + if (!visit_type_str(v, name, &str_or_null->u.s, errp)) {
> + return;
In this error-return code path, we don't seem to hand the
"str_or_null" pointer on to anybody, and we don't free it
either -- does this leak what we just allocated with g_new0() ?
> + }
> +
> + qapi_free_StrOrNull(*ptr);
> + *ptr = str_or_null;
> +}
thanks
-- PMM
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PULL 17/31] migration: Add a qdev property for StrOrNull
2025-12-23 14:29 ` [PULL 17/31] migration: Add a qdev property for StrOrNull Peter Xu
2026-01-22 13:21 ` Peter Maydell
@ 2026-01-22 13:23 ` Peter Maydell
2026-01-22 13:49 ` Fabiano Rosas
1 sibling, 1 reply; 39+ messages in thread
From: Peter Maydell @ 2026-01-22 13:23 UTC (permalink / raw)
To: Peter Xu; +Cc: qemu-devel, Fabiano Rosas
On Tue, 23 Dec 2025 at 14:32, Peter Xu <peterx@redhat.com> wrote:
>
> From: Fabiano Rosas <farosas@suse.de>
>
> The MigrationState is a QOM object with TYPE_DEVICE as a parent. This
> was done about eight years ago so the migration code could make use of
> qdev properties to define the defaults for the migration parameters
> and to be able to expose migration knobs for debugging via the
> '-global migration' command line option.
>
> Due to unrelated historical reasons, three of the migration parameters
> (TLS options) received different types when used via the
> query-migrate-parameters QMP command than with the
> migrate-set-parameters command. This has created a lot of duplication
> in the migration code and in the QAPI documentation because the whole
> of MigrationParameters had to be duplicated as well.
>
> The migration code is now being fixed to remove the duplication and
> for that to happen the offending fields need to be reconciled into a
> single type. The StrOrNull type is going to be used.
>
> To keep the command line compatibility, the parameters need to
> continue being exposed via qdev properties accessible from the command
> line. Introduce a qdev property StrOrNull just for that.
>
> Note that this code is being kept in migration/options.c as this
> version of StrOrNull doesn't need to handle QNULL because it was never
> a valid option in the previous command line, which took a string.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> Acked-by: Peter Xu <peterx@redhat.com>
> Link: https://lore.kernel.org/r/20251215220041.12657-4-farosas@suse.de
> Signed-off-by: Peter Xu <peterx@redhat.com>
Coverity also suspects a leak in the get function (CID 1643920):
> +static void get_StrOrNull(Object *obj, Visitor *v, const char *name,
> + void *opaque, Error **errp)
> +{
> + const Property *prop = opaque;
> + StrOrNull **ptr = object_field_prop_ptr(obj, prop);
> + StrOrNull *str_or_null = *ptr;
> +
> + if (!str_or_null) {
> + str_or_null = g_new0(StrOrNull, 1);
We allocate memory here, but we never pass this pointer
on to anybody else, and we don't free it either.
> + str_or_null->type = QTYPE_QSTRING;
> + str_or_null->u.s = g_strdup("");
> + } else {
> + /* the setter doesn't allow QNULL */
> + assert(str_or_null->type != QTYPE_QNULL);
> + }
> + visit_type_str(v, name, &str_or_null->u.s, errp);
> +}
> +
thanks
-- PMM
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PULL 17/31] migration: Add a qdev property for StrOrNull
2026-01-22 13:23 ` Peter Maydell
@ 2026-01-22 13:49 ` Fabiano Rosas
0 siblings, 0 replies; 39+ messages in thread
From: Fabiano Rosas @ 2026-01-22 13:49 UTC (permalink / raw)
To: Peter Maydell, Peter Xu; +Cc: qemu-devel
Peter Maydell <peter.maydell@linaro.org> writes:
> On Tue, 23 Dec 2025 at 14:32, Peter Xu <peterx@redhat.com> wrote:
>>
>> From: Fabiano Rosas <farosas@suse.de>
>>
>> The MigrationState is a QOM object with TYPE_DEVICE as a parent. This
>> was done about eight years ago so the migration code could make use of
>> qdev properties to define the defaults for the migration parameters
>> and to be able to expose migration knobs for debugging via the
>> '-global migration' command line option.
>>
>> Due to unrelated historical reasons, three of the migration parameters
>> (TLS options) received different types when used via the
>> query-migrate-parameters QMP command than with the
>> migrate-set-parameters command. This has created a lot of duplication
>> in the migration code and in the QAPI documentation because the whole
>> of MigrationParameters had to be duplicated as well.
>>
>> The migration code is now being fixed to remove the duplication and
>> for that to happen the offending fields need to be reconciled into a
>> single type. The StrOrNull type is going to be used.
>>
>> To keep the command line compatibility, the parameters need to
>> continue being exposed via qdev properties accessible from the command
>> line. Introduce a qdev property StrOrNull just for that.
>>
>> Note that this code is being kept in migration/options.c as this
>> version of StrOrNull doesn't need to handle QNULL because it was never
>> a valid option in the previous command line, which took a string.
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> Acked-by: Peter Xu <peterx@redhat.com>
>> Link: https://lore.kernel.org/r/20251215220041.12657-4-farosas@suse.de
>> Signed-off-by: Peter Xu <peterx@redhat.com>
>
> Coverity also suspects a leak in the get function (CID 1643920):
>
>> +static void get_StrOrNull(Object *obj, Visitor *v, const char *name,
>> + void *opaque, Error **errp)
>> +{
>> + const Property *prop = opaque;
>> + StrOrNull **ptr = object_field_prop_ptr(obj, prop);
>> + StrOrNull *str_or_null = *ptr;
>> +
>> + if (!str_or_null) {
>> + str_or_null = g_new0(StrOrNull, 1);
>
> We allocate memory here, but we never pass this pointer
> on to anybody else, and we don't free it either.
>
Thank you, I'll look into both issues right away.
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PULL 18/31] tests/qtest/migration: Add a NULL parameters test for TLS
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (16 preceding siblings ...)
2025-12-23 14:29 ` [PULL 17/31] migration: Add a qdev property for StrOrNull Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 19/31] migration: Normalize tls arguments Peter Xu
` (13 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu
From: Fabiano Rosas <farosas@suse.de>
Make sure the TLS options handling is working correctly with a NULL
parameter. This is relevant due to the usage of StrOrNull for the
tls-creds, tls-authz and tls-hostname options.
With this, all manners of passing TLS options are somehow covered by
the tests, we should not need to do manual testing when touching TLS
options code.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/r/20251215220041.12657-5-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qtest/migration/migration-qmp.h | 1 +
tests/qtest/migration/migration-qmp.c | 9 +++++
tests/qtest/migration/tls-tests.c | 56 +++++++++++++++++++++++++++
3 files changed, 66 insertions(+)
diff --git a/tests/qtest/migration/migration-qmp.h b/tests/qtest/migration/migration-qmp.h
index 44482d250f..940ffd5950 100644
--- a/tests/qtest/migration/migration-qmp.h
+++ b/tests/qtest/migration/migration-qmp.h
@@ -36,6 +36,7 @@ void migrate_set_parameter_str(QTestState *who, const char *parameter,
const char *value);
void migrate_set_parameter_strv(QTestState *who, const char *parameter,
char **strv);
+void migrate_set_parameter_null(QTestState *who, const char *parameter);
void migrate_set_parameter_bool(QTestState *who, const char *parameter,
int value);
void migrate_ensure_non_converge(QTestState *who);
diff --git a/tests/qtest/migration/migration-qmp.c b/tests/qtest/migration/migration-qmp.c
index c803fcee9d..5c46ceb3e6 100644
--- a/tests/qtest/migration/migration-qmp.c
+++ b/tests/qtest/migration/migration-qmp.c
@@ -458,6 +458,15 @@ void migrate_set_parameter_strv(QTestState *who, const char *parameter,
qtest_qmp_assert_success(who, command, parameter);
}
+void migrate_set_parameter_null(QTestState *who, const char *parameter)
+{
+ qtest_qmp_assert_success(who,
+ "{ 'execute': 'migrate-set-parameters',"
+ "'arguments': { %s: null } }",
+ parameter);
+ migrate_check_parameter_str(who, parameter, "");
+}
+
static long long migrate_get_parameter_bool(QTestState *who,
const char *parameter)
{
diff --git a/tests/qtest/migration/tls-tests.c b/tests/qtest/migration/tls-tests.c
index 21e9fec87d..e0e8a7335c 100644
--- a/tests/qtest/migration/tls-tests.c
+++ b/tests/qtest/migration/tls-tests.c
@@ -507,6 +507,57 @@ static void test_precopy_tcp_tls_psk_mismatch(void)
test_precopy_common(&args);
}
+static void *migrate_hook_start_no_tls(QTestState *from, QTestState *to)
+{
+ struct TestMigrateTLSPSKData *data =
+ g_new0(struct TestMigrateTLSPSKData, 1);
+
+ migrate_set_parameter_null(from, "tls-creds");
+ migrate_set_parameter_null(to, "tls-creds");
+
+ return data;
+}
+
+static void test_precopy_tcp_no_tls(void)
+{
+ MigrateCommon args = {
+ .listen_uri = "tcp:127.0.0.1:0",
+ .start_hook = migrate_hook_start_no_tls,
+ .end_hook = migrate_hook_end_tls_psk,
+ };
+
+ test_precopy_common(&args);
+}
+
+static void *
+migrate_hook_start_tls_x509_no_host(QTestState *from, QTestState *to)
+{
+ TestMigrateTLSX509 args = {
+ .verifyclient = true,
+ .clientcert = true,
+ .authzclient = true,
+ };
+ TestMigrateTLSX509Data *data = migrate_hook_start_tls_x509_common(from, to,
+ &args);
+ migrate_set_parameter_null(from, "tls-hostname");
+ migrate_set_parameter_null(to, "tls-hostname");
+
+ return data;
+}
+
+static void test_precopy_tcp_tls_no_hostname(void)
+{
+ MigrateCommon args = {
+ .listen_uri = "tcp:127.0.0.1:0",
+ .start_hook = migrate_hook_start_tls_x509_no_host,
+ .end_hook = migrate_hook_end_tls_x509,
+ .result = MIG_TEST_FAIL_DEST_QUIT_ERR,
+ .start.hide_stderr = true,
+ };
+
+ test_precopy_common(&args);
+}
+
#ifdef CONFIG_TASN1
static void test_precopy_tcp_tls_x509_default_host(void)
{
@@ -799,6 +850,11 @@ void migration_test_add_tls(MigrationTestEnv *env)
return;
}
+ migration_test_add("/migration/precopy/tcp/no-tls",
+ test_precopy_tcp_no_tls);
+ migration_test_add("/migration/precopy/tcp/tls/no-hostname",
+ test_precopy_tcp_tls_no_hostname);
+
migration_test_add("/migration/precopy/unix/tls/psk",
test_precopy_unix_tls_psk);
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 19/31] migration: Normalize tls arguments
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (17 preceding siblings ...)
2025-12-23 14:29 ` [PULL 18/31] tests/qtest/migration: Add a NULL parameters test for TLS Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2026-04-14 16:56 ` Maciej S. Szmigiero
2025-12-23 14:29 ` [PULL 20/31] migration: Remove MigrateSetParameters Peter Xu
` (12 subsequent siblings)
31 siblings, 1 reply; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Markus Armbruster
From: Fabiano Rosas <farosas@suse.de>
The migration parameters tls_creds, tls_authz and tls_hostname
currently have a non-uniform handling. When used as arguments to
migrate-set-parameters, their type is StrOrNull and when used as
return value from query-migrate-parameters their type is a plain
string.
Not only having to convert between the types is cumbersome, but it
also creates the issue of requiring two different QAPI types to be
used, one for each command. MigrateSetParameters is used for
migrate-set-parameters with the TLS arguments as StrOrNull while
MigrationParameters is used for query-migrate-parameters with the TLS
arguments as str.
Since StrOrNull could be considered a superset of str, change the type
of the TLS arguments in MigrationParameters to StrOrNull. Also ensure
that QTYPE_QNULL is never used.
1) migrate-set-parameters will always write QTYPE_QSTRING to
s->parameters, either an empty or non-empty string.
2) query-migrate-parameters will always return a QTYPE_QSTRING, either
empty or non-empty.
3) the migrate_tls_* helpers will always return a non-empty string or
NULL, for the internal migration code's consumption.
Points (1) and (2) above help simplify the parameters validation and
the query command handling because s->parameters is already kept in
the format that query-migrate-parameters (and info migrate_paramters)
expect. Point (3) is so people don't need to care about StrOrNull in
migration code.
This will allow the type duplication to be removed in the next
patches.
Note that the type of @tls_creds, @tls-hostname, @tls-authz changes
from str to StrOrNull in introspection of the query-migrate-parameters
command. We accept this imprecision to enable de-duplication.
There's no need to free the TLS options in
migration_instance_finalize() because they're freed by the qdev
properties .release method.
Temporary in this patch:
migrate_params_test_apply() copies s->parameters into a temporary
structure, so it's necessary to drop the references to the TLS options
if they were not set by the user to avoid double-free. This is fixed
in the next patches.
Acked-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-6-farosas@suse.de
[peterx: in hmp_info_migrate_parameters(), remove an extra dump of
max_postcopy_bandwidth, introduced likely by accident]
Signed-off-by: Peter Xu <peterx@redhat.com>
---
qapi/migration.json | 6 +-
migration/options.h | 1 +
migration/migration-hmp-cmds.c | 6 +-
migration/options.c | 144 +++++++++++++++++++--------------
migration/tls.c | 2 +-
5 files changed, 93 insertions(+), 66 deletions(-)
diff --git a/qapi/migration.json b/qapi/migration.json
index cf023bd29d..30a0eb2d7e 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1382,9 +1382,9 @@
'*cpu-throttle-initial': 'uint8',
'*cpu-throttle-increment': 'uint8',
'*cpu-throttle-tailslow': 'bool',
- '*tls-creds': 'str',
- '*tls-hostname': 'str',
- '*tls-authz': 'str',
+ '*tls-creds': 'StrOrNull',
+ '*tls-hostname': 'StrOrNull',
+ '*tls-authz': 'StrOrNull',
'*max-bandwidth': 'size',
'*avail-switchover-bandwidth': 'size',
'*downtime-limit': 'uint64',
diff --git a/migration/options.h b/migration/options.h
index a7b3262d1e..25fb316420 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -92,4 +92,5 @@ ZeroPageDetection migrate_zero_page_detection(void);
bool migrate_params_check(MigrationParameters *params, Error **errp);
void migrate_params_init(MigrationParameters *params);
+void migrate_tls_opts_free(MigrationParameters *params);
#endif
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 79426bf5d7..edc561a34a 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -360,15 +360,15 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
assert(params->tls_creds);
monitor_printf(mon, "%s: '%s'\n",
MigrationParameter_str(MIGRATION_PARAMETER_TLS_CREDS),
- params->tls_creds);
+ params->tls_creds->u.s);
assert(params->tls_hostname);
monitor_printf(mon, "%s: '%s'\n",
MigrationParameter_str(MIGRATION_PARAMETER_TLS_HOSTNAME),
- params->tls_hostname);
+ params->tls_hostname->u.s);
assert(params->tls_authz);
monitor_printf(mon, "%s: '%s'\n",
MigrationParameter_str(MIGRATION_PARAMETER_TLS_AUTHZ),
- params->tls_authz);
+ params->tls_authz->u.s);
assert(params->has_max_bandwidth);
monitor_printf(mon, "%s: %" PRIu64 " bytes/second\n",
MigrationParameter_str(MIGRATION_PARAMETER_MAX_BANDWIDTH),
diff --git a/migration/options.c b/migration/options.c
index d55f3104be..6ef3c56fb6 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -167,9 +167,10 @@ const Property migration_properties[] = {
DEFINE_PROP_SIZE("announce-step", MigrationState,
parameters.announce_step,
DEFAULT_MIGRATE_ANNOUNCE_STEP),
- DEFINE_PROP_STRING("tls-creds", MigrationState, parameters.tls_creds),
- DEFINE_PROP_STRING("tls-hostname", MigrationState, parameters.tls_hostname),
- DEFINE_PROP_STRING("tls-authz", MigrationState, parameters.tls_authz),
+ DEFINE_PROP_STR_OR_NULL("tls-creds", MigrationState, parameters.tls_creds),
+ DEFINE_PROP_STR_OR_NULL("tls-hostname", MigrationState,
+ parameters.tls_hostname),
+ DEFINE_PROP_STR_OR_NULL("tls-authz", MigrationState, parameters.tls_authz),
DEFINE_PROP_UINT64("x-vcpu-dirty-limit-period", MigrationState,
parameters.x_vcpu_dirty_limit_period,
DEFAULT_MIGRATE_VCPU_DIRTY_LIMIT_PERIOD),
@@ -259,6 +260,11 @@ static void release_StrOrNull(Object *obj, const char *name, void *opaque)
static void set_default_value_tls_opt(ObjectProperty *op, const Property *prop)
{
+ /*
+ * Initialization to the empty string here is important so
+ * query-migrate-parameters doesn't need to deal with a NULL value
+ * when it's called before any TLS option has been set.
+ */
object_property_set_default_str(op, "");
}
@@ -450,13 +456,6 @@ bool migrate_rdma(void)
return s->rdma_migration;
}
-bool migrate_tls(void)
-{
- MigrationState *s = migrate_get_current();
-
- return s->parameters.tls_creds && *s->parameters.tls_creds;
-}
-
typedef enum WriteTrackingSupport {
WT_SUPPORT_UNKNOWN = 0,
WT_SUPPORT_ABSENT,
@@ -931,21 +930,38 @@ const char *migrate_tls_authz(void)
{
MigrationState *s = migrate_get_current();
- return s->parameters.tls_authz;
+ if (*s->parameters.tls_authz->u.s) {
+ return s->parameters.tls_authz->u.s;
+ }
+
+ return NULL;
}
const char *migrate_tls_creds(void)
{
MigrationState *s = migrate_get_current();
- return s->parameters.tls_creds;
+ if (*s->parameters.tls_creds->u.s) {
+ return s->parameters.tls_creds->u.s;
+ }
+
+ return NULL;
}
const char *migrate_tls_hostname(void)
{
MigrationState *s = migrate_get_current();
- return s->parameters.tls_hostname;
+ if (*s->parameters.tls_hostname->u.s) {
+ return s->parameters.tls_hostname->u.s;
+ }
+
+ return NULL;
+}
+
+bool migrate_tls(void)
+{
+ return !!migrate_tls_creds();
}
uint64_t migrate_vcpu_dirty_limit_period(void)
@@ -985,6 +1001,25 @@ AnnounceParameters *migrate_announce_params(void)
return ≈
}
+void migrate_tls_opts_free(MigrationParameters *params)
+{
+ qapi_free_StrOrNull(params->tls_creds);
+ qapi_free_StrOrNull(params->tls_hostname);
+ qapi_free_StrOrNull(params->tls_authz);
+}
+
+/* normalize QTYPE_QNULL to QTYPE_QSTRING "" */
+static void tls_opt_to_str(StrOrNull *opt)
+{
+ if (!opt || opt->type == QTYPE_QSTRING) {
+ return;
+ }
+
+ qobject_unref(opt->u.n);
+ opt->type = QTYPE_QSTRING;
+ opt->u.s = g_strdup("");
+}
+
MigrationParameters *qmp_query_migrate_parameters(Error **errp)
{
MigrationParameters *params;
@@ -1000,10 +1035,9 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
params->cpu_throttle_increment = s->parameters.cpu_throttle_increment;
params->has_cpu_throttle_tailslow = true;
params->cpu_throttle_tailslow = s->parameters.cpu_throttle_tailslow;
- params->tls_creds = g_strdup(s->parameters.tls_creds);
- params->tls_hostname = g_strdup(s->parameters.tls_hostname);
- params->tls_authz = g_strdup(s->parameters.tls_authz ?
- s->parameters.tls_authz : "");
+ params->tls_creds = QAPI_CLONE(StrOrNull, s->parameters.tls_creds);
+ params->tls_hostname = QAPI_CLONE(StrOrNull, s->parameters.tls_hostname);
+ params->tls_authz = QAPI_CLONE(StrOrNull, s->parameters.tls_authz);
params->has_max_bandwidth = true;
params->max_bandwidth = s->parameters.max_bandwidth;
params->has_avail_switchover_bandwidth = true;
@@ -1063,9 +1097,6 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
void migrate_params_init(MigrationParameters *params)
{
- params->tls_hostname = g_strdup("");
- params->tls_creds = g_strdup("");
-
/* Set has_* up only for parameter checks */
params->has_throttle_trigger_threshold = true;
params->has_cpu_throttle_initial = true;
@@ -1243,7 +1274,7 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
#ifdef CONFIG_LINUX
if (migrate_zero_copy_send() &&
((params->has_multifd_compression && params->multifd_compression) ||
- (params->tls_creds && *params->tls_creds))) {
+ *params->tls_creds->u.s)) {
error_setg(errp,
"Zero copy only available for non-compressed non-TLS multifd migration");
return false;
@@ -1305,18 +1336,24 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
}
if (params->tls_creds) {
- assert(params->tls_creds->type == QTYPE_QSTRING);
- dest->tls_creds = params->tls_creds->u.s;
+ dest->tls_creds = QAPI_CLONE(StrOrNull, params->tls_creds);
+ } else {
+ /* clear the reference, it's owned by s->parameters */
+ dest->tls_creds = NULL;
}
if (params->tls_hostname) {
- assert(params->tls_hostname->type == QTYPE_QSTRING);
- dest->tls_hostname = params->tls_hostname->u.s;
+ dest->tls_hostname = QAPI_CLONE(StrOrNull, params->tls_hostname);
+ } else {
+ /* clear the reference, it's owned by s->parameters */
+ dest->tls_hostname = NULL;
}
if (params->tls_authz) {
- assert(params->tls_authz->type == QTYPE_QSTRING);
- dest->tls_authz = params->tls_authz->u.s;
+ dest->tls_authz = QAPI_CLONE(StrOrNull, params->tls_authz);
+ } else {
+ /* clear the reference, it's owned by s->parameters */
+ dest->tls_authz = NULL;
}
if (params->has_max_bandwidth) {
@@ -1425,21 +1462,19 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
}
if (params->tls_creds) {
- g_free(s->parameters.tls_creds);
- assert(params->tls_creds->type == QTYPE_QSTRING);
- s->parameters.tls_creds = g_strdup(params->tls_creds->u.s);
+ qapi_free_StrOrNull(s->parameters.tls_creds);
+ s->parameters.tls_creds = QAPI_CLONE(StrOrNull, params->tls_creds);
}
if (params->tls_hostname) {
- g_free(s->parameters.tls_hostname);
- assert(params->tls_hostname->type == QTYPE_QSTRING);
- s->parameters.tls_hostname = g_strdup(params->tls_hostname->u.s);
+ qapi_free_StrOrNull(s->parameters.tls_hostname);
+ s->parameters.tls_hostname = QAPI_CLONE(StrOrNull,
+ params->tls_hostname);
}
if (params->tls_authz) {
- g_free(s->parameters.tls_authz);
- assert(params->tls_authz->type == QTYPE_QSTRING);
- s->parameters.tls_authz = g_strdup(params->tls_authz->u.s);
+ qapi_free_StrOrNull(s->parameters.tls_authz);
+ s->parameters.tls_authz = QAPI_CLONE(StrOrNull, params->tls_authz);
}
if (params->has_max_bandwidth) {
@@ -1544,32 +1579,23 @@ void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
{
MigrationParameters tmp;
- /* TODO Rewrite "" to null instead for all three tls_* parameters */
- if (params->tls_creds
- && params->tls_creds->type == QTYPE_QNULL) {
- qobject_unref(params->tls_creds->u.n);
- params->tls_creds->type = QTYPE_QSTRING;
- params->tls_creds->u.s = strdup("");
- }
- if (params->tls_hostname
- && params->tls_hostname->type == QTYPE_QNULL) {
- qobject_unref(params->tls_hostname->u.n);
- params->tls_hostname->type = QTYPE_QSTRING;
- params->tls_hostname->u.s = strdup("");
- }
- if (params->tls_authz
- && params->tls_authz->type == QTYPE_QNULL) {
- qobject_unref(params->tls_authz->u.n);
- params->tls_authz->type = QTYPE_QSTRING;
- params->tls_authz->u.s = strdup("");
- }
+ /*
+ * Convert QTYPE_QNULL and NULL to the empty string (""). Even
+ * though NULL is cleaner to deal with in C code, that would force
+ * query-migrate-parameters to convert it once more to the empty
+ * string, so avoid that. The migrate_tls_*() helpers that expose
+ * the options to the rest of the migration code already use
+ * return NULL when the empty string is found.
+ */
+ tls_opt_to_str(params->tls_creds);
+ tls_opt_to_str(params->tls_hostname);
+ tls_opt_to_str(params->tls_authz);
migrate_params_test_apply(params, &tmp);
- if (!migrate_params_check(&tmp, errp)) {
- /* Invalid parameter */
- return;
+ if (migrate_params_check(&tmp, errp)) {
+ migrate_params_apply(params, errp);
}
- migrate_params_apply(params, errp);
+ migrate_tls_opts_free(&tmp);
}
diff --git a/migration/tls.c b/migration/tls.c
index 284a6194b2..56b5d1cc90 100644
--- a/migration/tls.c
+++ b/migration/tls.c
@@ -130,7 +130,7 @@ QIOChannelTLS *migration_tls_client_create(QIOChannel *ioc,
}
const char *tls_hostname = migrate_tls_hostname();
- if (tls_hostname && *tls_hostname) {
+ if (tls_hostname) {
hostname = tls_hostname;
}
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PULL 19/31] migration: Normalize tls arguments
2025-12-23 14:29 ` [PULL 19/31] migration: Normalize tls arguments Peter Xu
@ 2026-04-14 16:56 ` Maciej S. Szmigiero
2026-04-14 18:58 ` Peter Xu
0 siblings, 1 reply; 39+ messages in thread
From: Maciej S. Szmigiero @ 2026-04-14 16:56 UTC (permalink / raw)
To: Peter Xu, Fabiano Rosas; +Cc: Markus Armbruster, qemu-devel
On 23.12.2025 15:29, Peter Xu wrote:
> From: Fabiano Rosas <farosas@suse.de>
>
> The migration parameters tls_creds, tls_authz and tls_hostname
> currently have a non-uniform handling. When used as arguments to
> migrate-set-parameters, their type is StrOrNull and when used as
> return value from query-migrate-parameters their type is a plain
> string.
>
> Not only having to convert between the types is cumbersome, but it
> also creates the issue of requiring two different QAPI types to be
> used, one for each command. MigrateSetParameters is used for
> migrate-set-parameters with the TLS arguments as StrOrNull while
> MigrationParameters is used for query-migrate-parameters with the TLS
> arguments as str.
>
> Since StrOrNull could be considered a superset of str, change the type
> of the TLS arguments in MigrationParameters to StrOrNull. Also ensure
> that QTYPE_QNULL is never used.
>
> 1) migrate-set-parameters will always write QTYPE_QSTRING to
> s->parameters, either an empty or non-empty string.
>
> 2) query-migrate-parameters will always return a QTYPE_QSTRING, either
> empty or non-empty.
>
> 3) the migrate_tls_* helpers will always return a non-empty string or
> NULL, for the internal migration code's consumption.
>
> Points (1) and (2) above help simplify the parameters validation and
> the query command handling because s->parameters is already kept in
> the format that query-migrate-parameters (and info migrate_paramters)
> expect. Point (3) is so people don't need to care about StrOrNull in
> migration code.
>
> This will allow the type duplication to be removed in the next
> patches.
>
> Note that the type of @tls_creds, @tls-hostname, @tls-authz changes
> from str to StrOrNull in introspection of the query-migrate-parameters
> command. We accept this imprecision to enable de-duplication.
>
> There's no need to free the TLS options in
> migration_instance_finalize() because they're freed by the qdev
> properties .release method.
>
> Temporary in this patch:
> migrate_params_test_apply() copies s->parameters into a temporary
> structure, so it's necessary to drop the references to the TLS options
> if they were not set by the user to avoid double-free. This is fixed
> in the next patches.
>
> Acked-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> Link: https://lore.kernel.org/r/20251215220041.12657-6-farosas@suse.de
> [peterx: in hmp_info_migrate_parameters(), remove an extra dump of
> max_postcopy_bandwidth, introduced likely by accident]
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
> qapi/migration.json | 6 +-
> migration/options.h | 1 +
> migration/migration-hmp-cmds.c | 6 +-
> migration/options.c | 144 +++++++++++++++++++--------------
> migration/tls.c | 2 +-
> 5 files changed, 93 insertions(+), 66 deletions(-)
>
> diff --git a/migration/options.c b/migration/options.c
> index d55f3104be..6ef3c56fb6 100644
> --- a/migration/options.c
> +++ b/migration/options.c
(..)
> @@ -1243,7 +1274,7 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
> #ifdef CONFIG_LINUX
> if (migrate_zero_copy_send() &&
> ((params->has_multifd_compression && params->multifd_compression) ||
> - (params->tls_creds && *params->tls_creds))) {
> + *params->tls_creds->u.s)) {
> error_setg(errp,
> "Zero copy only available for non-compressed non-TLS multifd migration");
> return false;
The above change gives me easily triggerable NULL pointer dereference:
> $ qemu-system-x86_64 -monitor stdio -global migration.x-multifd=true -global migration.x-zero-copy-send=true
> QEMU 10.2.93 monitor - type 'help' for more information
> VNC server running on ::1:5900
> (qemu) migrate_set_parameter downtime-limit 500
> Segmentation fault
I guess params->tls_creds really needs that NULL check before being accessed.
Thanks,
Maciej
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PULL 19/31] migration: Normalize tls arguments
2026-04-14 16:56 ` Maciej S. Szmigiero
@ 2026-04-14 18:58 ` Peter Xu
2026-04-14 19:34 ` Fabiano Rosas
0 siblings, 1 reply; 39+ messages in thread
From: Peter Xu @ 2026-04-14 18:58 UTC (permalink / raw)
To: Maciej S. Szmigiero; +Cc: Fabiano Rosas, Markus Armbruster, qemu-devel
On Tue, Apr 14, 2026 at 06:56:27PM +0200, Maciej S. Szmigiero wrote:
> On 23.12.2025 15:29, Peter Xu wrote:
> > From: Fabiano Rosas <farosas@suse.de>
> >
> > The migration parameters tls_creds, tls_authz and tls_hostname
> > currently have a non-uniform handling. When used as arguments to
> > migrate-set-parameters, their type is StrOrNull and when used as
> > return value from query-migrate-parameters their type is a plain
> > string.
> >
> > Not only having to convert between the types is cumbersome, but it
> > also creates the issue of requiring two different QAPI types to be
> > used, one for each command. MigrateSetParameters is used for
> > migrate-set-parameters with the TLS arguments as StrOrNull while
> > MigrationParameters is used for query-migrate-parameters with the TLS
> > arguments as str.
> >
> > Since StrOrNull could be considered a superset of str, change the type
> > of the TLS arguments in MigrationParameters to StrOrNull. Also ensure
> > that QTYPE_QNULL is never used.
> >
> > 1) migrate-set-parameters will always write QTYPE_QSTRING to
> > s->parameters, either an empty or non-empty string.
> >
> > 2) query-migrate-parameters will always return a QTYPE_QSTRING, either
> > empty or non-empty.
> >
> > 3) the migrate_tls_* helpers will always return a non-empty string or
> > NULL, for the internal migration code's consumption.
> >
> > Points (1) and (2) above help simplify the parameters validation and
> > the query command handling because s->parameters is already kept in
> > the format that query-migrate-parameters (and info migrate_paramters)
> > expect. Point (3) is so people don't need to care about StrOrNull in
> > migration code.
> >
> > This will allow the type duplication to be removed in the next
> > patches.
> >
> > Note that the type of @tls_creds, @tls-hostname, @tls-authz changes
> > from str to StrOrNull in introspection of the query-migrate-parameters
> > command. We accept this imprecision to enable de-duplication.
> >
> > There's no need to free the TLS options in
> > migration_instance_finalize() because they're freed by the qdev
> > properties .release method.
> >
> > Temporary in this patch:
> > migrate_params_test_apply() copies s->parameters into a temporary
> > structure, so it's necessary to drop the references to the TLS options
> > if they were not set by the user to avoid double-free. This is fixed
> > in the next patches.
> >
> > Acked-by: Markus Armbruster <armbru@redhat.com>
> > Signed-off-by: Fabiano Rosas <farosas@suse.de>
> > Link: https://lore.kernel.org/r/20251215220041.12657-6-farosas@suse.de
> > [peterx: in hmp_info_migrate_parameters(), remove an extra dump of
> > max_postcopy_bandwidth, introduced likely by accident]
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > qapi/migration.json | 6 +-
> > migration/options.h | 1 +
> > migration/migration-hmp-cmds.c | 6 +-
> > migration/options.c | 144 +++++++++++++++++++--------------
> > migration/tls.c | 2 +-
> > 5 files changed, 93 insertions(+), 66 deletions(-)
> >
> > diff --git a/migration/options.c b/migration/options.c
> > index d55f3104be..6ef3c56fb6 100644
> > --- a/migration/options.c
> > +++ b/migration/options.c
> (..)
> > @@ -1243,7 +1274,7 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
> > #ifdef CONFIG_LINUX
> > if (migrate_zero_copy_send() &&
> > ((params->has_multifd_compression && params->multifd_compression) ||
> > - (params->tls_creds && *params->tls_creds))) {
> > + *params->tls_creds->u.s)) {
> > error_setg(errp,
> > "Zero copy only available for non-compressed non-TLS multifd migration");
> > return false;
> The above change gives me easily triggerable NULL pointer dereference:
> > $ qemu-system-x86_64 -monitor stdio -global migration.x-multifd=true -global migration.x-zero-copy-send=true
> > QEMU 10.2.93 monitor - type 'help' for more information
> > VNC server running on ::1:5900
> > (qemu) migrate_set_parameter downtime-limit 500
> > Segmentation fault
Oops..
>
> I guess params->tls_creds really needs that NULL check before being accessed.
Yeah, my gut feeling is we got this special casing of using a temp
parameter object.. I'll leave Fabiano to double check on that and send
patch..
Thanks for the report!
--
Peter Xu
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PULL 19/31] migration: Normalize tls arguments
2026-04-14 18:58 ` Peter Xu
@ 2026-04-14 19:34 ` Fabiano Rosas
0 siblings, 0 replies; 39+ messages in thread
From: Fabiano Rosas @ 2026-04-14 19:34 UTC (permalink / raw)
To: Peter Xu, Maciej S. Szmigiero; +Cc: Markus Armbruster, qemu-devel
Peter Xu <peterx@redhat.com> writes:
> On Tue, Apr 14, 2026 at 06:56:27PM +0200, Maciej S. Szmigiero wrote:
>> On 23.12.2025 15:29, Peter Xu wrote:
>> > From: Fabiano Rosas <farosas@suse.de>
>> >
>> > The migration parameters tls_creds, tls_authz and tls_hostname
>> > currently have a non-uniform handling. When used as arguments to
>> > migrate-set-parameters, their type is StrOrNull and when used as
>> > return value from query-migrate-parameters their type is a plain
>> > string.
>> >
>> > Not only having to convert between the types is cumbersome, but it
>> > also creates the issue of requiring two different QAPI types to be
>> > used, one for each command. MigrateSetParameters is used for
>> > migrate-set-parameters with the TLS arguments as StrOrNull while
>> > MigrationParameters is used for query-migrate-parameters with the TLS
>> > arguments as str.
>> >
>> > Since StrOrNull could be considered a superset of str, change the type
>> > of the TLS arguments in MigrationParameters to StrOrNull. Also ensure
>> > that QTYPE_QNULL is never used.
>> >
>> > 1) migrate-set-parameters will always write QTYPE_QSTRING to
>> > s->parameters, either an empty or non-empty string.
>> >
>> > 2) query-migrate-parameters will always return a QTYPE_QSTRING, either
>> > empty or non-empty.
>> >
>> > 3) the migrate_tls_* helpers will always return a non-empty string or
>> > NULL, for the internal migration code's consumption.
>> >
>> > Points (1) and (2) above help simplify the parameters validation and
>> > the query command handling because s->parameters is already kept in
>> > the format that query-migrate-parameters (and info migrate_paramters)
>> > expect. Point (3) is so people don't need to care about StrOrNull in
>> > migration code.
>> >
>> > This will allow the type duplication to be removed in the next
>> > patches.
>> >
>> > Note that the type of @tls_creds, @tls-hostname, @tls-authz changes
>> > from str to StrOrNull in introspection of the query-migrate-parameters
>> > command. We accept this imprecision to enable de-duplication.
>> >
>> > There's no need to free the TLS options in
>> > migration_instance_finalize() because they're freed by the qdev
>> > properties .release method.
>> >
>> > Temporary in this patch:
>> > migrate_params_test_apply() copies s->parameters into a temporary
>> > structure, so it's necessary to drop the references to the TLS options
>> > if they were not set by the user to avoid double-free. This is fixed
>> > in the next patches.
>> >
>> > Acked-by: Markus Armbruster <armbru@redhat.com>
>> > Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> > Link: https://lore.kernel.org/r/20251215220041.12657-6-farosas@suse.de
>> > [peterx: in hmp_info_migrate_parameters(), remove an extra dump of
>> > max_postcopy_bandwidth, introduced likely by accident]
>> > Signed-off-by: Peter Xu <peterx@redhat.com>
>> > ---
>> > qapi/migration.json | 6 +-
>> > migration/options.h | 1 +
>> > migration/migration-hmp-cmds.c | 6 +-
>> > migration/options.c | 144 +++++++++++++++++++--------------
>> > migration/tls.c | 2 +-
>> > 5 files changed, 93 insertions(+), 66 deletions(-)
>> >
>> > diff --git a/migration/options.c b/migration/options.c
>> > index d55f3104be..6ef3c56fb6 100644
>> > --- a/migration/options.c
>> > +++ b/migration/options.c
>> (..)
>> > @@ -1243,7 +1274,7 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
>> > #ifdef CONFIG_LINUX
>> > if (migrate_zero_copy_send() &&
>> > ((params->has_multifd_compression && params->multifd_compression) ||
>> > - (params->tls_creds && *params->tls_creds))) {
>> > + *params->tls_creds->u.s)) {
>> > error_setg(errp,
>> > "Zero copy only available for non-compressed non-TLS multifd migration");
>> > return false;
>> The above change gives me easily triggerable NULL pointer dereference:
>> > $ qemu-system-x86_64 -monitor stdio -global migration.x-multifd=true -global migration.x-zero-copy-send=true
>> > QEMU 10.2.93 monitor - type 'help' for more information
>> > VNC server running on ::1:5900
>> > (qemu) migrate_set_parameter downtime-limit 500
>> > Segmentation fault
>
> Oops..
>
Why do you guys still let me touch this codebase?
>>
>> I guess params->tls_creds really needs that NULL check before being accessed.
>
> Yeah, my gut feeling is we got this special casing of using a temp
> parameter object.. I'll leave Fabiano to double check on that and send
> patch..
>
> Thanks for the report!
Good that I write my bugs with matching fixes to go along.
https://lore.kernel.org/r/20260202224101.20568-3-farosas@suse.de
I'll repost with a better commit message.
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PULL 20/31] migration: Remove MigrateSetParameters
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (18 preceding siblings ...)
2025-12-23 14:29 ` [PULL 19/31] migration: Normalize tls arguments Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 21/31] qapi/migration: Don't document MigrationParameter Peter Xu
` (11 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Markus Armbruster
From: Fabiano Rosas <farosas@suse.de>
Now that the TLS options have been made the same between
migrate-set-parameters and query-migrate-parameters, a single type can
be used. Remove MigrateSetParameters.
The TLS options documentation from MigrationParameters were replaced
with the ones from MigrateSetParameters which was more complete.
Acked-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-7-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
---
qapi/migration.json | 239 +++------------------------------
migration/migration-hmp-cmds.c | 4 +-
migration/options.c | 6 +-
3 files changed, 26 insertions(+), 223 deletions(-)
diff --git a/qapi/migration.json b/qapi/migration.json
index 30a0eb2d7e..fa4491b9b0 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -993,7 +993,23 @@
'cpr-exec-command'] }
##
-# @MigrateSetParameters:
+# @migrate-set-parameters:
+#
+# Set migration parameters. All arguments are optional.
+#
+# Since: 2.4
+#
+# .. qmp-example::
+#
+# -> { "execute": "migrate-set-parameters" ,
+# "arguments": { "multifd-channels": 5 } }
+# <- { "return": {} }
+##
+{ 'command': 'migrate-set-parameters', 'boxed': true,
+ 'data': 'MigrationParameters' }
+
+##
+# @MigrationParameters:
#
# @announce-initial: Initial delay (in milliseconds) before sending
# the first announce (Since 4.0)
@@ -1155,222 +1171,6 @@
# @unstable: Members @x-checkpoint-delay and
# @x-vcpu-dirty-limit-period are experimental.
#
-# TODO: either fuse back into `MigrationParameters`, or make
-# `MigrationParameters` members mandatory
-#
-# Since: 2.4
-##
-{ 'struct': 'MigrateSetParameters',
- 'data': { '*announce-initial': 'size',
- '*announce-max': 'size',
- '*announce-rounds': 'size',
- '*announce-step': 'size',
- '*throttle-trigger-threshold': 'uint8',
- '*cpu-throttle-initial': 'uint8',
- '*cpu-throttle-increment': 'uint8',
- '*cpu-throttle-tailslow': 'bool',
- '*tls-creds': 'StrOrNull',
- '*tls-hostname': 'StrOrNull',
- '*tls-authz': 'StrOrNull',
- '*max-bandwidth': 'size',
- '*avail-switchover-bandwidth': 'size',
- '*downtime-limit': 'uint64',
- '*x-checkpoint-delay': { 'type': 'uint32',
- 'features': [ 'unstable' ] },
- '*multifd-channels': 'uint8',
- '*xbzrle-cache-size': 'size',
- '*max-postcopy-bandwidth': 'size',
- '*max-cpu-throttle': 'uint8',
- '*multifd-compression': 'MultiFDCompression',
- '*multifd-zlib-level': 'uint8',
- '*multifd-qatzip-level': 'uint8',
- '*multifd-zstd-level': 'uint8',
- '*block-bitmap-mapping': [ 'BitmapMigrationNodeAlias' ],
- '*x-vcpu-dirty-limit-period': { 'type': 'uint64',
- 'features': [ 'unstable' ] },
- '*vcpu-dirty-limit': 'uint64',
- '*mode': 'MigMode',
- '*zero-page-detection': 'ZeroPageDetection',
- '*direct-io': 'bool',
- '*cpr-exec-command': [ 'str' ]} }
-
-##
-# @migrate-set-parameters:
-#
-# Set various migration parameters.
-#
-# Since: 2.4
-#
-# .. qmp-example::
-#
-# -> { "execute": "migrate-set-parameters" ,
-# "arguments": { "multifd-channels": 5 } }
-# <- { "return": {} }
-##
-{ 'command': 'migrate-set-parameters', 'boxed': true,
- 'data': 'MigrateSetParameters' }
-
-##
-# @MigrationParameters:
-#
-# The optional members aren't actually optional.
-#
-# @announce-initial: Initial delay (in milliseconds) before sending
-# the first announce (Since 4.0)
-#
-# @announce-max: Maximum delay (in milliseconds) between packets in
-# the announcement (Since 4.0)
-#
-# @announce-rounds: Number of self-announce packets sent after
-# migration (Since 4.0)
-#
-# @announce-step: Increase in delay (in milliseconds) between
-# subsequent packets in the announcement (Since 4.0)
-#
-# @throttle-trigger-threshold: The ratio of bytes_dirty_period and
-# bytes_xfer_period to trigger throttling. It is expressed as
-# percentage. The default value is 50. (Since 5.0)
-#
-# @cpu-throttle-initial: Initial percentage of time guest cpus are
-# throttled when migration auto-converge is activated.
-# (Since 2.7)
-#
-# @cpu-throttle-increment: throttle percentage increase each time
-# auto-converge detects that migration is not making progress.
-# (Since 2.7)
-#
-# @cpu-throttle-tailslow: Make CPU throttling slower at tail stage.
-# At the tail stage of throttling, the Guest is very sensitive to
-# CPU percentage while the @cpu-throttle -increment is excessive
-# usually at tail stage. If this parameter is true, we will
-# compute the ideal CPU percentage used by the Guest, which may
-# exactly make the dirty rate match the dirty rate threshold.
-# Then we will choose a smaller throttle increment between the one
-# specified by @cpu-throttle-increment and the one generated by
-# ideal CPU percentage. Therefore, it is compatible to
-# traditional throttling, meanwhile the throttle increment won't
-# be excessive at tail stage. The default value is false.
-# (Since 5.1)
-#
-# @tls-creds: ID of the 'tls-creds' object that provides credentials
-# for establishing a TLS connection over the migration data
-# channel. On the outgoing side of the migration, the credentials
-# must be for a 'client' endpoint, while for the incoming side the
-# credentials must be for a 'server' endpoint. An empty string
-# means that QEMU will use plain text mode for migration, rather
-# than TLS. (Since 2.7)
-#
-# Note: 2.8 omits empty @tls-creds instead.
-#
-# @tls-hostname: migration target's hostname for validating the
-# server's x509 certificate identity. If empty, QEMU will use the
-# hostname from the migration URI, if any. (Since 2.7)
-#
-# Note: 2.8 omits empty @tls-hostname instead.
-#
-# @tls-authz: ID of the 'authz' object subclass that provides access
-# control checking of the TLS x509 certificate distinguished name.
-# (Since 4.0)
-#
-# @max-bandwidth: maximum speed for migration, in bytes per second.
-# (Since 2.8)
-#
-# @avail-switchover-bandwidth: to set the available bandwidth that
-# migration can use during switchover phase. **Note:** this does
-# not limit the bandwidth during switchover, but only for
-# calculations when making decisions to switchover. By default,
-# this value is zero, which means QEMU will estimate the bandwidth
-# automatically. This can be set when the estimated value is not
-# accurate, while the user is able to guarantee such bandwidth is
-# available when switching over. When specified correctly, this
-# can make the switchover decision much more accurate.
-# (Since 8.2)
-#
-# @downtime-limit: set maximum tolerated downtime for migration.
-# maximum downtime in milliseconds (Since 2.8)
-#
-# @x-checkpoint-delay: the delay time between two COLO checkpoints.
-# (Since 2.8)
-#
-# @multifd-channels: Number of channels used to migrate data in
-# parallel. This is the same number that the number of sockets
-# used for migration. The default value is 2 (since 4.0)
-#
-# @xbzrle-cache-size: cache size to be used by XBZRLE migration. It
-# needs to be a multiple of the target page size and a power of 2
-# (Since 2.11)
-#
-# @max-postcopy-bandwidth: Background transfer bandwidth during
-# postcopy. Defaults to 0 (unlimited). In bytes per second.
-# (Since 3.0)
-#
-# @max-cpu-throttle: maximum cpu throttle percentage. Defaults to 99.
-# (Since 3.1)
-#
-# @multifd-compression: Which compression method to use. Defaults to
-# none. (Since 5.0)
-#
-# @multifd-zlib-level: Set the compression level to be used in live
-# migration, the compression level is an integer between 0 and 9,
-# where 0 means no compression, 1 means the best compression
-# speed, and 9 means best compression ratio which will consume
-# more CPU. Defaults to 1. (Since 5.0)
-#
-# @multifd-qatzip-level: Set the compression level to be used in live
-# migration. The level is an integer between 1 and 9, where 1
-# means the best compression speed, and 9 means the best
-# compression ratio which will consume more CPU. Defaults to 1.
-# (Since 9.2)
-#
-# @multifd-zstd-level: Set the compression level to be used in live
-# migration, the compression level is an integer between 0 and 20,
-# where 0 means no compression, 1 means the best compression
-# speed, and 20 means best compression ratio which will consume
-# more CPU. Defaults to 1. (Since 5.0)
-#
-# @block-bitmap-mapping: Maps block nodes and bitmaps on them to
-# aliases for the purpose of dirty bitmap migration. Such aliases
-# may for example be the corresponding names on the opposite site.
-# The mapping must be one-to-one, but not necessarily complete: On
-# the source, unmapped bitmaps and all bitmaps on unmapped nodes
-# will be ignored. On the destination, encountering an unmapped
-# alias in the incoming migration stream will result in a report,
-# and all further bitmap migration data will then be discarded.
-# Note that the destination does not know about bitmaps it does
-# not receive, so there is no limitation or requirement regarding
-# the number of bitmaps received, or how they are named, or on
-# which nodes they are placed. By default (when this parameter
-# has never been set), bitmap names are mapped to themselves.
-# Nodes are mapped to their block device name if there is one, and
-# to their node name otherwise. (Since 5.2)
-#
-# @x-vcpu-dirty-limit-period: Periodic time (in milliseconds) of dirty
-# limit during live migration. Should be in the range 1 to
-# 1000ms. Defaults to 1000ms. (Since 8.1)
-#
-# @vcpu-dirty-limit: Dirtyrate limit (MB/s) during live migration.
-# Defaults to 1. (Since 8.1)
-#
-# @mode: Migration mode. See description in `MigMode`. Default is
-# 'normal'. (Since 8.2)
-#
-# @zero-page-detection: Whether and how to detect zero pages. See
-# description in `ZeroPageDetection`. Default is 'multifd'.
-# (since 9.0)
-#
-# @direct-io: Open migration files with O_DIRECT when possible. This
-# only has effect if the @mapped-ram capability is enabled.
-# (Since 9.1)
-#
-# @cpr-exec-command: Command to start the new QEMU process when @mode
-# is @cpr-exec. The first list element is the program's filename,
-# the remainder its arguments. (Since 10.2)
-#
-# Features:
-#
-# @unstable: Members @x-checkpoint-delay and
-# @x-vcpu-dirty-limit-period are experimental.
-#
# Since: 2.4
##
{ 'struct': 'MigrationParameters',
@@ -1410,7 +1210,10 @@
##
# @query-migrate-parameters:
#
-# Return information about the current migration parameters
+# Return information about the current migration parameters. Optional
+# members of the return value are always present, except
+# @block-bitmap-mapping, which is only present if it has been
+# previously set.
#
# Since: 2.4
#
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index edc561a34a..8b1096db86 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -578,7 +578,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
const char *param = qdict_get_str(qdict, "parameter");
const char *valuestr = qdict_get_str(qdict, "value");
Visitor *v = string_input_visitor_new(valuestr);
- MigrateSetParameters *p = g_new0(MigrateSetParameters, 1);
+ MigrationParameters *p = g_new0(MigrationParameters, 1);
uint64_t valuebw = 0;
uint64_t cache_size;
Error *err = NULL;
@@ -765,7 +765,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
qmp_migrate_set_parameters(p, &err);
cleanup:
- qapi_free_MigrateSetParameters(p);
+ qapi_free_MigrationParameters(p);
visit_free(v);
hmp_handle_error(mon, err);
}
diff --git a/migration/options.c b/migration/options.c
index 6ef3c56fb6..b17347e43b 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1312,7 +1312,7 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
return true;
}
-static void migrate_params_test_apply(MigrateSetParameters *params,
+static void migrate_params_test_apply(MigrationParameters *params,
MigrationParameters *dest)
{
*dest = migrate_get_current()->parameters;
@@ -1439,7 +1439,7 @@ static void migrate_params_test_apply(MigrateSetParameters *params,
}
}
-static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
+static void migrate_params_apply(MigrationParameters *params, Error **errp)
{
MigrationState *s = migrate_get_current();
@@ -1575,7 +1575,7 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
}
}
-void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
+void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
{
MigrationParameters tmp;
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 21/31] qapi/migration: Don't document MigrationParameter
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (19 preceding siblings ...)
2025-12-23 14:29 ` [PULL 20/31] migration: Remove MigrateSetParameters Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 22/31] migration: Run a post update routine after setting parameters Peter Xu
` (10 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Markus Armbruster
From: Fabiano Rosas <farosas@suse.de>
The MigrationParameter (singular) enumeration is not part of the
migration QMP API, it's only used for nicely converting HMP strings
into MigrationParameters (plural) members and for providing readline
completion.
Documenting this enum only serves to duplicate documentation between
MigrationParameter and MigrationParameters.
Add an exception to QAPIs pragma.json and stop documenting it.
The generated "QEMU QMP Reference Manual" now lists the enum members
as "Not documented." Tolerable.
Acked-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-8-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
---
qapi/migration.json | 154 +-------------------------------------------
qapi/pragma.json | 1 +
2 files changed, 3 insertions(+), 152 deletions(-)
diff --git a/qapi/migration.json b/qapi/migration.json
index fa4491b9b0..201dedd982 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -806,158 +806,8 @@
##
# @MigrationParameter:
#
-# Migration parameters enumeration
-#
-# @announce-initial: Initial delay (in milliseconds) before sending
-# the first announce (Since 4.0)
-#
-# @announce-max: Maximum delay (in milliseconds) between packets in
-# the announcement (Since 4.0)
-#
-# @announce-rounds: Number of self-announce packets sent after
-# migration (Since 4.0)
-#
-# @announce-step: Increase in delay (in milliseconds) between
-# subsequent packets in the announcement (Since 4.0)
-#
-# @throttle-trigger-threshold: The ratio of bytes_dirty_period and
-# bytes_xfer_period to trigger throttling. It is expressed as
-# percentage. The default value is 50. (Since 5.0)
-#
-# @cpu-throttle-initial: Initial percentage of time guest cpus are
-# throttled when migration auto-converge is activated. The
-# default value is 20. (Since 2.7)
-#
-# @cpu-throttle-increment: throttle percentage increase each time
-# auto-converge detects that migration is not making progress.
-# The default value is 10. (Since 2.7)
-#
-# @cpu-throttle-tailslow: Make CPU throttling slower at tail stage.
-# At the tail stage of throttling, the Guest is very sensitive to
-# CPU percentage while the @cpu-throttle -increment is excessive
-# usually at tail stage. If this parameter is true, we will
-# compute the ideal CPU percentage used by the Guest, which may
-# exactly make the dirty rate match the dirty rate threshold.
-# Then we will choose a smaller throttle increment between the one
-# specified by @cpu-throttle-increment and the one generated by
-# ideal CPU percentage. Therefore, it is compatible to
-# traditional throttling, meanwhile the throttle increment won't
-# be excessive at tail stage. The default value is false.
-# (Since 5.1)
-#
-# @tls-creds: ID of the 'tls-creds' object that provides credentials
-# for establishing a TLS connection over the migration data
-# channel. On the outgoing side of the migration, the credentials
-# must be for a 'client' endpoint, while for the incoming side the
-# credentials must be for a 'server' endpoint. Setting this to a
-# non-empty string enables TLS for all migrations. An empty
-# string means that QEMU will use plain text mode for migration,
-# rather than TLS. (Since 2.7)
-#
-# @tls-hostname: migration target's hostname for validating the
-# server's x509 certificate identity. If empty, QEMU will use the
-# hostname from the migration URI, if any. A non-empty value is
-# required when using x509 based TLS credentials and the migration
-# URI does not include a hostname, such as fd: or exec: based
-# migration. (Since 2.7)
-#
-# Note: empty value works only since 2.9.
-#
-# @tls-authz: ID of the 'authz' object subclass that provides access
-# control checking of the TLS x509 certificate distinguished name.
-# This object is only resolved at time of use, so can be deleted
-# and recreated on the fly while the migration server is active.
-# If missing, it will default to denying access (Since 4.0)
-#
-# @max-bandwidth: maximum speed for migration, in bytes per second.
-# (Since 2.8)
-#
-# @avail-switchover-bandwidth: to set the available bandwidth that
-# migration can use during switchover phase. **Note:** this does
-# not limit the bandwidth during switchover, but only for
-# calculations when making decisions to switchover. By default,
-# this value is zero, which means QEMU will estimate the bandwidth
-# automatically. This can be set when the estimated value is not
-# accurate, while the user is able to guarantee such bandwidth is
-# available when switching over. When specified correctly, this
-# can make the switchover decision much more accurate.
-# (Since 8.2)
-#
-# @downtime-limit: set maximum tolerated downtime for migration.
-# maximum downtime in milliseconds (Since 2.8)
-#
-# @x-checkpoint-delay: The delay time (in ms) between two COLO
-# checkpoints in periodic mode. (Since 2.8)
-#
-# @multifd-channels: Number of channels used to migrate data in
-# parallel. This is the same number that the number of sockets
-# used for migration. The default value is 2 (since 4.0)
-#
-# @xbzrle-cache-size: cache size to be used by XBZRLE migration. It
-# needs to be a multiple of the target page size and a power of 2
-# (Since 2.11)
-#
-# @max-postcopy-bandwidth: Background transfer bandwidth during
-# postcopy. Defaults to 0 (unlimited). In bytes per second.
-# (Since 3.0)
-#
-# @max-cpu-throttle: maximum cpu throttle percentage. Defaults to 99.
-# (Since 3.1)
-#
-# @multifd-compression: Which compression method to use. Defaults to
-# none. (Since 5.0)
-#
-# @multifd-zlib-level: Set the compression level to be used in live
-# migration, the compression level is an integer between 0 and 9,
-# where 0 means no compression, 1 means the best compression
-# speed, and 9 means best compression ratio which will consume
-# more CPU. Defaults to 1. (Since 5.0)
-#
-# @multifd-qatzip-level: Set the compression level to be used in live
-# migration. The level is an integer between 1 and 9, where 1
-# means the best compression speed, and 9 means the best
-# compression ratio which will consume more CPU. Defaults to 1.
-# (Since 9.2)
-#
-# @multifd-zstd-level: Set the compression level to be used in live
-# migration, the compression level is an integer between 0 and 20,
-# where 0 means no compression, 1 means the best compression
-# speed, and 20 means best compression ratio which will consume
-# more CPU. Defaults to 1. (Since 5.0)
-#
-# @block-bitmap-mapping: Maps block nodes and bitmaps on them to
-# aliases for the purpose of dirty bitmap migration. Such aliases
-# may for example be the corresponding names on the opposite site.
-# The mapping must be one-to-one, but not necessarily complete: On
-# the source, unmapped bitmaps and all bitmaps on unmapped nodes
-# will be ignored. On the destination, encountering an unmapped
-# alias in the incoming migration stream will result in a report,
-# and all further bitmap migration data will then be discarded.
-# Note that the destination does not know about bitmaps it does
-# not receive, so there is no limitation or requirement regarding
-# the number of bitmaps received, or how they are named, or on
-# which nodes they are placed. By default (when this parameter
-# has never been set), bitmap names are mapped to themselves.
-# Nodes are mapped to their block device name if there is one, and
-# to their node name otherwise. (Since 5.2)
-#
-# @x-vcpu-dirty-limit-period: Periodic time (in milliseconds) of dirty
-# limit during live migration. Should be in the range 1 to
-# 1000ms. Defaults to 1000ms. (Since 8.1)
-#
-# @vcpu-dirty-limit: Dirtyrate limit (MB/s) during live migration.
-# Defaults to 1. (Since 8.1)
-#
-# @mode: Migration mode. See description in `MigMode`. Default is
-# 'normal'. (Since 8.2)
-#
-# @zero-page-detection: Whether and how to detect zero pages. See
-# description in `ZeroPageDetection`. Default is 'multifd'.
-# (since 9.0)
-#
-# @direct-io: Open migration files with O_DIRECT when possible. This
-# only has effect if the @mapped-ram capability is enabled.
-# (Since 9.1)
+# Migration parameters enumeration. The enumeration values mirror the
+# members of @MigrationParameters.
#
# @cpr-exec-command: Command to start the new QEMU process when @mode
# is @cpr-exec. The first list element is the program's filename,
diff --git a/qapi/pragma.json b/qapi/pragma.json
index 023a2ef7bc..193bc39059 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -59,6 +59,7 @@
'IscsiTransport',
'KeyValueKind',
'MemoryDeviceInfoKind',
+ 'MigrationParameter',
'NetClientDriver',
'ObjectType',
'QKeyCode',
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 22/31] migration: Run a post update routine after setting parameters
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (20 preceding siblings ...)
2025-12-23 14:29 ` [PULL 21/31] qapi/migration: Don't document MigrationParameter Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 23/31] migration: Add a flag to track block-bitmap-mapping input Peter Xu
` (9 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu
From: Fabiano Rosas <farosas@suse.de>
Some migration parameters are updated immediately once they are set
via migrate-set-parameters. Move that work outside of
migrate_params_apply() and leave that function with the single
responsibility of setting s->parameters and not doing any
side-effects.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-9-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/options.c | 38 ++++++++++++++++++++++++++++----------
migration/ram.c | 2 +-
2 files changed, 29 insertions(+), 11 deletions(-)
diff --git a/migration/options.c b/migration/options.c
index b17347e43b..cb127e9c72 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1125,6 +1125,31 @@ void migrate_params_init(MigrationParameters *params)
params->has_cpr_exec_command = true;
}
+static void migrate_post_update_params(MigrationParameters *new, Error **errp)
+{
+ MigrationState *s = migrate_get_current();
+
+ if (new->has_max_bandwidth) {
+ if (s->to_dst_file && !migration_in_postcopy()) {
+ migration_rate_set(new->max_bandwidth);
+ }
+ }
+
+ if (new->has_x_checkpoint_delay) {
+ colo_checkpoint_delay_set();
+ }
+
+ if (new->has_xbzrle_cache_size) {
+ xbzrle_cache_resize(new->xbzrle_cache_size, errp);
+ }
+
+ if (new->has_max_postcopy_bandwidth) {
+ if (s->to_dst_file && migration_in_postcopy()) {
+ migration_rate_set(new->max_postcopy_bandwidth);
+ }
+ }
+}
+
/*
* Check whether the parameters are valid. Error will be put into errp
* (if provided). Return true if valid, otherwise false.
@@ -1439,7 +1464,7 @@ static void migrate_params_test_apply(MigrationParameters *params,
}
}
-static void migrate_params_apply(MigrationParameters *params, Error **errp)
+static void migrate_params_apply(MigrationParameters *params)
{
MigrationState *s = migrate_get_current();
@@ -1479,9 +1504,6 @@ static void migrate_params_apply(MigrationParameters *params, Error **errp)
if (params->has_max_bandwidth) {
s->parameters.max_bandwidth = params->max_bandwidth;
- if (s->to_dst_file && !migration_in_postcopy()) {
- migration_rate_set(s->parameters.max_bandwidth);
- }
}
if (params->has_avail_switchover_bandwidth) {
@@ -1494,7 +1516,6 @@ static void migrate_params_apply(MigrationParameters *params, Error **errp)
if (params->has_x_checkpoint_delay) {
s->parameters.x_checkpoint_delay = params->x_checkpoint_delay;
- colo_checkpoint_delay_set();
}
if (params->has_multifd_channels) {
@@ -1514,13 +1535,9 @@ static void migrate_params_apply(MigrationParameters *params, Error **errp)
}
if (params->has_xbzrle_cache_size) {
s->parameters.xbzrle_cache_size = params->xbzrle_cache_size;
- xbzrle_cache_resize(params->xbzrle_cache_size, errp);
}
if (params->has_max_postcopy_bandwidth) {
s->parameters.max_postcopy_bandwidth = params->max_postcopy_bandwidth;
- if (s->to_dst_file && migration_in_postcopy()) {
- migration_rate_set(s->parameters.max_postcopy_bandwidth);
- }
}
if (params->has_max_cpu_throttle) {
s->parameters.max_cpu_throttle = params->max_cpu_throttle;
@@ -1594,7 +1611,8 @@ void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
migrate_params_test_apply(params, &tmp);
if (migrate_params_check(&tmp, errp)) {
- migrate_params_apply(params, errp);
+ migrate_params_apply(params);
+ migrate_post_update_params(params, errp);
}
migrate_tls_opts_free(&tmp);
diff --git a/migration/ram.c b/migration/ram.c
index ecd81601e2..18e4e26eaf 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -176,7 +176,7 @@ static void XBZRLE_cache_unlock(void)
/**
* xbzrle_cache_resize: resize the xbzrle cache
*
- * This function is called from migrate_params_apply in main
+ * This function is called from migrate_post_update_params in main
* thread, possibly while a migration is in progress. A running
* migration may be using the cache and might finish during this call,
* hence changes to the cache are protected by XBZRLE.lock().
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 23/31] migration: Add a flag to track block-bitmap-mapping input
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (21 preceding siblings ...)
2025-12-23 14:29 ` [PULL 22/31] migration: Run a post update routine after setting parameters Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 24/31] migration: Remove checks for s->parameters has_* fields Peter Xu
` (8 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Kevin Wolf
From: Fabiano Rosas <farosas@suse.de>
The QAPI converts an empty list on the block-bitmap-mapping input into
a NULL BitmapMigrationNodeAliasList. The empty list is a valid input
for the block-bitmap-mapping option, so commit 3cba22c9ad ("migration:
Fix block_bitmap_mapping migration") started using the
s->parameters.has_block_bitmap_mapping field to tell when the user has
passed in an empty list vs. when no list has been passed at all.
Using s->parameters.has_block_bitmap_mapping field is only possible
because MigrationParameters has had its members made optional due to
historical reasons.
In order to make improvements to the way configuration options are set
for a migration, we'd like to reduce the open-coded usage of the has_*
fields of the global configuration object (s->parameters).
Add a separate boolean to track the status of the block_bitmap_mapping
option.
No functional change intended.
(this was verified to not regress iotest 300, which is the test that
3cba22c9ad refers to)
CC: Kevin Wolf <kwolf@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-10-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/migration.h | 7 +++++++
migration/migration-hmp-cmds.c | 3 ++-
migration/options.c | 6 +++---
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/migration/migration.h b/migration/migration.h
index e4b4f25deb..4d42e8f9a7 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -514,6 +514,13 @@ struct MigrationState {
QemuEvent postcopy_package_loaded_event;
GSource *hup_source;
+
+ /*
+ * The block-bitmap-mapping option is allowed to be an empty list,
+ * therefore we need a way to know whether the user has given
+ * anything as input.
+ */
+ bool has_block_bitmap_mapping;
};
void migrate_set_state(MigrationStatus *state, MigrationStatus old_state,
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 8b1096db86..a2863e6a2f 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -321,6 +321,7 @@ static void monitor_print_cpr_exec_command(Monitor *mon, strList *args)
void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
{
MigrationParameters *params;
+ MigrationState *s = migrate_get_current();
params = qmp_query_migrate_parameters(NULL);
@@ -404,7 +405,7 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
MigrationParameter_str(MIGRATION_PARAMETER_XBZRLE_CACHE_SIZE),
params->xbzrle_cache_size);
- if (params->has_block_bitmap_mapping) {
+ if (s->has_block_bitmap_mapping) {
const BitmapMigrationNodeAliasList *bmnal;
monitor_printf(mon, "%s:\n",
diff --git a/migration/options.c b/migration/options.c
index cb127e9c72..20e9438656 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -786,7 +786,7 @@ bool migrate_has_block_bitmap_mapping(void)
{
MigrationState *s = migrate_get_current();
- return s->parameters.has_block_bitmap_mapping;
+ return s->has_block_bitmap_mapping;
}
uint32_t migrate_checkpoint_delay(void)
@@ -1071,7 +1071,7 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
params->has_announce_step = true;
params->announce_step = s->parameters.announce_step;
- if (s->parameters.has_block_bitmap_mapping) {
+ if (s->has_block_bitmap_mapping) {
params->has_block_bitmap_mapping = true;
params->block_bitmap_mapping =
QAPI_CLONE(BitmapMigrationNodeAliasList,
@@ -1559,7 +1559,7 @@ static void migrate_params_apply(MigrationParameters *params)
qapi_free_BitmapMigrationNodeAliasList(
s->parameters.block_bitmap_mapping);
- s->parameters.has_block_bitmap_mapping = true;
+ s->has_block_bitmap_mapping = true;
s->parameters.block_bitmap_mapping =
QAPI_CLONE(BitmapMigrationNodeAliasList,
params->block_bitmap_mapping);
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 24/31] migration: Remove checks for s->parameters has_* fields
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (22 preceding siblings ...)
2025-12-23 14:29 ` [PULL 23/31] migration: Add a flag to track block-bitmap-mapping input Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 25/31] migration: Do away with usage of QERR_INVALID_PARAMETER_VALUE Peter Xu
` (7 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu
From: Fabiano Rosas <farosas@suse.de>
The migration parameters validation produces a temporary structure
which is the merge of the current parameter values (s->parameters,
MigrationParameters) with the new parameters set by the user
(former MigrateSetParameters).
When copying the values from s->parameters into the temporary
structure, the has_* fields are copied along, but when merging the
user-input values they are not.
During migrate_params_check(), only the parameters that have the
corresponding has_* field will be checked, so only the parameters that
were initialized in migrate_params_init() will be validated.
This causes (almost) all of the migration parameters to be validated
every time a parameter is set, regardless of which fields the user
touched, but it also skips validation of any values that are not set
in migrate_params_init().
It's not clear what was the intention of the original code, whether to
validate all fields always, or only validate what the user input
changed. Since the current situation is closer to the former option,
make the choice of validating all parameters by removing the checks
for the has_* fields when validating.
Note that bringing the user input into the temporary structure for
validation still needs to look at the has_* fields, otherwise any
parameters not set by the user (i.e. 0) would override the
corresponding value in s->parameters.
The empty migrate_params_init() will be kept because subsequent
patches will add code to it.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-11-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/migration.c | 2 +-
migration/options.c | 102 ++++++++++++------------------------------
2 files changed, 30 insertions(+), 74 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index f1378c3bf4..754abe0b5a 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2118,7 +2118,7 @@ static bool migrate_prepare(MigrationState *s, bool resume, Error **errp)
}
if (migrate_mode() == MIG_MODE_CPR_EXEC &&
- !s->parameters.has_cpr_exec_command) {
+ !s->parameters.cpr_exec_command) {
error_setg(errp, "cpr-exec mode requires setting cpr-exec-command");
return false;
}
diff --git a/migration/options.c b/migration/options.c
index 20e9438656..35fbe9a6ef 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1097,32 +1097,6 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
void migrate_params_init(MigrationParameters *params)
{
- /* Set has_* up only for parameter checks */
- params->has_throttle_trigger_threshold = true;
- params->has_cpu_throttle_initial = true;
- params->has_cpu_throttle_increment = true;
- params->has_cpu_throttle_tailslow = true;
- params->has_max_bandwidth = true;
- params->has_downtime_limit = true;
- params->has_x_checkpoint_delay = true;
- params->has_multifd_channels = true;
- params->has_multifd_compression = true;
- params->has_multifd_zlib_level = true;
- params->has_multifd_qatzip_level = true;
- params->has_multifd_zstd_level = true;
- params->has_xbzrle_cache_size = true;
- params->has_max_postcopy_bandwidth = true;
- params->has_max_cpu_throttle = true;
- params->has_announce_initial = true;
- params->has_announce_max = true;
- params->has_announce_rounds = true;
- params->has_announce_step = true;
- params->has_x_vcpu_dirty_limit_period = true;
- params->has_vcpu_dirty_limit = true;
- params->has_mode = true;
- params->has_zero_page_detection = true;
- params->has_direct_io = true;
- params->has_cpr_exec_command = true;
}
static void migrate_post_update_params(MigrationParameters *new, Error **errp)
@@ -1158,34 +1132,31 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
{
ERRP_GUARD();
- if (params->has_throttle_trigger_threshold &&
- (params->throttle_trigger_threshold < 1 ||
- params->throttle_trigger_threshold > 100)) {
+ if (params->throttle_trigger_threshold < 1 ||
+ params->throttle_trigger_threshold > 100) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"throttle_trigger_threshold",
"an integer in the range of 1 to 100");
return false;
}
- if (params->has_cpu_throttle_initial &&
- (params->cpu_throttle_initial < 1 ||
- params->cpu_throttle_initial > 99)) {
+ if (params->cpu_throttle_initial < 1 ||
+ params->cpu_throttle_initial > 99) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"cpu_throttle_initial",
"an integer in the range of 1 to 99");
return false;
}
- if (params->has_cpu_throttle_increment &&
- (params->cpu_throttle_increment < 1 ||
- params->cpu_throttle_increment > 99)) {
+ if (params->cpu_throttle_increment < 1 ||
+ params->cpu_throttle_increment > 99) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"cpu_throttle_increment",
"an integer in the range of 1 to 99");
return false;
}
- if (params->has_max_bandwidth && (params->max_bandwidth > SIZE_MAX)) {
+ if (params->max_bandwidth > SIZE_MAX) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"max_bandwidth",
"an integer in the range of 0 to "stringify(SIZE_MAX)
@@ -1193,8 +1164,7 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
return false;
}
- if (params->has_avail_switchover_bandwidth &&
- (params->avail_switchover_bandwidth > SIZE_MAX)) {
+ if (params->avail_switchover_bandwidth > SIZE_MAX) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"avail_switchover_bandwidth",
"an integer in the range of 0 to "stringify(SIZE_MAX)
@@ -1202,8 +1172,7 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
return false;
}
- if (params->has_downtime_limit &&
- (params->downtime_limit > MAX_MIGRATE_DOWNTIME)) {
+ if (params->downtime_limit > MAX_MIGRATE_DOWNTIME) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"downtime_limit",
"an integer in the range of 0 to "
@@ -1213,93 +1182,82 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
/* x_checkpoint_delay is now always positive */
- if (params->has_multifd_channels && (params->multifd_channels < 1)) {
+ if (params->multifd_channels < 1) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"multifd_channels",
"a value between 1 and 255");
return false;
}
- if (params->has_multifd_zlib_level &&
- (params->multifd_zlib_level > 9)) {
+ if (params->multifd_zlib_level > 9) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_zlib_level",
"a value between 0 and 9");
return false;
}
- if (params->has_multifd_qatzip_level &&
- ((params->multifd_qatzip_level > 9) ||
- (params->multifd_qatzip_level < 1))) {
+ if (params->multifd_qatzip_level > 9 ||
+ params->multifd_qatzip_level < 1) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_qatzip_level",
"a value between 1 and 9");
return false;
}
- if (params->has_multifd_zstd_level &&
- (params->multifd_zstd_level > 20)) {
+ if (params->multifd_zstd_level > 20) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_zstd_level",
"a value between 0 and 20");
return false;
}
- if (params->has_xbzrle_cache_size &&
- (params->xbzrle_cache_size < qemu_target_page_size() ||
- !is_power_of_2(params->xbzrle_cache_size))) {
+ if (params->xbzrle_cache_size < qemu_target_page_size() ||
+ !is_power_of_2(params->xbzrle_cache_size)) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"xbzrle_cache_size",
"a power of two no less than the target page size");
return false;
}
- if (params->has_max_cpu_throttle &&
- (params->max_cpu_throttle < params->cpu_throttle_initial ||
- params->max_cpu_throttle > 99)) {
+ if (params->max_cpu_throttle < params->cpu_throttle_initial ||
+ params->max_cpu_throttle > 99) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"max_cpu_throttle",
"an integer in the range of cpu_throttle_initial to 99");
return false;
}
- if (params->has_announce_initial &&
- params->announce_initial > 100000) {
+ if (params->announce_initial > 100000) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"announce_initial",
"a value between 0 and 100000");
return false;
}
- if (params->has_announce_max &&
- params->announce_max > 100000) {
+ if (params->announce_max > 100000) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"announce_max",
"a value between 0 and 100000");
return false;
}
- if (params->has_announce_rounds &&
- params->announce_rounds > 1000) {
+ if (params->announce_rounds > 1000) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"announce_rounds",
"a value between 0 and 1000");
return false;
}
- if (params->has_announce_step &&
- (params->announce_step < 1 ||
- params->announce_step > 10000)) {
+ if (params->announce_step < 1 ||
+ params->announce_step > 10000) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"announce_step",
"a value between 0 and 10000");
return false;
}
- if (params->has_block_bitmap_mapping &&
- !check_dirty_bitmap_mig_alias_map(params->block_bitmap_mapping, errp)) {
+ if (!check_dirty_bitmap_mig_alias_map(params->block_bitmap_mapping, errp)) {
error_prepend(errp, "Invalid mapping given for block-bitmap-mapping: ");
return false;
}
#ifdef CONFIG_LINUX
if (migrate_zero_copy_send() &&
- ((params->has_multifd_compression && params->multifd_compression) ||
- *params->tls_creds->u.s)) {
+ (params->multifd_compression || *params->tls_creds->u.s)) {
error_setg(errp,
"Zero copy only available for non-compressed non-TLS multifd migration");
return false;
@@ -1313,23 +1271,21 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
return false;
}
- if (params->has_x_vcpu_dirty_limit_period &&
- (params->x_vcpu_dirty_limit_period < 1 ||
- params->x_vcpu_dirty_limit_period > 1000)) {
+ if (params->x_vcpu_dirty_limit_period < 1 ||
+ params->x_vcpu_dirty_limit_period > 1000) {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
"x-vcpu-dirty-limit-period",
"a value between 1 and 1000");
return false;
}
- if (params->has_vcpu_dirty_limit &&
- (params->vcpu_dirty_limit < 1)) {
+ if (params->vcpu_dirty_limit < 1) {
error_setg(errp,
"Parameter 'vcpu_dirty_limit' must be greater than 1 MB/s");
return false;
}
- if (params->has_direct_io && params->direct_io && !qemu_has_direct_io()) {
+ if (params->direct_io && !qemu_has_direct_io()) {
error_setg(errp, "No build-time support for direct-io");
return false;
}
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 25/31] migration: Do away with usage of QERR_INVALID_PARAMETER_VALUE
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (23 preceding siblings ...)
2025-12-23 14:29 ` [PULL 24/31] migration: Remove checks for s->parameters has_* fields Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 26/31] migration: Extract code to mark all parameters as present Peter Xu
` (6 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu
From: Fabiano Rosas <farosas@suse.de>
The QERR_INVALID_PARAMETER_VALUE macro is documented as not to be used
in new code. Remove the usage from migration/options.c.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-12-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/migration.c | 3 +--
migration/options.c | 56 +++++++++++++++---------------------------
migration/page_cache.c | 6 ++---
migration/ram.c | 3 +--
4 files changed, 24 insertions(+), 44 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 754abe0b5a..635851fe8c 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2325,8 +2325,7 @@ static void qmp_migrate_finish(MigrationAddress *addr, bool resume_requested,
} else if (addr->transport == MIGRATION_ADDRESS_TYPE_FILE) {
file_start_outgoing_migration(s, &addr->u.file, &local_err);
} else {
- error_setg(&local_err, QERR_INVALID_PARAMETER_VALUE, "uri",
- "a valid migration protocol");
+ error_setg(&local_err, "uri is not a valid migration protocol");
migrate_set_state(&s->state, MIGRATION_STATUS_SETUP,
MIGRATION_STATUS_FAILED);
}
diff --git a/migration/options.c b/migration/options.c
index 35fbe9a6ef..4652478e6a 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1134,120 +1134,105 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
if (params->throttle_trigger_threshold < 1 ||
params->throttle_trigger_threshold > 100) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "throttle_trigger_threshold",
+ error_setg(errp, "Option throttle_trigger_threshold expects "
"an integer in the range of 1 to 100");
return false;
}
if (params->cpu_throttle_initial < 1 ||
params->cpu_throttle_initial > 99) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "cpu_throttle_initial",
+ error_setg(errp, "Option cpu_throttle_initial expects "
"an integer in the range of 1 to 99");
return false;
}
if (params->cpu_throttle_increment < 1 ||
params->cpu_throttle_increment > 99) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "cpu_throttle_increment",
+ error_setg(errp, "Option cpu_throttle_increment expects "
"an integer in the range of 1 to 99");
return false;
}
if (params->max_bandwidth > SIZE_MAX) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "max_bandwidth",
+ error_setg(errp, "Option max_bandwidth expects "
"an integer in the range of 0 to "stringify(SIZE_MAX)
" bytes/second");
return false;
}
if (params->avail_switchover_bandwidth > SIZE_MAX) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "avail_switchover_bandwidth",
+ error_setg(errp, "Option avail_switchover_bandwidth expects "
"an integer in the range of 0 to "stringify(SIZE_MAX)
" bytes/second");
return false;
}
if (params->downtime_limit > MAX_MIGRATE_DOWNTIME) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "downtime_limit",
+ error_setg(errp, "Option downtime_limit expects "
"an integer in the range of 0 to "
stringify(MAX_MIGRATE_DOWNTIME)" ms");
return false;
}
- /* x_checkpoint_delay is now always positive */
-
if (params->multifd_channels < 1) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "multifd_channels",
+ error_setg(errp, "Option multifd_channels expects "
"a value between 1 and 255");
return false;
}
if (params->multifd_zlib_level > 9) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_zlib_level",
+ error_setg(errp, "Option multifd_zlib_level expects "
"a value between 0 and 9");
return false;
}
if (params->multifd_qatzip_level > 9 ||
params->multifd_qatzip_level < 1) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_qatzip_level",
+ error_setg(errp, "Option multifd_qatzip_level expects "
"a value between 1 and 9");
return false;
}
if (params->multifd_zstd_level > 20) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_zstd_level",
+ error_setg(errp, "Option multifd_zstd_level expects "
"a value between 0 and 20");
return false;
}
if (params->xbzrle_cache_size < qemu_target_page_size() ||
!is_power_of_2(params->xbzrle_cache_size)) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "xbzrle_cache_size",
+ error_setg(errp, "Option xbzrle_cache_size expects "
"a power of two no less than the target page size");
return false;
}
if (params->max_cpu_throttle < params->cpu_throttle_initial ||
params->max_cpu_throttle > 99) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "max_cpu_throttle",
+ error_setg(errp, "max_Option cpu_throttle expects "
"an integer in the range of cpu_throttle_initial to 99");
return false;
}
if (params->announce_initial > 100000) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "announce_initial",
+ error_setg(errp, "Option announce_initial expects "
"a value between 0 and 100000");
return false;
}
if (params->announce_max > 100000) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "announce_max",
+ error_setg(errp, "Option announce_max expects "
"a value between 0 and 100000");
- return false;
+ return false;
}
if (params->announce_rounds > 1000) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "announce_rounds",
+ error_setg(errp, "Option announce_rounds expects "
"a value between 0 and 1000");
- return false;
+ return false;
}
if (params->announce_step < 1 ||
params->announce_step > 10000) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "announce_step",
+ error_setg(errp, "Option announce_step expects "
"a value between 0 and 10000");
- return false;
+ return false;
}
if (!check_dirty_bitmap_mig_alias_map(params->block_bitmap_mapping, errp)) {
@@ -1273,8 +1258,7 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
if (params->x_vcpu_dirty_limit_period < 1 ||
params->x_vcpu_dirty_limit_period > 1000) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "x-vcpu-dirty-limit-period",
+ error_setg(errp, "Option x-vcpu-dirty-limit-period expects "
"a value between 1 and 1000");
return false;
}
diff --git a/migration/page_cache.c b/migration/page_cache.c
index 6d4f7a9bbc..650b15e48c 100644
--- a/migration/page_cache.c
+++ b/migration/page_cache.c
@@ -45,15 +45,13 @@ PageCache *cache_init(uint64_t new_size, size_t page_size, Error **errp)
PageCache *cache;
if (new_size < page_size) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "cache size",
- "is smaller than one target page size");
+ error_setg(errp, "cache size is smaller than target page size");
return NULL;
}
/* round down to the nearest power of 2 */
if (!is_power_of_2(num_pages)) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "cache size",
- "is not a power of two number of pages");
+ error_setg(errp, "number of pages is not a power of two");
return NULL;
}
diff --git a/migration/ram.c b/migration/ram.c
index 18e4e26eaf..df7e154877 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -193,8 +193,7 @@ int xbzrle_cache_resize(uint64_t new_size, Error **errp)
/* Check for truncation */
if (new_size != (size_t)new_size) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "cache size",
- "exceeding address space");
+ error_setg(errp, "xbzrle cache size integer overflow");
return -1;
}
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 26/31] migration: Extract code to mark all parameters as present
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (24 preceding siblings ...)
2025-12-23 14:29 ` [PULL 25/31] migration: Do away with usage of QERR_INVALID_PARAMETER_VALUE Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 27/31] migration: Use QAPI_CLONE_MEMBERS in query_migrate_parameters Peter Xu
` (5 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Kevin Wolf
From: Fabiano Rosas <farosas@suse.de>
MigrationParameters needs to have all of its has_* fields marked as
true when used as the return of query_migrate_parameters because the
corresponding QMP command has all of its members non-optional by
design, despite them being marked as optional in migration.json.
Extract this code into a function and make it assert if any field is
missing. With this we ensure future changes will not inadvertently
leave any parameters missing.
Note that the block-bitmap-mapping is a special case because the empty
list is considered a valid value, so it has historically not been
present in the command's output if it has never been set.
CC: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-13-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/options.c | 89 ++++++++++++++++++++++++++++-----------------
1 file changed, 55 insertions(+), 34 deletions(-)
diff --git a/migration/options.c b/migration/options.c
index 4652478e6a..418cb44413 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1020,6 +1020,44 @@ static void tls_opt_to_str(StrOrNull *opt)
opt->u.s = g_strdup("");
}
+/*
+ * query-migrate-parameters expects all members of MigrationParameters
+ * to be present, but we cannot mark them non-optional in QAPI because
+ * the structure is also used for migrate-set-parameters, which needs
+ * the optionality. Force all parameters to be seen as present
+ * now. Note that this depends on some form of default being set for
+ * every member of MigrationParameters, currently done during qdev
+ * init using migration_properties defined in this file. The TLS
+ * options are a special case because they don't have a default and
+ * need to be normalized before use.
+ */
+static void migrate_mark_all_params_present(MigrationParameters *p)
+{
+ int len, n_str_args = 3; /* tls-creds, tls-hostname, tls-authz */
+ bool *has_fields[] = {
+ &p->has_throttle_trigger_threshold, &p->has_cpu_throttle_initial,
+ &p->has_cpu_throttle_increment, &p->has_cpu_throttle_tailslow,
+ &p->has_max_bandwidth, &p->has_avail_switchover_bandwidth,
+ &p->has_downtime_limit, &p->has_x_checkpoint_delay,
+ &p->has_multifd_channels, &p->has_multifd_compression,
+ &p->has_multifd_zlib_level, &p->has_multifd_qatzip_level,
+ &p->has_multifd_zstd_level, &p->has_xbzrle_cache_size,
+ &p->has_max_postcopy_bandwidth, &p->has_max_cpu_throttle,
+ &p->has_announce_initial, &p->has_announce_max, &p->has_announce_rounds,
+ &p->has_announce_step, &p->has_block_bitmap_mapping,
+ &p->has_x_vcpu_dirty_limit_period, &p->has_vcpu_dirty_limit,
+ &p->has_mode, &p->has_zero_page_detection, &p->has_direct_io,
+ &p->has_cpr_exec_command,
+ };
+
+ len = ARRAY_SIZE(has_fields);
+ assert(len + n_str_args == MIGRATION_PARAMETER__MAX);
+
+ for (int i = 0; i < len; i++) {
+ *has_fields[i] = true;
+ }
+}
+
MigrationParameters *qmp_query_migrate_parameters(Error **errp)
{
MigrationParameters *params;
@@ -1027,70 +1065,53 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
/* TODO use QAPI_CLONE() instead of duplicating it inline */
params = g_malloc0(sizeof(*params));
- params->has_throttle_trigger_threshold = true;
+
params->throttle_trigger_threshold = s->parameters.throttle_trigger_threshold;
- params->has_cpu_throttle_initial = true;
params->cpu_throttle_initial = s->parameters.cpu_throttle_initial;
- params->has_cpu_throttle_increment = true;
params->cpu_throttle_increment = s->parameters.cpu_throttle_increment;
- params->has_cpu_throttle_tailslow = true;
params->cpu_throttle_tailslow = s->parameters.cpu_throttle_tailslow;
params->tls_creds = QAPI_CLONE(StrOrNull, s->parameters.tls_creds);
params->tls_hostname = QAPI_CLONE(StrOrNull, s->parameters.tls_hostname);
params->tls_authz = QAPI_CLONE(StrOrNull, s->parameters.tls_authz);
- params->has_max_bandwidth = true;
params->max_bandwidth = s->parameters.max_bandwidth;
- params->has_avail_switchover_bandwidth = true;
params->avail_switchover_bandwidth = s->parameters.avail_switchover_bandwidth;
- params->has_downtime_limit = true;
params->downtime_limit = s->parameters.downtime_limit;
- params->has_x_checkpoint_delay = true;
params->x_checkpoint_delay = s->parameters.x_checkpoint_delay;
- params->has_multifd_channels = true;
params->multifd_channels = s->parameters.multifd_channels;
- params->has_multifd_compression = true;
params->multifd_compression = s->parameters.multifd_compression;
- params->has_multifd_zlib_level = true;
params->multifd_zlib_level = s->parameters.multifd_zlib_level;
- params->has_multifd_qatzip_level = true;
params->multifd_qatzip_level = s->parameters.multifd_qatzip_level;
- params->has_multifd_zstd_level = true;
params->multifd_zstd_level = s->parameters.multifd_zstd_level;
- params->has_xbzrle_cache_size = true;
params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
- params->has_max_postcopy_bandwidth = true;
params->max_postcopy_bandwidth = s->parameters.max_postcopy_bandwidth;
- params->has_max_cpu_throttle = true;
params->max_cpu_throttle = s->parameters.max_cpu_throttle;
- params->has_announce_initial = true;
params->announce_initial = s->parameters.announce_initial;
- params->has_announce_max = true;
params->announce_max = s->parameters.announce_max;
- params->has_announce_rounds = true;
params->announce_rounds = s->parameters.announce_rounds;
- params->has_announce_step = true;
params->announce_step = s->parameters.announce_step;
-
- if (s->has_block_bitmap_mapping) {
- params->has_block_bitmap_mapping = true;
- params->block_bitmap_mapping =
- QAPI_CLONE(BitmapMigrationNodeAliasList,
- s->parameters.block_bitmap_mapping);
- }
-
- params->has_x_vcpu_dirty_limit_period = true;
params->x_vcpu_dirty_limit_period = s->parameters.x_vcpu_dirty_limit_period;
- params->has_vcpu_dirty_limit = true;
params->vcpu_dirty_limit = s->parameters.vcpu_dirty_limit;
- params->has_mode = true;
params->mode = s->parameters.mode;
- params->has_zero_page_detection = true;
params->zero_page_detection = s->parameters.zero_page_detection;
- params->has_direct_io = true;
params->direct_io = s->parameters.direct_io;
- params->has_cpr_exec_command = true;
params->cpr_exec_command = QAPI_CLONE(strList,
s->parameters.cpr_exec_command);
+ params->block_bitmap_mapping =
+ QAPI_CLONE(BitmapMigrationNodeAliasList,
+ s->parameters.block_bitmap_mapping);
+
+ migrate_mark_all_params_present(params);
+
+ /*
+ * The block-bitmap-mapping breaks the expected API of
+ * query-migrate-parameters of having all members present. To keep
+ * compatibility, only emit this field if it's actually been
+ * set. The empty list is a valid value.
+ */
+ if (!s->has_block_bitmap_mapping) {
+ params->has_block_bitmap_mapping = false;
+ qapi_free_BitmapMigrationNodeAliasList(params->block_bitmap_mapping);
+ }
return params;
}
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 27/31] migration: Use QAPI_CLONE_MEMBERS in query_migrate_parameters
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (25 preceding siblings ...)
2025-12-23 14:29 ` [PULL 26/31] migration: Extract code to mark all parameters as present Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 28/31] tests/qtest/migration: Pass MigrateCommon into test functions Peter Xu
` (4 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu
From: Fabiano Rosas <farosas@suse.de>
QAPI_CLONE_MEMBERS is a better option than copying parameters one by
one because it operates on the entire struct and follows pointers. It
also avoids the need to alter this function every time a new parameter
is added.
For this to work, the has_* fields of s->parameters need to be already
set beforehand, so move migrate_mark_all_params_present() to the init
routine.
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-14-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/options.c | 43 +++----------------------------------------
1 file changed, 3 insertions(+), 40 deletions(-)
diff --git a/migration/options.c b/migration/options.c
index 418cb44413..9a5a39c886 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1060,47 +1060,9 @@ static void migrate_mark_all_params_present(MigrationParameters *p)
MigrationParameters *qmp_query_migrate_parameters(Error **errp)
{
- MigrationParameters *params;
MigrationState *s = migrate_get_current();
-
- /* TODO use QAPI_CLONE() instead of duplicating it inline */
- params = g_malloc0(sizeof(*params));
-
- params->throttle_trigger_threshold = s->parameters.throttle_trigger_threshold;
- params->cpu_throttle_initial = s->parameters.cpu_throttle_initial;
- params->cpu_throttle_increment = s->parameters.cpu_throttle_increment;
- params->cpu_throttle_tailslow = s->parameters.cpu_throttle_tailslow;
- params->tls_creds = QAPI_CLONE(StrOrNull, s->parameters.tls_creds);
- params->tls_hostname = QAPI_CLONE(StrOrNull, s->parameters.tls_hostname);
- params->tls_authz = QAPI_CLONE(StrOrNull, s->parameters.tls_authz);
- params->max_bandwidth = s->parameters.max_bandwidth;
- params->avail_switchover_bandwidth = s->parameters.avail_switchover_bandwidth;
- params->downtime_limit = s->parameters.downtime_limit;
- params->x_checkpoint_delay = s->parameters.x_checkpoint_delay;
- params->multifd_channels = s->parameters.multifd_channels;
- params->multifd_compression = s->parameters.multifd_compression;
- params->multifd_zlib_level = s->parameters.multifd_zlib_level;
- params->multifd_qatzip_level = s->parameters.multifd_qatzip_level;
- params->multifd_zstd_level = s->parameters.multifd_zstd_level;
- params->xbzrle_cache_size = s->parameters.xbzrle_cache_size;
- params->max_postcopy_bandwidth = s->parameters.max_postcopy_bandwidth;
- params->max_cpu_throttle = s->parameters.max_cpu_throttle;
- params->announce_initial = s->parameters.announce_initial;
- params->announce_max = s->parameters.announce_max;
- params->announce_rounds = s->parameters.announce_rounds;
- params->announce_step = s->parameters.announce_step;
- params->x_vcpu_dirty_limit_period = s->parameters.x_vcpu_dirty_limit_period;
- params->vcpu_dirty_limit = s->parameters.vcpu_dirty_limit;
- params->mode = s->parameters.mode;
- params->zero_page_detection = s->parameters.zero_page_detection;
- params->direct_io = s->parameters.direct_io;
- params->cpr_exec_command = QAPI_CLONE(strList,
- s->parameters.cpr_exec_command);
- params->block_bitmap_mapping =
- QAPI_CLONE(BitmapMigrationNodeAliasList,
- s->parameters.block_bitmap_mapping);
-
- migrate_mark_all_params_present(params);
+ MigrationParameters *params = QAPI_CLONE(MigrationParameters,
+ &s->parameters);
/*
* The block-bitmap-mapping breaks the expected API of
@@ -1118,6 +1080,7 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
void migrate_params_init(MigrationParameters *params)
{
+ migrate_mark_all_params_present(params);
}
static void migrate_post_update_params(MigrationParameters *new, Error **errp)
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 28/31] tests/qtest/migration: Pass MigrateCommon into test functions
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (26 preceding siblings ...)
2025-12-23 14:29 ` [PULL 27/31] migration: Use QAPI_CLONE_MEMBERS in query_migrate_parameters Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 29/31] tests/qtest/migration: Pass MigrateStart into cancel tests Peter Xu
` (3 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu
From: Fabiano Rosas <farosas@suse.de>
With the upcoming addition of the config QDict, the tests will need a
better way of managing the memory of the test data than putting the
test arguments on the stack of the test functions. The config QDict
will need to be merged into the arguments of migrate_qmp* functions,
which causes a refcount increment, so the test functions would need to
allocate and deref the config QDict themselves.
A better approach is to already pass the arguments into the test
functions and do the memory management in the existing wrapper. There
is already migration_test_destroy(), which is called for every test.
Do the following:
- merge the two existing wrappers, migration_test_wrapper() and
migration_test_wrapper_full(). The latter was pioneer in passing
data into the tests, but now all tests will receive data, so we
don't need it anymore.
The usage of migration_test_wrapper_full() was in passing a slightly
different test name string into the cancel tests, so still keep the
migration_test_add_suffix() function.
- add (char *name, MigrateCommon *args) to the signature of all test
functions.
- alter any code to stop allocating args on the stack and instead use
the object that came as parameter.
- pass args around as needed.
- while here, order args (MigrateCommon) before args->start
(MigrateStart) and put a blank like in between.
No functional change.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-26-farosas@suse.de
[peterx: fix a conflict with newly added mapped-ram+ignore-share test]
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qtest/migration/migration-util.h | 8 +-
tests/qtest/migration/compression-tests.c | 131 +++---
tests/qtest/migration/cpr-tests.c | 75 ++--
tests/qtest/migration/file-tests.c | 206 +++++-----
tests/qtest/migration/migration-util.c | 26 +-
tests/qtest/migration/misc-tests.c | 112 +++--
tests/qtest/migration/postcopy-tests.c | 82 ++--
tests/qtest/migration/precopy-tests.c | 348 +++++++---------
tests/qtest/migration/tls-tests.c | 477 ++++++++++------------
9 files changed, 652 insertions(+), 813 deletions(-)
diff --git a/tests/qtest/migration/migration-util.h b/tests/qtest/migration/migration-util.h
index 44815e9c42..e73d69bab0 100644
--- a/tests/qtest/migration/migration-util.h
+++ b/tests/qtest/migration/migration-util.h
@@ -15,6 +15,8 @@
#include "libqtest.h"
+#include "migration/framework.h"
+
typedef struct QTestMigrationState {
bool stop_seen;
bool resume_seen;
@@ -50,9 +52,11 @@ static inline bool probe_o_direct_support(const char *tmpfs)
bool ufd_version_check(bool *uffd_feature_thread_id);
bool kvm_dirty_ring_supported(void);
-void migration_test_add(const char *path, void (*fn)(void));
+
+void migration_test_add(const char *path,
+ void (*fn)(char *name, MigrateCommon *args));
void migration_test_add_suffix(const char *path, const char *suffix,
- void (*fn)(void *));
+ void (*fn)(char *name, MigrateCommon *args));
char *migrate_get_connect_uri(QTestState *who);
void migrate_set_ports(QTestState *to, QList *channel_list);
diff --git a/tests/qtest/migration/compression-tests.c b/tests/qtest/migration/compression-tests.c
index b827665b8e..845e622cd5 100644
--- a/tests/qtest/migration/compression-tests.c
+++ b/tests/qtest/migration/compression-tests.c
@@ -31,30 +31,25 @@ migrate_hook_start_precopy_tcp_multifd_zstd(QTestState *from,
return migrate_hook_start_precopy_tcp_multifd_common(from, to, "zstd");
}
-static void test_multifd_tcp_zstd(void)
+static void test_multifd_tcp_zstd(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "defer",
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- .start_hook = migrate_hook_start_precopy_tcp_multifd_zstd,
- };
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_precopy_tcp_multifd_zstd;
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
-static void test_multifd_postcopy_tcp_zstd(void)
+static void test_multifd_postcopy_tcp_zstd(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "defer",
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- .caps[MIGRATION_CAPABILITY_POSTCOPY_RAM] = true,
- },
- .start_hook = migrate_hook_start_precopy_tcp_multifd_zstd,
- };
-
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_precopy_tcp_multifd_zstd,
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+ args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_RAM] = true;
+
+ test_precopy_common(args);
}
#endif /* CONFIG_ZSTD */
@@ -69,16 +64,14 @@ migrate_hook_start_precopy_tcp_multifd_qatzip(QTestState *from,
return migrate_hook_start_precopy_tcp_multifd_common(from, to, "qatzip");
}
-static void test_multifd_tcp_qatzip(void)
+static void test_multifd_tcp_qatzip(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "defer",
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- .start_hook = migrate_hook_start_precopy_tcp_multifd_qatzip,
- };
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_precopy_tcp_multifd_qatzip;
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
#endif
@@ -90,16 +83,14 @@ migrate_hook_start_precopy_tcp_multifd_qpl(QTestState *from,
return migrate_hook_start_precopy_tcp_multifd_common(from, to, "qpl");
}
-static void test_multifd_tcp_qpl(void)
+static void test_multifd_tcp_qpl(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "defer",
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- .start_hook = migrate_hook_start_precopy_tcp_multifd_qpl,
- };
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_precopy_tcp_multifd_qpl;
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
#endif /* CONFIG_QPL */
@@ -111,16 +102,14 @@ migrate_hook_start_precopy_tcp_multifd_uadk(QTestState *from,
return migrate_hook_start_precopy_tcp_multifd_common(from, to, "uadk");
}
-static void test_multifd_tcp_uadk(void)
+static void test_multifd_tcp_uadk(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "defer",
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- .start_hook = migrate_hook_start_precopy_tcp_multifd_uadk,
- };
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_precopy_tcp_multifd_uadk;
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
#endif /* CONFIG_UADK */
@@ -132,25 +121,23 @@ migrate_hook_start_xbzrle(QTestState *from,
return NULL;
}
-static void test_precopy_unix_xbzrle(void)
+static void test_precopy_unix_xbzrle(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
- MigrateCommon args = {
- .connect_uri = uri,
- .listen_uri = uri,
- .start_hook = migrate_hook_start_xbzrle,
- .iterations = 2,
- .start = {
- .caps[MIGRATION_CAPABILITY_XBZRLE] = true,
- },
- /*
- * XBZRLE needs pages to be modified when doing the 2nd+ round
- * iteration to have real data pushed to the stream.
- */
- .live = true,
- };
-
- test_precopy_common(&args);
+
+ args->connect_uri = uri;
+ args->listen_uri = uri;
+ args->start_hook = migrate_hook_start_xbzrle;
+ args->iterations = 2;
+ /*
+ * XBZRLE needs pages to be modified when doing the 2nd+ round
+ * iteration to have real data pushed to the stream.
+ */
+ args->live = true;
+
+ args->start.caps[MIGRATION_CAPABILITY_XBZRLE] = true;
+
+ test_precopy_common(args);
}
static void *
@@ -167,16 +154,14 @@ migrate_hook_start_precopy_tcp_multifd_zlib(QTestState *from,
return migrate_hook_start_precopy_tcp_multifd_common(from, to, "zlib");
}
-static void test_multifd_tcp_zlib(void)
+static void test_multifd_tcp_zlib(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "defer",
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- .start_hook = migrate_hook_start_precopy_tcp_multifd_zlib,
- };
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_precopy_tcp_multifd_zlib;
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
static void migration_test_add_compression_smoke(MigrationTestEnv *env)
diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c
index 2a186c6f35..0d97b5b89f 100644
--- a/tests/qtest/migration/cpr-tests.c
+++ b/tests/qtest/migration/cpr-tests.c
@@ -27,21 +27,19 @@ static void *migrate_hook_start_mode_reboot(QTestState *from, QTestState *to)
return NULL;
}
-static void test_mode_reboot(void)
+static void test_mode_reboot(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
FILE_TEST_FILENAME);
- MigrateCommon args = {
- .start.mem_type = MEM_TYPE_SHMEM,
- .connect_uri = uri,
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_mode_reboot,
- .start = {
- .caps[MIGRATION_CAPABILITY_X_IGNORE_SHARED] = true,
- },
- };
-
- test_file_common(&args, true);
+
+ args->connect_uri = uri;
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_mode_reboot;
+
+ args->start.mem_type = MEM_TYPE_SHMEM;
+ args->start.caps[MIGRATION_CAPABILITY_X_IGNORE_SHARED] = true;
+
+ test_file_common(args, true);
}
static void *test_mode_transfer_start(QTestState *from, QTestState *to)
@@ -55,7 +53,7 @@ static void *test_mode_transfer_start(QTestState *from, QTestState *to)
* migration, and cannot connect synchronously to the monitor, so defer
* the target connection.
*/
-static void test_mode_transfer_common(bool incoming_defer)
+static void test_mode_transfer_common(MigrateCommon *args, bool incoming_defer)
{
g_autofree char *cpr_path = g_strdup_printf("%s/cpr.sock", tmpfs);
g_autofree char *mig_path = g_strdup_printf("%s/migsocket", tmpfs);
@@ -85,31 +83,31 @@ static void test_mode_transfer_common(bool incoming_defer)
opts_target = g_strdup_printf("-incoming cpr,addr.transport=socket,"
"addr.type=fd,addr.str=%d %s",
cpr_sockfd, opts);
- MigrateCommon args = {
- .start.opts_source = opts,
- .start.opts_target = opts_target,
- .start.defer_target_connect = true,
- .start.mem_type = MEM_TYPE_MEMFD,
- .listen_uri = incoming_defer ? "defer" : uri,
- .connect_channels = connect_channels,
- .cpr_channel = cpr_channel,
- .start_hook = test_mode_transfer_start,
- };
-
- if (test_precopy_common(&args) < 0) {
+
+ args->listen_uri = incoming_defer ? "defer" : uri;
+ args->connect_channels = connect_channels;
+ args->cpr_channel = cpr_channel;
+ args->start_hook = test_mode_transfer_start;
+
+ args->start.opts_source = opts;
+ args->start.opts_target = opts_target;
+ args->start.defer_target_connect = true;
+ args->start.mem_type = MEM_TYPE_MEMFD;
+
+ if (test_precopy_common(args) < 0) {
close(cpr_sockfd);
unlink(cpr_path);
}
}
-static void test_mode_transfer(void)
+static void test_mode_transfer(char *name, MigrateCommon *args)
{
- test_mode_transfer_common(NULL);
+ test_mode_transfer_common(args, false);
}
-static void test_mode_transfer_defer(void)
+static void test_mode_transfer_defer(char *name, MigrateCommon *args)
{
- test_mode_transfer_common(true);
+ test_mode_transfer_common(args, true);
}
static void set_cpr_exec_args(QTestState *who, MigrateCommon *args)
@@ -225,22 +223,21 @@ static void *test_mode_exec_start(QTestState *from, QTestState *to)
return NULL;
}
-static void test_mode_exec(void)
+static void test_mode_exec(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
FILE_TEST_FILENAME);
g_autofree char *listen_uri = g_strdup_printf("defer");
- MigrateCommon args = {
- .start.only_source = true,
- .start.opts_source = "-machine aux-ram-share=on -nodefaults",
- .start.mem_type = MEM_TYPE_MEMFD,
- .connect_uri = uri,
- .listen_uri = listen_uri,
- .start_hook = test_mode_exec_start,
- };
+ args->connect_uri = uri;
+ args->listen_uri = listen_uri;
+ args->start_hook = test_mode_exec_start;
+
+ args->start.only_source = true;
+ args->start.opts_source = "-machine aux-ram-share=on -nodefaults";
+ args->start.mem_type = MEM_TYPE_MEMFD;
- test_cpr_exec(&args);
+ test_cpr_exec(args);
}
void migration_test_add_cpr(MigrationTestEnv *env)
diff --git a/tests/qtest/migration/file-tests.c b/tests/qtest/migration/file-tests.c
index c196a703ff..5d1b861cf6 100644
--- a/tests/qtest/migration/file-tests.c
+++ b/tests/qtest/migration/file-tests.c
@@ -20,16 +20,14 @@
static char *tmpfs;
-static void test_precopy_file(void)
+static void test_precopy_file(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
FILE_TEST_FILENAME);
- MigrateCommon args = {
- .connect_uri = uri,
- .listen_uri = "defer",
- };
+ args->connect_uri = uri;
+ args->listen_uri = "defer";
- test_file_common(&args, true);
+ test_file_common(args, true);
}
#ifndef _WIN32
@@ -66,107 +64,94 @@ static void *migrate_hook_start_file_offset_fdset(QTestState *from,
return NULL;
}
-static void test_precopy_file_offset_fdset(void)
+static void test_precopy_file_offset_fdset(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("file:/dev/fdset/1,offset=%d",
FILE_TEST_OFFSET);
- MigrateCommon args = {
- .connect_uri = uri,
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_file_offset_fdset,
- };
+ args->connect_uri = uri;
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_file_offset_fdset;
- test_file_common(&args, false);
+ test_file_common(args, false);
}
#endif
-static void test_precopy_file_offset(void)
+static void test_precopy_file_offset(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("file:%s/%s,offset=%d", tmpfs,
FILE_TEST_FILENAME,
FILE_TEST_OFFSET);
- MigrateCommon args = {
- .connect_uri = uri,
- .listen_uri = "defer",
- };
- test_file_common(&args, false);
+ args->connect_uri = uri;
+ args->listen_uri = "defer";
+
+ test_file_common(args, false);
}
-static void test_precopy_file_offset_bad(void)
+static void test_precopy_file_offset_bad(char *name, MigrateCommon *args)
{
/* using a value not supported by qemu_strtosz() */
g_autofree char *uri = g_strdup_printf("file:%s/%s,offset=0x20M",
tmpfs, FILE_TEST_FILENAME);
- MigrateCommon args = {
- .connect_uri = uri,
- .listen_uri = "defer",
- .result = MIG_TEST_QMP_ERROR,
- };
- test_file_common(&args, false);
+ args->connect_uri = uri;
+ args->listen_uri = "defer";
+ args->result = MIG_TEST_QMP_ERROR;
+
+ test_file_common(args, false);
}
-static void test_precopy_file_mapped_ram_live(void)
+static void test_precopy_file_mapped_ram_live(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
FILE_TEST_FILENAME);
- MigrateCommon args = {
- .connect_uri = uri,
- .listen_uri = "defer",
- .start = {
- .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
- },
- };
-
- test_file_common(&args, false);
+
+ args->connect_uri = uri;
+ args->listen_uri = "defer";
+
+ args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true;
+
+ test_file_common(args, false);
}
-static void test_precopy_file_mapped_ram(void)
+static void test_precopy_file_mapped_ram(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
FILE_TEST_FILENAME);
- MigrateCommon args = {
- .connect_uri = uri,
- .listen_uri = "defer",
- .start = {
- .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
- },
- };
-
- test_file_common(&args, true);
+
+ args->connect_uri = uri;
+ args->listen_uri = "defer";
+
+ args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true;
+
+ test_file_common(args, true);
}
-static void test_multifd_file_mapped_ram_live(void)
+static void test_multifd_file_mapped_ram_live(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
FILE_TEST_FILENAME);
- MigrateCommon args = {
- .connect_uri = uri,
- .listen_uri = "defer",
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
- },
- };
-
- test_file_common(&args, false);
+ args->connect_uri = uri;
+ args->listen_uri = "defer";
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+ args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true;
+
+ test_file_common(args, false);
}
-static void test_multifd_file_mapped_ram(void)
+static void test_multifd_file_mapped_ram(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
FILE_TEST_FILENAME);
- MigrateCommon args = {
- .connect_uri = uri,
- .listen_uri = "defer",
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
- },
- };
-
- test_file_common(&args, true);
+
+ args->connect_uri = uri;
+ args->listen_uri = "defer";
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+ args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true;
+
+ test_file_common(args, true);
}
static void *migrate_hook_start_multifd_mapped_ram_dio(QTestState *from,
@@ -178,26 +163,23 @@ static void *migrate_hook_start_multifd_mapped_ram_dio(QTestState *from,
return NULL;
}
-static void test_multifd_file_mapped_ram_dio(void)
+static void test_multifd_file_mapped_ram_dio(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
FILE_TEST_FILENAME);
- MigrateCommon args = {
- .connect_uri = uri,
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_multifd_mapped_ram_dio,
- .start = {
- .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- };
+ args->connect_uri = uri;
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_multifd_mapped_ram_dio;
+
+ args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true;
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
if (!probe_o_direct_support(tmpfs)) {
g_test_skip("Filesystem does not support O_DIRECT");
return;
}
- test_file_common(&args, true);
+ test_file_common(args, true);
}
#ifndef _WIN32
@@ -252,45 +234,41 @@ static void *migrate_hook_start_multifd_mapped_ram_fdset(QTestState *from,
return NULL;
}
-static void test_multifd_file_mapped_ram_fdset(void)
+static void test_multifd_file_mapped_ram_fdset(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("file:/dev/fdset/1,offset=%d",
FILE_TEST_OFFSET);
- MigrateCommon args = {
- .connect_uri = uri,
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_multifd_mapped_ram_fdset,
- .end_hook = migrate_hook_end_multifd_mapped_ram_fdset,
- .start = {
- .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- };
-
- test_file_common(&args, true);
+
+ args->connect_uri = uri;
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_multifd_mapped_ram_fdset;
+ args->end_hook = migrate_hook_end_multifd_mapped_ram_fdset;
+
+ args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true;
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_file_common(args, true);
}
-static void test_multifd_file_mapped_ram_fdset_dio(void)
+static void test_multifd_file_mapped_ram_fdset_dio(char *name,
+ MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("file:/dev/fdset/1,offset=%d",
FILE_TEST_OFFSET);
- MigrateCommon args = {
- .connect_uri = uri,
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_multifd_mapped_ram_fdset_dio,
- .end_hook = migrate_hook_end_multifd_mapped_ram_fdset,
- .start = {
- .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- };
+ args->connect_uri = uri;
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_multifd_mapped_ram_fdset_dio;
+ args->end_hook = migrate_hook_end_multifd_mapped_ram_fdset;
+
+ args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true;
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
if (!probe_o_direct_support(tmpfs)) {
g_test_skip("Filesystem does not support O_DIRECT");
return;
}
- test_file_common(&args, true);
+ test_file_common(args, true);
}
#endif /* !_WIN32 */
@@ -303,20 +281,18 @@ static void migration_test_add_file_smoke(MigrationTestEnv *env)
test_multifd_file_mapped_ram_dio);
}
-static void test_precopy_file_mapped_ram_ignore_shared(void)
+static void
+test_precopy_file_mapped_ram_ignore_shared(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
FILE_TEST_FILENAME);
- MigrateCommon args = {
- .connect_uri = uri,
- .listen_uri = "defer",
- .start = {
- .caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true,
- .caps[MIGRATION_CAPABILITY_X_IGNORE_SHARED] = true,
- },
- };
-
- test_file_common(&args, true);
+ args->connect_uri = uri;
+ args->listen_uri = "defer";
+
+ args->start.caps[MIGRATION_CAPABILITY_MAPPED_RAM] = true;
+ args->start.caps[MIGRATION_CAPABILITY_X_IGNORE_SHARED] = true;
+
+ test_file_common(args, true);
}
void migration_test_add_file(MigrationTestEnv *env)
diff --git a/tests/qtest/migration/migration-util.c b/tests/qtest/migration/migration-util.c
index 642cf50c8d..c2462306a1 100644
--- a/tests/qtest/migration/migration-util.c
+++ b/tests/qtest/migration/migration-util.c
@@ -235,14 +235,15 @@ char *resolve_machine_version(const char *alias, const char *var1,
typedef struct {
char *name;
- void (*func)(void);
- void (*func_full)(void *);
+ MigrateCommon *data;
+ void (*func)(char *name, MigrateCommon *args);
} MigrationTest;
static void migration_test_destroy(gpointer data)
{
MigrationTest *test = (MigrationTest *)data;
+ g_free(test->data);
g_free(test->name);
g_free(test);
}
@@ -251,11 +252,14 @@ static void migration_test_wrapper(const void *data)
{
MigrationTest *test = (MigrationTest *)data;
+ test->data = g_new0(MigrateCommon, 1);
+
g_test_message("Running /%s%s", qtest_get_arch(), test->name);
- test->func();
+ test->func(test->name, test->data);
}
-void migration_test_add(const char *path, void (*fn)(void))
+void migration_test_add(const char *path,
+ void (*fn)(char *name, MigrateCommon *args))
{
MigrationTest *test = g_new0(MigrationTest, 1);
@@ -266,26 +270,18 @@ void migration_test_add(const char *path, void (*fn)(void))
migration_test_destroy);
}
-static void migration_test_wrapper_full(const void *data)
-{
- MigrationTest *test = (MigrationTest *)data;
-
- g_test_message("Running /%s%s", qtest_get_arch(), test->name);
- test->func_full(test->name);
-}
-
void migration_test_add_suffix(const char *path, const char *suffix,
- void (*fn)(void *))
+ void (*fn)(char *name, MigrateCommon *args))
{
MigrationTest *test = g_new0(MigrationTest, 1);
g_assert(g_str_has_suffix(path, "/"));
g_assert(!g_str_has_prefix(suffix, "/"));
- test->func_full = fn;
+ test->func = fn;
test->name = g_strconcat(path, suffix, NULL);
- qtest_add_data_func_full(test->name, test, migration_test_wrapper_full,
+ qtest_add_data_func_full(test->name, test, migration_test_wrapper,
migration_test_destroy);
}
diff --git a/tests/qtest/migration/misc-tests.c b/tests/qtest/migration/misc-tests.c
index 20edaa51f5..810e9e6549 100644
--- a/tests/qtest/migration/misc-tests.c
+++ b/tests/qtest/migration/misc-tests.c
@@ -22,14 +22,13 @@
static char *tmpfs;
-static void test_baddest(void)
+static void test_baddest(char *name, MigrateCommon *args)
{
- MigrateStart args = {
- .hide_stderr = true
- };
QTestState *from, *to;
- if (migrate_start(&from, &to, "tcp:127.0.0.1:0", &args)) {
+ args->start.hide_stderr = true;
+
+ if (migrate_start(&from, &to, "tcp:127.0.0.1:0", &args->start)) {
return;
}
migrate_qmp(from, to, "tcp:127.0.0.1:0", NULL, "{}");
@@ -38,24 +37,23 @@ static void test_baddest(void)
}
#ifndef _WIN32
-static void test_analyze_script(void)
+static void test_analyze_script(char *name, MigrateCommon *args)
{
- MigrateStart args = {
- .opts_source = "-uuid 11111111-1111-1111-1111-111111111111",
- };
QTestState *from, *to;
g_autofree char *uri = NULL;
g_autofree char *file = NULL;
int pid, wstatus;
const char *python = g_getenv("PYTHON");
+ args->start.opts_source = "-uuid 11111111-1111-1111-1111-111111111111";
+
if (!python) {
g_test_skip("PYTHON variable not set");
return;
}
/* dummy url */
- if (migrate_start(&from, &to, "tcp:127.0.0.1:0", &args)) {
+ if (migrate_start(&from, &to, "tcp:127.0.0.1:0", &args->start)) {
return;
}
@@ -92,16 +90,15 @@ static void test_analyze_script(void)
}
#endif
-static void test_ignore_shared(void)
+static void test_ignore_shared(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
QTestState *from, *to;
- MigrateStart args = {
- .mem_type = MEM_TYPE_SHMEM,
- .caps[MIGRATION_CAPABILITY_X_IGNORE_SHARED] = true,
- };
- if (migrate_start(&from, &to, uri, &args)) {
+ args->start.mem_type = MEM_TYPE_SHMEM;
+ args->start.caps[MIGRATION_CAPABILITY_X_IGNORE_SHARED] = true;
+
+ if (migrate_start(&from, &to, uri, &args->start)) {
return;
}
@@ -161,45 +158,37 @@ static void do_test_validate_uuid(MigrateStart *args, bool should_fail)
migrate_end(from, to, false);
}
-static void test_validate_uuid(void)
+static void test_validate_uuid(char *name, MigrateCommon *args)
{
- MigrateStart args = {
- .opts_source = "-uuid 11111111-1111-1111-1111-111111111111",
- .opts_target = "-uuid 11111111-1111-1111-1111-111111111111",
- };
+ args->start.opts_source = "-uuid 11111111-1111-1111-1111-111111111111";
+ args->start.opts_target = "-uuid 11111111-1111-1111-1111-111111111111";
- do_test_validate_uuid(&args, false);
+ do_test_validate_uuid(&args->start, false);
}
-static void test_validate_uuid_error(void)
+static void test_validate_uuid_error(char *name, MigrateCommon *args)
{
- MigrateStart args = {
- .opts_source = "-uuid 11111111-1111-1111-1111-111111111111",
- .opts_target = "-uuid 22222222-2222-2222-2222-222222222222",
- .hide_stderr = true,
- };
+ args->start.opts_source = "-uuid 11111111-1111-1111-1111-111111111111";
+ args->start.opts_target = "-uuid 22222222-2222-2222-2222-222222222222";
+ args->start.hide_stderr = true;
- do_test_validate_uuid(&args, true);
+ do_test_validate_uuid(&args->start, true);
}
-static void test_validate_uuid_src_not_set(void)
+static void test_validate_uuid_src_not_set(char *name, MigrateCommon *args)
{
- MigrateStart args = {
- .opts_target = "-uuid 22222222-2222-2222-2222-222222222222",
- .hide_stderr = true,
- };
+ args->start.opts_target = "-uuid 22222222-2222-2222-2222-222222222222";
+ args->start.hide_stderr = true;
- do_test_validate_uuid(&args, false);
+ do_test_validate_uuid(&args->start, false);
}
-static void test_validate_uuid_dst_not_set(void)
+static void test_validate_uuid_dst_not_set(char *name, MigrateCommon *args)
{
- MigrateStart args = {
- .opts_source = "-uuid 11111111-1111-1111-1111-111111111111",
- .hide_stderr = true,
- };
+ args->start.opts_source = "-uuid 11111111-1111-1111-1111-111111111111";
+ args->start.hide_stderr = true;
- do_test_validate_uuid(&args, false);
+ do_test_validate_uuid(&args->start, false);
}
static void do_test_validate_uri_channel(MigrateCommon *args)
@@ -226,34 +215,27 @@ static void do_test_validate_uri_channel(MigrateCommon *args)
migrate_end(from, to, false);
}
-static void test_validate_uri_channels_both_set(void)
+static void test_validate_uri_channels_both_set(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .start = {
- .hide_stderr = true,
- },
- .listen_uri = "defer",
- .connect_uri = "tcp:127.0.0.1:0",
- .connect_channels = ("[ { ""'channel-type': 'main',"
- " 'addr': { 'transport': 'socket',"
- " 'type': 'inet',"
- " 'host': '127.0.0.1',"
- " 'port': '0' } } ]"),
- };
-
- do_test_validate_uri_channel(&args);
+ args->listen_uri = "defer",
+ args->connect_uri = "tcp:127.0.0.1:0",
+ args->connect_channels = ("[ { ""'channel-type': 'main',"
+ " 'addr': { 'transport': 'socket',"
+ " 'type': 'inet',"
+ " 'host': '127.0.0.1',"
+ " 'port': '0' } } ]"),
+
+ args->start.hide_stderr = true;
+
+ do_test_validate_uri_channel(args);
}
-static void test_validate_uri_channels_none_set(void)
+static void test_validate_uri_channels_none_set(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .start = {
- .hide_stderr = true,
- },
- .listen_uri = "defer",
- };
-
- do_test_validate_uri_channel(&args);
+ args->listen_uri = "defer";
+ args->start.hide_stderr = true;
+
+ do_test_validate_uri_channel(args);
}
static void migration_test_add_misc_smoke(MigrationTestEnv *env)
diff --git a/tests/qtest/migration/postcopy-tests.c b/tests/qtest/migration/postcopy-tests.c
index 3773525843..7ae4d765d7 100644
--- a/tests/qtest/migration/postcopy-tests.c
+++ b/tests/qtest/migration/postcopy-tests.c
@@ -20,67 +20,51 @@
#include "qemu/range.h"
#include "qemu/sockets.h"
-static void test_postcopy(void)
+static void test_postcopy(char *name, MigrateCommon *args)
{
- MigrateCommon args = { };
-
- test_postcopy_common(&args);
+ test_postcopy_common(args);
}
-static void test_postcopy_suspend(void)
+static void test_postcopy_suspend(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .start.suspend_me = true,
- };
+ args->start.suspend_me = true;
- test_postcopy_common(&args);
+ test_postcopy_common(args);
}
-static void test_postcopy_preempt(void)
+static void test_postcopy_preempt(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .start = {
- .caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true,
- },
- };
+ args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true;
- test_postcopy_common(&args);
+ test_postcopy_common(args);
}
-static void test_postcopy_recovery(void)
+static void test_postcopy_recovery(char *name, MigrateCommon *args)
{
- MigrateCommon args = { };
-
- test_postcopy_recovery_common(&args);
+ test_postcopy_recovery_common(args);
}
-static void test_postcopy_recovery_fail_handshake(void)
+static void test_postcopy_recovery_fail_handshake(char *name,
+ MigrateCommon *args)
{
- MigrateCommon args = {
- .postcopy_recovery_fail_stage = POSTCOPY_FAIL_RECOVERY,
- };
+ args->postcopy_recovery_fail_stage = POSTCOPY_FAIL_RECOVERY;
- test_postcopy_recovery_common(&args);
+ test_postcopy_recovery_common(args);
}
-static void test_postcopy_recovery_fail_reconnect(void)
+static void test_postcopy_recovery_fail_reconnect(char *name,
+ MigrateCommon *args)
{
- MigrateCommon args = {
- .postcopy_recovery_fail_stage = POSTCOPY_FAIL_CHANNEL_ESTABLISH,
- };
+ args->postcopy_recovery_fail_stage = POSTCOPY_FAIL_CHANNEL_ESTABLISH;
- test_postcopy_recovery_common(&args);
+ test_postcopy_recovery_common(args);
}
-static void test_postcopy_preempt_recovery(void)
+static void test_postcopy_preempt_recovery(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .start = {
- .caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true,
- },
- };
+ args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true;
- test_postcopy_recovery_common(&args);
+ test_postcopy_recovery_common(args);
}
static void migration_test_add_postcopy_smoke(MigrationTestEnv *env)
@@ -94,27 +78,19 @@ static void migration_test_add_postcopy_smoke(MigrationTestEnv *env)
}
}
-static void test_multifd_postcopy(void)
+static void test_multifd_postcopy(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- };
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
- test_postcopy_common(&args);
+ test_postcopy_common(args);
}
-static void test_multifd_postcopy_preempt(void)
+static void test_multifd_postcopy_preempt(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- .caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true,
- },
- };
-
- test_postcopy_common(&args);
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+ args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true;
+
+ test_postcopy_common(args);
}
void migration_test_add_postcopy(MigrationTestEnv *env)
diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c
index 57ca623de5..086d06a31c 100644
--- a/tests/qtest/migration/precopy-tests.c
+++ b/tests/qtest/migration/precopy-tests.c
@@ -35,68 +35,64 @@
static char *tmpfs;
-static void test_precopy_unix_plain(void)
+static void test_precopy_unix_plain(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
- MigrateCommon args = {
- .listen_uri = uri,
- .connect_uri = uri,
- /*
- * The simplest use case of precopy, covering smoke tests of
- * get-dirty-log dirty tracking.
- */
- .live = true,
- };
- test_precopy_common(&args);
+ args->listen_uri = uri;
+ args->connect_uri = uri;
+ /*
+ * The simplest use case of precopy, covering smoke tests of
+ * get-dirty-log dirty tracking.
+ */
+ args->live = true;
+
+ test_precopy_common(args);
}
-static void test_precopy_unix_suspend_live(void)
+static void test_precopy_unix_suspend_live(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
- MigrateCommon args = {
- .listen_uri = uri,
- .connect_uri = uri,
- /*
- * despite being live, the test is fast because the src
- * suspends immediately.
- */
- .live = true,
- .start.suspend_me = true,
- };
- test_precopy_common(&args);
+ args->listen_uri = uri;
+ args->connect_uri = uri;
+ /*
+ * despite being live, the test is fast because the src
+ * suspends immediately.
+ */
+ args->live = true;
+
+ args->start.suspend_me = true;
+
+ test_precopy_common(args);
}
-static void test_precopy_unix_suspend_notlive(void)
+static void test_precopy_unix_suspend_notlive(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
- MigrateCommon args = {
- .listen_uri = uri,
- .connect_uri = uri,
- .start.suspend_me = true,
- };
- test_precopy_common(&args);
+ args->listen_uri = uri;
+ args->connect_uri = uri;
+ args->start.suspend_me = true;
+
+ test_precopy_common(args);
}
-static void test_precopy_unix_dirty_ring(void)
+static void test_precopy_unix_dirty_ring(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
- MigrateCommon args = {
- .start = {
- .use_dirty_ring = true,
- },
- .listen_uri = uri,
- .connect_uri = uri,
- /*
- * Besides the precopy/unix basic test, cover dirty ring interface
- * rather than get-dirty-log.
- */
- .live = true,
- };
- test_precopy_common(&args);
+ args->listen_uri = uri;
+ args->connect_uri = uri;
+ /*
+ * Besides the precopy/unix basic test, cover dirty ring interface
+ * rather than get-dirty-log.
+ */
+ args->live = true;
+
+ args->start.use_dirty_ring = true;
+
+ test_precopy_common(args);
}
#ifdef CONFIG_RDMA
@@ -162,7 +158,7 @@ static int new_rdma_link(char *buffer, bool ipv6)
return -1;
}
-static void __test_precopy_rdma_plain(bool ipv6)
+static void __test_precopy_rdma_plain(MigrateCommon *args, bool ipv6)
{
char buffer[128] = {};
@@ -187,50 +183,43 @@ static void __test_precopy_rdma_plain(bool ipv6)
**/
g_autofree char *uri = g_strdup_printf("rdma:%s:29200", buffer);
- MigrateCommon args = {
- .listen_uri = uri,
- .connect_uri = uri,
- };
+ args->listen_uri = uri;
+ args->connect_uri = uri;
- test_precopy_common(&args);
+ test_precopy_common(args);
}
-static void test_precopy_rdma_plain(void)
+static void test_precopy_rdma_plain(char *name, MigrateCommon *args)
{
- __test_precopy_rdma_plain(false);
+ __test_precopy_rdma_plain(args, false);
}
-static void test_precopy_rdma_plain_ipv6(void)
+static void test_precopy_rdma_plain_ipv6(char *name, MigrateCommon *args)
{
- __test_precopy_rdma_plain(true);
+ __test_precopy_rdma_plain(args, true);
}
#endif
-static void test_precopy_tcp_plain(void)
+static void test_precopy_tcp_plain(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "tcp:127.0.0.1:0",
- };
+ args->listen_uri = "tcp:127.0.0.1:0";
- test_precopy_common(&args);
+ test_precopy_common(args);
}
-static void test_precopy_tcp_switchover_ack(void)
+static void test_precopy_tcp_switchover_ack(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "tcp:127.0.0.1:0",
- .start = {
- .caps[MIGRATION_CAPABILITY_RETURN_PATH] = true,
- .caps[MIGRATION_CAPABILITY_SWITCHOVER_ACK] = true,
- },
- /*
- * Source VM must be running in order to consider the switchover ACK
- * when deciding to do switchover or not.
- */
- .live = true,
- };
+ args->listen_uri = "tcp:127.0.0.1:0";
+ /*
+ * Source VM must be running in order to consider the switchover ACK
+ * when deciding to do switchover or not.
+ */
+ args->live = true;
- test_precopy_common(&args);
+ args->start.caps[MIGRATION_CAPABILITY_RETURN_PATH] = true;
+ args->start.caps[MIGRATION_CAPABILITY_SWITCHOVER_ACK] = true;
+
+ test_precopy_common(args);
}
#ifndef _WIN32
@@ -291,15 +280,14 @@ static void migrate_hook_end_fd(QTestState *from,
qobject_unref(rsp);
}
-static void test_precopy_fd_socket(void)
+static void test_precopy_fd_socket(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "defer",
- .connect_uri = "fd:fd-mig",
- .start_hook = migrate_hook_start_fd,
- .end_hook = migrate_hook_end_fd,
- };
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->connect_uri = "fd:fd-mig";
+ args->start_hook = migrate_hook_start_fd;
+ args->end_hook = migrate_hook_end_fd;
+
+ test_precopy_common(args);
}
static void *migrate_hook_start_precopy_fd_file(QTestState *from,
@@ -331,15 +319,14 @@ static void *migrate_hook_start_precopy_fd_file(QTestState *from,
return NULL;
}
-static void test_precopy_fd_file(void)
+static void test_precopy_fd_file(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "defer",
- .connect_uri = "fd:fd-mig",
- .start_hook = migrate_hook_start_precopy_fd_file,
- .end_hook = migrate_hook_end_fd,
- };
- test_file_common(&args, true);
+ args->listen_uri = "defer";
+ args->connect_uri = "fd:fd-mig";
+ args->start_hook = migrate_hook_start_precopy_fd_file;
+ args->end_hook = migrate_hook_end_fd;
+
+ test_file_common(args, true);
}
#endif /* _WIN32 */
@@ -358,10 +345,9 @@ static void test_precopy_fd_file(void)
* To make things even worse, we need to run the initial stage at
* 3MB/s so we enter autoconverge even when host is (over)loaded.
*/
-static void test_auto_converge(void)
+static void test_auto_converge(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
- MigrateStart args = {};
QTestState *from, *to;
int64_t percentage;
@@ -374,7 +360,7 @@ static void test_auto_converge(void)
uint64_t prev_dirty_sync_cnt, dirty_sync_cnt;
int max_try_count, hit = 0;
- if (migrate_start(&from, &to, uri, &args)) {
+ if (migrate_start(&from, &to, uri, &args->start)) {
return;
}
@@ -486,76 +472,68 @@ migrate_hook_start_precopy_tcp_multifd_no_zero_page(QTestState *from,
return NULL;
}
-static void test_multifd_tcp_uri_none(void)
+static void test_multifd_tcp_uri_none(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_precopy_tcp_multifd,
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- /*
- * Multifd is more complicated than most of the features, it
- * directly takes guest page buffers when sending, make sure
- * everything will work alright even if guest page is changing.
- */
- .live = true,
- };
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_precopy_tcp_multifd;
+ /*
+ * Multifd is more complicated than most of the features, it
+ * directly takes guest page buffers when sending, make sure
+ * everything will work alright even if guest page is changing.
+ */
+ args->live = true;
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
-static void test_multifd_tcp_zero_page_legacy(void)
+static void test_multifd_tcp_zero_page_legacy(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_precopy_tcp_multifd_zero_page_legacy,
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- /*
- * Multifd is more complicated than most of the features, it
- * directly takes guest page buffers when sending, make sure
- * everything will work alright even if guest page is changing.
- */
- .live = true,
- };
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_precopy_tcp_multifd_zero_page_legacy;
+ /*
+ * Multifd is more complicated than most of the features, it
+ * directly takes guest page buffers when sending, make sure
+ * everything will work alright even if guest page is changing.
+ */
+ args->live = true;
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
-static void test_multifd_tcp_no_zero_page(void)
+static void test_multifd_tcp_no_zero_page(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_precopy_tcp_multifd_no_zero_page,
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- /*
- * Multifd is more complicated than most of the features, it
- * directly takes guest page buffers when sending, make sure
- * everything will work alright even if guest page is changing.
- */
- .live = true,
- };
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_precopy_tcp_multifd_no_zero_page;
+ /*
+ * Multifd is more complicated than most of the features, it
+ * directly takes guest page buffers when sending, make sure
+ * everything will work alright even if guest page is changing.
+ */
+ args->live = true;
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
-static void test_multifd_tcp_channels_none(void)
+static void test_multifd_tcp_channels_none(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_precopy_tcp_multifd,
- .live = true,
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- .connect_channels = ("[ { 'channel-type': 'main',"
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_precopy_tcp_multifd;
+ args->live = true;
+ args->connect_channels = ("[ { 'channel-type': 'main',"
" 'addr': { 'transport': 'socket',"
" 'type': 'inet',"
" 'host': '127.0.0.1',"
- " 'port': '0' } } ]"),
- };
- test_precopy_common(&args);
+ " 'port': '0' } } ]");
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
/*
@@ -569,14 +547,13 @@ static void test_multifd_tcp_channels_none(void)
*
* And see that it works
*/
-static void test_multifd_tcp_cancel(bool postcopy_ram)
+static void test_multifd_tcp_cancel(MigrateCommon *args, bool postcopy_ram)
{
- MigrateStart args = {
- .hide_stderr = true,
- };
QTestState *from, *to, *to2;
- if (migrate_start(&from, &to, "defer", &args)) {
+ args->start.hide_stderr = true;
+
+ if (migrate_start(&from, &to, "defer", &args->start)) {
return;
}
@@ -621,11 +598,9 @@ static void test_multifd_tcp_cancel(bool postcopy_ram)
*/
wait_for_migration_status(from, "cancelled", NULL);
- args = (MigrateStart){
- .only_target = true,
- };
+ args->start.only_target = true;
- if (migrate_start(&from, &to2, "defer", &args)) {
+ if (migrate_start(&from, &to2, "defer", &args->start)) {
return;
}
@@ -656,14 +631,14 @@ static void test_multifd_tcp_cancel(bool postcopy_ram)
migrate_end(from, to2, true);
}
-static void test_multifd_precopy_tcp_cancel(void)
+static void test_multifd_precopy_tcp_cancel(char *name, MigrateCommon *args)
{
- test_multifd_tcp_cancel(false);
+ test_multifd_tcp_cancel(args, false);
}
-static void test_multifd_postcopy_tcp_cancel(void)
+static void test_multifd_postcopy_tcp_cancel(char *name, MigrateCommon *args)
{
- test_multifd_tcp_cancel(true);
+ test_multifd_tcp_cancel(args, true);
}
static void test_cancel_src_after_failed(QTestState *from, QTestState *to,
@@ -786,17 +761,15 @@ static void test_cancel_src_pre_switchover(QTestState *from, QTestState *to,
(const char * []) { "completed", NULL });
}
-static void test_cancel_src_after_status(void *opaque)
+static void test_cancel_src_after_status(char *test_path, MigrateCommon *args)
{
- const char *test_path = opaque;
g_autofree char *phase = g_path_get_basename(test_path);
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
QTestState *from, *to;
- MigrateStart args = {
- .hide_stderr = true,
- };
- if (migrate_start(&from, &to, "defer", &args)) {
+ args->start.hide_stderr = true;
+
+ if (migrate_start(&from, &to, "defer", &args->start)) {
return;
}
@@ -980,7 +953,7 @@ static void dirtylimit_stop_vm(QTestState *vm)
unlink(path);
}
-static void test_vcpu_dirty_limit(void)
+static void test_vcpu_dirty_limit(char *name, MigrateCommon *args)
{
QTestState *vm;
int64_t origin_rate;
@@ -1107,7 +1080,7 @@ static void migrate_dirty_limit_wait_showup(QTestState *from,
* And see if dirty limit migration works correctly.
* This test case involves many passes, so it runs in slow mode only.
*/
-static void test_dirty_limit(void)
+static void test_dirty_limit(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
QTestState *from, *to;
@@ -1128,17 +1101,15 @@ static void test_dirty_limit(void)
*/
const int64_t expected_threshold = max_bandwidth * downtime_limit / 1000;
int max_try_count = 10;
- MigrateCommon args = {
- .start = {
- .hide_stderr = true,
- .use_dirty_ring = true,
- },
- .listen_uri = uri,
- .connect_uri = uri,
- };
+
+ args->start.hide_stderr = true;
+ args->start.use_dirty_ring = true;
+
+ args->listen_uri = uri;
+ args->connect_uri = uri;
/* Start src, dst vm */
- if (migrate_start(&from, &to, args.listen_uri, &args.start)) {
+ if (migrate_start(&from, &to, args->listen_uri, &args->start)) {
return;
}
@@ -1146,7 +1117,7 @@ static void test_dirty_limit(void)
migrate_dirty_limit_wait_showup(from, dirtylimit_period, dirtylimit_value);
/* Start migrate */
- migrate_qmp(from, to, args.connect_uri, NULL, "{}");
+ migrate_qmp(from, to, args->connect_uri, NULL, "{}");
/* Wait for dirty limit throttle begin */
throttle_us_per_full = 0;
@@ -1179,22 +1150,19 @@ static void test_dirty_limit(void)
/* Assert dirty limit is not in service */
g_assert_cmpint(throttle_us_per_full, ==, 0);
- args = (MigrateCommon) {
- .start = {
- .only_target = true,
- .use_dirty_ring = true,
- },
- .listen_uri = uri,
- .connect_uri = uri,
- };
+ args->listen_uri = uri;
+ args->connect_uri = uri;
+
+ args->start.only_target = true;
+ args->start.use_dirty_ring = true;
/* Restart dst vm, src vm already show up so we needn't wait anymore */
- if (migrate_start(&from, &to, args.listen_uri, &args.start)) {
+ if (migrate_start(&from, &to, args->listen_uri, &args->start)) {
return;
}
/* Start migrate */
- migrate_qmp(from, to, args.connect_uri, NULL, "{}");
+ migrate_qmp(from, to, args->connect_uri, NULL, "{}");
/* Wait for dirty limit throttle begin */
throttle_us_per_full = 0;
diff --git a/tests/qtest/migration/tls-tests.c b/tests/qtest/migration/tls-tests.c
index e0e8a7335c..6a20c65104 100644
--- a/tests/qtest/migration/tls-tests.c
+++ b/tests/qtest/migration/tls-tests.c
@@ -362,149 +362,128 @@ migrate_hook_end_tls_x509(QTestState *from,
}
#endif /* CONFIG_TASN1 */
-static void test_postcopy_tls_psk(void)
+static void test_postcopy_tls_psk(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .start_hook = migrate_hook_start_tls_psk_match,
- .end_hook = migrate_hook_end_tls_psk,
- };
+ args->start_hook = migrate_hook_start_tls_psk_match;
+ args->end_hook = migrate_hook_end_tls_psk;
- test_postcopy_common(&args);
+ test_postcopy_common(args);
}
-static void test_postcopy_preempt_tls_psk(void)
+static void test_postcopy_preempt_tls_psk(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .start_hook = migrate_hook_start_tls_psk_match,
- .end_hook = migrate_hook_end_tls_psk,
- .start = {
- .caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true,
- },
- };
+ args->start_hook = migrate_hook_start_tls_psk_match;
+ args->end_hook = migrate_hook_end_tls_psk;
- test_postcopy_common(&args);
+ args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true;
+
+ test_postcopy_common(args);
}
-static void test_postcopy_recovery_tls_psk(void)
+static void test_postcopy_recovery_tls_psk(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .start_hook = migrate_hook_start_tls_psk_match,
- .end_hook = migrate_hook_end_tls_psk,
- };
+ args->start_hook = migrate_hook_start_tls_psk_match;
+ args->end_hook = migrate_hook_end_tls_psk;
- test_postcopy_recovery_common(&args);
+ test_postcopy_recovery_common(args);
}
-static void test_multifd_postcopy_recovery_tls_psk(void)
+static void test_multifd_postcopy_recovery_tls_psk(char *name,
+ MigrateCommon *args)
{
- MigrateCommon args = {
- .start_hook = migrate_hook_start_tls_psk_match,
- .end_hook = migrate_hook_end_tls_psk,
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- };
+ args->start_hook = migrate_hook_start_tls_psk_match;
+ args->end_hook = migrate_hook_end_tls_psk;
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
- test_postcopy_recovery_common(&args);
+ test_postcopy_recovery_common(args);
}
/* This contains preempt+recovery+tls test altogether */
-static void test_postcopy_preempt_all(void)
-{
- MigrateCommon args = {
- .start_hook = migrate_hook_start_tls_psk_match,
- .end_hook = migrate_hook_end_tls_psk,
- .start = {
- .caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true,
- },
- };
+static void test_postcopy_preempt_all(char *name, MigrateCommon *args)
+{
+ args->start_hook = migrate_hook_start_tls_psk_match;
+ args->end_hook = migrate_hook_end_tls_psk;
- test_postcopy_recovery_common(&args);
+ args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true;
+
+ test_postcopy_recovery_common(args);
}
-static void test_multifd_postcopy_preempt_recovery_tls_psk(void)
+static void test_multifd_postcopy_preempt_recovery_tls_psk(char *name,
+ MigrateCommon *args)
{
- MigrateCommon args = {
- .start_hook = migrate_hook_start_tls_psk_match,
- .end_hook = migrate_hook_end_tls_psk,
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- .caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true,
- },
- };
+ args->start_hook = migrate_hook_start_tls_psk_match;
+ args->end_hook = migrate_hook_end_tls_psk;
- test_postcopy_recovery_common(&args);
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+ args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_PREEMPT] = true;
+
+ test_postcopy_recovery_common(args);
}
-static void test_precopy_unix_tls_psk(void)
+static void test_precopy_unix_tls_psk(char *name, MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
- MigrateCommon args = {
- .connect_uri = uri,
- .listen_uri = uri,
- .start_hook = migrate_hook_start_tls_psk_match,
- .end_hook = migrate_hook_end_tls_psk,
- };
- test_precopy_common(&args);
+ args->connect_uri = uri;
+ args->listen_uri = uri;
+ args->start_hook = migrate_hook_start_tls_psk_match;
+ args->end_hook = migrate_hook_end_tls_psk;
+
+ test_precopy_common(args);
}
#ifdef CONFIG_TASN1
-static void test_precopy_unix_tls_x509_default_host(void)
+static void test_precopy_unix_tls_x509_default_host(char *name,
+ MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
- MigrateCommon args = {
- .start = {
- .hide_stderr = true,
- },
- .connect_uri = uri,
- .listen_uri = uri,
- .start_hook = migrate_hook_start_tls_x509_default_host,
- .end_hook = migrate_hook_end_tls_x509,
- .result = MIG_TEST_FAIL_DEST_QUIT_ERR,
- };
- test_precopy_common(&args);
+ args->connect_uri = uri;
+ args->listen_uri = uri;
+ args->start_hook = migrate_hook_start_tls_x509_default_host;
+ args->end_hook = migrate_hook_end_tls_x509;
+ args->result = MIG_TEST_FAIL_DEST_QUIT_ERR;
+
+ args->start.hide_stderr = true;
+
+ test_precopy_common(args);
}
-static void test_precopy_unix_tls_x509_override_host(void)
+static void test_precopy_unix_tls_x509_override_host(char *name,
+ MigrateCommon *args)
{
g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
- MigrateCommon args = {
- .connect_uri = uri,
- .listen_uri = uri,
- .start_hook = migrate_hook_start_tls_x509_override_host,
- .end_hook = migrate_hook_end_tls_x509,
- };
- test_precopy_common(&args);
+ args->connect_uri = uri;
+ args->listen_uri = uri;
+ args->start_hook = migrate_hook_start_tls_x509_override_host;
+ args->end_hook = migrate_hook_end_tls_x509;
+
+ test_precopy_common(args);
}
#endif /* CONFIG_TASN1 */
-static void test_precopy_tcp_tls_psk_match(void)
+static void test_precopy_tcp_tls_psk_match(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "tcp:127.0.0.1:0",
- .start_hook = migrate_hook_start_tls_psk_match,
- .end_hook = migrate_hook_end_tls_psk,
- };
+ args->listen_uri = "tcp:127.0.0.1:0";
+ args->start_hook = migrate_hook_start_tls_psk_match;
+ args->end_hook = migrate_hook_end_tls_psk;
- test_precopy_common(&args);
+ test_precopy_common(args);
}
-static void test_precopy_tcp_tls_psk_mismatch(void)
+static void test_precopy_tcp_tls_psk_mismatch(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .start = {
- .hide_stderr = true,
- },
- .listen_uri = "tcp:127.0.0.1:0",
- .start_hook = migrate_hook_start_tls_psk_mismatch,
- .end_hook = migrate_hook_end_tls_psk,
- .result = MIG_TEST_FAIL,
- };
+ args->listen_uri = "tcp:127.0.0.1:0";
+ args->start_hook = migrate_hook_start_tls_psk_mismatch;
+ args->end_hook = migrate_hook_end_tls_psk;
+ args->result = MIG_TEST_FAIL;
+
+ args->start.hide_stderr = true;
- test_precopy_common(&args);
+ test_precopy_common(args);
}
static void *migrate_hook_start_no_tls(QTestState *from, QTestState *to)
@@ -518,15 +497,13 @@ static void *migrate_hook_start_no_tls(QTestState *from, QTestState *to)
return data;
}
-static void test_precopy_tcp_no_tls(void)
+static void test_precopy_tcp_no_tls(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "tcp:127.0.0.1:0",
- .start_hook = migrate_hook_start_no_tls,
- .end_hook = migrate_hook_end_tls_psk,
- };
+ args->listen_uri = "tcp:127.0.0.1:0";
+ args->start_hook = migrate_hook_start_no_tls;
+ args->end_hook = migrate_hook_end_tls_psk;
- test_precopy_common(&args);
+ test_precopy_common(args);
}
static void *
@@ -545,107 +522,96 @@ migrate_hook_start_tls_x509_no_host(QTestState *from, QTestState *to)
return data;
}
-static void test_precopy_tcp_tls_no_hostname(void)
+static void test_precopy_tcp_tls_no_hostname(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "tcp:127.0.0.1:0",
- .start_hook = migrate_hook_start_tls_x509_no_host,
- .end_hook = migrate_hook_end_tls_x509,
- .result = MIG_TEST_FAIL_DEST_QUIT_ERR,
- .start.hide_stderr = true,
- };
+ args->listen_uri = "tcp:127.0.0.1:0";
+ args->start_hook = migrate_hook_start_tls_x509_no_host;
+ args->end_hook = migrate_hook_end_tls_x509;
+ args->result = MIG_TEST_FAIL_DEST_QUIT_ERR;
+
+ args->start.hide_stderr = true;
- test_precopy_common(&args);
+ test_precopy_common(args);
}
#ifdef CONFIG_TASN1
-static void test_precopy_tcp_tls_x509_default_host(void)
+static void test_precopy_tcp_tls_x509_default_host(char *name,
+ MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "tcp:127.0.0.1:0",
- .start_hook = migrate_hook_start_tls_x509_default_host,
- .end_hook = migrate_hook_end_tls_x509,
- };
+ args->listen_uri = "tcp:127.0.0.1:0";
+ args->start_hook = migrate_hook_start_tls_x509_default_host;
+ args->end_hook = migrate_hook_end_tls_x509;
- test_precopy_common(&args);
+ test_precopy_common(args);
}
-static void test_precopy_tcp_tls_x509_override_host(void)
+static void test_precopy_tcp_tls_x509_override_host(char *name,
+ MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "tcp:127.0.0.1:0",
- .start_hook = migrate_hook_start_tls_x509_override_host,
- .end_hook = migrate_hook_end_tls_x509,
- };
+ args->listen_uri = "tcp:127.0.0.1:0";
+ args->start_hook = migrate_hook_start_tls_x509_override_host;
+ args->end_hook = migrate_hook_end_tls_x509;
- test_precopy_common(&args);
+ test_precopy_common(args);
}
-static void test_precopy_tcp_tls_x509_mismatch_host(void)
+static void test_precopy_tcp_tls_x509_mismatch_host(char *name,
+ MigrateCommon *args)
{
- MigrateCommon args = {
- .start = {
- .hide_stderr = true,
- },
- .listen_uri = "tcp:127.0.0.1:0",
- .start_hook = migrate_hook_start_tls_x509_mismatch_host,
- .end_hook = migrate_hook_end_tls_x509,
- .result = MIG_TEST_FAIL_DEST_QUIT_ERR,
- };
+ args->listen_uri = "tcp:127.0.0.1:0";
+ args->start_hook = migrate_hook_start_tls_x509_mismatch_host;
+ args->end_hook = migrate_hook_end_tls_x509;
+ args->result = MIG_TEST_FAIL_DEST_QUIT_ERR;
+
+ args->start.hide_stderr = true;
- test_precopy_common(&args);
+ test_precopy_common(args);
}
-static void test_precopy_tcp_tls_x509_friendly_client(void)
+static void test_precopy_tcp_tls_x509_friendly_client(char *name,
+ MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "tcp:127.0.0.1:0",
- .start_hook = migrate_hook_start_tls_x509_friendly_client,
- .end_hook = migrate_hook_end_tls_x509,
- };
+ args->listen_uri = "tcp:127.0.0.1:0";
+ args->start_hook = migrate_hook_start_tls_x509_friendly_client;
+ args->end_hook = migrate_hook_end_tls_x509;
- test_precopy_common(&args);
+ test_precopy_common(args);
}
-static void test_precopy_tcp_tls_x509_hostile_client(void)
+static void test_precopy_tcp_tls_x509_hostile_client(char *name,
+ MigrateCommon *args)
{
- MigrateCommon args = {
- .start = {
- .hide_stderr = true,
- },
- .listen_uri = "tcp:127.0.0.1:0",
- .start_hook = migrate_hook_start_tls_x509_hostile_client,
- .end_hook = migrate_hook_end_tls_x509,
- .result = MIG_TEST_FAIL,
- };
+ args->listen_uri = "tcp:127.0.0.1:0";
+ args->start_hook = migrate_hook_start_tls_x509_hostile_client;
+ args->end_hook = migrate_hook_end_tls_x509;
+ args->result = MIG_TEST_FAIL;
+
+ args->start.hide_stderr = true;
- test_precopy_common(&args);
+ test_precopy_common(args);
}
-static void test_precopy_tcp_tls_x509_allow_anon_client(void)
+static void test_precopy_tcp_tls_x509_allow_anon_client(char *name,
+ MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "tcp:127.0.0.1:0",
- .start_hook = migrate_hook_start_tls_x509_allow_anon_client,
- .end_hook = migrate_hook_end_tls_x509,
- };
+ args->listen_uri = "tcp:127.0.0.1:0";
+ args->start_hook = migrate_hook_start_tls_x509_allow_anon_client;
+ args->end_hook = migrate_hook_end_tls_x509;
- test_precopy_common(&args);
+ test_precopy_common(args);
}
-static void test_precopy_tcp_tls_x509_reject_anon_client(void)
+static void test_precopy_tcp_tls_x509_reject_anon_client(char *name,
+ MigrateCommon *args)
{
- MigrateCommon args = {
- .start = {
- .hide_stderr = true,
- },
- .listen_uri = "tcp:127.0.0.1:0",
- .start_hook = migrate_hook_start_tls_x509_reject_anon_client,
- .end_hook = migrate_hook_end_tls_x509,
- .result = MIG_TEST_FAIL,
- };
+ args->listen_uri = "tcp:127.0.0.1:0";
+ args->start_hook = migrate_hook_start_tls_x509_reject_anon_client;
+ args->end_hook = migrate_hook_end_tls_x509;
+ args->result = MIG_TEST_FAIL;
- test_precopy_common(&args);
+ args->start.hide_stderr = true;
+
+ test_precopy_common(args);
}
#endif /* CONFIG_TASN1 */
@@ -707,77 +673,70 @@ migrate_hook_start_multifd_tls_x509_reject_anon_client(QTestState *from,
}
#endif /* CONFIG_TASN1 */
-static void test_multifd_tcp_tls_psk_match(void)
+static void test_multifd_tcp_tls_psk_match(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_multifd_tcp_tls_psk_match,
- .end_hook = migrate_hook_end_tls_psk,
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- };
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_multifd_tcp_tls_psk_match;
+ args->end_hook = migrate_hook_end_tls_psk;
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
-static void test_multifd_tcp_tls_psk_mismatch(void)
+static void test_multifd_tcp_tls_psk_mismatch(char *name, MigrateCommon *args)
{
- MigrateCommon args = {
- .start = {
- .hide_stderr = true,
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_multifd_tcp_tls_psk_mismatch,
- .end_hook = migrate_hook_end_tls_psk,
- .result = MIG_TEST_FAIL,
- };
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_multifd_tcp_tls_psk_mismatch;
+ args->end_hook = migrate_hook_end_tls_psk;
+ args->result = MIG_TEST_FAIL;
+
+ args->start.hide_stderr = true;
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
-static void test_multifd_postcopy_tcp_tls_psk_match(void)
+static void test_multifd_postcopy_tcp_tls_psk_match(char *name,
+ MigrateCommon *args)
{
- MigrateCommon args = {
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- .caps[MIGRATION_CAPABILITY_POSTCOPY_RAM] = true,
- },
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_multifd_tcp_tls_psk_match,
- .end_hook = migrate_hook_end_tls_psk,
- };
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_multifd_tcp_tls_psk_match;
+ args->end_hook = migrate_hook_end_tls_psk;
- test_precopy_common(&args);
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+ args->start.caps[MIGRATION_CAPABILITY_POSTCOPY_RAM] = true;
+
+ test_precopy_common(args);
}
#ifdef CONFIG_TASN1
-static void test_multifd_tcp_tls_x509_default_host(void)
-{
- MigrateCommon args = {
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_multifd_tls_x509_default_host,
- .end_hook = migrate_hook_end_tls_x509,
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- };
- test_precopy_common(&args);
+static void test_multifd_tcp_tls_x509_default_host(char *name,
+ MigrateCommon *args)
+{
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_multifd_tls_x509_default_host;
+ args->end_hook = migrate_hook_end_tls_x509;
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
-static void test_multifd_tcp_tls_x509_override_host(void)
+static void test_multifd_tcp_tls_x509_override_host(char *name,
+ MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_multifd_tls_x509_override_host,
- .end_hook = migrate_hook_end_tls_x509,
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- };
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_multifd_tls_x509_override_host;
+ args->end_hook = migrate_hook_end_tls_x509;
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
-static void test_multifd_tcp_tls_x509_mismatch_host(void)
+static void test_multifd_tcp_tls_x509_mismatch_host(char *name,
+ MigrateCommon *args)
{
/*
* This has different behaviour to the non-multifd case.
@@ -792,45 +751,41 @@ static void test_multifd_tcp_tls_x509_mismatch_host(void)
* to load migration state, and thus just aborts the migration
* without exiting.
*/
- MigrateCommon args = {
- .start = {
- .hide_stderr = true,
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_multifd_tls_x509_mismatch_host,
- .end_hook = migrate_hook_end_tls_x509,
- .result = MIG_TEST_FAIL,
- };
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_multifd_tls_x509_mismatch_host;
+ args->end_hook = migrate_hook_end_tls_x509;
+ args->result = MIG_TEST_FAIL;
+
+ args->start.hide_stderr = true;
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
-static void test_multifd_tcp_tls_x509_allow_anon_client(void)
+static void test_multifd_tcp_tls_x509_allow_anon_client(char *name,
+ MigrateCommon *args)
{
- MigrateCommon args = {
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_multifd_tls_x509_allow_anon_client,
- .end_hook = migrate_hook_end_tls_x509,
- .start = {
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- };
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_multifd_tls_x509_allow_anon_client;
+ args->end_hook = migrate_hook_end_tls_x509;
+
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
-static void test_multifd_tcp_tls_x509_reject_anon_client(void)
+static void test_multifd_tcp_tls_x509_reject_anon_client(char *name,
+ MigrateCommon *args)
{
- MigrateCommon args = {
- .start = {
- .hide_stderr = true,
- .caps[MIGRATION_CAPABILITY_MULTIFD] = true,
- },
- .listen_uri = "defer",
- .start_hook = migrate_hook_start_multifd_tls_x509_reject_anon_client,
- .end_hook = migrate_hook_end_tls_x509,
- .result = MIG_TEST_FAIL,
- };
- test_precopy_common(&args);
+ args->listen_uri = "defer";
+ args->start_hook = migrate_hook_start_multifd_tls_x509_reject_anon_client;
+ args->end_hook = migrate_hook_end_tls_x509;
+ args->result = MIG_TEST_FAIL;
+
+ args->start.hide_stderr = true;
+ args->start.caps[MIGRATION_CAPABILITY_MULTIFD] = true;
+
+ test_precopy_common(args);
}
#endif /* CONFIG_TASN1 */
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 29/31] tests/qtest/migration: Pass MigrateStart into cancel tests
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (27 preceding siblings ...)
2025-12-23 14:29 ` [PULL 28/31] tests/qtest/migration: Pass MigrateCommon into test functions Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 30/31] migration: merge fragmented clear_dirty ioctls Peter Xu
` (2 subsequent siblings)
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu
From: Fabiano Rosas <farosas@suse.de>
Pass the "args" parameter to the cancel tests so they can access the
config object which will be part of this struct.
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251215220041.12657-27-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qtest/migration/precopy-tests.c | 25 +++++++++++++++----------
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c
index 086d06a31c..aca7ed51ef 100644
--- a/tests/qtest/migration/precopy-tests.c
+++ b/tests/qtest/migration/precopy-tests.c
@@ -642,7 +642,8 @@ static void test_multifd_postcopy_tcp_cancel(char *name, MigrateCommon *args)
}
static void test_cancel_src_after_failed(QTestState *from, QTestState *to,
- const char *uri, const char *phase)
+ const char *uri, const char *phase,
+ MigrateStart *args)
{
/*
* No migrate_incoming_qmp() at the start to force source into
@@ -669,7 +670,8 @@ static void test_cancel_src_after_failed(QTestState *from, QTestState *to,
}
static void test_cancel_src_after_cancelled(QTestState *from, QTestState *to,
- const char *uri, const char *phase)
+ const char *uri, const char *phase,
+ MigrateStart *args)
{
migrate_incoming_qmp(to, uri, NULL, "{ 'exit-on-error': false }");
@@ -693,7 +695,8 @@ static void test_cancel_src_after_cancelled(QTestState *from, QTestState *to,
}
static void test_cancel_src_after_complete(QTestState *from, QTestState *to,
- const char *uri, const char *phase)
+ const char *uri, const char *phase,
+ MigrateStart *args)
{
migrate_incoming_qmp(to, uri, NULL, "{ 'exit-on-error': false }");
@@ -714,7 +717,8 @@ static void test_cancel_src_after_complete(QTestState *from, QTestState *to,
}
static void test_cancel_src_after_none(QTestState *from, QTestState *to,
- const char *uri, const char *phase)
+ const char *uri, const char *phase,
+ MigrateStart *args)
{
/*
* Test that cancelling without a migration happening does not
@@ -735,7 +739,8 @@ static void test_cancel_src_after_none(QTestState *from, QTestState *to,
}
static void test_cancel_src_pre_switchover(QTestState *from, QTestState *to,
- const char *uri, const char *phase)
+ const char *uri, const char *phase,
+ MigrateStart *args)
{
migrate_set_capability(from, "pause-before-switchover", true);
migrate_set_capability(to, "pause-before-switchover", true);
@@ -775,20 +780,20 @@ static void test_cancel_src_after_status(char *test_path, MigrateCommon *args)
if (g_str_equal(phase, "cancelling") ||
g_str_equal(phase, "cancelled")) {
- test_cancel_src_after_cancelled(from, to, uri, phase);
+ test_cancel_src_after_cancelled(from, to, uri, phase, &args->start);
} else if (g_str_equal(phase, "completed")) {
- test_cancel_src_after_complete(from, to, uri, phase);
+ test_cancel_src_after_complete(from, to, uri, phase, &args->start);
} else if (g_str_equal(phase, "failed")) {
- test_cancel_src_after_failed(from, to, uri, phase);
+ test_cancel_src_after_failed(from, to, uri, phase, &args->start);
} else if (g_str_equal(phase, "none")) {
- test_cancel_src_after_none(from, to, uri, phase);
+ test_cancel_src_after_none(from, to, uri, phase, &args->start);
} else {
/* any state that comes before pre-switchover */
- test_cancel_src_pre_switchover(from, to, uri, phase);
+ test_cancel_src_pre_switchover(from, to, uri, phase, &args->start);
}
migrate_end(from, to, false);
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 30/31] migration: merge fragmented clear_dirty ioctls
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (28 preceding siblings ...)
2025-12-23 14:29 ` [PULL 29/31] tests/qtest/migration: Pass MigrateStart into cancel tests Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-23 14:29 ` [PULL 31/31] MAINTAINERS: remove David from "Memory API" section Peter Xu
2025-12-28 22:08 ` [PULL 00/31] Next patches Richard Henderson
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Xu, Chuang Xu
From: Chuang Xu <xuchuangxclwt@bytedance.com>
In our long-term experience in Bytedance, we've found that under
the same load, live migration of larger VMs with more devices is
often more difficult to converge (requiring a larger downtime limit).
Through some testing and calculations, we conclude that bitmap sync time
affects the calculation of live migration bandwidth.
When the addresses processed are not aligned, a large number of
clear_dirty ioctl occur (e.g. a 4MB misaligned memory can generate
2048 clear_dirty ioctls from two different memory_listener),
which increases the time required for bitmap_sync and makes it
more difficult for dirty pages to converge.
For a 64C256G vm with 8 vhost-user-net(32 queue per nic) and
16 vhost-user-blk(4 queue per blk), the sync time is as high as *73ms*
(tested with 10GBps dirty rate, the sync time increases as the dirty
page rate increases), Here are each part of the sync time:
- sync from kvm to ram_list: 2.5ms
- vhost_log_sync:3ms
- sync aligned memory from ram_list to RAMBlock: 5ms
- sync misaligned memory from ram_list to RAMBlock: 61ms
Attempt to merge those fragmented clear_dirty ioctls, then syncing
misaligned memory from ram_list to RAMBlock takes only about 1ms,
and the total sync time is only *12ms*.
Signed-off-by: Chuang Xu <xuchuangxclwt@bytedance.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20251218114220.83354-1-xuchuangxclwt@bytedance.com
[peterx: drop var "offset" in physical_memory_sync_dirty_bitmap]
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/system/physmem.h | 7 +++---
accel/tcg/cputlb.c | 5 +++--
migration/ram.c | 19 +++++-----------
system/memory.c | 2 +-
system/physmem.c | 48 +++++++++++++++++++++++++++-------------
5 files changed, 46 insertions(+), 35 deletions(-)
diff --git a/include/system/physmem.h b/include/system/physmem.h
index 879f6eae38..a59724ef10 100644
--- a/include/system/physmem.h
+++ b/include/system/physmem.h
@@ -39,9 +39,10 @@ uint64_t physical_memory_set_dirty_lebitmap(unsigned long *bitmap,
void physical_memory_dirty_bits_cleared(ram_addr_t start, ram_addr_t length);
-bool physical_memory_test_and_clear_dirty(ram_addr_t start,
- ram_addr_t length,
- unsigned client);
+uint64_t physical_memory_test_and_clear_dirty(ram_addr_t start,
+ ram_addr_t length,
+ unsigned client,
+ unsigned long *bmap);
DirtyBitmapSnapshot *
physical_memory_snapshot_and_clear_dirty(MemoryRegion *mr, hwaddr offset,
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index fd1606c856..c8827c8b0d 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -857,8 +857,9 @@ void tlb_flush_page_bits_by_mmuidx_all_cpus_synced(CPUState *src_cpu,
void tlb_protect_code(ram_addr_t ram_addr)
{
physical_memory_test_and_clear_dirty(ram_addr & TARGET_PAGE_MASK,
- TARGET_PAGE_SIZE,
- DIRTY_MEMORY_CODE);
+ TARGET_PAGE_SIZE,
+ DIRTY_MEMORY_CODE,
+ NULL);
}
/* update the TLB so that writes in physical page 'phys_addr' are no longer
diff --git a/migration/ram.c b/migration/ram.c
index df7e154877..c403fd73a6 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -941,7 +941,6 @@ static uint64_t physical_memory_sync_dirty_bitmap(RAMBlock *rb,
ram_addr_t start,
ram_addr_t length)
{
- ram_addr_t addr;
unsigned long word = BIT_WORD((start + rb->offset) >> TARGET_PAGE_BITS);
uint64_t num_dirty = 0;
unsigned long *dest = rb->bmap;
@@ -993,19 +992,11 @@ static uint64_t physical_memory_sync_dirty_bitmap(RAMBlock *rb,
memory_region_clear_dirty_bitmap(rb->mr, start, length);
}
} else {
- ram_addr_t offset = rb->offset;
-
- for (addr = 0; addr < length; addr += TARGET_PAGE_SIZE) {
- if (physical_memory_test_and_clear_dirty(
- start + addr + offset,
- TARGET_PAGE_SIZE,
- DIRTY_MEMORY_MIGRATION)) {
- long k = (start + addr) >> TARGET_PAGE_BITS;
- if (!test_and_set_bit(k, dest)) {
- num_dirty++;
- }
- }
- }
+ num_dirty = physical_memory_test_and_clear_dirty(
+ start + rb->offset,
+ length,
+ DIRTY_MEMORY_MIGRATION,
+ dest);
}
return num_dirty;
diff --git a/system/memory.c b/system/memory.c
index 8b84661ae3..666364392d 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -2424,7 +2424,7 @@ void memory_region_reset_dirty(MemoryRegion *mr, hwaddr addr,
{
assert(mr->ram_block);
physical_memory_test_and_clear_dirty(
- memory_region_get_ram_addr(mr) + addr, size, client);
+ memory_region_get_ram_addr(mr) + addr, size, client, NULL);
}
int memory_region_get_fd(MemoryRegion *mr)
diff --git a/system/physmem.c b/system/physmem.c
index c9869e4049..26bf30af17 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -1089,19 +1089,30 @@ void physical_memory_set_dirty_range(ram_addr_t start, ram_addr_t length,
}
}
-/* Note: start and end must be within the same ram block. */
-bool physical_memory_test_and_clear_dirty(ram_addr_t start,
+/*
+ * Note: start and end must be within the same ram block.
+ *
+ * @bmap usage:
+ * - When @bmap is provided, set bits for dirty pages, but
+ * only count those pages if the bit wasn't already set in @bmap.
+ * - When @bmap is NULL, count all dirty pages in the range.
+ *
+ * @return:
+ * - Number of dirty guest pages found within [start, start + length).
+ */
+uint64_t physical_memory_test_and_clear_dirty(ram_addr_t start,
ram_addr_t length,
- unsigned client)
+ unsigned client,
+ unsigned long *bmap)
{
DirtyMemoryBlocks *blocks;
unsigned long end, page, start_page;
- bool dirty = false;
+ uint64_t num_dirty = 0;
RAMBlock *ramblock;
uint64_t mr_offset, mr_size;
if (length == 0) {
- return false;
+ return 0;
}
end = TARGET_PAGE_ALIGN(start + length) >> TARGET_PAGE_BITS;
@@ -1118,12 +1129,19 @@ bool physical_memory_test_and_clear_dirty(ram_addr_t start,
while (page < end) {
unsigned long idx = page / DIRTY_MEMORY_BLOCK_SIZE;
unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE;
- unsigned long num = MIN(end - page,
- DIRTY_MEMORY_BLOCK_SIZE - offset);
- dirty |= bitmap_test_and_clear_atomic(blocks->blocks[idx],
- offset, num);
- page += num;
+ if (bitmap_test_and_clear_atomic(blocks->blocks[idx], offset, 1)) {
+ if (bmap) {
+ unsigned long k = page - (ramblock->offset >> TARGET_PAGE_BITS);
+ if (!test_and_set_bit(k, bmap)) {
+ num_dirty++;
+ }
+ } else {
+ num_dirty++;
+ }
+ }
+
+ page++;
}
mr_offset = (ram_addr_t)(start_page << TARGET_PAGE_BITS) - ramblock->offset;
@@ -1131,18 +1149,18 @@ bool physical_memory_test_and_clear_dirty(ram_addr_t start,
memory_region_clear_dirty_bitmap(ramblock->mr, mr_offset, mr_size);
}
- if (dirty) {
+ if (num_dirty) {
physical_memory_dirty_bits_cleared(start, length);
}
- return dirty;
+ return num_dirty;
}
static void physical_memory_clear_dirty_range(ram_addr_t addr, ram_addr_t length)
{
- physical_memory_test_and_clear_dirty(addr, length, DIRTY_MEMORY_MIGRATION);
- physical_memory_test_and_clear_dirty(addr, length, DIRTY_MEMORY_VGA);
- physical_memory_test_and_clear_dirty(addr, length, DIRTY_MEMORY_CODE);
+ physical_memory_test_and_clear_dirty(addr, length, DIRTY_MEMORY_MIGRATION, NULL);
+ physical_memory_test_and_clear_dirty(addr, length, DIRTY_MEMORY_VGA, NULL);
+ physical_memory_test_and_clear_dirty(addr, length, DIRTY_MEMORY_CODE, NULL);
}
DirtyBitmapSnapshot *physical_memory_snapshot_and_clear_dirty
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PULL 31/31] MAINTAINERS: remove David from "Memory API" section
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (29 preceding siblings ...)
2025-12-23 14:29 ` [PULL 30/31] migration: merge fragmented clear_dirty ioctls Peter Xu
@ 2025-12-23 14:29 ` Peter Xu
2025-12-28 22:08 ` [PULL 00/31] Next patches Richard Henderson
31 siblings, 0 replies; 39+ messages in thread
From: Peter Xu @ 2025-12-23 14:29 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Xu, David Hildenbrand (Red Hat),
Paolo Bonzini, Philippe Mathieu-Daudé
From: "David Hildenbrand (Red Hat)" <david@kernel.org>
I don't have a lot of capacity to do any maintanance (or even review) of
"Memory API" lately, so remove myself. Fortunately we still do have two
other maintainers and one reviewer :)
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: David Hildenbrand (Red Hat) <david@kernel.org>
Link: https://lore.kernel.org/r/20251222141438.409218-1-david@kernel.org
Signed-off-by: Peter Xu <peterx@redhat.com>
---
MAINTAINERS | 1 -
1 file changed, 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index dc6235e174..2dc3625a68 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3251,7 +3251,6 @@ T: git https://gitlab.com/stsquad/qemu gdbstub/next
Memory API
M: Paolo Bonzini <pbonzini@redhat.com>
M: Peter Xu <peterx@redhat.com>
-M: David Hildenbrand <david@kernel.org>
R: Philippe Mathieu-Daudé <philmd@linaro.org>
S: Supported
F: include/system/ioport.h
--
2.50.1
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PULL 00/31] Next patches
2025-12-23 14:29 [PULL 00/31] Next patches Peter Xu
` (30 preceding siblings ...)
2025-12-23 14:29 ` [PULL 31/31] MAINTAINERS: remove David from "Memory API" section Peter Xu
@ 2025-12-28 22:08 ` Richard Henderson
31 siblings, 0 replies; 39+ messages in thread
From: Richard Henderson @ 2025-12-28 22:08 UTC (permalink / raw)
To: Peter Xu, qemu-devel; +Cc: Fabiano Rosas
On 12/24/25 01:29, Peter Xu wrote:
> The following changes since commit 8dd5bceb2f9cc58481e9d22355a8d998220896de:
>
> Open 11.0 development tree (2025-12-23 14:45:38 +1100)
>
> are available in the Git repository at:
>
> https://gitlab.com/peterx/qemu.git tags/next-pull-request
>
> for you to fetch changes up to bcb411a005fdf39b76e99c14f3618c7b70f7774d:
>
> MAINTAINERS: remove David from "Memory API" section (2025-12-23 09:27:02 -0500)
>
> ----------------------------------------------------------------
> memory + migration pull
>
> - Pawel's misc fixes to mapped-ram when x-ignore-share is enabled
> - Peter's series to cleanup migration error reporting
> - Peter's added debug property for x-ignore-shared
> - Part of Fabiano's series on unify capabilities and parameters
> - Chuang's log_clear optimization on unaligned ramblocks
> - Maintainer file update from Ben (CPR++) and David (MemoryAPI-)
Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/11.0 as appropriate.
r~
^ permalink raw reply [flat|nested] 39+ messages in thread