Libguestfs - May 2016 - Re: [PATCH] v2v: handle subfolders in ova files

Some ova files have their ovf and other files inside a folder rather
than at the root of the tarball. Consider the paths relative to the
ovf file to cover this case too.
---
 v2v/Makefile.am                  |  1 +
 v2v/input_ova.ml                 |  3 +-
 v2v/test-v2v-i-ova-subfolders.sh | 88 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+), 1 deletion(-)
 create mode 100755 v2v/test-v2v-i-ova-subfolders.sh

diff --git a/v2v/Makefile.am b/v2v/Makefile.am
index 0203317..1d219b6 100644
--- a/v2v/Makefile.am
+++ b/v2v/Makefile.am
@@ -294,6 +294,7 @@ TESTS_ENVIRONMENT = $(top_builddir)/run --test
 TESTS = \
 	test-v2v-i-ova-formats.sh \
 	test-v2v-i-ova-gz.sh \
+	test-v2v-i-ova-subfolders.sh \
 	test-v2v-i-ova-two-disks.sh \
 	test-v2v-copy-to-local.sh \
 	test-v2v-bad-networks-and-bridges.sh
diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index 1aba662..b0d9357 100644
--- a/v2v/input_ova.ml
+++ b/v2v/input_ova.ml
@@ -165,6 +165,7 @@ object
     ) mf;
 
     (* Parse the ovf file. *)
+    let ovf_folder = Filename.dirname ovf in
     let xml = read_whole_file ovf in
     let doc = Xml.parse_memory xml in
 
@@ -265,7 +266,7 @@ object
             | Some s -> s in
 
           (* Does the file exist and is it readable? *)
