From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42199) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YH7l5-0002on-R5 for qemu-devel@nongnu.org; Fri, 30 Jan 2015 04:21:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YH7l2-0006d6-Hd for qemu-devel@nongnu.org; Fri, 30 Jan 2015 04:20:59 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33481) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YH7l2-0006d2-9f for qemu-devel@nongnu.org; Fri, 30 Jan 2015 04:20:56 -0500 Date: Fri, 30 Jan 2015 09:20:50 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20150130092050.GA2370@work-vm> References: <1422543997-22808-1-git-send-email-dgilbert@redhat.com> <1422543997-22808-2-git-send-email-dgilbert@redhat.com> <20150129151527.GE1102@redhat.com> <54CA500E.8030608@redhat.com> <20150129155425.GE2391@work-vm> <54CA594E.5030605@redhat.com> <20150129162815.GG2391@work-vm> <54CA71CD.3090304@redhat.com> <20150129202154.GH2391@work-vm> <87a910wtsn.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87a910wtsn.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [RFC 1/1] Execute arbitrary QMP commands from command line List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: amit.shah@redhat.com, liang.z.li@intel.com, qemu-devel@nongnu.org, quintela@redhat.com * Markus Armbruster (armbru@redhat.com) wrote: > "Dr. David Alan Gilbert" writes: > > > * Eric Blake (eblake@redhat.com) wrote: > >> On 01/29/2015 09:28 AM, Dr. David Alan Gilbert wrote: > >> > * Eric Blake (eblake@redhat.com) wrote: > >> > > On 01/29/2015 08:54 AM, Dr. David Alan Gilbert wrote: > >> > > >> The idea of a QMP command to trigger incoming migration looks > >> > > >> reasonable. We can probably use a qapi union for a nicer syntax, > >> > > >> something like: > >> > > >> > >> > > >> {"execute": "migrate-incoming", "arguments": { > >> > > >> "type": "tcp", "port": 44 } } > >> > > >> vs. > >> > > >> {"execute": "migrate-incoming", "arguments": { > >> > > >> "type": "fd", "fd": 0 } } > >> > > >> vs. > >> > > >> {"execute": "migrate-incoming", "arguments": { > >> > > >> "type": "exec", "command": [ "cat", "/path/to/file" ] } } > >> > > >> > >> > > >> and so forth. > >> > > > > >> > > > Compared to just taking a URI argument that Dan suggested, that's quite a > >> > > > bit of rework to do the reworking of each transport which is pretty > >> > > > trivial. > >> > > > >> > > Yes, but getting the interface right means that adding future extensions > >> > > will be easier, with less string parsing hacks. > > We have a general rule for QMP: no syntax embedded in string arguments, > use JSON. > > >> > I guess so, but I still have to maintain the -incoming string interface > >> > and an HMP equivalent of whatever we come up with here. > > The HMP equivalent may or may not be needed. If we decide we want it, I treat HMP as important as QMP, I don't break it or lose functionality on it. > reusing the command line's parser there probably makes more sense than > inventing yet another syntax. > > >> > So what would the .args_type look like in qmp-commands.hx; > >> > something like this? > >> > > >> > .args-type = "type:s,port:-i,host:-s,command:-s" > >> > >> No, it would be more like the blockdev-add interface, where one command > >> accepts a dictionary object containing a union of valid values, where > >> the set of valid values is determined by the discriminator field. > >> .args_type = "options:q". > > Note that blockdev-add has wraps its arguments rather inelegantly: it > takes a single argument 'options' of union type 'BlockdevOptions'. > Because of that, you have to write > > "arguments": { "options" : { ... } } > > instead of just > > "arguments": { ... } > > I'd love to get that cleaned up, but Kevin is already worrying about > backwards compatibility. He has a point in theory, because we neglected > to mark blockdev-add as unstable. I have a point in practice, because > blockdev-add hasn't been usable for real work (as some of our poor users > discovered the hard way) due to numerous restrictions we're still busy > lifting. Anyway, I digressed, back to the topic at hand. > > > What causes the parser to generate a 'BlockdevOptions' as opposed to any > > standard options type for the parameter of qmp_blockdev_add? > > qmp-commands.hx is a relict. It's still around because we still haven't > completed the conversion of QMP to QAPI. We suck :) > > The QAPI schema defines QMP commands. The marshalling / unmarshalling > code mapping between QMP the text protocol and actual QMP command > handlers written in C gets generated from the schema. > > docs/qapi-code-gen.txt explains this. A much improved version is > sitting in Eric's queue[*]. Perhaps Eric can provide a pointer to his > current version. > > qmp-commands.hx duplicates some of the schema information, partly in > dumbed down form, and adds a bit more. OK, to summarise how I'm hearing things so far: a) We want this as a QMP command b) With nice structured json arguments c) But the QMP parser is broken and the example that Eric wants me to follow isn't pretty. Am I missing something from that? Because at the moment I seem to be walking into a minefield of QMP parsers to add a simple bit of migration functionality. Dave > > > [*] Sorry Eric, could not resist poking you again :) -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK