Reviewed-by: Fabiano Rosas <farosas(a)suse.de&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231018112827.1325-2-quintela(a)redhat.com&gt; --- docs/devel/migration.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst index c3e1400c0c..4d6a98ae58 100644 --- a/docs/devel/migration.rst +++ b/docs/devel/migration.rst @@ -28,6 +28,8 @@ the guest to be stopped. Typically the time that the guest is unresponsive during live migration is the low hundred of milliseconds (notice that this depends on a lot of things). +.. contents:: + Transports ========== -- 2.41.0

Sometimes devices have different features depending of things outside of qemu. For instance the kernel. Document how to handle that cases. Acked-by: Peter Xu <peterx(a)redhat.com&gt; Reviewed-by: Fabiano Rosas <farosas(a)suse.de&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231018112827.1325-4-quintela(a)redhat.com&gt; --- docs/devel/migration.rst | 97 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst index 6fe275b1ec..974505e4a7 100644 --- a/docs/devel/migration.rst +++ b/docs/devel/migration.rst @@ -1138,3 +1138,100 @@ machine types to have the right value:: + { "virtio-blk-device", "num-queues", "1"}, ... }; + +A device with diferent features on both sides +--------------------------------------------- + +Let's assume that we are using the same QEMU binary on both sides, +just to make the things easier. But we have a device that has +different features on both sides of the migration. That can be +because the devices are different, because the kernel driver of both +devices have different features, whatever. + +How can we get this to work with migration. The way to do that is +"theoretically" easy. You have to get the features that the device +has in the source of the migration. The features that the device has +on the target of the migration, you get the intersection of the +features of both sides, and that is the way that you should launch +QEMU. + +Notice that this is not completely related to QEMU. The most +important thing here is that this should be handled by the managing +application that launches QEMU. If QEMU is configured correctly, the +migration will succeed. + +That said, actually doing it is complicated. Almost all devices are +bad at being able to be launched with only some features enabled. +With one big exception: cpus. + +You can read the documentation for QEMU x86 cpu models here: + +https://qemu-project.gitlab.io/qemu/system/qemu-cpu-models.html + +See when they talk about migration they recommend that one chooses the +newest cpu model that is supported for all cpus. + +Let's say that we have: + +Host A: + +Device X has the feature Y + +Host B: + +Device X has not the feature Y + +If we try to migrate without any care from host A to host B, it will +fail because when migration tries to load the feature Y on +destination, it will find that the hardware is not there. + +Doing this would be the equivalent of doing with cpus: + +Host A: + +$ qemu-system-x86_64 -cpu host + +Host B: + +$ qemu-system-x86_64 -cpu host + +When both hosts have different cpu features this is guaranteed to +fail. Especially if Host B has less features than host A. If host A +has less features than host B, sometimes it works. Important word of +last sentence is "sometimes". + +So, forgetting about cpu models and continuing with the -cpu host +example, let's see that the differences of the cpus is that Host A and +B have the following features: + +Features: 'pcid' 'stibp' 'taa-no' +Host A: X X +Host B: X + +And we want to migrate between them, the way configure both QEMU cpu +will be: + +Host A: + +$ qemu-system-x86_64 -cpu host,pcid=off,stibp=off + +Host B: + +$ qemu-system-x86_64 -cpu host,taa-no=off + +And you would be able to migrate between them. It is responsability +of the management application or of the user to make sure that the +configuration is correct. QEMU doesn't know how to look at this kind +of features in general. + +Notice that we don't recomend to use -cpu host for migration. It is +used in this example because it makes the example simpler. + +Other devices have worse control about individual features. If they +want to be able to migrate between hosts that show different features, +the device needs a way to configure which ones it is going to use. + +In this section we have considered that we are using the same QEMU +binary in both sides of the migration. If we use different QEMU +versions process, then we need to have into account all other +differences and the examples become even more complicated. -- 2.41.0

We don't allow non zero compressed pages since: commit 3edcd7e6ebae3ef0ac178eed5f4225803159562d Author: Peter Lieven <pl(a)kamp.de&gt; Date: Tue Mar 26 10:58:35 2013 +0100 migration: search for zero instead of dup pages RDMA case is a bit more complicated, but they don't handle it since: commit a1febc4950f2c6232c002f401d7cd409f6fa6a88 Author: Richard Henderson <rth(a)twiddle.net&gt; Date: Mon Aug 29 11:46:14 2016 -0700 cutils: Export only buffer_is_zero Reviewed-by: Fabiano Rosas <farosas(a)suse.de&gt; Reviewed-by: Peter Xu <peterx(a)redhat.com&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231019085259.13307-2-quintela(a)redhat.com&gt; --- migration/ram.c | 15 +++++++++++---- migration/rdma.c | 6 +++++- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index 92769902bb..4bfb20c94a 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -3715,16 +3715,18 @@ int ram_load_postcopy(QEMUFile *f, int channel) switch (flags & ~RAM_SAVE_FLAG_CONTINUE) { case RAM_SAVE_FLAG_ZERO: ch = qemu_get_byte(f); + if (ch != 0) { + error_report("Found a zero page with value %d", ch); + ret = -EINVAL; + break; + } /* * Can skip to set page_buffer when * this is a zero page and (block->page_size == TARGET_PAGE_SIZE). */ - if (ch || !matches_target_page_size) { + if (!matches_target_page_size) { memset(page_buffer, ch, TARGET_PAGE_SIZE); } - if (ch) { - tmp_page->all_zero = false; - } break; case RAM_SAVE_FLAG_PAGE: @@ -4030,6 +4032,11 @@ static int ram_load_precopy(QEMUFile *f) case RAM_SAVE_FLAG_ZERO: ch = qemu_get_byte(f); + if (ch != 0) { + error_report("Found a zero page with value %d", ch); + ret = -EINVAL; + break; + } ram_handle_compressed(host, ch, TARGET_PAGE_SIZE); break; diff --git a/migration/rdma.c b/migration/rdma.c index 2a1852ec7f..2d963fd147 100644 --- a/migration/rdma.c +++ b/migration/rdma.c @@ -3592,7 +3592,11 @@ int rdma_registration_handle(QEMUFile *f) host_addr = block->local_host_addr + (comp->offset - block->offset); - + if (comp->value) { + error_report("rdma: Zero page with non-zero (%d) value", + comp->value); + goto err; + } ram_handle_compressed(host_addr, comp->value, comp->length); break; -- 2.41.0

Reviewed-by: Fabiano Rosas <farosas(a)suse.de&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231019110724.15324-2-quintela(a)redhat.com&gt; --- migration/options.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/migration/options.c b/migration/options.c index 42fb818956..b8c3c3218d 100644 --- a/migration/options.c +++ b/migration/options.c @@ -618,6 +618,13 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp) } } + if (new_caps[MIGRATION_CAPABILITY_MULTIFD]) { + if (new_caps[MIGRATION_CAPABILITY_XBZRLE]) { + error_setg(errp, "Multifd is not compatible with xbzrle"); + return false; + } + } + return true; } -- 2.41.0

After previous patch, we disable the posiblity that we use compression together with xbzrle. So we can use directly migrate_compress(). Once there, now we don't need the rs parameter, so remove it. Reviewed-by: Fabiano Rosas <farosas(a)suse.de&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231019110724.15324-4-quintela(a)redhat.com&gt; --- migration/ram.c | 34 +++++++--------------------------- 1 file changed, 7 insertions(+), 27 deletions(-) diff --git a/migration/ram.c b/migration/ram.c index e0ad732ee8..8246663f64 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1291,8 +1291,6 @@ static int ram_save_multifd_page(QEMUFile *file, RAMBlock *block, return 1; } -static bool save_page_use_compression(RAMState *rs); - static int send_queued_data(CompressParam *param) { PageSearchStatus *pss = &ram_state->pss[RAM_CHANNEL_PRECOPY]; @@ -1329,9 +1327,9 @@ static int send_queued_data(CompressParam *param) return len; } -static void ram_flush_compressed_data(RAMState *rs) +static void ram_flush_compressed_data(void) { - if (!save_page_use_compression(rs)) { + if (!migrate_compress()) { return; } @@ -1393,7 +1391,7 @@ static int find_dirty_block(RAMState *rs, PageSearchStatus *pss) * Also If xbzrle is on, stop using the data compression at this * point. In theory, xbzrle can do better than compression. */ - ram_flush_compressed_data(rs); + ram_flush_compressed_data(); /* Hit the end of the list */ pss->block = QLIST_FIRST_RCU(&ram_list.blocks); @@ -2042,24 +2040,6 @@ int ram_save_queue_pages(const char *rbname, ram_addr_t start, ram_addr_t len) return 0; } -static bool save_page_use_compression(RAMState *rs) -{ - if (!migrate_compress()) { - return false; - } - - /* - * If xbzrle is enabled (e.g., after first round of migration), stop - * using the data compression. In theory, xbzrle can do better than - * compression. - */ - if (rs->xbzrle_started) { - return false; - } - - return true; -} - /* * try to compress the page before posting it out, return true if the page * has been properly handled by compression, otherwise needs other @@ -2068,7 +2048,7 @@ static bool save_page_use_compression(RAMState *rs) static bool save_compress_page(RAMState *rs, PageSearchStatus *pss, ram_addr_t offset) { - if (!save_page_use_compression(rs)) { + if (!migrate_compress()) { return false; } @@ -2083,7 +2063,7 @@ static bool save_compress_page(RAMState *rs, PageSearchStatus *pss, * much CPU resource. */ if (pss->block != pss->last_sent_block) { - ram_flush_compressed_data(rs); + ram_flush_compressed_data(); return false; } @@ -3135,7 +3115,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) * page is sent in one chunk. */ if (migrate_postcopy_ram()) { - ram_flush_compressed_data(rs); + ram_flush_compressed_data(); } /* @@ -3236,7 +3216,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque) } qemu_mutex_unlock(&rs->bitmap_mutex); - ram_flush_compressed_data(rs); + ram_flush_compressed_data(); int ret = rdma_registration_stop(f, RAM_CONTROL_FINISH); if (ret < 0) { -- 2.41.0

Move the goto to a while true. Reviewed-by: Lukas Straub <lukasstraub2(a)web.de&gt; Reviewed-by: Fabiano Rosas <farosas(a)suse.de&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231019110724.15324-6-quintela(a)redhat.com&gt; --- migration/ram-compress.c | 48 ++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/migration/ram-compress.c b/migration/ram-compress.c index ef03d60a6d..a991b15b7a 100644 --- a/migration/ram-compress.c +++ b/migration/ram-compress.c @@ -271,35 +271,35 @@ bool compress_page_with_multi_thread(RAMBlock *block, ram_addr_t offset, thread_count = migrate_compress_threads(); qemu_mutex_lock(&comp_done_lock); -retry: - for (int i = 0; i < thread_count; i++) { - if (comp_param[i].done) { - CompressParam *param = &comp_param[i]; - qemu_mutex_lock(&param->mutex); - param->done = false; - send_queued_data(param); - assert(qemu_file_buffer_empty(param->file)); - compress_reset_result(param); - set_compress_params(param, block, offset); - qemu_cond_signal(&param->cond); - qemu_mutex_unlock(&param->mutex); + while (true) { + for (int i = 0; i < thread_count; i++) { + if (comp_param[i].done) { + CompressParam *param = &comp_param[i]; + qemu_mutex_lock(&param->mutex); + param->done = false; + send_queued_data(param); + assert(qemu_file_buffer_empty(param->file)); + compress_reset_result(param); + set_compress_params(param, block, offset); + + qemu_cond_signal(&param->cond); + qemu_mutex_unlock(&param->mutex); + qemu_mutex_unlock(&comp_done_lock); + return true; + } + } + if (!wait) { qemu_mutex_unlock(&comp_done_lock); - return true; + return false; } - } - - /* - * wait for the free thread if the user specifies 'compress-wait-thread', - * otherwise we will post the page out in the main thread as normal page. - */ - if (wait) { + /* + * wait for a free thread if the user specifies + * 'compress-wait-thread', otherwise we will post the page out + * in the main thread as normal page. + */ qemu_cond_wait(&comp_done_cond, &comp_done_lock); - goto retry; } - qemu_mutex_unlock(&comp_done_lock); - - return false; } /* return the size after decompression, or negative value on error */ -- 2.41.0

So we can move more compression_counters stuff to ram-compress.c. Create compression_counters struct to add the stuff that was on MigrationState. Reviewed-by: Lukas Straub <lukasstraub2(a)web.de&gt; Reviewed-by: Fabiano Rosas <farosas(a)suse.de&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231019110724.15324-8-quintela(a)redhat.com&gt; --- migration/ram-compress.h | 1 + migration/ram.h | 1 - migration/ram-compress.c | 42 +++++++++++++++++++++++++++++++++++++++- migration/ram.c | 29 +-------------------------- 4 files changed, 43 insertions(+), 30 deletions(-) diff --git a/migration/ram-compress.h b/migration/ram-compress.h index b228640092..76dacd3ec7 100644 --- a/migration/ram-compress.h +++ b/migration/ram-compress.h @@ -71,5 +71,6 @@ void decompress_data_with_multi_threads(QEMUFile *f, void *host, int len); void populate_compress(MigrationInfo *info); uint64_t ram_compressed_pages(void); void update_compress_thread_counts(const CompressParam *param, int bytes_xmit); +void compress_update_rates(uint64_t page_count); #endif diff --git a/migration/ram.h b/migration/ram.h index 3f724b2f02..9f3ad1ee81 100644 --- a/migration/ram.h +++ b/migration/ram.h @@ -34,7 +34,6 @@ #include "io/channel.h" extern XBZRLECacheStats xbzrle_counters; -extern CompressionStats compression_counters; /* Should be holding either ram_list.mutex, or the RCU lock. */ #define RAMBLOCK_FOREACH_NOT_IGNORED(block) \ diff --git a/migration/ram-compress.c b/migration/ram-compress.c index f56e1f8e69..af42cab0fe 100644 --- a/migration/ram-compress.c +++ b/migration/ram-compress.c @@ -41,7 +41,20 @@ #include "ram.h" #include "migration-stats.h" -CompressionStats compression_counters; +static struct { + int64_t pages; + int64_t busy; + double busy_rate; + int64_t compressed_size; + double compression_rate; + /* compression statistics since the beginning of the period */ + /* amount of count that no free thread to compress data */ + uint64_t compress_thread_busy_prev; + /* amount bytes after compression */ + uint64_t compressed_size_prev; + /* amount of compressed pages */ + uint64_t compress_pages_prev; +} compression_counters; static CompressParam *comp_param; static QemuThread *compress_threads; @@ -518,3 +531,30 @@ void update_compress_thread_counts(const CompressParam *param, int bytes_xmit) compression_counters.pages++; } +void compress_update_rates(uint64_t page_count) +{ + if (!migrate_compress()) { + return; + } + compression_counters.busy_rate = (double)(compression_counters.busy - + compression_counters.compress_thread_busy_prev) / page_count; + compression_counters.compress_thread_busy_prev = + compression_counters.busy; + + double compressed_size = compression_counters.compressed_size - + compression_counters.compressed_size_prev; + if (compressed_size) { + double uncompressed_size = (compression_counters.pages - + compression_counters.compress_pages_prev) * + qemu_target_page_size(); + + /* Compression-Ratio = Uncompressed-size / Compressed-size */ + compression_counters.compression_rate = + uncompressed_size / compressed_size; + + compression_counters.compress_pages_prev = + compression_counters.pages; + compression_counters.compressed_size_prev = + compression_counters.compressed_size; + } +} diff --git a/migration/ram.c b/migration/ram.c index 46209388ec..f7daf2226e 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -369,13 +369,6 @@ struct RAMState { bool xbzrle_started; /* Are we on the last stage of migration */ bool last_stage; - /* compression statistics since the beginning of the period */ - /* amount of count that no free thread to compress data */ - uint64_t compress_thread_busy_prev; - /* amount bytes after compression */ - uint64_t compressed_size_prev; - /* amount of compressed pages */ - uint64_t compress_pages_prev; /* total handled target pages at the beginning of period */ uint64_t target_page_count_prev; @@ -945,7 +938,6 @@ uint64_t ram_get_total_transferred_pages(void) static void migration_update_rates(RAMState *rs, int64_t end_time) { uint64_t page_count = rs->target_page_count - rs->target_page_count_prev; - double compressed_size; /* calculate period counters */ stat64_set(&mig_stats.dirty_pages_rate, @@ -973,26 +965,7 @@ static void migration_update_rates(RAMState *rs, int64_t end_time) rs->xbzrle_pages_prev = xbzrle_counters.pages; rs->xbzrle_bytes_prev = xbzrle_counters.bytes; } - - if (migrate_compress()) { - compression_counters.busy_rate = (double)(compression_counters.busy - - rs->compress_thread_busy_prev) / page_count; - rs->compress_thread_busy_prev = compression_counters.busy; - - compressed_size = compression_counters.compressed_size - - rs->compressed_size_prev; - if (compressed_size) { - double uncompressed_size = (compression_counters.pages - - rs->compress_pages_prev) * TARGET_PAGE_SIZE; - - /* Compression-Ratio = Uncompressed-size / Compressed-size */ - compression_counters.compression_rate = - uncompressed_size / compressed_size; - - rs->compress_pages_prev = compression_counters.pages; - rs->compressed_size_prev = compression_counters.compressed_size; - } - } + compress_update_rates(page_count); } /* -- 2.41.0

As we export it, rename it compress_flush_data(). Reviewed-by: Fabiano Rosas <farosas(a)suse.de&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231019110724.15324-10-quintela(a)redhat.com&gt; --- migration/ram-compress.h | 1 + migration/ram-compress.c | 9 +++++++++ migration/ram.c | 17 ++++------------- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/migration/ram-compress.h b/migration/ram-compress.h index 636281ed97..7ba01e2882 100644 --- a/migration/ram-compress.h +++ b/migration/ram-compress.h @@ -73,5 +73,6 @@ uint64_t ram_compressed_pages(void); void update_compress_thread_counts(const CompressParam *param, int bytes_xmit); void compress_update_rates(uint64_t page_count); int compress_send_queued_data(CompressParam *param); +void compress_flush_data(void); #endif diff --git a/migration/ram-compress.c b/migration/ram-compress.c index af42cab0fe..1443a1cb45 100644 --- a/migration/ram-compress.c +++ b/migration/ram-compress.c @@ -558,3 +558,12 @@ void compress_update_rates(uint64_t page_count) compression_counters.compressed_size; } } + +void compress_flush_data(void) +{ + if (!migrate_compress()) { + return; + } + + flush_compressed_data(compress_send_queued_data); +} diff --git a/migration/ram.c b/migration/ram.c index b6d485358e..849752ef29 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -1300,15 +1300,6 @@ int compress_send_queued_data(CompressParam *param) return len; } -static void ram_flush_compressed_data(void) -{ - if (!migrate_compress()) { - return; - } - - flush_compressed_data(compress_send_queued_data); -} - #define PAGE_ALL_CLEAN 0 #define PAGE_TRY_AGAIN 1 #define PAGE_DIRTY_FOUND 2 @@ -1364,7 +1355,7 @@ static int find_dirty_block(RAMState *rs, PageSearchStatus *pss) * Also If xbzrle is on, stop using the data compression at this * point. In theory, xbzrle can do better than compression. */ - ram_flush_compressed_data(); + compress_flush_data(); /* Hit the end of the list */ pss->block = QLIST_FIRST_RCU(&ram_list.blocks); @@ -2036,7 +2027,7 @@ static bool save_compress_page(RAMState *rs, PageSearchStatus *pss, * much CPU resource. */ if (pss->block != pss->last_sent_block) { - ram_flush_compressed_data(); + compress_flush_data(); return false; } @@ -3083,7 +3074,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque) * page is sent in one chunk. */ if (migrate_postcopy_ram()) { - ram_flush_compressed_data(); + compress_flush_data(); } /* @@ -3184,7 +3175,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque) } qemu_mutex_unlock(&rs->bitmap_mutex); - ram_flush_compressed_data(); + compress_flush_data(); int ret = rdma_registration_stop(f, RAM_CONTROL_FINISH); if (ret < 0) { -- 2.41.0

We are moving to have all functions exported from ram-compress.c to start with compress_. Reviewed-by: Fabiano Rosas <farosas(a)suse.de&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231019110724.15324-12-quintela(a)redhat.com&gt; --- migration/ram-compress.h | 2 +- migration/ram-compress.c | 2 +- migration/ram.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/migration/ram-compress.h b/migration/ram-compress.h index e222887fb7..0d89a2f55e 100644 --- a/migration/ram-compress.h +++ b/migration/ram-compress.h @@ -68,7 +68,7 @@ int compress_threads_load_setup(QEMUFile *f); void decompress_data_with_multi_threads(QEMUFile *f, void *host, int len); void populate_compress(MigrationInfo *info); -uint64_t ram_compressed_pages(void); +uint64_t compress_ram_pages(void); void update_compress_thread_counts(const CompressParam *param, int bytes_xmit); void compress_update_rates(uint64_t page_count); int compress_send_queued_data(CompressParam *param); diff --git a/migration/ram-compress.c b/migration/ram-compress.c index 036e44085b..fa4388f6a6 100644 --- a/migration/ram-compress.c +++ b/migration/ram-compress.c @@ -516,7 +516,7 @@ void populate_compress(MigrationInfo *info) info->compression->compression_rate = compression_counters.compression_rate; } -uint64_t ram_compressed_pages(void) +uint64_t compress_ram_pages(void) { return compression_counters.pages; } diff --git a/migration/ram.c b/migration/ram.c index 849752ef29..6335564035 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -932,7 +932,7 @@ uint64_t ram_get_total_transferred_pages(void) { return stat64_get(&mig_stats.normal_pages) + stat64_get(&mig_stats.zero_pages) + - ram_compressed_pages() + xbzrle_counters.pages; + compress_ram_pages() + xbzrle_counters.pages; } static void migration_update_rates(RAMState *rs, int64_t end_time) -- 2.41.0

From: Thomas Huth <thuth(a)redhat.com&gt; Since the instance_init() function immediately tries to set the property to "true", the s390_skeys_set_migration_enabled() tries to register a savevm handler during instance_init(). However, instance_init() functions can be called multiple times, e.g. for introspection of devices. That means multiple instances of devices can be created during runtime (which is fine as long as they all don't get realized, too), so the "Prevent double registration of savevm handler" check in the s390_skeys_set_migration_enabled() function does not work at all as expected (since there could be more than one instance). Thus we must not call register_savevm_live() from an instance_init() function at all. Move this to the realize() function instead. This way we can also get rid of the property getter and setter functions completely, simplifying the code along the way quite a bit. Acked-by: David Hildenbrand <david(a)redhat.com&gt; Reviewed-by: Eric Farman <farman(a)linux.ibm.com&gt; Acked-by: Juan Quintela <quintela(a)redhat.com&gt; Signed-off-by: Thomas Huth <thuth(a)redhat.com&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231020150554.664422-2-thuth(a)redhat.com&gt; --- hw/s390x/s390-skeys.c | 35 ++++++++--------------------------- 1 file changed, 8 insertions(+), 27 deletions(-) diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c index 5024faf411..8e9d9e41e8 100644 --- a/hw/s390x/s390-skeys.c +++ b/hw/s390x/s390-skeys.c @@ -12,6 +12,7 @@ #include "qemu/osdep.h" #include "qemu/units.h" #include "hw/boards.h" +#include "hw/qdev-properties.h" #include "hw/s390x/storage-keys.h" #include "qapi/error.h" #include "qapi/qapi-commands-misc-target.h" @@ -432,58 +433,38 @@ static int s390_storage_keys_load(QEMUFile *f, void *opaque, int version_id) return ret; } -static inline bool s390_skeys_get_migration_enabled(Object *obj, Error **errp) -{ - S390SKeysState *ss = S390_SKEYS(obj); - - return ss->migration_enabled; -} - static SaveVMHandlers savevm_s390_storage_keys = { .save_state = s390_storage_keys_save, .load_state = s390_storage_keys_load, }; -static inline void s390_skeys_set_migration_enabled(Object *obj, bool value, - Error **errp) +static void s390_skeys_realize(DeviceState *dev, Error **errp) { - S390SKeysState *ss = S390_SKEYS(obj); - - /* Prevent double registration of savevm handler */ - if (ss->migration_enabled == value) { - return; - } - - ss->migration_enabled = value; + S390SKeysState *ss = S390_SKEYS(dev); if (ss->migration_enabled) { register_savevm_live(TYPE_S390_SKEYS, 0, 1, &savevm_s390_storage_keys, ss); - } else { - unregister_savevm(VMSTATE_IF(ss), TYPE_S390_SKEYS, ss); } } -static void s390_skeys_instance_init(Object *obj) -{ - object_property_add_bool(obj, "migration-enabled", - s390_skeys_get_migration_enabled, - s390_skeys_set_migration_enabled); - object_property_set_bool(obj, "migration-enabled", true, NULL); -} +static Property s390_skeys_props[] = { + DEFINE_PROP_BOOL("migration-enabled", S390SKeysState, migration_enabled, true), +}; static void s390_skeys_class_init(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); dc->hotpluggable = false; + dc->realize = s390_skeys_realize; + device_class_set_props(dc, s390_skeys_props); set_bit(DEVICE_CATEGORY_MISC, dc->categories); } static const TypeInfo s390_skeys_info = { .name = TYPE_S390_SKEYS, .parent = TYPE_DEVICE, - .instance_init = s390_skeys_instance_init, .instance_size = sizeof(S390SKeysState), .class_init = s390_skeys_class_init, .class_size = sizeof(S390SKeysClass), -- 2.41.0

From: Thomas Huth <thuth(a)redhat.com&gt; There's no need for dedicated handlers here if they don't do anything special. Acked-by: David Hildenbrand <david(a)redhat.com&gt; Reviewed-by: Eric Farman <farman(a)linux.ibm.com&gt; Acked-by: Juan Quintela <quintela(a)redhat.com&gt; Signed-off-by: Thomas Huth <thuth(a)redhat.com&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231020150554.664422-3-thuth(a)redhat.com&gt; --- hw/s390x/s390-stattrib.c | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c index 220e845d12..52f9fc036e 100644 --- a/hw/s390x/s390-stattrib.c +++ b/hw/s390x/s390-stattrib.c @@ -13,6 +13,7 @@ #include "qemu/units.h" #include "migration/qemu-file.h" #include "migration/register.h" +#include "hw/qdev-properties.h" #include "hw/s390x/storage-attributes.h" #include "qemu/error-report.h" #include "exec/ram_addr.h" @@ -340,6 +341,10 @@ static void s390_stattrib_realize(DeviceState *dev, Error **errp) } } +static Property s390_stattrib_props[] = { + DEFINE_PROP_BOOL("migration-enabled", S390StAttribState, migration_enabled, true), +}; + static void s390_stattrib_class_init(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); @@ -347,22 +352,7 @@ static void s390_stattrib_class_init(ObjectClass *oc, void *data) dc->hotpluggable = false; set_bit(DEVICE_CATEGORY_MISC, dc->categories); dc->realize = s390_stattrib_realize; -} - -static inline bool s390_stattrib_get_migration_enabled(Object *obj, - Error **errp) -{ - S390StAttribState *s = S390_STATTRIB(obj); - - return s->migration_enabled; -} - -static inline void s390_stattrib_set_migration_enabled(Object *obj, bool value, - Error **errp) -{ - S390StAttribState *s = S390_STATTRIB(obj); - - s->migration_enabled = value; + device_class_set_props(dc, s390_stattrib_props); } static SaveVMHandlers savevm_s390_stattrib_handlers = { @@ -383,10 +373,6 @@ static void s390_stattrib_instance_init(Object *obj) register_savevm_live(TYPE_S390_STATTRIB, 0, 0, &savevm_s390_stattrib_handlers, sas); - object_property_add_bool(obj, "migration-enabled", - s390_stattrib_get_migration_enabled, - s390_stattrib_set_migration_enabled); - object_property_set_bool(obj, "migration-enabled", true, NULL); sas->migration_cur_gfn = 0; } -- 2.41.0

From: Thomas Huth <thuth(a)redhat.com&gt; We must not call register_savevm_live() from an instance_init() function (since this could be called multiple times during device introspection). Move this to the realize() function instead. Acked-by: David Hildenbrand <david(a)redhat.com&gt; Reviewed-by: Eric Farman <farman(a)linux.ibm.com&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Signed-off-by: Thomas Huth <thuth(a)redhat.com&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231020150554.664422-4-thuth(a)redhat.com&gt; --- hw/s390x/s390-stattrib.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c index 52f9fc036e..54a138011c 100644 --- a/hw/s390x/s390-stattrib.c +++ b/hw/s390x/s390-stattrib.c @@ -331,6 +331,17 @@ static const TypeInfo qemu_s390_stattrib_info = { /* Generic abstract object: */ +static SaveVMHandlers savevm_s390_stattrib_handlers = { + .save_setup = cmma_save_setup, + .save_live_iterate = cmma_save_iterate, + .save_live_complete_precopy = cmma_save_complete, + .state_pending_exact = cmma_state_pending, + .state_pending_estimate = cmma_state_pending, + .save_cleanup = cmma_save_cleanup, + .load_state = cmma_load, + .is_active = cmma_active, +}; + static void s390_stattrib_realize(DeviceState *dev, Error **errp) { bool ambiguous = false; @@ -338,7 +349,11 @@ static void s390_stattrib_realize(DeviceState *dev, Error **errp) object_resolve_path_type("", TYPE_S390_STATTRIB, &ambiguous); if (ambiguous) { error_setg(errp, "storage_attributes device already exists"); + return; } + + register_savevm_live(TYPE_S390_STATTRIB, 0, 0, + &savevm_s390_stattrib_handlers, dev); } static Property s390_stattrib_props[] = { @@ -355,24 +370,10 @@ static void s390_stattrib_class_init(ObjectClass *oc, void *data) device_class_set_props(dc, s390_stattrib_props); } -static SaveVMHandlers savevm_s390_stattrib_handlers = { - .save_setup = cmma_save_setup, - .save_live_iterate = cmma_save_iterate, - .save_live_complete_precopy = cmma_save_complete, - .state_pending_exact = cmma_state_pending, - .state_pending_estimate = cmma_state_pending, - .save_cleanup = cmma_save_cleanup, - .load_state = cmma_load, - .is_active = cmma_active, -}; - static void s390_stattrib_instance_init(Object *obj) { S390StAttribState *sas = S390_STATTRIB(obj); - register_savevm_live(TYPE_S390_STATTRIB, 0, 0, - &savevm_s390_stattrib_handlers, sas); - sas->migration_cur_gfn = 0; } -- 2.41.0

From: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; The function is used on save at this point. The following commits will use it on load. Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com&gt; Reviewed-by: Juan Quintela <quintela(a)redhat.com&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231024084043.2926316-5-marcandre.lureau(a)redhat.com&gt; --- include/migration/vmstate.h | 2 +- migration/savevm.c | 2 +- migration/vmstate.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 1a31fb7293..1af181877c 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -1202,7 +1202,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd, void *opaque, JSONWriter *vmdesc, int version_id, Error **errp); -bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque); +bool vmstate_section_needed(const VMStateDescription *vmsd, void *opaque); #define VMSTATE_INSTANCE_ID_ANY -1 diff --git a/migration/savevm.c b/migration/savevm.c index 8622f229e5..ca5c7cebe0 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -985,7 +985,7 @@ static int vmstate_save(QEMUFile *f, SaveStateEntry *se, JSONWriter *vmdesc) if ((!se->ops || !se->ops->save_state) && !se->vmsd) { return 0; } - if (se->vmsd && !vmstate_save_needed(se->vmsd, se->opaque)) { + if (se->vmsd && !vmstate_section_needed(se->vmsd, se->opaque)) { trace_savevm_section_skip(se->idstr, se->section_id); return 0; } diff --git a/migration/vmstate.c b/migration/vmstate.c index 1cf9e45b85..16e33a5d34 100644 --- a/migration/vmstate.c +++ b/migration/vmstate.c @@ -324,7 +324,7 @@ static void vmsd_desc_field_end(const VMStateDescription *vmsd, } -bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque) +bool vmstate_section_needed(const VMStateDescription *vmsd, void *opaque) { if (vmsd->needed && !vmsd->needed(opaque)) { /* optional section not needed */ @@ -522,7 +522,7 @@ static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd, trace_vmstate_subsection_save_top(vmsd->name); while (sub && *sub) { - if (vmstate_save_needed(*sub, opaque)) { + if (vmstate_section_needed(*sub, opaque)) { const VMStateDescription *vmsdsub = *sub; uint8_t len; -- 2.41.0

We have lots of cases where we are using an instance_id==0 when we should be using VMSTATE_INSTANCE_ID_ANY (-1). Basically everything that can have more than one needs to have a proper instance_id or -1 and the system will take one for it. vmstate_register_any(): We register with -1. Reviewed-by: Stefan Berger <stefanb(a)linux.ibm.com&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231020090731.28701-2-quintela(a)redhat.com&gt; --- include/migration/vmstate.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 1af181877c..1ea97ccf2d 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -1230,6 +1230,23 @@ static inline int vmstate_register(VMStateIf *obj, int instance_id, opaque, -1, 0, NULL); } +/** + * vmstate_register_any() - legacy function to register state + * serialisation description and let the function choose the id + * + * New code shouldn't be using this function as QOM-ified devices have + * dc->vmsd to store the serialisation description. + * + * Returns: 0 on success, -1 on failure + */ +static inline int vmstate_register_any(VMStateIf *obj, + const VMStateDescription *vmsd, + void *opaque) +{ + return vmstate_register_with_alias_id(obj, VMSTATE_INSTANCE_ID_ANY, vmsd, + opaque, -1, 0, NULL); +} + void vmstate_unregister(VMStateIf *obj, const VMStateDescription *vmsd, void *opaque); -- 2.41.0

