All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 00/10] Migration 20230509 patches
@ 2023-05-09 19:17 Juan Quintela
  2023-05-09 19:17 ` [PULL 01/10] ram: Add public helper to set colo bitmap Juan Quintela
                   ` (10 more replies)
  0 siblings, 11 replies; 17+ messages in thread
From: Juan Quintela @ 2023-05-09 19:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz, Kevin Wolf,
	Paolo Bonzini, Hailiang Zhang, Eric Blake, Juan Quintela,
	qemu-block, Peter Xu, Markus Armbruster

The following changes since commit 271477b59e723250f17a7e20f139262057921b6a:

  Merge tag 'compression-code-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-05-08 20:38:05 +0100)

are available in the Git repository at:

  https://gitlab.com/juan.quintela/qemu.git tags/migration-20230509-pull-request

for you to fetch changes up to 5f43d297bc2b9530805ad8602c6e2ea284b08628:

  migration: block incoming colo when capability is disabled (2023-05-09 20:52:21 +0200)

----------------------------------------------------------------
Migration Pull request (20230509 vintage)

Hi
In this PULL request:
- 1st part of colo support for multifd (lukas)
- 1st part of disabling colo option (vladimir)

Please, apply.

----------------------------------------------------------------

Lukas Straub (3):
  ram: Add public helper to set colo bitmap
  ram: Let colo_flush_ram_cache take the bitmap_mutex
  multifd: Add the ramblock to MultiFDRecvParams

Vladimir Sementsov-Ogievskiy (7):
  block/meson.build: prefer positive condition for replication
  colo: make colo_checkpoint_notify static and provide simpler API
  build: move COLO under CONFIG_REPLICATION
  migration: drop colo_incoming_thread from MigrationIncomingState
  migration: process_incoming_migration_co: simplify code flow around
    ret
  migration: disallow change capabilities in COLO state
  migration: block incoming colo when capability is disabled

 block/meson.build              |  2 +-
 docs/COLO-FT.txt               |  1 +
 hmp-commands.hx                |  2 ++
 include/migration/colo.h       |  9 +++++-
 migration/colo.c               | 57 +++++++++++-----------------------
 migration/meson.build          |  6 ++--
 migration/migration-hmp-cmds.c |  2 ++
 migration/migration.c          | 35 ++++++++++++++-------
 migration/migration.h          |  2 --
 migration/multifd.c            | 11 +++----
 migration/multifd.h            |  2 ++
 migration/options.c            |  6 ++--
 migration/ram.c                | 19 ++++++++++--
 migration/ram.h                |  1 +
 qapi/migration.json            |  9 ++++--
 stubs/colo.c                   | 39 +++++++++++++++++++++++
 stubs/meson.build              |  1 +
 17 files changed, 131 insertions(+), 73 deletions(-)
 create mode 100644 stubs/colo.c

-- 
2.40.0



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PULL 01/10] ram: Add public helper to set colo bitmap
  2023-05-09 19:17 [PULL 00/10] Migration 20230509 patches Juan Quintela
@ 2023-05-09 19:17 ` Juan Quintela
  2023-05-09 19:17 ` [PULL 02/10] ram: Let colo_flush_ram_cache take the bitmap_mutex Juan Quintela
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Juan Quintela @ 2023-05-09 19:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz, Kevin Wolf,
	Paolo Bonzini, Hailiang Zhang, Eric Blake, Juan Quintela,
	qemu-block, Peter Xu, Markus Armbruster, Lukas Straub

From: Lukas Straub <lukasstraub2@web.de>

The overhead of the mutex in non-multifd mode is negligible,
because in that case its just the single thread taking the mutex.

This will be used in the next commits to add colo support to multifd.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <22d83cb428f37929563155531bfb69fd8953cc61.1683572883.git.lukasstraub2@web.de>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c | 17 ++++++++++++++---
 migration/ram.h |  1 +
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index f78e9912cd..0346c1c1ed 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3408,6 +3408,18 @@ static ram_addr_t host_page_offset_from_ram_block_offset(RAMBlock *block,
     return ((uintptr_t)block->host + offset) & (block->page_size - 1);
 }
 
