[Libvir] PATCH: Allow emptry bootloader

Latest XenD allows for default bootloader to be used if one is not defined for a paravirt OS. libvirt doesn't currently cope with this (see Solaris thread), since it requires either a bootloader or kernel to always be present. The attached patch allows for an emptry bootloader element to indicate that the domain should use the default. <domain type='xen' id='-1'> <name>rhel5pv</name> <uuid>614d7eb0-c6b1-5235-ac39-361b20f6cd49</uuid> <bootloader/> <os> <type>linux</type> </os> ...snip... </domain> The current API for looking up nodes in the SEXPR did not allow for the scenario where you might want to lookup a node with no content, eg (bootloader ) This distinction is needed here, so I added an sexpr_has() method to check for this. Second, it fixes a long standing bug where we'd record an error about the missing kernel or bootloader, but then continue generating (malformed) XML anyway. With this patch, it will correctly fail if the empty bootloader field is missing from the SEXPR. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

On Fri, Sep 28, 2007 at 11:12:41PM +0100, Daniel P. Berrange wrote:
Latest XenD allows for default bootloader to be used if one is not defined for a paravirt OS. libvirt doesn't currently cope with this (see Solaris thread), since it requires either a bootloader or kernel to always be present.
The attached patch allows for an emptry bootloader element to indicate that the domain should use the default.
<domain type='xen' id='-1'> <name>rhel5pv</name> <uuid>614d7eb0-c6b1-5235-ac39-361b20f6cd49</uuid> <bootloader/> <os> <type>linux</type> </os> ...snip... </domain>
The current API for looking up nodes in the SEXPR did not allow for the scenario where you might want to lookup a node with no content, eg
(bootloader )
This distinction is needed here, so I added an sexpr_has() method to check for this.
Second, it fixes a long standing bug where we'd record an error about the missing kernel or bootloader, but then continue generating (malformed) XML anyway. With this patch, it will correctly fail if the empty bootloader field is missing from the SEXPR.
Looks fine to me +1, 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/

On Sat, Sep 29, 2007 at 06:10:40AM -0400, Daniel Veillard wrote:
On Fri, Sep 28, 2007 at 11:12:41PM +0100, Daniel P. Berrange wrote:
Latest XenD allows for default bootloader to be used if one is not defined for a paravirt OS. libvirt doesn't currently cope with this (see Solaris thread), since it requires either a bootloader or kernel to always be present.
The attached patch allows for an emptry bootloader element to indicate that the domain should use the default.
<domain type='xen' id='-1'> <name>rhel5pv</name> <uuid>614d7eb0-c6b1-5235-ac39-361b20f6cd49</uuid> <bootloader/> <os> <type>linux</type> </os> ...snip... </domain>
The current API for looking up nodes in the SEXPR did not allow for the scenario where you might want to lookup a node with no content, eg
(bootloader )
This distinction is needed here, so I added an sexpr_has() method to check for this.
Second, it fixes a long standing bug where we'd record an error about the missing kernel or bootloader, but then continue generating (malformed) XML anyway. With this patch, it will correctly fail if the empty bootloader field is missing from the SEXPR.
Looks fine to me +1,
Ok, comitted this to CVS. Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|

On Sat, Sep 29, 2007 at 07:31:23PM +0100, Daniel P. Berrange wrote:
On Sat, Sep 29, 2007 at 06:10:40AM -0400, Daniel Veillard wrote:
On Fri, Sep 28, 2007 at 11:12:41PM +0100, Daniel P. Berrange wrote:
Latest XenD allows for default bootloader to be used if one is not defined for a paravirt OS. libvirt doesn't currently cope with this (see Solaris thread), since it requires either a bootloader or kernel to always be present.
The attached patch allows for an emptry bootloader element to indicate that the domain should use the default.
<domain type='xen' id='-1'> <name>rhel5pv</name> <uuid>614d7eb0-c6b1-5235-ac39-361b20f6cd49</uuid> <bootloader/> <os> <type>linux</type> </os> ...snip... </domain>
The current API for looking up nodes in the SEXPR did not allow for the scenario where you might want to lookup a node with no content, eg
(bootloader )
This distinction is needed here, so I added an sexpr_has() method to check for this.
Second, it fixes a long standing bug where we'd record an error about the missing kernel or bootloader, but then continue generating (malformed) XML anyway. With this patch, it will correctly fail if the empty bootloader field is missing from the SEXPR.
Looks fine to me +1,
Ok, comitted this to CVS.
This changed the output of tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml with the new empty <bootloader/>, updating in CVS RSN too :-) 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/

On Sun, Sep 30, 2007 at 08:58:38AM -0400, Daniel Veillard wrote:
On Sat, Sep 29, 2007 at 07:31:23PM +0100, Daniel P. Berrange wrote:
Ok, comitted this to CVS.
This changed the output of tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml with the new empty <bootloader/>, updating in CVS RSN too :-)
Doh, just discovered some versions of XenD also include an empty bogus bootloader tag for HVM guests - that test happens to use an SEXPR from one such XenD. I've added a fix to make use of the bootloader tag conditional on it being paravirt. Regards, Dan. -- |=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=| |=- Perl modules: http://search.cpan.org/~danberr/ -=| |=- Projects: http://freshmeat.net/~danielpb/ -=| |=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
participants (2)
-
Daniel P. Berrange
-
Daniel Veillard