-          let filename = exploded // filename in
+          let filename = ovf_folder // filename in
           Unix.access filename [Unix.R_OK];
 
           (* The spec allows the file to be gzip-compressed, in which case
diff --git a/v2v/test-v2v-i-ova-subfolders.sh b/v2v/test-v2v-i-ova-subfolders.sh
new file mode 100755
index 0000000..fd59f07
--- /dev/null
+++ b/v2v/test-v2v-i-ova-subfolders.sh
@@ -0,0 +1,88 @@
+#!/bin/bash -
+# libguestfs virt-v2v test script
+# Copyright (C) 2014 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.
+
+# Test -i ova option.
+
+unset CDPATH
+export LANG=C
+set -e
+set -x
+
+if [ -n "$SKIP_TEST_V2V_I_OVA_SH" ]; then
+    echo "$0: test skipped because environment variable is set"
+    exit 77
+fi
+
+if [ "$(guestfish get-backend)" = "uml" ]; then
+    echo "$0: test skipped because UML backend does not support
network"
+    exit 77
+fi
+
+f=../test-data/phony-guests/windows.img
+if ! test -f $f || ! test -s $f; then
+    echo "$0: test skipped because phony Windows image was not
created"
+    exit 77
+fi
+
+export VIRT_TOOLS_DATA_DIR="$srcdir/../test-data/fake-virt-tools"
+export VIRTIO_WIN="$srcdir/../test-data/fake-virtio-win"
+
+. $srcdir/../test-data/guestfs-hashsums.sh
+
+d=test-v2v-i-ova-subfolders.d
+rm -rf $d
+mkdir -p $d/subfolder
+
+vmdk=test-ova.vmdk
+ovf=test-v2v-i-ova.ovf
+mf=test-ova.mf
+ova=test-ova-subfolders.ova
+raw=TestOva-sda
+
+qemu-img convert $f -O vmdk $d/subfolder/$vmdk
+cp $ovf $d/subfolder/$ovf
+sha1=`do_sha1 $d/subfolder/$ovf`
+echo "SHA1($ovf)= $sha1" > $d/subfolder/$mf
+sha1=`do_sha1 $d/subfolder/$vmdk`
+echo "SHA1($vmdk)= $sha1" >> $d/subfolder/$mf
+
+pushd .
+cd $d
+tar -cf $ova subfolder
+rm -rf subfolder
+popd
+
+$VG virt-v2v --debug-gc \
+    -i ova $d/$ova \
+    -o local -of raw -os $d
+
+# Test the libvirt XML metadata and a disk was created.
+test -f $d/$raw
+test -f $d/TestOva.xml
+
+# Normalize the XML output.
+mv $d/TestOva.xml $d/TestOva.xml.old
+sed \
+    -e "s,source file='.*TestOva-sda',source
file='TestOva-sda'," \
+    -e "s,generated by.*--,generated by --," \
+    < $d/TestOva.xml.old > $d/TestOva.xml
+
+# Check the libvirt XML output.
+diff -u test-v2v-i-ova.xml $d/TestOva.xml
+
+rm -rf $d
-- 
2.6.6

On Thursday 19 May 2016 17:26:12 Cédric Bosdonnat wrote:
> Some ova files have their ovf and other files inside a folder rather
> than at the root of the tarball. Consider the paths relative to the
> ovf file to cover this case too.
> ---
Ouch... this standard is getting more funky implementations, I see...
what is the producer of this kind of ova?

A couple of notes below.
>  v2v/Makefile.am                  |  1 +
>  v2v/input_ova.ml                 |  3 +-
>  v2v/test-v2v-i-ova-subfolders.sh | 88
++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 91 insertions(+), 1 deletion(-)
>  create mode 100755 v2v/test-v2v-i-ova-subfolders.sh
> 
> diff --git a/v2v/Makefile.am b/v2v/Makefile.am
> index 0203317..1d219b6 100644
> --- a/v2v/Makefile.am
> +++ b/v2v/Makefile.am
> @@ -294,6 +294,7 @@ TESTS_ENVIRONMENT = $(top_builddir)/run --test
>  TESTS = \
>  	test-v2v-i-ova-formats.sh \
>  	test-v2v-i-ova-gz.sh \
> +	test-v2v-i-ova-subfolders.sh \
>  	test-v2v-i-ova-two-disks.sh \
>  	test-v2v-copy-to-local.sh \
>  	test-v2v-bad-networks-and-bridges.sh
> diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
> index 1aba662..b0d9357 100644
> --- a/v2v/input_ova.ml
> +++ b/v2v/input_ova.ml
> @@ -165,6 +165,7 @@ object
>      ) mf;
>  
>      (* Parse the ovf file. *)
> +    let ovf_folder = Filename.dirname ovf in
>      let xml = read_whole_file ovf in
>      let doc = Xml.parse_memory xml in
>  
> @@ -265,7 +266,7 @@ object
>              | Some s -> s in
>  
>            (* Does the file exist and is it readable? *)
> -          let filename = exploded // filename in
> +          let filename = ovf_folder // filename in
>            Unix.access filename [Unix.R_OK];
This would seem correct to me, at a quick glance.
> diff --git a/v2v/test-v2v-i-ova-subfolders.sh
b/v2v/test-v2v-i-ova-subfolders.sh
> new file mode 100755
> index 0000000..fd59f07
> --- /dev/null
> +++ b/v2v/test-v2v-i-ova-subfolders.sh
> @@ -0,0 +1,88 @@
> +#!/bin/bash -
> +# libguestfs virt-v2v test script
> +# Copyright (C) 2014 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.
> +
> +# Test -i ova option.
> +
> +unset CDPATH
> +export LANG=C
> +set -e
> +set -x
> +
> +if [ -n "$SKIP_TEST_V2V_I_OVA_SH" ]; then
This needs to reflect the test name.
> +    echo "$0: test skipped because environment variable is set"
> +    exit 77
> +fi
> +
> +if [ "$(guestfish get-backend)" = "uml" ]; then
> +    echo "$0: test skipped because UML backend does not support
network"
> +    exit 77
> +fi
> +
> +f=../test-data/phony-guests/windows.img
> +if ! test -f $f || ! test -s $f; then
> +    echo "$0: test skipped because phony Windows image was not
created"
> +    exit 77
> +fi
> +
> +export
VIRT_TOOLS_DATA_DIR="$srcdir/../test-data/fake-virt-tools"
> +export VIRTIO_WIN="$srcdir/../test-data/fake-virtio-win"
> +
> +. $srcdir/../test-data/guestfs-hashsums.sh
> +
> +d=test-v2v-i-ova-subfolders.d
> +rm -rf $d
> +mkdir -p $d/subfolder
> +
> +vmdk=test-ova.vmdk
> +ovf=test-v2v-i-ova.ovf
> +mf=test-ova.mf
> +ova=test-ova-subfolders.ova
> +raw=TestOva-sda
While I see this is mostly a copy from test-v2v-i-ova.sh, please change
the name of temporary and output files, otherwise there are conflicts
when this and test-v2v-i-ova.sh run in parallel.

This would be the fifth test-v2v-i-ova*.sh test, and they all look
quite close.  I'll take a look about unifying them somehow, otherwise
testing new cases in ova becomes a nightmare...


-- 
Pino Toscano

