* [PATCH v3 0/4] COLO: improve build options
@ 2023-04-27 20:29 Vladimir Sementsov-Ogievskiy
2023-04-27 20:29 ` [PATCH v3 1/4] block/meson.build: prefer positive condition for replication Vladimir Sementsov-Ogievskiy
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2023-04-27 20:29 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, michael.roth, armbru, eblake, jasowang, quintela,
zhanghailiang, philmd, thuth, berrange, marcandre.lureau,
pbonzini, dave, hreitz, kwolf, chen.zhang, lizhijian,
lukasstraub2, vsementsov
v3:
01: add r-bs
02: improve commit message
03: - improve commit message
- drop ifdefs from migration/colo.c which are not needed anymore
- don't move migrate_colo_enabled() (now just migrate_colo()), instead modify it inplace
- keep colo-compare.c for now (will be handled in updated 04 patch)
- so, no colo_compare_cleanup() stub needed for now, neither migrate_colo_enabled() stub
- keep Acked-by.
04: - improve commit message
- rename to --disable-colo-proxy to match subsystem name in MAINTAINERS
- don't introduce CONFIG_COLO_PROXY, it actually is not needed
- colo-compare.c is handled now and included if any of 'replication' and 'colo-proxy' are enabled
- so, we add colo_compare_cleanup() stub in a separate stub file
Hi all!
COLO substem seems to be useless when CONFIG_REPLICATION is unset, as we
simply don't allow to set x-colo capability in this case. So, let's not
compile in unreachable code and interface we cannot use when
CONFIG_REPLICATION is unset.
Also, provide personal configure option for COLO Proxy subsystem.
Vladimir Sementsov-Ogievskiy (4):
block/meson.build: prefer positive condition for replication
scripts/qapi: allow optional experimental enum values
build: move COLO under CONFIG_REPLICATION
configure: add --disable-colo-proxy option
block/meson.build | 2 +-
hmp-commands.hx | 2 ++
meson_options.txt | 2 ++
migration/colo.c | 28 -------------------------
migration/meson.build | 6 ++++--
migration/migration-hmp-cmds.c | 2 ++
migration/options.c | 17 ++++++++--------
net/meson.build | 14 +++++++++----
qapi/migration.json | 12 +++++++----
scripts/meson-buildoptions.sh | 3 +++
scripts/qapi/types.py | 2 ++
stubs/colo-compare.c | 7 +++++++
stubs/colo.c | 37 ++++++++++++++++++++++++++++++++++
stubs/meson.build | 2 ++
14 files changed, 88 insertions(+), 48 deletions(-)
create mode 100644 stubs/colo-compare.c
create mode 100644 stubs/colo.c
--
2.34.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 1/4] block/meson.build: prefer positive condition for replication
2023-04-27 20:29 [PATCH v3 0/4] COLO: improve build options Vladimir Sementsov-Ogievskiy
@ 2023-04-27 20:29 ` Vladimir Sementsov-Ogievskiy
2023-04-27 20:47 ` Lukas Straub
2023-04-27 20:29 ` [PATCH v3 2/4] scripts/qapi: allow optional experimental enum values Vladimir Sementsov-Ogievskiy
` (2 subsequent siblings)
3 siblings, 1 reply; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2023-04-27 20:29 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, michael.roth, armbru, eblake, jasowang, quintela,
zhanghailiang, philmd, thuth, berrange, marcandre.lureau,
pbonzini, dave, hreitz, kwolf, chen.zhang, lizhijian,
lukasstraub2, vsementsov
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>
---
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.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 2/4] scripts/qapi: allow optional experimental enum values
2023-04-27 20:29 [PATCH v3 0/4] COLO: improve build options Vladimir Sementsov-Ogievskiy
2023-04-27 20:29 ` [PATCH v3 1/4] block/meson.build: prefer positive condition for replication Vladimir Sementsov-Ogievskiy
@ 2023-04-27 20:29 ` Vladimir Sementsov-Ogievskiy
2023-04-27 20:29 ` [PATCH v3 3/4] build: move COLO under CONFIG_REPLICATION Vladimir Sementsov-Ogievskiy
2023-04-27 20:29 ` [PATCH v3 4/4] configure: add --disable-colo-proxy option Vladimir Sementsov-Ogievskiy
3 siblings, 0 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2023-04-27 20:29 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, michael.roth, armbru, eblake, jasowang, quintela,
zhanghailiang, philmd, thuth, berrange, marcandre.lureau,
pbonzini, dave, hreitz, kwolf, chen.zhang, lizhijian,
lukasstraub2, vsementsov
We have 'if' feature for some things in QAPI, including enum values.
But currently it doesn't work for experimental enum values, as in
generated QEnumLookup structure, the description for additional
features (for example - "unstable") is not surrounded by corresponding
"#ifdef"s.
So let's fix it.
We are going to use it in the next commit, to make unstable x-colo
migration capability optional:
{ 'name': 'x-colo', 'features': [ 'unstable' ], 'if': 'CONFIG_COLO' }
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
scripts/qapi/types.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index c39d054d2c..18f8734047 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -61,10 +61,12 @@ def gen_enum_lookup(name: str,
special_features = gen_special_features(memb.features)
if special_features != '0':
+ feats += memb.ifcond.gen_if()
feats += mcgen('''
[%(index)s] = %(special_features)s,
''',
index=index, special_features=special_features)
+ feats += memb.ifcond.gen_endif()
if feats:
ret += mcgen('''
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 3/4] build: move COLO under CONFIG_REPLICATION
2023-04-27 20:29 [PATCH v3 0/4] COLO: improve build options Vladimir Sementsov-Ogievskiy
2023-04-27 20:29 ` [PATCH v3 1/4] block/meson.build: prefer positive condition for replication Vladimir Sementsov-Ogievskiy
2023-04-27 20:29 ` [PATCH v3 2/4] scripts/qapi: allow optional experimental enum values Vladimir Sementsov-Ogievskiy
@ 2023-04-27 20:29 ` Vladimir Sementsov-Ogievskiy
2023-04-27 21:13 ` Lukas Straub
2023-04-28 7:30 ` Juan Quintela
2023-04-27 20:29 ` [PATCH v3 4/4] configure: add --disable-colo-proxy option Vladimir Sementsov-Ogievskiy
3 siblings, 2 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2023-04-27 20:29 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, michael.roth, armbru, eblake, jasowang, quintela,
zhanghailiang, philmd, thuth, berrange, marcandre.lureau,
pbonzini, dave, hreitz, kwolf, chen.zhang, lizhijian,
lukasstraub2, vsementsov
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>
---
hmp-commands.hx | 2 ++
migration/colo.c | 28 -------------------------
migration/meson.build | 6 ++++--
migration/migration-hmp-cmds.c | 2 ++
migration/options.c | 17 ++++++++--------
qapi/migration.json | 12 +++++++----
stubs/colo.c | 37 ++++++++++++++++++++++++++++++++++
stubs/meson.build | 1 +
8 files changed, 62 insertions(+), 43 deletions(-)
create mode 100644 stubs/colo.c
diff --git a/hmp-commands.hx b/hmp-commands.hx
index bb85ee1d26..fbd0932232 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1035,6 +1035,7 @@ SRST
migration (or once already in postcopy).
ERST
+#ifdef CONFIG_REPLICATION
{
.name = "x_colo_lost_heartbeat",
.args_type = "",
@@ -1043,6 +1044,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 07bfa21fea..e4af47eeeb 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"
@@ -68,7 +66,6 @@ static bool colo_runstate_is_stopped(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;
@@ -133,14 +130,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;
@@ -181,9 +174,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)
@@ -217,7 +207,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)
@@ -271,7 +260,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)
{
@@ -435,15 +423,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) {
@@ -561,15 +545,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();
@@ -748,7 +728,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);
@@ -765,9 +744,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);
@@ -874,15 +850,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 480ff6854a..9c6a8e65d3 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',
@@ -30,6 +28,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/options.c b/migration/options.c
index 912cbadddb..eef2bd0f16 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -171,7 +171,9 @@ Property migration_properties[] = {
DEFINE_PROP_MIG_CAP("x-postcopy-ram", MIGRATION_CAPABILITY_POSTCOPY_RAM),
DEFINE_PROP_MIG_CAP("x-postcopy-preempt",
MIGRATION_CAPABILITY_POSTCOPY_PREEMPT),
+#ifdef CONFIG_REPLICATION
DEFINE_PROP_MIG_CAP("x-colo", MIGRATION_CAPABILITY_X_COLO),
+#endif
DEFINE_PROP_MIG_CAP("x-release-ram", MIGRATION_CAPABILITY_RELEASE_RAM),
DEFINE_PROP_MIG_CAP("x-block", MIGRATION_CAPABILITY_BLOCK),
DEFINE_PROP_MIG_CAP("x-return-path", MIGRATION_CAPABILITY_RETURN_PATH),
@@ -209,9 +211,13 @@ bool migrate_block(void)
bool migrate_colo(void)
{
+#ifdef CONFIG_REPLICATION
MigrationState *s = migrate_get_current();
return s->capabilities[MIGRATION_CAPABILITY_X_COLO];
+#else
+ return false;
+#endif
}
bool migrate_compress(void)
@@ -401,7 +407,9 @@ INITIALIZE_MIGRATE_CAPS_SET(check_caps_background_snapshot,
MIGRATION_CAPABILITY_RDMA_PIN_ALL,
MIGRATION_CAPABILITY_COMPRESS,
MIGRATION_CAPABILITY_XBZRLE,
+#ifdef CONFIG_REPLICATION
MIGRATION_CAPABILITY_X_COLO,
+#endif
MIGRATION_CAPABILITY_VALIDATE_UUID,
MIGRATION_CAPABILITY_ZERO_COPY_SEND);
@@ -428,15 +436,6 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
}
#endif
-#ifndef CONFIG_REPLICATION
- if (new_caps[MIGRATION_CAPABILITY_X_COLO]) {
- error_setg(errp, "QEMU compiled without replication module"
- " can't enable COLO");
- error_append_hint(errp, "Please enable replication before COLO.\n");
- return false;
- }
-#endif
-
if (new_caps[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
/* This check is reasonably expensive, so only when it's being
* set the first time, also it's only the destination that needs
diff --git a/qapi/migration.json b/qapi/migration.json
index 2c35b7b9cf..5cb095f7b3 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -486,7 +486,8 @@
{ 'enum': 'MigrationCapability',
'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
'compress', 'events', 'postcopy-ram',
- { 'name': 'x-colo', 'features': [ 'unstable' ] },
+ { 'name': 'x-colo', 'features': [ 'unstable' ],
+ 'if': 'CONFIG_REPLICATION' },
'release-ram',
'block', 'return-path', 'pause-before-switchover', 'multifd',
'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
@@ -1381,7 +1382,8 @@
#
##
{ 'command': 'x-colo-lost-heartbeat',
- 'features': [ 'unstable' ] }
+ 'features': [ 'unstable' ],
+ 'if': 'CONFIG_REPLICATION' }
##
# @migrate_cancel:
@@ -1657,7 +1659,8 @@
##
{ 'struct': 'COLOStatus',
'data': { 'mode': 'COLOMode', 'last-mode': 'COLOMode',
- 'reason': 'COLOExitReason' } }
+ 'reason': 'COLOExitReason' },
+ 'if': 'CONFIG_REPLICATION' }
##
# @query-colo-status:
@@ -1674,7 +1677,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..f306ab45d6
--- /dev/null
+++ b/stubs/colo.c
@@ -0,0 +1,37 @@
+#include "qemu/osdep.h"
+#include "qemu/notify.h"
+#include "net/colo-compare.h"
+#include "migration/colo.h"
+#include "migration/migration.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-migration.h"
+
+void colo_shutdown(void)
+{
+ abort();
+}
+
+void *colo_process_incoming_thread(void *opaque)
+{
+ abort();
+}
+
+void colo_checkpoint_notify(void *opaque)
+{
+ abort();
+}
+
+void migrate_start_colo_process(MigrationState *s)
+{
+ 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.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 4/4] configure: add --disable-colo-proxy option
2023-04-27 20:29 [PATCH v3 0/4] COLO: improve build options Vladimir Sementsov-Ogievskiy
` (2 preceding siblings ...)
2023-04-27 20:29 ` [PATCH v3 3/4] build: move COLO under CONFIG_REPLICATION Vladimir Sementsov-Ogievskiy
@ 2023-04-27 20:29 ` Vladimir Sementsov-Ogievskiy
2023-04-27 21:18 ` Lukas Straub
2023-04-28 7:33 ` Juan Quintela
3 siblings, 2 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2023-04-27 20:29 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-block, michael.roth, armbru, eblake, jasowang, quintela,
zhanghailiang, philmd, thuth, berrange, marcandre.lureau,
pbonzini, dave, hreitz, kwolf, chen.zhang, lizhijian,
lukasstraub2, vsementsov
Add option to not build filter-mirror, filter-rewriter and
colo-compare when they are not needed.
There could be more agile configuration, for example add separate
options for each filter, but that may be done in future on demand. The
aim of this patch is to make possible to disable the whole COLO Proxy
subsystem.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
meson_options.txt | 2 ++
net/meson.build | 14 ++++++++++----
scripts/meson-buildoptions.sh | 3 +++
stubs/colo-compare.c | 7 +++++++
stubs/meson.build | 1 +
5 files changed, 23 insertions(+), 4 deletions(-)
create mode 100644 stubs/colo-compare.c
diff --git a/meson_options.txt b/meson_options.txt
index 2471dd02da..b59e7ae342 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -289,6 +289,8 @@ option('live_block_migration', type: 'feature', value: 'auto',
description: 'block migration in the main migration stream')
option('replication', type: 'feature', value: 'auto',
description: 'replication support')
+option('colo_proxy', type: 'feature', value: 'auto',
+ description: 'colo-proxy support')
option('bochs', type: 'feature', value: 'auto',
description: 'bochs image format support')
option('cloop', type: 'feature', value: 'auto',
diff --git a/net/meson.build b/net/meson.build
index 87afca3e93..4cfc850c69 100644
--- a/net/meson.build
+++ b/net/meson.build
@@ -1,13 +1,9 @@
softmmu_ss.add(files(
'announce.c',
'checksum.c',
- 'colo-compare.c',
- 'colo.c',
'dump.c',
'eth.c',
'filter-buffer.c',
- 'filter-mirror.c',
- 'filter-rewriter.c',
'filter.c',
'hub.c',
'net-hmp-cmds.c',
@@ -19,6 +15,16 @@ softmmu_ss.add(files(
'util.c',
))
+if get_option('replication').allowed() or \
+ get_option('colo_proxy').allowed()
+ softmmu_ss.add(files('colo-compare.c'))
+ softmmu_ss.add(files('colo.c'))
+endif
+
+if get_option('colo_proxy').allowed()
+ softmmu_ss.add(files('filter-mirror.c', 'filter-rewriter.c'))
+endif
+
softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
if have_l2tpv3
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index d4369a3ad8..036047ce6f 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -83,6 +83,7 @@ meson_options_help() {
printf "%s\n" ' capstone Whether and how to find the capstone library'
printf "%s\n" ' cloop cloop image format support'
printf "%s\n" ' cocoa Cocoa user interface (macOS only)'
+ printf "%s\n" ' colo-proxy colo-proxy support'
printf "%s\n" ' coreaudio CoreAudio sound support'
printf "%s\n" ' crypto-afalg Linux AF_ALG crypto backend driver'
printf "%s\n" ' curl CURL block device driver'
@@ -236,6 +237,8 @@ _meson_option_parse() {
--disable-cloop) printf "%s" -Dcloop=disabled ;;
--enable-cocoa) printf "%s" -Dcocoa=enabled ;;
--disable-cocoa) printf "%s" -Dcocoa=disabled ;;
+ --enable-colo-proxy) printf "%s" -Dcolo_proxy=enabled ;;
+ --disable-colo-proxy) printf "%s" -Dcolo_proxy=disabled ;;
--enable-coreaudio) printf "%s" -Dcoreaudio=enabled ;;
--disable-coreaudio) printf "%s" -Dcoreaudio=disabled ;;
--enable-coroutine-pool) printf "%s" -Dcoroutine_pool=true ;;
diff --git a/stubs/colo-compare.c b/stubs/colo-compare.c
new file mode 100644
index 0000000000..ec726665be
--- /dev/null
+++ b/stubs/colo-compare.c
@@ -0,0 +1,7 @@
+#include "qemu/osdep.h"
+#include "qemu/notify.h"
+#include "net/colo-compare.h"
+
+void colo_compare_cleanup(void)
+{
+}
diff --git a/stubs/meson.build b/stubs/meson.build
index 8412cad15f..a56645e2f7 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -46,6 +46,7 @@ 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('colo-compare.c'))
stub_ss.add(files('vmstate.c'))
stub_ss.add(files('vm-stop.c'))
stub_ss.add(files('win32-kbd-hook.c'))
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3 1/4] block/meson.build: prefer positive condition for replication
2023-04-27 20:29 ` [PATCH v3 1/4] block/meson.build: prefer positive condition for replication Vladimir Sementsov-Ogievskiy
@ 2023-04-27 20:47 ` Lukas Straub
0 siblings, 0 replies; 16+ messages in thread
From: Lukas Straub @ 2023-04-27 20:47 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: qemu-devel, qemu-block, michael.roth, armbru, eblake, jasowang,
quintela, zhanghailiang, philmd, thuth, berrange,
marcandre.lureau, pbonzini, dave, hreitz, kwolf, chen.zhang,
lizhijian
[-- Attachment #1: Type: text/plain, Size: 1118 bytes --]
On Thu, 27 Apr 2023 23:29:43 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote:
> 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>
> ---
> 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'))
--
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/4] build: move COLO under CONFIG_REPLICATION
2023-04-27 20:29 ` [PATCH v3 3/4] build: move COLO under CONFIG_REPLICATION Vladimir Sementsov-Ogievskiy
@ 2023-04-27 21:13 ` Lukas Straub
2023-04-28 7:30 ` Juan Quintela
1 sibling, 0 replies; 16+ messages in thread
From: Lukas Straub @ 2023-04-27 21:13 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: qemu-devel, qemu-block, michael.roth, armbru, eblake, jasowang,
quintela, zhanghailiang, philmd, thuth, berrange,
marcandre.lureau, pbonzini, dave, hreitz, kwolf, chen.zhang,
lizhijian
[-- Attachment #1: Type: text/plain, Size: 12465 bytes --]
On Thu, 27 Apr 2023 23:29:45 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote:
> 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>
Reviewed-by: Lukas Straub <lukasstraub2@web.de>
> ---
> hmp-commands.hx | 2 ++
> migration/colo.c | 28 -------------------------
> migration/meson.build | 6 ++++--
> migration/migration-hmp-cmds.c | 2 ++
> migration/options.c | 17 ++++++++--------
> qapi/migration.json | 12 +++++++----
> stubs/colo.c | 37 ++++++++++++++++++++++++++++++++++
> stubs/meson.build | 1 +
> 8 files changed, 62 insertions(+), 43 deletions(-)
> create mode 100644 stubs/colo.c
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index bb85ee1d26..fbd0932232 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1035,6 +1035,7 @@ SRST
> migration (or once already in postcopy).
> ERST
>
> +#ifdef CONFIG_REPLICATION
> {
> .name = "x_colo_lost_heartbeat",
> .args_type = "",
> @@ -1043,6 +1044,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 07bfa21fea..e4af47eeeb 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"
> @@ -68,7 +66,6 @@ static bool colo_runstate_is_stopped(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;
> @@ -133,14 +130,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;
> @@ -181,9 +174,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)
> @@ -217,7 +207,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)
> @@ -271,7 +260,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)
> {
> @@ -435,15 +423,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) {
> @@ -561,15 +545,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();
> @@ -748,7 +728,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);
> @@ -765,9 +744,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);
>
> @@ -874,15 +850,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 480ff6854a..9c6a8e65d3 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',
> @@ -30,6 +28,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/options.c b/migration/options.c
> index 912cbadddb..eef2bd0f16 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -171,7 +171,9 @@ Property migration_properties[] = {
> DEFINE_PROP_MIG_CAP("x-postcopy-ram", MIGRATION_CAPABILITY_POSTCOPY_RAM),
> DEFINE_PROP_MIG_CAP("x-postcopy-preempt",
> MIGRATION_CAPABILITY_POSTCOPY_PREEMPT),
> +#ifdef CONFIG_REPLICATION
> DEFINE_PROP_MIG_CAP("x-colo", MIGRATION_CAPABILITY_X_COLO),
> +#endif
> DEFINE_PROP_MIG_CAP("x-release-ram", MIGRATION_CAPABILITY_RELEASE_RAM),
> DEFINE_PROP_MIG_CAP("x-block", MIGRATION_CAPABILITY_BLOCK),
> DEFINE_PROP_MIG_CAP("x-return-path", MIGRATION_CAPABILITY_RETURN_PATH),
> @@ -209,9 +211,13 @@ bool migrate_block(void)
>
> bool migrate_colo(void)
> {
> +#ifdef CONFIG_REPLICATION
> MigrationState *s = migrate_get_current();
>
> return s->capabilities[MIGRATION_CAPABILITY_X_COLO];
> +#else
> + return false;
> +#endif
> }
>
> bool migrate_compress(void)
> @@ -401,7 +407,9 @@ INITIALIZE_MIGRATE_CAPS_SET(check_caps_background_snapshot,
> MIGRATION_CAPABILITY_RDMA_PIN_ALL,
> MIGRATION_CAPABILITY_COMPRESS,
> MIGRATION_CAPABILITY_XBZRLE,
> +#ifdef CONFIG_REPLICATION
> MIGRATION_CAPABILITY_X_COLO,
> +#endif
> MIGRATION_CAPABILITY_VALIDATE_UUID,
> MIGRATION_CAPABILITY_ZERO_COPY_SEND);
>
> @@ -428,15 +436,6 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
> }
> #endif
>
> -#ifndef CONFIG_REPLICATION
> - if (new_caps[MIGRATION_CAPABILITY_X_COLO]) {
> - error_setg(errp, "QEMU compiled without replication module"
> - " can't enable COLO");
> - error_append_hint(errp, "Please enable replication before COLO.\n");
> - return false;
> - }
> -#endif
> -
> if (new_caps[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
> /* This check is reasonably expensive, so only when it's being
> * set the first time, also it's only the destination that needs
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 2c35b7b9cf..5cb095f7b3 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -486,7 +486,8 @@
> { 'enum': 'MigrationCapability',
> 'data': ['xbzrle', 'rdma-pin-all', 'auto-converge', 'zero-blocks',
> 'compress', 'events', 'postcopy-ram',
> - { 'name': 'x-colo', 'features': [ 'unstable' ] },
> + { 'name': 'x-colo', 'features': [ 'unstable' ],
> + 'if': 'CONFIG_REPLICATION' },
> 'release-ram',
> 'block', 'return-path', 'pause-before-switchover', 'multifd',
> 'dirty-bitmaps', 'postcopy-blocktime', 'late-block-activate',
> @@ -1381,7 +1382,8 @@
> #
> ##
> { 'command': 'x-colo-lost-heartbeat',
> - 'features': [ 'unstable' ] }
> + 'features': [ 'unstable' ],
> + 'if': 'CONFIG_REPLICATION' }
>
> ##
> # @migrate_cancel:
> @@ -1657,7 +1659,8 @@
> ##
> { 'struct': 'COLOStatus',
> 'data': { 'mode': 'COLOMode', 'last-mode': 'COLOMode',
> - 'reason': 'COLOExitReason' } }
> + 'reason': 'COLOExitReason' },
> + 'if': 'CONFIG_REPLICATION' }
>
> ##
> # @query-colo-status:
> @@ -1674,7 +1677,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..f306ab45d6
> --- /dev/null
> +++ b/stubs/colo.c
> @@ -0,0 +1,37 @@
> +#include "qemu/osdep.h"
> +#include "qemu/notify.h"
> +#include "net/colo-compare.h"
> +#include "migration/colo.h"
> +#include "migration/migration.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-migration.h"
> +
> +void colo_shutdown(void)
> +{
> + abort();
> +}
> +
> +void *colo_process_incoming_thread(void *opaque)
> +{
> + abort();
> +}
> +
> +void colo_checkpoint_notify(void *opaque)
> +{
> + abort();
> +}
> +
> +void migrate_start_colo_process(MigrationState *s)
> +{
> + 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'))
--
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/4] configure: add --disable-colo-proxy option
2023-04-27 20:29 ` [PATCH v3 4/4] configure: add --disable-colo-proxy option Vladimir Sementsov-Ogievskiy
@ 2023-04-27 21:18 ` Lukas Straub
2023-04-27 21:30 ` Vladimir Sementsov-Ogievskiy
2023-04-28 7:33 ` Juan Quintela
1 sibling, 1 reply; 16+ messages in thread
From: Lukas Straub @ 2023-04-27 21:18 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: qemu-devel, qemu-block, michael.roth, armbru, eblake, jasowang,
quintela, zhanghailiang, philmd, thuth, berrange,
marcandre.lureau, pbonzini, dave, hreitz, kwolf, chen.zhang,
lizhijian
[-- Attachment #1: Type: text/plain, Size: 4699 bytes --]
On Thu, 27 Apr 2023 23:29:46 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote:
> Add option to not build filter-mirror, filter-rewriter and
> colo-compare when they are not needed.
>
> There could be more agile configuration, for example add separate
> options for each filter, but that may be done in future on demand. The
> aim of this patch is to make possible to disable the whole COLO Proxy
> subsystem.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> meson_options.txt | 2 ++
> net/meson.build | 14 ++++++++++----
> scripts/meson-buildoptions.sh | 3 +++
> stubs/colo-compare.c | 7 +++++++
> stubs/meson.build | 1 +
> 5 files changed, 23 insertions(+), 4 deletions(-)
> create mode 100644 stubs/colo-compare.c
>
> diff --git a/meson_options.txt b/meson_options.txt
> index 2471dd02da..b59e7ae342 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -289,6 +289,8 @@ option('live_block_migration', type: 'feature', value: 'auto',
> description: 'block migration in the main migration stream')
> option('replication', type: 'feature', value: 'auto',
> description: 'replication support')
> +option('colo_proxy', type: 'feature', value: 'auto',
> + description: 'colo-proxy support')
> option('bochs', type: 'feature', value: 'auto',
> description: 'bochs image format support')
> option('cloop', type: 'feature', value: 'auto',
> diff --git a/net/meson.build b/net/meson.build
> index 87afca3e93..4cfc850c69 100644
> --- a/net/meson.build
> +++ b/net/meson.build
> @@ -1,13 +1,9 @@
> softmmu_ss.add(files(
> 'announce.c',
> 'checksum.c',
> - 'colo-compare.c',
> - 'colo.c',
> 'dump.c',
> 'eth.c',
> 'filter-buffer.c',
> - 'filter-mirror.c',
> - 'filter-rewriter.c',
> 'filter.c',
> 'hub.c',
> 'net-hmp-cmds.c',
> @@ -19,6 +15,16 @@ softmmu_ss.add(files(
> 'util.c',
> ))
>
> +if get_option('replication').allowed() or \
> + get_option('colo_proxy').allowed()
> + softmmu_ss.add(files('colo-compare.c'))
> + softmmu_ss.add(files('colo.c'))
> +endif
> +
> +if get_option('colo_proxy').allowed()
> + softmmu_ss.add(files('filter-mirror.c', 'filter-rewriter.c'))
> +endif
> +
The last discussion didn't really come to a conclusion, but I still
think that 'filter-mirror.c' (which also contains filter-redirect)
should be left unchanged.
> softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
>
> if have_l2tpv3
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index d4369a3ad8..036047ce6f 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -83,6 +83,7 @@ meson_options_help() {
> printf "%s\n" ' capstone Whether and how to find the capstone library'
> printf "%s\n" ' cloop cloop image format support'
> printf "%s\n" ' cocoa Cocoa user interface (macOS only)'
> + printf "%s\n" ' colo-proxy colo-proxy support'
> printf "%s\n" ' coreaudio CoreAudio sound support'
> printf "%s\n" ' crypto-afalg Linux AF_ALG crypto backend driver'
> printf "%s\n" ' curl CURL block device driver'
> @@ -236,6 +237,8 @@ _meson_option_parse() {
> --disable-cloop) printf "%s" -Dcloop=disabled ;;
> --enable-cocoa) printf "%s" -Dcocoa=enabled ;;
> --disable-cocoa) printf "%s" -Dcocoa=disabled ;;
> + --enable-colo-proxy) printf "%s" -Dcolo_proxy=enabled ;;
> + --disable-colo-proxy) printf "%s" -Dcolo_proxy=disabled ;;
> --enable-coreaudio) printf "%s" -Dcoreaudio=enabled ;;
> --disable-coreaudio) printf "%s" -Dcoreaudio=disabled ;;
> --enable-coroutine-pool) printf "%s" -Dcoroutine_pool=true ;;
> diff --git a/stubs/colo-compare.c b/stubs/colo-compare.c
> new file mode 100644
> index 0000000000..ec726665be
> --- /dev/null
> +++ b/stubs/colo-compare.c
> @@ -0,0 +1,7 @@
> +#include "qemu/osdep.h"
> +#include "qemu/notify.h"
> +#include "net/colo-compare.h"
> +
> +void colo_compare_cleanup(void)
> +{
> +}
> diff --git a/stubs/meson.build b/stubs/meson.build
> index 8412cad15f..a56645e2f7 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -46,6 +46,7 @@ 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('colo-compare.c'))
> stub_ss.add(files('vmstate.c'))
> stub_ss.add(files('vm-stop.c'))
> stub_ss.add(files('win32-kbd-hook.c'))
--
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/4] configure: add --disable-colo-proxy option
2023-04-27 21:18 ` Lukas Straub
@ 2023-04-27 21:30 ` Vladimir Sementsov-Ogievskiy
2023-04-28 8:49 ` Zhang, Chen
0 siblings, 1 reply; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2023-04-27 21:30 UTC (permalink / raw)
To: Lukas Straub
Cc: qemu-devel, qemu-block, michael.roth, armbru, eblake, jasowang,
quintela, zhanghailiang, philmd, thuth, berrange,
marcandre.lureau, pbonzini, dave, hreitz, kwolf, chen.zhang,
lizhijian
On 28.04.23 00:18, Lukas Straub wrote:
> On Thu, 27 Apr 2023 23:29:46 +0300
> Vladimir Sementsov-Ogievskiy<vsementsov@yandex-team.ru> wrote:
>
>> Add option to not build filter-mirror, filter-rewriter and
>> colo-compare when they are not needed.
>>
>> There could be more agile configuration, for example add separate
>> options for each filter, but that may be done in future on demand. The
>> aim of this patch is to make possible to disable the whole COLO Proxy
>> subsystem.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy<vsementsov@yandex-team.ru>
>> ---
>> meson_options.txt | 2 ++
>> net/meson.build | 14 ++++++++++----
>> scripts/meson-buildoptions.sh | 3 +++
>> stubs/colo-compare.c | 7 +++++++
>> stubs/meson.build | 1 +
>> 5 files changed, 23 insertions(+), 4 deletions(-)
>> create mode 100644 stubs/colo-compare.c
>>
>> diff --git a/meson_options.txt b/meson_options.txt
>> index 2471dd02da..b59e7ae342 100644
>> --- a/meson_options.txt
>> +++ b/meson_options.txt
>> @@ -289,6 +289,8 @@ option('live_block_migration', type: 'feature', value: 'auto',
>> description: 'block migration in the main migration stream')
>> option('replication', type: 'feature', value: 'auto',
>> description: 'replication support')
>> +option('colo_proxy', type: 'feature', value: 'auto',
>> + description: 'colo-proxy support')
>> option('bochs', type: 'feature', value: 'auto',
>> description: 'bochs image format support')
>> option('cloop', type: 'feature', value: 'auto',
>> diff --git a/net/meson.build b/net/meson.build
>> index 87afca3e93..4cfc850c69 100644
>> --- a/net/meson.build
>> +++ b/net/meson.build
>> @@ -1,13 +1,9 @@
>> softmmu_ss.add(files(
>> 'announce.c',
>> 'checksum.c',
>> - 'colo-compare.c',
>> - 'colo.c',
>> 'dump.c',
>> 'eth.c',
>> 'filter-buffer.c',
>> - 'filter-mirror.c',
>> - 'filter-rewriter.c',
>> 'filter.c',
>> 'hub.c',
>> 'net-hmp-cmds.c',
>> @@ -19,6 +15,16 @@ softmmu_ss.add(files(
>> 'util.c',
>> ))
>>
>> +if get_option('replication').allowed() or \
>> + get_option('colo_proxy').allowed()
>> + softmmu_ss.add(files('colo-compare.c'))
>> + softmmu_ss.add(files('colo.c'))
>> +endif
>> +
>> +if get_option('colo_proxy').allowed()
>> + softmmu_ss.add(files('filter-mirror.c', 'filter-rewriter.c'))
>> +endif
>> +
> The last discussion didn't really come to a conclusion, but I still
> think that 'filter-mirror.c' (which also contains filter-redirect)
> should be left unchanged.
>
OK for me, I'll wait a bit for more comments and resend with
@@ -22,7 +22,7 @@ if get_option('replication').allowed() or \
endif
if get_option('colo_proxy').allowed()
- softmmu_ss.add(files('filter-mirror.c', 'filter-rewriter.c'))
+ softmmu_ss.add(files('filter-rewriter.c'))
endif
softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
applied here, if no other strong opinion.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/4] build: move COLO under CONFIG_REPLICATION
2023-04-27 20:29 ` [PATCH v3 3/4] build: move COLO under CONFIG_REPLICATION Vladimir Sementsov-Ogievskiy
2023-04-27 21:13 ` Lukas Straub
@ 2023-04-28 7:30 ` Juan Quintela
2023-04-28 8:32 ` Vladimir Sementsov-Ogievskiy
1 sibling, 1 reply; 16+ messages in thread
From: Juan Quintela @ 2023-04-28 7:30 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: qemu-devel, qemu-block, michael.roth, armbru, eblake, jasowang,
zhanghailiang, philmd, thuth, berrange, marcandre.lureau,
pbonzini, dave, hreitz, kwolf, chen.zhang, lizhijian,
lukasstraub2
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote:
> 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>
Nice patch. Thanks.
> @@ -68,7 +66,6 @@ static bool colo_runstate_is_stopped(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;
> @@ -133,14 +130,10 @@ static void secondary_vm_do_failover(void)
> if (mis->migration_incoming_co) {
> qemu_coroutine_enter(mis->migration_incoming_co);
> }
> -#else
> - abort();
> -#endif
> }
With only this chunks you have proved that your argument is right.
abort() is never a solution.
> diff --git a/migration/options.c b/migration/options.c
> index 912cbadddb..eef2bd0f16 100644
> --- a/migration/options.c
> +++ b/migration/options.c
> @@ -171,7 +171,9 @@ Property migration_properties[] = {
> DEFINE_PROP_MIG_CAP("x-postcopy-ram", MIGRATION_CAPABILITY_POSTCOPY_RAM),
> DEFINE_PROP_MIG_CAP("x-postcopy-preempt",
> MIGRATION_CAPABILITY_POSTCOPY_PREEMPT),
> +#ifdef CONFIG_REPLICATION
> DEFINE_PROP_MIG_CAP("x-colo", MIGRATION_CAPABILITY_X_COLO),
> +#endif
> DEFINE_PROP_MIG_CAP("x-release-ram", MIGRATION_CAPABILITY_RELEASE_RAM),
> DEFINE_PROP_MIG_CAP("x-block", MIGRATION_CAPABILITY_BLOCK),
> DEFINE_PROP_MIG_CAP("x-return-path", MIGRATION_CAPABILITY_RETURN_PATH),
> @@ -209,9 +211,13 @@ bool migrate_block(void)
> bool migrate_colo(void)
> {
> +#ifdef CONFIG_REPLICATION
> MigrationState *s = migrate_get_current();
>
> return s->capabilities[MIGRATION_CAPABILITY_X_COLO];
> +#else
> + return false;
> +#endif
> }
#ifdef 1
>
> bool migrate_compress(void)
> @@ -401,7 +407,9 @@ INITIALIZE_MIGRATE_CAPS_SET(check_caps_background_snapshot,
> MIGRATION_CAPABILITY_RDMA_PIN_ALL,
> MIGRATION_CAPABILITY_COMPRESS,
> MIGRATION_CAPABILITY_XBZRLE,
> +#ifdef CONFIG_REPLICATION
> MIGRATION_CAPABILITY_X_COLO,
> +#endif
> MIGRATION_CAPABILITY_VALIDATE_UUID,
> MIGRATION_CAPABILITY_ZERO_COPY_SEND);
>
#ifdef 2
> @@ -428,15 +436,6 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
> }
> #endif
>
> -#ifndef CONFIG_REPLICATION
> - if (new_caps[MIGRATION_CAPABILITY_X_COLO]) {
> - error_setg(errp, "QEMU compiled without replication module"
> - " can't enable COLO");
> - error_append_hint(errp, "Please enable replication before COLO.\n");
> - return false;
> - }
> -#endif
> -
> if (new_caps[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
> /* This check is reasonably expensive, so only when it's being
> * set the first time, also it's only the destination that needs
I would preffer if you removed #ifdef 1 and 2 and let this one in. I am
trying to get all capabilities to this format.
> diff --git a/stubs/colo.c b/stubs/colo.c
> new file mode 100644
> index 0000000000..f306ab45d6
> --- /dev/null
> +++ b/stubs/colo.c
> @@ -0,0 +1,37 @@
> +#include "qemu/osdep.h"
> +#include "qemu/notify.h"
> +#include "net/colo-compare.h"
> +#include "migration/colo.h"
> +#include "migration/migration.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-migration.h"
> +
> +void colo_shutdown(void)
> +{
> + abort();
> +}
This is wrong, it should be empty.
void migration_shutdown(void)
{
/*
* When the QEMU main thread exit, the COLO thread
* may wait a semaphore. So, we should wakeup the
* COLO thread before migration shutdown.
*/
colo_shutdown();
......
}
> +void *colo_process_incoming_thread(void *opaque)
> +{
> + abort();
> +}
At least print an error message?
> +void colo_checkpoint_notify(void *opaque)
> +{
> + abort();
> +}
Another error message.
It is independently of this patch, but I am thinking about changing the
interface and doing something like this in options.c
changing
if (params->has_x_checkpoint_delay) {
s->parameters.x_checkpoint_delay = params->x_checkpoint_delay;
if (migration_in_colo_state()) {
colo_checkpoint_notify(s);
}
}
To
if (params->has_x_checkpoint_delay) {
s->parameters.x_checkpoint_delay = params->x_checkpoint_delay;
colo_checkpoint_refresh(s);
}
That way we can convert it to an empty function.
> +void migrate_start_colo_process(MigrationState *s)
> +{
> + abort();
> +}
Another case of changing the function interface?
case MIGRATION_STATUS_COLO:
if (!migrate_colo()) {
error_report("%s: critical error: calling COLO code without "
"COLO enabled", __func__);
}
migrate_start_colo_process(s);
The changes of functions interfaces are independent of this patch.
Later, Juan.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/4] configure: add --disable-colo-proxy option
2023-04-27 20:29 ` [PATCH v3 4/4] configure: add --disable-colo-proxy option Vladimir Sementsov-Ogievskiy
2023-04-27 21:18 ` Lukas Straub
@ 2023-04-28 7:33 ` Juan Quintela
2023-04-28 8:30 ` Vladimir Sementsov-Ogievskiy
1 sibling, 1 reply; 16+ messages in thread
From: Juan Quintela @ 2023-04-28 7:33 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: qemu-devel, qemu-block, michael.roth, armbru, eblake, jasowang,
zhanghailiang, philmd, thuth, berrange, marcandre.lureau,
pbonzini, dave, hreitz, kwolf, chen.zhang, lizhijian,
lukasstraub2
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote:
> Add option to not build filter-mirror, filter-rewriter and
> colo-compare when they are not needed.
>
> There could be more agile configuration, for example add separate
> options for each filter, but that may be done in future on demand. The
> aim of this patch is to make possible to disable the whole COLO Proxy
> subsystem.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
As you have arrived to an agreement about what to do with
filter-rewriter, the rest of the patch is ok with me.
Reviewed-by: Juan Quintela <quintela@redhat.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/4] configure: add --disable-colo-proxy option
2023-04-28 7:33 ` Juan Quintela
@ 2023-04-28 8:30 ` Vladimir Sementsov-Ogievskiy
2023-04-28 8:53 ` Juan Quintela
2023-04-28 8:54 ` Juan Quintela
0 siblings, 2 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2023-04-28 8:30 UTC (permalink / raw)
To: quintela
Cc: qemu-devel, qemu-block, michael.roth, armbru, eblake, jasowang,
zhanghailiang, philmd, thuth, berrange, marcandre.lureau,
pbonzini, dave, hreitz, kwolf, chen.zhang, lizhijian,
lukasstraub2
On 28.04.23 10:33, Juan Quintela wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote:
>> Add option to not build filter-mirror, filter-rewriter and
>> colo-compare when they are not needed.
>>
>> There could be more agile configuration, for example add separate
>> options for each filter, but that may be done in future on demand. The
>> aim of this patch is to make possible to disable the whole COLO Proxy
>> subsystem.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>
> As you have arrived to an agreement about what to do with
> filter-rewriter, the rest of the patch is ok with me.
You mean filter-mirror, precisely this change to the patch:
@@ -22,7 +22,7 @@ if get_option('replication').allowed() or \
endif
if get_option('colo_proxy').allowed()
- softmmu_ss.add(files('filter-mirror.c', 'filter-rewriter.c'))
+ softmmu_ss.add(files('filter-rewriter.c'))
endif
softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
?
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 3/4] build: move COLO under CONFIG_REPLICATION
2023-04-28 7:30 ` Juan Quintela
@ 2023-04-28 8:32 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2023-04-28 8:32 UTC (permalink / raw)
To: quintela
Cc: qemu-devel, qemu-block, michael.roth, armbru, eblake, jasowang,
zhanghailiang, philmd, thuth, berrange, marcandre.lureau,
pbonzini, dave, hreitz, kwolf, chen.zhang, lizhijian,
lukasstraub2
On 28.04.23 10:30, Juan Quintela wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote:
>> 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>
>
> Nice patch. Thanks.
>
>> @@ -68,7 +66,6 @@ static bool colo_runstate_is_stopped(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;
>> @@ -133,14 +130,10 @@ static void secondary_vm_do_failover(void)
>> if (mis->migration_incoming_co) {
>> qemu_coroutine_enter(mis->migration_incoming_co);
>> }
>> -#else
>> - abort();
>> -#endif
>> }
>
> With only this chunks you have proved that your argument is right.
> abort() is never a solution.
>
>> diff --git a/migration/options.c b/migration/options.c
>> index 912cbadddb..eef2bd0f16 100644
>> --- a/migration/options.c
>> +++ b/migration/options.c
>> @@ -171,7 +171,9 @@ Property migration_properties[] = {
>> DEFINE_PROP_MIG_CAP("x-postcopy-ram", MIGRATION_CAPABILITY_POSTCOPY_RAM),
>> DEFINE_PROP_MIG_CAP("x-postcopy-preempt",
>> MIGRATION_CAPABILITY_POSTCOPY_PREEMPT),
>> +#ifdef CONFIG_REPLICATION
>> DEFINE_PROP_MIG_CAP("x-colo", MIGRATION_CAPABILITY_X_COLO),
>> +#endif
>> DEFINE_PROP_MIG_CAP("x-release-ram", MIGRATION_CAPABILITY_RELEASE_RAM),
>> DEFINE_PROP_MIG_CAP("x-block", MIGRATION_CAPABILITY_BLOCK),
>> DEFINE_PROP_MIG_CAP("x-return-path", MIGRATION_CAPABILITY_RETURN_PATH),
>> @@ -209,9 +211,13 @@ bool migrate_block(void)
>
>> bool migrate_colo(void)
>> {
>> +#ifdef CONFIG_REPLICATION
>> MigrationState *s = migrate_get_current();
>>
>> return s->capabilities[MIGRATION_CAPABILITY_X_COLO];
>> +#else
>> + return false;
>> +#endif
>> }
>
> #ifdef 1
>
>>
>> bool migrate_compress(void)
>> @@ -401,7 +407,9 @@ INITIALIZE_MIGRATE_CAPS_SET(check_caps_background_snapshot,
>> MIGRATION_CAPABILITY_RDMA_PIN_ALL,
>> MIGRATION_CAPABILITY_COMPRESS,
>> MIGRATION_CAPABILITY_XBZRLE,
>> +#ifdef CONFIG_REPLICATION
>> MIGRATION_CAPABILITY_X_COLO,
>> +#endif
>> MIGRATION_CAPABILITY_VALIDATE_UUID,
>> MIGRATION_CAPABILITY_ZERO_COPY_SEND);
>>
>
> #ifdef 2
>
>> @@ -428,15 +436,6 @@ bool migrate_caps_check(bool *old_caps, bool *new_caps, Error **errp)
>> }
>> #endif
>>
>> -#ifndef CONFIG_REPLICATION
>> - if (new_caps[MIGRATION_CAPABILITY_X_COLO]) {
>> - error_setg(errp, "QEMU compiled without replication module"
>> - " can't enable COLO");
>> - error_append_hint(errp, "Please enable replication before COLO.\n");
>> - return false;
>> - }
>> -#endif
>> -
>> if (new_caps[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
>> /* This check is reasonably expensive, so only when it's being
>> * set the first time, also it's only the destination that needs
>
> I would preffer if you removed #ifdef 1 and 2 and let this one in. I am
> trying to get all capabilities to this format.
OK, let's start with your idea, interface may be removed later if we want.
>
>
>> diff --git a/stubs/colo.c b/stubs/colo.c
>> new file mode 100644
>> index 0000000000..f306ab45d6
>> --- /dev/null
>> +++ b/stubs/colo.c
>> @@ -0,0 +1,37 @@
>> +#include "qemu/osdep.h"
>> +#include "qemu/notify.h"
>> +#include "net/colo-compare.h"
>> +#include "migration/colo.h"
>> +#include "migration/migration.h"
>> +#include "qapi/error.h"
>> +#include "qapi/qapi-commands-migration.h"
>> +
>> +void colo_shutdown(void)
>> +{
>> + abort();
>> +}
>
> This is wrong, it should be empty.
Oops right. Good catch
>
> void migration_shutdown(void)
> {
> /*
> * When the QEMU main thread exit, the COLO thread
> * may wait a semaphore. So, we should wakeup the
> * COLO thread before migration shutdown.
> */
> colo_shutdown();
>
> ......
>
> }
>
>
>
>> +void *colo_process_incoming_thread(void *opaque)
>> +{
>> + abort();
>> +}
>
> At least print an error message?
OK
>
>> +void colo_checkpoint_notify(void *opaque)
>> +{
>> + abort();
>> +}
>
> Another error message.
>
> It is independently of this patch, but I am thinking about changing the
> interface and doing something like this in options.c
>
> changing
>
> if (params->has_x_checkpoint_delay) {
> s->parameters.x_checkpoint_delay = params->x_checkpoint_delay;
> if (migration_in_colo_state()) {
> colo_checkpoint_notify(s);
> }
> }
>
> To
>
> if (params->has_x_checkpoint_delay) {
> s->parameters.x_checkpoint_delay = params->x_checkpoint_delay;
> colo_checkpoint_refresh(s);
> }
>
> That way we can convert it to an empty function.
Sounds good. I can make a patch and include it to v4
>
>> +void migrate_start_colo_process(MigrationState *s)
>> +{
>> + abort();
>> +}
>
> Another case of changing the function interface?
>
> case MIGRATION_STATUS_COLO:
> if (!migrate_colo()) {
> error_report("%s: critical error: calling COLO code without "
> "COLO enabled", __func__);
> }
I think, it could be abort()
> migrate_start_colo_process(s);
>
> The changes of functions interfaces are independent of this patch.
>
Still, making empty stubs is clearer than stubs with abort(). I'll try and if changing the interface is not too much, will include it in v4.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH v3 4/4] configure: add --disable-colo-proxy option
2023-04-27 21:30 ` Vladimir Sementsov-Ogievskiy
@ 2023-04-28 8:49 ` Zhang, Chen
0 siblings, 0 replies; 16+ messages in thread
From: Zhang, Chen @ 2023-04-28 8:49 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, Lukas Straub
Cc: qemu-devel, qemu-block, michael.roth, armbru, eblake, jasowang,
quintela, Zhang, Hailiang, philmd, thuth, berrange,
marcandre.lureau, pbonzini, dave, hreitz, kwolf, lizhijian
> -----Original Message-----
> From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Sent: Friday, April 28, 2023 5:30 AM
> To: Lukas Straub <lukasstraub2@web.de>
> Cc: qemu-devel@nongnu.org; qemu-block@nongnu.org;
> michael.roth@amd.com; armbru@redhat.com; eblake@redhat.com;
> jasowang@redhat.com; quintela@redhat.com; Zhang, Hailiang
> <zhanghailiang@xfusion.com>; philmd@linaro.org; thuth@redhat.com;
> berrange@redhat.com; marcandre.lureau@redhat.com;
> pbonzini@redhat.com; dave@treblig.org; hreitz@redhat.com;
> kwolf@redhat.com; Zhang, Chen <chen.zhang@intel.com>;
> lizhijian@fujitsu.com
> Subject: Re: [PATCH v3 4/4] configure: add --disable-colo-proxy option
>
> On 28.04.23 00:18, Lukas Straub wrote:
> > On Thu, 27 Apr 2023 23:29:46 +0300
> > Vladimir Sementsov-Ogievskiy<vsementsov@yandex-team.ru> wrote:
> >
> >> Add option to not build filter-mirror, filter-rewriter and
> >> colo-compare when they are not needed.
> >>
> >> There could be more agile configuration, for example add separate
> >> options for each filter, but that may be done in future on demand.
> >> The aim of this patch is to make possible to disable the whole COLO
> >> Proxy subsystem.
> >>
> >> Signed-off-by: Vladimir
> >> Sementsov-Ogievskiy<vsementsov@yandex-team.ru>
> >> ---
> >> meson_options.txt | 2 ++
> >> net/meson.build | 14 ++++++++++----
> >> scripts/meson-buildoptions.sh | 3 +++
> >> stubs/colo-compare.c | 7 +++++++
> >> stubs/meson.build | 1 +
> >> 5 files changed, 23 insertions(+), 4 deletions(-)
> >> create mode 100644 stubs/colo-compare.c
> >>
> >> diff --git a/meson_options.txt b/meson_options.txt index
> >> 2471dd02da..b59e7ae342 100644
> >> --- a/meson_options.txt
> >> +++ b/meson_options.txt
> >> @@ -289,6 +289,8 @@ option('live_block_migration', type: 'feature',
> value: 'auto',
> >> description: 'block migration in the main migration stream')
> >> option('replication', type: 'feature', value: 'auto',
> >> description: 'replication support')
> >> +option('colo_proxy', type: 'feature', value: 'auto',
> >> + description: 'colo-proxy support')
> >> option('bochs', type: 'feature', value: 'auto',
> >> description: 'bochs image format support')
> >> option('cloop', type: 'feature', value: 'auto', diff --git
> >> a/net/meson.build b/net/meson.build index 87afca3e93..4cfc850c69
> >> 100644
> >> --- a/net/meson.build
> >> +++ b/net/meson.build
> >> @@ -1,13 +1,9 @@
> >> softmmu_ss.add(files(
> >> 'announce.c',
> >> 'checksum.c',
> >> - 'colo-compare.c',
> >> - 'colo.c',
> >> 'dump.c',
> >> 'eth.c',
> >> 'filter-buffer.c',
> >> - 'filter-mirror.c',
Need fix here for filter-mirror.c too.
> >> - 'filter-rewriter.c',
> >> 'filter.c',
> >> 'hub.c',
> >> 'net-hmp-cmds.c',
> >> @@ -19,6 +15,16 @@ softmmu_ss.add(files(
> >> 'util.c',
> >> ))
> >>
> >> +if get_option('replication').allowed() or \
> >> + get_option('colo_proxy').allowed()
> >> + softmmu_ss.add(files('colo-compare.c'))
> >> + softmmu_ss.add(files('colo.c'))
> >> +endif
> >> +
> >> +if get_option('colo_proxy').allowed()
> >> + softmmu_ss.add(files('filter-mirror.c', 'filter-rewriter.c'))
> >> +endif
> >> +
> > The last discussion didn't really come to a conclusion, but I still
> > think that 'filter-mirror.c' (which also contains filter-redirect)
> > should be left unchanged.
> >
>
> OK for me, I'll wait a bit for more comments and resend with
>
> @@ -22,7 +22,7 @@ if get_option('replication').allowed() or \
> endif
>
> if get_option('colo_proxy').allowed()
> - softmmu_ss.add(files('filter-mirror.c', 'filter-rewriter.c'))
> + softmmu_ss.add(files('filter-rewriter.c'))
> endif
>
> softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
>
>
> applied here, if no other strong opinion.
>
It's OK to me except for the filter-mirror.c related comments.
Thanks
Chen
> --
> Best regards,
> Vladimir
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/4] configure: add --disable-colo-proxy option
2023-04-28 8:30 ` Vladimir Sementsov-Ogievskiy
@ 2023-04-28 8:53 ` Juan Quintela
2023-04-28 8:54 ` Juan Quintela
1 sibling, 0 replies; 16+ messages in thread
From: Juan Quintela @ 2023-04-28 8:53 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: qemu-devel, qemu-block, michael.roth, armbru, eblake, jasowang,
zhanghailiang, philmd, thuth, berrange, marcandre.lureau,
pbonzini, dave, hreitz, kwolf, chen.zhang, lizhijian,
lukasstraub2
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote:
> On 28.04.23 10:33, Juan Quintela wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote:
>>> Add option to not build filter-mirror, filter-rewriter and
>>> colo-compare when they are not needed.
>>>
>>> There could be more agile configuration, for example add separate
>>> options for each filter, but that may be done in future on demand. The
>>> aim of this patch is to make possible to disable the whole COLO Proxy
>>> subsystem.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> As you have arrived to an agreement about what to do with
>> filter-rewriter, the rest of the patch is ok with me.
>
> You mean filter-mirror, precisely this change to the patch:
>
> @@ -22,7 +22,7 @@ if get_option('replication').allowed() or \
> endif
> if get_option('colo_proxy').allowed()
> - softmmu_ss.add(files('filter-mirror.c', 'filter-rewriter.c'))
> + softmmu_ss.add(files('filter-rewriter.c'))
> endif
> softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
>
> ?
Oops, yes.
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3 4/4] configure: add --disable-colo-proxy option
2023-04-28 8:30 ` Vladimir Sementsov-Ogievskiy
2023-04-28 8:53 ` Juan Quintela
@ 2023-04-28 8:54 ` Juan Quintela
1 sibling, 0 replies; 16+ messages in thread
From: Juan Quintela @ 2023-04-28 8:54 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: qemu-devel, qemu-block, michael.roth, armbru, eblake, jasowang,
zhanghailiang, philmd, thuth, berrange, marcandre.lureau,
pbonzini, dave, hreitz, kwolf, chen.zhang, lizhijian,
lukasstraub2
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote:
> On 28.04.23 10:33, Juan Quintela wrote:
>> Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote:
>>> Add option to not build filter-mirror, filter-rewriter and
>>> colo-compare when they are not needed.
>>>
>>> There could be more agile configuration, for example add separate
>>> options for each filter, but that may be done in future on demand. The
>>> aim of this patch is to make possible to disable the whole COLO Proxy
>>> subsystem.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
>> As you have arrived to an agreement about what to do with
>> filter-rewriter, the rest of the patch is ok with me.
>
> You mean filter-mirror, precisely this change to the patch:
>
> @@ -22,7 +22,7 @@ if get_option('replication').allowed() or \
> endif
> if get_option('colo_proxy').allowed()
> - softmmu_ss.add(files('filter-mirror.c', 'filter-rewriter.c'))
> + softmmu_ss.add(files('filter-rewriter.c'))
> endif
> softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
>
> ?
Oops, yes.
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2023-04-28 8:54 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-27 20:29 [PATCH v3 0/4] COLO: improve build options Vladimir Sementsov-Ogievskiy
2023-04-27 20:29 ` [PATCH v3 1/4] block/meson.build: prefer positive condition for replication Vladimir Sementsov-Ogievskiy
2023-04-27 20:47 ` Lukas Straub
2023-04-27 20:29 ` [PATCH v3 2/4] scripts/qapi: allow optional experimental enum values Vladimir Sementsov-Ogievskiy
2023-04-27 20:29 ` [PATCH v3 3/4] build: move COLO under CONFIG_REPLICATION Vladimir Sementsov-Ogievskiy
2023-04-27 21:13 ` Lukas Straub
2023-04-28 7:30 ` Juan Quintela
2023-04-28 8:32 ` Vladimir Sementsov-Ogievskiy
2023-04-27 20:29 ` [PATCH v3 4/4] configure: add --disable-colo-proxy option Vladimir Sementsov-Ogievskiy
2023-04-27 21:18 ` Lukas Straub
2023-04-27 21:30 ` Vladimir Sementsov-Ogievskiy
2023-04-28 8:49 ` Zhang, Chen
2023-04-28 7:33 ` Juan Quintela
2023-04-28 8:30 ` Vladimir Sementsov-Ogievskiy
2023-04-28 8:53 ` Juan Quintela
2023-04-28 8:54 ` 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.