[Libvir] [PATCH] #4: Change interface between xen_unified & its underlying drivers

[Richard W.M. Jones]

Daniel Veillard wrote:
> On Wed, Jul 04, 2007 at 05:24:03AM +0100, Daniel P. Berrange wrote:
>> On Tue, Jul 03, 2007 at 04:19:13PM +0100, Richard W.M. Jones wrote:
>>> This patch starts by removing the id, name and version fields from
>>> virDriver.
>>>
>>> It also removes getMaxVcpus and the domainLookup* fields, which will
>>> make more sense when you see patches #6 and #7 in this series.
>> Yes, i guess 4, 6 & 7 should really be all one patch - since if you
>> apply , but not 6 & 7 the driver is non-functional. Anyway, the combo
>> of this & the other patches look like they're doing what I'd expect,
>> so objections here...
> 
>   Honnestly I'm not really convinced as such the patches are an improvement.
> You introduce a new structure, it's still indirect, how is that better ?

I didn't explain it well, but here's why: At the moment if you want to 
find out how, say, "reboot" is implemented you need to first of all 
examine all 5 underlying drivers to see which have a non-NULL function 
in that slot in the driver table.  Secondly you need to examine 
xen_unified.c in two places: at the top of the file to see what order 
the backend drivers are normally called in, then at the 
xenUnifiedDomainReboot function itself to see if the function follows 
that order or is one of the exceptions that does its own thing.  So you 
have to examine 7 places in the code.  I want to bring that all together 
so that you only have to examine one place (xenUnifiedDomainReboot) to 
see how it is implemented.

The fact that at the moment virDriver is a dual-purpose structure was 
just about convenience when I originally implemented xen-unified.  It 
makes no particular sense to have the interface between libvirt.c and 
drivers be the same as the xen-internal interface between xen_unified.c 
and its underlying drivers.

> Now if you want to get rid of the indirection, why not but:
>    - this forces to make all those functions public again

In dynamic libs these functions will be hidden because of the 
libvirt_syms file.  In static libvirt.a the functions will be exported, 
but they are already exported at the moment via the driver table, so 
there is no practical difference.

>    - puts more logic in xen_unified, I'm not sure its code would
>      become more maintainable
> This would probably help in debugging sessions, that's right.
> Do we want to do this while Xen support is still in flux, we need
> to do Xen-API support (on localhost, I would not extend over network)
> in a not so distant future, the driver based approach simplifies the 
> integration of new code.

I really don't think the patch as it stands is a major change though - I 
mean the calls are still going through a structure, it just happens to 
be a xen-specific structure rather than the virDriver structure.

Rich.

-- 
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3237 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20070704/03234c1b/attachment-0001.bin>