On Thu, Nov 01, 2007 at 05:37:00PM +0900, Masayuki Sunou wrote: > In message <20071031134628.GG8217@xxxxxxxxxx> > "Re: [PATCH] Fix failure of HVM domain definition on Xen 3.0.3 or earlier(BZ328841)" > "Daniel Veillard <veillard@xxxxxxxxxx>" 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@xxxxxxxxxxxxxxxxx> > > > > 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@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list