Libguestfs - May 2012 - [PATCH 0/2] Don't fail if 'type' (disk format) attribute is missing from libvirt XML (RHBZ#701814).

https://bugzilla.redhat.com/show_bug.cgi?id=701814

From: "Richard W.M. Jones" <rjones at redhat.com>

Old versions of libvirt allowed you to define disks like this:

    <disk type='file' device='disk'>
      <driver name='qemu'/>
      ...

Since the <driver> element does not have a 'type' attribute (which
defines the format), we are supposed to do autodetection, so the
format should be undefined.

However what actually happened was that the code in
Sys::Guestfs::Lib::open_guest received format as an empty string from
the xpath query, causing libguestfs to give an error.

If the xpath query returns the format as an empty string, undefine it.
---
 perl/lib/Sys/Guestfs/Lib.pm |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/perl/lib/Sys/Guestfs/Lib.pm b/perl/lib/Sys/Guestfs/Lib.pm
index 42d2e81..2ccc09a 100644
--- a/perl/lib/Sys/Guestfs/Lib.pm
+++ b/perl/lib/Sys/Guestfs/Lib.pm
@@ -234,7 +234,11 @@ sub open_guest
 
             # Get the disk format (may not be set).
             my $format = $p->find ('./driver/@type', $node);
-            $format = $format->to_literal if $format;
+            if ($format) {
+                $format = $format->to_literal;
+            } else {
+                undef $format; # RHBZ#701814.
+            }
 
             push @disks, [ $filename, $format ];
         }
-- 
1.7.10

From: "Richard W.M. Jones" <rjones at redhat.com>

This commit adds a tests/xml directory, and an LD_PRELOAD module which
can fake arbitrary libvirt XML from an external file (and is therefore
a much more flexible test than using the libvirt test:// driver
alone).

Also added is one regression test for:
https://bugzilla.redhat.com/show_bug.cgi?id=701814

Loading the given libvirt XML using Sys::Guestfs::Lib::open_guest
used to fail with the error:

  format parameter is empty or contains disallowed characters at
/home/rjones/d/libguestfs/perl/blib/lib/Sys/Guestfs/Lib.pm line 256.

Thanks to Tom Horsley for supplying the test data.
---
 Makefile.am                    |    1 +
 configure.ac                   |    1 +
 tests/xml/Makefile.am          |   49 ++++++++++++++++++++++
 tests/xml/fake_libvirt_xml.c   |   87 ++++++++++++++++++++++++++++++++++++++++
 tests/xml/rhbz701814-faked.xml |   70 ++++++++++++++++++++++++++++++++
 tests/xml/rhbz701814-node.xml  |   19 +++++++++
 tests/xml/rhbz701814.pl        |   30 ++++++++++++++
 7 files changed, 257 insertions(+)
 create mode 100644 tests/xml/Makefile.am
 create mode 100644 tests/xml/fake_libvirt_xml.c
 create mode 100644 tests/xml/rhbz701814-faked.xml
 create mode 100644 tests/xml/rhbz701814-node.xml
 create mode 100755 tests/xml/rhbz701814.pl

diff --git a/Makefile.am b/Makefile.am
index 3ff6608..a483918 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -43,6 +43,7 @@ SUBDIRS += tests/luks
 SUBDIRS += tests/md
 SUBDIRS += tests/ntfsclone
 SUBDIRS += tests/btrfs
+SUBDIRS += tests/xml
 SUBDIRS += tests/regressions
 endif
 
diff --git a/configure.ac b/configure.ac
index dc1f27a..ca1c146 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1344,6 +1344,7 @@ AC_CONFIG_FILES([Makefile
                  tests/protocol/Makefile
                  tests/qemu/Makefile
                  tests/regressions/Makefile
+                 tests/xml/Makefile
                  tools/Makefile])
 AC_OUTPUT
 
diff --git a/tests/xml/Makefile.am b/tests/xml/Makefile.am
new file mode 100644
index 0000000..d9abd5c
--- /dev/null
+++ b/tests/xml/Makefile.am
@@ -0,0 +1,49 @@
+# libguestfs
+# Copyright (C) 2012 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+include $(top_srcdir)/subdir-rules.mk
+
+if HAVE_LIBVIRT
+
+# This LD_PRELOAD library can be used to precisely control the XML
+# returned by libvirt.
+check_LTLIBRARIES = libfakevirtxml.la
+
+libfakevirtxml_la_SOURCES = fake_libvirt_xml.c
+libfakevirtxml_la_CFLAGS = $(LIBVIRT_CFLAGS)
+# -version-info and -rpath force libtool to build a shared library.
+libfakevirtxml_la_LDFLAGS = -version-info 0:0:0 -rpath /nowhere
+
+random_val := $(shell awk 'BEGIN{srand(); print 1+int(255*rand())}'
< /dev/null)
+
+TESTS = \
+	rhbz701814.pl
+
+TESTS_ENVIRONMENT = \
+	MALLOC_PERTURB_=$(random_val) \
+	abs_srcdir=$(abs_srcdir) \
+	LD_PRELOAD=.libs/libfakevirtxml.so \
+	$(top_builddir)/run
+
+endif
+
+EXTRA_DIST = \
+	rhbz701814.pl \
+	rhbz701814-faked.xml \
+	rhbz701814-node.xml
+
+CLEANFILES = *~
diff --git a/tests/xml/fake_libvirt_xml.c b/tests/xml/fake_libvirt_xml.c
new file mode 100644
index 0000000..6119241
--- /dev/null
+++ b/tests/xml/fake_libvirt_xml.c
@@ -0,0 +1,87 @@
+/* libguestfs
+ * Copyright (C) 2012 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+
+#include <libvirt/libvirt.h>
+
+char *
+virDomainGetXMLDesc (virDomainPtr dom, unsigned int flags)
+{
+  const char *path;
+  int fd;
+  struct stat statbuf;
+  char *buf, *p;
+  size_t n;
+  ssize_t r;
+
+  path = getenv ("FAKE_LIBVIRT_XML");
+
+  if (!path) {
+    fprintf (stderr, "environment variable FAKE_LIBVIRT_XML is not
set\n");
+    _exit (1);
+  }
+
+  fprintf (stderr,
+           "fake_libvirt_xml: returning fake libvirt XML from %s\n",
path);
+
+  fd = open (path, O_RDONLY | O_CLOEXEC);
+  if (fd == -1) {
+    perror (path);
+    _exit (1);
+  }
+
+  if (fstat (fd, &statbuf) == -1) {
+    perror ("fstat");
+    _exit (1);
+  }
+
+  buf = malloc (statbuf.st_size + 1);
+  if (buf == NULL) {
+    perror ("malloc");
+    _exit (1);
+  }
+
+  for (n = 0, p = buf; n < statbuf.st_size; ++n) {
+    r = read (fd, p, statbuf.st_size - n);
+    if (r == -1) {
+      perror ("read");
+      _exit (1);
+    }
+    if (r == 0)
+      break;
+    n += r;
+    p += r;
+  }
+
+  *p = '\0';
+
+  if (close (fd) == -1) {
+    perror ("close");
+    _exit (1);
+  }
+
+  return buf;                   /* caller frees */
+}
diff --git a/tests/xml/rhbz701814-faked.xml b/tests/xml/rhbz701814-faked.xml
new file mode 100644
index 0000000..ae31915
--- /dev/null
+++ b/tests/xml/rhbz701814-faked.xml
@@ -0,0 +1,70 @@
+<!-- supplied for the regression test by Tom Horsley -->
+<domain type='test'>
+  <name>winxppro</name>
+  <uuid>6d626c31-643c-ca9a-d36d-3c1612e39bbd</uuid>
+  <memory>1048576</memory>
+  <currentMemory>1048576</currentMemory>
+  <vcpu>2</vcpu>
+  <os>
+    <type arch='x86_64'
machine='pc-0.11'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <clock offset='localtime'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-spice-wrapper</emulator>
+    <disk type='file' device='disk'>
+      <driver name='qemu'/>
+      <source file='/dev/null'/>
+      <target dev='vda' bus='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x00'
slot='0x04' function='0x0'/>
+    </disk>
+    <disk type='file' device='cdrom'>
+      <source file='/dev/null'/>
+      <target dev='hdc' bus='ide'/>
+      <readonly/>
+      <address type='drive' controller='0' bus='1'
unit='0'/>
+    </disk>
+    <disk type='file' device='disk'>
+      <driver name='qemu'/>
+      <source file='/dev/null'/>
+      <target dev='vdb' bus='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x00'
slot='0x07' function='0x0'/>
+    </disk>
+    <controller type='ide' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00'
slot='0x01' function='0x1'/>
+    </controller>
+    <interface type='bridge'>
+      <mac address='54:52:00:62:7b:5c'/>
+      <source bridge='br0'/>
+      <model type='virtio'/>
+      <address type='pci' domain='0x0000' bus='0x00'
slot='0x05' function='0x0'/>
+    </interface>
+    <serial type='pty'>
+      <target port='0'/>
+    </serial>
+    <console type='pty'>
+      <target type='serial' port='0'/>
+    </console>
+    <input type='mouse' bus='ps2'/>
+    <graphics type='vnc' port='-1'
autoport='yes'/>
+    <sound model='ac97'>
+      <address type='pci' domain='0x0000' bus='0x00'
slot='0x03' function='0x0'/>
+    </sound>
+    <video>
+      <model type='cirrus' vram='9216'
heads='1'/>
+      <address type='pci' domain='0x0000' bus='0x00'
slot='0x02' function='0x0'/>
+    </video>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00'
slot='0x06' function='0x0'/>
+    </memballoon>
+  </devices>
+</domain>
+
diff --git a/tests/xml/rhbz701814-node.xml b/tests/xml/rhbz701814-node.xml
new file mode 100644
index 0000000..5266873
--- /dev/null
+++ b/tests/xml/rhbz701814-node.xml
@@ -0,0 +1,19 @@
+<!-- This is dummy XML needed by the libvirt test:// driver.  It will
+     be overridden by the fake libvirt XML preload module. -->
+<node>
+<domain type='test'>
+  <name>winxppro</name>
+  <memory>1048576</memory>
+  <os>
+    <type>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <devices>
+    <disk type='file' device='disk'>
+      <driver name='qemu'/>
+      <source file='/dev/null'/>
+      <target dev='vda' bus='virtio'/>
+    </disk>
+  </devices>
+</domain>
+</node>
diff --git a/tests/xml/rhbz701814.pl b/tests/xml/rhbz701814.pl
new file mode 100755
index 0000000..bd33692
--- /dev/null
+++ b/tests/xml/rhbz701814.pl
@@ -0,0 +1,30 @@
+#!/usr/bin/perl -w
+# Copyright (C) 2012 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Regression test for:
+# https://bugzilla.redhat.com/show_bug.cgi?id=701814
+
+use strict;
+
+use Sys::Guestfs;
+use Sys::Guestfs::Lib qw(open_guest);
+
+$ENV{FAKE_LIBVIRT_XML} = "rhbz701814-faked.xml";
+my $abs_srcdir = $ENV{abs_srcdir};
+
+my $g = open_guest (["winxppro"],
+                    address =>
"test://$abs_srcdir/rhbz701814-node.xml");
-- 
1.7.10