Otherwise qom-test fails. ok 4 /i386/qom/x-remote qemu-system-i386: savevm_state_handler_insert: Detected duplicate SaveStateEntry: id=isa-ide, instance_id=0x0 Broken pipe ../../../../../mnt/code/qemu/full/tests/qtest/libqtest.c:195: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0) Aborted (core dumped) $ Reviewed-by: Stefan Berger <stefanb(a)linux.ibm.com&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231020090731.28701-4-quintela(a)redhat.com&gt; --- hw/ide/isa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ide/isa.c b/hw/ide/isa.c index 95053e026f..ea60c08116 100644 --- a/hw/ide/isa.c +++ b/hw/ide/isa.c @@ -73,7 +73,7 @@ static void isa_ide_realizefn(DeviceState *dev, Error **errp) ide_bus_init(&s->bus, sizeof(s->bus), dev, 0, 2); ide_init_ioport(&s->bus, isadev, s->iobase, s->iobase2); ide_bus_init_output_irq(&s->bus, isa_get_irq(isadev, s->irqnum)); - vmstate_register(VMSTATE_IF(dev), 0, &vmstate_ide_isa, s); + vmstate_register_any(VMSTATE_IF(dev), &vmstate_ide_isa, s); ide_bus_register_restart_cb(&s->bus); } -- 2.41.0

Current code does: - register pre_2_10_vmstate_dummy_icp with "icp/server" and instance dependinfg on cpu number - for newer machines, it register vmstate_icp with "icp/server" name and instance 0 - now it unregisters "icp/server" for the 1st instance. This is wrong at many levels: - we shouldn't have two VMSTATEDescriptions with the same name - In case this is the only solution that we can came with, it needs to be: * register pre_2_10_vmstate_dummy_icp * unregister pre_2_10_vmstate_dummy_icp * register real vmstate_icp Created vmstate_replace_hack_for_ppc() with warnings left and right that it is a hack. CC: Cedric Le Goater <clg(a)kaod.org&gt; CC: Daniel Henrique Barboza <danielhb413(a)gmail.com&gt; CC: David Gibson <david(a)gibson.dropbear.id.au&gt; CC: Greg Kurz <groug(a)kaod.org&gt; Reviewed-by: Nicholas Piggin <npiggin(a)gmail.com&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231020090731.28701-8-quintela(a)redhat.com&gt; --- include/migration/vmstate.h | 11 +++++++++++ hw/intc/xics.c | 18 ++++++++++++++++-- hw/ppc/spapr.c | 25 +++++++++++++++++++++++-- migration/savevm.c | 18 ++++++++++++++++++ 4 files changed, 68 insertions(+), 4 deletions(-) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 1ea97ccf2d..9821918631 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -1230,6 +1230,17 @@ static inline int vmstate_register(VMStateIf *obj, int instance_id, opaque, -1, 0, NULL); } +/** + * vmstate_replace_hack_for_ppc() - ppc used to abuse vmstate_register + * + * Don't even think about using this function in new code. + * + * Returns: 0 on success, -1 on failure + */ +int vmstate_replace_hack_for_ppc(VMStateIf *obj, int instance_id, + const VMStateDescription *vmsd, + void *opaque); + /** * vmstate_register_any() - legacy function to register state * serialisation description and let the function choose the id diff --git a/hw/intc/xics.c b/hw/intc/xics.c index c7f8abd71e..c77e986136 100644 --- a/hw/intc/xics.c +++ b/hw/intc/xics.c @@ -335,8 +335,22 @@ static void icp_realize(DeviceState *dev, Error **errp) return; } } - - vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp); + /* + * The way that pre_2_10_icp is handling is really, really hacky. + * We used to have here this call: + * + * vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp); + * + * But we were doing: + * pre_2_10_vmstate_register_dummy_icp() + * this vmstate_register() + * pre_2_10_vmstate_unregister_dummy_icp() + * + * So for a short amount of time we had to vmstate entries with + * the same name. This fixes it. + */ + vmstate_replace_hack_for_ppc(NULL, icp->cs->cpu_index, + &vmstate_icp_server, icp); } static void icp_unrealize(DeviceState *dev) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index b25093be28..df09aa9d6a 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -143,6 +143,11 @@ static bool pre_2_10_vmstate_dummy_icp_needed(void *opaque) } static const VMStateDescription pre_2_10_vmstate_dummy_icp = { + /* + * Hack ahead. We can't have two devices with the same name and + * instance id. So I rename this to pass make check. + * Real help from people who knows the hardware is needed. + */ .name = "icp/server", .version_id = 1, .minimum_version_id = 1, @@ -155,16 +160,32 @@ static const VMStateDescription pre_2_10_vmstate_dummy_icp = { }, }; +/* + * See comment in hw/intc/xics.c:icp_realize() + * + * You have to remove vmstate_replace_hack_for_ppc() when you remove + * the machine types that need the following function. + */ static void pre_2_10_vmstate_register_dummy_icp(int i) { vmstate_register(NULL, i, &pre_2_10_vmstate_dummy_icp, (void *)(uintptr_t) i); } +/* + * See comment in hw/intc/xics.c:icp_realize() + * + * You have to remove vmstate_replace_hack_for_ppc() when you remove + * the machine types that need the following function. + */ static void pre_2_10_vmstate_unregister_dummy_icp(int i) { - vmstate_unregister(NULL, &pre_2_10_vmstate_dummy_icp, - (void *)(uintptr_t) i); + /* + * This used to be: + * + * vmstate_unregister(NULL, &pre_2_10_vmstate_dummy_icp, + * (void *)(uintptr_t) i); + */ } int spapr_max_server_number(SpaprMachineState *spapr) diff --git a/migration/savevm.c b/migration/savevm.c index ca5c7cebe0..1d1639c4b6 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -846,6 +846,24 @@ static void vmstate_check(const VMStateDescription *vmsd) } } +/* + * See comment in hw/intc/xics.c:icp_realize() + * + * This function can be removed when + * pre_2_10_vmstate_register_dummy_icp() is removed. + */ +int vmstate_replace_hack_for_ppc(VMStateIf *obj, int instance_id, + const VMStateDescription *vmsd, + void *opaque) +{ + SaveStateEntry *se = find_se(vmsd->name, instance_id); + + if (se) { + savevm_state_handler_remove(se); + } + return vmstate_register(obj, instance_id, vmsd, opaque); +} + int vmstate_register_with_alias_id(VMStateIf *obj, uint32_t instance_id, const VMStateDescription *vmsd, void *opaque, int alias_id, -- 2.41.0

Reviewed-by: Stefan Berger <stefanb(a)linux.ibm.com&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231020090731.28701-11-quintela(a)redhat.com&gt; --- docs/devel/migration.rst | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst index be913630c3..240eb16d90 100644 --- a/docs/devel/migration.rst +++ b/docs/devel/migration.rst @@ -167,13 +167,17 @@ An example (from hw/input/pckbd.c) } }; -We are declaring the state with name "pckbd". -The ``version_id`` is 3, and the fields are 4 uint8_t in a KBDState structure. -We registered this with: +We are declaring the state with name "pckbd". The ``version_id`` is +3, and there are 4 uint8_t fields in the KBDState structure. We +registered this ``VMSTATEDescription`` with one of the following +functions. The first one will generate a device ``instance_id`` +different for each registration. Use the second one if you already +have an id that is different for each instance of the device: .. code:: c - vmstate_register(NULL, 0, &vmstate_kbd, s); + vmstate_register_any(NULL, &vmstate_kbd, s); + vmstate_register(NULL, instance_id, &vmstate_kbd, s); For devices that are ``qdev`` based, we can register the device in the class init function: -- 2.41.0

We can have more than one eeprom93xx. For instance: e100_nic_realize() -> eeprom93xx_new() Reviewed-by: Stefan Berger <stefanb(a)linux.ibm.com&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231020090731.28701-13-quintela(a)redhat.com&gt; --- hw/nvram/eeprom93xx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/nvram/eeprom93xx.c b/hw/nvram/eeprom93xx.c index 1081e2cc0d..57d63638d7 100644 --- a/hw/nvram/eeprom93xx.c +++ b/hw/nvram/eeprom93xx.c @@ -321,7 +321,7 @@ eeprom_t *eeprom93xx_new(DeviceState *dev, uint16_t nwords) /* Output DO is tristate, read results in 1. */ eeprom->eedo = 1; logout("eeprom = 0x%p, nwords = %u\n", eeprom, nwords); - vmstate_register(VMSTATE_IF(dev), 0, &vmstate_eeprom, eeprom); + vmstate_register_any(VMSTATE_IF(dev), &vmstate_eeprom, eeprom); return eeprom; } -- 2.41.0

Create a new filter that removes the two warnings for test 183. Reviewed-by: Hanna Czenczek <hreitz(a)redhat.com&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231018115513.2163-2-quintela(a)redhat.com&gt; --- tests/qemu-iotests/183 | 2 +- tests/qemu-iotests/common.filter | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/183 b/tests/qemu-iotests/183 index ee62939e72..b85770458e 100755 --- a/tests/qemu-iotests/183 +++ b/tests/qemu-iotests/183 @@ -90,7 +90,7 @@ echo reply="$(_send_qemu_cmd $src \ "{ 'execute': 'migrate', 'arguments': { 'uri': 'unix:${MIG_SOCKET}', 'blk': true } }" \ - 'return\|error')" + 'return\|error' | _filter_migration_block_deprecated)" echo "$reply" if echo "$reply" | grep "compiled without old-style" > /dev/null; then _notrun "migrate -b support not compiled in" diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index fc3c64bcb8..2846c83808 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -359,5 +359,12 @@ _filter_qcow2_compression_type_bit() -e 's/\(incompatible_features.*\), 3\(,.*\)/\1\2/' } +# filter warnings caused for block migration deprecation +_filter_migration_block_deprecated() +{ + gsed -e '/warning: parameter .blk. is deprecated; use blockdev-mirror with NBD instead/d' \ + -e '/warning: block migration is deprecated; use blockdev-mirror with NBD instead/d' +} + # make sure this script returns success true -- 2.41.0

Use blocked-mirror with NBD instead. Acked-by: Stefan Hajnoczi <stefanha(a)redhat.com&gt; Reviewed-by: Thomas Huth <thuth(a)redhat.com&gt; Reviewed-by: Markus Armbruster <armbru(a)redhat.com&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231018115513.2163-4-quintela(a)redhat.com&gt; --- docs/about/deprecated.rst | 9 +++++++++ qapi/migration.json | 7 ++++--- migration/migration-hmp-cmds.c | 5 +++++ migration/migration.c | 5 +++++ 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index fc6adf1dea..98b0f14e69 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -469,3 +469,12 @@ Use blockdev-mirror with NBD instead. As an intermediate step the ``inc`` functionality can be achieved by setting the ``block-incremental`` migration parameter to ``true``. But this parameter is also deprecated. + +``blk`` migrate command option (since 8.2) +'''''''''''''''''''''''''''''''''''''''''' + +Use blockdev-mirror with NBD instead. + +As an intermediate step the ``blk`` functionality can be achieved by +setting the ``block`` migration capability to ``true``. But this +capability is also deprecated. diff --git a/qapi/migration.json b/qapi/migration.json index fa7f4f2575..3765c2b662 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -1526,8 +1526,8 @@ # # Features: # -# @deprecated: Member @inc is deprecated. Use blockdev-mirror with -# NBD instead. +# @deprecated: Members @inc and @blk are deprecated. Use +# blockdev-mirror with NBD instead. # # Returns: nothing on success # @@ -1550,7 +1550,8 @@ # <- { "return": {} } ## { 'command': 'migrate', - 'data': {'uri': 'str', '*blk': 'bool', + 'data': {'uri': 'str', + '*blk': { 'type': 'bool', 'features': [ 'deprecated' ] }, '*inc': { 'type': 'bool', 'features': [ 'deprecated' ] }, '*detach': 'bool', '*resume': 'bool' } } diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c index 83176f5bae..dfe98da355 100644 --- a/migration/migration-hmp-cmds.c +++ b/migration/migration-hmp-cmds.c @@ -750,6 +750,11 @@ void hmp_migrate(Monitor *mon, const QDict *qdict) " use blockdev-mirror with NBD instead"); } + if (blk) { + warn_report("option '-b' is deprecated;" + " use blockdev-mirror with NBD instead"); + } + qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, true, resume, &err); if (hmp_handle_error(mon, err)) { diff --git a/migration/migration.c b/migration/migration.c index 06a706ad90..cb2d7161b5 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -1625,6 +1625,11 @@ static bool migrate_prepare(MigrationState *s, bool blk, bool blk_inc, " use blockdev-mirror with NBD instead"); } + if (blk) { + warn_report("parameter 'blk' is deprecated;" + " use blockdev-mirror with NBD instead"); + } + if (resume) { if (s->state != MIGRATION_STATUS_POSTCOPY_PAUSED) { error_setg(errp, "Cannot resume if there is no " -- 2.41.0