+void colo_record_bitmap(RAMBlock *block, ram_addr_t *normal, uint normal_num)
+{
+    qemu_mutex_lock(&ram_state->bitmap_mutex);
+    for (int i = 0; i < normal_num; i++) {
+        ram_addr_t offset = normal[i];
+        ram_state->migration_dirty_pages += !test_and_set_bit(
+                                                offset >> TARGET_PAGE_BITS,
+                                                block->bmap);
+    }
+    qemu_mutex_unlock(&ram_state->bitmap_mutex);
+}
+
 static inline void *colo_cache_from_block_offset(RAMBlock *block,
                              ram_addr_t offset, bool record_bitmap)
 {
@@ -3425,9 +3437,8 @@ static inline void *colo_cache_from_block_offset(RAMBlock *block,
     * It help us to decide which pages in ram cache should be flushed
     * into VM's RAM later.
     */
-    if (record_bitmap &&
-        !test_and_set_bit(offset >> TARGET_PAGE_BITS, block->bmap)) {
-        ram_state->migration_dirty_pages++;
+    if (record_bitmap) {
+        colo_record_bitmap(block, &offset, 1);
     }
     return block->colo_cache + offset;
 }
diff --git a/migration/ram.h b/migration/ram.h
index 6fffbeb5f1..887d1fbae6 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -82,6 +82,7 @@ int colo_init_ram_cache(void);
 void colo_flush_ram_cache(void);
 void colo_release_ram_cache(void);
 void colo_incoming_start_dirty_log(void);
+void colo_record_bitmap(RAMBlock *block, ram_addr_t *normal, uint normal_num);
 
 /* Background snapshot */
 bool ram_write_tracking_available(void);
-- 
2.40.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PULL 02/10] ram: Let colo_flush_ram_cache take the bitmap_mutex
  2023-05-09 19:17 [PULL 00/10] Migration 20230509 patches Juan Quintela
  2023-05-09 19:17 ` [PULL 01/10] ram: Add public helper to set colo bitmap Juan Quintela
@ 2023-05-09 19:17 ` Juan Quintela
  2023-05-09 19:17 ` [PULL 03/10] multifd: Add the ramblock to MultiFDRecvParams Juan Quintela
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Juan Quintela @ 2023-05-09 19:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz, Kevin Wolf,
	Paolo Bonzini, Hailiang Zhang, Eric Blake, Juan Quintela,
	qemu-block, Peter Xu, Markus Armbruster, Lukas Straub

From: Lukas Straub <lukasstraub2@web.de>

This is not required, colo_flush_ram_cache does not run concurrently
with the multifd threads since the cache is only flushed after
everything has been received. But it makes me more comfortable.

This will be used in the next commits to add colo support to multifd.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <35cb23ba854151d38a31e3a5c8a1020e4283cb4a.1683572883.git.lukasstraub2@web.de>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/ram.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/migration/ram.c b/migration/ram.c
index 0346c1c1ed..3fa720dad9 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3814,6 +3814,7 @@ void colo_flush_ram_cache(void)
     unsigned long offset = 0;
 
     memory_global_dirty_log_sync();
+    qemu_mutex_lock(&ram_state->bitmap_mutex);
     WITH_RCU_READ_LOCK_GUARD() {
         RAMBLOCK_FOREACH_NOT_IGNORED(block) {
             ramblock_sync_dirty_bitmap(ram_state, block);
@@ -3848,6 +3849,7 @@ void colo_flush_ram_cache(void)
             }
         }
     }
+    qemu_mutex_unlock(&ram_state->bitmap_mutex);
     trace_colo_flush_ram_cache_end();
 }
 
-- 
2.40.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PULL 03/10] multifd: Add the ramblock to MultiFDRecvParams
  2023-05-09 19:17 [PULL 00/10] Migration 20230509 patches Juan Quintela
  2023-05-09 19:17 ` [PULL 01/10] ram: Add public helper to set colo bitmap Juan Quintela
  2023-05-09 19:17 ` [PULL 02/10] ram: Let colo_flush_ram_cache take the bitmap_mutex Juan Quintela
@ 2023-05-09 19:17 ` Juan Quintela
  2023-05-09 19:17 ` [PULL 04/10] block/meson.build: prefer positive condition for replication Juan Quintela
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Juan Quintela @ 2023-05-09 19:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz, Kevin Wolf,
	Paolo Bonzini, Hailiang Zhang, Eric Blake, Juan Quintela,
	qemu-block, Peter Xu, Markus Armbruster, Lukas Straub

From: Lukas Straub <lukasstraub2@web.de>

This will be used in the next commits to add colo support to multifd.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <88135197411df1a71d7832962b39abf60faf0021.1683572883.git.lukasstraub2@web.de>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/multifd.c | 11 +++++------
 migration/multifd.h |  2 ++
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index 4e71c19292..5c4298eadf 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -281,7 +281,6 @@ static void multifd_send_fill_packet(MultiFDSendParams *p)
 static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
 {
     MultiFDPacket_t *packet = p->packet;
-    RAMBlock *block;
     int i;
 
     packet->magic = be32_to_cpu(packet->magic);
@@ -331,21 +330,21 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
 
     /* make sure that ramblock is 0 terminated */
     packet->ramblock[255] = 0;
-    block = qemu_ram_block_by_name(packet->ramblock);
-    if (!block) {
+    p->block = qemu_ram_block_by_name(packet->ramblock);
+    if (!p->block) {
         error_setg(errp, "multifd: unknown ram block %s",
                    packet->ramblock);
         return -1;
     }
 
-    p->host = block->host;
+    p->host = p->block->host;
     for (i = 0; i < p->normal_num; i++) {
         uint64_t offset = be64_to_cpu(packet->offset[i]);
 
-        if (offset > (block->used_length - p->page_size)) {
+        if (offset > (p->block->used_length - p->page_size)) {
             error_setg(errp, "multifd: offset too long %" PRIu64
                        " (max " RAM_ADDR_FMT ")",
-                       offset, block->used_length);
+                       offset, p->block->used_length);
             return -1;
         }
         p->normal[i] = offset;
diff --git a/migration/multifd.h b/migration/multifd.h
index 7cfc265148..a835643b48 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -175,6 +175,8 @@ typedef struct {
     uint32_t next_packet_size;
     /* packets sent through this channel */
     uint64_t num_packets;
+    /* ramblock */
+    RAMBlock *block;
     /* ramblock host address */
     uint8_t *host;
     /* non zero pages recv through this channel */
-- 
2.40.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PULL 04/10] block/meson.build: prefer positive condition for replication
  2023-05-09 19:17 [PULL 00/10] Migration 20230509 patches Juan Quintela
                   ` (2 preceding siblings ...)
  2023-05-09 19:17 ` [PULL 03/10] multifd: Add the ramblock to MultiFDRecvParams Juan Quintela
@ 2023-05-09 19:17 ` Juan Quintela
  2023-05-09 19:17 ` [PULL 05/10] colo: make colo_checkpoint_notify static and provide simpler API Juan Quintela
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Juan Quintela @ 2023-05-09 19:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz, Kevin Wolf,
	Paolo Bonzini, Hailiang Zhang, Eric Blake, Juan Quintela,
	qemu-block, Peter Xu, Markus Armbruster,
	Vladimir Sementsov-Ogievskiy, Philippe Mathieu-Daudé,
	Lukas Straub, Zhang Chen

From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Message-Id: <20230428194928.1426370-2-vsementsov@yandex-team.ru>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 block/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/meson.build b/block/meson.build
index 382bec0e7d..b9a72e219b 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -84,7 +84,7 @@ block_ss.add(when: 'CONFIG_WIN32', if_true: files('file-win32.c', 'win32-aio.c')
 block_ss.add(when: 'CONFIG_POSIX', if_true: [files('file-posix.c'), coref, iokit])
 block_ss.add(when: libiscsi, if_true: files('iscsi-opts.c'))
 block_ss.add(when: 'CONFIG_LINUX', if_true: files('nvme.c'))
-if not get_option('replication').disabled()
+if get_option('replication').allowed()
   block_ss.add(files('replication.c'))
 endif
 block_ss.add(when: libaio, if_true: files('linux-aio.c'))
-- 
2.40.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PULL 05/10] colo: make colo_checkpoint_notify static and provide simpler API
  2023-05-09 19:17 [PULL 00/10] Migration 20230509 patches Juan Quintela
                   ` (3 preceding siblings ...)
  2023-05-09 19:17 ` [PULL 04/10] block/meson.build: prefer positive condition for replication Juan Quintela
@ 2023-05-09 19:17 ` Juan Quintela
  2023-05-09 19:17 ` [PULL 06/10] build: move COLO under CONFIG_REPLICATION Juan Quintela
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Juan Quintela @ 2023-05-09 19:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz, Kevin Wolf,
	Paolo Bonzini, Hailiang Zhang, Eric Blake, Juan Quintela,
	qemu-block, Peter Xu, Markus Armbruster,
	Vladimir Sementsov-Ogievskiy, Zhang Chen

From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

colo_checkpoint_notify() is mostly used in colo.c. Outside we use it
once when x-checkpoint-delay migration parameter is set. So, let's
simplify the external API to only that function - notify COLO that
parameter was set. This make external API more robust and hides
implementation details from external callers. Also this helps us to
make COLO module optional in further patch (i.e. we are going to add
possibility not build the COLO module).

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Message-Id: <20230428194928.1426370-3-vsementsov@yandex-team.ru>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 include/migration/colo.h |  9 ++++++++-
 migration/colo.c         | 29 ++++++++++++++++++-----------
 migration/options.c      |  4 +---
 3 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/include/migration/colo.h b/include/migration/colo.h
index 5fbe1a6d5d..7ef315473e 100644
--- a/include/migration/colo.h
+++ b/include/migration/colo.h
@@ -36,6 +36,13 @@ COLOMode get_colo_mode(void);
 /* failover */
 void colo_do_failover(void);
 
-void colo_checkpoint_notify(void *opaque);
+/*
+ * colo_checkpoint_delay_set
+ *
+ * Handles change of x-checkpoint-delay migration parameter, called from
+ * migrate_params_apply() to notify COLO module about the change.
+ */
+void colo_checkpoint_delay_set(void);
+
 void colo_shutdown(void);
 #endif
diff --git a/migration/colo.c b/migration/colo.c
index 07bfa21fea..c9e0b909b9 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -65,6 +65,24 @@ static bool colo_runstate_is_stopped(void)
     return runstate_check(RUN_STATE_COLO) || !runstate_is_running();
 }
 
+static void colo_checkpoint_notify(void *opaque)
+{
+    MigrationState *s = opaque;
+    int64_t next_notify_time;
+
+    qemu_event_set(&s->colo_checkpoint_event);
+    s->colo_checkpoint_time = qemu_clock_get_ms(QEMU_CLOCK_HOST);
+    next_notify_time = s->colo_checkpoint_time + migrate_checkpoint_delay();
+    timer_mod(s->colo_delay_timer, next_notify_time);
+}
+
+void colo_checkpoint_delay_set(void)
+{
+    if (migration_in_colo_state()) {
+        colo_checkpoint_notify(migrate_get_current());
+    }
+}
+
 static void secondary_vm_do_failover(void)
 {
 /* COLO needs enable block-replication */
@@ -644,17 +662,6 @@ out:
     }
 }
 
-void colo_checkpoint_notify(void *opaque)
-{
-    MigrationState *s = opaque;
-    int64_t next_notify_time;
-
-    qemu_event_set(&s->colo_checkpoint_event);
-    s->colo_checkpoint_time = qemu_clock_get_ms(QEMU_CLOCK_HOST);
-    next_notify_time = s->colo_checkpoint_time + migrate_checkpoint_delay();
-    timer_mod(s->colo_delay_timer, next_notify_time);
-}
-
 void migrate_start_colo_process(MigrationState *s)
 {
     qemu_mutex_unlock_iothread();
diff --git a/migration/options.c b/migration/options.c
index 2e759cc306..9d92b15b76 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1253,9 +1253,7 @@ static void migrate_params_apply(MigrateSetParameters *params, Error **errp)
 
     if (params->has_x_checkpoint_delay) {
         s->parameters.x_checkpoint_delay = params->x_checkpoint_delay;
-        if (migration_in_colo_state()) {
-            colo_checkpoint_notify(s);
-        }
+        colo_checkpoint_delay_set();
     }
 
     if (params->has_block_incremental) {
-- 
2.40.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PULL 06/10] build: move COLO under CONFIG_REPLICATION
  2023-05-09 19:17 [PULL 00/10] Migration 20230509 patches Juan Quintela
                   ` (4 preceding siblings ...)
  2023-05-09 19:17 ` [PULL 05/10] colo: make colo_checkpoint_notify static and provide simpler API Juan Quintela
@ 2023-05-09 19:17 ` Juan Quintela
  2023-05-09 19:17 ` [PULL 07/10] migration: drop colo_incoming_thread from MigrationIncomingState Juan Quintela
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Juan Quintela @ 2023-05-09 19:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz, Kevin Wolf,
	Paolo Bonzini, Hailiang Zhang, Eric Blake, Juan Quintela,
	qemu-block, Peter Xu, Markus Armbruster,
	Vladimir Sementsov-Ogievskiy

From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

We don't allow to use x-colo capability when replication is not
configured. So, no reason to build COLO when replication is disabled,
it's unusable in this case.

Note also that the check in migrate_caps_check() is not the only
restriction: some functions in migration/colo.c will just abort if
called with not defined CONFIG_REPLICATION, for example:

    migration_iteration_finish()
       case MIGRATION_STATUS_COLO:
           migrate_start_colo_process()
               colo_process_checkpoint()
                   abort()

It could probably make sense to have possibility to enable COLO without
REPLICATION, but this requires deeper audit of colo & replication code,
which may be done later if needed.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Acked-by: Dr. David Alan Gilbert <dave@treblig.org>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Message-Id: <20230428194928.1426370-4-vsementsov@yandex-team.ru>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hmp-commands.hx                |  2 ++
 migration/colo.c               | 28 ------------------------
 migration/meson.build          |  6 ++++--
 migration/migration-hmp-cmds.c |  2 ++
 migration/migration.c          |  6 ++++++
 qapi/migration.json            |  9 +++++---
 stubs/colo.c                   | 39 ++++++++++++++++++++++++++++++++++
 stubs/meson.build              |  1 +
 8 files changed, 60 insertions(+), 33 deletions(-)
 create mode 100644 stubs/colo.c

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9afbb54a51..2cbd0f77a0 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1052,6 +1052,7 @@ SRST
   migration (or once already in postcopy).
 ERST
 
+#ifdef CONFIG_REPLICATION
     {
         .name       = "x_colo_lost_heartbeat",
         .args_type  = "",
@@ -1060,6 +1061,7 @@ ERST
                       "a failover or takeover is needed.",
         .cmd = hmp_x_colo_lost_heartbeat,
     },
+#endif
 
 SRST
 ``x_colo_lost_heartbeat``
diff --git a/migration/colo.c b/migration/colo.c
index c9e0b909b9..6c7c313956 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -26,9 +26,7 @@
 #include "qemu/rcu.h"
 #include "migration/failover.h"
 #include "migration/ram.h"
-#ifdef CONFIG_REPLICATION
 #include "block/replication.h"
-#endif
 #include "net/colo-compare.h"
 #include "net/colo.h"
 #include "block/block.h"
@@ -86,7 +84,6 @@ void colo_checkpoint_delay_set(void)
 static void secondary_vm_do_failover(void)
 {
 /* COLO needs enable block-replication */
-#ifdef CONFIG_REPLICATION
     int old_state;
     MigrationIncomingState *mis = migration_incoming_get_current();
     Error *local_err = NULL;
@@ -151,14 +148,10 @@ static void secondary_vm_do_failover(void)
     if (mis->migration_incoming_co) {
         qemu_coroutine_enter(mis->migration_incoming_co);
     }
-#else
-    abort();
-#endif
 }
 
 static void primary_vm_do_failover(void)
 {
-#ifdef CONFIG_REPLICATION
     MigrationState *s = migrate_get_current();
     int old_state;
     Error *local_err = NULL;
@@ -199,9 +192,6 @@ static void primary_vm_do_failover(void)
 
     /* Notify COLO thread that failover work is finished */
     qemu_sem_post(&s->colo_exit_sem);
-#else
-    abort();
-#endif
 }
 
 COLOMode get_colo_mode(void)
@@ -235,7 +225,6 @@ void colo_do_failover(void)
     }
 }
 
-#ifdef CONFIG_REPLICATION
 void qmp_xen_set_replication(bool enable, bool primary,
                              bool has_failover, bool failover,
                              Error **errp)
@@ -289,7 +278,6 @@ void qmp_xen_colo_do_checkpoint(Error **errp)
     /* Notify all filters of all NIC to do checkpoint */
     colo_notify_filters_event(COLO_EVENT_CHECKPOINT, errp);
 }
-#endif
 
 COLOStatus *qmp_query_colo_status(Error **errp)
 {
@@ -453,15 +441,11 @@ static int colo_do_checkpoint_transaction(MigrationState *s,
     }
     qemu_mutex_lock_iothread();
 
-#ifdef CONFIG_REPLICATION
     replication_do_checkpoint_all(&local_err);
     if (local_err) {
         qemu_mutex_unlock_iothread();
         goto out;
     }
-#else
-        abort();
-#endif
 
     colo_send_message(s->to_dst_file, COLO_MESSAGE_VMSTATE_SEND, &local_err);
     if (local_err) {
@@ -579,15 +563,11 @@ static void colo_process_checkpoint(MigrationState *s)
     object_unref(OBJECT(bioc));
 
     qemu_mutex_lock_iothread();
-#ifdef CONFIG_REPLICATION
     replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
     if (local_err) {
         qemu_mutex_unlock_iothread();
         goto out;
     }
-#else
-        abort();
-#endif
 
     vm_start();
     qemu_mutex_unlock_iothread();
@@ -755,7 +735,6 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
         return;
     }
 
-#ifdef CONFIG_REPLICATION
     replication_get_error_all(&local_err);
     if (local_err) {
         error_propagate(errp, local_err);
@@ -772,9 +751,6 @@ static void colo_incoming_process_checkpoint(MigrationIncomingState *mis,
         qemu_mutex_unlock_iothread();
         return;
     }
-#else
-    abort();
-#endif
     /* Notify all filters of all NIC to do checkpoint */
     colo_notify_filters_event(COLO_EVENT_CHECKPOINT, &local_err);
 
@@ -881,15 +857,11 @@ void *colo_process_incoming_thread(void *opaque)
     object_unref(OBJECT(bioc));
 
     qemu_mutex_lock_iothread();
-#ifdef CONFIG_REPLICATION
     replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
     if (local_err) {
         qemu_mutex_unlock_iothread();
         goto out;
     }
-#else
-        abort();
-#endif
     vm_start();
     qemu_mutex_unlock_iothread();
     trace_colo_vm_state_change("stop", "run");
diff --git a/migration/meson.build b/migration/meson.build
index 75de868bb7..eb41b77db9 100644
--- a/migration/meson.build
+++ b/migration/meson.build
@@ -13,8 +13,6 @@ softmmu_ss.add(files(
   'block-dirty-bitmap.c',
   'channel.c',
   'channel-block.c',
-  'colo-failover.c',
-  'colo.c',
   'exec.c',
   'fd.c',
   'global_state.c',
@@ -33,6 +31,10 @@ softmmu_ss.add(files(
   'threadinfo.c',
 ), gnutls)
 
+if get_option('replication').allowed()
+  softmmu_ss.add(files('colo-failover.c', 'colo.c'))
+endif
+
 softmmu_ss.add(when: rdma, if_true: files('rdma.c'))
 if get_option('live_block_migration').allowed()
   softmmu_ss.add(files('block.c'))
diff --git a/migration/migration-hmp-cmds.c b/migration/migration-hmp-cmds.c
index 4e9f00e7dc..9885d7c9f7 100644
--- a/migration/migration-hmp-cmds.c
+++ b/migration/migration-hmp-cmds.c
@@ -643,6 +643,7 @@ void hmp_migrate_start_postcopy(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, err);
 }
 
+#ifdef CONFIG_REPLICATION
 void hmp_x_colo_lost_heartbeat(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
@@ -650,6 +651,7 @@ void hmp_x_colo_lost_heartbeat(Monitor *mon, const QDict *qdict)
     qmp_x_colo_lost_heartbeat(&err);
     hmp_handle_error(mon, err);
 }
+#endif
 
 typedef struct HMPMigrationStatus {
     QEMUTimer *timer;
diff --git a/migration/migration.c b/migration/migration.c
index 0ee07802a5..01ee92e699 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -392,6 +392,12 @@ void migration_incoming_disable_colo(void)
 
 int migration_incoming_enable_colo(void)
 {
+#ifndef CONFIG_REPLICATION
+    error_report("ENABLE_COLO command come in migration stream, but COLO "
+                 "module is not built in");
+    return -ENOTSUP;
+#endif
+
     if (ram_block_discard_disable(true)) {
         error_report("COLO: cannot disable RAM discard");
         return -EBUSY;
diff --git a/qapi/migration.json b/qapi/migration.json
index 82000adce4..30e2542f1b 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1383,7 +1383,8 @@
 #
 ##
 { 'command': 'x-colo-lost-heartbeat',
-  'features': [ 'unstable' ] }
+  'features': [ 'unstable' ],
+  'if': 'CONFIG_REPLICATION' }
 
 ##
 # @migrate_cancel:
@@ -1659,7 +1660,8 @@
 ##
 { 'struct': 'COLOStatus',
   'data': { 'mode': 'COLOMode', 'last-mode': 'COLOMode',
-            'reason': 'COLOExitReason' } }
+            'reason': 'COLOExitReason' },
+  'if': 'CONFIG_REPLICATION' }
 
 ##
 # @query-colo-status:
@@ -1676,7 +1678,8 @@
 # Since: 3.1
 ##
 { 'command': 'query-colo-status',
-  'returns': 'COLOStatus' }
+  'returns': 'COLOStatus',
+  'if': 'CONFIG_REPLICATION' }
 
 ##
 # @migrate-recover:
diff --git a/stubs/colo.c b/stubs/colo.c
new file mode 100644
index 0000000000..cf9816d368
--- /dev/null
+++ b/stubs/colo.c
@@ -0,0 +1,39 @@
+#include "qemu/osdep.h"
+#include "qemu/notify.h"
+#include "net/colo-compare.h"
+#include "migration/colo.h"
+#include "migration/migration.h"
+#include "qemu/error-report.h"
+#include "qapi/qapi-commands-migration.h"
+
+void colo_shutdown(void)
+{
+}
+
+void *colo_process_incoming_thread(void *opaque)
+{
+    error_report("Impossible happend: trying to start COLO thread when COLO "
+                 "module is not built in");
+    abort();
+}
+
+void colo_checkpoint_delay_set(void)
+{
+}
+
+void migrate_start_colo_process(MigrationState *s)
+{
+    error_report("Impossible happend: trying to start COLO when COLO "
+                 "module is not built in");
+    abort();
+}
+
+bool migration_in_colo_state(void)
+{
+    return false;
+}
+
+bool migration_incoming_in_colo_state(void)
+{
+    return false;
+}
diff --git a/stubs/meson.build b/stubs/meson.build
index b2b5956d97..8412cad15f 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -45,6 +45,7 @@ stub_ss.add(files('target-get-monitor-def.c'))
 stub_ss.add(files('target-monitor-defs.c'))
 stub_ss.add(files('trace-control.c'))
 stub_ss.add(files('uuid.c'))
+stub_ss.add(files('colo.c'))
 stub_ss.add(files('vmstate.c'))
 stub_ss.add(files('vm-stop.c'))
 stub_ss.add(files('win32-kbd-hook.c'))
-- 
2.40.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PULL 07/10] migration: drop colo_incoming_thread from MigrationIncomingState
  2023-05-09 19:17 [PULL 00/10] Migration 20230509 patches Juan Quintela
                   ` (5 preceding siblings ...)
  2023-05-09 19:17 ` [PULL 06/10] build: move COLO under CONFIG_REPLICATION Juan Quintela
@ 2023-05-09 19:17 ` Juan Quintela
  2023-05-09 19:17 ` [PULL 08/10] migration: process_incoming_migration_co: simplify code flow around ret Juan Quintela
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Juan Quintela @ 2023-05-09 19:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz, Kevin Wolf,
	Paolo Bonzini, Hailiang Zhang, Eric Blake, Juan Quintela,
	qemu-block, Peter Xu, Markus Armbruster,
	Vladimir Sementsov-Ogievskiy, Zhang Chen

From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

have_colo_incoming_thread variable is unused. colo_incoming_thread can
be local.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Message-Id: <20230428194928.1426370-6-vsementsov@yandex-team.ru>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration.c | 7 ++++---
 migration/migration.h | 2 --
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 01ee92e699..3ab3b1c3e6 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -544,6 +544,8 @@ process_incoming_migration_co(void *opaque)
 
     /* we get COLO info, and know if we are in COLO mode */
     if (!ret && migration_incoming_colo_enabled()) {
+        QemuThread colo_incoming_thread;
+
         /* Make sure all file formats throw away their mutable metadata */
         bdrv_activate_all(&local_err);
         if (local_err) {
@@ -551,14 +553,13 @@ process_incoming_migration_co(void *opaque)
             goto fail;
         }
 
-        qemu_thread_create(&mis->colo_incoming_thread, "COLO incoming",
+        qemu_thread_create(&colo_incoming_thread, "COLO incoming",
              colo_process_incoming_thread, mis, QEMU_THREAD_JOINABLE);
-        mis->have_colo_incoming_thread = true;
         qemu_coroutine_yield();
 
         qemu_mutex_unlock_iothread();
         /* Wait checkpoint incoming thread exit before free resource */
-        qemu_thread_join(&mis->colo_incoming_thread);
+        qemu_thread_join(&colo_incoming_thread);
         qemu_mutex_lock_iothread();
         /* We hold the global iothread lock, so it is safe here */
         colo_release_ram_cache();
diff --git a/migration/migration.h b/migration/migration.h
index 3a918514e7..7721c7658b 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -162,8 +162,6 @@ struct MigrationIncomingState {
 
     int state;
 
-    bool have_colo_incoming_thread;
-    QemuThread colo_incoming_thread;
     /* The coroutine we should enter (back) after failover */
     Coroutine *migration_incoming_co;
     QemuSemaphore colo_incoming_sem;
-- 
2.40.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PULL 08/10] migration: process_incoming_migration_co: simplify code flow around ret
  2023-05-09 19:17 [PULL 00/10] Migration 20230509 patches Juan Quintela
                   ` (6 preceding siblings ...)
  2023-05-09 19:17 ` [PULL 07/10] migration: drop colo_incoming_thread from MigrationIncomingState Juan Quintela
@ 2023-05-09 19:17 ` Juan Quintela
  2023-05-09 19:17 ` [PULL 09/10] migration: disallow change capabilities in COLO state Juan Quintela
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 17+ messages in thread
From: Juan Quintela @ 2023-05-09 19:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz, Kevin Wolf,
	Paolo Bonzini, Hailiang Zhang, Eric Blake, Juan Quintela,
	qemu-block, Peter Xu, Markus Armbruster,
	Vladimir Sementsov-Ogievskiy, Zhang Chen

From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Message-Id: <20230428194928.1426370-7-vsementsov@yandex-team.ru>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 3ab3b1c3e6..230f91f5a7 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -542,8 +542,13 @@ process_incoming_migration_co(void *opaque)
         /* Else if something went wrong then just fall out of the normal exit */
     }
 
+    if (ret < 0) {
+        error_report("load of migration failed: %s", strerror(-ret));
+        goto fail;
+    }
+
     /* we get COLO info, and know if we are in COLO mode */
-    if (!ret && migration_incoming_colo_enabled()) {
+    if (migration_incoming_colo_enabled()) {
         QemuThread colo_incoming_thread;
 
         /* Make sure all file formats throw away their mutable metadata */
@@ -565,10 +570,6 @@ process_incoming_migration_co(void *opaque)
         colo_release_ram_cache();
     }
 
-    if (ret < 0) {
-        error_report("load of migration failed: %s", strerror(-ret));
-        goto fail;
-    }
     mis->bh = qemu_bh_new(process_incoming_migration_bh, mis);
     qemu_bh_schedule(mis->bh);
     mis->migration_incoming_co = NULL;
-- 
2.40.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PULL 09/10] migration: disallow change capabilities in COLO state
  2023-05-09 19:17 [PULL 00/10] Migration 20230509 patches Juan Quintela
                   ` (7 preceding siblings ...)
  2023-05-09 19:17 ` [PULL 08/10] migration: process_incoming_migration_co: simplify code flow around ret Juan Quintela
@ 2023-05-09 19:17 ` Juan Quintela
  2023-05-09 19:17 ` [PULL 10/10] migration: block incoming colo when capability is disabled Juan Quintela
  2023-05-10 10:17 ` [PULL 00/10] Migration 20230509 patches Richard Henderson
  10 siblings, 0 replies; 17+ messages in thread
From: Juan Quintela @ 2023-05-09 19:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz, Kevin Wolf,
	Paolo Bonzini, Hailiang Zhang, Eric Blake, Juan Quintela,
	qemu-block, Peter Xu, Markus Armbruster,
	Vladimir Sementsov-Ogievskiy

From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

COLO is not listed as running state in migrate_is_running(), so, it's
theoretically possible to disable colo capability in COLO state and the
unexpected error in migration_iteration_finish() is reachable.

Let's disallow that in qmp_migrate_set_capabilities. Than the error
becomes absolutely unreachable: we can get into COLO state only with
enabled capability and can't disable it while we are in COLO state. So
substitute the error by simple assertion.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Message-Id: <20230428194928.1426370-10-vsementsov@yandex-team.ru>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration.c | 5 +----
 migration/options.c   | 2 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 230f91f5a7..4959f7ee44 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2781,10 +2781,7 @@ static void migration_iteration_finish(MigrationState *s)
         runstate_set(RUN_STATE_POSTMIGRATE);
         break;
     case MIGRATION_STATUS_COLO:
-        if (!migrate_colo()) {
-            error_report("%s: critical error: calling COLO code without "
-                         "COLO enabled", __func__);
-        }
+        assert(migrate_colo());
         migrate_start_colo_process(s);
         s->vm_was_running = true;
         /* Fallthrough */
diff --git a/migration/options.c b/migration/options.c
index 9d92b15b76..7ed88b7b32 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -598,7 +598,7 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
     MigrationCapabilityStatusList *cap;
     bool new_caps[MIGRATION_CAPABILITY__MAX];
 
-    if (migration_is_running(s->state)) {
+    if (migration_is_running(s->state) || migration_in_colo_state()) {
         error_setg(errp, QERR_MIGRATION_ACTIVE);
         return;
     }
-- 
2.40.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PULL 10/10] migration: block incoming colo when capability is disabled
  2023-05-09 19:17 [PULL 00/10] Migration 20230509 patches Juan Quintela
                   ` (8 preceding siblings ...)
  2023-05-09 19:17 ` [PULL 09/10] migration: disallow change capabilities in COLO state Juan Quintela
@ 2023-05-09 19:17 ` Juan Quintela
  2023-05-10 10:17 ` [PULL 00/10] Migration 20230509 patches Richard Henderson
  10 siblings, 0 replies; 17+ messages in thread
From: Juan Quintela @ 2023-05-09 19:17 UTC (permalink / raw)
  To: qemu-devel
  Cc: Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz, Kevin Wolf,
	Paolo Bonzini, Hailiang Zhang, Eric Blake, Juan Quintela,
	qemu-block, Peter Xu, Markus Armbruster,
	Vladimir Sementsov-Ogievskiy, Lukas Straub, Zhang Chen

From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

We generally require same set of capabilities on source and target.
Let's require x-colo capability to use COLO on target.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
Reviewed-by: Zhang Chen <chen.zhang@intel.com>
Message-Id: <20230428194928.1426370-11-vsementsov@yandex-team.ru>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 docs/COLO-FT.txt      | 1 +
 migration/migration.c | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt
index 8ec653f81c..2e760a4aee 100644
--- a/docs/COLO-FT.txt
+++ b/docs/COLO-FT.txt
@@ -210,6 +210,7 @@ children.0=childs0 \
 
 3. On Secondary VM's QEMU monitor, issue command
 {"execute":"qmp_capabilities"}
+{"execute": "migrate-set-capabilities", "arguments": {"capabilities": [ {"capability": "x-colo", "state": true } ] } }
 {"execute": "nbd-server-start", "arguments": {"addr": {"type": "inet", "data": {"host": "0.0.0.0", "port": "9999"} } } }
 {"execute": "nbd-server-add", "arguments": {"device": "parent0", "writable": true } }
 
diff --git a/migration/migration.c b/migration/migration.c
index 4959f7ee44..7558c8edbd 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -398,6 +398,12 @@ int migration_incoming_enable_colo(void)
     return -ENOTSUP;
 #endif
 
+    if (!migrate_colo()) {
+        error_report("ENABLE_COLO command come in migration stream, but c-colo "
+                     "capability is not set");
+        return -EINVAL;
+    }
+
     if (ram_block_discard_disable(true)) {
         error_report("COLO: cannot disable RAM discard");
         return -EBUSY;
-- 
2.40.0



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PULL 00/10] Migration 20230509 patches
  2023-05-09 19:17 [PULL 00/10] Migration 20230509 patches Juan Quintela
                   ` (9 preceding siblings ...)
  2023-05-09 19:17 ` [PULL 10/10] migration: block incoming colo when capability is disabled Juan Quintela
@ 2023-05-10 10:17 ` Richard Henderson
  2023-05-10 12:20   ` Juan Quintela
  10 siblings, 1 reply; 17+ messages in thread
From: Richard Henderson @ 2023-05-10 10:17 UTC (permalink / raw)
  To: Juan Quintela, qemu-devel
  Cc: Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz, Kevin Wolf,
	Paolo Bonzini, Hailiang Zhang, Eric Blake, qemu-block, Peter Xu,
	Markus Armbruster

On 5/9/23 20:17, Juan Quintela wrote:
> The following changes since commit 271477b59e723250f17a7e20f139262057921b6a:
> 
>    Merge tag 'compression-code-pull-request' of https://gitlab.com/juan.quintela/qemu into staging (2023-05-08 20:38:05 +0100)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/juan.quintela/qemu.git tags/migration-20230509-pull-request
> 
> for you to fetch changes up to 5f43d297bc2b9530805ad8602c6e2ea284b08628:
> 
>    migration: block incoming colo when capability is disabled (2023-05-09 20:52:21 +0200)
> 
> ----------------------------------------------------------------
> Migration Pull request (20230509 vintage)
> 
> Hi
> In this PULL request:
> - 1st part of colo support for multifd (lukas)
> - 1st part of disabling colo option (vladimir)
> 
> Please, apply.

Build failures.

https://gitlab.com/qemu-project/qemu/-/jobs/4257605099#L2241

    85 | void colo_record_bitmap(RAMBlock *block, ram_addr_t *normal, uint normal_num);
       |                                                              ^~~~
       |                                                              u_int


r~



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PULL 00/10] Migration 20230509 patches
  2023-05-10 10:17 ` [PULL 00/10] Migration 20230509 patches Richard Henderson
@ 2023-05-10 12:20   ` Juan Quintela
  2023-05-10 12:35     ` Richard Henderson
  0 siblings, 1 reply; 17+ messages in thread
From: Juan Quintela @ 2023-05-10 12:20 UTC (permalink / raw)
  To: Richard Henderson
  Cc: qemu-devel, Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz,
	Kevin Wolf, Paolo Bonzini, Hailiang Zhang, Eric Blake,
	qemu-block, Peter Xu, Markus Armbruster

Richard Henderson <richard.henderson@linaro.org> wrote:
> On 5/9/23 20:17, Juan Quintela wrote:
>> The following changes since commit 271477b59e723250f17a7e20f139262057921b6a:
>>    Merge tag 'compression-code-pull-request' of
>> https://gitlab.com/juan.quintela/qemu into staging (2023-05-08
>> 20:38:05 +0100)
>> are available in the Git repository at:
>>    https://gitlab.com/juan.quintela/qemu.git
>> tags/migration-20230509-pull-request
>> for you to fetch changes up to
>> 5f43d297bc2b9530805ad8602c6e2ea284b08628:
>>    migration: block incoming colo when capability is disabled
>> (2023-05-09 20:52:21 +0200)
>> ----------------------------------------------------------------
>> Migration Pull request (20230509 vintage)
>> Hi
>> In this PULL request:
>> - 1st part of colo support for multifd (lukas)
>> - 1st part of disabling colo option (vladimir)
>> Please, apply.
>
> Build failures.
>
> https://gitlab.com/qemu-project/qemu/-/jobs/4257605099#L2241
>
>    85 | void colo_record_bitmap(RAMBlock *block, ram_addr_t *normal, uint normal_num);
>       |                                                              ^~~~
>       |                                                              u_int
>

Grrr

And the worst thing is that hate those types, tried to get then out
long, long ago for a similar problem.

Will resend, sorry for the noise.

Later, Juan.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PULL 00/10] Migration 20230509 patches
  2023-05-10 12:20   ` Juan Quintela
