On Thu, Nov 01, 2007 at 05:37:00PM +0900, Masayuki Sunou wrote:
In message <20071031134628.GG8217@redhat.com> "Re: [Libvir] [PATCH] Fix failure of HVM domain definition on Xen 3.0.3 or earlier(BZ328841)" "Daniel Veillard <veillard@redhat.com>" wrote:
Hi
VirDomainDefineXML() fails, when HVM domain using CD-ROM is created on Xen 3.0.3 or earlier. -->BZ328841(https://bugzilla.redhat.com/show_bug.cgi?id=328841)
One of attached patches (define.patch) fixes it.
Could you please explain the patch, it changes the XML parsing and seems to change the S-Expr generation. How ? Providing examples or explanations of how the patch works will help.
This patch converts XML without the source element which virt-install makes as follows.
<disk type='file' device='cdrom'> convert <target dev='hdc'/> -----------> disk = [ ",hdc:cdrom,r" ] <readonly/> </disk>
[Detail] 1. This does not fail when the source element doesn't exist and device is CD-ROM. 2. This adjusts the size of source to 0 in calculating buflen when the source element doesn't exist. 3. This outputs source to the configuration file as before when the source element exists. And this doesn't output source to the configuration file when the source element doesn't exist. 4. This does not free source in cleanup when the source element doesn't exist.
okay, now it makes far more sense to me. I understand the logic of what you're trying to achieve. Applied and commited to CVS, thanks a lot !
But, when domain is started by virsh start using the configuration file created by fixing this, the problem that the CD-ROM device is missed occurs.
So, another patch (start.patch) fixes this problem.
Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com>
Again this seems to change parsing of S-Expr input to accept an empty value. Could you explain what this patch does, i.e. what kind of input is being targetted and what is changed by the patch.
This patch fixes to accept the following forms as mentioned above.
disk = [ ",hdc:cdrom,r" ]
I think that libvirt should analyze this form, because it exists also in the sample of Xen. (/etc/xen/xmexample.hvm)
okay, understood however your change then skips the initialization of tmp which the compiler complained might be used later in that block of code (though I think it's actually not possible, but it's better to avoid warnings). Since tmp is a reference to src and src is the empty string in this case I just added tmp = &src[0]; to the block initializing drvName, I hope it's okay, That done I applied and commited that change too, thanks a lot for the patch and explanations, 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/