| Message ID | 20230419225232.508121-4-vsementsov@yandex-team.ru |
|---|---|
| State | New |
| Headers | show |
| Series | COLO: improve build options | expand |
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. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > +bool migrate_colo_enabled(void) > +{ > + MigrationState *s = migrate_get_current(); > + return s->enabled_capabilities[MIGRATION_CAPABILITY_X_COLO]; > +} I consolidated all the capabilities check on my series on the list on options.c, so this will go there. > diff --git a/migration/migration.c b/migration/migration.c > index bda4789193..2382958364 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -165,7 +165,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 Why? I very much preffer the capability to be there and just fail when we try to enable it. That way we only need the #ifdef in one place and not all over the place. > MIGRATION_CAPABILITY_VALIDATE_UUID, > MIGRATION_CAPABILITY_ZERO_COPY_SEND); > > @@ -1329,15 +1331,6 @@ static bool migrate_caps_check(bool *cap_list, > } > #endif > > -#ifndef CONFIG_REPLICATION > - if (cap_list[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 > - See, like this O:-) > diff --git a/qapi/migration.json b/qapi/migration.json > index c84fa10e86..93863fa88c 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', Aha, It is for this that you changed the black magic on the previous patch. Looks ok from my ignorance. As said before, I would not remove the capability, left it the way it was. You have less code (less #ifdefs that you just had to add), and enabling/disabling checking capabilities don't need anything from replication. > # > ## > { 'command': 'x-colo-lost-heartbeat', > - 'features': [ 'unstable' ] } > + 'features': [ 'unstable' ], > + 'if': 'CONFIG_REPLICATION' } > > ## > # @migrate_cancel: > @@ -1685,7 +1687,8 @@ > ## > { 'struct': 'COLOStatus', > 'data': { 'mode': 'COLOMode', 'last-mode': 'COLOMode', > - 'reason': 'COLOExitReason' } } > + 'reason': 'COLOExitReason' }, > + 'if': 'CONFIG_REPLICATION' } > > ## > # @query-colo-status: > @@ -1702,7 +1705,8 @@ > # Since: 3.1 > ## > { 'command': 'query-colo-status', > - 'returns': 'COLOStatus' } > + 'returns': 'COLOStatus', > + 'if': 'CONFIG_REPLICATION' } > > ## > # @migrate-recover: Disabling the command is ok for me. > diff --git a/stubs/colo.c b/stubs/colo.c > new file mode 100644 > index 0000000000..45c8ac0cc6 ... > +bool migrate_colo_enabled(void) > +{ > + return false; > +} You don't need this one if you follow my idea. Notice that I fully agree with being able to disable colo O:-) Later, Juan.
On 20.04.23 13:03, 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. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > >> +bool migrate_colo_enabled(void) >> +{ >> + MigrationState *s = migrate_get_current(); >> + return s->enabled_capabilities[MIGRATION_CAPABILITY_X_COLO]; >> +} > > I consolidated all the capabilities check on my series on the list on > options.c, so this will go there. > >> diff --git a/migration/migration.c b/migration/migration.c >> index bda4789193..2382958364 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -165,7 +165,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 > > Why? > > I very much preffer the capability to be there and just fail when we try > to enable it. That way we only need the #ifdef in one place and not all > over the place. > >> MIGRATION_CAPABILITY_VALIDATE_UUID, >> MIGRATION_CAPABILITY_ZERO_COPY_SEND); >> >> @@ -1329,15 +1331,6 @@ static bool migrate_caps_check(bool *cap_list, >> } >> #endif >> >> -#ifndef CONFIG_REPLICATION >> - if (cap_list[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 >> - > > See, like this O:-) > >> diff --git a/qapi/migration.json b/qapi/migration.json >> index c84fa10e86..93863fa88c 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', > > > Aha, It is for this that you changed the black magic on the previous > patch. Looks ok from my ignorance. As said before, I would not remove > the capability, left it the way it was. > > You have less code (less #ifdefs that you just had to add), and > enabling/disabling checking capabilities don't need anything from replication. Yes, I had a sense of doubt while adding these #ifdefs. Still, on the other hand I feel that it's strange to have public interface which only can say "I'm not built in" :) Actually with my way, we have just two #ifdefs instead of one. Seems, not too many. And instead of "I'm not supported" error we just not include the public interface for unsupported feature. It seems to be better user experience. What do you think?
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote: > On 20.04.23 13:03, 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. >>> >>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> >> >>> +bool migrate_colo_enabled(void) >>> +{ >>> + MigrationState *s = migrate_get_current(); >>> + return s->enabled_capabilities[MIGRATION_CAPABILITY_X_COLO]; >>> +} >> Aha, It is for this that you changed the black magic on the previous >> patch. Looks ok from my ignorance. As said before, I would not remove >> the capability, left it the way it was. >> You have less code (less #ifdefs that you just had to add), and >> enabling/disabling checking capabilities don't need anything from replication. > > Yes, I had a sense of doubt while adding these #ifdefs. > > Still, on the other hand I feel that it's strange to have public interface which only can say "I'm not built in" :) > > Actually with my way, we have just two #ifdefs instead of one. Seems, > not too many. And instead of "I'm not supported" error we just not > include the public interface for unsupported feature. It seems to be > better user experience. What do you think? I preffer the regularity that all capabilities are the same, and the only place to look if something is disabled is a single place. But on the other hand, the main user is libvirt, so Daniel, what does libvirt preffers? /me guess that they have to do both anyways to detect old versions, but who knows. Later, Juan.
* 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. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > --- > hmp-commands.hx | 2 ++ > migration/colo.c | 6 +++++ > migration/meson.build | 6 +++-- > migration/migration-hmp-cmds.c | 2 ++ > migration/migration.c | 19 +++----------- > net/meson.build | 5 +++- > qapi/migration.json | 12 ++++++--- > stubs/colo.c | 47 ++++++++++++++++++++++++++++++++++ > stubs/meson.build | 1 + > 9 files changed, 78 insertions(+), 22 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 We seem to be inconsistent about whether the ifdef includes the SRST/ERST doc section; some ifdef do and some don't; and thus it depends whether or not you want the command documented even though it's compiled out. I think it's probably OK, but maybe worth reconsidering: Acked-by: Dr. David Alan Gilbert <dave@treblig.org> > SRST > ``x_colo_lost_heartbeat`` > diff --git a/migration/colo.c b/migration/colo.c > index 0716e64689..089c491d70 100644 > --- a/migration/colo.c > +++ b/migration/colo.c > @@ -196,6 +196,12 @@ COLOMode get_colo_mode(void) > } > } > > +bool migrate_colo_enabled(void) > +{ > + MigrationState *s = migrate_get_current(); > + return s->enabled_capabilities[MIGRATION_CAPABILITY_X_COLO]; > +} > + > void colo_do_failover(void) > { > /* Make sure VM stopped while failover happened. */ > diff --git a/migration/meson.build b/migration/meson.build > index 0d1bb9f96e..3fccf79f12 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-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 72519ea99f..4601c48f41 100644 > --- a/migration/migration-hmp-cmds.c > +++ b/migration/migration-hmp-cmds.c > @@ -640,6 +640,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; > @@ -647,6 +648,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 bda4789193..2382958364 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -165,7 +165,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); > > @@ -1329,15 +1331,6 @@ static bool migrate_caps_check(bool *cap_list, > } > #endif > > -#ifndef CONFIG_REPLICATION > - if (cap_list[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 (cap_list[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 > @@ -3577,12 +3570,6 @@ fail: > MIGRATION_STATUS_FAILED); > } > > -bool migrate_colo_enabled(void) > -{ > - MigrationState *s = migrate_get_current(); > - return s->enabled_capabilities[MIGRATION_CAPABILITY_X_COLO]; > -} > - > typedef enum MigThrError { > /* No error detected */ > MIG_THR_ERR_NONE = 0, > @@ -4537,7 +4524,9 @@ static 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), > diff --git a/net/meson.build b/net/meson.build > index 87afca3e93..38d50b8c96 100644 > --- a/net/meson.build > +++ b/net/meson.build > @@ -1,7 +1,6 @@ > softmmu_ss.add(files( > 'announce.c', > 'checksum.c', > - 'colo-compare.c', > 'colo.c', > 'dump.c', > 'eth.c', > @@ -19,6 +18,10 @@ softmmu_ss.add(files( > 'util.c', > )) > > +if get_option('replication').allowed() > + softmmu_ss.add(files('colo-compare.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..93863fa88c 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', > @@ -1409,7 +1410,8 @@ > # > ## > { 'command': 'x-colo-lost-heartbeat', > - 'features': [ 'unstable' ] } > + 'features': [ 'unstable' ], > + 'if': 'CONFIG_REPLICATION' } > > ## > # @migrate_cancel: > @@ -1685,7 +1687,8 @@ > ## > { 'struct': 'COLOStatus', > 'data': { 'mode': 'COLOMode', 'last-mode': 'COLOMode', > - 'reason': 'COLOExitReason' } } > + 'reason': 'COLOExitReason' }, > + 'if': 'CONFIG_REPLICATION' } > > ## > # @query-colo-status: > @@ -1702,7 +1705,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..45c8ac0cc6 > --- /dev/null > +++ b/stubs/colo.c > @@ -0,0 +1,47 @@ > +#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_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; > +} > + > +bool migrate_colo_enabled(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 >
> -----Original Message----- > From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > Sent: Thursday, April 20, 2023 6:53 AM > To: qemu-devel@nongnu.org > Cc: 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; Vladimir Sementsov-Ogievskiy <vsementsov@yandex- > team.ru> > Subject: [PATCH v2 3/4] build: move COLO under CONFIG_REPLICATION > > 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. Yes, you are right for current status. Because COLO best practices is replication + colo live migration + colo proxy. But doesn't mean it has to be done in all scenarios as I explanation in V1. The better way is allow to use x-colo capability firstly, and separate this patch with two config options: --disable-replication and --disable-x-colo. Thanks Chen > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > --- > hmp-commands.hx | 2 ++ > migration/colo.c | 6 +++++ > migration/meson.build | 6 +++-- > migration/migration-hmp-cmds.c | 2 ++ > migration/migration.c | 19 +++----------- > net/meson.build | 5 +++- > qapi/migration.json | 12 ++++++--- > stubs/colo.c | 47 ++++++++++++++++++++++++++++++++++ > stubs/meson.build | 1 + > 9 files changed, 78 insertions(+), 22 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 > 0716e64689..089c491d70 100644 > --- a/migration/colo.c > +++ b/migration/colo.c > @@ -196,6 +196,12 @@ COLOMode get_colo_mode(void) > } > } > > +bool migrate_colo_enabled(void) > +{ > + MigrationState *s = migrate_get_current(); > + return s->enabled_capabilities[MIGRATION_CAPABILITY_X_COLO]; > +} > + > void colo_do_failover(void) > { > /* Make sure VM stopped while failover happened. */ diff --git > a/migration/meson.build b/migration/meson.build index > 0d1bb9f96e..3fccf79f12 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-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 72519ea99f..4601c48f41 100644 > --- a/migration/migration-hmp-cmds.c > +++ b/migration/migration-hmp-cmds.c > @@ -640,6 +640,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; > @@ -647,6 +648,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 > bda4789193..2382958364 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -165,7 +165,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); > > @@ -1329,15 +1331,6 @@ static bool migrate_caps_check(bool *cap_list, > } > #endif > > -#ifndef CONFIG_REPLICATION > - if (cap_list[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 (cap_list[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 @@ - > 3577,12 +3570,6 @@ fail: > MIGRATION_STATUS_FAILED); } > > -bool migrate_colo_enabled(void) > -{ > - MigrationState *s = migrate_get_current(); > - return s->enabled_capabilities[MIGRATION_CAPABILITY_X_COLO]; > -} > - > typedef enum MigThrError { > /* No error detected */ > MIG_THR_ERR_NONE = 0, > @@ -4537,7 +4524,9 @@ static 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), > diff --git a/net/meson.build b/net/meson.build index > 87afca3e93..38d50b8c96 100644 > --- a/net/meson.build > +++ b/net/meson.build > @@ -1,7 +1,6 @@ > softmmu_ss.add(files( > 'announce.c', > 'checksum.c', > - 'colo-compare.c', > 'colo.c', > 'dump.c', > 'eth.c', > @@ -19,6 +18,10 @@ softmmu_ss.add(files( > 'util.c', > )) > > +if get_option('replication').allowed() > + softmmu_ss.add(files('colo-compare.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..93863fa88c 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', @@ -1409,7 > +1410,8 @@ # ## { 'command': 'x-colo-lost-heartbeat', > - 'features': [ 'unstable' ] } > + 'features': [ 'unstable' ], > + 'if': 'CONFIG_REPLICATION' } > > ## > # @migrate_cancel: > @@ -1685,7 +1687,8 @@ > ## > { 'struct': 'COLOStatus', > 'data': { 'mode': 'COLOMode', 'last-mode': 'COLOMode', > - 'reason': 'COLOExitReason' } } > + 'reason': 'COLOExitReason' }, > + 'if': 'CONFIG_REPLICATION' } > > ## > # @query-colo-status: > @@ -1702,7 +1705,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..45c8ac0cc6 > --- /dev/null > +++ b/stubs/colo.c > @@ -0,0 +1,47 @@ > +#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_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; > +} > + > +bool migrate_colo_enabled(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
On 21.04.23 06:02, Zhang, Chen wrote: > > >> -----Original Message----- >> From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> >> Sent: Thursday, April 20, 2023 6:53 AM >> To: qemu-devel@nongnu.org >> Cc: 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; Vladimir Sementsov-Ogievskiy <vsementsov@yandex- >> team.ru> >> Subject: [PATCH v2 3/4] build: move COLO under CONFIG_REPLICATION >> >> 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. > > Yes, you are right for current status. Because COLO best practices is replication + colo live migration + colo proxy. > But doesn't mean it has to be done in all scenarios as I explanation in V1. > The better way is allow to use x-colo capability firstly, and separate this patch > with two config options: --disable-replication and --disable-x-colo. > But what for? We for sure don't have such scenarios now (COLO without replication), as it's not allowed by far 7e934f5b27eee1b0d7 (by you and David). If you think we need such scenario, I think it should be a separate series which reverts 7e934f5b27eee1b0d7 and adds corresponding test and probably documentation.
> -----Original Message----- > From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > Sent: Friday, April 21, 2023 4:36 PM > To: Zhang, Chen <chen.zhang@intel.com>; qemu-devel@nongnu.org > Cc: 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; lizhijian@fujitsu.com > Subject: Re: [PATCH v2 3/4] build: move COLO under CONFIG_REPLICATION > > On 21.04.23 06:02, Zhang, Chen wrote: > > > > > >> -----Original Message----- > >> From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > >> Sent: Thursday, April 20, 2023 6:53 AM > >> To: qemu-devel@nongnu.org > >> Cc: 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; Vladimir Sementsov-Ogievskiy > >> <vsementsov@yandex- team.ru> > >> Subject: [PATCH v2 3/4] build: move COLO under CONFIG_REPLICATION > >> > >> 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. > > > > Yes, you are right for current status. Because COLO best practices is > replication + colo live migration + colo proxy. > > But doesn't mean it has to be done in all scenarios as I explanation in V1. > > The better way is allow to use x-colo capability firstly, and separate > > this patch with two config options: --disable-replication and --disable-x- > colo. > > > > But what for? We for sure don't have such scenarios now (COLO without > replication), as it's not allowed by far 7e934f5b27eee1b0d7 (by you and > David). > > If you think we need such scenario, I think it should be a separate series > which reverts 7e934f5b27eee1b0d7 and adds corresponding test and > probably documentation. In the patch 7e934f5b27eee1b0d7 said it's for current independent disk mode, And what we talked about before is the shared disk mode. Rethink about the COLO shared disk mode, this feature still needs some enabling works. It looks OK for now and separate the build options when enabling COLO shared disk mode. Thanks Chen > > > -- > Best regards, > Vladimir
On 23.04.23 04:54, Zhang, Chen wrote: > >> -----Original Message----- >> From: Vladimir Sementsov-Ogievskiy<vsementsov@yandex-team.ru> >> Sent: Friday, April 21, 2023 4:36 PM >> To: Zhang, Chen<chen.zhang@intel.com>;qemu-devel@nongnu.org >> Cc: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;lizhijian@fujitsu.com >> Subject: Re: [PATCH v2 3/4] build: move COLO under CONFIG_REPLICATION >> >> On 21.04.23 06:02, Zhang, Chen wrote: >>> >>>> -----Original Message----- >>>> From: Vladimir Sementsov-Ogievskiy<vsementsov@yandex-team.ru> >>>> Sent: Thursday, April 20, 2023 6:53 AM >>>> To:qemu-devel@nongnu.org >>>> Cc: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; Vladimir Sementsov-Ogievskiy >>>> <vsementsov@yandex- team.ru> >>>> Subject: [PATCH v2 3/4] build: move COLO under CONFIG_REPLICATION >>>> >>>> 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. >>> Yes, you are right for current status. Because COLO best practices is >> replication + colo live migration + colo proxy. >>> But doesn't mean it has to be done in all scenarios as I explanation in V1. >>> The better way is allow to use x-colo capability firstly, and separate >>> this patch with two config options: --disable-replication and --disable-x- >> colo. >> But what for? We for sure don't have such scenarios now (COLO without >> replication), as it's not allowed by far 7e934f5b27eee1b0d7 (by you and >> David). >> >> If you think we need such scenario, I think it should be a separate series >> which reverts 7e934f5b27eee1b0d7 and adds corresponding test and >> probably documentation. > In the patch 7e934f5b27eee1b0d7 said it's for current independent disk mode, > And what we talked about before is the shared disk mode. > Rethink about the COLO shared disk mode, this feature still needs some enabling works. > It looks OK for now and separate the build options when enabling COLO shared disk mode. I've started working on this, and now I see, that check in the migrate_caps_check() is not the only place. migration/colo.c has also several abort() points. For example, colo_process_checkpoint will simply abort if CONFIG_REPLICATION not defined. So for sure, current code is not prepared to use COLO with REPLICATION disabled. If this possibility is needed it requires more work. Personally, I don't think that possibility to enable COLO with disabled REPLICATION is really needed and I know nobody who need it, so that seems to be extra work.
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> wrote: > On 23.04.23 04:54, Zhang, Chen wrote: >> >>> -----Original Message----- >>> From: Vladimir Sementsov-Ogievskiy<vsementsov@yandex-team.ru> >>> Sent: Friday, April 21, 2023 4:36 PM >>> To: Zhang, Chen<chen.zhang@intel.com>;qemu-devel@nongnu.org >>> Cc: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;lizhijian@fujitsu.com >>> Subject: Re: [PATCH v2 3/4] build: move COLO under CONFIG_REPLICATION >>> >>> On 21.04.23 06:02, Zhang, Chen wrote: >>>> >>>>> -----Original Message----- >>>>> From: Vladimir Sementsov-Ogievskiy<vsementsov@yandex-team.ru> >>>>> Sent: Thursday, April 20, 2023 6:53 AM >>>>> To:qemu-devel@nongnu.org >>>>> Cc: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; Vladimir Sementsov-Ogievskiy >>>>> <vsementsov@yandex- team.ru> >>>>> Subject: [PATCH v2 3/4] build: move COLO under CONFIG_REPLICATION >>>>> >>>>> 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. >>>> Yes, you are right for current status. Because COLO best practices is >>> replication + colo live migration + colo proxy. >>>> But doesn't mean it has to be done in all scenarios as I explanation in V1. >>>> The better way is allow to use x-colo capability firstly, and separate >>>> this patch with two config options: --disable-replication and --disable-x- >>> colo. >>> But what for? We for sure don't have such scenarios now (COLO without >>> replication), as it's not allowed by far 7e934f5b27eee1b0d7 (by you and >>> David). >>> >>> If you think we need such scenario, I think it should be a separate series >>> which reverts 7e934f5b27eee1b0d7 and adds corresponding test and >>> probably documentation. >> In the patch 7e934f5b27eee1b0d7 said it's for current independent disk mode, >> And what we talked about before is the shared disk mode. >> Rethink about the COLO shared disk mode, this feature still needs some enabling works. >> It looks OK for now and separate the build options when enabling COLO shared disk mode. > > I've started working on this, and now I see, that check in the migrate_caps_check() is not the only place. > > migration/colo.c has also several abort() points. For example, > colo_process_checkpoint will simply abort if CONFIG_REPLICATION not > defined. > > So for sure, current code is not prepared to use COLO with REPLICATION disabled. > > If this possibility is needed it requires more work. Personally, I > don't think that possibility to enable COLO with disabled REPLICATION > is really needed and I know nobody who need it, so that seems to be > extra work. Whoever does the work to make COLO without REPLICATION work, it can also do the aditional work of splitting it. Changing a configure file is going to be the smaller of its problems. Later, Juan.
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 0716e64689..089c491d70 100644 --- a/migration/colo.c +++ b/migration/colo.c @@ -196,6 +196,12 @@ COLOMode get_colo_mode(void) } } +bool migrate_colo_enabled(void) +{ + MigrationState *s = migrate_get_current(); + return s->enabled_capabilities[MIGRATION_CAPABILITY_X_COLO]; +} + void colo_do_failover(void) { /* Make sure VM stopped while failover happened. */ diff --git a/migration/meson.build b/migration/meson.build index 0d1bb9f96e..3fccf79f12 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-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 72519ea99f..4601c48f41 100644 --- a/migration/migration-hmp-cmds.c +++ b/migration/migration-hmp-cmds.c @@ -640,6 +640,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; @@ -647,6 +648,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 bda4789193..2382958364 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -165,7 +165,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); @@ -1329,15 +1331,6 @@ static bool migrate_caps_check(bool *cap_list, } #endif -#ifndef CONFIG_REPLICATION - if (cap_list[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 (cap_list[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 @@ -3577,12 +3570,6 @@ fail: MIGRATION_STATUS_FAILED); } -bool migrate_colo_enabled(void) -{ - MigrationState *s = migrate_get_current(); - return s->enabled_capabilities[MIGRATION_CAPABILITY_X_COLO]; -} - typedef enum MigThrError { /* No error detected */ MIG_THR_ERR_NONE = 0, @@ -4537,7 +4524,9 @@ static 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), diff --git a/net/meson.build b/net/meson.build index 87afca3e93..38d50b8c96 100644 --- a/net/meson.build +++ b/net/meson.build @@ -1,7 +1,6 @@ softmmu_ss.add(files( 'announce.c', 'checksum.c', - 'colo-compare.c', 'colo.c', 'dump.c', 'eth.c', @@ -19,6 +18,10 @@ softmmu_ss.add(files( 'util.c', )) +if get_option('replication').allowed() + softmmu_ss.add(files('colo-compare.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..93863fa88c 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', @@ -1409,7 +1410,8 @@ # ## { 'command': 'x-colo-lost-heartbeat', - 'features': [ 'unstable' ] } + 'features': [ 'unstable' ], + 'if': 'CONFIG_REPLICATION' } ## # @migrate_cancel: @@ -1685,7 +1687,8 @@ ## { 'struct': 'COLOStatus', 'data': { 'mode': 'COLOMode', 'last-mode': 'COLOMode', - 'reason': 'COLOExitReason' } } + 'reason': 'COLOExitReason' }, + 'if': 'CONFIG_REPLICATION' } ## # @query-colo-status: @@ -1702,7 +1705,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..45c8ac0cc6 --- /dev/null +++ b/stubs/colo.c @@ -0,0 +1,47 @@ +#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_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; +} + +bool migrate_colo_enabled(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'))
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. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> --- hmp-commands.hx | 2 ++ migration/colo.c | 6 +++++ migration/meson.build | 6 +++-- migration/migration-hmp-cmds.c | 2 ++ migration/migration.c | 19 +++----------- net/meson.build | 5 +++- qapi/migration.json | 12 ++++++--- stubs/colo.c | 47 ++++++++++++++++++++++++++++++++++ stubs/meson.build | 1 + 9 files changed, 78 insertions(+), 22 deletions(-) create mode 100644 stubs/colo.c