@ 2023-05-10 12:35     ` Richard Henderson
  2023-05-10 14:08       ` Juan Quintela
  0 siblings, 1 reply; 17+ messages in thread
From: Richard Henderson @ 2023-05-10 12:35 UTC (permalink / raw)
  To: quintela
  Cc: qemu-devel, Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz,
	Kevin Wolf, Paolo Bonzini, Hailiang Zhang, Eric Blake,
	qemu-block, Peter Xu, Markus Armbruster

On 5/10/23 13:20, Juan Quintela wrote:
> Richard Henderson <richard.henderson@linaro.org> wrote:
>> On 5/9/23 20:17, Juan Quintela wrote:
>>> The following changes since commit 271477b59e723250f17a7e20f139262057921b6a:
>>>     Merge tag 'compression-code-pull-request' of
>>> https://gitlab.com/juan.quintela/qemu into staging (2023-05-08
>>> 20:38:05 +0100)
>>> are available in the Git repository at:
>>>     https://gitlab.com/juan.quintela/qemu.git
>>> tags/migration-20230509-pull-request
>>> for you to fetch changes up to
>>> 5f43d297bc2b9530805ad8602c6e2ea284b08628:
>>>     migration: block incoming colo when capability is disabled
>>> (2023-05-09 20:52:21 +0200)
>>> ----------------------------------------------------------------
>>> Migration Pull request (20230509 vintage)
>>> Hi
>>> In this PULL request:
>>> - 1st part of colo support for multifd (lukas)
>>> - 1st part of disabling colo option (vladimir)
>>> Please, apply.
>>
>> Build failures.
>>
>> https://gitlab.com/qemu-project/qemu/-/jobs/4257605099#L2241
>>
>>     85 | void colo_record_bitmap(RAMBlock *block, ram_addr_t *normal, uint normal_num);
>>        |                                                              ^~~~
>>        |                                                              u_int
>>
> 
> Grrr
> 
> And the worst thing is that hate those types, tried to get then out
> long, long ago for a similar problem.

