[Libvir] PATCH 10/20: switch QEMU to using the driver API

[Daniel P. Berrange]

On Fri, Jun 22, 2007 at 08:45:59AM -0400, Daniel Veillard wrote:
> 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 ?

Again the dispatch.c file is killed in the last patch, so I wasn't really
focusing on making nice code in dispatch.c Just functional enough for me
to verify it works after each patch in the series is applied, so I didn't
waste too much effort on code that is going away :-)

>   +1 but maybe we need to revisit a bit that code.

Yep, by deleting the entire file :-)

Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=|