From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:58668) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S0Iw4-0006vj-On for qemu-devel@nongnu.org; Wed, 22 Feb 2012 15:37:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S0Iw3-0007pp-83 for qemu-devel@nongnu.org; Wed, 22 Feb 2012 15:37:12 -0500 Received: from mx1.redhat.com ([209.132.183.28]:11989) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S0Iw3-0007pg-16 for qemu-devel@nongnu.org; Wed, 22 Feb 2012 15:37:11 -0500 Date: Wed, 22 Feb 2012 18:37:09 -0200 From: Luiz Capitulino Message-ID: <20120222183709.6d3fb14a@doriath.home> In-Reply-To: <4F455095.3010508@codemonkey.ws> References: <4F450170.5070806@codemonkey.ws> <4F4513FE.4070100@redhat.com> <4F45275E.4000304@codemonkey.ws> <20120222182526.58ac6c50@doriath.home> <4F455095.3010508@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/3] qapi: Allow QMP/QAPI commands to have array inputs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Kevin Wolf , jcody@redhat.com, qemu-devel@nongnu.org, Markus Armbruster On Wed, 22 Feb 2012 14:31:17 -0600 Anthony Liguori wrote: > On 02/22/2012 02:25 PM, Luiz Capitulino wrote: > > On Wed, 22 Feb 2012 11:35:26 -0600 > > Anthony Liguori wrote: > > > >> On 02/22/2012 10:12 AM, Jeff Cody wrote: > >>> On 02/22/2012 09:53 AM, Anthony Liguori wrote: > >>>> On 02/20/2012 11:31 AM, Jeff Cody wrote: > >>>>> The QAPI scripts allow for generating commands that receive parameter > >>>>> input consisting of a list of custom structs, but the QMP input paramter > >>>>> checking did not support receiving a qlist as opposed to a qdict for > >>>>> input. > >>>> > >>>> What are you trying to send on the wire? Something like > >>>> > >>>> {"execute": "foo", "arguments": [ "a", "b", "c" ]} > >>>> > >>>> ? > >>>> > >>>> That's not valid per the QMP protocol. "arguments" must always be a QMP > >>>> dictionary. > >>>> > >>> > >>> Not a bare list like that (perhaps my wording was poor), but rather an array of > >>> custom structs, that contain QMP dicts. In this particular case: > >>> > >>> {"execute": "blockdev-group-snapshot-sync", "arguments": [{ "device": > >>> "ide-hd0","snapshot-file":"/some/place/my-image","format": "qcow2" }]} > >>> > >>> When specifying this in the schema JSON file as shown below[1], the QAPI scripts > >>> generate the marshaller& visitor functions that handle everything correctly. > >>> The monitor code, however, could not deal with the QList container for the input > >>> parameters, prior to passing the data to the generated functions. > >>> > >>> > >>> [1] JSON schema definition: > >>> > >>> { 'type': 'SnapshotDev', > >>> 'data': {'device': 'str', 'snapshot-file': 'str', '*format': 'str' } } > >>> > >>> { 'command': 'blockdev-group-snapshot-sync', > >>> 'data': { 'dev': [ 'SnapshotDev' ] } } > >> > >> > >> This will end up looking like: > >> > >> { "execute": "blockdev-group-snapshot-sync", > >> "arguments": { "dev": [{"device": "ide-hd0"....}] } } > >> > >> Which should work as expected in QMP. HMP argument validation doesn't handle > >> arguments of lists but IMHO, it's time to kill that code. > >> > >> Luiz, what do you think? > > > > I don't think the code can just be dropped (if that's what you meant), as there > > are some features on top of it, like auto-completion, suffix support (G, M, k, etc), > > but it certainly can be improved. > > > > Also note that QMP does argument validation too, which is unfortunate enough to > > be based on HMP's, > > Once everything is converted to middle mode, this validation can be dropped for > QMP. The generated code should do thorough checking of arguments and can handle > complex types. That's cool, although we'll be switching to the new server too (hence the whole current server will be dropped).