Libguestfs - Aug 2014 - Re: [PATCH] v2v: adding input -i ova

Shahar:

This is the same patch as you posted, but I have rebased it on top of
current HEAD.

You'll have to do save the next email to a file, and do:

  git reset --hard HEAD^
  git pull
  git am /path/to/saved_email

There are no changes in this patch, except what is needed to make it
compile.  Will follow-up with comments.

Rich.

From: Shahar Havivi <shaharh@redhat.com>

Hi,
This is a partial patch, for now extracting the ova file and verify the SHA1
sum,
TODO: parse vmware ovf to Types.source

Comments will be appreciated,
Thanks,
Shahar Havivi.

Signed-off-by: Shahar Havivi <shaharh@redhat.com>
---
 po/POTFILES-ml    |  1 +
 v2v/Makefile.am   |  2 ++
 v2v/cmdline.ml    | 14 ++++++--
 v2v/input_ova.ml  | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 v2v/input_ova.mli | 22 +++++++++++++
 v2v/types.mli     |  2 +-
 v2v/utils.ml      | 17 ++++++++++
 7 files changed, 152 insertions(+), 3 deletions(-)
 create mode 100644 v2v/input_ova.ml
 create mode 100644 v2v/input_ova.mli

diff --git a/po/POTFILES-ml b/po/POTFILES-ml
index 2001f47..35a94d3 100644
--- a/po/POTFILES-ml
+++ b/po/POTFILES-ml
@@ -88,6 +88,7 @@ v2v/convert_linux.ml
 v2v/convert_windows.ml
 v2v/input_disk.ml
 v2v/input_libvirt.ml
+v2v/input_ova.ml
 v2v/lib_esx.ml
 v2v/lib_linux.ml
 v2v/output_RHEV.ml
diff --git a/v2v/Makefile.am b/v2v/Makefile.am
index 3eec99a..5f2fa74 100644
--- a/v2v/Makefile.am
+++ b/v2v/Makefile.am
@@ -32,6 +32,7 @@ SOURCES_MLI = \
 	lib_linux.mli \
 	input_disk.mli \
 	input_libvirt.mli \
+	input_ova.mli \
 	output_libvirt.mli \
 	output_local.mli \
 	output_RHEV.mli \
@@ -48,6 +49,7 @@ SOURCES_ML = \
 	lib_linux.ml \
 	input_disk.ml \
 	input_libvirt.ml \
+	input_ova.ml \
 	convert_linux.ml \
 	convert_windows.ml \
 	output_libvirt.ml \
diff --git a/v2v/cmdline.ml b/v2v/cmdline.ml
index bec6d51..7842a4f 100644
--- a/v2v/cmdline.ml
+++ b/v2v/cmdline.ml
@@ -54,6 +54,7 @@ let parse_cmdline ()      | "disk" |
"local" -> input_mode := `Disk
     | "libvirt" -> input_mode := `Libvirt
     | "libvirtxml" -> input_mode := `LibvirtXML
+    | "ova" -> input_mode := `OVA
     | s ->
       error (f_"unknown -i option: %s") s
   in
@@ -105,7 +106,7 @@ let parse_cmdline ()    let argspec = Arg.align [
     "--bridge",  Arg.String add_bridge,     "in:out " ^
s_"Map bridge 'in' to 'out'";
     "--debug-gc",Arg.Set debug_gc,          " " ^
s_"Debug GC and memory allocations";
-    "-i",        Arg.String set_input_mode,
"disk|libvirt|libvirtxml " ^ s_"Set input mode (default:
libvirt)";
+    "-i",        Arg.String set_input_mode,
"disk|libvirt|libvirtxml|ova " ^ s_"Set input mode (default:
libvirt)";
     "-ic",       Arg.Set_string input_conn, "uri " ^
s_"Libvirt URI";
     "-if",       Arg.Set_string input_format,
                                             "format " ^ s_"Input
