Re: [libvirt] [PATCH 00/22] Extend remote generator to generate function bodies too
On Sun, Apr 24, 2011 at 11:13:47AM +0200, Matthias Bolte wrote:
Richard W.M. Jones suggested [1] that the code that directly deals
with the
XDR protocol should be generated. The remote_generate_stubs.pl script
already generates all the headers, just the bodies in the daemon and remote
driver are manually written. But most of the functions just follow simple
patterns. So I extended the generator to exploit this patterns and move
11 kLOC code from manually written to generated code.
As expected the generator itself is pretty complex & hard to understand,
but given the complexity of our API there's nothing that can really be
done about this.
I see it takes a blacklist approach, so I'm wondering what the behaviour
is if someone adds a new function that it isn't designed to cope with
yet. Is it able to raise immediate errors for stuff it can't cope with,
or does it silently generate bogus code like our python generator often
does ? Ideally the former, but it isn't critical - just something we
need to be aware of for future API patch review
Also, as per my other patch today, I think we shouldn't actually store
the generated bodies in GIT, once this is done.
During this I came a cross many small variations and problems in the
XDR
protocol. For example, NWFilterDefineXML has a flags parameter in the public
API, but it's not transferred in the XDR protocol. Another things is the
variations in the usage of unsigned VS signed types. This comes in two forms.
public API VS XDR procotol and in between different functions. For example,
some functions use int for the flags paramater and some use unsigned int.
This results in quite a lot of special case handling in the generator.
cfg.mk | 10 +-
daemon/Makefile.am | 46 +-
daemon/qemu_dispatch_args.h | 2 +-
daemon/qemu_dispatch_bodies.c | 6 +
daemon/qemu_dispatch_prototypes.h | 2 +-
daemon/qemu_dispatch_ret.h | 2 +-
daemon/qemu_dispatch_table.h | 2 +-
daemon/remote.c | 5765 +----------------------------------
daemon/remote_dispatch_args.h | 2 +-
daemon/remote_dispatch_bodies.c | 5933 +++++++++++++++++++++++++++++++++++
daemon/remote_dispatch_prototypes.h | 80 +-
daemon/remote_dispatch_ret.h | 2 +-
daemon/remote_dispatch_table.h | 158 +-
daemon/remote_generate_stubs.pl | 195 --
daemon/remote_generator.pl | 1198 +++++++
po/POTFILES.in | 1 +
src/Makefile.am | 13 +-
src/remote/qemu_client_bodies.c | 4 +
src/remote/qemu_protocol.c | 2 +-
src/remote/qemu_protocol.h | 2 +-
src/remote/qemu_protocol.x | 2 +-
src/remote/remote_client_bodies.c | 4664 +++++++++++++++++++++++++++
src/remote/remote_driver.c | 4907 +----------------------------
src/remote/remote_protocol.c | 26 +-
src/remote/remote_protocol.h | 26 +-
src/remote/remote_protocol.x | 34 +-
src/remote_protocol-structs | 26 +-
27 files changed, 12093 insertions(+), 11017 deletions(-)
It is pretty hard to review this, but I've looked at the end result
remote_client_bodies.c and remote_dispatch_bodies.c files and they
both look sane. Also, the protocol definition itself hasn't been
changed, so I'm inclined to ACK this and let us deal with any fallout
as followups.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|