On Wed, Jul 04, 2007 at 12:21:18PM +0100, Richard W.M. Jones wrote:
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.
Hum, okay :-)
+1
but ultimately the real fix is to drop that sub driver structure.
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/