[Libguestfs] [PATCH v4 6/6] v2v: ova: don't extract files from OVA if it's not needed
Pino Toscano
ptoscano at redhat.com
Wed Jan 4 09:59:15 UTC 2017
On Sunday, 18 December 2016 23:16:33 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 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 at redhat.com>
> ---
I cannot find anything big wrong at a first glance -- few notes below.
> diff --git a/v2v/Makefile.am b/v2v/Makefile.am
> index cbb974f..433c295 100644
> --- a/v2v/Makefile.am
> +++ b/v2v/Makefile.am
> @@ -257,6 +257,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 \
The new test needs to be added to EXTRA_DIST below (see lines around
340).
> diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
> index b116be8..c3fc911 100644
> --- a/v2v/input_ova.ml
> +++ b/v2v/input_ova.ml
> + let qemu_img_version () =
> + let cmd = "qemu-img --version" in
> + let lines = external_command cmd in
'cmd' can be avoided (like done right above, in untar_metadata).
> + match lines with
> + | [] -> error ("'qemu-img --version' returned no output")
> + | line :: _ ->
> + let rex = Str.regexp "qemu-img version \\([0-9]+\\)\\.\\([0-9]+\\)" in
Too long line, can you please wrap it?
> + 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;
'line' needs to be indented by a couple of spaces more (like done
below).
> + if qmajor > 2 || (qmajor == 2 && qminor >= 8) then (
> + (* If QEMU is recent enough we don't have to extract everything. We
I'd start the "We" directly in the second line of the comment.
> + let find_file_in_tar tar filename =
> + let lines = external_command (sprintf "tar tRvf %s" (Filename.quote tar)) in
Too long line.
> + if (List.length elems) = 8 && (List.hd elems) = "block" then (
Most probably the round brackets are not needed in these cases.
> + sprintf
> + "json:{ \"file\": { \"driver\":\"raw\", \"offset\":\"%Ld\", \"size\":\"%Ld\", \"file\": { \"filename\":\"%s\" } } }"
> + offset size ova
This could use the JSON module (in mllib) to create this JSON; also
please check whether the filename needs escaping and/or
qemu_input_filename (see its description in Common_utils).
--
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20170104/d6160f76/attachment.sig>
More information about the Libguestfs
mailing list