From: Luiz Capitulino <lcapitulino@redhat.com>
To: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: aliguori@linux.vnet.ibm.com, qemu-devel@nongnu.org,
Markus Armbruster <armbru@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v1 00/23] QAPI Infrastructure Round 1
Date: Wed, 18 May 2011 14:45:23 -0300 [thread overview]
Message-ID: <20110518144523.56c06c20@doriath> (raw)
In-Reply-To: <4DD3F4D0.7070101@linux.vnet.ibm.com>
On Wed, 18 May 2011 11:33:20 -0500
Michael Roth <mdroth@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<mdroth@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<mdroth@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.
>
prev parent reply other threads:[~2011-05-18 17:45 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-18 0:51 [Qemu-devel] [PATCH v1 00/23] QAPI Infrastructure Round 1 Michael Roth
2011-05-18 0:51 ` [Qemu-devel] [PATCH v1][ 01/23] Add hard build dependency on glib Michael Roth
2011-05-18 0:51 ` [Qemu-devel] [PATCH v1][ 02/23] error-propagation: base code for error propagation Michael Roth
2011-05-18 0:51 ` [Qemu-devel] [PATCH v1][ 03/23] error-propagation: build qemu with with error-propagation bits Michael Roth
2011-05-18 13:53 ` Luiz Capitulino
2011-05-18 0:51 ` [Qemu-devel] [PATCH v1][ 04/23] qerror: refactor error to make the human message reusable Michael Roth
2011-05-18 8:09 ` Stefan Hajnoczi
2011-05-18 13:54 ` Luiz Capitulino
2011-05-18 0:51 ` [Qemu-devel] [PATCH v1][ 05/23] qlist: add qlist_first()/qlist_next() Michael Roth
2011-05-18 8:12 ` Stefan Hajnoczi
2011-05-18 0:51 ` [Qemu-devel] [PATCH v1][ 06/23] qapi: add module init types for qapi Michael Roth
2011-05-18 0:51 ` [Qemu-devel] [PATCH v1][ 07/23] qapi: add ordereddict/qapi.py helper libraries Michael Roth
2011-05-18 0:51 ` [Qemu-devel] [PATCH v1][ 08/23] qapi: add qapi-types.py code generator Michael Roth
2011-05-18 0:51 ` [Qemu-devel] [PATCH v1][ 09/23] qapi: add qapi-visit.py " Michael Roth
2011-05-18 0:51 ` [Qemu-devel] [PATCH v1][ 10/23] qapi: add qapi-commands.py " Michael Roth
2011-05-18 0:51 ` [Qemu-devel] [PATCH v1][ 11/23] qapi: add qapi-types-core.h Michael Roth
2011-05-18 0:51 ` [Qemu-devel] [PATCH v1][ 12/23] qapi: add qapi-visit-core.h Michael Roth
2011-05-18 0:52 ` [Qemu-devel] [PATCH v1][ 13/23] qapi: add QMP input visiter Michael Roth
2011-05-18 8:35 ` Stefan Hajnoczi
2011-05-18 0:52 ` [Qemu-devel] [PATCH v1][ 14/23] qapi: add QMP output visiter Michael Roth
2011-05-18 8:44 ` Stefan Hajnoczi
2011-05-18 0:52 ` [Qemu-devel] [PATCH v1][ 15/23] qapi: add QAPI dealloc visiter Michael Roth
2011-05-18 0:52 ` [Qemu-devel] [PATCH v1][ 16/23] qapi: add command registration/lookup functions Michael Roth
2011-05-18 0:52 ` [Qemu-devel] [PATCH v1][ 17/23] qapi: add QMP dispatch functions Michael Roth
2011-05-18 8:58 ` Stefan Hajnoczi
2011-05-18 0:52 ` [Qemu-devel] [PATCH v1][ 18/23] qapi: add base declaration/types for QMP Michael Roth
2011-05-18 0:52 ` [Qemu-devel] [PATCH v1][ 19/23] qapi: test schema used for unit tests Michael Roth
2011-05-18 0:52 ` [Qemu-devel] [PATCH v1][ 20/23] qapi: add test-visiter, tests for gen. visiter code Michael Roth
2011-05-18 0:52 ` [Qemu-devel] [PATCH v1][ 21/23] qapi: Makefile changes to build test-visiter Michael Roth
2011-05-18 0:52 ` [Qemu-devel] [PATCH v1][ 22/23] qapi: add test-qmp-commands, tests for gen. marshalling/dispatch code Michael Roth
2011-05-18 0:52 ` [Qemu-devel] [PATCH v1][ 23/23] qapi: Makefile changes to build test-qmp-commands Michael Roth
2011-05-18 13:46 ` [Qemu-devel] [PATCH v1 00/23] QAPI Infrastructure Round 1 Luiz Capitulino
2011-05-18 14:43 ` Michael Roth
2011-05-18 15:10 ` Luiz Capitulino
2011-05-18 16:33 ` Michael Roth
2011-05-18 17:45 ` Luiz Capitulino [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110518144523.56c06c20@doriath \
--to=lcapitulino@redhat.com \
--cc=aliguori@linux.vnet.ibm.com \
--cc=armbru@redhat.com \
--cc=mdroth@linux.vnet.ibm.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).