From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57854) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bxF38-0002Nl-2M for qemu-devel@nongnu.org; Thu, 20 Oct 2016 11:14:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bxF36-0006zO-T7 for qemu-devel@nongnu.org; Thu, 20 Oct 2016 11:14:30 -0400 Date: Thu, 20 Oct 2016 16:14:18 +0100 From: "Daniel P. Berrange" Message-ID: <20161020151418.GW12145@redhat.com> Reply-To: "Daniel P. Berrange" References: <1474982001-20878-1-git-send-email-berrange@redhat.com> <87vawnnjpi.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <87vawnnjpi.fsf@dusky.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v14 00/19] QAPI/QOM work for non-scalar object properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org, Max Reitz , Paolo Bonzini , Andreas =?utf-8?Q?F=C3=A4rber?= , Kevin Wolf On Thu, Oct 20, 2016 at 05:06:33PM +0200, Markus Armbruster wrote: > I've warmed quite a bit to this series while reviewing it. In > particular, I've come around to like structuring the command line -> > QAPI pipeline exactly like the JSON -> QAPI pipeline, namely 1. parse > into QObject, 2. convert to QAPI with the QObject input visitor. > QObject serves as abstract syntax here. The differences between JSON > and command line result in different abstract syntax, which in turn > necessitates two cases in the input visitor. The series adds more than > two, to cater for option visitor funnies. Perhaps we can do without > some of them. > > The other way to skin this cat would be an improved options visitor. > Has its advantages and disadvantages, but the big one is that you > already did the work for QObject input visitor solution. > > The one major issue I have with the series is that it adds to the > growing body of QemuOpts hacks/workarounds. > > We've pushed QemuOpts beyond well its design limits. What started as a > simple store for scalar configuration parameters structured as key=value > lists, plus command line and configuration file syntax, has grown three > ways to specify lists (repeated keys, basically an implementation > accident that got pressed into service; special integer list syntax in > the options visitor, later adopted by the string input visitor, > hopefully compatibly; and the block layer's dotted key convention) and a > way to specify arbitrary complex values (block layer's dotted key > convention again). Of these, only "repeated keys" is in QemuOpts > proper, all the others are bolted on and used only when needed. How > they interact is not obvious. > > This series marries all the bolted-on stuff and puts it in the QObject > visitor. That's actually an improvement of sorts; at least it's in just > two places now. But it's still a smorgasbord of syntactical/semantic > options. > > I feel it's time to stop working around the design limits of QemuOpts > and start replacing them by something that serves our current needs. We > basically need the expressive power of JSON on the command line. Syntax > is debatable, but it should be *one* concrete command-line syntax, > parsed by *one* parser into *one* kind of abstract syntax tree, where > the only optional variations are the ones forced upon us by backward > compatibility. > > We can't do this for 2.8, obviously. We can try for 2.9. I have pretty > specific ideas on how it should be done, so I guess it's only fair I > give it a try myself. > > I know the block layer wants to use bits of this series to save some > coding work for certain features targeting 2.8. I have no objections as > long as it doesn't create new ABI. Exception: I'm okay with applying > dotted key convention to more of the same, e.g. new block drivers. > > Sounds sane? It is all fine with me. Of the patch series proposed, I think patches 1->6 ought to be safe to take for 2.8, without exposing new ABI: 1. option: make parse_option_bool/number non-static 2. qdict: implement a qdict_crumple method for un-flattening a dict 3. qapi: add trace events for visitor 4. qapi: rename QmpInputVisitor to QObjectInputVisitor 5. qapi: rename QmpOutputVisitor to QObjectOutputVisitor 6. qapi: don't pass two copies of TestInputVisitorData to tests The rest are clearly not an option for 2.8 since freeze is barely 1 week away, and I'm travelling all next week Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :|