Re: [libvirt] PATCH: 9/14: Convert XenD XML generator to generic API
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(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/