[libvirt] PATCH: 9/14: Convert XenD XML generator to generic API

[Daniel Veillard]

On Thu, Jul 24, 2008 at 02:24:43PM +0100, Daniel P. Berrange wrote:
> On Thu, Jul 24, 2008 at 08:24:32AM -0400, Daniel Veillard wrote:
> You are correct that we are removing <script> element here, but
> I have only done this for the <interface type='bridge'>. It does
> not really make sense to customize the script for bringing up an
> interface, because then its not really bridging in the sense thta
> the libvirt XML describes. THe script parameter is still used for
> the generic network config with  <interface type='ethernet'>, but
> I don't think I have a test case for that.

  okay, that's my only remaining concern then, we may break people
doing weird stuff with 2 different kind of bridging.

> >   pygrub probanly means a linux boot right, but 
> 
> Yep, I decided we should be explicit and always add the
> <os> block even if we have a bootloader, so people don't
> need to special case the paravirt guest config in this
> area.

  okay

> >   We create currentMemory from memory value or I'm mistaken ?
> 
> Yes, if there is no explicit  'maxmem' setting, we set it based
> on the 'memory' value, and vica-verca. So when outputting the
> XML we will always have both elements. 

  
> >   hum hard to decript, but here it seems we lost the value of the sound
> > emulation, going from 'idontexit,es1370,all' to 'all' this looks fishy
> 
> The parser is a little more fussy here - it expects either the single
> string 'all', or a list of models. It's not letting you mix a list of
> strings and 'all' at the same time. I will look at fixing this again.

  okay this makes sense then

> > 
> >   Same thing soundhw went from 'sb16,es1370,idontexist,es1370more' to
> > 'sb16,es1370', it's a bit weird
> 
> In this case, it would previously just ignore those bogus values, but
> the parser now rejects them explicitly. The real world, if you had a 
> Xen config with a bogus 'idontexist' you'd get a nice error message 
> when dumping the XML
> 
>   "unexpected sound model idontexist"
> 
> at lesat you would if I had remembered to call __virRaiseError(), which
> I notice I missed in this particular example. So I'll fix that bit.

  the only problem is that you do error checking in libvirt instead of
letting Xen act on it, as a result you need to maintain the list. But
since we need the checking for QEmu and Xen should be a subset i assume
that's the right approach,

  +1

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/