On Thu, 2016-05-19 at 19:41 +0200, Pino Toscano wrote:
> On Thursday 19 May 2016 17:26:12 Cédric Bosdonnat wrote:
> > Some ova files have their ovf and other files inside a folder
> > rather
> > than at the root of the tarball. Consider the paths relative to the
> > ovf file to cover this case too.
> > ---
> 
> Ouch... this standard is getting more funky implementations, I see...
> what is the producer of this kind of ova?
SUSE Studio. In fact I'm porting a fix we had on the old v2v.
> A couple of notes below.
> 
> >  v2v/Makefile.am                  |  1 +
> >  v2v/input_ova.ml                 |  3 +-
> >  v2v/test-v2v-i-ova-subfolders.sh | 88
> > ++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 91 insertions(+), 1 deletion(-)
> >  create mode 100755 v2v/test-v2v-i-ova-subfolders.sh
> > 
> > diff --git a/v2v/Makefile.am b/v2v/Makefile.am
> > index 0203317..1d219b6 100644
> > --- a/v2v/Makefile.am
> > +++ b/v2v/Makefile.am
> > @@ -294,6 +294,7 @@ TESTS_ENVIRONMENT = $(top_builddir)/run --test
> >  TESTS = \
> >  	test-v2v-i-ova-formats.sh \
> >  	test-v2v-i-ova-gz.sh \
> > +	test-v2v-i-ova-subfolders.sh \
> >  	test-v2v-i-ova-two-disks.sh \
> >  	test-v2v-copy-to-local.sh \
> >  	test-v2v-bad-networks-and-bridges.sh
> > diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
> > index 1aba662..b0d9357 100644
> > --- a/v2v/input_ova.ml
> > +++ b/v2v/input_ova.ml
> > @@ -165,6 +165,7 @@ object
> >      ) mf;
> >  
> >      (* Parse the ovf file. *)
> > +    let ovf_folder = Filename.dirname ovf in
> >      let xml = read_whole_file ovf in
> >      let doc = Xml.parse_memory xml in
> >  
> > @@ -265,7 +266,7 @@ object
> >              | Some s -> s in
> >  
> >            (* Does the file exist and is it readable? *)
> > -          let filename = exploded // filename in
> > +          let filename = ovf_folder // filename in
> >            Unix.access filename [Unix.R_OK];
> 
> This would seem correct to me, at a quick glance.
> 
> > diff --git a/v2v/test-v2v-i-ova-subfolders.sh b/v2v/test-v2v-i-ova
> > -subfolders.sh
> > new file mode 100755
> > index 0000000..fd59f07
> > --- /dev/null
> > +++ b/v2v/test-v2v-i-ova-subfolders.sh
> > @@ -0,0 +1,88 @@
> > +#!/bin/bash -
> > +# libguestfs virt-v2v test script
> > +# Copyright (C) 2014 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.
> > +
> > +# Test -i ova option.
> > +
> > +unset CDPATH
> > +export LANG=C
> > +set -e
> > +set -x
> > +
> > +if [ -n "$SKIP_TEST_V2V_I_OVA_SH" ]; then
> 
> This needs to reflect the test name.
Indeed, that one managed to escape my attention.
> > +    echo "$0: test skipped because environment variable is
set"
> > +    exit 77
> > +fi
> > +
> > +if [ "$(guestfish get-backend)" = "uml" ]; then
> > +    echo "$0: test skipped because UML backend does not support
> > network"
> > +    exit 77
> > +fi
> > +
> > +f=../test-data/phony-guests/windows.img
> > +if ! test -f $f || ! test -s $f; then
> > +    echo "$0: test skipped because phony Windows image was not
> > created"
> > +    exit 77
> > +fi
> > +
> > +export
VIRT_TOOLS_DATA_DIR="$srcdir/../test-data/fake-virt-tools"
> > +export VIRTIO_WIN="$srcdir/../test-data/fake-virtio-win"
> > +
> > +. $srcdir/../test-data/guestfs-hashsums.sh
> > +
> > +d=test-v2v-i-ova-subfolders.d
> > +rm -rf $d
> > +mkdir -p $d/subfolder
> > +
> > +vmdk=test-ova.vmdk
> > +ovf=test-v2v-i-ova.ovf
> > +mf=test-ova.mf
> > +ova=test-ova-subfolders.ova
> > +raw=TestOva-sda
> 
> While I see this is mostly a copy from test-v2v-i-ova.sh, please
> change
> the name of temporary and output files, otherwise there are conflicts
> when this and test-v2v-i-ova.sh run in parallel.
Yes, it is a copy of the test-v2v-i-ova.sh. It could even have been a
special test case of it.

The temporary output all happens in $d which has a name based on the
test... any more I forgot there?

--
Cedric
> This would be the fifth test-v2v-i-ova*.sh test, and they all look
> quite close.  I'll take a look about unifying them somehow, otherwise
> testing new cases in ova becomes a nightmare...