Acked-by: Stefan Hajnoczi <stefanha(a)redhat.com&gt; Acked-by: Peter Xu <peterx(a)redhat.com&gt; Reviewed-by: Markus Armbruster <armbru(a)redhat.com&gt; Signed-off-by: Juan Quintela <quintela(a)redhat.com&gt; Message-ID: <20231018115513.2163-6-quintela(a)redhat.com&gt; --- docs/about/deprecated.rst | 8 +++++ qapi/migration.json | 63 ++++++++++++++++++++++++++------------- migration/options.c | 13 ++++++++ 3 files changed, 64 insertions(+), 20 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 7ae872162d..e7f17827d3 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -488,3 +488,11 @@ devices or none. Please see "QMP invocation for live storage migration with ``blockdev-mirror`` + NBD" in docs/interop/live-block-operations.rst for a detailed explanation. + +old compression method (since 8.2) +'''''''''''''''''''''''''''''''''' + +Compression method fails too much. Too many races. We are going to +remove it if nobody fixes it. For starters, migration-test +compression tests are disabled becase they fail randomly. If you need +compression, use multifd compression methods. diff --git a/qapi/migration.json b/qapi/migration.json index e3b00a215b..e6610af428 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -272,6 +272,10 @@ # Features: # # @deprecated: Member @disk is deprecated because block migration is. +# Member @compression is deprecated because it is unreliable and +# untested. It is recommended to use multifd migration, which +# offers an alternative compression implementation that is +# reliable and tested. # # Since: 0.14 ## @@ -289,7 +293,7 @@ '*blocked-reasons': ['str'], '*postcopy-blocktime': 'uint32', '*postcopy-vcpu-blocktime': ['uint32'], - '*compression': 'CompressionStats', + '*compression': { 'type': 'CompressionStats', 'features': [ 'deprecated' ] }, '*socket-address': ['SocketAddress'], '*dirty-limit-throttle-time-per-round': 'uint64', '*dirty-limit-ring-full-time': 'uint64'} } @@ -530,7 +534,10 @@ # Features: # # @deprecated: Member @block is deprecated. Use blockdev-mirror with -# NBD instead. +# NBD instead. Member @compression is deprecated because it is +# unreliable and untested. It is recommended to use multifd +# migration, which offers an alternative compression +# implementation that is reliable and tested. # # @unstable: Members @x-colo and @x-ignore-shared are experimental. # @@ -538,7 +545,8 @@ ## { 'enum': 'MigrationCapability', 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks', - 'compress', 'events', 'postcopy-ram', + { 'name': 'compress', 'features': [ 'deprecated' ] }, + 'events', 'postcopy-ram', { 'name': 'x-colo', 'features': [ 'unstable' ] }, 'release-ram', { 'name': 'block', 'features': [ 'deprecated' ] }, @@ -844,7 +852,9 @@ # Features: # # @deprecated: Member @block-incremental is deprecated. Use -# blockdev-mirror with NBD instead. +# blockdev-mirror with NBD instead. Members @compress-level, +# @compress-threads, @decompress-threads and @compress-wait-thread +# are deprecated because @compression is deprecated. # # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period # are experimental. @@ -854,8 +864,11 @@ { 'enum': 'MigrationParameter', 'data': ['announce-initial', 'announce-max', 'announce-rounds', 'announce-step', - 'compress-level', 'compress-threads', 'decompress-threads', - 'compress-wait-thread', 'throttle-trigger-threshold', + { 'name': 'compress-level', 'features': [ 'deprecated' ] }, + { 'name': 'compress-threads', 'features': [ 'deprecated' ] }, + { 'name': 'decompress-threads', 'features': [ 'deprecated' ] }, + { 'name': 'compress-wait-thread', 'features': [ 'deprecated' ] }, + 'throttle-trigger-threshold', 'cpu-throttle-initial', 'cpu-throttle-increment', 'cpu-throttle-tailslow', 'tls-creds', 'tls-hostname', 'tls-authz', 'max-bandwidth', @@ -1023,7 +1036,9 @@ # Features: # # @deprecated: Member @block-incremental is deprecated. Use -# blockdev-mirror with NBD instead. +# blockdev-mirror with NBD instead. Members @compress-level, +# @compress-threads, @decompress-threads and @compress-wait-thread +# are deprecated because @compression is deprecated. # # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period # are experimental. @@ -1038,10 +1053,14 @@ '*announce-max': 'size', '*announce-rounds': 'size', '*announce-step': 'size', - '*compress-level': 'uint8', - '*compress-threads': 'uint8', - '*compress-wait-thread': 'bool', - '*decompress-threads': 'uint8', + '*compress-level': { 'type': 'uint8', + 'features': [ 'deprecated' ] }, + '*compress-threads': { 'type': 'uint8', + 'features': [ 'deprecated' ] }, + '*compress-wait-thread': { 'type': 'bool', + 'features': [ 'deprecated' ] }, + '*decompress-threads': { 'type': 'uint8', + 'features': [ 'deprecated' ] }, '*throttle-trigger-threshold': 'uint8', '*cpu-throttle-initial': 'uint8', '*cpu-throttle-increment': 'uint8', @@ -1078,7 +1097,7 @@ # Example: # # -> { "execute": "migrate-set-parameters" , -# "arguments": { "compress-level": 1 } } +# "arguments": { "multifd-channels": 5 } } # <- { "return": {} } ## { 'command': 'migrate-set-parameters', 'boxed': true, @@ -1241,7 +1260,9 @@ # Features: # # @deprecated: Member @block-incremental is deprecated. Use -# blockdev-mirror with NBD instead. +# blockdev-mirror with NBD instead. Members @compress-level, +# @compress-threads, @decompress-threads and @compress-wait-thread +# are deprecated because @compression is deprecated. # # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period # are experimental. @@ -1253,10 +1274,14 @@ '*announce-max': 'size', '*announce-rounds': 'size', '*announce-step': 'size', - '*compress-level': 'uint8', - '*compress-threads': 'uint8', - '*compress-wait-thread': 'bool', - '*decompress-threads': 'uint8', + '*compress-level': { 'type': 'uint8', + 'features': [ 'deprecated' ] }, + '*compress-threads': { 'type': 'uint8', + 'features': [ 'deprecated' ] }, + '*compress-wait-thread': { 'type': 'bool', + 'features': [ 'deprecated' ] }, + '*decompress-threads': { 'type': 'uint8', + 'features': [ 'deprecated' ] }, '*throttle-trigger-threshold': 'uint8', '*cpu-throttle-initial': 'uint8', '*cpu-throttle-increment': 'uint8', @@ -1296,10 +1321,8 @@ # # -> { "execute": "query-migrate-parameters" } # <- { "return": { -# "decompress-threads": 2, +# "multifd-channels": 2, # "cpu-throttle-increment": 10, -# "compress-threads": 8, -# "compress-level": 1, # "cpu-throttle-initial": 20, # "max-bandwidth": 33554432, # "downtime-limit": 300 diff --git a/migration/options.c b/migration/options.c index ae8ab47e32..9a39826ca5 100644 --- a/migration/options.c +++ b/migration/options.c @@ -483,6 +483,11 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp) " use blockdev-mirror with NBD instead"); } + if (new_caps[MIGRATION_CAPABILITY_COMPRESS]) { + warn_report("old compression method is deprecated;" + " use multifd compression methods instead"); + } + #ifndef CONFIG_REPLICATION if (new_caps[MIGRATION_CAPABILITY_X_COLO]) { error_setg(errp, "QEMU compiled without replication module" @@ -1335,18 +1340,26 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp) /* TODO use QAPI_CLONE() instead of duplicating it inline */ if (params->has_compress_level) { + warn_report("old compression is deprecated;" + " use multifd compression methods instead"); s->parameters.compress_level = params->compress_level; } if (params->has_compress_threads) { + warn_report("old compression is deprecated;" + " use multifd compression methods instead"); s->parameters.compress_threads = params->compress_threads; } if (params->has_compress_wait_thread) { + warn_report("old compression is deprecated;" + " use multifd compression methods instead"); s->parameters.compress_wait_thread = params->compress_wait_thread; } if (params->has_decompress_threads) { + warn_report("old compression is deprecated;" + " use multifd compression methods instead"); s->parameters.decompress_threads = params->decompress_threads; } -- 2.41.0

Stefan Hajnoczi <stefanha(a)gmail.com&gt; wrote: start with s390x: Errors: 32/840 qemu:qtest+qtest-s390x / qtest-s390x/qom-test ERROR 50.27s killed by signal 6 SIGABRT 104/840 qemu:qtest+qtest-s390x / qtest-s390x/test-hmp ERROR 51.55s killed by signal 6 SIGABRT 189/840 qemu:qtest+qtest-s390x / qtest-s390x/boot-serial-test ERROR 54.07s killed by signal 6 SIGABRT 192/840 qemu:qtest+qtest-s390x / qtest-s390x/qos-test ERROR 51.29s killed by signal 6 SIGABRT 519/840 qemu:qtest+qtest-s390x / qtest-s390x/test-filter-mirror ERROR 50.36s killed by signal 6 SIGABRT 520/840 qemu:qtest+qtest-s390x / qtest-s390x/test-netfilter ERROR 51.03s killed by signal 6 SIGABRT 522/840 qemu:qtest+qtest-s390x / qtest-s390x/device-plug-test ERROR 50.99s killed by signal 6 SIGABRT 523/840 qemu:qtest+qtest-s390x / qtest-s390x/test-filter-redirector ERROR 54.14s killed by signal 6 SIGABRT 524/840 qemu:qtest+qtest-s390x / qtest-s390x/drive_del-test ERROR 53.40s killed by signal 6 SIGABRT 525/840 qemu:qtest+qtest-s390x / qtest-s390x/virtio-ccw-test ERROR 54.67s killed by signal 6 SIGABRT 526/840 qemu:qtest+qtest-s390x / qtest-s390x/device-introspect-test ERROR 51.15s killed by signal 6 SIGABRT 527/840 qemu:qtest+qtest-s390x / qtest-s390x/cpu-plug-test ERROR 51.21s killed by signal 6 SIGABRT 535/840 qemu:qtest+qtest-s390x / qtest-s390x/qmp-test ERROR 51.18s killed by signal 6 SIGABRT 534/840 qemu:qtest+qtest-s390x / qtest-s390x/machine-none-test ERROR 51.21s killed by signal 6 SIGABRT 533/840 qemu:qtest+qtest-s390x / qtest-s390x/qmp-cmd-test ERROR 51.22s killed by signal 6 SIGABRT 549/840 qemu:qtest+qtest-s390x / qtest-s390x/readconfig-test ERROR 51.20s killed by signal 6 SIGABRT 644/840 qemu:block / io-qcow2-001 ERROR 0.32s exit status 1 645/840 qemu:block / io-qcow2-002 ERROR 0.32s exit status 1 646/840 qemu:block / io-qcow2-003 ERROR 0.34s exit status 1 647/840 qemu:block / io-qcow2-004 ERROR 0.31s exit status 1 648/840 qemu:block / io-qcow2-005 ERROR 0.43s exit status 1 649/840 qemu:block / io-qcow2-007 ERROR 0.34s exit status 1 650/840 qemu:block / io-qcow2-008 ERROR 0.63s exit status 1 651/840 qemu:block / io-qcow2-009 ERROR 0.32s exit status 1 652/840 qemu:block / io-qcow2-010 ERROR 0.30s exit status 1 654/840 qemu:block / io-qcow2-011 ERROR 0.31s exit status 1 655/840 qemu:block / io-qcow2-012 ERROR 0.36s exit status 1 657/840 qemu:block / io-qcow2-013 ERROR 0.51s exit status 1 658/840 qemu:block / io-qcow2-017 ERROR 0.37s exit status 1 659/840 qemu:block / io-qcow2-018 ERROR 0.31s exit status 1 660/840 qemu:block / io-qcow2-019 ERROR 0.36s exit status 1 661/840 qemu:block / io-qcow2-020 ERROR 0.34s exit status 1 662/840 qemu:block / io-qcow2-021 ERROR 0.31s exit status 1 663/840 qemu:block / io-qcow2-022 ERROR 0.30s exit status 1 664/840 qemu:block / io-qcow2-024 ERROR 0.30s exit status 1 665/840 qemu:block / io-qcow2-025 ERROR 0.32s exit status 1 666/840 qemu:block / io-qcow2-027 ERROR 0.32s exit status 1 667/840 qemu:block / io-qcow2-029 ERROR 0.34s exit status 1 668/840 qemu:block / io-qcow2-031 ERROR 0.31s exit status 1 669/840 qemu:block / io-qcow2-032 ERROR 0.31s exit status 1 670/840 qemu:block / io-qcow2-033 ERROR 0.31s exit status 1 671/840 qemu:block / io-qcow2-034 ERROR 0.55s exit status 1 672/840 qemu:block / io-qcow2-035 ERROR 0.59s exit status 1 673/840 qemu:block / io-qcow2-037 ERROR 0.32s exit status 1 674/840 qemu:block / io-qcow2-036 ERROR 0.33s exit status 1 675/840 qemu:block / io-qcow2-038 ERROR 0.31s exit status 1 676/840 qemu:block / io-qcow2-040 ERROR 0.30s exit status 1 677/840 qemu:block / io-qcow2-039 ERROR 0.32s exit status 1 679/840 qemu:block / io-qcow2-041 ERROR 0.32s exit status 1 680/840 qemu:block / io-qcow2-042 ERROR 0.31s exit status 1 681/840 qemu:block / io-qcow2-043 ERROR 0.32s exit status 1 682/840 qemu:block / io-qcow2-046 ERROR 0.32s exit status 1 683/840 qemu:block / io-qcow2-047 ERROR 0.32s exit status 1 684/840 qemu:block / io-qcow2-048 ERROR 0.31s exit status 1 685/840 qemu:block / io-qcow2-049 ERROR 0.32s exit status 1 686/840 qemu:block / io-qcow2-050 ERROR 0.32s exit status 1 687/840 qemu:block / io-qcow2-052 ERROR 0.30s exit status 1 688/840 qemu:block / io-qcow2-053 ERROR 0.32s exit status 1 689/840 qemu:block / io-qcow2-054 ERROR 0.32s exit status 1 690/840 qemu:block / io-qcow2-060 ERROR 0.30s exit status 1 691/840 qemu:block / io-qcow2-061 ERROR 0.33s exit status 1 692/840 qemu:block / io-qcow2-062 ERROR 0.34s exit status 1 693/840 qemu:block / io-qcow2-063 ERROR 0.31s exit status 1 694/840 qemu:block / io-qcow2-066 ERROR 0.35s exit status 1 695/840 qemu:block / io-qcow2-069 ERROR 0.33s exit status 1 696/840 qemu:block / io-qcow2-072 ERROR 0.30s exit status 1 697/840 qemu:block / io-qcow2-071 ERROR 0.34s exit status 1 698/840 qemu:block / io-qcow2-073 ERROR 0.37s exit status 1 699/840 qemu:block / io-qcow2-074 ERROR 0.33s exit status 1 700/840 qemu:block / io-qcow2-079 ERROR 0.32s exit status 1 701/840 qemu:block / io-qcow2-080 ERROR 0.36s exit status 1 702/840 qemu:block / io-qcow2-086 ERROR 0.33s exit status 1 703/840 qemu:block / io-qcow2-089 ERROR 0.35s exit status 1 704/840 qemu:block / io-qcow2-090 ERROR 0.35s exit status 1 705/840 qemu:block / io-qcow2-097 ERROR 0.32s exit status 1 706/840 qemu:block / io-qcow2-098 ERROR 0.30s exit status 1 707/840 qemu:block / io-qcow2-099 ERROR 0.32s exit status 1 708/840 qemu:block / io-qcow2-104 ERROR 0.32s exit status 1 709/840 qemu:block / io-qcow2-103 ERROR 0.34s exit status 1 710/840 qemu:block / io-qcow2-105 ERROR 0.33s exit status 1 711/840 qemu:block / io-qcow2-107 ERROR 0.34s exit status 1 712/840 qemu:block / io-qcow2-108 ERROR 0.31s exit status 1 713/840 qemu:block / io-qcow2-110 ERROR 0.32s exit status 1 714/840 qemu:block / io-qcow2-111 ERROR 0.32s exit status 1 715/840 qemu:block / io-qcow2-114 ERROR 0.31s exit status 1 716/840 qemu:block / io-qcow2-120 ERROR 0.30s exit status 1 717/840 qemu:block / io-qcow2-117 ERROR 0.34s exit status 1 719/840 qemu:block / io-qcow2-126 ERROR 0.30s exit status 1 720/840 qemu:block / io-qcow2-127 ERROR 0.35s exit status 1 718/840 qemu:qtest+qtest-s390x / qtest-s390x/netdev-socket ERROR 51.24s killed by signal 6 SIGABRT 721/840 qemu:block / io-qcow2-133 ERROR 0.34s exit status 1 722/840 qemu:block / io-qcow2-134 ERROR 0.34s exit status 1 723/840 qemu:block / io-qcow2-137 ERROR 0.32s exit status 1 724/840 qemu:block / io-qcow2-138 ERROR 0.32s exit status 1 725/840 qemu:block / io-qcow2-140 ERROR 0.37s exit status 1 726/840 qemu:block / io-qcow2-141 ERROR 0.33s exit status 1 727/840 qemu:block / io-qcow2-143 ERROR 0.34s exit status 1 728/840 qemu:block / io-qcow2-150 ERROR 0.33s exit status 1 729/840 qemu:block / io-qcow2-154 ERROR 0.33s exit status 1 730/840 qemu:block / io-qcow2-156 ERROR 0.38s exit status 1 732/840 qemu:block / io-qcow2-158 ERROR 0.35s exit status 1 733/840 qemu:block / io-qcow2-159 ERROR 0.33s exit status 1 734/840 qemu:block / io-qcow2-161 ERROR 0.40s exit status 1 735/840 qemu:block / io-qcow2-170 ERROR 0.57s exit status 1 736/840 qemu:block / io-qcow2-174 ERROR 0.46s exit status 1 737/840 qemu:block / io-qcow2-176 ERROR 0.47s exit status 1 738/840 qemu:block / io-qcow2-177 ERROR 0.63s exit status 1 739/840 qemu:block / io-qcow2-172 ERROR 0.69s exit status 1 740/840 qemu:block / io-qcow2-181 ERROR 0.36s exit status 1 741/840 qemu:block / io-qcow2-179 ERROR 0.50s exit status 1 742/840 qemu:block / io-qcow2-184 ERROR 0.54s exit status 1 743/840 qemu:block / io-qcow2-186 ERROR 0.38s exit status 1 744/840 qemu:block / io-qcow2-187 ERROR 0.40s exit status 1 745/840 qemu:block / io-qcow2-190 ERROR 0.43s exit status 1 746/840 qemu:block / io-qcow2-191 ERROR 0.37s exit status 1 747/840 qemu:block / io-qcow2-192 ERROR 0.35s exit status 1 748/840 qemu:block / io-qcow2-195 ERROR 0.35s exit status 1 749/840 qemu:block / io-qcow2-214 ERROR 0.32s exit status 1 750/840 qemu:block / io-qcow2-203 ERROR 0.39s exit status 1 751/840 qemu:block / io-qcow2-217 ERROR 0.38s exit status 1 752/840 qemu:block / io-qcow2-220 ERROR 0.38s exit status 1 753/840 qemu:block / io-qcow2-226 ERROR 0.41s exit status 1 754/840 qemu:block / io-qcow2-229 ERROR 0.37s exit status 1 755/840 qemu:block / io-qcow2-244 ERROR 0.34s exit status 1 756/840 qemu:block / io-qcow2-249 ERROR 0.34s exit status 1 757/840 qemu:block / io-qcow2-251 ERROR 0.35s exit status 1 758/840 qemu:block / io-qcow2-252 ERROR 0.35s exit status 1 759/840 qemu:block / io-qcow2-256 ERROR 0.44s exit status 1 760/840 qemu:block / io-qcow2-265 ERROR 0.33s exit status 1 761/840 qemu:block / io-qcow2-268 ERROR 0.35s exit status 1 762/840 qemu:block / io-qcow2-267 ERROR 0.42s exit status 1 763/840 qemu:block / io-qcow2-271 ERROR 0.34s exit status 1 764/840 qemu:block / io-qcow2-283 ERROR 0.33s exit status 1 765/840 qemu:block / io-qcow2-287 ERROR 0.37s exit status 1 766/840 qemu:block / io-qcow2-290 ERROR 0.35s exit status 1 767/840 qemu:block / io-qcow2-292 ERROR 0.34s exit status 1 768/840 qemu:block / io-qcow2-313 ERROR 0.35s exit status 1 769/840 qemu:block / io-qcow2-copy-before-write ERROR 0.37s exit status 1 770/840 qemu:block / io-qcow2-299 ERROR 0.46s exit status 1 771/840 qemu:block / io-qcow2-iothreads-commit-active ERROR 0.34s exit status 1 772/840 qemu:block / io-qcow2-detect-zeroes-registered-buf ERROR 0.41s exit status 1 773/840 qemu:block / io-qcow2-iothreads-resize ERROR 0.38s exit status 1 774/840 qemu:block / io-qcow2-nbd-multiconn ERROR 0.34s exit status 1 778/840 qemu:block / io-qcow2-nbd-qemu-allocation ERROR 0.34s exit status 1 782/840 qemu:block / io-qcow2-qemu-img-close-errors ERROR 0.37s exit status 1 787/840 qemu:block / io-qcow2-qsd-jobs ERROR 0.30s exit status 1 797/840 qemu:block / io-qcow2-regression-vhdx-log ERROR 0.35s exit status 1 It looks like I have really broke s390 (there are changes on my patches for s390) and qcow2 iotests (I know I am very good, but breaking every qcow2 iotest without touching qcow2 at all look strange). Now looking into aarch64: 501/841 qemu:qtest+qtest-s390x / qtest-s390x/qom-test ERROR 50.36s killed by signal 6 SIGABRT 523/841 qemu:qtest+qtest-s390x / qtest-s390x/test-hmp ERROR 50.07s killed by signal 6 SIGABRT 562/841 qemu:qtest+qtest-s390x / qtest-s390x/boot-serial-test ERROR 51.24s killed by signal 6 SIGABRT 563/841 qemu:qtest+qtest-s390x / qtest-s390x/qos-test ERROR 51.22s killed by signal 6 SIGABRT 823/841 qemu:qtest+qtest-s390x / qtest-s390x/device-plug-test ERROR 50.26s killed by signal 6 SIGABRT 822/841 qemu:qtest+qtest-s390x / qtest-s390x/cpu-plug-test ERROR 50.22s killed by signal 6 SIGABRT 824/841 qemu:qtest+qtest-s390x / qtest-s390x/machine-none-test ERROR 50.03s killed by signal 6 SIGABRT 821/841 qemu:qtest+qtest-s390x / qtest-s390x/test-netfilter ERROR 50.50s killed by signal 6 SIGABRT 825/841 qemu:qtest+qtest-s390x / qtest-s390x/device-introspect-test ERROR 50.10s killed by signal 6 SIGABRT 827/841 qemu:qtest+qtest-s390x / qtest-s390x/test-filter-redirector ERROR 50.39s killed by signal 6 SIGABRT 828/841 qemu:qtest+qtest-s390x / qtest-s390x/drive_del-test ERROR 50.36s killed by signal 6 SIGABRT 829/841 qemu:qtest+qtest-s390x / qtest-s390x/virtio-ccw-test ERROR 50.25s killed by signal 6 SIGABRT 826/841 qemu:qtest+qtest-s390x / qtest-s390x/test-filter-mirror ERROR 50.42s killed by signal 6 SIGABRT 830/841 qemu:qtest+qtest-s390x / qtest-s390x/qmp-cmd-test ERROR 51.61s killed by signal 6 SIGABRT 831/841 qemu:qtest+qtest-s390x / qtest-s390x/readconfig-test ERROR 50.96s killed by signal 6 SIGABRT 832/841 qemu:qtest+qtest-s390x / qtest-s390x/qmp-test ERROR 51.64s killed by signal 6 SIGABRT 833/841 qemu:qtest+qtest-s390x / qtest-s390x/netdev-socket ERROR 50.64s killed by signal 6 SIGABRT x64 freebsd: Useless report $ cirrus-run -v --show-build-log always .gitlab-ci.d/cirrus/$NAME.yml 01:18:03 build 5298808968445952: TRIGGERED build 5298808968445952: EXECUTING build 5298808968445952: EXECUTING build 5298808968445952: EXECUTING build 5298808968445952: EXECUTING build 5298808968445952: EXECUTING build 5298808968445952: EXECUTING build 5298808968445952: EXECUTING build 5298808968445952: EXECUTING build 5298808968445952: EXECUTING .... I will let this for last. So far: I run full qemu-iotets on x68_64, not the subset of make check: - passed all for raw - failed for qcow2 (130) Not being able to see what was going on, I tested on master, and I got the same failure: $ ./check 130 -qcow2 QEMU -- "/scratch/qemu/gcc/full/all/qemu-system-x86_64" -nodefaults -display none -accel qtest QEMU_IMG -- "/scratch/qemu/gcc/full/all/qemu-img" QEMU_IO -- "/scratch/qemu/gcc/full/all/qemu-io" --cache writeback --aio threads -f qcow2 QEMU_NBD -- "/scratch/qemu/gcc/full/all/qemu-nbd" IMGFMT -- qcow2 IMGPROTO -- file PLATFORM -- Linux/x86_64 fada.mitica 6.5.6-200.fc38.x86_64 TEST_DIR -- /scratch/qemu/gcc/full/all/tests/qemu-iotests/scratch SOCK_DIR -- /tmp/tmp_p9ha0gt GDB_OPTIONS -- VALGRIND_QEMU -- PRINT_QEMU_OUTPUT -- 130 fail [14:43:56] [14:43:57] 0.5s (last: 0.4s) output mismatch (see /scratch/qemu/gcc/full/all/tests/qemu-iotests/scratch/qcow2-file-130/130.out.bad) --- /mnt/code/qemu/full/tests/qemu-iotests/130.out +++ /scratch/qemu/gcc/full/all/tests/qemu-iotests/scratch/qcow2-file-130/130.out.bad @@ -11,18 +11,14 @@ QEMU X.Y.Z monitor - type 'help' for more information (qemu) commit testdisk (qemu) -image: TEST_DIR/t.IMGFMT -file format: IMGFMT -virtual size: 64 MiB (67108864 bytes) +qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Failed to get shared "write" lock +Is another process using the image [TEST_DIR/t.IMGFMT]? Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.orig backing_fmt=raw QEMU X.Y.Z monitor - type 'help' for more information (qemu) commit testdisk (qemu) -image: TEST_DIR/t.IMGFMT -file format: IMGFMT -virtual size: 64 MiB (67108864 bytes) -backing file: TEST_DIR/t.IMGFMT.orig -backing file format: raw +qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Failed to get shared "write" lock +Is another process using the image [TEST_DIR/t.IMGFMT]? So I guess there is some locking issues there, not related to my changes at all. Here it don't fails every time, but it fails something like 9 out of 10 times (in master, the same on my PULL request). On x86_64, the s390x emulated tests work perfectly. So so far I got: - they fail native on s390 - they fail emulated on aarch64 - they pass emulated on x86_64 Here I am, hunting for an s390x machine. After being burned on x86_64, I start with master, not with my PULL request: (main *)$ ./check 108 -qcow2 QEMU -- "/home/quintela/scratch/qemu/gcc/qemu/all/qemu-system-s390x" -nodefaults -display none -accel qtest QEMU_IMG -- "/home/quintela/scratch/qemu/gcc/qemu/all/qemu-img" QEMU_IO -- "/home/quintela/scratch/qemu/gcc/qemu/all/qemu-io" --cache writeback --aio threads -f qcow2 QEMU_NBD -- "/home/quintela/scratch/qemu/gcc/qemu/all/qemu-nbd" IMGFMT -- qcow2 IMGPROTO -- file PLATFORM -- Linux/s390x s390x-kvm-007.lab.eng.rdu2.redhat.com 5.14.0-284.38.1.el9_2.s390x TEST_DIR -- /home/quintela/scratch/qemu/gcc/qemu/all/tests/qemu-iotests/scratch SOCK_DIR -- /tmp/tmpls01m7ji GDB_OPTIONS -- VALGRIND_QEMU -- PRINT_QEMU_OUTPUT -- 108 fail [08:24:54] [08:24:56] 1.7s output mismatch (see /home/quintela/scratch/qemu/gcc/qemu/all/tests/qemu-iotests/scratch/qcow2-file-108/108.out.bad) --- /home/quintela/code/qemu/qemu/tests/qemu-iotests/108.out +++ /home/quintela/scratch/qemu/gcc/qemu/all/tests/qemu-iotests/scratch/qcow2-file-108/108.out.bad @@ -152,6 +152,8 @@ --- Rebuilding refcount structures on block devices --- +fuse: failed to exec fusermount3: No such file or directory +qemu-storage-daemon: --export fuse,id=fuse-export,node-name=export-node,mountpoint=/home/quintela/scratch/qemu/gcc/qemu/all/tests/qemu-iotests/scratch/qcow2-file-108/fuse-export,writable=on,growable=off,allow-other=off: Failed to mount FUSE session to export { "execute": "qmp_capabilities" } {"return": {}} { "execute": "blockdev-create", @@ -188,4 +190,6 @@ Double checking the fixed image now... No errors were found on the image. +cat: /home/quintela/scratch/qemu/gcc/qemu/all/tests/qemu-iotests/scratch/qcow2-file-108/qsd.pid: No such file or directory +/home/quintela/code/qemu/qemu/tests/qemu-iotests/108: line 396: kill: `': not a pid or valid job spec *** done Failures: 108 Failed 1 of 1 iotests So with further evidence, I will say that qemu-iotests failures are not due to my changes. /me goes to compile his PULL request on s390: Thread 1 "qemu-system-s39" received signal SIGSEGV, Segmentation fault. 0x0000aaaaab05b828 in qdev_class_add_legacy_property ( prop=0xaaaaab5fffe0 <savevm_s390_storage_keys>, dc=0xaaaaab88be60) at ../../../../../quintela/code/qemu/full/hw/core/qdev-properties.c:938 938 if (!prop->info->print && prop->info->get) { Missing separate debuginfos, use: dnf debuginfo-install libeconf-0.4.1-3.el9_2.aarch64 (gdb) bt #0 0x0000aaaaab05b828 in qdev_class_add_legacy_property (prop=0xaaaaab5fffe0 <savevm_s390_storage_keys>, dc=0xaaaaab88be60) at ../../../../../quintela/code/qemu/full/hw/core/qdev-properties.c:938 #1 device_class_set_props (dc=dc@entry=0xaaaaab88be60, props=props@entry=0xaaaaab5fff88 <s390_skeys_props>) at ../../../../../quintela/code/qemu/full/hw/core/qdev-properties.c:954 #2 0x0000aaaaaaf6a370 in s390_skeys_class_init (oc=<optimized out>, data=<optimized out>) at ../../../../../quintela/code/qemu/full/hw/s390x/s390-skeys.c:461 #3 0x0000aaaaab06222c in type_initialize (ti=0xaaaaab74ea60) at ../../../../../quintela/code/qemu/full/qom/object.c:1108 #4 object_class_foreach_tramp (key=<optimized out>, value=0xaaaaab74ea60, opaque=0xffffffffe498) at ../../../../../quintela/code/qemu/full/qom/object.c:1095 #5 0x0000fffff6fe92d8 in g_hash_table_foreach (hash_table=0xaaaaab70caa0 = {...}, func=0xaaaaab0621b0 <object_class_foreach_tramp>, user_data=0xffffffffe498) at ../glib/ghash.c:2065 #6 0x0000aaaaab062798 in object_class_foreach (opaque=<optimized out>, include_abstract=<optimized out>, implements_type=<optimized out>, fn=<optimized out>) at ../../../../../quintela/code/qemu/full/qom/object.c:87 #7 object_class_get_list (implements_type=implements_type@entry=0xaaaaab3cb670 "machine", include_abstract=include_abstract@entry=false) at ../../../../../quintela/code/qemu/full/qom/object.c:1174 #8 0x0000aaaaaaef30fc in select_machine (errp=<optimized out>, qdict=0xaaaaab773250) at ../../../../../quintela/code/qemu/full/system/vl.c:1644 #9 qemu_create_machine (qdict=0xaaaaab773250) at ../../../../../quintela/code/qemu/full/system/vl.c:2078 #10 qemu_init (argc=<optimized out>, argv=0xffffffffe7d8) at ../../../../../quintela/code/qemu/full/system/vl.c:3668 #11 0x0000aaaaaade7fbc in main (argc=<optimized out>, argv=<optimized out>) at ../../../../../quintela/code/qemu/full/system/main.c:47 OK, something fishing there. Will post PULL request without the vmstate_register() changes. What is weird to me is that it fails in s390x native, and aarch64 host with s390x tcg. But it works for x86_64 host and s390x tcg. Sniff. Later, Juan.