Message ID | 20181208111606.8505-28-marcandre.lureau@redhat.com |
---|---|
State | New |
Headers | show |
Series | Hi, | expand |
Marc-André Lureau <marcandre.lureau@redhat.com> writes: > Add #if defined(CONFIG_REPLICATION) in generated code, and adjust the > code accordingly. > > Made 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 closer to killing qmp_unregister_commands_hack(). > > * enum BlockdevDriver value "replication" in command blockdev-add > > * BlockdevOptions variant @replication > > and related structures. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Reviewed-by: Markus Armbruster <armbru@redhat.com> > --- > qapi/block-core.json | 13 +++++++++---- > qapi/migration.json | 12 ++++++++---- > migration/colo.c | 16 ++++------------ > monitor.c | 5 ----- > 4 files changed, 21 insertions(+), 25 deletions(-) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index 92e0205d91..762000f31f 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -2623,7 +2623,9 @@ > 'copy-on-read', 'dmg', 'file', 'ftp', 'ftps', 'gluster', > 'host_cdrom', 'host_device', 'http', 'https', 'iscsi', 'luks', > 'nbd', 'nfs', 'null-aio', 'null-co', 'nvme', 'parallels', 'qcow', > - 'qcow2', 'qed', 'quorum', 'raw', 'rbd', 'replication', 'sheepdog', > + 'qcow2', 'qed', 'quorum', 'raw', 'rbd', > + { 'name': 'replication', 'if': 'defined(CONFIG_REPLICATION)' }, > + 'sheepdog', > 'ssh', 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] } > > ## > @@ -3380,7 +3382,8 @@ > # > # Since: 2.9 > ## > -{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] } > +{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ], > + 'if': 'defined(CONFIG_REPLICATION)' } > > ## > # @BlockdevOptionsReplication: > @@ -3398,7 +3401,8 @@ > { 'struct': 'BlockdevOptionsReplication', > 'base': 'BlockdevOptionsGenericFormat', > 'data': { 'mode': 'ReplicationMode', > - '*top-id': 'str' } } > + '*top-id': 'str' }, > + 'if': 'defined(CONFIG_REPLICATION)' } > > ## > # @NFSTransport: > @@ -3714,7 +3718,8 @@ > 'quorum': 'BlockdevOptionsQuorum', > 'raw': 'BlockdevOptionsRaw', > 'rbd': 'BlockdevOptionsRbd', > - 'replication':'BlockdevOptionsReplication', > + 'replication': { 'type': 'BlockdevOptionsReplication', > + 'if': 'defined(CONFIG_REPLICATION)' }, > 'sheepdog': 'BlockdevOptionsSheepdog', > 'ssh': 'BlockdevOptionsSsh', > 'throttle': 'BlockdevOptionsThrottle', > diff --git a/qapi/migration.json b/qapi/migration.json > index 5fd33316a0..31b589ec26 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -1257,7 +1257,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: > @@ -1272,7 +1273,8 @@ > # Since: 2.9 > ## > { 'struct': 'ReplicationStatus', > - 'data': { 'error': 'bool', '*desc': 'str' } } > + 'data': { 'error': 'bool', '*desc': 'str' }, > + 'if': 'defined(CONFIG_REPLICATION)' } > > ## > # @query-xen-replication-status: > @@ -1289,7 +1291,8 @@ > # Since: 2.9 > ## > { 'command': 'query-xen-replication-status', > - 'returns': 'ReplicationStatus' } > + 'returns': 'ReplicationStatus', > + 'if': 'defined(CONFIG_REPLICATION)' } > > ## > # @xen-colo-do-checkpoint: > @@ -1305,7 +1308,8 @@ > # > # Since: 2.9 > ## > -{ 'command': 'xen-colo-do-checkpoint' } > +{ 'command': 'xen-colo-do-checkpoint', > + 'if': 'defined(CONFIG_REPLICATION)' } > > ## > # @COLOStatus: > diff --git a/migration/colo.c b/migration/colo.c > index fcff04c78c..398b239d1c 100644 > --- a/migration/colo.c > +++ b/migration/colo.c > @@ -24,7 +24,9 @@ > #include "trace.h" > #include "qemu/error-report.h" > #include "migration/failover.h" > +#ifdef CONFIG_REPLICATION > #include "replication.h" > +#endif > #include "net/colo-compare.h" > #include "net/colo.h" > #include "block/block.h" > @@ -201,11 +203,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; > @@ -224,14 +226,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); > > @@ -246,19 +244,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 /* CONFIG_REPLICATION */ after the endif wouldn't hurt. > > COLOStatus *qmp_query_colo_status(Error **errp) > { > diff --git a/monitor.c b/monitor.c > index d39390c2f2..16a180dd50 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -1146,11 +1146,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"); > qmp_unregister_command(&qmp_commands, "query-sev"); Only TARGET conditionals left here. Good. Reviewed-by: Markus Armbruster <armbru@redhat.com>
Hi On Tue, Dec 11, 2018 at 9:15 PM Markus Armbruster <armbru@redhat.com> wrote: > > Marc-André Lureau <marcandre.lureau@redhat.com> writes: > > > Add #if defined(CONFIG_REPLICATION) in generated code, and adjust the > > code accordingly. > > > > Made 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 closer to killing qmp_unregister_commands_hack(). > > > > * enum BlockdevDriver value "replication" in command blockdev-add > > > > * BlockdevOptions variant @replication > > > > and related structures. > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > Reviewed-by: Markus Armbruster <armbru@redhat.com> > > --- > > qapi/block-core.json | 13 +++++++++---- > > qapi/migration.json | 12 ++++++++---- > > migration/colo.c | 16 ++++------------ > > monitor.c | 5 ----- > > 4 files changed, 21 insertions(+), 25 deletions(-) > > > > diff --git a/qapi/block-core.json b/qapi/block-core.json > > index 92e0205d91..762000f31f 100644 > > --- a/qapi/block-core.json > > +++ b/qapi/block-core.json > > @@ -2623,7 +2623,9 @@ > > 'copy-on-read', 'dmg', 'file', 'ftp', 'ftps', 'gluster', > > 'host_cdrom', 'host_device', 'http', 'https', 'iscsi', 'luks', > > 'nbd', 'nfs', 'null-aio', 'null-co', 'nvme', 'parallels', 'qcow', > > - 'qcow2', 'qed', 'quorum', 'raw', 'rbd', 'replication', 'sheepdog', > > + 'qcow2', 'qed', 'quorum', 'raw', 'rbd', > > + { 'name': 'replication', 'if': 'defined(CONFIG_REPLICATION)' }, > > + 'sheepdog', > > 'ssh', 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] } > > > > ## > > @@ -3380,7 +3382,8 @@ > > # > > # Since: 2.9 > > ## > > -{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] } > > +{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ], > > + 'if': 'defined(CONFIG_REPLICATION)' } > > > > ## > > # @BlockdevOptionsReplication: > > @@ -3398,7 +3401,8 @@ > > { 'struct': 'BlockdevOptionsReplication', > > 'base': 'BlockdevOptionsGenericFormat', > > 'data': { 'mode': 'ReplicationMode', > > - '*top-id': 'str' } } > > + '*top-id': 'str' }, > > + 'if': 'defined(CONFIG_REPLICATION)' } > > > > ## > > # @NFSTransport: > > @@ -3714,7 +3718,8 @@ > > 'quorum': 'BlockdevOptionsQuorum', > > 'raw': 'BlockdevOptionsRaw', > > 'rbd': 'BlockdevOptionsRbd', > > - 'replication':'BlockdevOptionsReplication', > > + 'replication': { 'type': 'BlockdevOptionsReplication', > > + 'if': 'defined(CONFIG_REPLICATION)' }, > > 'sheepdog': 'BlockdevOptionsSheepdog', > > 'ssh': 'BlockdevOptionsSsh', > > 'throttle': 'BlockdevOptionsThrottle', > > diff --git a/qapi/migration.json b/qapi/migration.json > > index 5fd33316a0..31b589ec26 100644 > > --- a/qapi/migration.json > > +++ b/qapi/migration.json > > @@ -1257,7 +1257,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: > > @@ -1272,7 +1273,8 @@ > > # Since: 2.9 > > ## > > { 'struct': 'ReplicationStatus', > > - 'data': { 'error': 'bool', '*desc': 'str' } } > > + 'data': { 'error': 'bool', '*desc': 'str' }, > > + 'if': 'defined(CONFIG_REPLICATION)' } > > > > ## > > # @query-xen-replication-status: > > @@ -1289,7 +1291,8 @@ > > # Since: 2.9 > > ## > > { 'command': 'query-xen-replication-status', > > - 'returns': 'ReplicationStatus' } > > + 'returns': 'ReplicationStatus', > > + 'if': 'defined(CONFIG_REPLICATION)' } > > > > ## > > # @xen-colo-do-checkpoint: > > @@ -1305,7 +1308,8 @@ > > # > > # Since: 2.9 > > ## > > -{ 'command': 'xen-colo-do-checkpoint' } > > +{ 'command': 'xen-colo-do-checkpoint', > > + 'if': 'defined(CONFIG_REPLICATION)' } > > > > ## > > # @COLOStatus: > > diff --git a/migration/colo.c b/migration/colo.c > > index fcff04c78c..398b239d1c 100644 > > --- a/migration/colo.c > > +++ b/migration/colo.c > > @@ -24,7 +24,9 @@ > > #include "trace.h" > > #include "qemu/error-report.h" > > #include "migration/failover.h" > > +#ifdef CONFIG_REPLICATION > > #include "replication.h" > > +#endif > > #include "net/colo-compare.h" > > #include "net/colo.h" > > #include "block/block.h" > > @@ -201,11 +203,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; > > @@ -224,14 +226,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); > > > > @@ -246,19 +244,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 > > /* CONFIG_REPLICATION */ after the endif wouldn't hurt. > > > > > COLOStatus *qmp_query_colo_status(Error **errp) > > { > > diff --git a/monitor.c b/monitor.c > > index d39390c2f2..16a180dd50 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -1146,11 +1146,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"); > > qmp_unregister_command(&qmp_commands, "query-sev"); > > Only TARGET conditionals left here. Good. IIRC, they are removed in part 3. > > Reviewed-by: Markus Armbruster <armbru@redhat.com>
diff --git a/qapi/block-core.json b/qapi/block-core.json index 92e0205d91..762000f31f 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2623,7 +2623,9 @@ 'copy-on-read', 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom', 'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs', 'null-aio', 'null-co', 'nvme', 'parallels', 'qcow', - 'qcow2', 'qed', 'quorum', 'raw', 'rbd', 'replication', 'sheepdog', + 'qcow2', 'qed', 'quorum', 'raw', 'rbd', + { 'name': 'replication', 'if': 'defined(CONFIG_REPLICATION)' }, + 'sheepdog', 'ssh', 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] } ## @@ -3380,7 +3382,8 @@ # # Since: 2.9 ## -{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] } +{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ], + 'if': 'defined(CONFIG_REPLICATION)' } ## # @BlockdevOptionsReplication: @@ -3398,7 +3401,8 @@ { 'struct': 'BlockdevOptionsReplication', 'base': 'BlockdevOptionsGenericFormat', 'data': { 'mode': 'ReplicationMode', - '*top-id': 'str' } } + '*top-id': 'str' }, + 'if': 'defined(CONFIG_REPLICATION)' } ## # @NFSTransport: @@ -3714,7 +3718,8 @@ 'quorum': 'BlockdevOptionsQuorum', 'raw': 'BlockdevOptionsRaw', 'rbd': 'BlockdevOptionsRbd', - 'replication':'BlockdevOptionsReplication', + 'replication': { 'type': 'BlockdevOptionsReplication', + 'if': 'defined(CONFIG_REPLICATION)' }, 'sheepdog': 'BlockdevOptionsSheepdog', 'ssh': 'BlockdevOptionsSsh', 'throttle': 'BlockdevOptionsThrottle', diff --git a/qapi/migration.json b/qapi/migration.json index 5fd33316a0..31b589ec26 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -1257,7 +1257,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: @@ -1272,7 +1273,8 @@ # Since: 2.9 ## { 'struct': 'ReplicationStatus', - 'data': { 'error': 'bool', '*desc': 'str' } } + 'data': { 'error': 'bool', '*desc': 'str' }, + 'if': 'defined(CONFIG_REPLICATION)' } ## # @query-xen-replication-status: @@ -1289,7 +1291,8 @@ # Since: 2.9 ## { 'command': 'query-xen-replication-status', - 'returns': 'ReplicationStatus' } + 'returns': 'ReplicationStatus', + 'if': 'defined(CONFIG_REPLICATION)' } ## # @xen-colo-do-checkpoint: @@ -1305,7 +1308,8 @@ # # Since: 2.9 ## -{ 'command': 'xen-colo-do-checkpoint' } +{ 'command': 'xen-colo-do-checkpoint', + 'if': 'defined(CONFIG_REPLICATION)' } ## # @COLOStatus: diff --git a/migration/colo.c b/migration/colo.c index fcff04c78c..398b239d1c 100644 --- a/migration/colo.c +++ b/migration/colo.c @@ -24,7 +24,9 @@ #include "trace.h" #include "qemu/error-report.h" #include "migration/failover.h" +#ifdef CONFIG_REPLICATION #include "replication.h" +#endif #include "net/colo-compare.h" #include "net/colo.h" #include "block/block.h" @@ -201,11 +203,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; @@ -224,14 +226,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); @@ -246,19 +244,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 COLOStatus *qmp_query_colo_status(Error **errp) { diff --git a/monitor.c b/monitor.c index d39390c2f2..16a180dd50 100644 --- a/monitor.c +++ b/monitor.c @@ -1146,11 +1146,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"); qmp_unregister_command(&qmp_commands, "query-sev");