[Libvir] [PATCH] Fix strange error message in virsh attach-device

[Masayuki Sunou]

Hi Daniel

Thank you for your comment.

This patch changes about the following points.

>  buf.use = 0 means we are using 0 bytes of the buffer. That's the semantic
> of buffer usage, if you really want to make it zero terminated, just use
>        buf.content[0] = 0;
>
1. This patch initializes the data with "buf.content[0] = 0".

>   use buf->use instead of strlen(buf.content)
>
2. This patch checks the data size with "buf->use".


Thanks,
Masayuki Sunou.

-------------------------------------------------------------------------------
Index: src/xml.c
===================================================================
RCS file: /data/cvs/libvirt/src/xml.c,v
retrieving revision 1.74
diff -u -p -r1.74 xml.c
--- src/xml.c	1 Jun 2007 14:03:24 -0000	1.74
+++ src/xml.c	4 Jun 2007 02:48:41 -0000
@@ -1482,6 +1482,7 @@ virParseXMLDevice(virConnectPtr conn, ch
         return (NULL);
     buf.size = 1000;
     buf.use = 0;
+    buf.content[0] = 0;
     xml = xmlReadDoc((const xmlChar *) xmldesc, "domain.xml", NULL,
                      XML_PARSE_NOENT | XML_PARSE_NONET |
                      XML_PARSE_NOERROR | XML_PARSE_NOWARNING);
@@ -1493,6 +1494,9 @@ virParseXMLDevice(virConnectPtr conn, ch
     if (xmlStrEqual(node->name, BAD_CAST "disk")) {
         if (virDomainParseXMLDiskDesc(conn, node, &buf, hvm, xendConfigVersion) != 0)
             goto error;
+        /* SXP is not created when device is "floppy". */
+       else if (buf.use == 0)
+           goto error;
     }
     else if (xmlStrEqual(node->name, BAD_CAST "interface")) {
         if (virDomainParseXMLIfDesc(conn, node, &buf, hvm) != 0)
-------------------------------------------------------------------------------

In message <20070601135415.GE9279 at redhat.com>
   "Re: [Libvir] [PATCH] Fix strange error message in virsh attach-device"
   "Daniel Veillard <veillard at redhat.com>" wrote:

> On Fri, May 25, 2007 at 07:42:12PM +0900, Masayuki Sunou wrote:
> > Hi
> > 
> > Now, virsh attach-device displays the following messages when "device"
> > attribute of "disk" element is "floppy" in XML.
> > 
> > # virsh attach-device HVM_RH4_file floppy.xml
> > libvir: Xen Daemon error : POST operation failed: (xend.err 'Invalid Configuration: XendConfig: @\x14\xce not a valid device type')
> > error: Failed to attach device from floppy.xml
> 
>   can you provide floppy.xml content too to test the problem ?
> 
> > This patch fixes this strange error message displaying "@\x14\xce".
> 
>   okay, buffer problem.
> 
> > Description of this patch 
> >   VirDomainParseXMLDiskDesc() returns 0 without putting data in virBuffer,
> >   when "floppy" is set.
> >   Therefore, virParseXMLDevice() returns no initialized data, and
> >   xenDaemonAttachDevice() passes Xen this data. 
> >   Because virDomainParseXMLDesc() works wrong when this patch changes
> >   return value of VirDomainParseXMLDiskDesc(),this patch fixes
> >   virParseXMLDevice() as follows.
> >     - Initialize data
> >     - Return error if data does not change.
> >   As a result, xenDaemonAttachDevice() doesn't pass strange data to Xen,
> >   and Xen doesn't return strange error message.
> > 
> > 
> > Signed-off-by: Masayuki Sunou <fj1826dm at aa.jp.fujitsu.com>
> > 
> > Thanks,
> > Masayuki Sunou.
> > 
> > ----------------------------------------------------------------------
> > Index: src/xml.c
> > ===================================================================
> > RCS file: /data/cvs/libvirt/src/xml.c,v
> > retrieving revision 1.73
> > diff -u -p -r1.73 xml.c
> > --- src/xml.c   23 Apr 2007 07:41:23 -0000      1.73
> > +++ src/xml.c   25 May 2007 09:46:56 -0000
> > @@ -1481,6 +1481,7 @@ virParseXMLDevice(virConnectPtr conn, ch
> >          return (NULL);
> >      buf.size = 1000;
> >      buf.use = 0;
> > +    memset(buf.content, 0, (size_t)buf.size);
> 
>  buf.use = 0 means we are using 0 bytes of the buffer. That's the semantic
> of buffer usage, if you really want to make it zero terminated, just use
>        buf.content[0] = 0;
> 
> >      xml = xmlReadDoc((const xmlChar *) xmldesc, "domain.xml", NULL,
> >                       XML_PARSE_NOENT | XML_PARSE_NONET |
> >                       XML_PARSE_NOERROR | XML_PARSE_NOWARNING);
> > @@ -1492,6 +1493,9 @@ virParseXMLDevice(virConnectPtr conn, ch
> >      if (xmlStrEqual(node->name, BAD_CAST "disk")) {
> >          if (virDomainParseXMLDiskDesc(conn, node, &buf, hvm, xendConfigVersion) != 0)
> >              goto error;
> > +         /* SXP is not created when device is "floppy". */
> > +        else if (!strlen(buf.content))
> > +            goto error;
> 
>   use buf->use instead of strlen(buf.content)
> 
> If you provide the XML to reproduce it I could double-check this, 
> 
>   thanks,
> 
> 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/
>