[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]