format (for -i disk)";
@@ -231,7 +232,16 @@ read the man page virt-v2v(1).
         | [filename] -> filename
         | _ ->
           error (f_"expecting a libvirt XML file name on the command
line") in
-      Input_libvirt.input_libvirtxml verbose filename in
+      Input_libvirt.input_libvirtxml verbose filename
+
+    | `OVA ->
+      (* -i ova: Expecting an ova filename (tar file). *)
+      let filename +        match args with
+        | [filename] -> filename
+        | _ ->
+          error (f_"expecting an OVA file name on the command line")
in
+      Input_ova.input_ova verbose filename in
 
   (* Parse the output mode. *)
   let output diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
new file mode 100644
index 0000000..65c47dc
--- /dev/null
+++ b/v2v/input_ova.ml
@@ -0,0 +1,97 @@
+(* virt-v2v
+ * Copyright (C) 2009-2014 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+open Printf
+
+open Common_gettext.Gettext
+open Common_utils
+
+open Types
+open Utils
+
+let contains +  true
+
+let parse_ovf dir ovf +  let source = {
+    s_dom_type = "kvm";
+    s_name = "";
+    s_orig_name = "";
+    s_memory = 0L;
+    s_vcpu = 1;
+    s_arch = "";
+    s_features = [""];
+    s_display = None;
+    s_disks = [];
+    s_removables = [];
+    s_nics = [];
+  } in
+  source
+
+class input_ova verbose ova +object
+  inherit input verbose
+
+  method as_options = "-i ova " ^ ova
+
+  method source () +    (* get ova directory *)
+    let dir = Filename.dirname (absolute_path ova) in
+    (* extract ova (tar) file *)
+    let cmd = sprintf ("tar -xf %s -C %s") (ova) (dir) in
+
+    if Sys.command (cmd) <> 0 then
+      error (f_"error running command: %s") cmd
+        exit 1;
+
+    let files = Sys.readdir(dir) in
+    let mf = ref "" in
+    let ovf = ref "" in
+    (* search for the ovf file *)
+    Array.iter (fun file ->
+      let len = String.length file in
+      if len >= 4 && String.lowercase (String.sub file (len-4) 4) =
".ovf" then
+          ovf := file
+      else if len >= 3 && String.lowercase (String.sub file (len-3)
3) = ".mf" then
+        mf := file
+    ) files;
+
+    (* verify sha1 from manifest file *)
+    let mf = dir // !mf in
+    let rex = Str.regexp "SHA1(\\(.*\\))= *\\(.*?\\).*$" in
+    let lines = read_file mf in
+    List.iter (
+      fun line -> 
+        if Str.string_match rex line 0 then
+          let file = Str.matched_group 1 line in
+          let sha1 = Str.matched_group 2 line in
+
+          let cmd = sprintf "sha1sum %s" (dir // file) in
+          let out = external_command ~prog cmd in
+          if List.exists (fun line -> string_contains line sha1) out ==
false then
+            error (f_"Checksum of %s does not match manifes sha1 %s")
(file) (sha1)
+              exit 1;
+        else
+          error (f_"error cant parse mf: %s, line: %s") (mf) (line)
+            exit 1;
+    ) lines;
+
+    parse_ovf dir ovf
+end
+
+let input_ova = new input_ova
diff --git a/v2v/input_ova.mli b/v2v/input_ova.mli
new file mode 100644
index 0000000..d4c347e
--- /dev/null
+++ b/v2v/input_ova.mli
@@ -0,0 +1,22 @@
+(* virt-v2v
+ * Copyright (C) 2009-2014 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *)
+
+(** [-i ova] source. *)
+
+val input_ova : bool -> string -> Types.input
+(** [input_ova filename] sets up an input from vmware ova file. *)
diff --git a/v2v/types.mli b/v2v/types.mli
index cf934ea..0d512b2 100644
--- a/v2v/types.mli
+++ b/v2v/types.mli
@@ -67,7 +67,7 @@ val string_of_source : source -> string
 val string_of_source_disk : source_disk -> string
 
 type overlay = {
-  ov_overlay : string;       (** Local overlay file (qcow2 format). *)
+  ov_overlay : string;       (** Local overlgy file (qcow2 format). *)
   ov_target_file : string;   (** Destination file. *)
   ov_target_format : string; (** Destination format (eg. -of option). *)
   ov_sd : string;            (** sdX libguestfs name of disk. *)
diff --git a/v2v/utils.ml b/v2v/utils.ml
index b541b1d..c4a370b 100644
--- a/v2v/utils.ml
+++ b/v2v/utils.ml
@@ -90,3 +90,20 @@ let remove_duplicates xs      | x :: xs -> Hashtbl.add h x
true; x :: loop xs
   in
   loop xs
+
+let string_contains s1 s2 +  let re = Str.regexp_string s2 in
+  try ignore (Str.string_match re s1 0); true
+  with Not_found ->
+    false
+
+let read_file filename +  let lines = ref [] in
+  let chan = open_in filename in
+  try
+    while true; do
+      lines := input_line chan :: !lines
+        done; []
+  with End_of_file ->
+    close_in chan;
+  List.rev !lines
-- 
2.0.4

On Thu, Aug 21, 2014 at 01:50:18PM +0100, Richard W.M. Jones
wrote:
> +    (* extract ova (tar) file *)
> +    let cmd = sprintf ("tar -xf %s -C %s") (ova) (dir) in
Lots of extra parentheses here :-) The same command can be written
more naturally without any of them:

   let cmd = sprintf "tar -xf %s -C %s" ova dir in

However I think what you might have meant is to call the `quote'
function to safely quote arguments, so that you're not adding security
holes through unescaped input, ie:

   let cmd = sprintf "tar -xf %s -C %s" (quote ova) (quote dir) in

Also I don't think uncompressing into the OVA directory is a good
idea.  However it works for now until we work out how and if we are
able to avoid copying those large disk images unnecessarily.
> +    if Sys.command (cmd) <> 0 then
You don't need to put parentheses around command line arguments:

   if Sys.command cmd <> 0 then

In functional languages, function application is written:

  f a b c

meaning call function `f' with 3 parameters `a', `b' and `c'.

Function application binds tightest, so:

  f a b c + 3

means (f a b c) + 3
> +      error (f_"error running command: %s") cmd
> +        exit 1;
The error function already calls exit (see mllib/common_utils.ml).

What you're actually doing here is exploiting a bug in ocaml-gettext
where it prevents the compiler from correctly detecting extra
parameters passed to a printf-like function.  Just delete "exit 1"
(but not the semicolon).  Same thing several times later on too.
> +    let files = Sys.readdir(dir) in
No parens needed around function parameters.
> +    let mf = ref "" in
> +    let ovf = ref "" in
> +    (* search for the ovf file *)
> +    Array.iter (fun file ->
> +      let len = String.length file in
> +      if len >= 4 && String.lowercase (String.sub file (len-4)
4) = ".ovf" then
> +          ovf := file
> +      else if len >= 3 && String.lowercase (String.sub file
(len-3) 3) = ".mf" then
> +        mf := file
> +    ) files;
> +
> +    (* verify sha1 from manifest file *)
> +    let mf = dir // !mf in
> +    let rex = Str.regexp "SHA1(\\(.*\\))= *\\(.*?\\).*$" in
> +    let lines = read_file mf in
> +    List.iter (
> +      fun line -> 
> +        if Str.string_match rex line 0 then
> +          let file = Str.matched_group 1 line in
> +          let sha1 = Str.matched_group 2 line in
> +
> +          let cmd = sprintf "sha1sum %s" (dir // file) in
Quoting needed, so:

      let cmd = sprintf "sha1sum %s" (quote (dir // file)) in
> +          let out = external_command ~prog cmd in
> +          if List.exists (fun line -> string_contains line sha1) out ==
false then
You wouldn't normally write `== false'.  This is more natural and
shorter:

       if not (List.exists (fun line -> string_contains line sha1) out) then
> +            error (f_"Checksum of %s does not match manifes sha1
%s") (file) (sha1)
Don't need parens around parameters `file' and `sha1'.

(f_ ...) is a special case because it's calling a function called
`f_', ie. the gettext function.  Therefore the parser could not
work out if you are calling:

  error f_ "Checksum of ..."

(error + 2 parameters) unless you put in the parentheses:

  error (f_"Checksum of ...")

(1 parameter which is the result of calling function f_).
>  type overlay = {
> -  ov_overlay : string;       (** Local overlay file (qcow2 format). *)
> +  ov_overlay : string;       (** Local overlgy file (qcow2 format). *)
Typo added.
> +let string_contains s1 s2 
There's a function already defined for this in mllib/common_utils.ml.
`string_find'.  It returns -1 if not found, or >= 0 if found.
> +let read_file filename 
See mllib/common_utils.ml `read_whole_file'.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/