Re: [PATCH] util: open XML files before calling libxml2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 07, 2024 at 03:59:52PM +0100, Daniel P. Berrangé wrote:
> Libxml2 has awful error reporting behaviour when reading files. When
> we fail to load a file from the test driver we see:
> 
>   $ virsh -c test:///wibble.xml
>   I/O warning : failed to load external entity "/wibble.xml"
>   error: failed to connect to the hypervisor
>   error: XML error: failed to parse xml document '/wibble.xml'
> 
> where the I/O warning line is something printed by libxml2 itself,
> which also lacks any useful detail.
> 
> Switching to our own file reading code we can massively improve
> things:
> 
>   $ ./build/tools/virsh -c test:///wibble.xml
>   error: failed to connect to the hypervisor
>   error: Failed to open file '/wibble.xml': No such file or directory
> 
> Using 10 MB as an upper limit on XML file size ought to be sufficient
> for any XML files libvirt is reading.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> ---
>  src/util/virxml.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/virxml.c b/src/util/virxml.c
> index a7b75fd7b3..f6b937b277 100644
> --- a/src/util/virxml.c
> +++ b/src/util/virxml.c
> @@ -1133,6 +1133,7 @@ virXMLParseHelper(int domcode,
>      xmlNodePtr rootnode;
>      const char *docname;
>      int parseFlags = XML_PARSE_NONET | XML_PARSE_NOWARNING;
> +    g_autofree char *xmlStrPtr = NULL;
>  
>      if (filename)
>          docname = filename;
> @@ -1155,10 +1156,11 @@ virXMLParseHelper(int domcode,
>      }
>  
>      if (filename) {
> -        xml = xmlCtxtReadFile(pctxt, filename, NULL, parseFlags);
> -    } else {
> -        xml = xmlCtxtReadDoc(pctxt, BAD_CAST xmlStr, url, NULL, parseFlags);
> +        if (virFileReadAll(filename, 1024*1024*10, &xmlStrPtr) < 0)
> +            return NULL;
> +        xmlStr = xmlStrPtr;
>      }
> +    xml = xmlCtxtReadDoc(pctxt, BAD_CAST xmlStr, url, NULL, parseFlags);
>  
>      if (!xml) {
>          if (virGetLastErrorCode() == VIR_ERR_OK) {

Seems sensible:

Reviewed-by: Richard W.M. Jones <rjones@xxxxxxxxxx>


-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux