* [PULL 1/7] migration: Disable postcopy + multifd migration
2023-04-26 18:12 [PULL 0/7] Migration 20230426 patches Juan Quintela
@ 2023-04-26 18:12 ` Juan Quintela
2023-04-26 18:12 ` [PULL 2/7] MAINTAINERS: Add Leonardo and Peter as reviewers Juan Quintela
` (6 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Juan Quintela @ 2023-04-26 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Cleber Rosa, Juan Quintela, John Snow, Leonardo Bras,
Dr . David Alan Gilbert
From: Leonardo Bras <leobras@redhat.com>
Since the introduction of multifd, it's possible to perform a multifd
migration and finish it using postcopy.
A bug introduced by yank (fixed on cfc3bcf373) was previously preventing
a successful use of this migration scenario, and now thing should be
working on most scenarios.
But since there is not enough testing/support nor any reported users for
this scenario, we should disable this combination before it may cause any
problems for users.
Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Leonardo Bras <leobras@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/options.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/migration/options.c b/migration/options.c
index 8e8753d9be..dd97c99391 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -322,6 +322,11 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
error_setg(errp, "Postcopy is not compatible with ignore-shared");
return false;
}
+
+ if (new_caps[MIGRATION_CAPABILITY_MULTIFD]) {
+ error_setg(errp, "Postcopy is not yet compatible with multifd");
+ return false;
+ }
}
if (new_caps[MIGRATION_CAPABILITY_BACKGROUND_SNAPSHOT]) {
--
2.40.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 2/7] MAINTAINERS: Add Leonardo and Peter as reviewers
2023-04-26 18:12 [PULL 0/7] Migration 20230426 patches Juan Quintela
2023-04-26 18:12 ` [PULL 1/7] migration: Disable postcopy + multifd migration Juan Quintela
@ 2023-04-26 18:12 ` Juan Quintela
2023-04-26 18:12 ` [PULL 3/7] migration: Move migrate_use_tls() to options.c Juan Quintela
` (5 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Juan Quintela @ 2023-04-26 18:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Cleber Rosa, Juan Quintela, John Snow, Leonardo Bras
Now that David has stepped down with Migration maintainership,
Leonardo and Peter has volunteer to review the migration patches.
This way they got CC'd on every migration patch.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>
Acked-by: Leonardo Bras <leobras@redhat.com>
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 24154f5721..1aa50c0fe2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3150,6 +3150,8 @@ F: scripts/checkpatch.pl
Migration
M: Juan Quintela <quintela@redhat.com>
+R: Peter Xu <peterx@redhat.com>
+R: Leonardo Bras <leobras@redhat.com>
S: Maintained
F: hw/core/vmstate-if.c
F: include/hw/vmstate-if.h
--
2.40.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 3/7] migration: Move migrate_use_tls() to options.c
2023-04-26 18:12 [PULL 0/7] Migration 20230426 patches Juan Quintela
2023-04-26 18:12 ` [PULL 1/7] migration: Disable postcopy + multifd migration Juan Quintela
2023-04-26 18:12 ` [PULL 2/7] MAINTAINERS: Add Leonardo and Peter as reviewers Juan Quintela
@ 2023-04-26 18:12 ` Juan Quintela
2023-04-26 18:12 ` [PULL 4/7] migration: Move qmp_migrate_set_parameters() " Juan Quintela
` (4 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Juan Quintela @ 2023-04-26 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Cleber Rosa, Juan Quintela, John Snow, Leonardo Bras,
Vladimir Sementsov-Ogievskiy
Once there, rename it to migrate_tls() and make it return bool for
consistency.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
Fix typos found by fabiano
---
migration/migration.c | 9 ---------
migration/migration.h | 2 --
migration/options.c | 11 ++++++++++-
migration/options.h | 1 +
migration/tls.c | 3 ++-
5 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 53dd59f6f6..02c2355d0d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2177,15 +2177,6 @@ void qmp_migrate_continue(MigrationStatus state, Error **errp)
qemu_sem_post(&s->pause_sem);
}
-int migrate_use_tls(void)
-{
- MigrationState *s;
-
- s = migrate_get_current();
-
- return s->parameters.tls_creds && *s->parameters.tls_creds;
-}
-
/* migration thread support */
/*
* Something bad happened to the RP stream, mark an error
diff --git a/migration/migration.h b/migration/migration.h
index dcf906868d..2b71df8617 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -447,8 +447,6 @@ bool migration_is_blocked(Error **errp);
bool migration_in_postcopy(void);
MigrationState *migrate_get_current(void);
-int migrate_use_tls(void);
-
uint64_t ram_get_total_transferred_pages(void);
/* Sending on the return path - generic and then for each message type */
diff --git a/migration/options.c b/migration/options.c
index dd97c99391..943936a320 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -214,6 +214,15 @@ bool migrate_postcopy(void)
return migrate_postcopy_ram() || migrate_dirty_bitmaps();
}
+bool migrate_tls(void)
+{
+ MigrationState *s;
+
+ s = migrate_get_current();
+
+ return s->parameters.tls_creds && *s->parameters.tls_creds;
+}
+
typedef enum WriteTrackingSupport {
WT_SUPPORT_UNKNOWN = 0,
WT_SUPPORT_ABSENT,
@@ -368,7 +377,7 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
new_caps[MIGRATION_CAPABILITY_COMPRESS] ||
new_caps[MIGRATION_CAPABILITY_XBZRLE] ||
migrate_multifd_compression() ||
- migrate_use_tls())) {
+ migrate_tls())) {
error_setg(errp,
"Zero copy only available for non-compressed non-TLS multifd migration");
return false;
diff --git a/migration/options.h b/migration/options.h
index 1b78fa9f3d..13318a16c7 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -46,6 +46,7 @@ bool migrate_zero_copy_send(void);
*/
bool migrate_postcopy(void);
+bool migrate_tls(void);
/* capabilities helpers */
diff --git a/migration/tls.c b/migration/tls.c
index 4d2166a209..acd38e0b62 100644
--- a/migration/tls.c
+++ b/migration/tls.c
@@ -22,6 +22,7 @@
#include "channel.h"
#include "migration.h"
#include "tls.h"
+#include "options.h"
#include "crypto/tlscreds.h"
#include "qemu/error-report.h"
#include "qapi/error.h"
@@ -165,7 +166,7 @@ void migration_tls_channel_connect(MigrationState *s,
bool migrate_channel_requires_tls_upgrade(QIOChannel *ioc)
{
- if (!migrate_use_tls()) {
+ if (!migrate_tls()) {
return false;
}
--
2.40.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 4/7] migration: Move qmp_migrate_set_parameters() to options.c
2023-04-26 18:12 [PULL 0/7] Migration 20230426 patches Juan Quintela
` (2 preceding siblings ...)
2023-04-26 18:12 ` [PULL 3/7] migration: Move migrate_use_tls() to options.c Juan Quintela
@ 2023-04-26 18:12 ` Juan Quintela
2023-04-26 18:12 ` [PULL 5/7] migration: Allow postcopy_ram_supported_by_host() to report err Juan Quintela
` (3 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Juan Quintela @ 2023-04-26 18:12 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Cleber Rosa, Juan Quintela, John Snow, Leonardo Bras,
Vladimir Sementsov-Ogievskiy
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
migration/migration.c | 420 ------------------------------------------
migration/options.c | 418 +++++++++++++++++++++++++++++++++++++++++
migration/options.h | 11 ++
3 files changed, 429 insertions(+), 420 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 02c2355d0d..22e8586623 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -67,19 +67,10 @@
#define MAX_THROTTLE (128 << 20) /* Migration transfer speed throttling */
-/* Amount of time to allocate to each "chunk" of bandwidth-throttled
- * data. */
-#define BUFFER_DELAY 100
-#define XFER_LIMIT_RATIO (1000 / BUFFER_DELAY)
-
/* Time in milliseconds we are allowed to stop the source,
* for sending the last part */
#define DEFAULT_MIGRATE_SET_DOWNTIME 300
-/* Maximum migrate downtime set to 2000 seconds */
-#define MAX_MIGRATE_DOWNTIME_SECONDS 2000
-#define MAX_MIGRATE_DOWNTIME (MAX_MIGRATE_DOWNTIME_SECONDS * 1000)
-
/* Default compression thread count */
#define DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT 8
/* Default decompression thread count, usually decompression is at
@@ -1140,417 +1131,6 @@ MigrationInfo *qmp_query_migrate(Error **errp)
return info;
}
-/*
- * Check whether the parameters are valid. Error will be put into errp
- * (if provided). Return true if valid, otherwise false.
- */
-static bool migrate_params_check(MigrationParameters *params, Error **errp)
-{
- if (params->has_compress_level &&
- (params->compress_level > 9)) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "compress_level",
- "a value between 0 and 9");
- return false;
- }
-
- if (params->has_compress_threads && (params->compress_threads < 1)) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "compress_threads",
- "a value between 1 and 255");
- return false;
- }
-
- if (params->has_decompress_threads && (params->decompress_threads < 1)) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "decompress_threads",
- "a value between 1 and 255");
- return false;
- }
-
- if (params->has_throttle_trigger_threshold &&
- (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)) {
- 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)) {
- 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)) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "max_bandwidth",
- "an integer in the range of 0 to "stringify(SIZE_MAX)
- " bytes/second");
- return false;
- }
-
- if (params->has_downtime_limit &&
- (params->downtime_limit > MAX_MIGRATE_DOWNTIME)) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- "downtime_limit",
- "an integer in the range of 0 to "
- stringify(MAX_MIGRATE_DOWNTIME)" ms");
- return false;
- }
-
- /* x_checkpoint_delay is now always positive */
-
- if (params->has_multifd_channels && (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)) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_zlib_level",
- "a value between 0 and 9");
- return false;
- }
-
- if (params->has_multifd_zstd_level &&
- (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))) {
- 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)) {
- 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) {
- 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) {
- 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) {
- 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)) {
- 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)) {
- 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 && *params->tls_creds))) {
- error_setg(errp,
- "Zero copy only available for non-compressed non-TLS multifd migration");
- return false;
- }
-#endif
-
- return true;
-}
-
-static void migrate_params_test_apply(MigrateSetParameters *params,
- MigrationParameters *dest)
-{
- *dest = migrate_get_current()->parameters;
-
- /* TODO use QAPI_CLONE() instead of duplicating it inline */
-
- if (params->has_compress_level) {
- dest->compress_level = params->compress_level;
- }
-
- if (params->has_compress_threads) {
- dest->compress_threads = params->compress_threads;
- }
-
- if (params->has_compress_wait_thread) {
- dest->compress_wait_thread = params->compress_wait_thread;
- }
-
- if (params->has_decompress_threads) {
- dest->decompress_threads = params->decompress_threads;
- }
-
- if (params->has_throttle_trigger_threshold) {
- dest->throttle_trigger_threshold = params->throttle_trigger_threshold;
- }
-
- if (params->has_cpu_throttle_initial) {
- dest->cpu_throttle_initial = params->cpu_throttle_initial;
- }
-
- if (params->has_cpu_throttle_increment) {
- dest->cpu_throttle_increment = params->cpu_throttle_increment;
- }
-
- if (params->has_cpu_throttle_tailslow) {
- dest->cpu_throttle_tailslow = params->cpu_throttle_tailslow;
- }
-
- if (params->tls_creds) {
- assert(params->tls_creds->type == QTYPE_QSTRING);
- dest->tls_creds = params->tls_creds->u.s;
- }
-
- if (params->tls_hostname) {
- assert(params->tls_hostname->type == QTYPE_QSTRING);
- dest->tls_hostname = params->tls_hostname->u.s;
- }
-
- if (params->has_max_bandwidth) {
- dest->max_bandwidth = params->max_bandwidth;
- }
-
- if (params->has_downtime_limit) {
- dest->downtime_limit = params->downtime_limit;
- }
-
- if (params->has_x_checkpoint_delay) {
- dest->x_checkpoint_delay = params->x_checkpoint_delay;
- }
-
- if (params->has_block_incremental) {
- dest->block_incremental = params->block_incremental;
- }
- if (params->has_multifd_channels) {
- dest->multifd_channels = params->multifd_channels;
- }
- if (params->has_multifd_compression) {
- dest->multifd_compression = params->multifd_compression;
- }
- if (params->has_xbzrle_cache_size) {
- dest->xbzrle_cache_size = params->xbzrle_cache_size;
- }
- if (params->has_max_postcopy_bandwidth) {
- dest->max_postcopy_bandwidth = params->max_postcopy_bandwidth;
- }
- if (params->has_max_cpu_throttle) {
- dest->max_cpu_throttle = params->max_cpu_throttle;
- }
- if (params->has_announce_initial) {
- dest->announce_initial = params->announce_initial;
- }
- if (params->has_announce_max) {
- dest->announce_max = params->announce_max;
- }
- if (params->has_announce_rounds) {
- dest->announce_rounds = params->announce_rounds;
- }
- if (params->has_announce_step) {
- dest->announce_step = params->announce_step;
- }
-
- if (params->has_block_bitmap_mapping) {
- dest->has_block_bitmap_mapping = true;
- dest->block_bitmap_mapping = params->block_bitmap_mapping;
- }
-}
-
-static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
-{
- MigrationState *s = migrate_get_current();
-
- /* TODO use QAPI_CLONE() instead of duplicating it inline */
-
- if (params->has_compress_level) {
- s->parameters.compress_level = params->compress_level;
- }
-
- if (params->has_compress_threads) {
- s->parameters.compress_threads = params->compress_threads;
- }
-
- if (params->has_compress_wait_thread) {
- s->parameters.compress_wait_thread = params->compress_wait_thread;
- }
-
- if (params->has_decompress_threads) {
- s->parameters.decompress_threads = params->decompress_threads;
- }
-
- if (params->has_throttle_trigger_threshold) {
- s->parameters.throttle_trigger_threshold = params->throttle_trigger_threshold;
- }
-
- if (params->has_cpu_throttle_initial) {
- s->parameters.cpu_throttle_initial = params->cpu_throttle_initial;
- }
-
- if (params->has_cpu_throttle_increment) {
- s->parameters.cpu_throttle_increment = params->cpu_throttle_increment;
- }
-
- if (params->has_cpu_throttle_tailslow) {
- s->parameters.cpu_throttle_tailslow = params->cpu_throttle_tailslow;
- }
-
- 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);
- }
-
- 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);
- }
-
- 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);
- }
-
- if (params->has_max_bandwidth) {
- s->parameters.max_bandwidth = params->max_bandwidth;
- if (s->to_dst_file && !migration_in_postcopy()) {
- qemu_file_set_rate_limit(s->to_dst_file,
- s->parameters.max_bandwidth / XFER_LIMIT_RATIO);
- }
- }
-
- if (params->has_downtime_limit) {
- s->parameters.downtime_limit = params->downtime_limit;
- }
-
- if (params->has_x_checkpoint_delay) {
- s->parameters.x_checkpoint_delay = params->x_checkpoint_delay;
- if (migration_in_colo_state()) {
- colo_checkpoint_notify(s);
- }
- }
-
- if (params->has_block_incremental) {
- s->parameters.block_incremental = params->block_incremental;
- }
- if (params->has_multifd_channels) {
- s->parameters.multifd_channels = params->multifd_channels;
- }
- if (params->has_multifd_compression) {
- s->parameters.multifd_compression = params->multifd_compression;
- }
- 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()) {
- qemu_file_set_rate_limit(s->to_dst_file,
- s->parameters.max_postcopy_bandwidth / XFER_LIMIT_RATIO);
- }
- }
- if (params->has_max_cpu_throttle) {
- s->parameters.max_cpu_throttle = params->max_cpu_throttle;
- }
- if (params->has_announce_initial) {
- s->parameters.announce_initial = params->announce_initial;
- }
- if (params->has_announce_max) {
- s->parameters.announce_max = params->announce_max;
- }
- if (params->has_announce_rounds) {
- s->parameters.announce_rounds = params->announce_rounds;
- }
- if (params->has_announce_step) {
- s->parameters.announce_step = params->announce_step;
- }
-
- if (params->has_block_bitmap_mapping) {
- qapi_free_BitmapMigrationNodeAliasList(
- s->parameters.block_bitmap_mapping);
-
- s->parameters.has_block_bitmap_mapping = true;
- s->parameters.block_bitmap_mapping =
- QAPI_CLONE(BitmapMigrationNodeAliasList,
- params->block_bitmap_mapping);
- }
-}
-
-void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
-{
- MigrationParameters tmp;
-
- /* TODO Rewrite "" to null instead */
- 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("");
- }
- /* TODO Rewrite "" to null instead */
- 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("");
- }
-
- migrate_params_test_apply(params, &tmp);
-
- if (!migrate_params_check(&tmp, errp)) {
- /* Invalid parameter */
- return;
- }
-
- migrate_params_apply(params, errp);
-}
-
-
void qmp_migrate_start_postcopy(Error **errp)
{
MigrationState *s = migrate_get_current();
diff --git a/migration/options.c b/migration/options.c
index 943936a320..fe7d7754c4 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -12,17 +12,25 @@
*/
#include "qemu/osdep.h"
+#include "exec/target_page.h"
#include "qapi/clone-visitor.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-migration.h"
#include "qapi/qapi-visit-migration.h"
#include "qapi/qmp/qerror.h"
+#include "qapi/qmp/qnull.h"
#include "sysemu/runstate.h"
+#include "migration/colo.h"
#include "migration/misc.h"
#include "migration.h"
+#include "qemu-file.h"
#include "ram.h"
#include "options.h"
+/* Maximum migrate downtime set to 2000 seconds */
+#define MAX_MIGRATE_DOWNTIME_SECONDS 2000
+#define MAX_MIGRATE_DOWNTIME (MAX_MIGRATE_DOWNTIME_SECONDS * 1000)
+
bool migrate_auto_converge(void)
{
MigrationState *s;
@@ -734,3 +742,413 @@ MigrationParameters *qmp_query_migrate_parameters(Error **errp)
return params;
}
+
+/*
+ * Check whether the parameters are valid. Error will be put into errp
+ * (if provided). Return true if valid, otherwise false.
+ */
+bool migrate_params_check(MigrationParameters *params, Error **errp)
+{
+ if (params->has_compress_level &&
+ (params->compress_level > 9)) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "compress_level",
+ "a value between 0 and 9");
+ return false;
+ }
+
+ if (params->has_compress_threads && (params->compress_threads < 1)) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+ "compress_threads",
+ "a value between 1 and 255");
+ return false;
+ }
+
+ if (params->has_decompress_threads && (params->decompress_threads < 1)) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+ "decompress_threads",
+ "a value between 1 and 255");
+ return false;
+ }
+
+ if (params->has_throttle_trigger_threshold &&
+ (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)) {
+ 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)) {
+ 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)) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+ "max_bandwidth",
+ "an integer in the range of 0 to "stringify(SIZE_MAX)
+ " bytes/second");
+ return false;
+ }
+
+ if (params->has_downtime_limit &&
+ (params->downtime_limit > MAX_MIGRATE_DOWNTIME)) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+ "downtime_limit",
+ "an integer in the range of 0 to "
+ stringify(MAX_MIGRATE_DOWNTIME)" ms");
+ return false;
+ }
+
+ /* x_checkpoint_delay is now always positive */
+
+ if (params->has_multifd_channels && (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)) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "multifd_zlib_level",
+ "a value between 0 and 9");
+ return false;
+ }
+
+ if (params->has_multifd_zstd_level &&
+ (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))) {
+ 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)) {
+ 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) {
+ 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) {
+ 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) {
+ 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)) {
+ 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)) {
+ 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 && *params->tls_creds))) {
+ error_setg(errp,
+ "Zero copy only available for non-compressed non-TLS multifd migration");
+ return false;
+ }
+#endif
+
+ return true;
+}
+
+static void migrate_params_test_apply(MigrateSetParameters *params,
+ MigrationParameters *dest)
+{
+ *dest = migrate_get_current()->parameters;
+
+ /* TODO use QAPI_CLONE() instead of duplicating it inline */
+
+ if (params->has_compress_level) {
+ dest->compress_level = params->compress_level;
+ }
+
+ if (params->has_compress_threads) {
+ dest->compress_threads = params->compress_threads;
+ }
+
+ if (params->has_compress_wait_thread) {
+ dest->compress_wait_thread = params->compress_wait_thread;
+ }
+
+ if (params->has_decompress_threads) {
+ dest->decompress_threads = params->decompress_threads;
+ }
+
+ if (params->has_throttle_trigger_threshold) {
+ dest->throttle_trigger_threshold = params->throttle_trigger_threshold;
+ }
+
+ if (params->has_cpu_throttle_initial) {
+ dest->cpu_throttle_initial = params->cpu_throttle_initial;
+ }
+
+ if (params->has_cpu_throttle_increment) {
+ dest->cpu_throttle_increment = params->cpu_throttle_increment;
+ }
+
+ if (params->has_cpu_throttle_tailslow) {
+ dest->cpu_throttle_tailslow = params->cpu_throttle_tailslow;
+ }
+
+ if (params->tls_creds) {
+ assert(params->tls_creds->type == QTYPE_QSTRING);
+ dest->tls_creds = params->tls_creds->u.s;
+ }
+
+ if (params->tls_hostname) {
+ assert(params->tls_hostname->type == QTYPE_QSTRING);
+ dest->tls_hostname = params->tls_hostname->u.s;
+ }
+
+ if (params->has_max_bandwidth) {
+ dest->max_bandwidth = params->max_bandwidth;
+ }
+
+ if (params->has_downtime_limit) {
+ dest->downtime_limit = params->downtime_limit;
+ }
+
+ if (params->has_x_checkpoint_delay) {
+ dest->x_checkpoint_delay = params->x_checkpoint_delay;
+ }
+
+ if (params->has_block_incremental) {
+ dest->block_incremental = params->block_incremental;
+ }
+ if (params->has_multifd_channels) {
+ dest->multifd_channels = params->multifd_channels;
+ }
+ if (params->has_multifd_compression) {
+ dest->multifd_compression = params->multifd_compression;
+ }
+ if (params->has_xbzrle_cache_size) {
+ dest->xbzrle_cache_size = params->xbzrle_cache_size;
+ }
+ if (params->has_max_postcopy_bandwidth) {
+ dest->max_postcopy_bandwidth = params->max_postcopy_bandwidth;
+ }
+ if (params->has_max_cpu_throttle) {
+ dest->max_cpu_throttle = params->max_cpu_throttle;
+ }
+ if (params->has_announce_initial) {
+ dest->announce_initial = params->announce_initial;
+ }
+ if (params->has_announce_max) {
+ dest->announce_max = params->announce_max;
+ }
+ if (params->has_announce_rounds) {
+ dest->announce_rounds = params->announce_rounds;
+ }
+ if (params->has_announce_step) {
+ dest->announce_step = params->announce_step;
+ }
+
+ if (params->has_block_bitmap_mapping) {
+ dest->has_block_bitmap_mapping = true;
+ dest->block_bitmap_mapping = params->block_bitmap_mapping;
+ }
+}
+
+static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
+{
+ MigrationState *s = migrate_get_current();
+
+ /* TODO use QAPI_CLONE() instead of duplicating it inline */
+
+ if (params->has_compress_level) {
+ s->parameters.compress_level = params->compress_level;
+ }
+
+ if (params->has_compress_threads) {
+ s->parameters.compress_threads = params->compress_threads;
+ }
+
+ if (params->has_compress_wait_thread) {
+ s->parameters.compress_wait_thread = params->compress_wait_thread;
+ }
+
+ if (params->has_decompress_threads) {
+ s->parameters.decompress_threads = params->decompress_threads;
+ }
+
+ if (params->has_throttle_trigger_threshold) {
+ s->parameters.throttle_trigger_threshold = params->throttle_trigger_threshold;
+ }
+
+ if (params->has_cpu_throttle_initial) {
+ s->parameters.cpu_throttle_initial = params->cpu_throttle_initial;
+ }
+
+ if (params->has_cpu_throttle_increment) {
+ s->parameters.cpu_throttle_increment = params->cpu_throttle_increment;
+ }
+
+ if (params->has_cpu_throttle_tailslow) {
+ s->parameters.cpu_throttle_tailslow = params->cpu_throttle_tailslow;
+ }
+
+ 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);
+ }
+
+ 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);
+ }
+
+ 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);
+ }
+
+ if (params->has_max_bandwidth) {
+ s->parameters.max_bandwidth = params->max_bandwidth;
+ if (s->to_dst_file && !migration_in_postcopy()) {
+ qemu_file_set_rate_limit(s->to_dst_file,
+ s->parameters.max_bandwidth / XFER_LIMIT_RATIO);
+ }
+ }
+
+ if (params->has_downtime_limit) {
+ s->parameters.downtime_limit = params->downtime_limit;
+ }
+
+ if (params->has_x_checkpoint_delay) {
+ s->parameters.x_checkpoint_delay = params->x_checkpoint_delay;
+ if (migration_in_colo_state()) {
+ colo_checkpoint_notify(s);
+ }
+ }
+
+ if (params->has_block_incremental) {
+ s->parameters.block_incremental = params->block_incremental;
+ }
+ if (params->has_multifd_channels) {
+ s->parameters.multifd_channels = params->multifd_channels;
+ }
+ if (params->has_multifd_compression) {
+ s->parameters.multifd_compression = params->multifd_compression;
+ }
+ 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()) {
+ qemu_file_set_rate_limit(s->to_dst_file,
+ s->parameters.max_postcopy_bandwidth / XFER_LIMIT_RATIO);
+ }
+ }
+ if (params->has_max_cpu_throttle) {
+ s->parameters.max_cpu_throttle = params->max_cpu_throttle;
+ }
+ if (params->has_announce_initial) {
+ s->parameters.announce_initial = params->announce_initial;
+ }
+ if (params->has_announce_max) {
+ s->parameters.announce_max = params->announce_max;
+ }
+ if (params->has_announce_rounds) {
+ s->parameters.announce_rounds = params->announce_rounds;
+ }
+ if (params->has_announce_step) {
+ s->parameters.announce_step = params->announce_step;
+ }
+
+ if (params->has_block_bitmap_mapping) {
+ qapi_free_BitmapMigrationNodeAliasList(
+ s->parameters.block_bitmap_mapping);
+
+ s->parameters.has_block_bitmap_mapping = true;
+ s->parameters.block_bitmap_mapping =
+ QAPI_CLONE(BitmapMigrationNodeAliasList,
+ params->block_bitmap_mapping);
+ }
+}
+
+void qmp_migrate_set_parameters(MigrateSetParameters *params, Error **errp)
+{
+ MigrationParameters tmp;
+
+ /* TODO Rewrite "" to null instead */
+ 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("");
+ }
+ /* TODO Rewrite "" to null instead */
+ 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("");
+ }
+
+ migrate_params_test_apply(params, &tmp);
+
+ if (!migrate_params_check(&tmp, errp)) {
+ /* Invalid parameter */
+ return;
+ }
+
+ migrate_params_apply(params, errp);
+}
diff --git a/migration/options.h b/migration/options.h
index 13318a16c7..89067e59a0 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -14,6 +14,13 @@
#ifndef QEMU_MIGRATION_OPTIONS_H
#define QEMU_MIGRATION_OPTIONS_H
+/* constants */
+
+/* Amount of time to allocate to each "chunk" of bandwidth-throttled
+ * data. */
+#define BUFFER_DELAY 100
+#define XFER_LIMIT_RATIO (1000 / BUFFER_DELAY)
+
/* capabilities */
bool migrate_auto_converge(void);
@@ -74,4 +81,8 @@ int migrate_multifd_zstd_level(void);
uint8_t migrate_throttle_trigger_threshold(void);
uint64_t migrate_xbzrle_cache_size(void);
+/* parameters helpers */
+
+bool migrate_params_check(MigrationParameters *params, Error **errp);
+
#endif
--
2.40.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 5/7] migration: Allow postcopy_ram_supported_by_host() to report err
2023-04-26 18:12 [PULL 0/7] Migration 20230426 patches Juan Quintela
` (3 preceding siblings ...)
2023-04-26 18:12 ` [PULL 4/7] migration: Move qmp_migrate_set_parameters() " Juan Quintela
@ 2023-04-26 18:12 ` Juan Quintela
2023-04-26 18:12 ` [PULL 6/7] migration/vmstate-dump: Dump array size too as "num" Juan Quintela
` (2 subsequent siblings)
7 siblings, 0 replies; 11+ messages in thread
From: Juan Quintela @ 2023-04-26 18:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Cleber Rosa, Juan Quintela, John Snow, Leonardo Bras
From: Peter Xu <peterx@redhat.com>
Instead of print it to STDERR, bring the error upwards so that it can be
reported via QMP responses.
E.g.:
{ "execute": "migrate-set-capabilities" ,
"arguments": { "capabilities":
[ { "capability": "postcopy-ram", "state": true } ] } }
{ "error":
{ "class": "GenericError",
"desc": "Postcopy is not supported: Host backend files need to be TMPFS
or HUGETLBFS only" } }
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/options.c | 8 ++----
migration/postcopy-ram.c | 59 ++++++++++++++++++++++------------------
migration/postcopy-ram.h | 3 +-
migration/savevm.c | 3 +-
4 files changed, 39 insertions(+), 34 deletions(-)
diff --git a/migration/options.c b/migration/options.c
index fe7d7754c4..c6030587cf 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -302,6 +302,7 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
{
MigrationIncomingState *mis = migration_incoming_get_current();
+ ERRP_GUARD();
#ifndef CONFIG_LIVE_BLOCK_MIGRATION
if (new_caps[MIGRATION_CAPABILITY_BLOCK]) {
error_setg(errp, "QEMU compiled without old-style (blk/-b, inc/-i) "
@@ -327,11 +328,8 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
*/
if (!old_caps[MIGRATION_CAPABILITY_POSTCOPY_RAM] &&
runstate_check(RUN_STATE_INMIGRATE) &&
- !postcopy_ram_supported_by_host(mis)) {
- /* postcopy_ram_supported_by_host will have emitted a more
- * detailed message
- */
- error_setg(errp, "Postcopy is not supported");
+ !postcopy_ram_supported_by_host(mis, errp)) {
+ error_prepend(errp, "Postcopy is not supported: ");
return false;
}
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 0711500036..7c280480c2 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -283,11 +283,13 @@ static bool request_ufd_features(int ufd, uint64_t features)
return true;
}
-static bool ufd_check_and_apply(int ufd, MigrationIncomingState *mis)
+static bool ufd_check_and_apply(int ufd, MigrationIncomingState *mis,
+ Error **errp)
{
uint64_t asked_features = 0;
static uint64_t supported_features;
+ ERRP_GUARD();
/*
* it's not possible to
* request UFFD_API twice per one fd
@@ -295,7 +297,7 @@ static bool ufd_check_and_apply(int ufd, MigrationIncomingState *mis)
*/
if (!supported_features) {
if (!receive_ufd_features(&supported_features)) {
- error_report("%s failed", __func__);
+ error_setg(errp, "Userfault feature detection failed");
return false;
}
}
@@ -317,8 +319,7 @@ static bool ufd_check_and_apply(int ufd, MigrationIncomingState *mis)
* userfault file descriptor
*/
if (!request_ufd_features(ufd, asked_features)) {
- error_report("%s failed: features %" PRIu64, __func__,
- asked_features);
+ error_setg(errp, "Failed features %" PRIu64, asked_features);
return false;
}
@@ -329,7 +330,8 @@ static bool ufd_check_and_apply(int ufd, MigrationIncomingState *mis)
have_hp = supported_features & UFFD_FEATURE_MISSING_HUGETLBFS;
#endif
if (!have_hp) {
- error_report("Userfault on this host does not support huge pages");
+ error_setg(errp,
+ "Userfault on this host does not support huge pages");
return false;
}
}
@@ -338,7 +340,7 @@ static bool ufd_check_and_apply(int ufd, MigrationIncomingState *mis)
/* Callback from postcopy_ram_supported_by_host block iterator.
*/
-static int test_ramblock_postcopiable(RAMBlock *rb)
+static int test_ramblock_postcopiable(RAMBlock *rb, Error **errp)
{
const char *block_name = qemu_ram_get_idstr(rb);
ram_addr_t length = qemu_ram_get_used_length(rb);
@@ -346,16 +348,18 @@ static int test_ramblock_postcopiable(RAMBlock *rb)
QemuFsType fs;
if (length % pagesize) {
- error_report("Postcopy requires RAM blocks to be a page size multiple,"
- " block %s is 0x" RAM_ADDR_FMT " bytes with a "
- "page size of 0x%zx", block_name, length, pagesize);
+ error_setg(errp,
+ "Postcopy requires RAM blocks to be a page size multiple,"
+ " block %s is 0x" RAM_ADDR_FMT " bytes with a "
+ "page size of 0x%zx", block_name, length, pagesize);
return 1;
}
if (rb->fd >= 0) {
fs = qemu_fd_getfs(rb->fd);
if (fs != QEMU_FS_TYPE_TMPFS && fs != QEMU_FS_TYPE_HUGETLBFS) {
- error_report("Host backend files need to be TMPFS or HUGETLBFS only");
+ error_setg(errp,
+ "Host backend files need to be TMPFS or HUGETLBFS only");
return 1;
}
}
@@ -368,7 +372,8 @@ static int test_ramblock_postcopiable(RAMBlock *rb)
* normally fine since if the postcopy succeeds it gets turned back on at the
* end.
*/
-bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
+bool postcopy_ram_supported_by_host(MigrationIncomingState *mis,
+ Error **errp)
{
long pagesize = qemu_real_host_page_size();
int ufd = -1;
@@ -377,29 +382,27 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
struct uffdio_register reg_struct;
struct uffdio_range range_struct;
uint64_t feature_mask;
- Error *local_err = NULL;
RAMBlock *block;
+ ERRP_GUARD();
if (qemu_target_page_size() > pagesize) {
- error_report("Target page size bigger than host page size");
+ error_setg(errp, "Target page size bigger than host page size");
goto out;
}
ufd = uffd_open(O_CLOEXEC);
if (ufd == -1) {
- error_report("%s: userfaultfd not available: %s", __func__,
- strerror(errno));
+ error_setg(errp, "Userfaultfd not available: %s", strerror(errno));
goto out;
}
/* Give devices a chance to object */
- if (postcopy_notify(POSTCOPY_NOTIFY_PROBE, &local_err)) {
- error_report_err(local_err);
+ if (postcopy_notify(POSTCOPY_NOTIFY_PROBE, errp)) {
goto out;
}
/* Version and features check */
- if (!ufd_check_and_apply(ufd, mis)) {
+ if (!ufd_check_and_apply(ufd, mis, errp)) {
goto out;
}
@@ -417,7 +420,7 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
* affect in reality, or we can revisit.
*/
RAMBLOCK_FOREACH(block) {
- if (test_ramblock_postcopiable(block)) {
+ if (test_ramblock_postcopiable(block, errp)) {
goto out;
}
}
@@ -427,7 +430,7 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
* it was enabled.
*/
if (munlockall()) {
- error_report("%s: munlockall: %s", __func__, strerror(errno));
+ error_setg(errp, "munlockall() failed: %s", strerror(errno));
goto out;
}
@@ -439,8 +442,7 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
testarea = mmap(NULL, pagesize, PROT_READ | PROT_WRITE, MAP_PRIVATE |
MAP_ANONYMOUS, -1, 0);
if (testarea == MAP_FAILED) {
- error_report("%s: Failed to map test area: %s", __func__,
- strerror(errno));
+ error_setg(errp, "Failed to map test area: %s", strerror(errno));
goto out;
}
g_assert(QEMU_PTR_IS_ALIGNED(testarea, pagesize));
@@ -450,14 +452,14 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
reg_struct.mode = UFFDIO_REGISTER_MODE_MISSING;
if (ioctl(ufd, UFFDIO_REGISTER, ®_struct)) {
- error_report("%s userfault register: %s", __func__, strerror(errno));
+ error_setg(errp, "UFFDIO_REGISTER failed: %s", strerror(errno));
goto out;
}
range_struct.start = (uintptr_t)testarea;
range_struct.len = pagesize;
if (ioctl(ufd, UFFDIO_UNREGISTER, &range_struct)) {
- error_report("%s userfault unregister: %s", __func__, strerror(errno));
+ error_setg(errp, "UFFDIO_UNREGISTER failed: %s", strerror(errno));
goto out;
}
@@ -465,8 +467,8 @@ bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
(__u64)1 << _UFFDIO_COPY |
(__u64)1 << _UFFDIO_ZEROPAGE;
if ((reg_struct.ioctls & feature_mask) != feature_mask) {
- error_report("Missing userfault map features: %" PRIx64,
- (uint64_t)(~reg_struct.ioctls & feature_mask));
+ error_setg(errp, "Missing userfault map features: %" PRIx64,
+ (uint64_t)(~reg_struct.ioctls & feature_mask));
goto out;
}
@@ -1188,6 +1190,8 @@ static int postcopy_temp_pages_setup(MigrationIncomingState *mis)
int postcopy_ram_incoming_setup(MigrationIncomingState *mis)
{
+ Error *local_err = NULL;
+
/* Open the fd for the kernel to give us userfaults */
mis->userfault_fd = uffd_open(O_CLOEXEC | O_NONBLOCK);
if (mis->userfault_fd == -1) {
@@ -1200,7 +1204,8 @@ int postcopy_ram_incoming_setup(MigrationIncomingState *mis)
* Although the host check already tested the API, we need to
* do the check again as an ABI handshake on the new fd.
*/
- if (!ufd_check_and_apply(mis->userfault_fd, mis)) {
+ if (!ufd_check_and_apply(mis->userfault_fd, mis, &local_err)) {
+ error_report_err(local_err);
return -1;
}
diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
index b4867a32d5..442ab89752 100644
--- a/migration/postcopy-ram.h
+++ b/migration/postcopy-ram.h
@@ -14,7 +14,8 @@
#define QEMU_POSTCOPY_RAM_H
/* Return true if the host supports everything we need to do postcopy-ram */
-bool postcopy_ram_supported_by_host(MigrationIncomingState *mis);
+bool postcopy_ram_supported_by_host(MigrationIncomingState *mis,
+ Error **errp);
/*
* Make all of RAM sensitive to accesses to areas that haven't yet been written
diff --git a/migration/savevm.c b/migration/savevm.c
index 9671211339..211eff3a8b 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1753,7 +1753,8 @@ static int loadvm_postcopy_handle_advise(MigrationIncomingState *mis,
return -EINVAL;
}
- if (!postcopy_ram_supported_by_host(mis)) {
+ if (!postcopy_ram_supported_by_host(mis, &local_err)) {
+ error_report_err(local_err);
postcopy_state_set(POSTCOPY_INCOMING_NONE);
return -1;
}
--
2.40.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 6/7] migration/vmstate-dump: Dump array size too as "num"
2023-04-26 18:12 [PULL 0/7] Migration 20230426 patches Juan Quintela
` (4 preceding siblings ...)
2023-04-26 18:12 ` [PULL 5/7] migration: Allow postcopy_ram_supported_by_host() to report err Juan Quintela
@ 2023-04-26 18:12 ` Juan Quintela
2023-04-26 18:12 ` [PULL 7/7] vmstate-static-checker: Recognize "num" field Juan Quintela
2023-04-27 7:51 ` [PULL 0/7] Migration 20230426 patches Richard Henderson
7 siblings, 0 replies; 11+ messages in thread
From: Juan Quintela @ 2023-04-26 18:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Cleber Rosa, Juan Quintela, John Snow, Leonardo Bras
From: Peter Xu <peterx@redhat.com>
For VMS_ARRAY typed vmsd fields, also dump the number of entries in the
array in -vmstate-dump.
Without such information, vmstate static checker can report false negatives
of incompatible vmsd on VMS_ARRAY typed fields, when the src/dst do not
have the same type of array defined. It's because in the checker we only
check against size of fields within a VMSD field.
One example: e1000e used to have a field defined as a boolean array with 5
entries, then removed it and replaced it with UNUSED (in 31e3f318c8b535):
- VMSTATE_BOOL_ARRAY(core.eitr_intr_pending, E1000EState,
- E1000E_MSIX_VEC_NUM),
+ VMSTATE_UNUSED(E1000E_MSIX_VEC_NUM),
It's a legal replacement but vmstate static checker is not happy with it,
because it checks only against the "size" field between the two
fields (here one is BOOL_ARRAY, the other is UNUSED):
For BOOL_ARRAY:
{
"field": "core.eitr_intr_pending",
"version_id": 0,
"field_exists": false,
"size": 1
},
For UNUSED:
{
"field": "unused",
"version_id": 0,
"field_exists": false,
"size": 5
},
It's not the script to blame because there's just not enough information
dumped to show the total size of the entry for an array. Add it.
Note that this will not break old vmstate checker because the field will
just be ignored.
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
migration/savevm.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/migration/savevm.c b/migration/savevm.c
index 211eff3a8b..a9181b444b 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -536,6 +536,9 @@ static void dump_vmstate_vmsf(FILE *out_file, const VMStateField *field,
field->version_id);
fprintf(out_file, "%*s\"field_exists\": %s,\n", indent, "",
field->field_exists ? "true" : "false");
+ if (field->flags & VMS_ARRAY) {
+ fprintf(out_file, "%*s\"num\": %d,\n", indent, "", field->num);
+ }
fprintf(out_file, "%*s\"size\": %zu", indent, "", field->size);
if (field->vmsd != NULL) {
fprintf(out_file, ",\n");
--
2.40.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PULL 7/7] vmstate-static-checker: Recognize "num" field
2023-04-26 18:12 [PULL 0/7] Migration 20230426 patches Juan Quintela
` (5 preceding siblings ...)
2023-04-26 18:12 ` [PULL 6/7] migration/vmstate-dump: Dump array size too as "num" Juan Quintela
@ 2023-04-26 18:12 ` Juan Quintela
2023-04-27 7:51 ` [PULL 0/7] Migration 20230426 patches Richard Henderson
7 siblings, 0 replies; 11+ messages in thread
From: Juan Quintela @ 2023-04-26 18:12 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Cleber Rosa, Juan Quintela, John Snow, Leonardo Bras
From: Peter Xu <peterx@redhat.com>
Recognize this field for VMS_ARRAY typed vmsd fields, then we can do proper
size matching with previous patch.
Note that this is compatible with old -dump-vmstate output, because when
"num" is not there we'll still use the old "size" only.
Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
scripts/vmstate-static-checker.py | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/scripts/vmstate-static-checker.py b/scripts/vmstate-static-checker.py
index dfeee8231a..9c0e6b81f2 100755
--- a/scripts/vmstate-static-checker.py
+++ b/scripts/vmstate-static-checker.py
@@ -134,6 +134,11 @@ def exists_in_substruct(fields, item):
return check_fields_match(fields["Description"]["name"],
substruct_fields[0]["field"], item)
+def size_total(entry):
+ size = entry["size"]
+ if "num" not in entry:
+ return size
+ return size * entry["num"]
def check_fields(src_fields, dest_fields, desc, sec):
# This function checks for all the fields in a section. If some
@@ -249,17 +254,19 @@ def check_fields(src_fields, dest_fields, desc, sec):
continue
if s_item["field"] == "unused" or d_item["field"] == "unused":
- if s_item["size"] == d_item["size"]:
+ s_size = size_total(s_item)
+ d_size = size_total(d_item)
+ if s_size == d_size:
continue
if d_item["field"] == "unused":
advance_dest = False
- unused_count = d_item["size"] - s_item["size"]
+ unused_count = d_size - s_size;
continue
if s_item["field"] == "unused":
advance_src = False
- unused_count = s_item["size"] - d_item["size"]
+ unused_count = s_size - d_size
continue
print("Section \"" + sec + "\",", end=' ')
--
2.40.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PULL 0/7] Migration 20230426 patches
2023-04-26 18:12 [PULL 0/7] Migration 20230426 patches Juan Quintela
` (6 preceding siblings ...)
2023-04-26 18:12 ` [PULL 7/7] vmstate-static-checker: Recognize "num" field Juan Quintela
@ 2023-04-27 7:51 ` Richard Henderson
2023-04-27 8:19 ` Juan Quintela
7 siblings, 1 reply; 11+ messages in thread
From: Richard Henderson @ 2023-04-27 7:51 UTC (permalink / raw)
To: Juan Quintela, qemu-devel; +Cc: Peter Xu, Cleber Rosa, John Snow, Leonardo Bras
On 4/26/23 19:12, Juan Quintela wrote:
> The following changes since commit 9c894df3a37d675652390f7dbbe2f65b7bad7efa:
>
> migration: Create migrate_max_bandwidth() function (2023-04-24 15:01:47 +0200)
>
> are available in the Git repository at:
>
> https://gitlab.com/juan.quintela/qemu.git tags/migration-20230426-pull-request
>
> for you to fetch changes up to 7b5cd8ff519e9fe3df6cda65428a6f1aa28a6ced:
>
> vmstate-static-checker: Recognize "num" field (2023-04-26 19:17:55 +0200)
>
> ----------------------------------------------------------------
> Migration Pull request
>
> Hi
>
> This PULL request is on top of migration-20230424 already queued by
> Richard.
>
> It contains:
> - MAINTAINERS: make peter and leo reviewers for migration (juan)
> - Disable postcopy + multifd together. It needs at least to call
> send_sync before it will work. (juan)
> - Improve postcopy error messages (peter)
> - vmstate checker: Compare sizes of arrays correctly (peter)
> - Move more capability functions to options.c (juan)
>
> Please, apply.
Build failures:
https://gitlab.com/qemu-project/qemu/-/jobs/4187732684#L2303
../migration/postcopy-ram.c:1368:6: error: conflicting types for
'postcopy_ram_supported_by_host'; have '_Bool(MigrationIncomingState *)'
1368 | bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../migration/migration.h:27,
from ../migration/postcopy-ram.c:22:
../migration/postcopy-ram.h:17:6: note: previous declaration of
'postcopy_ram_supported_by_host' with type '_Bool(MigrationIncomingState *, Error **)'
17 | bool postcopy_ram_supported_by_host(MigrationIncomingState *mis,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
r~
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PULL 0/7] Migration 20230426 patches
2023-04-27 7:51 ` [PULL 0/7] Migration 20230426 patches Richard Henderson
@ 2023-04-27 8:19 ` Juan Quintela
0 siblings, 0 replies; 11+ messages in thread
From: Juan Quintela @ 2023-04-27 8:19 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel, Peter Xu, Cleber Rosa, John Snow, Leonardo Bras
Richard Henderson <richard.henderson@linaro.org> wrote:
> On 4/26/23 19:12, Juan Quintela wrote:
>> The following changes since commit 9c894df3a37d675652390f7dbbe2f65b7bad7efa:
>> migration: Create migrate_max_bandwidth() function (2023-04-24
>> 15:01:47 +0200)
>> are available in the Git repository at:
>> https://gitlab.com/juan.quintela/qemu.git
>> tags/migration-20230426-pull-request
>> for you to fetch changes up to
>> 7b5cd8ff519e9fe3df6cda65428a6f1aa28a6ced:
>> vmstate-static-checker: Recognize "num" field (2023-04-26
>> 19:17:55 +0200)
>> ----------------------------------------------------------------
>> Migration Pull request
>> Hi
>> This PULL request is on top of migration-20230424 already queued by
>> Richard.
>> It contains:
>> - MAINTAINERS: make peter and leo reviewers for migration (juan)
>> - Disable postcopy + multifd together. It needs at least to call
>> send_sync before it will work. (juan)
>> - Improve postcopy error messages (peter)
>> - vmstate checker: Compare sizes of arrays correctly (peter)
>> - Move more capability functions to options.c (juan)
>> Please, apply.
>
> Build failures:
>
> https://gitlab.com/qemu-project/qemu/-/jobs/4187732684#L2303
Fixed.
Sorry about that.
It only fails on hosts that don't support userfaultd() (i.e. non-linux).
Will resend.
Sorry for the noise.
Later, Juan.
>
> ../migration/postcopy-ram.c:1368:6: error: conflicting types for
> 'postcopy_ram_supported_by_host'; have '_Bool(MigrationIncomingState
> *)'
> 1368 | bool postcopy_ram_supported_by_host(MigrationIncomingState *mis)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> In file included from ../migration/migration.h:27,
> from ../migration/postcopy-ram.c:22:
> ../migration/postcopy-ram.h:17:6: note: previous declaration of
> 'postcopy_ram_supported_by_host' with type
> '_Bool(MigrationIncomingState *, Error **)'
> 17 | bool postcopy_ram_supported_by_host(MigrationIncomingState *mis,
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>
> r~
^ permalink raw reply [flat|nested] 11+ messages in thread