colo: Reuse the return path from migration on primary and secondary side
Use the return-path capability with colo and reuse the opened return path file on both primary and secondary side. This fixes a crash in colo where migration_cancel() races with colo closing s->rp_state.from_dst_file. Signed-off-by: Lukas Straub <lukasstraub2@web.de> Reviewed-by: Peter Xu <peterx@redhat.com> Link: https://lore.kernel.org/qemu-devel/20260302-colo_unit_test_multifd-v11-21-d653fb3b1d80@web.de Signed-off-by: Fabiano Rosas <farosas@suse.de>
Lukas Straub committed
Mar 2, 2026 at 12:45 UTC
161e6039759a77debee9434ee55a086c8a85ed60
5 files changed
+34
-20
docs/system/qemu-colo.rst
+2
-2
@@ -227,7 +227,7 @@ any IP's here, except for the ``$primary_ip`` variable::
227
**3.** On Secondary VM's QEMU monitor, issue command::
228
229
{"execute":"qmp_capabilities"}
230
- {"execute": "migrate-set-capabilities", "arguments": {"capabilities": [ {"capability": "x-colo", "state": true } ] } }
230
+ {"execute": "migrate-set-capabilities", "arguments": {"capabilities": [ {"capability": "return-path", "state": true }, {"capability": "x-colo", "state": true } ] } }
231
{"execute": "nbd-server-start", "arguments": {"addr": {"type": "inet", "data": {"host": "0.0.0.0", "port": "9999"} } } }
232
{"execute": "nbd-server-add", "arguments": {"device": "parent0", "writable": true } }
233
@@ -244,7 +244,7 @@ Note:
244
{"execute":"qmp_capabilities"}
245
{"execute": "blockdev-add", "arguments": {"driver": "nbd", "node-name": "nbd0", "server": {"type": "inet", "host": "127.0.0.2", "port": "9999"}, "export": "parent0", "detect-zeroes": "on"} }
246
{"execute": "x-blockdev-change", "arguments":{"parent": "colo-disk0", "node": "nbd0" } }
247
- {"execute": "migrate-set-capabilities", "arguments": {"capabilities": [ {"capability": "x-colo", "state": true } ] } }
247
+ {"execute": "migrate-set-capabilities", "arguments": {"capabilities": [ {"capability": "return-path", "state": true }, {"capability": "x-colo", "state": true } ] } }
248
{"execute": "migrate", "arguments": {"uri": "tcp:127.0.0.2:9998" } }
249
250
Note:
migration/colo.c
+9
-17
@@ -539,6 +539,8 @@ static void colo_process_checkpoint(MigrationState *s)
539
Error *local_err = NULL;
540
int ret;
541
542
+ assert(s->rp_state.from_dst_file);
543
+ assert(!s->rp_state.rp_thread_created);
544
if (get_colo_mode() != COLO_MODE_PRIMARY) {
545
error_report("COLO mode must be COLO_MODE_PRIMARY");
546
return;
@@ -546,12 +548,6 @@ static void colo_process_checkpoint(MigrationState *s)
548
549
failover_init_state();
550
549
- s->rp_state.from_dst_file = qemu_file_get_return_path(s->to_dst_file);
550
- if (!s->rp_state.from_dst_file) {
551
- error_report("Open QEMUFile from_dst_file failed");
552
- goto out;
553
- }
554
-
551
packets_compare_notifier.notify = colo_compare_notify_checkpoint;
552
colo_compare_register_notifier(&packets_compare_notifier);
553
@@ -636,16 +632,6 @@ out:
632
colo_compare_unregister_notifier(&packets_compare_notifier);
633
timer_free(s->colo_delay_timer);
634
qemu_event_destroy(&s->colo_checkpoint_event);
639
-
640
- /*
641
- * Must be called after failover BH is completed,
642
- * Or the failover BH may shutdown the wrong fd that
643
- * re-used by other threads after we release here.
644
- */
645
- if (s->rp_state.from_dst_file) {
646
- qemu_fclose(s->rp_state.from_dst_file);
647
- s->rp_state.from_dst_file = NULL;
648
- }
635
}
636
637
void migrate_start_colo_process(MigrationState *s)
@@ -838,6 +824,7 @@ static void *colo_process_incoming_thread(void *opaque)
824
migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
825
MIGRATION_STATUS_COLO);
826
827
+ assert(mis->to_src_file);
828
if (get_colo_mode() != COLO_MODE_SECONDARY) {
829
error_report("COLO mode must be COLO_MODE_SECONDARY");
830
return NULL;
@@ -854,7 +841,6 @@ static void *colo_process_incoming_thread(void *opaque)
841
842
failover_init_state();
843
857
- mis->to_src_file = qemu_file_get_return_path(mis->from_src_file);
844
/*
845
* Note: the communication between Primary side and Secondary side
846
* should be sequential, we set the fd to unblocked in migration incoming
@@ -866,6 +852,12 @@ static void *colo_process_incoming_thread(void *opaque)
852
goto out;
853
}
854
855
+ /*
856
+ * rp thread still running on primary side, shut it down to go into
857
+ * colo state.
858
+ */
859
+ migrate_send_rp_shut(mis, 0);
860
+
861
colo_incoming_start_dirty_log();
862
863
bioc = qio_channel_buffer_new(COLO_BUFFER_BASE_SIZE);
migration/options.c
+9
-1
@@ -575,7 +575,15 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
575
ERRP_GUARD();
576
MigrationIncomingState *mis = migration_incoming_get_current();
577
578
-#ifndef CONFIG_REPLICATION
578
+#ifdef CONFIG_REPLICATION
579
+ if (new_caps[MIGRATION_CAPABILITY_X_COLO]) {
580
+ if (!new_caps[MIGRATION_CAPABILITY_RETURN_PATH]) {
581
+ error_setg(errp, "Capability 'x-colo' requires capability "
582
+ "'return-path'");
583
+ return false;
584
+ }
585
+ }
586
+#else
587
if (new_caps[MIGRATION_CAPABILITY_X_COLO]) {
588
error_setg(errp, "QEMU compiled without replication module"
589
" can't enable COLO");
tests/qtest/migration/colo-tests.c
+1
@@ -42,6 +42,7 @@ static int test_colo_common(MigrateCommon *args,
42
* used in production.
43
*/
44
args->start.oob = true;
45
+ args->start.caps[MIGRATION_CAPABILITY_RETURN_PATH] = true;
46
args->start.caps[MIGRATION_CAPABILITY_X_COLO] = true;
47
48
if (migrate_start(&from, &to, args->listen_uri, &args->start)) {
tests/qtest/migration/framework.c
+13
@@ -216,6 +216,19 @@ static void migrate_start_set_capabilities(QTestState *from, QTestState *to,
216
* MigrationCapability_lookup and MIGRATION_CAPABILITY_ constants
217
* are from qapi-types-migration.h.
218
*/
219
+
220
+ /*
221
+ * Enable return path first, since other features depend on it.
222
+ */
223
+ if (args->caps[MIGRATION_CAPABILITY_RETURN_PATH]) {
224
+ if (from) {
225
+ migrate_set_capability(from, "return-path", true);
226
+ }
227
+ if (to) {
228
+ migrate_set_capability(to, "return-path", true);
229
+ }
230
+ }
231
+
232
for (uint8_t i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
233
if (!args->caps[i]) {
234
continue;