Where do these types come from, and can we poison them on the qemu side?


r~



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PULL 00/10] Migration 20230509 patches
  2023-05-10 12:35     ` Richard Henderson
@ 2023-05-10 14:08       ` Juan Quintela
  2023-05-10 14:46         ` Richard Henderson
  0 siblings, 1 reply; 17+ messages in thread
From: Juan Quintela @ 2023-05-10 14:08 UTC (permalink / raw)
  To: Richard Henderson
  Cc: qemu-devel, Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz,
	Kevin Wolf, Paolo Bonzini, Hailiang Zhang, Eric Blake,
	qemu-block, Peter Xu, Markus Armbruster

Richard Henderson <richard.henderson@linaro.org> wrote:
> On 5/10/23 13:20, Juan Quintela wrote:
>> Richard Henderson <richard.henderson@linaro.org> wrote:
>>> On 5/9/23 20:17, Juan Quintela wrote:
>>>> The following changes since commit 271477b59e723250f17a7e20f139262057921b6a:
>>>>     Merge tag 'compression-code-pull-request' of
>>>> https://gitlab.com/juan.quintela/qemu into staging (2023-05-08
>>>> 20:38:05 +0100)
>>>> are available in the Git repository at:
>>>>     https://gitlab.com/juan.quintela/qemu.git
>>>> tags/migration-20230509-pull-request
>>>> for you to fetch changes up to
>>>> 5f43d297bc2b9530805ad8602c6e2ea284b08628:
>>>>     migration: block incoming colo when capability is disabled
>>>> (2023-05-09 20:52:21 +0200)
>>>> ----------------------------------------------------------------
>>>> Migration Pull request (20230509 vintage)
>>>> Hi
>>>> In this PULL request:
>>>> - 1st part of colo support for multifd (lukas)
>>>> - 1st part of disabling colo option (vladimir)
>>>> Please, apply.
>>>
>>> Build failures.
>>>
>>> https://gitlab.com/qemu-project/qemu/-/jobs/4257605099#L2241
>>>
>>>     85 | void colo_record_bitmap(RAMBlock *block, ram_addr_t *normal, uint normal_num);
>>>        |                                                              ^~~~
>>>        |                                                              u_int
>>>
>> Grrr
>> And the worst thing is that hate those types, tried to get then out
>> long, long ago for a similar problem.

While I was not looking, some of them were corrected:

commit d7df0b41dc38327388c3f19fdf4246793d4a1e4b
Author: Marc-André Lureau <marcandre.lureau@redhat.com>
Date:   Thu Jan 17 15:43:53 2019 +0400

    slirp: prefer c99 types over BSD kind

    Replace:
    - u_char -> uint8_t
    - u_short -> uint16_t
    - u_long -> uint32_t
    - u_int -> unsigned
    - caddr_t -> char *

> Where do these types come from, and can we poison them on the qemu side?

grep " uint;" on my system includes.  I know that there are more
creative ways to define it.

/usr/include/ffi-x86_64.h\0278:  ffi_arg   uint;
/usr/include/sys/types.h\0150:typedef unsigned int uint;

#ifdef __USE_MISC
/* Old compatibility names for C types.  */
typedef unsigned long int ulong;
typedef unsigned short int ushort;
typedef unsigned int uint;
#endif

I guess we get those someway.

/usr/include/nspr4/obsolete/protypes.h\052:typedef PRUintn uint;
/usr/include/mysql/server/my_global.h\0465:typedef unsigned int uint;
/usr/include/boost/iostreams/filter/zlib.hpp\047:typedef uint32_t uint;
/usr/include/qt5/QtCore/qglobal.h\0275:typedef unsigned int uint;

in qt it is defined for everything.

If I disable the ones in sys/types.h

I got:

cc -m64 -mcx16 -Ilibqemu-loongarch64-linux-user.fa.p -I. -I../../../../mnt/code/qemu/full -Itarget/loongarch -I../../../../mnt/code/qemu/full/target/loongarch -I../../../../mnt/code/qemu/full/common-user/host/x86_64 -I../../../../mnt/code/qemu/full/linux-user/include/host/x86_64 -I../../../../mnt/code/qemu/full/linux-user/include -Ilinux-user -I../../../../mnt/code/qemu/full/linux-user -I../../../../mnt/code/qemu/full/linux-user/loongarch64 -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/capstone -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem /mnt/code/qemu/full/linux-headers -isystem linux-headers -iquote . -iquote /mnt/code/qemu/full -iquote /mnt/code/qemu/full/include -iquote /mnt/code/qemu/full/tcg/i386 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -isystem../../../../mnt/code/qemu/full/linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="loongarch64-linux-user-config-target.h"' '-DCONFIG_DEVICES="loongarch64-linux-user-config-devices.h"' -MD -MQ libqemu-loongarch64-linux-user.fa.p/linux-user_syscall.c.o -MF libqemu-loongarch64-linux-user.fa.p/linux-user_syscall.c.o.d -o libqemu-loongarch64-linux-user.fa.p/linux-user_syscall.c.o -c ../../../../mnt/code/qemu/full/linux-user/syscall.c
../../../../mnt/code/qemu/full/linux-user/syscall.c:317:32: error: unknown type name ‘uint’; did you mean ‘guint’?
  317 | _syscall3(int, sys_getdents64, uint, fd, struct linux_dirent64 *, dirp, uint, count);
      |                                ^~~~

There are some other friends that fail in linux-user/syscall.c

I will post an RFC with my findings.

Later, Juan.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PULL 00/10] Migration 20230509 patches
  2023-05-10 14:08       ` Juan Quintela
@ 2023-05-10 14:46         ` Richard Henderson
  0 siblings, 0 replies; 17+ messages in thread
From: Richard Henderson @ 2023-05-10 14:46 UTC (permalink / raw)
  To: quintela
  Cc: qemu-devel, Dr. David Alan Gilbert, Leonardo Bras, Hanna Reitz,
	Kevin Wolf, Paolo Bonzini, Hailiang Zhang, Eric Blake,
	qemu-block, Peter Xu, Markus Armbruster

On 5/10/23 15:08, Juan Quintela wrote:
> grep " uint;" on my system includes.  I know that there are more
> creative ways to define it.
> 
> /usr/include/ffi-x86_64.h\0278:  ffi_arg   uint;

Thankfully only a structure member.  :-)

> /usr/include/sys/types.h\0150:typedef unsigned int uint;

Oof.

> /usr/include/nspr4/obsolete/protypes.h\052:typedef PRUintn uint;
> /usr/include/mysql/server/my_global.h\0465:typedef unsigned int uint;
> /usr/include/boost/iostreams/filter/zlib.hpp\047:typedef uint32_t uint;
> /usr/include/qt5/QtCore/qglobal.h\0275:typedef unsigned int uint;
> 
> in qt it is defined for everything.

Ok.

> ../../../../mnt/code/qemu/full/linux-user/syscall.c:317:32: error: unknown type name ‘uint’; did you mean ‘guint’?
>    317 | _syscall3(int, sys_getdents64, uint, fd, struct linux_dirent64 *, dirp, uint, count);
>        |                                ^~~~

Fixable.

> I will post an RFC with my findings.

Thanks.


r~



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PULL 09/10] migration: disallow change capabilities in COLO state
  2023-05-10 18:09 Juan Quintela
@ 2023-05-10 18:09 ` Juan Quintela
  0 siblings, 0 replies; 17+ messages in thread
