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@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list