Marc-André Lureau <marcandre.lur...@redhat.com> writes:
> Add #if defined(CONFIG_REPLICATION) in generated code, and adjust the
> code accordingly.
>
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> ---
> qapi-schema.json | 12 ++++++++----
> migration/colo.c | 14 ++------------
> monitor.c | 5 -----
> 3 files changed, 10 insertions(+), 21 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index bcee3157b0..2f4528c769 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -6337,7 +6337,8 @@
> # Since: 2.9
> ##
> { 'command': 'xen-set-replication',
> - 'data': { 'enable': 'bool', 'primary': 'bool', '*failover' : 'bool' } }
> + 'data': { 'enable': 'bool', 'primary': 'bool', '*failover' : 'bool' },
> + 'if': 'defined(CONFIG_REPLICATION)' }
>
> ##
> # @ReplicationStatus:
> @@ -6352,7 +6353,8 @@
> # Since: 2.9
> ##
> { 'struct': 'ReplicationStatus',
> - 'data': { 'error': 'bool', '*desc': 'str' } }
> + 'data': { 'error': 'bool', '*desc': 'str' },
> + 'if': 'defined(CONFIG_REPLICATION)' }
>
> ##
> # @query-xen-replication-status:
> @@ -6369,7 +6371,8 @@
> # Since: 2.9
> ##
> { 'command': 'query-xen-replication-status',
> - 'returns': 'ReplicationStatus' }
> + 'returns': 'ReplicationStatus',
> + 'if': 'defined(CONFIG_REPLICATION)' }
>
> ##
> # @xen-colo-do-checkpoint:
> @@ -6385,7 +6388,8 @@
> #
> # Since: 2.9
> ##
> -{ 'command': 'xen-colo-do-checkpoint' }
> +{ 'command': 'xen-colo-do-checkpoint',
> + 'if': 'defined(CONFIG_REPLICATION)' }
>
> ##
> # @GICCapability:
> diff --git a/migration/colo.c b/migration/colo.c
> index a4255432ac..3bff9fc9a4 100644
> --- a/migration/colo.c
> +++ b/migration/colo.c
> @@ -147,11 +147,11 @@ void colo_do_failover(MigrationState *s)
> }
> }
>
> +#ifdef CONFIG_REPLICATION
> void qmp_xen_set_replication(bool enable, bool primary,
> bool has_failover, bool failover,
> Error **errp)
> {
> -#ifdef CONFIG_REPLICATION
> ReplicationMode mode = primary ?
> REPLICATION_MODE_PRIMARY :
> REPLICATION_MODE_SECONDARY;
> @@ -170,14 +170,10 @@ void qmp_xen_set_replication(bool enable, bool primary,
> }
> replication_stop_all(failover, failover ? NULL : errp);
> }
> -#else
> - abort();
> -#endif
> }
>
> ReplicationStatus *qmp_query_xen_replication_status(Error **errp)
> {
> -#ifdef CONFIG_REPLICATION
> Error *err = NULL;
> ReplicationStatus *s = g_new0(ReplicationStatus, 1);
>
> @@ -192,19 +188,13 @@ ReplicationStatus
> *qmp_query_xen_replication_status(Error **errp)
>
> error_free(err);
> return s;
> -#else
> - abort();
> -#endif
> }
>
> void qmp_xen_colo_do_checkpoint(Error **errp)
> {
> -#ifdef CONFIG_REPLICATION
> replication_do_checkpoint_all(errp);
> -#else
> - abort();
> -#endif
> }
> +#endif
>
> static void colo_send_message(QEMUFile *f, COLOMessage msg,
> Error **errp)
> diff --git a/monitor.c b/monitor.c
> index 4bf6a3ea2e..383c84d162 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -970,11 +970,6 @@ static void qmp_query_qmp_schema(QDict *qdict, QObject
> **ret_data,
> */
> static void qmp_unregister_commands_hack(void)
> {
> -#ifndef CONFIG_REPLICATION
> - qmp_unregister_command(&qmp_commands, "xen-set-replication");
> - qmp_unregister_command(&qmp_commands, "query-xen-replication-status");
> - qmp_unregister_command(&qmp_commands, "xen-colo-do-checkpoint");
> -#endif
> #ifndef TARGET_I386
> qmp_unregister_command(&qmp_commands, "rtc-reset-reinjection");
> #endif
Same drill as for PATCH 16.
Commands you make conditional:
* xen-set-replication, query-xen-replication-status,
xen-colo-do-checkpoint
Before the patch, we first register the commands unconditionally in
generated code (requires a stub), then conditionally unregister in
qmp_unregister_commands_hack().
Afterwards, we register only when CONFIG_REPLICATION. The command
fails exactly the same, with CommandNotFound.
Improvement, because now query-qmp-schema is accurate, and we're one
step close to killing qmp_unregister_commands_hack().
Check for completeness by reviewing the case insensitive occurrences of
replication in the schema not covered by your patch, and review uses of
CONFIG_REPLICATION for possible schema connections (I did that for
CONFIG_VNC [PATCH 16] and CONFIG_SPICE [PATCH 17], too):
* enum BlockdevDriver value "replication" in command blockdev-add
* Makefile.objs:block-obj-$(CONFIG_REPLICATION) += replication.o
block/Makefile.objs:block-obj-$(CONFIG_REPLICATION) += replication.o
I think the following should be ifdef CONFIG_REPLICATION: enum
BlockdevDriver value @replication BlockdevOptions variant @replication,
BlockdevOptionsReplication, BlockdevOptionsReplicationMode.
Some of this analysis should perhaps be worked into the commit message.
