On Wed, 18 May 2011 11:33:20 -0500 Michael Roth <mdr...@linux.vnet.ibm.com> wrote:
> On 05/18/2011 10:10 AM, Luiz Capitulino wrote: > > On Wed, 18 May 2011 09:43:37 -0500 > > Michael Roth<mdr...@linux.vnet.ibm.com> wrote: > > > >> On 05/18/2011 08:46 AM, Luiz Capitulino wrote: > >>> On Tue, 17 May 2011 19:51:47 -0500 > >>> Michael Roth<mdr...@linux.vnet.ibm.com> wrote: > >>> > >>>> These apply on top of master, and can also be obtained from: > >>>> git://repo.or.cz/qemu/mdroth.git qapi_round1_v1 > >>> > >>> Nice to see this moving forward. > >>> > >>>> These patches are a backport of some of the QAPI-related work from > >>>> Anthony's > >>>> glib tree. The main goal is to get the basic code generation > >>>> infrastructure in > >>>> place so that it can be used by the guest agent to implement a QMP-like > >>>> guest > >>>> interface, and so that future work regarding the QMP conversion to QAPI > >>>> can be > >>>> decoupled from the infrastructure bits. > >>>> > >>>> Round1 incorporates the following components from Anthony's tree: > >>>> > >>>> - Pulls in GLib libraries (core GLib, GThreads, and GIO) > >>>> > >>>> - Adds code to do exception-like error propagation > >>>> > >>>> - New error reporting functions > >>>> > >>>> - Schema-based code generation for QAPI types and synchronous QMP > >>>> commands > >>>> using visiter patterns to cut reduce the amount of code generated > >>>> by the > >>>> previous scripts. This is just infrastructure, QMP will remain > >>>> untouched > >>>> until the actual conversion efforts are underway. Only a set of > >>>> unit tests > >>>> and, in the guest, virtagent, will utilize this infrastructure > >>>> initially. > >>> > >>> This series introduces quite a lot of infrastructure w/o adding a single > >>> real > >>> user. This has some disadvantages, the most important one being that we > >>> can't > >>> test it for real (unit-tests are important, but don't replace real usage). > >>> Another disadvantage is that, reviewers don't actually see how this is > >>> going to > >>> be used and can't comment on API level improvements/bugs. > >> > >> The guest agent user will mirror the QMP user pretty closely, but I > >> could see why it'd be nice to have an actual QMP user as part of the > >> series. I think we decided on IRC that an incremental QMP conversion > >> wouldn't be the best route and should instead be done as part of a > >> single concerted effort. So one approach I would propose is to have > >> example conversions tacked on to the end of this series. > > > > Yes, that would be good. > > > >> So for this series we'd have 1 or 2 example conversions for synchronous > >> QMP functions. Future infrastructure patches could provide examples for > >> async QMP/proxied QMP/QMP event/qcfg/etc users as the relevant > >> infrastructure bits are added. > > > > I think the examples have to use all the added infrastructure. For example, > > if you're not adding async commands, then we'd have to drop the async > > support > > from the series. > > Yup I agree. I actually tried to cull some of the async/proxy stuff from > this series but there were some hooks and code gen bits I left in. I'll > clean it up a bit better for the next submission. > > > > > I'm tempted to say that we should try to reduce the code generator (and all > > the infrastructure around it) to generated only the bits that are going to > > be > > used by the examples. But I'm not sure if the work involved is worth it. > > > > It wouldn't be too bad for this submission, far as I can tell. > Generation of async QMP commands and event types are the main thing > ones. The main complication is losing work from the glib tree if we're > not careful, since the initial commits were pretty much the whole > shebang. But it shouldn't be too difficult to piece things back together > as we go. Nice. > >> So long as the example conversions capture the general use cases, we'd > >> still be able to decouple conversion efforts from infrastructure (with > >> any corner cases fixed as a part of those efforts), while allowing the > >> infrastructure code to be reviewed in the proper context. > > > > Yes. > > > >>> I prefer an incremental approach. We could try to split this series in > >>> smaller > >>> parts and change current QMP to use that parts. This will make review > >>> easier > >>> and will make it possible to do incremental testing too. > >>> > >> > >> I could split the code conversion stuff out into a separate series. So > >> we'd have: > > > > Looks good to me. > > > >> Round 1: error-related changes > > > > I'm already taking care of this one. I hope to have patches soon. The > > problem > > here is that I'm very serious about testing and am going to test each > > converted handler. Unfortunately, most of the testing is done by hand today > > :( > > but kvm-autotest has some support for testing error paths and libvirt has a > > more general suite too. > > > > Ok, cool. A pretty good swath of the error stuff is needed to avoid > breakage for Round 2/3, but if you can point me to a repo I can base > this series on that and send you patches for error-related dependencies. That's my repo: git://repo.or.cz/qemu/qmp-unstable.git qapi-qerror-v1 But be aware that I rebase it often. > > >> Round 2: json-related changes > > > > I think I saw patches flying on the list, did you submit then? Do they > > depend on the error stuff? > > > > That was probably a pull request I sent a week or so ago to Anthony for > the glib tree. I think they got lost in the noise of all this reworking. > I'd also been carrying them in my virtagent series for a while. Round 2 > would have those as well as the ones in the glib tree. I'll make sure to > give those a good bit of testing. > > Some of them do make use of error propagation, but that's it as far as I > can tell. > > >> Round 3: code conversion infra + examples > >> > >> If we take the approach mentioned above, anyway. > >> > >> Otherwise I don't see how we could decouple any QMP conversion efforts > >> from infrastructure (which I think was considered desirable). In terms > >> of the code generation it's basically all or nothing, with the exception > >> of the unit tests we've added. Did you have something else in mind? > > > > Your plan looks good to me. I mean, maybe it' me who's is still catching up > > with all that stuff and want it to go slower so that I can fully absorb it > > and try to make sure it won't break anything before it's merged. > > > > On the other hand, we might want to discuss errors separately for example, > > as it's not specified to QMP. > > > > Yah, hopefully the proposed Rounds are granular enough that everyone can > absorb what's going on. Stefan H. also suggested adding some > documentation for schema/code generation usage, which might help in that > department as well. I'll try to get that included. >