From: Juan Quintela @ 2023-05-10 18:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Peter Xu, Eric Blake, Hanna Reitz,
	Dr. David Alan Gilbert, Markus Armbruster, Leonardo Bras,
	qemu-block, Hailiang Zhang, Paolo Bonzini, Juan Quintela,
	Vladimir Sementsov-Ogievskiy

From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>

COLO is not listed as running state in migrate_is_running(), so, it's
theoretically possible to disable colo capability in COLO state and the
unexpected error in migration_iteration_finish() is reachable.

Let's disallow that in qmp_migrate_set_capabilities. Than the error
becomes absolutely unreachable: we can get into COLO state only with
enabled capability and can't disable it while we are in COLO state. So
substitute the error by simple assertion.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Message-Id: <20230428194928.1426370-10-vsementsov@yandex-team.ru>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/migration.c | 5 +----
 migration/options.c   | 2 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 140b2a4de6..bb254e4f07 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2785,10 +2785,7 @@ static void migration_iteration_finish(MigrationState *s)
         runstate_set(RUN_STATE_POSTMIGRATE);
         break;
     case MIGRATION_STATUS_COLO:
-        if (!migrate_colo()) {
-            error_report("%s: critical error: calling COLO code without "
-                         "COLO enabled", __func__);
-        }
+        assert(migrate_colo());
         migrate_start_colo_process(s);
         s->vm_was_running = true;
         /* Fallthrough */
