[Libvir] [PATCH] Fix failure of HVM domain definition on Xen 3.0.3 or earlier(BZ328841)

[Daniel Veillard]

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