Re: [Libvir] PATCH 10/20: switch QEMU to using the driver API
On Fri, Jun 22, 2007 at 02:56:58AM +0100, Daniel P. Berrange wrote:
This is the 2nd key patch in the series. It switches the
qemud/driver.c
file methods to all follow the official libvirt internal driver API.
NB, since we're using dummy statically declared virConnect object in
the dispatch.c file, we can't use the regular virGetDomain function
for constructing virDomainPtr objects at this time. Thus we have a
hack to just malloc a virDomain struct & manually fill in the fields.
This hack will go away in a later patch.
Okay, just two nitpicks, one about lack of comments, okay those
functions are basically described as part of the driver API so it's a
bit like cut and paste, but I still think it's nicer to have next to
the code, and some lines are really long,
- for (i = 0 ; i < QEMUD_MAX_NUM_NETWORKS ; i++) {
- names[i] =
&out->qemud_packet_server_data_u.listNetworksReply.networks[i*QEMUD_MAX_NAME_LEN];
- }
There is a lot of place like that where the code manipulates
all the dereference chain a lot of time, why not use temporary
variables to make the code more readable and help the compiler
w.r.t. generating good code ?
+1 but maybe we need to revisit a bit that code.
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/