Tomáš Golembiovský
2017-Jan-30 21:43 UTC
[Libguestfs] [PATCH v6 0/3] Import directly from OVA tar archive if possible
v6: - just rebase v5: - rebase, patches 1,3,5 were merged - 1/3: we still need to discuss whether to detect compressed discs - 2/3: - renamed argument noempty to keep_empty - tests were not run - 3/3: - using JSON module to generate JSON (as suggested by Pino) - all the other comments raised by Pino v4: - rebase to more recent master - 1/6: using just "quote" instead of "Filename.quote" - 2/6: reformated block of code according to Richards suggestion - 4/6: added tests for nsplit v3: Addressed Pino's comments, namely: - input_ova.ml - untar takes list of paths - renamed untar_partial to untar_metadata - replaced uggly regex with nsplit - tests - test changes are part of the main commit - renamed test-data/guestfs-hashsums.sh to test-data/test-utils.sh - renamed qemu_version to qemu_is_version and moved it to test-data/test-utils.sh - normalize paths in expect files 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 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. Tomáš Golembiovský (3): v2v: ova: don't detect compressed disks, read the OVF instead mllib: modify nsplit to take optional noempty and count arguments v2v: ova: don't extract files from OVA if it's not needed mllib/common_utils.ml | 13 ++- mllib/common_utils.mli | 12 +- mllib/common_utils_tests.ml | 30 +++++ test-data/test-utils.sh | 20 ++++ v2v/Makefile.am | 2 + v2v/input_ova.ml | 198 +++++++++++++++++++++++++++++--- 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 | 13 ++- 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 | 72 ++++++++++++ v2v/test-v2v-i-ova-two-disks.expected2 | 19 +++ v2v/test-v2v-i-ova-two-disks.sh | 13 ++- 16 files changed, 561 insertions(+), 30 deletions(-) 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.11.0
Tomáš Golembiovský
2017-Jan-30 21:43 UTC
[Libguestfs] [PATCH v6 1/3] 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 | 9 ++++++++- v2v/test-v2v-i-ova-gz.ovf | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml index 9b8d28aca..40f723633 100644 --- a/v2v/input_ova.ml +++ b/v2v/input_ova.ml @@ -276,6 +276,13 @@ 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 compression in OVF: %s") s in + (* Does the file exist and is it readable? *) let filename = ovf_folder // filename in Unix.access filename [Unix.R_OK]; @@ -284,7 +291,7 @@ object * we must uncompress it into the tmpdir. *) let filename - if detect_file_type filename = `GZip then ( + if compressed then ( let new_filename = tmpdir // String.random8 () ^ ".vmdk" in let cmd sprintf "zcat %s > %s" (quote filename) (quote new_filename) in diff --git a/v2v/test-v2v-i-ova-gz.ovf b/v2v/test-v2v-i-ova-gz.ovf index e10ad2bc5..4a03e85fb 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.11.0
Tomáš Golembiovský
2017-Jan-30 21:43 UTC
[Libguestfs] [PATCH v6 2/3] mllib: modify nsplit to take optional noempty and count arguments
Added two new optional arguments to nsplit: * keep_empty: if set to false empty elements are not stored in the returned list. The default is to keep the empty elements * count: specifies how many splits to perform; negative count (the default) means do as many splits as possible Added tests for nsplit. Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- mllib/common_utils.ml | 13 ++++++++++--- mllib/common_utils.mli | 12 ++++++++++-- mllib/common_utils_tests.ml | 30 ++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 5 deletions(-) diff --git a/mllib/common_utils.ml b/mllib/common_utils.ml index e9ae6a4a2..6c5df5e1c 100644 --- a/mllib/common_utils.ml +++ b/mllib/common_utils.ml @@ -130,15 +130,22 @@ module String = struct done; if not !r then s else Bytes.to_string b2 - let rec nsplit sep str + let rec nsplit ?(keep_empty = true) ?(count = -1) sep str let len = length str in let seplen = length sep in let i = find str sep in - if i = -1 then [str] + if i = -1 || count = 0 then + if str = "" && not keep_empty then [] else [str] else ( let s' = sub str 0 i in let s'' = sub str (i+seplen) (len-i-seplen) in - s' :: nsplit sep s'' + let elem, count + if s' = "" && not keep_empty then + [], count + else + [s'], if count > 0 then count-1 else count + in + elem @ nsplit ~keep_empty:keep_empty ~count:count sep s'' ) let split sep str diff --git a/mllib/common_utils.mli b/mllib/common_utils.mli index 722e528e5..5494c018c 100644 --- a/mllib/common_utils.mli +++ b/mllib/common_utils.mli @@ -80,9 +80,17 @@ module String : sig [str] with [s2]. *) val replace_char : string -> char -> char -> string (** Replace character in string. *) - val nsplit : string -> string -> string list + val nsplit : ?keep_empty:bool -> ?count:int -> string -> string -> string list (** [nsplit sep str] splits [str] into multiple strings at each - separator [sep]. *) + separator [sep]. + + If [keep_empty] is set to false empty elements not included in the + returned list. By default empty elements are kept. + + If [count] is specified it says how many splits to perform. I.e. the + returned array will have at most [count]+1 elements. Negative [count] + (the default) means do as many splits as possible. + *) val split : string -> string -> string * string (** [split sep str] splits [str] at the first occurrence of the separator [sep], returning the part before and the part after. diff --git a/mllib/common_utils_tests.ml b/mllib/common_utils_tests.ml index 77b0524c1..e30179323 100644 --- a/mllib/common_utils_tests.ml +++ b/mllib/common_utils_tests.ml @@ -109,6 +109,35 @@ let test_string_find ctx assert_equal_int (-1) (String.find "" "baz"); assert_equal_int (-1) (String.find "foobar" "baz") +(* Test Common_utils.String.nsplit *) +let test_string_nsplit ctx + (* Basic functionality *) + assert_equal_stringlist [""] (String.nsplit "|" ""); + assert_equal_stringlist ["A"] (String.nsplit "|" "A"); + assert_equal_stringlist ["A"; "B"] (String.nsplit "|" "A|B"); + assert_equal_stringlist ["A"; "B"; "C"] (String.nsplit "|" "A|B|C"); + assert_equal_stringlist ["A"; "B"; "C"; "D"] (String.nsplit "|" "A|B|C|D"); + assert_equal_stringlist [""; "A"; ""] (String.nsplit "|" "|A|"); + assert_equal_stringlist ["A"; ""; ""; "B"] (String.nsplit "|" "A|||B"); + assert_equal_stringlist ["A"; "B"; "C"; "D"] (String.nsplit "<>" "A<>B<>C<>D"); + + (* keep_empty option *) + assert_equal_stringlist [ ] (String.nsplit ~keep_empty:false "|" ""); + assert_equal_stringlist ["A"] (String.nsplit ~keep_empty:false "|" "|A|"); + assert_equal_stringlist ["A"; "B"] (String.nsplit ~keep_empty:false "|" "A|||B"); + assert_equal_stringlist [""; "A"; ""] (String.nsplit ~keep_empty:true "|" "|A|"); + assert_equal_stringlist ["A"; ""; ""; "B"] (String.nsplit ~keep_empty:true "|" "A|||B"); + + (* count option *) + assert_equal_stringlist ["A"; "B"; "C"; "D"] (String.nsplit ~count:(-1) "|" "A|B|C|D"); + assert_equal_stringlist ["A|B|C|D"] (String.nsplit ~count:0 "|" "A|B|C|D"); + assert_equal_stringlist ["A"; "B|C|D"] (String.nsplit ~count:1 "|" "A|B|C|D"); + assert_equal_stringlist ["A"; "B"; "C|D"] (String.nsplit ~count:2 "|" "A|B|C|D"); + assert_equal_stringlist ["A"; "B"; "C"; "D"] (String.nsplit ~count:10 "|" "A|B|C|D"); + + (* count and keep_empty together *) + assert_equal_stringlist ["A"; "B"; "|||C|D"] (String.nsplit ~keep_empty:false ~count:2 "|" "|||A||||B||||C|D") + (* Test Common_utils.String.lines_split. *) let test_string_lines_split ctx assert_equal_stringlist [""] (String.lines_split ""); @@ -135,6 +164,7 @@ let suite "strings.is_prefix" >:: test_string_is_prefix; "strings.is_suffix" >:: test_string_is_suffix; "strings.find" >:: test_string_find; + "strings.nsplit" >:: test_string_nsplit; "strings.lines_split" >:: test_string_lines_split; ] -- 2.11.0
Tomáš Golembiovský
2017-Jan-30 21:43 UTC
[Libguestfs] [PATCH v6 3/3] 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 puts much lower requirement on available disk space. Since the virt-v2v behaviour for OVA input now depends on QEMU version available this affects some of the tests. The affected tests will have two *.expect files and the expected result also has to depend on the QEMU used. Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- test-data/test-utils.sh | 20 ++++ v2v/Makefile.am | 2 + v2v/input_ova.ml | 189 +++++++++++++++++++++++++++++--- v2v/test-v2v-i-ova-formats.sh | 5 +- v2v/test-v2v-i-ova-subfolders.expected2 | 18 +++ v2v/test-v2v-i-ova-subfolders.sh | 13 ++- 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 | 72 ++++++++++++ v2v/test-v2v-i-ova-two-disks.expected2 | 19 ++++ v2v/test-v2v-i-ova-two-disks.sh | 13 ++- 12 files changed, 502 insertions(+), 23 deletions(-) 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/test-utils.sh b/test-data/test-utils.sh index 86a5aaf12..04e833308 100755 --- a/test-data/test-utils.sh +++ b/test-data/test-utils.sh @@ -54,3 +54,23 @@ do_sha256 () ;; esac } + +# Returns 0 if QEMU version is greater or equal to the arguments +qemu_is_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 9189aaf12..d62ac477e 100644 --- a/v2v/Makefile.am +++ b/v2v/Makefile.am @@ -260,6 +260,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 \ @@ -359,6 +360,7 @@ EXTRA_DIST += \ test-v2v-i-ova-subfolders.expected \ test-v2v-i-ova-subfolders.ovf \ test-v2v-i-ova-subfolders.sh \ + test-v2v-i-ova-tar.sh \ test-v2v-i-ova-two-disks.expected \ test-v2v-i-ova-two-disks.ovf \ test-v2v-i-ova-two-disks.sh \ diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml index 40f723633..01ba80686 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 [paths] is specified it is + * a list of paths in the tar archive. + *) + let untar ?(format = "") ?paths file outdir + let cmd + [ "tar"; sprintf "-x%sf" format; file; "-C"; outdir ] + @ (match paths 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,56 @@ object tmpfile in + (* Untar only ovf and manifest from the archive *) + let untar_metadata ova outdir + let files + external_command (sprintf "tar -tf %s" (Filename.quote ova)) in + 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 ova outdir in + + let qemu_img_version () + let lines = external_command "qemu-img --version" 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_"failed to parse qemu-img version(%S), assuming 0.9") + line; + 0, 9 + ) else ( + warning (f_"failed to read qemu-img version(%S), assuming 0.9") + 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_metadata 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 +132,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 +145,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 +186,68 @@ 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 (sprintf "tar tRvf %s" (Filename.quote 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 elems = String.nsplit ~keep_empty:false ~count:7 " " line in + if List.length elems = 8 && List.hd elems = "block" then ( + let elems = Array.of_list elems in + let offset = elems.(1) in + let size = elems.(4) in + let fname = elems.(7) in + + if fname <> filename then + loop lines + else ( + let offset + try + (* There should be a colon at the end *) + let i = String.rindex offset ':' in + if i == (String.length offset)-1 then + Int64.of_string (String.sub offset 0 i) + else + raise (Failure "colon at wrong position") + with Failure _ | Not_found -> + 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 + + (* 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 + error (f_"failed to parse line returned by tar: %S") line + ) + in + loop lines + in + + let subfolder folder parent + if folder = parent then + "" + else if String.is_prefix folder (parent // "") then + let len = String.length parent in + String.sub folder (len+1) (String.length folder-len-1) + else + assert false + in + (* Search for the ovf file. *) let ovf = find_files exploded ".ovf" in let ovf @@ -152,6 +265,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 +274,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,9 +401,25 @@ object | Some "gzip" -> true | Some s -> error (f_"unsupported compression in OVF: %s") s in - (* Does the file exist and is it readable? *) - let filename = ovf_folder // filename in - Unix.access filename [Unix.R_OK]; + let partial + if compressed && partial then ( + (* We cannot access compressed disk inside the tar; we have to + * extract it *) + untar ~paths:[(subfolder ovf_folder exploded) // filename] + ova tmpdir; + false + ) + else + partial in + + let filename + if partial then + (subfolder ovf_folder exploded) // filename + else ( + (* Does the file exist and is it readable? *) + Unix.access (ovf_folder // filename) [Unix.R_OK]; + ovf_folder // filename + ) in (* The spec allows the file to be gzip-compressed, in which case * we must uncompress it into the tmpdir. @@ -302,9 +436,38 @@ object ) else filename in + let qemu_uri + if not partial then ( + 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 + let doc = [ + "file", JSON.Dict [ + "driver", JSON.String "raw"; + "offset", JSON.Int64 offset; + "size", JSON.Int64 size; + "file", JSON.Dict [ + "filename", JSON.String ova] + ] + ] in + let x + sprintf "json:%s" (JSON.string_of_doc ~fmt:Compact doc) + in + (debug "json: %s" x; x) + ) in + let disk = { s_disk_id = i; - s_qemu_uri = filename; + s_qemu_uri = qemu_uri; s_format = Some "vmdk"; s_controller = controller; } in diff --git a/v2v/test-v2v-i-ova-formats.sh b/v2v/test-v2v-i-ova-formats.sh index bd3e30ee8..0eec600eb 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" @@ -63,9 +63,6 @@ cp ../test-v2v-i-ova-formats.ovf . 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 000000000..87996d202 --- /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.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 4fd1acea3..422959036 100755 --- a/v2v/test-v2v-i-ova-subfolders.sh +++ b/v2v/test-v2v-i-ova-subfolders.sh @@ -56,10 +56,17 @@ 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_is_version 2 8 ; then + # normalize the output + sed -i -e "s,\"$d/,\"," $d/source + 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 000000000..7049aeecc --- /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 000000000..1aa54dc37 --- /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-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 000000000..4827c7e9b --- /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 000000000..c3b0588f4 --- /dev/null +++ b/v2v/test-v2v-i-ova-tar.sh @@ -0,0 +1,72 @@ +#!/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/test-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 +cp ../test-v2v-i-ova-tar.ovf . +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_is_version 2 8 ; then + # normalize the output + sed -i -e "s,\"$d/,\"," $d/source + 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 000000000..b12ca1bd6 --- /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.ova" } } } (vmdk) [scsi] + json:{ "file": { "driver": "raw", "offset": 21504, "size": 102400, "file": { "filename": "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 26dd19860..310aff1b9 100755 --- a/v2v/test-v2v-i-ova-two-disks.sh +++ b/v2v/test-v2v-i-ova-two-disks.sh @@ -60,10 +60,17 @@ 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_is_version 2 8 ; then + # normalize the output + sed -i -e "s,\"$d/,\"," $d/source + 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.11.0
Cedric Bosdonnat
2017-Jan-31 08:39 UTC
Re: [Libguestfs] [PATCH v6 3/3] v2v: ova: don't extract files from OVA if it's not needed
On Mon, 2017-01-30 at 22:43 +0100, 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 puts much lower requirement on available > disk space. > > Since the virt-v2v behaviour for OVA input now depends on QEMU version > available this affects some of the tests. The affected tests will have > two *.expect files and the expected result also has to depend on the > QEMU used. > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> > --- > test-data/test-utils.sh | 20 ++++ > v2v/Makefile.am | 2 + > v2v/input_ova.ml | 189 +++++++++++++++++++++++++++++--- > v2v/test-v2v-i-ova-formats.sh | 5 +- > v2v/test-v2v-i-ova-subfolders.expected2 | 18 +++ > v2v/test-v2v-i-ova-subfolders.sh | 13 ++- > 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 | 72 ++++++++++++ > v2v/test-v2v-i-ova-two-disks.expected2 | 19 ++++ > v2v/test-v2v-i-ova-two-disks.sh | 13 ++- > 12 files changed, 502 insertions(+), 23 deletions(-) > 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/test-utils.sh b/test-data/test-utils.sh > index 86a5aaf12..04e833308 100755 > --- a/test-data/test-utils.sh > +++ b/test-data/test-utils.sh > @@ -54,3 +54,23 @@ do_sha256 () > ;; > esac > } > + > +# Returns 0 if QEMU version is greater or equal to the arguments > +qemu_is_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 9189aaf12..d62ac477e 100644 > --- a/v2v/Makefile.am > +++ b/v2v/Makefile.am > @@ -260,6 +260,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 \ > @@ -359,6 +360,7 @@ EXTRA_DIST += \ > test-v2v-i-ova-subfolders.expected \ > test-v2v-i-ova-subfolders.ovf \ > test-v2v-i-ova-subfolders.sh \ > + test-v2v-i-ova-tar.sh \ > test-v2v-i-ova-two-disks.expected \ > test-v2v-i-ova-two-disks.ovf \ > test-v2v-i-ova-two-disks.sh \ > diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml > index 40f723633..01ba80686 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 [paths] is specified it is > + * a list of paths in the tar archive. > + *) > + let untar ?(format = "") ?paths file outdir > + let cmd > + [ "tar"; sprintf "-x%sf" format; file; "-C"; outdir ] > + @ (match paths 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,56 @@ object > > tmpfile in > > + (* Untar only ovf and manifest from the archive *) > + let untar_metadata ova outdir > + let files > + external_command (sprintf "tar -tf %s" (Filename.quote ova)) in > + 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 ova outdir in > + > + let qemu_img_version () > + let lines = external_command "qemu-img --version" 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_"failed to parse qemu-img version(%S), assuming 0.9") > + line; > + 0, 9 > + ) else ( > + warning (f_"failed to read qemu-img version(%S), assuming 0.9") > + 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_metadata 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 +132,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 +145,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 +186,68 @@ 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 (sprintf "tar tRvf %s" (Filename.quote 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 elems = String.nsplit ~keep_empty:false ~count:7 " " line in > + if List.length elems = 8 && List.hd elems = "block" then ( > + let elems = Array.of_list elems in > + let offset = elems.(1) in > + let size = elems.(4) in > + let fname = elems.(7) in > + > + if fname <> filename then > + loop lines > + else ( > + let offset > + try > + (* There should be a colon at the end *) > + let i = String.rindex offset ':' in > + if i == (String.length offset)-1 then > + Int64.of_string (String.sub offset 0 i) > + else > + raise (Failure "colon at wrong position") > + with Failure _ | Not_found -> > + 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 > + > + (* 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 > + error (f_"failed to parse line returned by tar: %S") line > + ) > + in > + loop lines > + in > + > + let subfolder folder parent > + if folder = parent then > + "" > + else if String.is_prefix folder (parent // "") then > + let len = String.length parent in > + String.sub folder (len+1) (String.length folder-len-1) > + else > + assert false > + in > +It would be good to get this function in mllib rather since it could (and will) be reused by other code. -- Cedric> (* Search for the ovf file. *) > let ovf = find_files exploded ".ovf" in > let ovf > @@ -152,6 +265,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 +274,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,9 +401,25 @@ object > | Some "gzip" -> true > | Some s -> error (f_"unsupported compression in OVF: %s") s in > > - (* Does the file exist and is it readable? *) > - let filename = ovf_folder // filename in > - Unix.access filename [Unix.R_OK]; > + let partial > + if compressed && partial then ( > + (* We cannot access compressed disk inside the tar; we have to > + * extract it *) > + untar ~paths:[(subfolder ovf_folder exploded) // filename] > + ova tmpdir; > + false > + ) > + else > + partial in > + > + let filename > + if partial then > + (subfolder ovf_folder exploded) // filename > + else ( > + (* Does the file exist and is it readable? *) > + Unix.access (ovf_folder // filename) [Unix.R_OK]; > + ovf_folder // filename > + ) in > > (* The spec allows the file to be gzip-compressed, in which case > * we must uncompress it into the tmpdir. > @@ -302,9 +436,38 @@ object > ) > else filename in > > + let qemu_uri > + if not partial then ( > + 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 > + let doc = [ > + "file", JSON.Dict [ > + "driver", JSON.String "raw"; > + "offset", JSON.Int64 offset; > + "size", JSON.Int64 size; > + "file", JSON.Dict [ > + "filename", JSON.String ova] > + ] > + ] in > + let x > + sprintf "json:%s" (JSON.string_of_doc ~fmt:Compact doc) > + in > + (debug "json: %s" x; x) > + ) in > + > let disk = { > s_disk_id = i; > - s_qemu_uri = filename; > + s_qemu_uri = qemu_uri; > s_format = Some "vmdk"; > s_controller = controller; > } in > diff --git a/v2v/test-v2v-i-ova-formats.sh b/v2v/test-v2v-i-ova-formats.sh > index bd3e30ee8..0eec600eb 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" > @@ -63,9 +63,6 @@ cp ../test-v2v-i-ova-formats.ovf . > > 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 000000000..87996d202 > --- /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.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 4fd1acea3..422959036 100755 > --- a/v2v/test-v2v-i-ova-subfolders.sh > +++ b/v2v/test-v2v-i-ova-subfolders.sh > @@ -56,10 +56,17 @@ 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_is_version 2 8 ; then > + # normalize the output > + sed -i -e "s,\"$d/,\"," $d/source > + 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 000000000..7049aeecc > --- /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 000000000..1aa54dc37 > --- /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-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 000000000..4827c7e9b > --- /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 000000000..c3b0588f4 > --- /dev/null > +++ b/v2v/test-v2v-i-ova-tar.sh > @@ -0,0 +1,72 @@ > +#!/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/test-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 > +cp ../test-v2v-i-ova-tar.ovf . > +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_is_version 2 8 ; then > + # normalize the output > + sed -i -e "s,\"$d/,\"," $d/source > + 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 000000000..b12ca1bd6 > --- /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.ova" } } } > (vmdk) [scsi] > + json:{ "file": { "driver": "raw", "offset": 21504, "size": 102400, "file": { "filename": "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 26dd19860..310aff1b9 100755 > --- a/v2v/test-v2v-i-ova-two-disks.sh > +++ b/v2v/test-v2v-i-ova-two-disks.sh > @@ -60,10 +60,17 @@ 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_is_version 2 8 ; then > + # normalize the output > + sed -i -e "s,\"$d/,\"," $d/source > + 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
Richard W.M. Jones
2017-Jan-31 11:35 UTC
Re: [Libguestfs] [PATCH v6 2/3] mllib: modify nsplit to take optional noempty and count arguments
On Mon, Jan 30, 2017 at 10:43:15PM +0100, Tomáš Golembiovský wrote:> Added two new optional arguments to nsplit: > > * keep_empty: if set to false empty elements are not stored in the > returned list. The default is to keep the empty elementsThe ?keep_empty flag is pointless. It's simpler and more clear to write: List.filter ((<>) "") (nsplit ...) when you don't want empty elements. However the ?count flag is fine, since that's what Perl's split function also does.> - let rec nsplit sep str > + let rec nsplit ?(keep_empty = true) ?(count = -1) sep str > let len = length str in > let seplen = length sep in > let i = find str sep in > - if i = -1 then [str] > + if i = -1 || count = 0 then > + if str = "" && not keep_empty then [] else [str] > else ( > let s' = sub str 0 i in > let s'' = sub str (i+seplen) (len-i-seplen) in > - s' :: nsplit sep s'' > + let elem, count > + if s' = "" && not keep_empty then > + [], count > + else > + [s'], if count > 0 then count-1 else count > + in > + elem @ nsplit ~keep_empty:keep_empty ~count:count sep s''You can write this line as: elem @ nsplit ~count sep s'' (after dropping keep_empty). Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Richard W.M. Jones
2017-Jan-31 12:12 UTC
Re: [Libguestfs] [PATCH v6 3/3] v2v: ova: don't extract files from OVA if it's not needed
On Mon, Jan 30, 2017 at 10:43:16PM +0100, Tomáš Golembiovský wrote:> + QV=$(expr match "$(qemu-img --version)" 'qemu-img version \([0-9]\+\.[0-9]\+\)')Maybe consider using bash's internal regexp functions? We already use them in many other places, try doing: $ git grep '=~' -- \*.sh> diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml > index 40f723633..01ba80686 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 [paths] is specified it is > + * a list of paths in the tar archive. > + *) > + let untar ?(format = "") ?paths file outdir > + let cmd > + [ "tar"; sprintf "-x%sf" format; file; "-C"; outdir ] > + @ (match paths with None -> [] | Some p -> p)I don't think you need parens around the match statement here(?)> + let qemu_img_version () > + let lines = external_command "qemu-img --version" 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_"failed to parse qemu-img version(%S), assuming 0.9") > + line; > + 0, 9 > + ) else ( > + warning (f_"failed to read qemu-img version(%S), assuming 0.9") > + line; > + 0, 9 > + ) > + inI can't put my finger on exactly why, but I'm unhappy about this function. I think one improvement would be to move it to v2v/utils.ml. It clutters up the code being buried here, when really it's a utility function.> + (* Find file in [tar] archive and return at which byte it starts and how > + * long it is. > + *) > + let find_file_in_tar tar filenameThis function is saying "utility" to me as well. An advantage of putting these functions into utils.ml is that they will have a properly defined and typed interface (in utils.mli) so that people won't need to understand exactly how they work in order to work out the inputs and outputs.> + let subfolder folder parent > + if folder = parent then > + "" > + else if String.is_prefix folder (parent // "") then > + let len = String.length parent in > + String.sub folder (len+1) (String.length folder-len-1) > + else > + assert false > + inAnd I think Cedric already mentions this is a utility function. He has a use for it elsewhere, so wants it in mllib/common_utils.ml.> * we must uncompress it into the tmpdir. > + let qemu_uri > + if not partial then ( > + 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 > + let doc = [ > + "file", JSON.Dict [ > + "driver", JSON.String "raw"; > + "offset", JSON.Int64 offset; > + "size", JSON.Int64 size; > + "file", JSON.Dict [ > + "filename", JSON.String ova] > + ] > + ] in > + let x > + sprintf "json:%s" (JSON.string_of_doc ~fmt:Compact doc) > + inA few style issues in this code. Put "in" on a line by itself if you're defining a function. Put "in" on the previous line if you're defining a value. As both offset, size and x are values, move "in" to the previous line. Also the sprintf "json:..." should be indented.> + (debug "json: %s" x; x)You don't need the parens here, and probably it's clearer to put the return value on the next line.> -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" > @@ -63,9 +63,6 @@ cp ../test-v2v-i-ova-formats.ovf . > > for format in $formats; do > case "$format" in > - tar) > - tar -cf test-$format.ova test-v2v-i-ova-formats.ovf disk1.vmdk disk1.mf > - ;;Is dropping this test necessary? --- Patch looks good overall. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Reasonably Related Threads
- [PATCH v2 4/5] v2v: ova: don't extract files from OVA if it's not needed
- [PATCH v6 3/3] v2v: ova: don't extract files from OVA if it's not needed
- Re: [PATCH v2 4/5] v2v: ova: don't extract files from OVA if it's not needed
- [PATCH v7 1/1] v2v: ova: don't extract files from OVA if it's not needed
- [PATCH v4 6/6] v2v: ova: don't extract files from OVA if it's not needed