[{"id":1821524,"web_url":"http://patchwork.ozlabs.org/comment/1821524/","msgid":"<87a7ymaexj.fsf@dusky.pond.sub.org>","list_archive_url":null,"date":"2017-12-13T14:19:20","subject":"Re: [Qemu-devel] [PATCH v3 38/50] qapi: add conditions to\n\tREPLICATION type/commands on the schema","submitter":{"id":2645,"url":"http://patchwork.ozlabs.org/api/people/2645/","name":"Markus Armbruster","email":"armbru@redhat.com"},"content":"Marc-André Lureau writes:\n\n> Add #if defined(CONFIG_REPLICATION) in generated code, and adjust the\n> code accordingly.\n>\n> Made conditional:\n>\n> * xen-set-replication, query-xen-replication-status,\n> xen-colo-do-checkpoint\n>\n> Before the patch, we first register the commands unconditionally in\n> generated code (requires a stub), then conditionally unregister in\n> qmp_unregister_commands_hack().\n>\n> Afterwards, we register only when CONFIG_REPLICATION. The command\n> fails exactly the same, with CommandNotFound.\n>\n> Improvement, because now query-qmp-schema is accurate, and we're one\n> step close to killing qmp_unregister_commands_hack().\n\ns/close/closer/\n\n>\n> * enum BlockdevDriver value \"replication\" in command blockdev-add\n>\n> As well as enum BlockdevDriver value @replication BlockdevOptions\n\nMissing a comma after @replication?\n\n> variant @replication, BlockdevOptionsReplication,\n> BlockdevOptionsReplicationMode.\n>\n> Signed-off-by: Marc-André Lureau \n> ---\n> qapi/block-core.json | 15 ++++++++++-----\n> qapi/migration.json | 12 ++++++++----\n> migration/colo.c | 16 ++++------------\n> monitor.c | 5 -----\n> 4 files changed, 22 insertions(+), 26 deletions(-)\n>\n> diff --git a/qapi/block-core.json b/qapi/block-core.json\n> index bb11815608..6f897cf157 100644\n> --- a/qapi/block-core.json\n> +++ b/qapi/block-core.json\n> @@ -2232,8 +2232,10 @@\n> 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',\n> 'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs',\n> 'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed',\n> - 'quorum', 'raw', 'rbd', 'replication', 'sheepdog', 'ssh',\n> - 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] }\n> + 'quorum', 'raw', 'rbd',\n> + { 'name': 'replication', 'if': 'defined(CONFIG_REPLICATION)' },\n> + 'sheepdog', 'ssh', 'throttle',\n> + 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] }\n> \n> ##\n> # @BlockdevOptionsFile:\n> @@ -2855,7 +2857,8 @@\n> #\n> # Since: 2.9\n> ##\n> -{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ] }\n> +{ 'enum' : 'ReplicationMode', 'data' : [ 'primary', 'secondary' ],\n> + 'if': 'defined(CONFIG_REPLICATION)' }\n> \n> ##\n> # @BlockdevOptionsReplication:\n> @@ -2873,7 +2876,8 @@\n> { 'struct': 'BlockdevOptionsReplication',\n> 'base': 'BlockdevOptionsGenericFormat',\n> 'data': { 'mode': 'ReplicationMode',\n> - '*top-id': 'str' } }\n> + '*top-id': 'str' },\n> + 'if': 'defined(CONFIG_REPLICATION)' }\n> \n> ##\n> # @NFSTransport:\n> @@ -3168,7 +3172,8 @@\n> 'quorum': 'BlockdevOptionsQuorum',\n> 'raw': 'BlockdevOptionsRaw',\n> 'rbd': 'BlockdevOptionsRbd',\n> - 'replication':'BlockdevOptionsReplication',\n> + 'replication': { 'type': 'BlockdevOptionsReplication',\n> + 'if': 'defined(CONFIG_REPLICATION)' },\n> 'sheepdog': 'BlockdevOptionsSheepdog',\n> 'ssh': 'BlockdevOptionsSsh',\n> 'throttle': 'BlockdevOptionsThrottle',\n> diff --git a/qapi/migration.json b/qapi/migration.json\n> index ee2b3b8733..44d2ef9c94 100644\n> --- a/qapi/migration.json\n> +++ b/qapi/migration.json\n> @@ -1034,7 +1034,8 @@\n> # Since: 2.9\n> ##\n> { 'command': 'xen-set-replication',\n> - 'data': { 'enable': 'bool', 'primary': 'bool', '*failover' : 'bool' } }\n> + 'data': { 'enable': 'bool', 'primary': 'bool', '*failover' : 'bool' },\n> + 'if': 'defined(CONFIG_REPLICATION)' }\n> \n> ##\n> # @ReplicationStatus:\n> @@ -1049,7 +1050,8 @@\n> # Since: 2.9\n> ##\n> { 'struct': 'ReplicationStatus',\n> - 'data': { 'error': 'bool', '*desc': 'str' } }\n> + 'data': { 'error': 'bool', '*desc': 'str' },\n> + 'if': 'defined(CONFIG_REPLICATION)' }\n> \n> ##\n> # @query-xen-replication-status:\n> @@ -1066,7 +1068,8 @@\n> # Since: 2.9\n> ##\n> { 'command': 'query-xen-replication-status',\n> - 'returns': 'ReplicationStatus' }\n> + 'returns': 'ReplicationStatus',\n> + 'if': 'defined(CONFIG_REPLICATION)' }\n> \n> ##\n> # @xen-colo-do-checkpoint:\n> @@ -1082,4 +1085,5 @@\n> #\n> # Since: 2.9\n> ##\n> -{ 'command': 'xen-colo-do-checkpoint' }\n> +{ 'command': 'xen-colo-do-checkpoint',\n> + 'if': 'defined(CONFIG_REPLICATION)' }\n> diff --git a/migration/colo.c b/migration/colo.c\n> index dee3aa8bf7..010c9e02e8 100644\n> --- a/migration/colo.c\n> +++ b/migration/colo.c\n> @@ -22,7 +22,9 @@\n> #include \"trace.h\"\n> #include \"qemu/error-report.h\"\n> #include \"migration/failover.h\"\n> +#ifdef CONFIG_REPLICATION\n> #include \"replication.h\"\n> +#endif\n> #include \"qmp-commands.h\"\n> \n> static bool vmstate_loading;\n> @@ -147,11 +149,11 @@ void colo_do_failover(MigrationState *s)\n> }\n> }\n> \n> +#ifdef CONFIG_REPLICATION\n> void qmp_xen_set_replication(bool enable, bool primary,\n> bool has_failover, bool failover,\n> Error **errp)\n> {\n> -#ifdef CONFIG_REPLICATION\n> ReplicationMode mode = primary ?\n> REPLICATION_MODE_PRIMARY :\n> REPLICATION_MODE_SECONDARY;\n> @@ -170,14 +172,10 @@ void qmp_xen_set_replication(bool enable, bool primary,\n> }\n> replication_stop_all(failover, failover ? NULL : errp);\n> }\n> -#else\n> - abort();\n> -#endif\n> }\n> \n> ReplicationStatus *qmp_query_xen_replication_status(Error **errp)\n> {\n> -#ifdef CONFIG_REPLICATION\n> Error *err = NULL;\n> ReplicationStatus *s = g_new0(ReplicationStatus, 1);\n> \n> @@ -192,19 +190,13 @@ ReplicationStatus *qmp_query_xen_replication_status(Error **errp)\n> \n> error_free(err);\n> return s;\n> -#else\n> - abort();\n> -#endif\n> }\n> \n> void qmp_xen_colo_do_checkpoint(Error **errp)\n> {\n> -#ifdef CONFIG_REPLICATION\n> replication_do_checkpoint_all(errp);\n> -#else\n> - abort();\n> -#endif\n> }\n> +#endif\n> \n> static void colo_send_message(QEMUFile *f, COLOMessage msg,\n> Error **errp)\n> diff --git a/monitor.c b/monitor.c\n> index 135a1e0821..b5ddcf8c67 100644\n> --- a/monitor.c\n> +++ b/monitor.c\n> @@ -970,11 +970,6 @@ static void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,\n> */\n> static void qmp_unregister_commands_hack(void)\n> {\n> -#ifndef CONFIG_REPLICATION\n> - qmp_unregister_command(&qmp_commands, \"xen-set-replication\");\n> - qmp_unregister_command(&qmp_commands, \"query-xen-replication-status\");\n> - qmp_unregister_command(&qmp_commands, \"xen-colo-do-checkpoint\");\n> -#endif\n> #ifndef TARGET_I386\n> qmp_unregister_command(&qmp_commands, \"rtc-reset-reinjection\");\n> #endif","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yxf3F22gxz9sBZ\n\tfor ;\n\tThu, 14 Dec 2017 01:19:53 +1100 (AEDT)","from localhost ([::1]:35757 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1eP7t1-0002gW-Dp\n\tfor incoming@patchwork.ozlabs.org; Wed, 13 Dec 2017 09:19:51 -0500","from eggs.gnu.org ([2001:4830:134:3::10]:53043)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1eP7sf-0002gR-NY\n\tfor qemu-devel@nongnu.org; Wed, 13 Dec 2017 09:19:30 -0500","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1eP7sc-00051U-EK\n\tfor qemu-devel@nongnu.org; Wed, 13 Dec 2017 09:19:29 -0500","from mx1.redhat.com ([209.132.183.28]:46896)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1eP7sc-0004zn-5X\n\tfor qemu-devel@nongnu.org; Wed, 13 Dec 2017 09:19:26 -0500","from smtp.corp.redhat.com\n\t(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 6C78B5F7BF;\n\tWed, 13 Dec 2017 14:19:25 +0000 (UTC)","from blackfin.pond.sub.org (ovpn-116-74.ams2.redhat.com\n\t[10.36.116.74])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id DF7E860172;\n\tWed, 13 Dec 2017 14:19:21 +0000 (UTC)","by blackfin.pond.sub.org (Postfix, from userid 1000)\n\tid A838B1138658; Wed, 13 Dec 2017 15:19:20 +0100 (CET)"],"From":"Markus Armbruster ","To":"=?utf-8?q?Marc-Andr=C3=A9?= Lureau ","References":"<20170911110623.24981-1-marcandre.lureau@redhat.com>\n\t<20170911110623.24981-39-marcandre.lureau@redhat.com>","Date":"Wed, 13 Dec 2017 15:19:20 +0100","In-Reply-To":"<20170911110623.24981-39-marcandre.lureau@redhat.com> (\n\t=?utf-8?b?Ik1hcmMtQW5kcsOp?= Lureau\"'s message of \"Mon,\n\t11 Sep 2017 \t13:06:11 +0200\")","Message-ID":"<87a7ymaexj.fsf@dusky.pond.sub.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux)","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.11","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.39]);\n\tWed, 13 Dec 2017 14:19:25 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v3 38/50] qapi: add conditions to\n\tREPLICATION type/commands on the schema","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"Juan Quintela , qemu-devel@nongnu.org,\n\t\"Dr. David Alan Gilbert\" ,\n\tzhanghailiang ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]