diff --git a/migration/options.c b/migration/options.c
index 9d92b15b76..7ed88b7b32 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -598,7 +598,7 @@ void qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
     MigrationCapabilityStatusList *cap;
     bool new_caps[MIGRATION_CAPABILITY__MAX];
 
-    if (migration_is_running(s->state)) {
+    if (migration_is_running(s->state) || migration_in_colo_state()) {
         error_setg(errp, QERR_MIGRATION_ACTIVE);
         return;
     }
-- 
2.40.1



^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2023-05-10 18:11 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-09 19:17 [PULL 00/10] Migration 20230509 patches Juan Quintela
2023-05-09 19:17 ` [PULL 01/10] ram: Add public helper to set colo bitmap Juan Quintela
2023-05-09 19:17 ` [PULL 02/10] ram: Let colo_flush_ram_cache take the bitmap_mutex Juan Quintela
2023-05-09 19:17 ` [PULL 03/10] multifd: Add the ramblock to MultiFDRecvParams Juan Quintela
2023-05-09 19:17 ` [PULL 04/10] block/meson.build: prefer positive condition for replication Juan Quintela
2023-05-09 19:17 ` [PULL 05/10] colo: make colo_checkpoint_notify static and provide simpler API Juan Quintela
2023-05-09 19:17 ` [PULL 06/10] build: move COLO under CONFIG_REPLICATION Juan Quintela
2023-05-09 19:17 ` [PULL 07/10] migration: drop colo_incoming_thread from MigrationIncomingState Juan Quintela
2023-05-09 19:17 ` [PULL 08/10] migration: process_incoming_migration_co: simplify code flow around ret Juan Quintela
2023-05-09 19:17 ` [PULL 09/10] migration: disallow change capabilities in COLO state Juan Quintela
2023-05-09 19:17 ` [PULL 10/10] migration: block incoming colo when capability is disabled Juan Quintela
2023-05-10 10:17 ` [PULL 00/10] Migration 20230509 patches Richard Henderson
2023-05-10 12:20   ` Juan Quintela
2023-05-10 12:35     ` Richard Henderson
2023-05-10 14:08       ` Juan Quintela
2023-05-10 14:46         ` Richard Henderson
2023-05-10 18:09 Juan Quintela
2023-05-10 18:09 ` [PULL 09/10] migration: disallow change capabilities in COLO state Juan Quintela

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.