Tomáš Golembiovský
2016-Nov-12 15:37 UTC
[Libguestfs] [PATCH v2 0/5] Import directly from OVA tar archive if possible
This series is related to the problem of inefficient import of OVA files. The needed enhancements of QEMU were merged into the codebase and should be available in QEMU 2.8. From there we can use 'size' and 'offset' options in raw driver to tell QEMU to use only subset of a file as an image. The patch set is more or less complete. The only outstanding issue is the missing detection of sparse files inside tar. But this can be done in a separate patch. As pointed out before I didn't find a way how to detect that by using the tar tool only and would probably require use of some external library. The first three patches are just preparation. The main work is in patch four. Last patch fixes the tests. v2: - rewritten the tar invocations, the output processing is now done in OcaML rather than with a shell code; it turned out to be easier and more readable than I have feared. - added QEMU version check - addressed Pino's comments - changed tests; the expected result is now based on the QEMU used during testing Tomáš Golembiovský (5): mllib: compute checksum of file inside tar v2v: ova: don't detect compressed disks, read the OVF instead v2v: ova: move the untar function v2v: ova: don't extract files from OVA if it's not needed v2v: update tests to match changes in OVA import mllib/checksums.ml | 11 +- mllib/checksums.mli | 2 +- test-data/utils.sh | 21 ++++ v2v/Makefile.am | 1 + v2v/input_ova.ml | 184 +++++++++++++++++++++++++++----- v2v/test-v2v-i-ova-formats.sh | 5 +- v2v/test-v2v-i-ova-gz.ovf | 2 +- v2v/test-v2v-i-ova-subfolders.expected2 | 18 ++++ v2v/test-v2v-i-ova-subfolders.sh | 12 ++- v2v/test-v2v-i-ova-tar.expected | 18 ++++ v2v/test-v2v-i-ova-tar.expected2 | 18 ++++ v2v/test-v2v-i-ova-tar.ovf | 138 ++++++++++++++++++++++++ v2v/test-v2v-i-ova-tar.sh | 70 ++++++++++++ v2v/test-v2v-i-ova-two-disks.expected2 | 19 ++++ v2v/test-v2v-i-ova-two-disks.sh | 12 ++- 15 files changed, 493 insertions(+), 38 deletions(-) create mode 100755 test-data/utils.sh create mode 100644 v2v/test-v2v-i-ova-subfolders.expected2 create mode 100644 v2v/test-v2v-i-ova-tar.expected create mode 100644 v2v/test-v2v-i-ova-tar.expected2 create mode 100644 v2v/test-v2v-i-ova-tar.ovf create mode 100755 v2v/test-v2v-i-ova-tar.sh create mode 100644 v2v/test-v2v-i-ova-two-disks.expected2 -- 2.10.1
Tomáš Golembiovský
2016-Nov-12 15:37 UTC
[Libguestfs] [PATCH v2 1/5] mllib: compute checksum of file inside tar
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
mllib/checksums.ml | 11 +++++++++--
mllib/checksums.mli | 2 +-
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/mllib/checksums.ml b/mllib/checksums.ml
index dfa8c3a..0907499 100644
--- a/mllib/checksums.ml
+++ b/mllib/checksums.ml
@@ -45,7 +45,7 @@ let of_string csum_type csum_value | "sha512"
-> SHA512 csum_value
| _ -> invalid_arg csum_type
-let verify_checksum csum filename +let verify_checksum csum ?(tar) filename
let prog, csum_ref match csum with
| SHA1 c -> "sha1sum", c
@@ -53,7 +53,14 @@ let verify_checksum csum filename | SHA512 c ->
"sha512sum", c
in
- let cmd = sprintf "%s %s" prog (Filename.quote filename) in
+ let cmd + match tar with
+ | None ->
+ sprintf "%s %s" prog (Filename.quote filename)
+ | Some tar ->
+ sprintf "tar xOf %s %s | %s"
+ (Filename.quote tar) (Filename.quote filename) prog
+ in
let lines = external_command cmd in
match lines with
| [] ->
diff --git a/mllib/checksums.mli b/mllib/checksums.mli
index 0074837..2440324 100644
--- a/mllib/checksums.mli
+++ b/mllib/checksums.mli
@@ -29,7 +29,7 @@ val of_string : string -> string -> csum_t
Raise [Invalid_argument] if the checksum type is not known. *)
-val verify_checksum : csum_t -> string -> unit
+val verify_checksum : csum_t -> ?tar:string -> string -> unit
(** Verify the checksum of the file. *)
val verify_checksums : csum_t list -> string -> unit
--
2.10.1
Tomáš Golembiovský
2016-Nov-12 15:37 UTC
[Libguestfs] [PATCH v2 2/5] v2v: ova: don't detect compressed disks, read the OVF instead
The information whether the disk is gzip compressed or not is stored
in the OVF. There is no reason to do the detection.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
v2v/input_ova.ml | 36 ++++++++++++++++++++----------------
v2v/test-v2v-i-ova-gz.ovf | 2 +-
2 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index b283629..db884d9 100644
--- a/v2v/input_ova.ml
+++ b/v2v/input_ova.ml
@@ -275,25 +275,29 @@ object
| None -> error (f_"no href in ovf:File (id=%s)")
file_ref
| Some s -> s in
+ let expr = sprintf
"/ovf:Envelope/ovf:References/ovf:File[@ovf:id='%s']/@ovf:compression"
file_ref in
+ let compressed + match xpath_string expr with
+ | None | Some "identity" -> false
+ | Some "gzip" -> true
+ | Some s -> error (f_"unsupported comprression in OVF:
%s") s in
+
+ let filename = if compressed then (
+ let new_filename = tmpdir // String.random8 () ^ ".vmdk"
in
+ let cmd + sprintf "zcat %s > %s" (quote
ovf_folder // filename) (quote new_filename) in
+ if shell_command cmd <> 0 then
+ error (f_"error uncompressing %s, see earlier error
messages")
+ filename;
+ new_filename
+ )
+ else
+ ovf_folder // filename
+ in
+
(* Does the file exist and is it readable? *)
- let filename = ovf_folder // filename in
Unix.access filename [Unix.R_OK];
- (* The spec allows the file to be gzip-compressed, in which case
- * we must uncompress it into the tmpdir.
- *)
- let filename - if detect_file_type filename = `GZip then (
- let new_filename = tmpdir // String.random8 () ^
".vmdk" in
- let cmd - sprintf "zcat %s > %s"
(quote filename) (quote new_filename) in
- if shell_command cmd <> 0 then
- error (f_"error uncompressing %s, see earlier error
messages")
- filename;
- new_filename
- )
- else filename in
-
let disk = {
s_disk_id = i;
s_qemu_uri = filename;
diff --git a/v2v/test-v2v-i-ova-gz.ovf b/v2v/test-v2v-i-ova-gz.ovf
index e10ad2b..4a03e85 100644
--- a/v2v/test-v2v-i-ova-gz.ovf
+++ b/v2v/test-v2v-i-ova-gz.ovf
@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="UTF-8"?>
<Envelope vmw:buildId="build-1750787"
xmlns="http://schemas.dmtf.org/ovf/envelope/1"
xmlns:cim="http://schemas.dmtf.org/wbem/wscim/1/common"
xmlns:ovf="http://schemas.dmtf.org/ovf/envelope/1"
xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData"
xmlns:vmw="http://www.vmware.com/schema/ovf"
xmlns:vssd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
<References>
- <File ovf:href="disk1.vmdk.gz" ovf:id="file1"
ovf:size="7804077568"/>
+ <File ovf:href="disk1.vmdk.gz" ovf:id="file1"
ovf:size="7804077568" ovf:compression="gzip"/>
</References>
<DiskSection>
<Info>Virtual disk information</Info>
--
2.10.1
Tomáš Golembiovský
2016-Nov-12 15:37 UTC
[Libguestfs] [PATCH v2 3/5] v2v: ova: move the untar function
Move the untar function so it can be used later in the code.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
v2v/input_ova.ml | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index db884d9..f76fe82 100644
--- a/v2v/input_ova.ml
+++ b/v2v/input_ova.ml
@@ -38,6 +38,12 @@ object
method as_options = "-i ova " ^ ova
method source () +
+ let untar ?(format = "") file outdir + let cmd = [
"tar"; sprintf "-x%sf" format; file; "-C"; outdir
] in
+ if run_command cmd <> 0 then
+ error (f_"error unpacking %s, see earlier error messages")
ova in
+
(* Extract ova file. *)
let exploded (* The spec allows a directory to be specified as an
ova. This
@@ -61,11 +67,6 @@ object
tmpfile in
- let untar ?(format = "") file outdir - let cmd = [
"tar"; sprintf "-x%sf" format; file; "-C"; outdir
] in
- if run_command cmd <> 0 then
- error (f_"error unpacking %s, see earlier error
messages") ova in
-
match detect_file_type ova with
| `Tar ->
(* Normal ovas are tar file (not compressed). *)
--
2.10.1
Tomáš Golembiovský
2016-Nov-12 15:37 UTC
[Libguestfs] [PATCH v2 4/5] v2v: ova: don't extract files from OVA if it's not needed
We don't have to always extract all files from the OVA archive. The OVA,
as defined in the standard, is plain tar. We can work directly over the
tar archive if we use correct 'offset' and 'size' options when
defining
the backing file for QEMU.
This leads to improvements in speed and puts much lower requirement on
available disk space.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
v2v/input_ova.ml | 177 +++++++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 154 insertions(+), 23 deletions(-)
diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index f76fe82..7a3e27b 100644
--- a/v2v/input_ova.ml
+++ b/v2v/input_ova.ml
@@ -39,17 +39,23 @@ object
method source ()
- let untar ?(format = "") file outdir - let cmd = [
"tar"; sprintf "-x%sf" format; file; "-C"; outdir
] in
+ (* Untar part or all files from tar archive. If [path] is specified it is
+ * a path in the tar archive.
+ *)
+ let untar ?(format = "") ?(path) file outdir + let cmd +
[ "tar"; sprintf "-x%sf" format; file; "-C";
outdir ]
+ @ (match path with None -> [] | Some p -> [p])
+ in
if run_command cmd <> 0 then
error (f_"error unpacking %s, see earlier error messages")
ova in
(* Extract ova file. *)
- let exploded + let exploded, partial (* The spec allows a
directory to be specified as an ova. This
* is also pretty convenient.
*)
- if is_directory ova then ova
+ if is_directory ova then ova, false
else (
let uncompress_head zcat file let cmd = sprintf "%s
%s" zcat (quote file) in
@@ -67,11 +73,53 @@ object
tmpfile in
+ (* Untar only ovf and manifest from the archive *)
+ let untar_partial file outdir + let files +
external_command (sprintf "tar -tf %s" (Filename.quote file)) in
+ List.iter (fun f ->
+ if Filename.check_suffix f ".ovf" ||
+ Filename.check_suffix f ".mf" then
+ untar ~path:f file outdir
+ ) files in
+
+ let qemu_img_version () + let cmd = "qemu-img
--version" in
+ let lines = external_command cmd in
+ match lines with
+ | [] -> error ("'qemu-img --version' returned no
output")
+ | line :: _ ->
+ let rex = Str.regexp "qemu-img version
\\([0-9]+\\)\\.\\([0-9]+\\)" in
+ if Str.string_match rex line 0 then (
+ try
+ int_of_string (Str.matched_group 1 line),
+ int_of_string (Str.matched_group 2 line)
+ with Failure _ ->
+ warning (f_"warning: failed to read qemu-img version!
Line: %S")
+ line;
+ 0, 9
+ ) else (
+ warning (f_"warning: failed to read qemu-img version!
Line: %S")
+ line;
+ 0, 9
+ )
+ in
+
match detect_file_type ova with
| `Tar ->
(* Normal ovas are tar file (not compressed). *)
- untar ova tmpdir;
- tmpdir
+ let qmajor, qminor = qemu_img_version () in
+ if qmajor > 2 || (qmajor == 2 && qminor >= 8) then (
+ (* If QEMU is recent enough we don't have to extract
everything. We
+ * can access disks inside the tar archive.
+ *)
+ untar_partial ova tmpdir;
+ tmpdir, true
+ ) else (
+ untar ova tmpdir;
+ tmpdir, false
+ )
+
| `Zip ->
(* However, although not permitted by the spec, people ship
* zip files as ova too.
@@ -81,7 +129,7 @@ object
[ "-j"; "-d"; tmpdir; ova ] in
if run_command cmd <> 0 then
error (f_"error unpacking %s, see earlier error
messages") ova;
- tmpdir
+ tmpdir, false
| (`GZip|`XZ) as format ->
let zcat, tar_fmt match format with
@@ -94,7 +142,7 @@ object
(match tmpfiletype with
| `Tar ->
untar ~format:tar_fmt ova tmpdir;
- tmpdir
+ tmpdir, false
| `Zip | `GZip | `XZ | `Unknown ->
error (f_"%s: unsupported file format\n\nFormats which we
currently understand for '-i ova' are: tar (uncompressed, compress with
gzip or xz), zip") ova
)
@@ -135,6 +183,59 @@ object
loop [dir]
in
+ (* Find file in [tar] archive and return at which byte it starts and how
+ * long it is.
+ *)
+ let find_file_in_tar tar filename + let lines = external_command
(stringify_args [ "tar"; "tRvf"; tar ]) in
+ let rec loop lines + match lines with
+ | [] -> raise Not_found
+ | line :: lines -> (
+ (* Lines have the form:
+ * block <offset>: <perms> <owner>/<group>
<size> <mdate> <mtime> <file>
+ *)
+ let rex = Str.regexp "^block \\([0-9]+\\): \\([^ \\t]+\\) \\([^
\\t]+\\) +\\([^ \\t]+\\) \\([^ \\t]+\\) \\([^ \\t]+\\) \\(.+\\)$" in
+ if Str.string_match rex line 0 then (
+ let offset = Str.matched_group 1 line in
+ let size = Str.matched_group 4 line in
+ let fname = Str.matched_group 7 line in
+
+ let offset + try Int64.of_string offset
+ with Failure _ ->
+ error (f_"invalid offset returned by tar: %S") offset
in
+
+ let size + try Int64.of_string size
+ with Failure _ ->
+ error (f_"invalid size returned by tar: %S") size in
+
+ if fname = filename then
+ (* Note: Offset is actualy block number and there is a single
+ * block with tar header at the beginning of the file. So skip
+ * the header and convert the block number to bytes before
+ * returning.
+ *)
+ (offset +^ 1L) *^ 512L, size
+ else loop lines
+ ) else
+ error (f_"failed to parse line returned by tar: %S") line
+ )
+ in
+ loop lines
+ in
+
+ let subfolder folder parent + if String.is_prefix folder (parent //
"") then
+ let len = String.length parent in
+ String.sub folder (len+1) (String.length folder-len-1)
+ else if folder = parent then
+ ""
+ else
+ assert false
+ in
+
(* Search for the ovf file. *)
let ovf = find_files exploded ".ovf" in
let ovf @@ -152,6 +253,7 @@ object
fun mf ->
debug "processing manifest %s" mf;
let mf_folder = Filename.dirname mf in
+ let mf_subfolder = subfolder mf_folder exploded in
let chan = open_in mf in
let rec loop () let line = input_line chan in
@@ -160,7 +262,11 @@ object
let disk = Str.matched_group 2 line in
let expected = Str.matched_group 3 line in
let csum = Checksums.of_string mode expected in
- try Checksums.verify_checksum csum (mf_folder // disk)
+ try
+ if partial then
+ Checksums.verify_checksum csum ~tar:ova (mf_subfolder // disk)
+ else
+ Checksums.verify_checksum csum (mf_folder // disk)
with Checksums.Mismatched_checksum (_, actual) ->
error (f_"checksum of disk %s does not match manifest %s
(actual %s(%s) = %s, expected %s(%s) = %s)")
disk mf mode disk actual mode disk expected;
@@ -283,25 +389,50 @@ object
| Some "gzip" -> true
| Some s -> error (f_"unsupported comprression in OVF:
%s") s in
- let filename = if compressed then (
- let new_filename = tmpdir // String.random8 () ^ ".vmdk"
in
- let cmd - sprintf "zcat %s > %s" (quote
ovf_folder // filename) (quote new_filename) in
- if shell_command cmd <> 0 then
- error (f_"error uncompressing %s, see earlier error
messages")
- filename;
- new_filename
- )
- else
- ovf_folder // filename
+ let filename, partial + if compressed then (
+ if partial then
+ untar ~path:((subfolder ovf_folder exploded) // filename)
+ ova tmpdir;
+ let new_filename = tmpdir // String.random8 () ^
".vmdk" in
+ let cmd + sprintf "zcat %s > %s"
(quote ovf_folder // filename) (quote new_filename) in
+ if shell_command cmd <> 0 then
+ error (f_"error uncompressing %s, see earlier error
messages")
+ filename;
+ new_filename, false
+ )
+ else if partial then
+ (subfolder ovf_folder exploded) // filename, partial
+ else
+ ovf_folder // filename, partial
in
- (* Does the file exist and is it readable? *)
- Unix.access filename [Unix.R_OK];
+ let qemu_uri + if not partial then (
+ (* Does the file exist and is it readable? *)
+ Unix.access filename [Unix.R_OK];
+ filename
+ )
+ else (
+ let offset, size + try find_file_in_tar ova
filename
+ with Not_found ->
+ error (f_"file '%s' not found in the ova")
filename
+ in
+ (* QEMU requires size aligned to 512 bytes. This is safe because
+ * tar also works with 512 byte blocks.
+ *)
+ let size = roundup64 size 512L in
+ sprintf
+ "json:{ \"file\": {
\"driver\":\"raw\", \"offset\":\"%Ld\",
\"size\":\"%Ld\", \"file\": {
\"filename\":\"%s\" } } }"
+ offset size ova
+ )
+ in
let disk = {
s_disk_id = i;
- s_qemu_uri = filename;
+ s_qemu_uri = qemu_uri;
s_format = Some "vmdk";
s_controller = controller;
} in
--
2.10.1
Tomáš Golembiovský
2016-Nov-12 15:37 UTC
[Libguestfs] [PATCH v2 5/5] v2v: update tests to match changes in OVA import
The virt-v2v behaviour for OVA input now depends on QEMU version
available. The tests affected by this now have two *.expect files and
the expected result now also depends on the QEMU used.
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
test-data/utils.sh | 21 +++++
v2v/Makefile.am | 1 +
v2v/test-v2v-i-ova-formats.sh | 5 +-
v2v/test-v2v-i-ova-subfolders.expected2 | 18 +++++
v2v/test-v2v-i-ova-subfolders.sh | 12 ++-
v2v/test-v2v-i-ova-tar.expected | 18 +++++
v2v/test-v2v-i-ova-tar.expected2 | 18 +++++
v2v/test-v2v-i-ova-tar.ovf | 138 ++++++++++++++++++++++++++++++++
v2v/test-v2v-i-ova-tar.sh | 70 ++++++++++++++++
v2v/test-v2v-i-ova-two-disks.expected2 | 19 +++++
v2v/test-v2v-i-ova-two-disks.sh | 12 ++-
11 files changed, 322 insertions(+), 10 deletions(-)
create mode 100755 test-data/utils.sh
create mode 100644 v2v/test-v2v-i-ova-subfolders.expected2
create mode 100644 v2v/test-v2v-i-ova-tar.expected
create mode 100644 v2v/test-v2v-i-ova-tar.expected2
create mode 100644 v2v/test-v2v-i-ova-tar.ovf
create mode 100755 v2v/test-v2v-i-ova-tar.sh
create mode 100644 v2v/test-v2v-i-ova-two-disks.expected2
diff --git a/test-data/utils.sh b/test-data/utils.sh
new file mode 100755
index 0000000..49e173f
--- /dev/null
+++ b/test-data/utils.sh
@@ -0,0 +1,21 @@
+#!/bin/bash -
+
+# Returns 0 if QEMU version is greater or equal to the arguments
+qemu_version() {
+ if [ $# -ne 2 ] ; then
+ echo "Usage: $0 <major_version> <minor_version>"
>&2
+ return 3
+ fi
+
+ QV=$(expr match "$(qemu-img --version)" 'qemu-img version
\([0-9]\+\.[0-9]\+\)')
+ [ -z "$QV" ] && return 2
+
+ QMAJ=$(echo "$QV" | cut -d. -f1)
+ QMIN=$(echo "$QV" | cut -d. -f2)
+
+ if [ \( $QMAJ -gt $1 \) -o \( $QMAJ -eq $1 -a $QMIN -ge $2 \) ] ; then
+ return 0
+ fi
+
+ return 1
+}
diff --git a/v2v/Makefile.am b/v2v/Makefile.am
index 5e3c3eb..621ba10 100644
--- a/v2v/Makefile.am
+++ b/v2v/Makefile.am
@@ -258,6 +258,7 @@ TESTS_ENVIRONMENT = $(top_builddir)/run --test
TESTS = \
test-v2v-docs.sh \
+ test-v2v-i-ova-tar.sh \
test-v2v-i-ova-formats.sh \
test-v2v-i-ova-gz.sh \
test-v2v-i-ova-subfolders.sh \
diff --git a/v2v/test-v2v-i-ova-formats.sh b/v2v/test-v2v-i-ova-formats.sh
index d113994..ba51440 100755
--- a/v2v/test-v2v-i-ova-formats.sh
+++ b/v2v/test-v2v-i-ova-formats.sh
@@ -22,7 +22,7 @@ unset CDPATH
export LANG=C
set -e
-formats="tar zip tar-gz tar-xz"
+formats="zip tar-gz tar-xz"
if [ -n "$SKIP_TEST_V2V_I_OVA_FORMATS_SH" ]; then
echo "$0: test skipped because environment variable is set"
@@ -62,9 +62,6 @@ echo -e "SHA1(disk1.vmdk)= $sha\r" > disk1.mf
for format in $formats; do
case "$format" in
- tar)
- tar -cf test-$format.ova ../test-v2v-i-ova-formats.ovf disk1.vmdk
disk1.mf
- ;;
zip)
zip -r test ../test-v2v-i-ova-formats.ovf disk1.vmdk disk1.mf
mv test.zip test-$format.ova
diff --git a/v2v/test-v2v-i-ova-subfolders.expected2
b/v2v/test-v2v-i-ova-subfolders.expected2
new file mode 100644
index 0000000..38aa3b1
--- /dev/null
+++ b/v2v/test-v2v-i-ova-subfolders.expected2
@@ -0,0 +1,18 @@
+Source guest information (--print-source option):
+
+ source name: 2K8R2EESP1_2_Medium
+hypervisor type: vmware
+ memory: 1073741824 (bytes)
+ nr vCPUs: 1
+ CPU features:
+ firmware: uefi
+ display:
+ video:
+ sound:
+disks:
+ json:{ "file": { "driver":"raw",
"offset":"2048", "size":"10240",
"file": {
"filename":"test-v2v-i-ova-subfolders.d/test.ova" } } }
(vmdk) [scsi]
+removable media:
+ CD-ROM [ide] in slot 0
+NICs:
+ Network "Network adapter 1"
+
diff --git a/v2v/test-v2v-i-ova-subfolders.sh b/v2v/test-v2v-i-ova-subfolders.sh
index c49f7cb..89a9a3e 100755
--- a/v2v/test-v2v-i-ova-subfolders.sh
+++ b/v2v/test-v2v-i-ova-subfolders.sh
@@ -35,6 +35,7 @@ fi
export VIRT_TOOLS_DATA_DIR="$srcdir/../test-data/fake-virt-tools"
. $srcdir/../test-data/guestfs-hashsums.sh
+. $srcdir/../test-data/utils.sh
d=test-v2v-i-ova-subfolders.d
rm -rf $d
@@ -56,10 +57,15 @@ popd
# normalize the output.
$VG virt-v2v --debug-gc --quiet \
-i ova $d/test.ova \
- --print-source |
-sed 's,[^ \t]*\(subfolder/disk.*\.vmdk\),\1,' > $d/source
+ --print-source > $d/source
# Check the parsed source is what we expect.
-diff -u test-v2v-i-ova-subfolders.expected $d/source
+if qemu_version 2 8 ; then
+ diff -u test-v2v-i-ova-subfolders.expected2 $d/source
+else
+ # normalize the output
+ sed -i -e 's,[^ \t]*\(subfolder/disk.*\.vmdk\),\1,' $d/source
+ diff -u test-v2v-i-ova-subfolders.expected $d/source
+fi
rm -rf $d
diff --git a/v2v/test-v2v-i-ova-tar.expected b/v2v/test-v2v-i-ova-tar.expected
new file mode 100644
index 0000000..7049aee
--- /dev/null
+++ b/v2v/test-v2v-i-ova-tar.expected
@@ -0,0 +1,18 @@
+Source guest information (--print-source option):
+
+ source name: 2K8R2EESP1_2_Medium
+hypervisor type: vmware
+ memory: 1073741824 (bytes)
+ nr vCPUs: 1
+ CPU features:
+ firmware: uefi
+ display:
+ video:
+ sound:
+disks:
+ disk1.vmdk (vmdk) [scsi]
+removable media:
+ CD-ROM [ide] in slot 0
+NICs:
+ Network "Network adapter 1"
+
diff --git a/v2v/test-v2v-i-ova-tar.expected2 b/v2v/test-v2v-i-ova-tar.expected2
new file mode 100644
index 0000000..f63a399
--- /dev/null
+++ b/v2v/test-v2v-i-ova-tar.expected2
@@ -0,0 +1,18 @@
+Source guest information (--print-source option):
+
+ source name: 2K8R2EESP1_2_Medium
+hypervisor type: vmware
+ memory: 1073741824 (bytes)
+ nr vCPUs: 1
+ CPU features:
+ firmware: uefi
+ display:
+ video:
+ sound:
+disks:
+ json:{ "file": { "driver":"raw",
"offset":"9216", "size":"10240",
"file": {
"filename":"test-v2v-i-ova-tar.d/test-tar.ova" } } } (vmdk)
[scsi]
+removable media:
+ CD-ROM [ide] in slot 0
+NICs:
+ Network "Network adapter 1"
+
diff --git a/v2v/test-v2v-i-ova-tar.ovf b/v2v/test-v2v-i-ova-tar.ovf
new file mode 100644
index 0000000..4827c7e
--- /dev/null
+++ b/v2v/test-v2v-i-ova-tar.ovf
@@ -0,0 +1,138 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Envelope vmw:buildId="build-1750787"
xmlns="http://schemas.dmtf.org/ovf/envelope/1"
xmlns:cim="http://schemas.dmtf.org/wbem/wscim/1/common"
xmlns:ovf="http://schemas.dmtf.org/ovf/envelope/1"
xmlns:rasd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData"
xmlns:vmw="http://www.vmware.com/schema/ovf"
xmlns:vssd="http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
+ <References>
+ <File ovf:href="disk1.vmdk" ovf:id="file1"
ovf:size="7804077568"/>
+ </References>
+ <DiskSection>
+ <Info>Virtual disk information</Info>
+ <Disk ovf:capacity="50" ovf:capacityAllocationUnits="byte
* 2^30" ovf:diskId="vmdisk1" ovf:fileRef="file1"
ovf:format="http://www.vmware.com/interfaces/specifications/vmdk.html#streamOptimized"
ovf:populatedSize="18975752192"/>
+ </DiskSection>
+ <NetworkSection>
+ <Info>The list of logical networks</Info>
+ <Network ovf:name="PG-VLAN60">
+ <Description>The PG-VLAN60 network</Description>
+ </Network>
+ </NetworkSection>
+ <VirtualSystem ovf:id="2K8R2EESP1_2_Medium">
+ <Info>A virtual machine</Info>
+ <Name>2K8R2EESP1_2_Medium</Name>
+ <OperatingSystemSection ovf:id="103"
vmw:osType="windows7Server64Guest">
+ <Info>The kind of installed guest operating system</Info>
+ <Description>Microsoft Windows Server 2008 R2
(64-bit)</Description>
+ </OperatingSystemSection>
+ <VirtualHardwareSection>
+ <Info>Virtual hardware requirements</Info>
+ <System>
+ <vssd:ElementName>Virtual Hardware
Family</vssd:ElementName>
+ <vssd:InstanceID>0</vssd:InstanceID>
+
<vssd:VirtualSystemIdentifier>2K8R2EESP1_2_Medium</vssd:VirtualSystemIdentifier>
+ <vssd:VirtualSystemType>vmx-10</vssd:VirtualSystemType>
+ </System>
+ <Item>
+ <rasd:AllocationUnits>hertz * 10^6</rasd:AllocationUnits>
+ <rasd:Description>Number of Virtual CPUs</rasd:Description>
+ <rasd:ElementName>1 virtual CPU(s)</rasd:ElementName>
+ <rasd:InstanceID>1</rasd:InstanceID>
+ <rasd:ResourceType>3</rasd:ResourceType>
+ <rasd:VirtualQuantity>1</rasd:VirtualQuantity>
+ </Item>
+ <Item>
+ <rasd:AllocationUnits>byte * 2^20</rasd:AllocationUnits>
+ <rasd:Description>Memory Size</rasd:Description>
+ <rasd:ElementName>1024MB of memory</rasd:ElementName>
+ <rasd:InstanceID>2</rasd:InstanceID>
+ <rasd:ResourceType>4</rasd:ResourceType>
+ <rasd:VirtualQuantity>1024</rasd:VirtualQuantity>
+ </Item>
+ <Item>
+ <rasd:Address>0</rasd:Address>
+ <rasd:Description>SCSI Controller</rasd:Description>
+ <rasd:ElementName>SCSI controller 0</rasd:ElementName>
+ <rasd:InstanceID>3</rasd:InstanceID>
+ <rasd:ResourceSubType>lsilogicsas</rasd:ResourceSubType>
+ <rasd:ResourceType>6</rasd:ResourceType>
+ <vmw:Config ovf:required="false"
vmw:key="slotInfo.pciSlotNumber" vmw:value="160"/>
+ </Item>
+ <Item>
+ <rasd:Address>1</rasd:Address>
+ <rasd:Description>IDE Controller</rasd:Description>
+ <rasd:ElementName>IDE 1</rasd:ElementName>
+ <rasd:InstanceID>4</rasd:InstanceID>
+ <rasd:ResourceType>5</rasd:ResourceType>
+ </Item>
+ <Item>
+ <rasd:Address>0</rasd:Address>
+ <rasd:Description>IDE Controller</rasd:Description>
+ <rasd:ElementName>IDE 0</rasd:ElementName>
+ <rasd:InstanceID>5</rasd:InstanceID>
+ <rasd:ResourceType>5</rasd:ResourceType>
+ </Item>
+ <Item ovf:required="false">
+ <rasd:AutomaticAllocation>false</rasd:AutomaticAllocation>
+ <rasd:ElementName>Video card</rasd:ElementName>
+ <rasd:InstanceID>6</rasd:InstanceID>
+ <rasd:ResourceType>24</rasd:ResourceType>
+ <vmw:Config ovf:required="false"
vmw:key="enable3DSupport" vmw:value="false"/>
+ <vmw:Config ovf:required="false"
vmw:key="use3dRenderer" vmw:value="automatic"/>
+ <vmw:Config ovf:required="false"
vmw:key="useAutoDetect" vmw:value="true"/>
+ <vmw:Config ovf:required="false"
vmw:key="videoRamSizeInKB" vmw:value="4096"/>
+ </Item>
+ <Item ovf:required="false">
+ <rasd:AutomaticAllocation>false</rasd:AutomaticAllocation>
+ <rasd:ElementName>VMCI device</rasd:ElementName>
+ <rasd:InstanceID>7</rasd:InstanceID>
+ <rasd:ResourceSubType>vmware.vmci</rasd:ResourceSubType>
+ <rasd:ResourceType>1</rasd:ResourceType>
+ <vmw:Config ovf:required="false"
vmw:key="allowUnrestrictedCommunication"
vmw:value="false"/>
+ <vmw:Config ovf:required="false"
vmw:key="slotInfo.pciSlotNumber" vmw:value="32"/>
+ </Item>
+ <Item ovf:required="false">
+ <rasd:AddressOnParent>0</rasd:AddressOnParent>
+ <rasd:AutomaticAllocation>false</rasd:AutomaticAllocation>
+ <rasd:ElementName>CD/DVD drive 1</rasd:ElementName>
+ <rasd:InstanceID>8</rasd:InstanceID>
+ <rasd:Parent>4</rasd:Parent>
+
<rasd:ResourceSubType>vmware.cdrom.atapi</rasd:ResourceSubType>
+ <rasd:ResourceType>15</rasd:ResourceType>
+ </Item>
+ <Item>
+ <rasd:AddressOnParent>0</rasd:AddressOnParent>
+ <rasd:ElementName>Hard disk 1</rasd:ElementName>
+ <rasd:HostResource>ovf:/disk/vmdisk1</rasd:HostResource>
+ <rasd:InstanceID>9</rasd:InstanceID>
+ <rasd:Parent>3</rasd:Parent>
+ <rasd:ResourceType>17</rasd:ResourceType>
+ <vmw:Config ovf:required="false"
vmw:key="backing.writeThrough" vmw:value="false"/>
+ </Item>
+ <Item>
+ <rasd:AddressOnParent>7</rasd:AddressOnParent>
+ <rasd:AutomaticAllocation>true</rasd:AutomaticAllocation>
+ <rasd:Connection>PG-VLAN60</rasd:Connection>
+ <rasd:Description>E1000 ethernet adapter on
"PG-VLAN60"</rasd:Description>
+ <rasd:ElementName>Network adapter 1</rasd:ElementName>
+ <rasd:InstanceID>11</rasd:InstanceID>
+ <rasd:ResourceSubType>E1000</rasd:ResourceSubType>
+ <rasd:ResourceType>10</rasd:ResourceType>
+ <vmw:Config ovf:required="false"
vmw:key="slotInfo.pciSlotNumber" vmw:value="33"/>
+ <vmw:Config ovf:required="false"
vmw:key="wakeOnLanEnabled" vmw:value="true"/>
+ </Item>
+ <vmw:Config ovf:required="false"
vmw:key="cpuHotAddEnabled" vmw:value="false"/>
+ <vmw:Config ovf:required="false"
vmw:key="cpuHotRemoveEnabled" vmw:value="false"/>
+ <vmw:Config ovf:required="false"
vmw:key="firmware" vmw:value="efi"/>
+ <vmw:Config ovf:required="false"
vmw:key="virtualICH7MPresent" vmw:value="false"/>
+ <vmw:Config ovf:required="false"
vmw:key="virtualSMCPresent" vmw:value="false"/>
+ <vmw:Config ovf:required="false"
vmw:key="memoryHotAddEnabled" vmw:value="false"/>
+ <vmw:Config ovf:required="false"
vmw:key="nestedHVEnabled" vmw:value="false"/>
+ <vmw:Config ovf:required="false"
vmw:key="powerOpInfo.powerOffType" vmw:value="soft"/>
+ <vmw:Config ovf:required="false"
vmw:key="powerOpInfo.resetType" vmw:value="soft"/>
+ <vmw:Config ovf:required="false"
vmw:key="powerOpInfo.standbyAction"
vmw:value="checkpoint"/>
+ <vmw:Config ovf:required="false"
vmw:key="powerOpInfo.suspendType" vmw:value="hard"/>
+ <vmw:Config ovf:required="false"
vmw:key="tools.afterPowerOn" vmw:value="true"/>
+ <vmw:Config ovf:required="false"
vmw:key="tools.afterResume" vmw:value="true"/>
+ <vmw:Config ovf:required="false"
vmw:key="tools.beforeGuestShutdown" vmw:value="true"/>
+ <vmw:Config ovf:required="false"
vmw:key="tools.beforeGuestStandby" vmw:value="true"/>
+ <vmw:Config ovf:required="false"
vmw:key="tools.syncTimeWithHost" vmw:value="false"/>
+ <vmw:Config ovf:required="false"
vmw:key="tools.toolsUpgradePolicy"
vmw:value="upgradeAtPowerCycle"/>
+ </VirtualHardwareSection>
+ </VirtualSystem>
+</Envelope>
diff --git a/v2v/test-v2v-i-ova-tar.sh b/v2v/test-v2v-i-ova-tar.sh
new file mode 100755
index 0000000..49f7a4b
--- /dev/null
+++ b/v2v/test-v2v-i-ova-tar.sh
@@ -0,0 +1,70 @@
+#!/bin/bash -
+# libguestfs virt-v2v test script
+# Copyright (C) 2014-2016 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 with ova file compressed in different ways
+
+unset CDPATH
+export LANG=C
+set -e
+
+if [ -n "$SKIP_TEST_V2V_I_OVA_FORMATS_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
+
+export VIRT_TOOLS_DATA_DIR="$srcdir/../test-data/fake-virt-tools"
+
+. $srcdir/../test-data/guestfs-hashsums.sh
+. $srcdir/../test-data/utils.sh
+
+d=test-v2v-i-ova-tar.d
+rm -rf $d
+mkdir $d
+
+pushd $d
+
+# Create a phony OVA. This is only a test of source parsing, not
+# conversion, so the contents of the disks doesn't matter.
+truncate -s 10k disk1.vmdk
+sha=`do_sha1 disk1.vmdk`
+echo -e "SHA1(disk1.vmdk)= $sha\r" > disk1.mf
+tar -cf test-tar.ova ../test-v2v-i-ova-tar.ovf disk1.vmdk disk1.mf
+
+popd
+
+# Run virt-v2v but only as far as the --print-source stage
+$VG virt-v2v --debug-gc --quiet \
+ -i ova $d/test-tar.ova \
+ --print-source > $d/source
+
+# Check the parsed source is what we expect.
+if qemu_version 2 8 ; then
+ diff -u test-v2v-i-ova-tar.expected2 $d/source
+else
+ # normalize the output
+ sed -i -e 's,[^ \t]*\(disk.*.vmdk\),\1,' $d/source
+ diff -u test-v2v-i-ova-tar.expected $d/source
+fi
+
+
+rm -rf $d
diff --git a/v2v/test-v2v-i-ova-two-disks.expected2
b/v2v/test-v2v-i-ova-two-disks.expected2
new file mode 100644
index 0000000..57d6240
--- /dev/null
+++ b/v2v/test-v2v-i-ova-two-disks.expected2
@@ -0,0 +1,19 @@
+Source guest information (--print-source option):
+
+ source name: 2K8R2EESP1_2_Medium
+hypervisor type: vmware
+ memory: 1073741824 (bytes)
+ nr vCPUs: 1
+ CPU features:
+ firmware: bios
+ display:
+ video:
+ sound:
+disks:
+ json:{ "file": { "driver":"raw",
"offset":"9728", "size":"10240",
"file": {
"filename":"test-v2v-i-ova-two-disks.d/test.ova" } } }
(vmdk) [scsi]
+ json:{ "file": { "driver":"raw",
"offset":"21504", "size":"102400",
"file": {
"filename":"test-v2v-i-ova-two-disks.d/test.ova" } } }
(vmdk) [scsi]
+removable media:
+ CD-ROM [ide] in slot 0
+NICs:
+ Network "Network adapter 1"
+
diff --git a/v2v/test-v2v-i-ova-two-disks.sh b/v2v/test-v2v-i-ova-two-disks.sh
index aefd90e..8a4c877 100755
--- a/v2v/test-v2v-i-ova-two-disks.sh
+++ b/v2v/test-v2v-i-ova-two-disks.sh
@@ -36,6 +36,7 @@ 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
+. $srcdir/../test-data/utils.sh
d=test-v2v-i-ova-two-disks.d
rm -rf $d
@@ -59,10 +60,15 @@ popd
# normalize the output.
$VG virt-v2v --debug-gc --quiet \
-i ova $d/test.ova \
- --print-source |
-sed 's,[^ \t]*\(disk.*.vmdk\),\1,' > $d/source
+ --print-source > $d/source
# Check the parsed source is what we expect.
-diff -u test-v2v-i-ova-two-disks.expected $d/source
+if qemu_version 2 8 ; then
+ diff -u test-v2v-i-ova-two-disks.expected2 $d/source
+else
+ # normalize the output
+ sed -i -e 's,[^ \t]*\(disk.*.vmdk\),\1,' $d/source
+ diff -u test-v2v-i-ova-two-disks.expected $d/source
+fi
rm -rf $d
--
2.10.1
Pino Toscano
2016-Nov-21 14:17 UTC
Re: [Libguestfs] [PATCH v2 1/5] mllib: compute checksum of file inside tar
On Saturday, 12 November 2016 16:37:49 CET Tomáš Golembiovský wrote:> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> > --- > mllib/checksums.ml | 11 +++++++++-- > mllib/checksums.mli | 2 +- > 2 files changed, 10 insertions(+), 3 deletions(-) > > diff --git a/mllib/checksums.ml b/mllib/checksums.ml > index dfa8c3a..0907499 100644 > --- a/mllib/checksums.ml > +++ b/mllib/checksums.ml > @@ -45,7 +45,7 @@ let of_string csum_type csum_value > | "sha512" -> SHA512 csum_value > | _ -> invalid_arg csum_type > > -let verify_checksum csum filename > +let verify_checksum csum ?(tar) filenameRound parenthesis are not needed here, ?tar works fine as well.> let prog, csum_ref > match csum with > | SHA1 c -> "sha1sum", c > @@ -53,7 +53,14 @@ let verify_checksum csum filename > | SHA512 c -> "sha512sum", c > in > > - let cmd = sprintf "%s %s" prog (Filename.quote filename) in > + let cmd > + match tar with > + | None -> > + sprintf "%s %s" prog (Filename.quote filename) > + | Some tar -> > + sprintf "tar xOf %s %s | %s" > + (Filename.quote tar) (Filename.quote filename) progThis assumes the tar is uncompressed -- not a big issue, but I'd note that down in the API doc in the .mli. With the two small issues fixed, this LGTM. Thanks, -- Pino Toscano
Pino Toscano
2016-Nov-21 14:29 UTC
Re: [Libguestfs] [PATCH v2 2/5] v2v: ova: don't detect compressed disks, read the OVF instead
On Saturday, 12 November 2016 16:37:50 CET Tomáš Golembiovský wrote:> The information whether the disk is gzip compressed or not is stored > in the OVF. There is no reason to do the detection. > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> > --- > v2v/input_ova.ml | 36 ++++++++++++++++++++---------------- > v2v/test-v2v-i-ova-gz.ovf | 2 +- > 2 files changed, 21 insertions(+), 17 deletions(-) > > diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml > index b283629..db884d9 100644 > --- a/v2v/input_ova.ml > +++ b/v2v/input_ova.ml > @@ -275,25 +275,29 @@ object > | None -> error (f_"no href in ovf:File (id=%s)") file_ref > | Some s -> s in > > + let expr = sprintf "/ovf:Envelope/ovf:References/ovf:File[@ovf:id='%s']/@ovf:compression" file_ref in > + let compressed > + match xpath_string expr with > + | None | Some "identity" -> false > + | Some "gzip" -> true > + | Some s -> error (f_"unsupported comprression in OVF: %s") s inTypo here, "compression". Also, the two "expr" and "compressed" let could be isolated in the "let filename" block.> + > + let filename = if compressed then ( > + let new_filename = tmpdir // String.random8 () ^ ".vmdk" in > + let cmd > + sprintf "zcat %s > %s" (quote ovf_folder // filename) (quote new_filename) in > + if shell_command cmd <> 0 then > + error (f_"error uncompressing %s, see earlier error messages") > + filename; > + new_filename > + ) > + else > + ovf_folder // filename > + inIMHO this code should be where the current code is: Unix.access is currently done on the original file, while with your code it would be done only on the uncompressed file (either the uncompressed original, or the decompressed one). IMHO it is always better to do the existency check on our side, and assume that the uncompressed file will exist if zcat didn't fail. Thanks, -- Pino Toscano
Pino Toscano
2016-Nov-21 15:21 UTC
Re: [Libguestfs] [PATCH v2 4/5] v2v: ova: don't extract files from OVA if it's not needed
On Saturday, 12 November 2016 16:37:52 CET Tomáš Golembiovský wrote:> We don't have to always extract all files from the OVA archive. The OVA, > as defined in the standard, is plain tar. We can work directly over the > tar archive if we use correct 'offset' and 'size' options when defining > the backing file for QEMU. > > This leads to improvements in speed and puts much lower requirement on > available disk space. > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> > --- > v2v/input_ova.ml | 177 +++++++++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 154 insertions(+), 23 deletions(-) > > diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml > index f76fe82..7a3e27b 100644 > --- a/v2v/input_ova.ml > +++ b/v2v/input_ova.ml > @@ -39,17 +39,23 @@ object > > method source () > > - let untar ?(format = "") file outdir > - let cmd = [ "tar"; sprintf "-x%sf" format; file; "-C"; outdir ] in > + (* Untar part or all files from tar archive. If [path] is specified it is > + * a path in the tar archive. > + *) > + let untar ?(format = "") ?(path) file outdirNo need for parenthesis around "path". Also, tar supports extracting more files at the same time, so I'd change "path" into "paths", and just call this once later on ...> + let cmd > + [ "tar"; sprintf "-x%sf" format; file; "-C"; outdir ] > + @ (match path with None -> [] | Some p -> [p]) > + in > if run_command cmd <> 0 then > error (f_"error unpacking %s, see earlier error messages") ova in > > (* Extract ova file. *) > - let exploded > + let exploded, partial > (* The spec allows a directory to be specified as an ova. This > * is also pretty convenient. > *) > - if is_directory ova then ova > + if is_directory ova then ova, false > else ( > let uncompress_head zcat file > let cmd = sprintf "%s %s" zcat (quote file) in > @@ -67,11 +73,53 @@ object > > tmpfile in > > + (* Untar only ovf and manifest from the archive *) > + let untar_partial file outdirI'd rename this as "untar_metadata", as it seems a better fitting name. YMMV.> + let files > + external_command (sprintf "tar -tf %s" (Filename.quote file)) in > + List.iter (fun f -> > + if Filename.check_suffix f ".ovf" || > + Filename.check_suffix f ".mf" then > + untar ~path:f file outdir > + ) files in... i.e. here: this can be changed to: let files filter_map ( fun f -> if Filename.check_suffix f ".ovf" || Filename.check_suffix f ".mf" then Some f else None ) files in untar ~paths:files file outdir> + let qemu_img_version () > + let cmd = "qemu-img --version" in > + let lines = external_command cmd in > + match lines with > + | [] -> error ("'qemu-img --version' returned no output") > + | line :: _ -> > + let rex = Str.regexp "qemu-img version \\([0-9]+\\)\\.\\([0-9]+\\)" in > + if Str.string_match rex line 0 then ( > + try > + int_of_string (Str.matched_group 1 line), > + int_of_string (Str.matched_group 2 line) > + with Failure _ -> > + warning (f_"warning: failed to read qemu-img version! Line: %S") > + line;"warning" is already printed by the warning function; also I'd simplify the message as: warning (f_"failed to parse the version of qemu-img (%S), assuming 0.9") line> + 0, 9 > + ) else ( > + warning (f_"warning: failed to read qemu-img version! Line: %S") > + line;Ditto.> + 0, 9 > + ) > + in > + > match detect_file_type ova with > | `Tar -> > (* Normal ovas are tar file (not compressed). *) > - untar ova tmpdir; > - tmpdir > + let qmajor, qminor = qemu_img_version () in > + if qmajor > 2 || (qmajor == 2 && qminor >= 8) then ( > + (* If QEMU is recent enough we don't have to extract everything. We > + * can access disks inside the tar archive. > + *) > + untar_partial ova tmpdir; > + tmpdir, true > + ) else ( > + untar ova tmpdir; > + tmpdir, false > + ) > + > | `Zip -> > (* However, although not permitted by the spec, people ship > * zip files as ova too. > @@ -81,7 +129,7 @@ object > [ "-j"; "-d"; tmpdir; ova ] in > if run_command cmd <> 0 then > error (f_"error unpacking %s, see earlier error messages") ova; > - tmpdir > + tmpdir, false > | (`GZip|`XZ) as format -> > let zcat, tar_fmt > match format with > @@ -94,7 +142,7 @@ object > (match tmpfiletype with > | `Tar -> > untar ~format:tar_fmt ova tmpdir; > - tmpdir > + tmpdir, false > | `Zip | `GZip | `XZ | `Unknown -> > error (f_"%s: unsupported file format\n\nFormats which we currently understand for '-i ova' are: tar (uncompressed, compress with gzip or xz), zip") ova > ) > @@ -135,6 +183,59 @@ object > loop [dir] > in > > + (* Find file in [tar] archive and return at which byte it starts and how > + * long it is. > + *) > + let find_file_in_tar tar filename > + let lines = external_command (stringify_args [ "tar"; "tRvf"; tar ]) instringify_args is meant to be used for displaying, rather than passing it back as shell command. Please just build a command string as done in the rest of the places.> + let rec loop lines > + match lines with > + | [] -> raise Not_found > + | line :: lines -> ( > + (* Lines have the form: > + * block <offset>: <perms> <owner>/<group> <size> <mdate> <mtime> <file> > + *) > + let rex = Str.regexp "^block \\([0-9]+\\): \\([^ \\t]+\\) \\([^ \\t]+\\) +\\([^ \\t]+\\) \\([^ \\t]+\\) \\([^ \\t]+\\) \\(.+\\)$" inWhoa how unreadable :/ The alternative would be using String.nsplit, but it would play badly with filenames with spaces inside the archive. Maybe a faster alternative could be: a) find the 6th empty space, and pick the substring of all the rest of the string after it b) filter the lines by picking only the wanted file, checking using (a) c) once a single line is found, use String.nsplit and parse only parts 1 (removing the ':' suffix) and 4 It feels slightly more complex, but this way we can avoid running the regex until we find the right filename.> + if Str.string_match rex line 0 then ( > + let offset = Str.matched_group 1 line in > + let size = Str.matched_group 4 line in > + let fname = Str.matched_group 7 line in > + > + let offset > + try Int64.of_string offset > + with Failure _ -> > + error (f_"invalid offset returned by tar: %S") offset in > + > + let size > + try Int64.of_string size > + with Failure _ -> > + error (f_"invalid size returned by tar: %S") size in > + > + if fname = filename then > + (* Note: Offset is actualy block number and there is a single > + * block with tar header at the beginning of the file. So skip > + * the header and convert the block number to bytes before > + * returning. > + *) > + (offset +^ 1L) *^ 512L, size > + else loop lines > + ) else > + error (f_"failed to parse line returned by tar: %S") line > + ) > + in > + loop lines > + in > + > + let subfolder folder parent > + if String.is_prefix folder (parent // "") then > + let len = String.length parent in > + String.sub folder (len+1) (String.length folder-len-1) > + else if folder = parent then > + ""I'd invert the two checks (first the = one, then is_prefix).> + else > + assert false > + in > + > (* Search for the ovf file. *) > let ovf = find_files exploded ".ovf" in > let ovf > @@ -152,6 +253,7 @@ object > fun mf -> > debug "processing manifest %s" mf; > let mf_folder = Filename.dirname mf in > + let mf_subfolder = subfolder mf_folder exploded in > let chan = open_in mf in > let rec loop () > let line = input_line chan in > @@ -160,7 +262,11 @@ object > let disk = Str.matched_group 2 line in > let expected = Str.matched_group 3 line in > let csum = Checksums.of_string mode expected in > - try Checksums.verify_checksum csum (mf_folder // disk) > + try > + if partial then > + Checksums.verify_checksum csum ~tar:ova (mf_subfolder // disk) > + else > + Checksums.verify_checksum csum (mf_folder // disk) > with Checksums.Mismatched_checksum (_, actual) -> > error (f_"checksum of disk %s does not match manifest %s (actual %s(%s) = %s, expected %s(%s) = %s)") > disk mf mode disk actual mode disk expected; > @@ -283,25 +389,50 @@ object > | Some "gzip" -> true > | Some s -> error (f_"unsupported comprression in OVF: %s") s in > > - let filename = if compressed then ( > - let new_filename = tmpdir // String.random8 () ^ ".vmdk" in > - let cmd > - sprintf "zcat %s > %s" (quote ovf_folder // filename) (quote new_filename) in > - if shell_command cmd <> 0 then > - error (f_"error uncompressing %s, see earlier error messages") > - filename; > - new_filename > - ) > - else > - ovf_folder // filename > + let filename, partial > + if compressed then ( > + if partial then > + untar ~path:((subfolder ovf_folder exploded) // filename) > + ova tmpdir; > + let new_filename = tmpdir // String.random8 () ^ ".vmdk" in > + let cmd > + sprintf "zcat %s > %s" (quote ovf_folder // filename) (quote new_filename) in > + if shell_command cmd <> 0 then > + error (f_"error uncompressing %s, see earlier error messages") > + filename;I guess parts of this block could be shared/factored out with the same bits used earlier. Thanks, -- Pino Toscano
Pino Toscano
2016-Nov-21 15:41 UTC
Re: [Libguestfs] [PATCH v2 5/5] v2v: update tests to match changes in OVA import
On Saturday, 12 November 2016 16:37:53 CET Tomáš Golembiovský wrote:> The virt-v2v behaviour for OVA input now depends on QEMU version > available. The tests affected by this now have two *.expect files and > the expected result now also depends on the QEMU used. > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> > ---This IMHO should be part of patch #4 as well, otherwise applying only patch #4 without this could lead to test failures (e.g. when bisecting).> test-data/utils.sh | 21 +++++qemu-utils.sh? Also, it must be added to EXTRA_DIST in test-data/Makefile.am.> v2v/Makefile.am | 1 + > v2v/test-v2v-i-ova-formats.sh | 5 +- > v2v/test-v2v-i-ova-subfolders.expected2 | 18 +++++ > v2v/test-v2v-i-ova-subfolders.sh | 12 ++- > v2v/test-v2v-i-ova-tar.expected | 18 +++++ > v2v/test-v2v-i-ova-tar.expected2 | 18 +++++ > v2v/test-v2v-i-ova-tar.ovf | 138 ++++++++++++++++++++++++++++++++The ovf and expected files are basically copies of the -formats versions -- would it be possible to reuse them?> diff --git a/test-data/utils.sh b/test-data/utils.sh > new file mode 100755 > index 0000000..49e173f > --- /dev/null > +++ b/test-data/utils.sh > @@ -0,0 +1,21 @@ > +#!/bin/bash - > + > +# Returns 0 if QEMU version is greater or equal to the arguments > +qemu_version() {This name would suggest it prints the version of qemu, while it really checks against a wanted version -- I'd suggest qemu_is_version, just like the simple tool we have for shell tests involving libvirt, libvirt-is-version (see src/libvirt-is-version.c).> diff --git a/v2v/test-v2v-i-ova-subfolders.sh b/v2v/test-v2v-i-ova-subfolders.sh > index c49f7cb..89a9a3e 100755 > --- a/v2v/test-v2v-i-ova-subfolders.sh > +++ b/v2v/test-v2v-i-ova-subfolders.sh > @@ -35,6 +35,7 @@ fi > export VIRT_TOOLS_DATA_DIR="$srcdir/../test-data/fake-virt-tools" > > . $srcdir/../test-data/guestfs-hashsums.sh > +. $srcdir/../test-data/utils.sh > > d=test-v2v-i-ova-subfolders.d > rm -rf $d > @@ -56,10 +57,15 @@ popd > # normalize the output. > $VG virt-v2v --debug-gc --quiet \ > -i ova $d/test.ova \ > - --print-source | > -sed 's,[^ \t]*\(subfolder/disk.*\.vmdk\),\1,' > $d/source > + --print-source > $d/source > > # Check the parsed source is what we expect. > -diff -u test-v2v-i-ova-subfolders.expected $d/source > +if qemu_version 2 8 ; thenHere I'd normalize the output, by removing "$d/" (i.e. the path of the subdirectory plus the trailing slash): sed -i -e "s,$d/,." $d/source> diff --git a/v2v/test-v2v-i-ova-tar.sh b/v2v/test-v2v-i-ova-tar.sh > new file mode 100755 > index 0000000..49f7a4b > --- /dev/null > +++ b/v2v/test-v2v-i-ova-tar.sh > @@ -0,0 +1,70 @@ > +#!/bin/bash - > +# libguestfs virt-v2v test script > +# Copyright (C) 2014-2016 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 with ova file compressed in different ways > + > +unset CDPATH > +export LANG=C > +set -e > + > +if [ -n "$SKIP_TEST_V2V_I_OVA_FORMATS_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 > + > +export VIRT_TOOLS_DATA_DIR="$srcdir/../test-data/fake-virt-tools" > + > +. $srcdir/../test-data/guestfs-hashsums.sh > +. $srcdir/../test-data/utils.sh > + > +d=test-v2v-i-ova-tar.d > +rm -rf $d > +mkdir $d > + > +pushd $d > + > +# Create a phony OVA. This is only a test of source parsing, not > +# conversion, so the contents of the disks doesn't matter. > +truncate -s 10k disk1.vmdk > +sha=`do_sha1 disk1.vmdk` > +echo -e "SHA1(disk1.vmdk)= $sha\r" > disk1.mf > +tar -cf test-tar.ova ../test-v2v-i-ova-tar.ovf disk1.vmdk disk1.mf > + > +popd > + > +# Run virt-v2v but only as far as the --print-source stage > +$VG virt-v2v --debug-gc --quiet \ > + -i ova $d/test-tar.ova \ > + --print-source > $d/source > + > +# Check the parsed source is what we expect. > +if qemu_version 2 8 ; thenDitto.> diff --git a/v2v/test-v2v-i-ova-two-disks.sh b/v2v/test-v2v-i-ova-two-disks.sh > index aefd90e..8a4c877 100755 > --- a/v2v/test-v2v-i-ova-two-disks.sh > +++ b/v2v/test-v2v-i-ova-two-disks.sh > @@ -36,6 +36,7 @@ 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 > +. $srcdir/../test-data/utils.sh > > d=test-v2v-i-ova-two-disks.d > rm -rf $d > @@ -59,10 +60,15 @@ popd > # normalize the output. > $VG virt-v2v --debug-gc --quiet \ > -i ova $d/test.ova \ > - --print-source | > -sed 's,[^ \t]*\(disk.*.vmdk\),\1,' > $d/source > + --print-source > $d/source > > # Check the parsed source is what we expect. > -diff -u test-v2v-i-ova-two-disks.expected $d/source > +if qemu_version 2 8 ; thenDitto. Thanks, -- Pino Toscano
Apparently Analagous Threads
- [PATCH 1/5] mllib: compute checksum of file inside tar
- [PATCH v3 1/6] mllib: compute checksum of file inside tar
- [PATCH v3 01/10] mllib: factorize code to add Checksum.get_checksum function
- [PATCH v2 1/7] mllib: factorize code to add Checksum.get_checksum function
- Re: [PATCH v2 1/7] mllib: factorize code to add Checksum.get_checksum function