Libguestfs - Oct 2015 - Re: [PATCH v3 11/13] v2v: add --in-place mode

In this mode, converting of the VM configuration, setting up the
rollback path for error cases, transforming the VM storage and so on is
taken care of by a third-party toolset, and virt-v2v is only supposed to
tune up the guest OS directly inside the source VM, to enable it to boot
and run under the input hypervisor.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 v2v/cmdline.ml |  7 ++++-
 v2v/v2v.ml     | 87 ++++++++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 70 insertions(+), 24 deletions(-)

diff --git a/v2v/cmdline.ml b/v2v/cmdline.ml
index 3e04c48..d4bddce 100644
--- a/v2v/cmdline.ml
+++ b/v2v/cmdline.ml
@@ -36,6 +36,7 @@ let parse_cmdline ()    let dcpath = ref None in
   let input_conn = ref None in
   let input_format = ref None in
+  let in_place = ref false in
   let output_conn = ref None in
   let output_format = ref None in
   let output_name = ref None in
@@ -159,6 +160,7 @@ let parse_cmdline ()                                        
"uri " ^ s_"Libvirt URI";
     "-if",       Arg.String (set_string_option_once "-if"
input_format),
                                             "format " ^ s_"Input
format (for -i disk)";
+    "--in-place", Arg.Set in_place,         " " ^
s_"Only tune the guest in the input VM";
     "--machine-readable", Arg.Set machine_readable, " " ^
s_"Make output machine readable";
     "-n",        Arg.String add_network,    "in:out " ^
s_"Map network 'in' to 'out'";
     "--network", Arg.String add_network,    "in:out " ^
ditto;
@@ -224,6 +226,7 @@ read the man page virt-v2v(1).
   let input_conn = !input_conn in
   let input_format = !input_format in
   let input_mode = !input_mode in
+  let in_place = !in_place in
   let machine_readable = !machine_readable in
   let network_map = !network_map in
   let no_trim = !no_trim in
@@ -313,6 +316,8 @@ read the man page virt-v2v(1).
       Input_ova.input_ova filename in
 
   (* Parse the output mode. *)
+  if output_mode <> `Not_set && in_place then
+    error (f_"-o and --in-place cannot be used at the same time");
   let output      match output_mode with
     | `Glance ->
@@ -409,6 +414,6 @@ read the man page virt-v2v(1).
       Output_vdsm.output_vdsm os vdsm_params vmtype output_alloc in
 
   input, output,
-  debug_overlays, do_copy, network_map, no_trim,
+  debug_overlays, do_copy, in_place, network_map, no_trim,
   output_alloc, output_format, output_name,
   print_source, root_choice
