From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42431) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VlhZK-0006a5-Hh for qemu-devel@nongnu.org; Wed, 27 Nov 2013 11:02:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VlhZE-0005Rk-J0 for qemu-devel@nongnu.org; Wed, 27 Nov 2013 11:02:26 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51851) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VlhZE-0005Re-9h for qemu-devel@nongnu.org; Wed, 27 Nov 2013 11:02:20 -0500 Message-ID: <52960D10.8010506@redhat.com> Date: Wed, 27 Nov 2013 16:17:36 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1385001528-12003-1-git-send-email-imammedo@redhat.com> <1385001528-12003-6-git-send-email-imammedo@redhat.com> <87r4aaxdqt.fsf@blackfin.pond.sub.org> <20131125163642.4d832fd5@nial.usersys.redhat.com> <20131125160337.GB10326@redhat.com> <52937BA2.6020605@redhat.com> <52937E44.10503@redhat.com> <874n6y7wyc.fsf@blackfin.pond.sub.org> In-Reply-To: <874n6y7wyc.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 05/27] qapi: add SIZE type parser to string_input_visitor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: peter.maydell@linaro.org, aliguori@amazon.com, quintela@redhat.com, hutao@cn.fujitsu.com, "Michael S. Tsirkin" , mjt@tls.msk.ru, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, vasilis.liaskovitis@profitbricks.com, chegu_vinod@hp.com, kraxel@redhat.com, stefanha@redhat.com, Igor Mammedov , stefanb@linux.vnet.ibm.com, marcel.a@redhat.com, lcapitulino@redhat.com, afaerber@suse.de Il 27/11/2013 15:15, Markus Armbruster ha scritto: >>> >> This is unfortunately a counter-example to the rule that HMP commands >>> >> should always be implemented in terms of their QMP counterparts. I do >>> >> not believe this is really a problem. It can be fixed later; for now, I >>> >> think "perfect is the enemy of good" applies. > I don't get why there's a counter-example. Take as an example the current implementation of netdev_add. void hmp_netdev_add(Monitor *mon, const QDict *qdict) { ... QemuOpts *opts = qemu_opts_from_qdict(qemu_find_opts("netdev"), qdict, &err); netdev_add(opts, &err); ... } int qmp_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret) { opts_list = qemu_find_opts_err("netdev", &local_err); opts = qemu_opts_from_qdict(opts_list, qdict, &local_err); netdev_add(opts, &local_err); ... return 0; exit_err: ... } These obey the rules you have below (there's just the weirdness that the QMP command handler is not autogenerated). But as you see above, this is really the same code for both handlers, they only differ in error handling minutiae. This is possible with netdev_add (and you could do the same for device_add) because the QMP interface sucks and it is not well defined unless you make all values strings (qemu_opts_from_dict tries to do something meaningful for integers and floats, but given the existence of qdev property types such as hex32 I wouldn't trust it too much). If we want a really different interface between HMP and QMP, one human-oriented and the other JSON-oriented, I'm not sure that you can share much code between the two implementation. Paolo > The rules are > > 1. A QMP command handler (which needs to be of type int (*)(Monitor *, > const QDict *params, QObject **ret_data)) should be a thin wrapper > around a function with a type appropriate for C that contains all the > logic. The wrapper merely translates. > > 2. HMP commands are to be built on top of these functions, not their > handler wrappers.