* [PULL 01/18] migration: Cleanup migrate_fd_cleanup() on accessing to_dst_file
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 02/18] migration: Put thread names together with macros Peter Xu
` (17 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Peter Xu
The cleanup function can in many cases needs cleanup on its own.
The major thing we want to do here is not referencing to_dst_file when
without the file mutex. When at it, touch things elsewhere too to make it
look slightly better in general.
One thing to mention is, migration_thread has its own "running" boolean, so
it doesn't need to rely on to_dst_file being non-NULL. Multifd has a
dependency so it needs to be skipped if to_dst_file is not yet set; add a
richer comment for such reason.
Resolves: Coverity CID 1527402
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20240919163042.116767-1-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/migration.c | 32 +++++++++++++++++++-------------
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 021faee2f3..f5f428e764 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1405,6 +1405,9 @@ void migrate_set_state(MigrationStatus *state, MigrationStatus old_state,
static void migrate_fd_cleanup(MigrationState *s)
{
MigrationEventType type;
+ QEMUFile *tmp = NULL;
+
+ trace_migrate_fd_cleanup();
g_free(s->hostname);
s->hostname = NULL;
@@ -1415,26 +1418,29 @@ static void migrate_fd_cleanup(MigrationState *s)
close_return_path_on_source(s);
- if (s->to_dst_file) {
- QEMUFile *tmp;
-
- trace_migrate_fd_cleanup();
+ if (s->migration_thread_running) {
bql_unlock();
- if (s->migration_thread_running) {
- qemu_thread_join(&s->thread);
- s->migration_thread_running = false;
- }
+ qemu_thread_join(&s->thread);
+ s->migration_thread_running = false;
bql_lock();
+ }
- multifd_send_shutdown();
- qemu_mutex_lock(&s->qemu_file_lock);
+ WITH_QEMU_LOCK_GUARD(&s->qemu_file_lock) {
+ /*
+ * Close the file handle without the lock to make sure the critical
+ * section won't block for long.
+ */
tmp = s->to_dst_file;
s->to_dst_file = NULL;
- qemu_mutex_unlock(&s->qemu_file_lock);
+ }
+
+ if (tmp) {
/*
- * Close the file handle without the lock to make sure the
- * critical section won't block for long.
+ * We only need to shutdown multifd if tmp!=NULL, because if
+ * tmp==NULL, it means the main channel isn't established, while
+ * multifd is only setup after that (in migration_thread()).
*/
+ multifd_send_shutdown();
migration_ioc_unregister_yank_from_file(tmp);
qemu_fclose(tmp);
}
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 02/18] migration: Put thread names together with macros
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
2024-10-30 15:57 ` [PULL 01/18] migration: Cleanup migrate_fd_cleanup() on accessing to_dst_file Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 03/18] migration: Ensure vmstate_save() sets errp Peter Xu
` (16 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Maydell, Peter Xu, Hyman Huang, Zhang Chen
Keep migration thread names together, so it's easier to see a list of all
possible migration threads.
Still two functional changes below besides the macro defintions:
- There's one dirty rate thread that we overlooked before, now we add
that too and name it as "mig/dirtyrate" following the old rules.
- The old name "mig/src/rp-thr" has "-thr" but it may not be useful if
it's a thread name anyway, while "rp" can be slightly hard to read.
Taking this chance to rename it to "mig/src/return", hopefully a better
name.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Acked-by: Hyman Huang <yong.huang@smartx.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Link: https://lore.kernel.org/r/20241011153652.517440-1-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/migration.h | 14 ++++++++++++++
migration/colo.c | 3 ++-
migration/dirtyrate.c | 6 ++++--
migration/migration.c | 9 +++++----
migration/multifd.c | 6 +++---
migration/postcopy-ram.c | 6 ++++--
migration/savevm.c | 3 ++-
7 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/migration/migration.h b/migration/migration.h
index 38aa1402d5..b9ce5aa4ff 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -28,6 +28,20 @@
#include "sysemu/runstate.h"
#include "migration/misc.h"
+#define MIGRATION_THREAD_SNAPSHOT "mig/snapshot"
+#define MIGRATION_THREAD_DIRTY_RATE "mig/dirtyrate"
+
+#define MIGRATION_THREAD_SRC_MAIN "mig/src/main"
+#define MIGRATION_THREAD_SRC_MULTIFD "mig/src/send_%d"
+#define MIGRATION_THREAD_SRC_RETURN "mig/src/return"
+#define MIGRATION_THREAD_SRC_TLS "mig/src/tls"
+
+#define MIGRATION_THREAD_DST_COLO "mig/dst/colo"
+#define MIGRATION_THREAD_DST_MULTIFD "mig/src/recv_%d"
+#define MIGRATION_THREAD_DST_FAULT "mig/dst/fault"
+#define MIGRATION_THREAD_DST_LISTEN "mig/dst/listen"
+#define MIGRATION_THREAD_DST_PREEMPT "mig/dst/preempt"
+
struct PostcopyBlocktimeContext;
#define MIGRATION_RESUME_ACK_VALUE (1)
diff --git a/migration/colo.c b/migration/colo.c
index 6449490221..9590f281d0 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -935,7 +935,8 @@ void coroutine_fn colo_incoming_co(void)
assert(bql_locked());
assert(migration_incoming_colo_enabled());
- qemu_thread_create(&th, "mig/dst/colo", colo_process_incoming_thread,
+ qemu_thread_create(&th, MIGRATION_THREAD_DST_COLO,
+ colo_process_incoming_thread,
mis, QEMU_THREAD_JOINABLE);
mis->colo_incoming_co = qemu_coroutine_self();
diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 233acb0855..a74a6aeb56 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -29,6 +29,7 @@
#include "sysemu/runstate.h"
#include "exec/memory.h"
#include "qemu/xxhash.h"
+#include "migration.h"
/*
* total_dirty_pages is procted by BQL and is used
@@ -839,8 +840,9 @@ void qmp_calc_dirty_rate(int64_t calc_time,
init_dirtyrate_stat(config);
- qemu_thread_create(&thread, "get_dirtyrate", get_dirtyrate_thread,
- (void *)&config, QEMU_THREAD_DETACHED);
+ qemu_thread_create(&thread, MIGRATION_THREAD_DIRTY_RATE,
+ get_dirtyrate_thread, (void *)&config,
+ QEMU_THREAD_DETACHED);
}
diff --git a/migration/migration.c b/migration/migration.c
index f5f428e764..7609e0feed 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2484,7 +2484,7 @@ static int open_return_path_on_source(MigrationState *ms)
trace_open_return_path_on_source();
- qemu_thread_create(&ms->rp_state.rp_thread, "mig/src/rp-thr",
+ qemu_thread_create(&ms->rp_state.rp_thread, MIGRATION_THREAD_SRC_RETURN,
source_return_path_thread, ms, QEMU_THREAD_JOINABLE);
ms->rp_state.rp_thread_created = true;
@@ -3473,7 +3473,8 @@ static void *migration_thread(void *opaque)
Error *local_err = NULL;
int ret;
- thread = migration_threads_add("live_migration", qemu_get_thread_id());
+ thread = migration_threads_add(MIGRATION_THREAD_SRC_MAIN,
+ qemu_get_thread_id());
rcu_register_thread();
@@ -3823,10 +3824,10 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
}
if (migrate_background_snapshot()) {
- qemu_thread_create(&s->thread, "mig/snapshot",
+ qemu_thread_create(&s->thread, MIGRATION_THREAD_SNAPSHOT,
bg_migration_thread, s, QEMU_THREAD_JOINABLE);
} else {
- qemu_thread_create(&s->thread, "mig/src/main",
+ qemu_thread_create(&s->thread, MIGRATION_THREAD_SRC_MAIN,
migration_thread, s, QEMU_THREAD_JOINABLE);
}
s->migration_thread_running = true;
diff --git a/migration/multifd.c b/migration/multifd.c
index 9b200f4ad9..697fe86fdf 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -723,7 +723,7 @@ static bool multifd_tls_channel_connect(MultiFDSendParams *p,
args->p = p;
p->tls_thread_created = true;
- qemu_thread_create(&p->tls_thread, "mig/src/tls",
+ qemu_thread_create(&p->tls_thread, MIGRATION_THREAD_SRC_TLS,
multifd_tls_handshake_thread, args,
QEMU_THREAD_JOINABLE);
return true;
@@ -841,7 +841,7 @@ bool multifd_send_setup(void)
+ sizeof(uint64_t) * page_count;
p->packet = g_malloc0(p->packet_len);
}
- p->name = g_strdup_printf("mig/src/send_%d", i);
+ p->name = g_strdup_printf(MIGRATION_THREAD_SRC_MULTIFD, i);
p->write_flags = 0;
if (!multifd_new_send_channel_create(p, &local_err)) {
@@ -1259,7 +1259,7 @@ int multifd_recv_setup(Error **errp)
+ sizeof(uint64_t) * page_count;
p->packet = g_malloc0(p->packet_len);
}
- p->name = g_strdup_printf("mig/dst/recv_%d", i);
+ p->name = g_strdup_printf(MIGRATION_THREAD_DST_MULTIFD, i);
p->normal = g_new0(ram_addr_t, page_count);
p->zero = g_new0(ram_addr_t, page_count);
}
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 83f6160a36..a535fd2e30 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -1230,7 +1230,8 @@ int postcopy_ram_incoming_setup(MigrationIncomingState *mis)
return -1;
}
- postcopy_thread_create(mis, &mis->fault_thread, "mig/dst/fault",
+ postcopy_thread_create(mis, &mis->fault_thread,
+ MIGRATION_THREAD_DST_FAULT,
postcopy_ram_fault_thread, QEMU_THREAD_JOINABLE);
mis->have_fault_thread = true;
@@ -1250,7 +1251,8 @@ int postcopy_ram_incoming_setup(MigrationIncomingState *mis)
* This thread needs to be created after the temp pages because
* it'll fetch RAM_CHANNEL_POSTCOPY PostcopyTmpPage immediately.
*/
- postcopy_thread_create(mis, &mis->postcopy_prio_thread, "mig/dst/preempt",
+ postcopy_thread_create(mis, &mis->postcopy_prio_thread,
+ MIGRATION_THREAD_DST_PREEMPT,
postcopy_preempt_thread, QEMU_THREAD_JOINABLE);
mis->preempt_thread_status = PREEMPT_THREAD_CREATED;
}
diff --git a/migration/savevm.c b/migration/savevm.c
index 7e1e27182a..e796436979 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2131,7 +2131,8 @@ static int loadvm_postcopy_handle_listen(MigrationIncomingState *mis)
}
mis->have_listen_thread = true;
- postcopy_thread_create(mis, &mis->listen_thread, "mig/dst/listen",
+ postcopy_thread_create(mis, &mis->listen_thread,
+ MIGRATION_THREAD_DST_LISTEN,
postcopy_ram_listen_thread, QEMU_THREAD_DETACHED);
trace_loadvm_postcopy_handle_listen("return");
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 03/18] migration: Ensure vmstate_save() sets errp
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
2024-10-30 15:57 ` [PULL 01/18] migration: Cleanup migrate_fd_cleanup() on accessing to_dst_file Peter Xu
2024-10-30 15:57 ` [PULL 02/18] migration: Put thread names together with macros Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 04/18] accel/tcg/icount-common: Remove the reference to the unused header file Peter Xu
` (15 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Hanna Czenczek, Peter Xu
From: Hanna Czenczek <hreitz@redhat.com>
migration/savevm.c contains some calls to vmstate_save() that are
followed by migrate_set_error() if the integer return value indicates an
error. migrate_set_error() requires that the `Error *` object passed to
it is set. Therefore, vmstate_save() is assumed to always set *errp on
error.
Right now, that assumption is not met: vmstate_save_state_v() (called
internally by vmstate_save()) will not set *errp if
vmstate_subsection_save() or vmsd->post_save() fail. Fix that by adding
an *errp parameter to vmstate_subsection_save(), and by generating a
generic error in case post_save() fails (as is already done for
pre_save()).
Without this patch, qemu will crash after vmstate_subsection_save() or
post_save() have failed inside of a vmstate_save() call (unless
migrate_set_error() then happen to discard the new error because
s->error is already set). This happens e.g. when receiving the state
from a virtio-fs back-end (virtiofsd) fails.
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Link: https://lore.kernel.org/r/20241015170437.310358-1-hreitz@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/vmstate.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index ff5d589a6d..fa002b24e8 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -22,7 +22,8 @@
#include "trace.h"
static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
- void *opaque, JSONWriter *vmdesc);
+ void *opaque, JSONWriter *vmdesc,
+ Error **errp);
static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque);
@@ -441,12 +442,13 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
json_writer_end_array(vmdesc);
}
- ret = vmstate_subsection_save(f, vmsd, opaque, vmdesc);
+ ret = vmstate_subsection_save(f, vmsd, opaque, vmdesc, errp);
if (vmsd->post_save) {
int ps_ret = vmsd->post_save(opaque);
- if (!ret) {
+ if (!ret && ps_ret) {
ret = ps_ret;
+ error_setg(errp, "post-save failed: %s", vmsd->name);
}
}
return ret;
@@ -518,7 +520,8 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
}
static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
- void *opaque, JSONWriter *vmdesc)
+ void *opaque, JSONWriter *vmdesc,
+ Error **errp)
{
const VMStateDescription * const *sub = vmsd->subsections;
bool vmdesc_has_subsections = false;
@@ -546,7 +549,7 @@ static int vmstate_subsection_save(QEMUFile *f, const VMStateDescription *vmsd,
qemu_put_byte(f, len);
qemu_put_buffer(f, (uint8_t *)vmsdsub->name, len);
qemu_put_be32(f, vmsdsub->version_id);
- ret = vmstate_save_state(f, vmsdsub, opaque, vmdesc);
+ ret = vmstate_save_state_with_err(f, vmsdsub, opaque, vmdesc, errp);
if (ret) {
return ret;
}
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 04/18] accel/tcg/icount-common: Remove the reference to the unused header file
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (2 preceding siblings ...)
2024-10-30 15:57 ` [PULL 03/18] migration: Ensure vmstate_save() sets errp Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 05/18] migration: Stop CPU throttling conditionally Peter Xu
` (14 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Hyman Huang, Peter Xu
From: Hyman Huang <yong.huang@smartx.com>
Signed-off-by: Hyman Huang <yong.huang@smartx.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/5e33b423d0b8506e5cb33fff42b50aa301b7731b.1729146786.git.yong.huang@smartx.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
accel/tcg/icount-common.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/accel/tcg/icount-common.c b/accel/tcg/icount-common.c
index 8d3d3a7e9d..30bf8500dc 100644
--- a/accel/tcg/icount-common.c
+++ b/accel/tcg/icount-common.c
@@ -36,7 +36,6 @@
#include "sysemu/runstate.h"
#include "hw/core/cpu.h"
#include "sysemu/cpu-timers.h"
-#include "sysemu/cpu-throttle.h"
#include "sysemu/cpu-timers-internal.h"
/*
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 05/18] migration: Stop CPU throttling conditionally
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (3 preceding siblings ...)
2024-10-30 15:57 ` [PULL 04/18] accel/tcg/icount-common: Remove the reference to the unused header file Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 06/18] migration: Move cpu-throttole.c from system to migration Peter Xu
` (13 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Hyman Huang, Peter Xu
From: Hyman Huang <yong.huang@smartx.com>
Since CPU throttling only occurs when auto-converge
is on, stop it conditionally.
Signed-off-by: Hyman Huang <yong.huang@smartx.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/f0c787080bb9ab0c37952f0ca5bfaa525d5ddd14.1729146786.git.yong.huang@smartx.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/migration.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/migration/migration.c b/migration/migration.c
index 7609e0feed..e81c70b9d2 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3295,7 +3295,9 @@ static MigIterateState migration_iteration_run(MigrationState *s)
static void migration_iteration_finish(MigrationState *s)
{
/* If we enabled cpu throttling for auto-converge, turn it off. */
- cpu_throttle_stop();
+ if (migrate_auto_converge()) {
+ cpu_throttle_stop();
+ }
bql_lock();
switch (s->state) {
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 06/18] migration: Move cpu-throttole.c from system to migration
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (4 preceding siblings ...)
2024-10-30 15:57 ` [PULL 05/18] migration: Stop CPU throttling conditionally Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 07/18] migration: Remove "rs" parameter in migration_bitmap_sync_precopy Peter Xu
` (12 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Hyman Huang, Peter Xu
From: Hyman Huang <yong.huang@smartx.com>
Move cpu-throttle.c from system to migration since it's
only used for migration; this makes us avoid exporting the
util functions and variables in misc.h but export them in
migration.h when implementing the periodic ramblock dirty
sync feature in the upcoming commits.
Since CPU throttle timers are only used in migration, move
their registry to migration_object_init.
Signed-off-by: Hyman Huang <yong.huang@smartx.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/c1b3efaa0cb49e03d422e9da97bdb65cc3d234d1.1729146786.git.yong.huang@smartx.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
{include/sysemu => migration}/cpu-throttle.h | 0
{system => migration}/cpu-throttle.c | 2 +-
migration/migration.c | 5 ++++-
migration/ram.c | 2 +-
system/cpu-timers.c | 3 ---
migration/meson.build | 1 +
migration/trace-events | 3 +++
system/meson.build | 1 -
system/trace-events | 3 ---
9 files changed, 10 insertions(+), 10 deletions(-)
rename {include/sysemu => migration}/cpu-throttle.h (100%)
rename {system => migration}/cpu-throttle.c (99%)
diff --git a/include/sysemu/cpu-throttle.h b/migration/cpu-throttle.h
similarity index 100%
rename from include/sysemu/cpu-throttle.h
rename to migration/cpu-throttle.h
diff --git a/system/cpu-throttle.c b/migration/cpu-throttle.c
similarity index 99%
rename from system/cpu-throttle.c
rename to migration/cpu-throttle.c
index 7632dc6143..fa47ee2e21 100644
--- a/system/cpu-throttle.c
+++ b/migration/cpu-throttle.c
@@ -27,7 +27,7 @@
#include "hw/core/cpu.h"
#include "qemu/main-loop.h"
#include "sysemu/cpus.h"
-#include "sysemu/cpu-throttle.h"
+#include "cpu-throttle.h"
#include "trace.h"
/* vcpu throttling controls */
diff --git a/migration/migration.c b/migration/migration.c
index e81c70b9d2..05c8cd50b4 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -24,7 +24,7 @@
#include "socket.h"
#include "sysemu/runstate.h"
#include "sysemu/sysemu.h"
-#include "sysemu/cpu-throttle.h"
+#include "cpu-throttle.h"
#include "rdma.h"
#include "ram.h"
#include "migration/global_state.h"
@@ -263,6 +263,9 @@ void migration_object_init(void)
ram_mig_init();
dirty_bitmap_mig_init();
+
+ /* Initialize cpu throttle timers */
+ cpu_throttle_init();
}
typedef struct {
diff --git a/migration/ram.c b/migration/ram.c
index 326ce7eb79..54d352b152 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -52,7 +52,7 @@
#include "exec/target_page.h"
#include "qemu/rcu_queue.h"
#include "migration/colo.h"
-#include "sysemu/cpu-throttle.h"
+#include "cpu-throttle.h"
#include "savevm.h"
#include "qemu/iov.h"
#include "multifd.h"
diff --git a/system/cpu-timers.c b/system/cpu-timers.c
index 0b31c9a1b6..856e502e34 100644
--- a/system/cpu-timers.c
+++ b/system/cpu-timers.c
@@ -35,7 +35,6 @@
#include "sysemu/runstate.h"
#include "hw/core/cpu.h"
#include "sysemu/cpu-timers.h"
-#include "sysemu/cpu-throttle.h"
#include "sysemu/cpu-timers-internal.h"
/* clock and ticks */
@@ -272,6 +271,4 @@ void cpu_timers_init(void)
seqlock_init(&timers_state.vm_clock_seqlock);
qemu_spin_init(&timers_state.vm_clock_lock);
vmstate_register(NULL, 0, &vmstate_timers, &timers_state);
-
- cpu_throttle_init();
}
diff --git a/migration/meson.build b/migration/meson.build
index 66d3de86f0..d53cf3417a 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -13,6 +13,7 @@ system_ss.add(files(
'block-dirty-bitmap.c',
'channel.c',
'channel-block.c',
+ 'cpu-throttle.c',
'dirtyrate.c',
'exec.c',
'fd.c',
diff --git a/migration/trace-events b/migration/trace-events
index c65902f042..9a19599804 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -378,3 +378,6 @@ migration_block_progression(unsigned percent) "Completed %u%%"
# page_cache.c
migration_pagecache_init(int64_t max_num_items) "Setting cache buckets to %" PRId64
migration_pagecache_insert(void) "Error allocating page"
+
+# cpu-throttle.c
+cpu_throttle_set(int new_throttle_pct) "set guest CPU throttled by %d%%"
diff --git a/system/meson.build b/system/meson.build
index a296270cb0..4952f4b2c7 100644
--- a/system/meson.build
+++ b/system/meson.build
@@ -10,7 +10,6 @@ system_ss.add(files(
'balloon.c',
'bootdevice.c',
'cpus.c',
- 'cpu-throttle.c',
'cpu-timers.c',
'datadir.c',
'dirtylimit.c',
diff --git a/system/trace-events b/system/trace-events
index 074d001e90..2ed1d59b1f 100644
--- a/system/trace-events
+++ b/system/trace-events
@@ -44,6 +44,3 @@ dirtylimit_state_finalize(void)
dirtylimit_throttle_pct(int cpu_index, uint64_t pct, int64_t time_us) "CPU[%d] throttle percent: %" PRIu64 ", throttle adjust time %"PRIi64 " us"
dirtylimit_set_vcpu(int cpu_index, uint64_t quota) "CPU[%d] set dirty page rate limit %"PRIu64
dirtylimit_vcpu_execute(int cpu_index, int64_t sleep_time_us) "CPU[%d] sleep %"PRIi64 " us"
-
-# cpu-throttle.c
-cpu_throttle_set(int new_throttle_pct) "set guest CPU throttled by %d%%"
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 07/18] migration: Remove "rs" parameter in migration_bitmap_sync_precopy
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (5 preceding siblings ...)
2024-10-30 15:57 ` [PULL 06/18] migration: Move cpu-throttole.c from system to migration Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 08/18] migration: Support periodic RAMBlock dirty bitmap sync Peter Xu
` (11 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Hyman Huang, Peter Xu
From: Hyman Huang <yong.huang@smartx.com>
The global static variable ram_state in fact is referred to by the
"rs" parameter in migration_bitmap_sync_precopy. For ease of calling
by the callees, use the global variable directly in
migration_bitmap_sync_precopy and remove "rs" parameter.
The migration_bitmap_sync_precopy will be exported in the next commit.
Signed-off-by: Hyman Huang <yong.huang@smartx.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Link: https://lore.kernel.org/r/283c335d61463bf477160da91b24da45cdaf3e43.1729146786.git.yong.huang@smartx.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/migration/ram.c b/migration/ram.c
index 54d352b152..9b5b350405 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1088,9 +1088,10 @@ static void migration_bitmap_sync(RAMState *rs, bool last_stage)
}
}
-static void migration_bitmap_sync_precopy(RAMState *rs, bool last_stage)
+static void migration_bitmap_sync_precopy(bool last_stage)
{
Error *local_err = NULL;
+ assert(ram_state);
/*
* The current notifier usage is just an optimization to migration, so we
@@ -1101,7 +1102,7 @@ static void migration_bitmap_sync_precopy(RAMState *rs, bool last_stage)
local_err = NULL;
}
- migration_bitmap_sync(rs, last_stage);
+ migration_bitmap_sync(ram_state, last_stage);
if (precopy_notify(PRECOPY_NOTIFY_AFTER_BITMAP_SYNC, &local_err)) {
error_report_err(local_err);
@@ -2782,7 +2783,7 @@ static bool ram_init_bitmaps(RAMState *rs, Error **errp)
if (!ret) {
goto out_unlock;
}
- migration_bitmap_sync_precopy(rs, false);
+ migration_bitmap_sync_precopy(false);
}
}
out_unlock:
@@ -3248,7 +3249,7 @@ static int ram_save_complete(QEMUFile *f, void *opaque)
WITH_RCU_READ_LOCK_GUARD() {
if (!migration_in_postcopy()) {
- migration_bitmap_sync_precopy(rs, true);
+ migration_bitmap_sync_precopy(true);
}
ret = rdma_registration_start(f, RAM_CONTROL_FINISH);
@@ -3330,7 +3331,7 @@ static void ram_state_pending_exact(void *opaque, uint64_t *must_precopy,
if (!migration_in_postcopy()) {
bql_lock();
WITH_RCU_READ_LOCK_GUARD() {
- migration_bitmap_sync_precopy(rs, false);
+ migration_bitmap_sync_precopy(false);
}
bql_unlock();
}
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 08/18] migration: Support periodic RAMBlock dirty bitmap sync
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (6 preceding siblings ...)
2024-10-30 15:57 ` [PULL 07/18] migration: Remove "rs" parameter in migration_bitmap_sync_precopy Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 09/18] tests/migration: Add case for periodic ramblock dirty sync Peter Xu
` (10 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Hyman Huang, Peter Xu
From: Hyman Huang <yong.huang@smartx.com>
When VM is configured with huge memory, the current throttle logic
doesn't look like to scale, because migration_trigger_throttle()
is only called for each iteration, so it won't be invoked for a long
time if one iteration can take a long time.
The periodic dirty sync aims to fix the above issue by synchronizing
the ramblock from remote dirty bitmap and, when necessary, triggering
the CPU throttle multiple times during a long iteration.
This is a trade-off between synchronization overhead and CPU throttle
impact.
Signed-off-by: Hyman Huang <yong.huang@smartx.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/f61f1b3653f2acf026901103e1c73d157d38b08f.1729146786.git.yong.huang@smartx.com
[peterx: make prev_cnt global, and reset for each migration]
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/cpu-throttle.h | 14 ++++++++
migration/migration.h | 1 +
migration/cpu-throttle.c | 70 +++++++++++++++++++++++++++++++++++++++-
migration/migration.c | 14 ++++++--
migration/ram.c | 2 +-
migration/trace-events | 1 +
6 files changed, 98 insertions(+), 4 deletions(-)
diff --git a/migration/cpu-throttle.h b/migration/cpu-throttle.h
index d65bdef6d0..420702b8d3 100644
--- a/migration/cpu-throttle.h
+++ b/migration/cpu-throttle.h
@@ -65,4 +65,18 @@ bool cpu_throttle_active(void);
*/
int cpu_throttle_get_percentage(void);
+/**
+ * cpu_throttle_dirty_sync_timer_tick:
+ *
+ * Dirty sync timer hook.
+ */
+void cpu_throttle_dirty_sync_timer_tick(void *opaque);
+
+/**
+ * cpu_throttle_dirty_sync_timer:
+ *
+ * Start or stop the dirty sync timer.
+ */
+void cpu_throttle_dirty_sync_timer(bool enable);
+
#endif /* SYSEMU_CPU_THROTTLE_H */
diff --git a/migration/migration.h b/migration/migration.h
index b9ce5aa4ff..7dc59c5e8d 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -551,4 +551,5 @@ int migration_rp_wait(MigrationState *s);
*/
void migration_rp_kick(MigrationState *s);
+void migration_bitmap_sync_precopy(bool last_stage);
#endif
diff --git a/migration/cpu-throttle.c b/migration/cpu-throttle.c
index fa47ee2e21..3df287d8d3 100644
--- a/migration/cpu-throttle.c
+++ b/migration/cpu-throttle.c
@@ -28,16 +28,23 @@
#include "qemu/main-loop.h"
#include "sysemu/cpus.h"
#include "cpu-throttle.h"
+#include "migration.h"
+#include "migration-stats.h"
#include "trace.h"
/* vcpu throttling controls */
-static QEMUTimer *throttle_timer;
+static QEMUTimer *throttle_timer, *throttle_dirty_sync_timer;
static unsigned int throttle_percentage;
+static bool throttle_dirty_sync_timer_active;
+static uint64_t throttle_dirty_sync_count_prev;
#define CPU_THROTTLE_PCT_MIN 1
#define CPU_THROTTLE_PCT_MAX 99
#define CPU_THROTTLE_TIMESLICE_NS 10000000
+/* Making sure RAMBlock dirty bitmap is synchronized every five seconds */
+#define CPU_THROTTLE_DIRTY_SYNC_TIMESLICE_MS 5000
+
static void cpu_throttle_thread(CPUState *cpu, run_on_cpu_data opaque)
{
double pct;
@@ -112,6 +119,7 @@ void cpu_throttle_set(int new_throttle_pct)
void cpu_throttle_stop(void)
{
qatomic_set(&throttle_percentage, 0);
+ cpu_throttle_dirty_sync_timer(false);
}
bool cpu_throttle_active(void)
@@ -124,8 +132,68 @@ int cpu_throttle_get_percentage(void)
return qatomic_read(&throttle_percentage);
}
+void cpu_throttle_dirty_sync_timer_tick(void *opaque)
+{
+ uint64_t sync_cnt = stat64_get(&mig_stats.dirty_sync_count);
+
+ /*
+ * The first iteration copies all memory anyhow and has no
+ * effect on guest performance, therefore omit it to avoid
+ * paying extra for the sync penalty.
+ */
+ if (sync_cnt <= 1) {
+ goto end;
+ }
+
+ if (sync_cnt == throttle_dirty_sync_count_prev) {
+ trace_cpu_throttle_dirty_sync();
+ WITH_RCU_READ_LOCK_GUARD() {
+ migration_bitmap_sync_precopy(false);
+ }
+ }
+
+end:
+ throttle_dirty_sync_count_prev = stat64_get(&mig_stats.dirty_sync_count);
+
+ timer_mod(throttle_dirty_sync_timer,
+ qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL_RT) +
+ CPU_THROTTLE_DIRTY_SYNC_TIMESLICE_MS);
+}
+
+static bool cpu_throttle_dirty_sync_active(void)
+{
+ return qatomic_read(&throttle_dirty_sync_timer_active);
+}
+
+void cpu_throttle_dirty_sync_timer(bool enable)
+{
+ assert(throttle_dirty_sync_timer);
+
+ if (enable) {
+ if (!cpu_throttle_dirty_sync_active()) {
+ /*
+ * Always reset the dirty sync count cache, in case migration
+ * was cancelled once.
+ */
+ throttle_dirty_sync_count_prev = 0;
+ timer_mod(throttle_dirty_sync_timer,
+ qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL_RT) +
+ CPU_THROTTLE_DIRTY_SYNC_TIMESLICE_MS);
+ qatomic_set(&throttle_dirty_sync_timer_active, 1);
+ }
+ } else {
+ if (cpu_throttle_dirty_sync_active()) {
+ timer_del(throttle_dirty_sync_timer);
+ qatomic_set(&throttle_dirty_sync_timer_active, 0);
+ }
+ }
+}
+
void cpu_throttle_init(void)
{
throttle_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT,
cpu_throttle_timer_tick, NULL);
+ throttle_dirty_sync_timer =
+ timer_new_ms(QEMU_CLOCK_VIRTUAL_RT,
+ cpu_throttle_dirty_sync_timer_tick, NULL);
}
diff --git a/migration/migration.c b/migration/migration.c
index 05c8cd50b4..bcb735869b 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3297,12 +3297,17 @@ static MigIterateState migration_iteration_run(MigrationState *s)
static void migration_iteration_finish(MigrationState *s)
{
- /* If we enabled cpu throttling for auto-converge, turn it off. */
+ bql_lock();
+
+ /*
+ * If we enabled cpu throttling for auto-converge, turn it off.
+ * Stopping CPU throttle should be serialized by BQL to avoid
+ * racing for the throttle_dirty_sync_timer.
+ */
if (migrate_auto_converge()) {
cpu_throttle_stop();
}
- bql_lock();
switch (s->state) {
case MIGRATION_STATUS_COMPLETED:
runstate_set(RUN_STATE_POSTMIGRATE);
@@ -3520,6 +3525,11 @@ static void *migration_thread(void *opaque)
qemu_savevm_send_colo_enable(s->to_dst_file);
}
+ if (migrate_auto_converge()) {
+ /* Start RAMBlock dirty bitmap sync timer */
+ cpu_throttle_dirty_sync_timer(true);
+ }
+
bql_lock();
ret = qemu_savevm_state_setup(s->to_dst_file, &local_err);
bql_unlock();
diff --git a/migration/ram.c b/migration/ram.c
index 9b5b350405..d284f63854 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1088,7 +1088,7 @@ static void migration_bitmap_sync(RAMState *rs, bool last_stage)
}
}
-static void migration_bitmap_sync_precopy(bool last_stage)
+void migration_bitmap_sync_precopy(bool last_stage)
{
Error *local_err = NULL;
assert(ram_state);
diff --git a/migration/trace-events b/migration/trace-events
index 9a19599804..0638183056 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -381,3 +381,4 @@ migration_pagecache_insert(void) "Error allocating page"
# cpu-throttle.c
cpu_throttle_set(int new_throttle_pct) "set guest CPU throttled by %d%%"
+cpu_throttle_dirty_sync(void) ""
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 09/18] tests/migration: Add case for periodic ramblock dirty sync
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (7 preceding siblings ...)
2024-10-30 15:57 ` [PULL 08/18] migration: Support periodic RAMBlock dirty bitmap sync Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 10/18] migration/dirtyrate: Silence warning about strcpy() on OpenBSD Peter Xu
` (9 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Hyman Huang, Peter Xu
From: Hyman Huang <yong.huang@smartx.com>
Signed-off-by: Hyman Huang <yong.huang@smartx.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/cb61504f1a1e9d5f2ca4dac12e518deb076ce9f3.1729146786.git.yong.huang@smartx.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
tests/qtest/migration-test.c | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 95e45b5029..e6a2803e71 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2791,6 +2791,8 @@ static void test_migrate_auto_converge(void)
* so we need to decrease a bandwidth.
*/
const int64_t init_pct = 5, inc_pct = 25, max_pct = 95;
+ uint64_t prev_dirty_sync_cnt, dirty_sync_cnt;
+ int max_try_count, hit = 0;
if (test_migrate_start(&from, &to, uri, &args)) {
return;
@@ -2827,6 +2829,36 @@ static void test_migrate_auto_converge(void)
} while (true);
/* The first percentage of throttling should be at least init_pct */
g_assert_cmpint(percentage, >=, init_pct);
+
+ /*
+ * End the loop when the dirty sync count greater than 1.
+ */
+ while ((dirty_sync_cnt = get_migration_pass(from)) < 2) {
+ usleep(1000 * 1000);
+ }
+
+ prev_dirty_sync_cnt = dirty_sync_cnt;
+
+ /*
+ * The RAMBlock dirty sync count must changes in 5 seconds, here we set
+ * the timeout to 10 seconds to ensure it changes.
+ *
+ * Note that migrate_ensure_non_converge set the max-bandwidth to 3MB/s,
+ * while the qtest mem is >= 100MB, one iteration takes at least 33s (100/3)
+ * to complete; this ensures that the RAMBlock dirty sync occurs.
+ */
+ max_try_count = 10;
+ while (--max_try_count) {
+ dirty_sync_cnt = get_migration_pass(from);
+ if (dirty_sync_cnt != prev_dirty_sync_cnt) {
+ hit = 1;
+ break;
+ }
+ prev_dirty_sync_cnt = dirty_sync_cnt;
+ sleep(1);
+ }
+ g_assert_cmpint(hit, ==, 1);
+
/* Now, when we tested that throttling works, let it converge */
migrate_ensure_converge(from);
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 10/18] migration/dirtyrate: Silence warning about strcpy() on OpenBSD
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (8 preceding siblings ...)
2024-10-30 15:57 ` [PULL 09/18] tests/migration: Add case for periodic ramblock dirty sync Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 11/18] migration: Deprecate query-migrationthreads command Peter Xu
` (8 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel
Cc: Fabiano Rosas, Peter Maydell, Thomas Huth, Hyman Huang, Peter Xu
From: Thomas Huth <thuth@redhat.com>
The linker on OpenBSD complains:
ld: warning: dirtyrate.c:447 (../src/migration/dirtyrate.c:447)(...):
warning: strcpy() is almost always misused, please use strlcpy()
It's currently not a real problem in this case since both arrays
have the same size (256 bytes). But just in case somebody changes
the size of the source array in the future, let's better play safe
and use g_strlcpy() here instead, with an additional check that the
string has been copied as a whole.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Hyman Huang <yong.huang@smartx.com>
Link: https://lore.kernel.org/r/20241022063402.184213-1-thuth@redhat.com
[peterx: Fix over-80 chars]
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/dirtyrate.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index a74a6aeb56..f7e86686fc 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -437,6 +437,7 @@ static void get_ramblock_dirty_info(RAMBlock *block,
struct DirtyRateConfig *config)
{
uint64_t sample_pages_per_gigabytes = config->sample_pages_per_gigabytes;
+ gsize len;
/* Right shift 30 bits to calc ramblock size in GB */
info->sample_pages_count = (qemu_ram_get_used_length(block) *
@@ -445,7 +446,9 @@ static void get_ramblock_dirty_info(RAMBlock *block,
info->ramblock_pages = qemu_ram_get_used_length(block) >>
qemu_target_page_bits();
info->ramblock_addr = qemu_ram_get_host_addr(block);
- strcpy(info->idstr, qemu_ram_get_idstr(block));
+ len = g_strlcpy(info->idstr, qemu_ram_get_idstr(block),
+ sizeof(info->idstr));
+ g_assert(len < sizeof(info->idstr));
}
static void free_ramblock_dirty_info(struct RamblockDirtyInfo *infos, int count)
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 11/18] migration: Deprecate query-migrationthreads command
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (9 preceding siblings ...)
2024-10-30 15:57 ` [PULL 10/18] migration/dirtyrate: Silence warning about strcpy() on OpenBSD Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 12/18] migration: Take migration object refcount earlier for threads Peter Xu
` (7 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Peter Xu, Markus Armbruster
Per previous discussion [1,2], this patch deprecates query-migrationthreads
command.
To summarize, the major reason of the deprecation is due to no sensible way
to consume the API properly:
(1) The reported list of threads are incomplete (ignoring destination
threads and non-multifd threads).
(2) For CPU pinning, there's no way to properly pin the threads with
the API if the threads will start running right away after migration
threads can be queried, so the threads will always run on the default
cores for a short window.
(3) For VM debugging, one can use "-name $VM,debug-threads=on" instead,
which will provide proper names for all migration threads.
[1] https://lore.kernel.org/r/20240930195837.825728-1-peterx@redhat.com
[2] https://lore.kernel.org/r/20241011153417.516715-1-peterx@redhat.com
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Acked-by: Markus Armbruster <armbru@redhat.com>
Link: https://lore.kernel.org/r/20241022194501.1022443-1-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
docs/about/deprecated.rst | 8 ++++++++
qapi/migration.json | 7 ++++++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index ce38a3d0cf..100ba66fe1 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -147,6 +147,14 @@ options are removed in favor of using explicit ``blockdev-create`` and
``blockdev-add`` calls. See :doc:`/interop/live-block-operations` for
details.
+``query-migrationthreads`` (since 9.2)
+''''''''''''''''''''''''''''''''''''''
+
+To be removed with no replacement, as it reports only a limited set of
+threads (for example, it only reports source side of multifd threads,
+without reporting any destination threads, or non-multifd source threads).
+For debugging purpose, please use ``-name $VM,debug-threads=on`` instead.
+
Incorrectly typed ``device_add`` arguments (since 6.2)
''''''''''''''''''''''''''''''''''''''''''''''''''''''
diff --git a/qapi/migration.json b/qapi/migration.json
index 3af6aa1740..a605dc26db 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -2284,12 +2284,17 @@
#
# Returns information of migration threads
#
+# Features:
+#
+# @deprecated: This command is deprecated with no replacement yet.
+#
# Returns: @MigrationThreadInfo
#
# Since: 7.2
##
{ 'command': 'query-migrationthreads',
- 'returns': ['MigrationThreadInfo'] }
+ 'returns': ['MigrationThreadInfo'],
+ 'features': ['deprecated'] }
##
# @snapshot-save:
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 12/18] migration: Take migration object refcount earlier for threads
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (10 preceding siblings ...)
2024-10-30 15:57 ` [PULL 11/18] migration: Deprecate query-migrationthreads command Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 13/18] migration: Unexport dirty_bitmap_mig_init() Peter Xu
` (6 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Peter Xu, Cédric Le Goater
Both migration thread or background snapshot thread will take a refcount of
the migration object at the entrace of the thread function.
That makes sense, because it protects the object from being freed by the
main thread in migration_shutdown() later, but it might still race with it
if the thread is scheduled too late. Consider the case right after
pthread_create() happened, VM shuts down with the object released, but
right after that the migration thread finally got created, referencing
MigrationState* in the opaque pointer which is already freed.
The only 100% safe way to make sure it won't get freed is taking the
refcount right before the thread is created, meanwhile when BQL is held.
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20241024213056.1395400-2-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/migration.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index bcb735869b..de80d64dda 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3488,7 +3488,6 @@ static void *migration_thread(void *opaque)
rcu_register_thread();
- object_ref(OBJECT(s));
update_iteration_initial_status(s);
if (!multifd_send_setup()) {
@@ -3626,7 +3625,6 @@ static void *bg_migration_thread(void *opaque)
int ret;
rcu_register_thread();
- object_ref(OBJECT(s));
migration_rate_set(RATE_LIMIT_DISABLED);
@@ -3838,6 +3836,14 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
}
}
+ /*
+ * Take a refcount to make sure the migration object won't get freed by
+ * the main thread already in migration_shutdown().
+ *
+ * The refcount will be released at the end of the thread function.
+ */
+ object_ref(OBJECT(s));
+
if (migrate_background_snapshot()) {
qemu_thread_create(&s->thread, MIGRATION_THREAD_SNAPSHOT,
bg_migration_thread, s, QEMU_THREAD_JOINABLE);
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 13/18] migration: Unexport dirty_bitmap_mig_init()
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (11 preceding siblings ...)
2024-10-30 15:57 ` [PULL 12/18] migration: Take migration object refcount earlier for threads Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 14/18] migration: Unexport ram_mig_init() Peter Xu
` (5 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Peter Xu, Cédric Le Goater
It's only used within migration/, so it shouldn't be exported.
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20241024213056.1395400-3-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/migration/misc.h | 3 ---
migration/migration.h | 4 ++++
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index bfadc5613b..df57be6b5e 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -108,7 +108,4 @@ bool migration_incoming_postcopy_advised(void);
/* True if background snapshot is active */
bool migration_in_bg_snapshot(void);
-/* migration/block-dirty-bitmap.c */
-void dirty_bitmap_mig_init(void);
-
#endif
diff --git a/migration/migration.h b/migration/migration.h
index 7dc59c5e8d..0956e9274b 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -552,4 +552,8 @@ int migration_rp_wait(MigrationState *s);
void migration_rp_kick(MigrationState *s);
void migration_bitmap_sync_precopy(bool last_stage);
+
+/* migration/block-dirty-bitmap.c */
+void dirty_bitmap_mig_init(void);
+
#endif
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 14/18] migration: Unexport ram_mig_init()
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (12 preceding siblings ...)
2024-10-30 15:57 ` [PULL 13/18] migration: Unexport dirty_bitmap_mig_init() Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 15/18] migration: Drop migration_is_setup_or_active() Peter Xu
` (4 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Peter Xu, Cédric Le Goater
It's only used within migration/.
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20241024213056.1395400-4-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/migration/misc.h | 1 -
migration/ram.h | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index df57be6b5e..e8490e3af5 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -39,7 +39,6 @@ void precopy_add_notifier(NotifierWithReturn *n);
void precopy_remove_notifier(NotifierWithReturn *n);
int precopy_notify(PrecopyNotifyReason reason, Error **errp);
-void ram_mig_init(void);
void qemu_guest_free_page_hint(void *addr, size_t len);
bool migrate_ram_is_ignored(RAMBlock *block);
diff --git a/migration/ram.h b/migration/ram.h
index bc0318b834..0d1981f888 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -44,6 +44,7 @@ extern XBZRLECacheStats xbzrle_counters;
INTERNAL_RAMBLOCK_FOREACH(block) \
if (!qemu_ram_is_migratable(block)) {} else
+void ram_mig_init(void);
int xbzrle_cache_resize(uint64_t new_size, Error **errp);
uint64_t ram_bytes_remaining(void);
uint64_t ram_bytes_total(void);
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 15/18] migration: Drop migration_is_setup_or_active()
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (13 preceding siblings ...)
2024-10-30 15:57 ` [PULL 14/18] migration: Unexport ram_mig_init() Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 16/18] migration: Drop migration_is_idle() Peter Xu
` (3 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Peter Xu, Cédric Le Goater
This helper is mostly the same as migration_is_running(), except that one
has COLO reported as true, the other has CANCELLING reported as true.
Per my past years experience on the state changes, none of them should
matter.
To make it slightly safer, report both COLO || CANCELLING to be true in
migration_is_running(), then drop the other one. We kept the 1st only
because the name is simpler, and clear enough.
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20241024213056.1395400-5-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/migration/misc.h | 4 ++--
hw/vfio/common.c | 2 +-
migration/migration.c | 35 +++--------------------------------
migration/ram.c | 5 ++---
net/vhost-vdpa.c | 3 +--
5 files changed, 9 insertions(+), 40 deletions(-)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index e8490e3af5..86ef160f19 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -52,11 +52,12 @@ void dump_vmstate_json_to_file(FILE *out_fp);
/* migration/migration.c */
void migration_object_init(void);
void migration_shutdown(void);
+
bool migration_is_idle(void);
bool migration_is_active(void);
bool migration_is_device(void);
+bool migration_is_running(void);
bool migration_thread_is_self(void);
-bool migration_is_setup_or_active(void);
typedef enum MigrationEventType {
MIG_EVENT_PRECOPY_SETUP,
@@ -95,7 +96,6 @@ void migration_add_notifier_mode(NotifierWithReturn *notify,
MigrationNotifyFunc func, MigMode mode);
void migration_remove_notifier(NotifierWithReturn *notify);
-bool migration_is_running(void);
void migration_file_set_error(int ret, Error *err);
/* True if incoming migration entered POSTCOPY_INCOMING_DISCARD */
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 36d0cf6585..dcef44fe55 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -149,7 +149,7 @@ bool vfio_viommu_preset(VFIODevice *vbasedev)
static void vfio_set_migration_error(int ret)
{
- if (migration_is_setup_or_active()) {
+ if (migration_is_running()) {
migration_file_set_error(ret, NULL);
}
}
diff --git a/migration/migration.c b/migration/migration.c
index de80d64dda..cab65ba8db 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1113,33 +1113,6 @@ void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value)
migrate_send_rp_message(mis, MIG_RP_MSG_RESUME_ACK, sizeof(buf), &buf);
}
-/*
- * Return true if we're already in the middle of a migration
- * (i.e. any of the active or setup states)
- */
-bool migration_is_setup_or_active(void)
-{
- MigrationState *s = current_migration;
-
- switch (s->state) {
- case MIGRATION_STATUS_ACTIVE:
- case MIGRATION_STATUS_POSTCOPY_ACTIVE:
- case MIGRATION_STATUS_POSTCOPY_PAUSED:
- case MIGRATION_STATUS_POSTCOPY_RECOVER_SETUP:
- case MIGRATION_STATUS_POSTCOPY_RECOVER:
- case MIGRATION_STATUS_SETUP:
- case MIGRATION_STATUS_PRE_SWITCHOVER:
- case MIGRATION_STATUS_DEVICE:
- case MIGRATION_STATUS_WAIT_UNPLUG:
- case MIGRATION_STATUS_COLO:
- return true;
-
- default:
- return false;
-
- }
-}
-
bool migration_is_running(void)
{
MigrationState *s = current_migration;
@@ -1155,11 +1128,10 @@ bool migration_is_running(void)
case MIGRATION_STATUS_DEVICE:
case MIGRATION_STATUS_WAIT_UNPLUG:
case MIGRATION_STATUS_CANCELLING:
+ case MIGRATION_STATUS_COLO:
return true;
-
default:
return false;
-
}
}
@@ -1658,8 +1630,7 @@ bool migration_incoming_postcopy_advised(void)
bool migration_in_bg_snapshot(void)
{
- return migrate_background_snapshot() &&
- migration_is_setup_or_active();
+ return migrate_background_snapshot() && migration_is_running();
}
bool migration_is_idle(void)
@@ -2332,7 +2303,7 @@ static void *source_return_path_thread(void *opaque)
trace_source_return_path_thread_entry();
rcu_register_thread();
- while (migration_is_setup_or_active()) {
+ while (migration_is_running()) {
trace_source_return_path_thread_loop_top();
header_type = qemu_get_be16(rp);
diff --git a/migration/ram.c b/migration/ram.c
index d284f63854..5646a0b882 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2860,7 +2860,7 @@ void qemu_guest_free_page_hint(void *addr, size_t len)
size_t used_len, start, npages;
/* This function is currently expected to be used during live migration */
- if (!migration_is_setup_or_active()) {
+ if (!migration_is_running()) {
return;
}
@@ -3208,8 +3208,7 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
}
out:
- if (ret >= 0
- && migration_is_setup_or_active()) {
+ if (ret >= 0 && migration_is_running()) {
if (migrate_multifd() && migrate_multifd_flush_after_each_section() &&
!migrate_mapped_ram()) {
ret = multifd_ram_flush_and_sync();
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index 46b02c50be..231b45246c 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -375,8 +375,7 @@ static int vhost_vdpa_net_data_start(NetClientState *nc)
assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
- if (s->always_svq ||
- migration_is_setup_or_active()) {
+ if (s->always_svq || migration_is_running()) {
v->shadow_vqs_enabled = true;
} else {
v->shadow_vqs_enabled = false;
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 16/18] migration: Drop migration_is_idle()
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (14 preceding siblings ...)
2024-10-30 15:57 ` [PULL 15/18] migration: Drop migration_is_setup_or_active() Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 17/18] migration/ram: Add load start trace event Peter Xu
` (2 subsequent siblings)
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Peter Xu, Cédric Le Goater
Now with the current migration_is_running(), it will report exactly the
opposite of what will be reported by migration_is_idle().
Drop migration_is_idle(), instead use "!migration_is_running()" which
should be identical on functionality.
In reality, most of the idle check is inverted, so it's even easier to
write with "migrate_is_running()" check.
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20241024213056.1395400-6-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
include/migration/misc.h | 1 -
hw/virtio/virtio-mem.c | 2 +-
migration/migration.c | 21 +--------------------
migration/ram.c | 2 +-
system/qdev-monitor.c | 4 ++--
5 files changed, 5 insertions(+), 25 deletions(-)
diff --git a/include/migration/misc.h b/include/migration/misc.h
index 86ef160f19..804eb23c06 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -53,7 +53,6 @@ void dump_vmstate_json_to_file(FILE *out_fp);
void migration_object_init(void);
void migration_shutdown(void);
-bool migration_is_idle(void);
bool migration_is_active(void);
bool migration_is_device(void);
bool migration_is_running(void);
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index ae1e81d7ba..80ada89551 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -188,7 +188,7 @@ static bool virtio_mem_is_busy(void)
* after plugging them) until we're running on the destination (as we didn't
* migrate these blocks when they were unplugged).
*/
- return migration_in_incoming_postcopy() || !migration_is_idle();
+ return migration_in_incoming_postcopy() || migration_is_running();
}
typedef int (*virtio_mem_range_cb)(VirtIOMEM *vmem, void *arg,
diff --git a/migration/migration.c b/migration/migration.c
index cab65ba8db..04d7e67897 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1633,25 +1633,6 @@ bool migration_in_bg_snapshot(void)
return migrate_background_snapshot() && migration_is_running();
}
-bool migration_is_idle(void)
-{
- MigrationState *s = current_migration;
-
- if (!s) {
- return true;
- }
-
- switch (s->state) {
- case MIGRATION_STATUS_NONE:
- case MIGRATION_STATUS_CANCELLED:
- case MIGRATION_STATUS_COMPLETED:
- case MIGRATION_STATUS_FAILED:
- return true;
- default:
- return false;
- }
-}
-
bool migration_is_active(void)
{
MigrationState *s = current_migration;
@@ -1730,7 +1711,7 @@ static bool is_busy(Error **reasonp, Error **errp)
ERRP_GUARD();
/* Snapshots are similar to migrations, so check RUN_STATE_SAVE_VM too. */
- if (runstate_check(RUN_STATE_SAVE_VM) || !migration_is_idle()) {
+ if (runstate_check(RUN_STATE_SAVE_VM) || migration_is_running()) {
error_propagate_prepend(errp, *reasonp,
"disallowing migration blocker "
"(migration/snapshot in progress) for: ");
diff --git a/migration/ram.c b/migration/ram.c
index 5646a0b882..504b48d584 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4498,7 +4498,7 @@ static void ram_mig_ram_block_resized(RAMBlockNotifier *n, void *host,
return;
}
- if (!migration_is_idle()) {
+ if (migration_is_running()) {
/*
* Precopy code on the source cannot deal with the size of RAM blocks
* changing at random points in time - especially after sending the
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 44994ea0e1..320c47b72d 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -679,7 +679,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
return NULL;
}
- if (!migration_is_idle()) {
+ if (migration_is_running()) {
error_setg(errp, "device_add not allowed while migrating");
return NULL;
}
@@ -928,7 +928,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
return;
}
- if (!migration_is_idle() && !dev->allow_unplug_during_migration) {
+ if (migration_is_running() && !dev->allow_unplug_during_migration) {
error_setg(errp, "device_del not allowed while migrating");
return;
}
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 17/18] migration/ram: Add load start trace event
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (15 preceding siblings ...)
2024-10-30 15:57 ` [PULL 16/18] migration: Drop migration_is_idle() Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-30 15:57 ` [PULL 18/18] migration/multifd: Zero p->flags before starting filling a packet Peter Xu
2024-10-31 13:28 ` [PULL 00/18] Migration 20241030 patches Peter Maydell
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Maciej S. Szmigiero, Peter Xu
From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
There's a RAM load complete trace event but there wasn't its start equivalent.
Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/94ddfa7ecb83a78f73b82867dd30c8767592d257.1730203967.git.maciej.szmigiero@oracle.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/ram.c | 1 +
migration/trace-events | 1 +
2 files changed, 2 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index 504b48d584..12031df4e5 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -4294,6 +4294,7 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id)
* it will be necessary to reduce the granularity of this
* critical section.
*/
+ trace_ram_load_start();
WITH_RCU_READ_LOCK_GUARD() {
if (postcopy_running) {
/*
diff --git a/migration/trace-events b/migration/trace-events
index 0638183056..bb0e0cc6dc 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -115,6 +115,7 @@ colo_flush_ram_cache_end(void) ""
save_xbzrle_page_skipping(void) ""
save_xbzrle_page_overflow(void) ""
ram_save_iterate_big_wait(uint64_t milliconds, int iterations) "big wait: %" PRIu64 " milliseconds, %d iterations"
+ram_load_start(void) ""
ram_load_complete(int ret, uint64_t seq_iter) "exit_code %d seq iteration %" PRIu64
ram_write_tracking_ramblock_start(const char *block_id, size_t page_size, void *addr, size_t length) "%s: page_size: %zu addr: %p length: %zu"
ram_write_tracking_ramblock_stop(const char *block_id, size_t page_size, void *addr, size_t length) "%s: page_size: %zu addr: %p length: %zu"
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PULL 18/18] migration/multifd: Zero p->flags before starting filling a packet
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (16 preceding siblings ...)
2024-10-30 15:57 ` [PULL 17/18] migration/ram: Add load start trace event Peter Xu
@ 2024-10-30 15:57 ` Peter Xu
2024-10-31 13:28 ` [PULL 00/18] Migration 20241030 patches Peter Maydell
18 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-30 15:57 UTC (permalink / raw)
To: qemu-devel; +Cc: Fabiano Rosas, Peter Maydell, Maciej S. Szmigiero, Peter Xu
From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com>
This way there aren't stale flags there.
p->flags can't contain SYNC to be sent at the next RAM packet since syncs
are now handled separately in multifd_send_thread.
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
Link: https://lore.kernel.org/r/1c96b6cdb797e6f035eb1a4ad9bfc24f4c7f5df8.1730203967.git.maciej.szmigiero@oracle.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/multifd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/migration/multifd.c b/migration/multifd.c
index 697fe86fdf..4374e14a96 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -600,6 +600,7 @@ static void *multifd_send_thread(void *opaque)
* qatomic_store_release() in multifd_send().
*/
if (qatomic_load_acquire(&p->pending_job)) {
+ p->flags = 0;
p->iovs_num = 0;
assert(!multifd_payload_empty(p->data));
@@ -651,7 +652,6 @@ static void *multifd_send_thread(void *opaque)
}
/* p->next_packet_size will always be zero for a SYNC packet */
stat64_add(&mig_stats.multifd_bytes, p->packet_len);
- p->flags = 0;
}
qatomic_set(&p->pending_sync, false);
--
2.45.0
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PULL 00/18] Migration 20241030 patches
2024-10-30 15:57 [PULL 00/18] Migration 20241030 patches Peter Xu
` (17 preceding siblings ...)
2024-10-30 15:57 ` [PULL 18/18] migration/multifd: Zero p->flags before starting filling a packet Peter Xu
@ 2024-10-31 13:28 ` Peter Maydell
2024-10-31 14:50 ` Peter Xu
18 siblings, 1 reply; 23+ messages in thread
From: Peter Maydell @ 2024-10-31 13:28 UTC (permalink / raw)
To: Peter Xu; +Cc: qemu-devel, Fabiano Rosas
On Wed, 30 Oct 2024 at 15:57, Peter Xu <peterx@redhat.com> wrote:
>
> The following changes since commit cc5adbbd50d81555b8eb73602ec16fde40b55be4:
>
> Merge tag 'pull-tpm-2024-10-18-1' of https://github.com/stefanberger/qemu-tpm into staging (2024-10-18 15:45:02 +0100)
>
> are available in the Git repository at:
>
> https://gitlab.com/peterx/qemu.git tags/migration-20241030-pull-request
>
> for you to fetch changes up to 53a60118d2654dd8e595e61f4e767ff747fd0b69:
>
> migration/multifd: Zero p->flags before starting filling a packet (2024-10-30 11:32:41 -0400)
>
> ----------------------------------------------------------------
> Migration pull request for softfreeze
>
> NOTE: checkpatch.pl could report a false positive on this branch:
>
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #21:
> {include/sysemu => migration}/cpu-throttle.h | 0
>
> That's covered by "F: migration/" entry.
>
> Changelog:
>
> - Peter's cleanup patch on migrate_fd_cleanup()
> - Peter's cleanup patch to introduce thread name macros
> - Hanna's error path fix for vmstate subsection save()s
> - Hyman's auto converge enhancement on background dirty sync
> - Peter's additional tracepoints for save state entries
> - Thomas's build fix for OpenBSD in dirtyrate.c
> - Peter's deprecation of query-migrationthreads command
> - Peter's cleanup/fixes from the "export misc.h" series
> - Maciej's two small patches from multifd+vfio series
> {include/sysemu => migration}/cpu-throttle.h | 14 ++
Hi; this fails to build on macos:
https://gitlab.com/qemu-project/qemu/-/jobs/8237753019
../ui/cocoa.m:40:10: fatal error: 'sysemu/cpu-throttle.h' file not found
40 | #include "sysemu/cpu-throttle.h"
| ^~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
thanks
-- PMM
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PULL 00/18] Migration 20241030 patches
2024-10-31 13:28 ` [PULL 00/18] Migration 20241030 patches Peter Maydell
@ 2024-10-31 14:50 ` Peter Xu
2024-10-31 14:52 ` Peter Maydell
0 siblings, 1 reply; 23+ messages in thread
From: Peter Xu @ 2024-10-31 14:50 UTC (permalink / raw)
To: Peter Maydell, Yong Huang; +Cc: qemu-devel, Fabiano Rosas
On Thu, Oct 31, 2024 at 01:28:31PM +0000, Peter Maydell wrote:
> On Wed, 30 Oct 2024 at 15:57, Peter Xu <peterx@redhat.com> wrote:
> >
> > The following changes since commit cc5adbbd50d81555b8eb73602ec16fde40b55be4:
> >
> > Merge tag 'pull-tpm-2024-10-18-1' of https://github.com/stefanberger/qemu-tpm into staging (2024-10-18 15:45:02 +0100)
> >
> > are available in the Git repository at:
> >
> > https://gitlab.com/peterx/qemu.git tags/migration-20241030-pull-request
> >
> > for you to fetch changes up to 53a60118d2654dd8e595e61f4e767ff747fd0b69:
> >
> > migration/multifd: Zero p->flags before starting filling a packet (2024-10-30 11:32:41 -0400)
> >
> > ----------------------------------------------------------------
> > Migration pull request for softfreeze
> >
> > NOTE: checkpatch.pl could report a false positive on this branch:
> >
> > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> > #21:
> > {include/sysemu => migration}/cpu-throttle.h | 0
> >
> > That's covered by "F: migration/" entry.
> >
> > Changelog:
> >
> > - Peter's cleanup patch on migrate_fd_cleanup()
> > - Peter's cleanup patch to introduce thread name macros
> > - Hanna's error path fix for vmstate subsection save()s
> > - Hyman's auto converge enhancement on background dirty sync
> > - Peter's additional tracepoints for save state entries
> > - Thomas's build fix for OpenBSD in dirtyrate.c
> > - Peter's deprecation of query-migrationthreads command
> > - Peter's cleanup/fixes from the "export misc.h" series
> > - Maciej's two small patches from multifd+vfio series
>
>
> > {include/sysemu => migration}/cpu-throttle.h | 14 ++
>
> Hi; this fails to build on macos:
>
> https://gitlab.com/qemu-project/qemu/-/jobs/8237753019
>
> ../ui/cocoa.m:40:10: fatal error: 'sysemu/cpu-throttle.h' file not found
> 40 | #include "sysemu/cpu-throttle.h"
> | ^~~~~~~~~~~~~~~~~~~~~~~
> 1 error generated.
Errr, cocoa.m used cpu-throttle for Speed feature.. so it needs it to be
exported.
Luckily cocoa.m is only compiled in system_ss. The simplest fix so far is
to only move cpu-throttle.c, not cpu-throttle.h yet in the commit:
"migration: Move cpu-throttole.c from system to migration"
Fixup patch to be squashed:
===8<===
From 68515db81e28832cbd24b1cdbc12aeb618c9de54 Mon Sep 17 00:00:00 2001
From: Peter Xu <peterx@redhat.com>
Date: Thu, 31 Oct 2024 10:37:29 -0400
Subject: [PATCH] fixup! migration: Move cpu-throttole.c from system to
migration
Signed-off-by: Peter Xu <peterx@redhat.com>
---
{migration => include/sysemu}/cpu-throttle.h | 0
migration/cpu-throttle.c | 2 +-
migration/migration.c | 2 +-
migration/ram.c | 2 +-
4 files changed, 3 insertions(+), 3 deletions(-)
rename {migration => include/sysemu}/cpu-throttle.h (100%)
diff --git a/migration/cpu-throttle.h b/include/sysemu/cpu-throttle.h
similarity index 100%
rename from migration/cpu-throttle.h
rename to include/sysemu/cpu-throttle.h
diff --git a/migration/cpu-throttle.c b/migration/cpu-throttle.c
index 3df287d8d3..5179019e33 100644
--- a/migration/cpu-throttle.c
+++ b/migration/cpu-throttle.c
@@ -27,7 +27,7 @@
#include "hw/core/cpu.h"
#include "qemu/main-loop.h"
#include "sysemu/cpus.h"
-#include "cpu-throttle.h"
+#include "sysemu/cpu-throttle.h"
#include "migration.h"
#include "migration-stats.h"
#include "trace.h"
diff --git a/migration/migration.c b/migration/migration.c
index 04d7e67897..aedf7f0751 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -24,7 +24,7 @@
#include "socket.h"
#include "sysemu/runstate.h"
#include "sysemu/sysemu.h"
-#include "cpu-throttle.h"
+#include "sysemu/cpu-throttle.h"
#include "rdma.h"
#include "ram.h"
#include "migration/global_state.h"
diff --git a/migration/ram.c b/migration/ram.c
index 12031df4e5..05ff9eb328 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -52,7 +52,7 @@
#include "exec/target_page.h"
#include "qemu/rcu_queue.h"
#include "migration/colo.h"
-#include "cpu-throttle.h"
+#include "sysemu/cpu-throttle.h"
#include "savevm.h"
#include "qemu/iov.h"
#include "multifd.h"
--
2.45.0
===8<===
Hyman/Fabiano, feel free to comment if there's any objections. I'll wait
for 1 day for that before I send the v2 pull with the planned fix.
Thanks,
--
Peter Xu
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PULL 00/18] Migration 20241030 patches
2024-10-31 14:50 ` Peter Xu
@ 2024-10-31 14:52 ` Peter Maydell
2024-10-31 15:14 ` Peter Xu
0 siblings, 1 reply; 23+ messages in thread
From: Peter Maydell @ 2024-10-31 14:52 UTC (permalink / raw)
To: Peter Xu; +Cc: Yong Huang, qemu-devel, Fabiano Rosas
On Thu, 31 Oct 2024 at 14:50, Peter Xu <peterx@redhat.com> wrote:
>
> On Thu, Oct 31, 2024 at 01:28:31PM +0000, Peter Maydell wrote:
> > Hi; this fails to build on macos:
> >
> > https://gitlab.com/qemu-project/qemu/-/jobs/8237753019
> >
> > ../ui/cocoa.m:40:10: fatal error: 'sysemu/cpu-throttle.h' file not found
> > 40 | #include "sysemu/cpu-throttle.h"
> > | ^~~~~~~~~~~~~~~~~~~~~~~
> > 1 error generated.
>
> Errr, cocoa.m used cpu-throttle for Speed feature.. so it needs it to be
> exported.
>
> Luckily cocoa.m is only compiled in system_ss. The simplest fix so far is
> to only move cpu-throttle.c, not cpu-throttle.h yet in the commit:
>
> "migration: Move cpu-throttole.c from system to migration"
>
> Fixup patch to be squashed:
>
> ===8<===
> From 68515db81e28832cbd24b1cdbc12aeb618c9de54 Mon Sep 17 00:00:00 2001
> From: Peter Xu <peterx@redhat.com>
> Date: Thu, 31 Oct 2024 10:37:29 -0400
> Subject: [PATCH] fixup! migration: Move cpu-throttole.c from system to
> migration
If you're fixing up that commit anyway you could also fix
the typo in the commit message: s/throttole/throttle/.
-- PMM
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PULL 00/18] Migration 20241030 patches
2024-10-31 14:52 ` Peter Maydell
@ 2024-10-31 15:14 ` Peter Xu
0 siblings, 0 replies; 23+ messages in thread
From: Peter Xu @ 2024-10-31 15:14 UTC (permalink / raw)
To: Peter Maydell; +Cc: Yong Huang, qemu-devel, Fabiano Rosas
On Thu, Oct 31, 2024 at 02:52:28PM +0000, Peter Maydell wrote:
> On Thu, 31 Oct 2024 at 14:50, Peter Xu <peterx@redhat.com> wrote:
> >
> > On Thu, Oct 31, 2024 at 01:28:31PM +0000, Peter Maydell wrote:
> > > Hi; this fails to build on macos:
> > >
> > > https://gitlab.com/qemu-project/qemu/-/jobs/8237753019
> > >
> > > ../ui/cocoa.m:40:10: fatal error: 'sysemu/cpu-throttle.h' file not found
> > > 40 | #include "sysemu/cpu-throttle.h"
> > > | ^~~~~~~~~~~~~~~~~~~~~~~
> > > 1 error generated.
> >
> > Errr, cocoa.m used cpu-throttle for Speed feature.. so it needs it to be
> > exported.
> >
> > Luckily cocoa.m is only compiled in system_ss. The simplest fix so far is
> > to only move cpu-throttle.c, not cpu-throttle.h yet in the commit:
> >
> > "migration: Move cpu-throttole.c from system to migration"
> >
> > Fixup patch to be squashed:
> >
> > ===8<===
> > From 68515db81e28832cbd24b1cdbc12aeb618c9de54 Mon Sep 17 00:00:00 2001
> > From: Peter Xu <peterx@redhat.com>
> > Date: Thu, 31 Oct 2024 10:37:29 -0400
> > Subject: [PATCH] fixup! migration: Move cpu-throttole.c from system to
> > migration
>
> If you're fixing up that commit anyway you could also fix
> the typo in the commit message: s/throttole/throttle/.
Will do.
--
Peter Xu
^ permalink raw reply [flat|nested] 23+ messages in thread