From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52863) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vtfen-0004ED-OB for qemu-devel@nongnu.org; Thu, 19 Dec 2013 10:37:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Vtfei-0008GX-O9 for qemu-devel@nongnu.org; Thu, 19 Dec 2013 10:37:01 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55015) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Vtfei-0008GO-DH for qemu-devel@nongnu.org; Thu, 19 Dec 2013 10:36:56 -0500 Date: Thu, 19 Dec 2013 17:40:36 +0200 From: "Michael S. Tsirkin" Message-ID: <20131219154036.GA2717@redhat.com> 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> <20131219144037.GB17508@redhat.com> <52B3115D.50907@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <52B3115D.50907@redhat.com> 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: Paolo Bonzini Cc: peter.maydell@linaro.org, aliguori@amazon.com, stefanb@linux.vnet.ibm.com, hutao@cn.fujitsu.com, quintela@redhat.com, mjt@tls.msk.ru, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, vasilis.liaskovitis@profitbricks.com, kraxel@redhat.com, stefanha@redhat.com, Igor Mammedov , marcel.a@redhat.com, lcapitulino@redhat.com, chegu_vinod@hp.com, afaerber@suse.de, Markus Armbruster On Thu, Dec 19, 2013 at 04:31:41PM +0100, Paolo Bonzini wrote: > Il 19/12/2013 15:40, Michael S. Tsirkin ha scritto: > > On Mon, Nov 25, 2013 at 09:43:48AM -0700, Eric Blake wrote: > >> On 11/25/2013 09:32 AM, Paolo Bonzini wrote: > >> > >>>> Yes please. Firing up a calculator to figure out how much is 1G is not > >>>> friendly, neither is firing it up to figure out what did management do > >>>> with QMP. It should be a text based interface not a binary one. > >> > >> Right now, QMP takes an 'int', which does not allow a suffix. Libvirt > >> prefers passing a value in 'bytes', rather than risking confusion on > >> whether a value in G was rounded (up, down? to nearest power of 10 or > >> power of 2?). Unfortunately, yes, that means you need a calculator when > >> parsing QMP logs to see whether the 1073741824 passed by libvirt is the > >> 1G you had in mind. > >> > >> HMP, qtest, and any other decent shell around raw QMP is more than > >> welcome to provide human-usable wrappers that takes "1G" as a string and > >> turns it into the raw int used by the underlying QMP. In fact, I > >> encourage it. > > > > How will it know 1G is not e.g. an ID? > > Because all properties are associated to a name. In the case of a > memory device, the name could be "size" or "id". When requesting the > id, QEMU will use visit_type_str. When requesting the size, QEMU will > use visit_type_size. Different functions are called, and the different > functions accept different input. QEMU can do this, but if you want a frontend with QMP as backend, it won't. So you end up re-implementing this logic in all frontends: instead of just passing on properties they need to know where 1G is a number and where it's a name. > To explain further what Markus dismissed as "against the design of QMP", > there is another way to choose the way your input is parsed, and that is > simply by choosing a different visitor. > > QMP uses strongly-typed JSON, so it uses a QmpInputVisitor. HMP and > command-line just parse text, so it uses an OptsVisitor. > > So we have: > > -object memory-ram,size=1M,id=foo (command-line) > object_add memory-ram,size=1M,id=foo (HMP) > > that use OptsVisitor and converts 1M to 1048576. For QMP instead > > {'command': 'object-add', 'arguments': { > 'qom-type': 'memory-ram', 'id': 'foo', > 'props': {'size':1048576}}} > > uses QmpInputVisitor which rejects any 'size' that is not a JSON > integer. We definitely do not want to allow QMP input such as this: > > {'command': 'object-add', 'arguments': { > 'qom-type': 'memory-ram', 'id': 'foo', > 'props': {'size':'1M'}}} > > That's against the idea of making QMP as strongly-typed as possible. > > Paolo > > > We can invent rules like "IDs should not start with a number", but these > > rules are better enforced in a single place for consistency, and it's > > likely too late to enforce that in HMP. > > > >>> 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. > >> > >> Hey - I just realized that now that we have anonymous unions, we could > >> theoretically extend QMP to allow a union between 'int' and 'string' - > >> if an 'int' is passed, it is in bytes; if a 'string' is passed, then > >> parse it the way HMP would (so the string "1G" would be equivalent to > >> the raw int 1073741824). But I don't know if it will help you (libvirt > >> will still prefer to use raw ints in any QMP log you read off of libvirt > >> interactions). > > > > Yes, I think that would address the issue. > > > >> Eric Blake eblake redhat com +1-919-301-3266 > >> Libvirt virtualization library http://libvirt.org > >> > > > > >