From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45336) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YH875-0006c2-7Z for qemu-devel@nongnu.org; Fri, 30 Jan 2015 04:43:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YH870-00059b-6F for qemu-devel@nongnu.org; Fri, 30 Jan 2015 04:43:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36813) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YH86z-00059R-V1 for qemu-devel@nongnu.org; Fri, 30 Jan 2015 04:43:38 -0500 From: Markus Armbruster 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> <20150130092050.GA2370@work-vm> Date: Fri, 30 Jan 2015 10:43:34 +0100 In-Reply-To: <20150130092050.GA2370@work-vm> (David Alan Gilbert's message of "Fri, 30 Jan 2015 09:20:50 +0000") Message-ID: <87fvasr4c9.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain 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: "Dr. David Alan Gilbert" Cc: amit.shah@redhat.com, liang.z.li@intel.com, qemu-devel@nongnu.org, quintela@redhat.com "Dr. David Alan Gilbert" writes: > * 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. There is just one QMP parser. I wouldn't call it "broken". It parses fine. It's use by blockdev-add is inelegant. Would you like me to prototype a "migrate-incoming" command for you that does nothing?