diff --git a/v2v/v2v.ml b/v2v/v2v.ml
index 23bd708..26a9b64 100644
--- a/v2v/v2v.ml
+++ b/v2v/v2v.ml
@@ -41,12 +41,16 @@ let print_mpstat chan { mp_dev = dev; mp_path = path;
   fprintf chan "  bsize=%Ld blocks=%Ld bfree=%Ld bavail=%Ld\n"
     s.G.bsize s.G.blocks s.G.bfree s.G.bavail
 
+type conversion_mode +    | Copying of overlay list * target list
+    | In_place
+
 let () = Random.self_init ()
 
 let rec main ()    (* Handle the command line. *)
   let input, output,
-    debug_overlays, do_copy, network_map, no_trim,
+    debug_overlays, do_copy, in_place, network_map, no_trim,
     output_alloc, output_format, output_name, print_source, root_choice     
Cmdline.parse_cmdline () in
 
@@ -57,12 +61,26 @@ let rec main ()  
   let source = open_source input print_source in
   let source = amend_source source output_name network_map in
-  let overlays = create_overlays source.s_disks in
-  let targets = init_targets overlays source output output_format in
 
-  message (f_"Opening the overlay");
+  let conversion_mode +    if not in_place then (
+      let overlays = create_overlays source.s_disks in
+      let targets = init_targets overlays source output output_format in
+      Copying (overlays, targets)
+    )
+    else In_place in
+
+  let guestfs_kind = (match conversion_mode with
+                      | Copying (_, _) -> "overlay"
+                      | In_place -> "source VM"
+                     ) in
+
+  message (f_"Opening the %s") guestfs_kind;
   let g = open_guestfs () in
-  populate_overlays g overlays;
+  (match conversion_mode with
+   | Copying (overlays, _) -> populate_overlays g overlays
+   | In_place -> populate_disks g source.s_disks
+  );
 
   g#launch ();
 
@@ -72,9 +90,16 @@ let rec main ()  
   let mpstats = get_mpstats g in
   check_free_space mpstats;
-  check_target_free_space mpstats source targets output;
+  (match conversion_mode with
+   | Copying (_, targets) ->
+       check_target_free_space mpstats source targets output
+   | In_place -> ()
+  );
 
-  let keep_serial_console = output#keep_serial_console in
+  let keep_serial_console = (match conversion_mode with
+                             | Copying (_, _) -> output#keep_serial_console
+                             | In_place -> true
+                            ) in
   let guestcaps = do_convert g inspect source keep_serial_console in
 
   if no_trim <> ["*"] && (do_copy || debug_overlays)
then (
@@ -86,29 +111,34 @@ let rec main ()      do_fstrim g no_trim inspect;
   );
 
-  message (f_"Closing the overlay");
+  message (f_"Closing the %s") guestfs_kind;
   g#close ();
 
-  let target_firmware = get_target_firmware inspect guestcaps source output in
+  (match conversion_mode with
+   | In_place -> ()
+   | Copying (overlays, targets) ->
+       let target_firmware +         get_target_firmware inspect guestcaps
source output in
 
-  message (f_"Assigning disks to buses");
-  let target_buses = target_bus_assignment source targets guestcaps in
-  if verbose () then
-    printf "%s%!" (string_of_target_buses target_buses);
+       message (f_"Assigning disks to buses");
+       let target_buses = target_bus_assignment source targets guestcaps in
+       if verbose () then
+         printf "%s%!" (string_of_target_buses target_buses);
 
-  let targets -    if not do_copy then targets
-    else copy_targets targets input output output_alloc in
+       let targets +         if not do_copy then targets
+         else copy_targets targets input output output_alloc in
 
-  (* Create output metadata. *)
-  message (f_"Creating output metadata");
-  output#create_metadata source targets target_buses guestcaps inspect
-                         target_firmware;
+       (* Create output metadata. *)
+       message (f_"Creating output metadata");
+       output#create_metadata source targets target_buses guestcaps inspect
+                             target_firmware;
 
-  if debug_overlays then preserve_overlays overlays source.s_name;
+       if debug_overlays then preserve_overlays overlays source.s_name;
 
-  message (f_"Finishing off");
-  delete_target_on_exit := false  (* Don't delete target on exit. *)
+       delete_target_on_exit := false  (* Don't delete target on exit. *)
+  );
+  message (f_"Finishing off")
 
 and open_source input print_source    message (f_"Opening the source
%s") input#as_options;
@@ -269,6 +299,17 @@ and populate_overlays (g:G.guestfs) overlays         
~copyonread:true
   ) overlays
 
+and populate_disks (g:G.guestfs) src_disks +  List.iter (
+    fun ({s_qemu_uri = qemu_uri; s_format = format}) ->
+      match format with
+      | None ->
+        g#add_drive_opts qemu_uri ~cachemode:"unsafe"
~discard:"besteffort"
+      | Some fmt ->
+        g#add_drive_opts qemu_uri ~format:fmt ~cachemode:"unsafe"
+                          ~discard:"besteffort"
+  ) src_disks
+
 and inspect_source g root_choice    let roots = g#inspect_os () in
   let roots = Array.to_list roots in
-- 
2.4.3

Can you repost the final 3 patches, rebased on top of current master?

More comments below ...

On Tue, Oct 20, 2015 at 04:08:19PM +0300, Roman Kagan
wrote:
> +  let guestfs_kind = (match conversion_mode with
> +                      | Copying (_, _) -> "overlay"
> +                      | In_place -> "source VM"
> +                     ) in
> +
> +  message (f_"Opening the %s") guestfs_kind;
This is a bit cleaner if written as:
  (match conversion_mode with
   | Copying _ -> message (f_"Opening the overlay")
   | In_place -> message (f_"Opening the source VM")
  );

It's obviously shorter, but there's another reason for doing this
which is it allows the translators to see the complete message for
better translation (although in this case it doesn't make a huge
difference).

You'll have to do the same change with the "Closing ..." message,
since guestfs_kind is no longer defined.
> -  let keep_serial_console = output#keep_serial_console in
> +  let keep_serial_console = (match conversion_mode with
> +                             | Copying (_, _) ->
output#keep_serial_console
> +                             | In_place -> true
> +                            ) in
I don't think this change is correct.  I think you should call
#keep_serial_console in both cases, since this is controlled by the
guest type, not the conversion mode.
> -  message (f_"Closing the overlay");
> +  message (f_"Closing the %s") guestfs_kind;
See above.
> +and populate_disks (g:G.guestfs) src_disks 
As far as I know, the type annotation here should not be necessary.
But if it is necessary, then put some spaces in, ie:

                 ...  (g : G.guestfs) ...
> +  List.iter (
> +    fun ({s_qemu_uri = qemu_uri; s_format = format}) ->
> +      match format with
> +      | None ->
> +        g#add_drive_opts qemu_uri ~cachemode:"unsafe"
~discard:"besteffort"
> +      | Some fmt ->
> +        g#add_drive_opts qemu_uri ~format:fmt
~cachemode:"unsafe"
> +                          ~discard:"besteffort"
You don't need the match statement here, just use ?format
as a parameter, ie:

    fun ({s_qemu_uri = qemu_uri; s_format = format}) ->
        g#add_drive_opts qemu_uri ?format
                ~cachemode:"unsafe" ~discard:"besteffort"

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/

On Tue, Oct 20, 2015 at 04:02:39PM +0100, Richard W.M. Jones
wrote:
> Can you repost the final 3 patches, rebased on top of current master?
Sure.
> On Tue, Oct 20, 2015 at 04:08:19PM +0300, Roman Kagan wrote:
> > -  let keep_serial_console = output#keep_serial_console in
> > +  let keep_serial_console = (match conversion_mode with
> > +                             | Copying (_, _) ->
output#keep_serial_console
> > +                             | In_place -> true
> > +                            ) in
> 
> I don't think this change is correct.  I think you should call
> #keep_serial_console in both cases, since this is controlled by the
> guest type, not the conversion mode.
Indeed.  Thanks!
> > +and populate_disks (g:G.guestfs) src_disks > 
> As far as I know, the type annotation here should not be necessary.
It isn't indeed.  In my previous submission when defined with 'let'
it
triggered type mismatch error without annotation; now with 'and' it no
longer does.  So yes I'll drop it.

I'll take your comments into account and resubmit.

Thanks,
Roman.