RE: [PATCH] replication: compile out some staff when replication is not configured

[Zhang, Chen]

> -----Original Message-----
> From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Sent: Tuesday, April 11, 2023 10:51 PM
> To: qemu-devel@nongnu.org
> Cc: qemu-block@nongnu.org; pbonzini@redhat.com; armbru@redhat.com;
> eblake@redhat.com; jasowang@redhat.com; dgilbert@redhat.com;
> quintela@redhat.com; hreitz@redhat.com; kwolf@redhat.com; Zhang,
> Hailiang <zhanghailiang@xfusion.com>; Zhang, Chen
> <chen.zhang@intel.com>; lizhijian@fujitsu.com;
> wencongyang2@huawei.com; xiechanglong.d@gmail.com; den-
> plotnikov@yandex-team.ru; Vladimir Sementsov-Ogievskiy
> <vsementsov@yandex-team.ru>
> Subject: [PATCH] replication: compile out some staff when replication is not
> configured
> 
> Don't compile-in replication-related files when replication is disabled in
> config.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
> 
> Hi all!
> 
> I'm unsure, should it be actually separate --disable-colo / --enable-colo
> options or it's really used only together with replication staff.. So, I 
> decided
> to start with simpler variant.
> 

For replication, I think there's nothing wrong with the idea.
But not so for COLO.  COLO project consists of three independent parts: 
Replication, migration, net-proxy.
Each one have ability to run alone for other proposals. For example we can just 
run filter-mirror/redirector for networking
Analysis/debugs. Although the best practice of COLO is to make the three 
modules work together, in fact, we can also
use only some modules of COLO for other usage scenarios. Like COLO migration + 
net-proxy for shared disk, etc...
So I think no need to disable all COLO related modules when replication is not 
configured.
For details:
https://wiki.qemu.org/Features/COLO

Thanks
Chen

> 
>  block/meson.build     |  2 +-
>  migration/meson.build |  6 ++++--
>  net/meson.build       |  8 ++++----
>  qapi/migration.json   |  6 ++++--
>  stubs/colo.c          | 46 +++++++++++++++++++++++++++++++++++++++++++
>  stubs/meson.build     |  1 +
>  6 files changed, 60 insertions(+), 9 deletions(-)  create mode 100644
> stubs/colo.c
> 
> 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')) diff --git
> a/migration/meson.build b/migration/meson.build index
> 0d1bb9f96e..8180eaea7b 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',
> @@ -29,6 +27,10 @@ softmmu_ss.add(files(
>    'threadinfo.c',
>  ), gnutls)
> 
> +if get_option('replication').allowed()
> +  softmmu_ss.add(files('colo.c', 'colo-failover.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/net/meson.build b/net/meson.build index
> 87afca3e93..634ab71cc6 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,10 @@ softmmu_ss.add(files(
>    'util.c',
>  ))
> 
> +if get_option('replication').allowed()
> +  softmmu_ss.add(files('colo-compare.c', 'colo.c', 'filter-rewriter.c',
> +'filter-mirror.c')) endif
> +
>  softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
> 
>  if have_l2tpv3
> diff --git a/qapi/migration.json b/qapi/migration.json index
> c84fa10e86..5b81e09369 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1685,7 +1685,8 @@
>  ##
>  { 'struct': 'COLOStatus',
>    'data': { 'mode': 'COLOMode', 'last-mode': 'COLOMode',
> -            'reason': 'COLOExitReason' } }
> +            'reason': 'COLOExitReason' },
> +  'if': 'CONFIG_REPLICATION' }
> 
>  ##
>  # @query-colo-status:
> @@ -1702,7 +1703,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..5a02540baa
> --- /dev/null
> +++ b/stubs/colo.c
> @@ -0,0 +1,46 @@
> +#include "qemu/osdep.h"
> +#include "qemu/notify.h"
> +#include "net/colo-compare.h"
> +#include "migration/colo.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-migration.h"
> +
> +void colo_compare_cleanup(void)
> +{
> +    abort();
> +}
> +
> +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;
> +}
> +
> +void qmp_x_colo_lost_heartbeat(Error **errp) {
> +    error_setg(errp, "COLO support is not built in"); }
> 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