Libguestfs - Jun 2010 - [PATCH] Improve error message when LibvirtXML is given invalid domain XML

If a user accidentally passed a valid XML document to the LibvirtXML which
wasn't actually a libvirt domain XML document, they would end up getting
obtuse
errors from a libguestfs library. This change does 2 sanity checks:

1. After parsing a the XML document, check /domain/name exists.
2. Check that the domain defines some storage

The second check is generally useful, as we can't usefully do anything with
a
domain which has no storage.
---
 lib/Sys/VirtV2V/Connection.pm            |    3 +++
 lib/Sys/VirtV2V/Connection/LibVirtXML.pm |    5 +++++
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/lib/Sys/VirtV2V/Connection.pm b/lib/Sys/VirtV2V/Connection.pm
index da6a44b..4dc659d 100644
--- a/lib/Sys/VirtV2V/Connection.pm
+++ b/lib/Sys/VirtV2V/Connection.pm
@@ -186,6 +186,9 @@ sub _storage_iterate
         $source_e->setAttribute($source->getName(), '');
     }
 
+    die(user_message(__("Guest doesn't define any recognised storage
devices")))
+        unless (@paths > 0);
+
     $self->{paths} = \@paths;
     $self->{devices} = \@devices;
 }
diff --git a/lib/Sys/VirtV2V/Connection/LibVirtXML.pm
b/lib/Sys/VirtV2V/Connection/LibVirtXML.pm
index 729f1e8..ac3e82b 100644
--- a/lib/Sys/VirtV2V/Connection/LibVirtXML.pm
+++ b/lib/Sys/VirtV2V/Connection/LibVirtXML.pm
@@ -95,6 +95,11 @@ sub _get_dom
     # Display any parse errors
     die(user_message(__x("Unable to parse domain from file {path}:
{error}",
                          path => $self->{path}, error => $@))) if
($@);
+
+    # Check it looks like domain XML
+    my ($dummy) = $self->{dom}->findnodes('/domain/name');
+    die(user_message(__x("{path} doesn't look like a libvirt domain
XML file",
+                         path => $self->{path}))) unless
(defined($dummy));
 }
 
 =back
-- 
1.7.1

On Fri, Jun 18, 2010 at 03:18:14PM +0100, Matthew Booth
wrote:
> If a user accidentally passed a valid XML document to the LibvirtXML which
> wasn't actually a libvirt domain XML document, they would end up
getting obtuse
> errors from a libguestfs library. This change does 2 sanity checks:
> 
> 1. After parsing a the XML document, check /domain/name exists.
> 2. Check that the domain defines some storage
> 
> The second check is generally useful, as we can't usefully do anything
with a
> domain which has no storage.
> ---
>  lib/Sys/VirtV2V/Connection.pm            |    3 +++
>  lib/Sys/VirtV2V/Connection/LibVirtXML.pm |    5 +++++
>  2 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/lib/Sys/VirtV2V/Connection.pm b/lib/Sys/VirtV2V/Connection.pm
> index da6a44b..4dc659d 100644
> --- a/lib/Sys/VirtV2V/Connection.pm
> +++ b/lib/Sys/VirtV2V/Connection.pm
> @@ -186,6 +186,9 @@ sub _storage_iterate
>          $source_e->setAttribute($source->getName(), '');
>      }
>  
> +    die(user_message(__("Guest doesn't define any recognised
storage devices")))
> +        unless (@paths > 0);
> +
>      $self->{paths} = \@paths;
>      $self->{devices} = \@devices;
>  }
> diff --git a/lib/Sys/VirtV2V/Connection/LibVirtXML.pm
b/lib/Sys/VirtV2V/Connection/LibVirtXML.pm
> index 729f1e8..ac3e82b 100644
> --- a/lib/Sys/VirtV2V/Connection/LibVirtXML.pm
> +++ b/lib/Sys/VirtV2V/Connection/LibVirtXML.pm
> @@ -95,6 +95,11 @@ sub _get_dom
>      # Display any parse errors
>      die(user_message(__x("Unable to parse domain from file {path}:
{error}",
>                           path => $self->{path}, error => $@))) if
($@);
> +
> +    # Check it looks like domain XML
> +    my ($dummy) = $self->{dom}->findnodes('/domain/name');
> +    die(user_message(__x("{path} doesn't look like a libvirt
domain XML file",
> +                         path => $self->{path}))) unless
(defined($dummy));
>  }
>  
>  =back
> -- 
> 1.7.1
ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora