First off, thanks for the review. IIUC you are not opposed to the idea of adding the in-place operation mode in general, are you? I'll work through your comments to make the patch fit better with the rest of the code. On Tue, Jul 28, 2015 at 11:54:59AM +0100, Richard W.M. Jones wrote:

On Mon, Jul 27, 2015 at 07:29:57PM +0300, Roman Kagan wrote:

...

--- a/tests/guests/guests.xml.in +++ b/tests/guests/guests.xml.in

Let's not add this to tests/guests, since this disk image is only used for a single test in the v2v/ subdirectory.

You can change the new test in the v2v/ subdirectory so it creates (or is supplied with) the XML. There are various tests which work like that already - eg: v2v/test-v2v-cdrom.*

OK, makes sense, thanks for the suggestion. While at this: do you think adding the test for the newly added option should go in the same commit with the option itself? It may be easier to review separately, but YMMV, so please let me know which way you'd like it better.

...

+ let overlays = ( + if not in_place then ( [...] + ) else []

The overlays array shouldn't be an empty list in the case where we are doing an in_place conversion. It should be None (and the other case should be Some overlays).

However this makes me wonder if there is a better way to structure this code. As it stands, when using --in-place, the output object is set (pretty much randomly) to Output_libvirt.output_libvirt. So a bunch of methods on Output_libvirt.output_libvirt are called which don't make sense.

Right. Actually when developing the patch I hesitated between better structure and less changes, and chose the latter as I wasn't sure the feature would be welcome at all. That's why I used empty lists here (and all over the place): as a result I didn't need multiple extra conditionals, because the code which wasn't supposed to execute in in_place case didn't as it was passed an empty list to work on. That said, if we're up to restructuring (which is IMO desirable here anyway since the main function is now about 400 lines) I'll try to make it look more natural.

In virt-sparsify, --in-place is handled essentially by a completely different main function in a different module, and that might be a less error-prone way to do things here too.

Thanks for the suggestion, I'll have a look.

Change the if statements around so that

if not in_place then (* old code *) else (* new code *)

I envisage that the not in_place route will remain the usual way to use virt-v2v.

OK

...

@@ -518,6 +523,18 @@ C<root>. You will get an error if virt-v2v is unable to mount/write to the Export Storage Domain.

+=item B<--in-place>

This appears in the wrong place in the list of options (sorted by 'p' rather than 'i'). I don't know if this was intentional, but I think it's wrong.

I didn't realize the options went in a strict sorting order. Besides, at first I thought I'd make it a new -o suboption; then I changed my mind but didn't bother to adjust the position in the list. Thanks, Roman.

Opening the source VM and amending the properties in its internal representation in accordance with command-line options fit nicely into two isolated functions. Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 71 +++++++++++++++++++++++++++++++++----------------------------- 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index 9cb4a27..a2cf249 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -43,21 +43,7 @@ let print_mpstat chan { mp_dev = dev; mp_path = path; let () = Random.self_init () -let rec main () = - (* Handle the command line. *) - let input, output, - debug_gc, debug_overlays, do_copy, network_map, no_trim, - output_alloc, output_format, output_name, print_source, root_choice = - Cmdline.parse_cmdline () in - - (* Print the version, easier than asking users to tell us. *) - if verbose () then - printf "%s: %s %s (%s)\n%!" - prog Config.package_name Config.package_version Config.host_cpu; - - if debug_gc then - at_exit (fun () -> Gc.compact()); - +let open_source input print_source = message (f_"Opening the source %s") input#as_options; let source = input#source () in @@ -68,7 +54,6 @@ let rec main () = printf "%s\n" (string_of_source source); exit 0 ); - if verbose () then printf "%s%!" (string_of_source source); (match source.s_hypervisor with @@ -87,6 +72,9 @@ let rec main () = assert (disk.s_qemu_uri <> ""); ) source.s_disks; + source + +let amend_source source output_name network_map = (* Map source name. *) let source = match output_name with @@ -97,26 +85,43 @@ let rec main () = | Some name -> { source with s_name = name } in (* Map networks and bridges. *) - let source = - let { s_nics = nics } = source in - let nics = List.map ( - fun ({ s_vnet_type = t; s_vnet = vnet } as nic) -> + let nics = List.map ( + fun ({ s_vnet_type = t; s_vnet = vnet } as nic) -> + try + (* Look for a --network or --bridge parameter which names this + * network/bridge (eg. --network in:out). + *) + let new_name = List.assoc (t, vnet) network_map in + { nic with s_vnet = new_name } + with Not_found -> try - (* Look for a --network or --bridge parameter which names this - * network/bridge (eg. --network in:out). - *) - let new_name = List.assoc (t, vnet) network_map in + (* Not found, so look for a default mapping (eg. --network out). *) + let new_name = List.assoc (t, "") network_map in { nic with s_vnet = new_name } with Not_found -> - try - (* Not found, so look for a default mapping (eg. --network out). *) - let new_name = List.assoc (t, "") network_map in - { nic with s_vnet = new_name } - with Not_found -> - (* Not found, so return the original NIC unchanged. *) - nic - ) nics in - { source with s_nics = nics } in + (* Not found, so return the original NIC unchanged. *) + nic + ) source.s_nics in + + { source with s_nics = nics } + +let rec main () = + (* Handle the command line. *) + let input, output, + debug_gc, debug_overlays, do_copy, network_map, no_trim, + output_alloc, output_format, output_name, print_source, root_choice = + Cmdline.parse_cmdline () in + + (* Print the version, easier than asking users to tell us. *) + if verbose () then + printf "%s: %s %s (%s)\n%!" + prog Config.package_name Config.package_version Config.host_cpu; + + if debug_gc then + at_exit (fun () -> Gc.compact()); + + let source = open_source input print_source in + let source = amend_source source output_name network_map in (* Create a qcow2 v3 overlay to protect the source image(s). There * is a specific reason to use the newer qcow2 variant: Because the -- 2.4.3

Iterating over source disks and creating temporary overlays for easy rollback fits nicely into a separate function. In addition, determining their size doesn't need to wait until the guestfs is launched: the size can be obtained via disk_virtual_size() method. Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 91 +++++++++++++++++++++++++++++--------------------------------- 1 file changed, 42 insertions(+), 49 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index a2cf249..b3dfa07 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -105,6 +105,45 @@ let amend_source source output_name network_map = { source with s_nics = nics } +let create_overlays src_disks = + (* Create a qcow2 v3 overlay to protect the source image(s). There + * is a specific reason to use the newer qcow2 variant: Because the + * L2 table can store zero clusters efficiently, and because + * discarded blocks are stored as zero clusters, this should allow us + * to fstrim/blkdiscard and avoid copying significant parts of the + * data over the wire. + *) + message (f_"Creating overlays to protect the sources from being modified"); + let overlay_dir = (new Guestfs.guestfs ())#get_cachedir () in + List.mapi ( + fun i ({ s_qemu_uri = qemu_uri; s_format = format } as source) -> + let overlay_file = + Filename.temp_file ~temp_dir:overlay_dir "v2vovl" ".qcow2" in + unlink_on_exit overlay_file; + + let options = + "compat=1.1" ^ + (match format with None -> "" + | Some fmt -> ",backing_fmt=" ^ fmt) in + let cmd = + sprintf "qemu-img create -q -f qcow2 -b %s -o %s %s" + (quote qemu_uri) (quote options) overlay_file in + if verbose () then printf "%s\n%!" cmd; + if Sys.command cmd <> 0 then + error (f_"qemu-img command failed, see earlier errors"); + + (* Sanity check created overlay (see below). *) + if not ((new G.guestfs ())#disk_has_backing_file overlay_file) then + error (f_"internal error: qemu-img did not create overlay with backing file"); + + let sd = "sd" ^ drive_name i in + + let vsize = (new G.guestfs ())#disk_virtual_size overlay_file in + + { ov_overlay_file = overlay_file; ov_sd = sd; + ov_virtual_size = vsize; ov_source = source } + ) src_disks + let rec main () = (* Handle the command line. *) let input, output, @@ -122,40 +161,7 @@ let rec main () = let source = open_source input print_source in let source = amend_source source output_name network_map in - - (* Create a qcow2 v3 overlay to protect the source image(s). There - * is a specific reason to use the newer qcow2 variant: Because the - * L2 table can store zero clusters efficiently, and because - * discarded blocks are stored as zero clusters, this should allow us - * to fstrim/blkdiscard and avoid copying significant parts of the - * data over the wire. - *) - message (f_"Creating an overlay to protect the source from being modified"); - let overlay_dir = (new Guestfs.guestfs ())#get_cachedir () in - let overlays = - List.map ( - fun ({ s_qemu_uri = qemu_uri; s_format = format } as source) -> - let overlay_file = - Filename.temp_file ~temp_dir:overlay_dir "v2vovl" ".qcow2" in - unlink_on_exit overlay_file; - - let options = - "compat=1.1" ^ - (match format with None -> "" - | Some fmt -> ",backing_fmt=" ^ fmt) in - let cmd = - sprintf "qemu-img create -q -f qcow2 -b %s -o %s %s" - (quote qemu_uri) (quote options) overlay_file in - if verbose () then printf "%s\n%!" cmd; - if Sys.command cmd <> 0 then - error (f_"qemu-img command failed, see earlier errors"); - - (* Sanity check created overlay (see below). *) - if not ((new G.guestfs ())#disk_has_backing_file overlay_file) then - error (f_"internal error: qemu-img did not create overlay with backing file"); - - overlay_file, source - ) source.s_disks in + let overlays = create_overlays source.s_disks in (* Open the guestfs handle. *) message (f_"Opening the overlay"); @@ -164,7 +170,7 @@ let rec main () = if verbose () then g#set_verbose true; g#set_network true; List.iter ( - fun (overlay_file, _) -> + fun ({ov_overlay_file = overlay_file}) -> g#add_drive_opts overlay_file ~format:"qcow2" ~cachemode:"unsafe" ~discard:"besteffort" ~copyonread:true @@ -172,20 +178,6 @@ let rec main () = g#launch (); - (* Create the list of overlays structs. Query each disk for its - * virtual size, and fill in a few other fields. - *) - let overlays = - mapi ( - fun i (overlay_file, source) -> - let sd = "sd" ^ drive_name i in - let dev = "/dev/" ^ sd in - let vsize = g#blockdev_getsize64 dev in - - { ov_overlay_file = overlay_file; ov_sd = sd; - ov_virtual_size = vsize; ov_source = source } - ) overlays in - (* Work out where we will write the final output. Do this early * just so we can display errors to the user before doing too much * work. @@ -451,6 +443,7 @@ let rec main () = (* Save overlays if --debug-overlays option was used. *) if debug_overlays then ( + let overlay_dir = (new Guestfs.guestfs ())#get_cachedir () in List.iter ( fun ov -> let saved_filename = -- 2.4.3

Besides, it doesn't need guestfs handle open so move this step earlier in the process. Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 73 ++++++++++++++++++++++++++++++++------------------------------ 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index b3dfa07..c20cbf0 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -144,40 +144,7 @@ let create_overlays src_disks = ov_virtual_size = vsize; ov_source = source } ) src_disks -let rec main () = - (* Handle the command line. *) - let input, output, - debug_gc, debug_overlays, do_copy, network_map, no_trim, - output_alloc, output_format, output_name, print_source, root_choice = - Cmdline.parse_cmdline () in - - (* Print the version, easier than asking users to tell us. *) - if verbose () then - printf "%s: %s %s (%s)\n%!" - prog Config.package_name Config.package_version Config.host_cpu; - - if debug_gc then - at_exit (fun () -> Gc.compact()); - - 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 - - (* Open the guestfs handle. *) - message (f_"Opening the overlay"); - let g = new G.guestfs () in - if trace () then g#set_trace true; - if verbose () then g#set_verbose true; - g#set_network true; - List.iter ( - fun ({ov_overlay_file = overlay_file}) -> - g#add_drive_opts overlay_file - ~format:"qcow2" ~cachemode:"unsafe" ~discard:"besteffort" - ~copyonread:true - ) overlays; - - g#launch (); - +let init_targets overlays source output output_format = (* Work out where we will write the final output. Do this early * just so we can display errors to the user before doing too much * work. @@ -213,7 +180,43 @@ let rec main () = target_actual_size = None; target_overlay = ov } ) overlays in - let targets = output#prepare_targets source targets in + + output#prepare_targets source targets + +let rec main () = + (* Handle the command line. *) + let input, output, + debug_gc, debug_overlays, do_copy, network_map, no_trim, + output_alloc, output_format, output_name, print_source, root_choice = + Cmdline.parse_cmdline () in + + (* Print the version, easier than asking users to tell us. *) + if verbose () then + printf "%s: %s %s (%s)\n%!" + prog Config.package_name Config.package_version Config.host_cpu; + + if debug_gc then + at_exit (fun () -> Gc.compact()); + + 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 + + (* Open the guestfs handle. *) + message (f_"Opening the overlay"); + let g = new G.guestfs () in + if trace () then g#set_trace true; + if verbose () then g#set_verbose true; + g#set_network true; + List.iter ( + fun ({ov_overlay_file = overlay_file}) -> + g#add_drive_opts overlay_file + ~format:"qcow2" ~cachemode:"unsafe" ~discard:"besteffort" + ~copyonread:true + ) overlays; + + g#launch (); (* Inspection - this also mounts up the filesystems. *) message (f_"Inspecting the overlay"); -- 2.4.3

Factor the size checks out to separate functions. Besides, move the definitions of functions called therein before the definition of the new functions, for better readability. Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 378 +++++++++++++++++++++++++++++++------------------------------ 1 file changed, 190 insertions(+), 188 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index c20cbf0..53456ea 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -183,6 +183,194 @@ let init_targets overlays source output output_format = output#prepare_targets source targets +(* Conversion can fail if there is no space on the guest filesystems + * (RHBZ#1139543). To avoid this situation, check there is some + * headroom. Mainly we care about the root filesystem. + *) +let check_free_space mpstats = + message (f_"Checking for sufficient free disk space in the guest"); + List.iter ( + fun { mp_path = mp; + mp_statvfs = { G.bfree = bfree; blocks = blocks; bsize = bsize } } -> + (* Ignore small filesystems. *) + let total_size = blocks *^ bsize in + if total_size > 100_000_000L then ( + (* bfree = free blocks for root user *) + let free_bytes = bfree *^ bsize in + let needed_bytes = + match mp with + | "/" -> + (* We may install some packages, and they would usually go + * on the root filesystem. + *) + 20_000_000L + | "/boot" -> + (* We usually regenerate the initramfs, which has a + * typical size of 20-30MB. Hence: + *) + 50_000_000L + | _ -> + (* For everything else, just make sure there is some free space. *) + 10_000_000L in + + if free_bytes < needed_bytes then + error (f_"not enough free space for conversion on filesystem '%s'. %Ld bytes free < %Ld bytes needed") + mp free_bytes needed_bytes + ) + ) mpstats + +(* Estimate the space required on the target for each disk. It is the + * maximum space that might be required, but in reasonable cases much + * less space would actually be needed. + * + * As a starting point we could take ov_virtual_size (plus a tiny + * overhead for qcow2 headers etc) as the maximum. However that's not + * very useful. Other information we have available is: + * + * - The list of filesystems across the source disk(s). + * + * - The disk used/free of each of those filesystems, and the + * filesystem type. + * + * Note that we do NOT have the used size of the source disk (because + * it may be remote). + * + * How do you attribute filesystem usage through to backing disks, + * since one filesystem might span multiple disks? + * + * How do you account for non-filesystem usage (eg. swap, partitions + * that libguestfs cannot read, the space between LVs/partitions)? + * + * Another wildcard is that although we try to run {c fstrim} on each + * source filesystem, it can fail in some common scenarios. Also + * qemu-img will do zero detection. Both of these can be big wins when + * they work. + * + * The algorithm used here is this: + * + * (1) Calculate the total virtual size of all guest filesystems. + * eg: [ "/boot" = 500 MB, "/" = 2.5 GB ], total = 3 GB + * + * (2) Calculate the total virtual size of all source disks. + * eg: [ sda = 1 GB, sdb = 3 GB ], total = 4 GB + * + * (3) The ratio of (1):(2) is the maximum that could be freed up if + * all filesystems were effectively zeroed out during the conversion. + * eg. ratio = 3/4 + * + * (4) Work out how much filesystem space we are likely to save if + * fstrim works, but exclude a few cases where fstrim will probably + * fail (eg. filesystems that don't support fstrim). This is the + * conversion saving. + * eg. [ "/boot" = 200 MB used, "/" = 1 GB used ], saving = 3 - 1.2 = 1.8 + * + * (5) Scale the conversion saving (4) by the ratio (3), and allocate + * that saving across all source disks in proportion to their + * virtual size. + * eg. scaled saving is 1.8 * 3/4 = 1.35 GB + * sda has 1/4 of total virtual size, so it gets a saving of 1.35/4 + * sda final estimated size = 1 - (1.35/4) = 0.6625 GB + * sdb has 3/4 of total virtual size, so it gets a saving of 3 * 1.35 / 4 + * sdb final estimate size = 3 - (3*1.35/4) = 1.9875 GB + *) +let estimate_target_size mpstats targets = + let sum = List.fold_left (+^) 0L in + + (* (1) *) + let fs_total_size = + sum ( + List.map (fun { mp_statvfs = s } -> s.G.blocks *^ s.G.bsize) mpstats + ) in + if verbose () then + printf "estimate_target_size: fs_total_size = %Ld [%s]\n%!" + fs_total_size (human_size fs_total_size); + + (* (2) *) + let source_total_size = + sum (List.map (fun t -> t.target_overlay.ov_virtual_size) targets) in + if verbose () then + printf "estimate_target_size: source_total_size = %Ld [%s]\n%!" + source_total_size (human_size source_total_size); + + if source_total_size = 0L then (* Avoid divide by zero error. *) + targets + else ( + (* (3) Store the ratio as a float to avoid overflows later. *) + let ratio = + Int64.to_float fs_total_size /. Int64.to_float source_total_size in + if verbose () then + printf "estimate_target_size: ratio = %.3f\n%!" ratio; + + (* (4) *) + let fs_free = + sum ( + List.map ( + function + (* On filesystems supported by fstrim, assume we can save all + * the free space. + *) + | { mp_vfs = "ext2"|"ext3"|"ext4"|"xfs"; mp_statvfs = s } -> + s.G.bfree *^ s.G.bsize + + (* fstrim is only supported on NTFS very recently, and has a + * lot of limitations. So make the safe assumption for now + * that it's not going to work. + *) + | { mp_vfs = "ntfs" } -> 0L + + (* For other filesystems, sorry we can't free anything :-/ *) + | _ -> 0L + ) mpstats + ) in + if verbose () then + printf "estimate_target_size: fs_free = %Ld [%s]\n%!" + fs_free (human_size fs_free); + let scaled_saving = Int64.of_float (Int64.to_float fs_free *. ratio) in + if verbose () then + printf "estimate_target_size: scaled_saving = %Ld [%s]\n%!" + scaled_saving (human_size scaled_saving); + + (* (5) *) + let targets = List.map ( + fun ({ target_overlay = ov } as t) -> + let size = ov.ov_virtual_size in + let proportion = + Int64.to_float size /. Int64.to_float source_total_size in + let estimated_size = + size -^ Int64.of_float (proportion *. Int64.to_float scaled_saving) in + if verbose () then + printf "estimate_target_size: %s: %Ld [%s]\n%!" + ov.ov_sd estimated_size (human_size estimated_size); + { t with target_estimated_size = Some estimated_size } + ) targets in + + targets + ) + +let get_mpstats g = + (* Collect statvfs information from the guest mountpoints. *) + let mpstats = List.map ( + fun (dev, path) -> + let statvfs = g#statvfs path in + let vfs = g#vfs_type dev in + { mp_dev = dev; mp_path = path; mp_statvfs = statvfs; mp_vfs = vfs } + ) (g#mountpoints ()) in + + if verbose () then ( + (* This is useful for debugging speed / fstrim issues. *) + printf "mpstats:\n"; + List.iter (print_mpstat Pervasives.stdout) mpstats + ); + + mpstats + +let check_target_free_space mpstats source targets output = + (* Estimate space required on target for each disk. Note this is a max. *) + message (f_"Estimating space required on target for each disk"); + let targets = estimate_target_size mpstats targets in + + output#check_target_free_space source targets + let rec main () = (* Handle the command line. *) let input, output, @@ -222,32 +410,9 @@ let rec main () = message (f_"Inspecting the overlay"); let inspect = inspect_source g root_choice in - (* The guest free disk space check and the target free space - * estimation both require statvfs information from mountpoints, so - * get that information first. - *) - let mpstats = List.map ( - fun (dev, path) -> - let statvfs = g#statvfs path in - let vfs = g#vfs_type dev in - { mp_dev = dev; mp_path = path; mp_statvfs = statvfs; mp_vfs = vfs } - ) (g#mountpoints ()) in - - if verbose () then ( - (* This is useful for debugging speed / fstrim issues. *) - printf "mpstats:\n"; - List.iter (print_mpstat Pervasives.stdout) mpstats - ); - - (* Check there is enough free space to perform conversion. *) - message (f_"Checking for sufficient free disk space in the guest"); + let mpstats = get_mpstats g in check_free_space mpstats; - - (* Estimate space required on target for each disk. Note this is a max. *) - message (f_"Estimating space required on target for each disk"); - let targets = estimate_target_size mpstats targets in - - output#check_target_free_space source targets; + check_target_free_space mpstats source targets output; (* Conversion. *) let guestcaps = @@ -597,41 +762,6 @@ and inspect_source g root_choice = if verbose () then printf "%s%!" (string_of_inspect inspect); inspect -(* Conversion can fail if there is no space on the guest filesystems - * (RHBZ#1139543). To avoid this situation, check there is some - * headroom. Mainly we care about the root filesystem. - *) -and check_free_space mpstats = - List.iter ( - fun { mp_path = mp; - mp_statvfs = { G.bfree = bfree; blocks = blocks; bsize = bsize } } -> - (* Ignore small filesystems. *) - let total_size = blocks *^ bsize in - if total_size > 100_000_000L then ( - (* bfree = free blocks for root user *) - let free_bytes = bfree *^ bsize in - let needed_bytes = - match mp with - | "/" -> - (* We may install some packages, and they would usually go - * on the root filesystem. - *) - 20_000_000L - | "/boot" -> - (* We usually regenerate the initramfs, which has a - * typical size of 20-30MB. Hence: - *) - 50_000_000L - | _ -> - (* For everything else, just make sure there is some free space. *) - 10_000_000L in - - if free_bytes < needed_bytes then - error (f_"not enough free space for conversion on filesystem '%s'. %Ld bytes free < %Ld bytes needed") - mp free_bytes needed_bytes - ) - ) mpstats - (* Perform the fstrim. The trimming bit is easy. Dealing with the * [--no-trim] parameter .. not so much. *) @@ -691,134 +821,6 @@ and do_fstrim g no_trim inspect = ) ) fses -(* Estimate the space required on the target for each disk. It is the - * maximum space that might be required, but in reasonable cases much - * less space would actually be needed. - * - * As a starting point we could take ov_virtual_size (plus a tiny - * overhead for qcow2 headers etc) as the maximum. However that's not - * very useful. Other information we have available is: - * - * - The list of filesystems across the source disk(s). - * - * - The disk used/free of each of those filesystems, and the - * filesystem type. - * - * Note that we do NOT have the used size of the source disk (because - * it may be remote). - * - * How do you attribute filesystem usage through to backing disks, - * since one filesystem might span multiple disks? - * - * How do you account for non-filesystem usage (eg. swap, partitions - * that libguestfs cannot read, the space between LVs/partitions)? - * - * Another wildcard is that although we try to run {c fstrim} on each - * source filesystem, it can fail in some common scenarios. Also - * qemu-img will do zero detection. Both of these can be big wins when - * they work. - * - * The algorithm used here is this: - * - * (1) Calculate the total virtual size of all guest filesystems. - * eg: [ "/boot" = 500 MB, "/" = 2.5 GB ], total = 3 GB - * - * (2) Calculate the total virtual size of all source disks. - * eg: [ sda = 1 GB, sdb = 3 GB ], total = 4 GB - * - * (3) The ratio of (1):(2) is the maximum that could be freed up if - * all filesystems were effectively zeroed out during the conversion. - * eg. ratio = 3/4 - * - * (4) Work out how much filesystem space we are likely to save if - * fstrim works, but exclude a few cases where fstrim will probably - * fail (eg. filesystems that don't support fstrim). This is the - * conversion saving. - * eg. [ "/boot" = 200 MB used, "/" = 1 GB used ], saving = 3 - 1.2 = 1.8 - * - * (5) Scale the conversion saving (4) by the ratio (3), and allocate - * that saving across all source disks in proportion to their - * virtual size. - * eg. scaled saving is 1.8 * 3/4 = 1.35 GB - * sda has 1/4 of total virtual size, so it gets a saving of 1.35/4 - * sda final estimated size = 1 - (1.35/4) = 0.6625 GB - * sdb has 3/4 of total virtual size, so it gets a saving of 3 * 1.35 / 4 - * sdb final estimate size = 3 - (3*1.35/4) = 1.9875 GB - *) -and estimate_target_size mpstats targets = - let sum = List.fold_left (+^) 0L in - - (* (1) *) - let fs_total_size = - sum ( - List.map (fun { mp_statvfs = s } -> s.G.blocks *^ s.G.bsize) mpstats - ) in - if verbose () then - printf "estimate_target_size: fs_total_size = %Ld [%s]\n%!" - fs_total_size (human_size fs_total_size); - - (* (2) *) - let source_total_size = - sum (List.map (fun t -> t.target_overlay.ov_virtual_size) targets) in - if verbose () then - printf "estimate_target_size: source_total_size = %Ld [%s]\n%!" - source_total_size (human_size source_total_size); - - if source_total_size = 0L then (* Avoid divide by zero error. *) - targets - else ( - (* (3) Store the ratio as a float to avoid overflows later. *) - let ratio = - Int64.to_float fs_total_size /. Int64.to_float source_total_size in - if verbose () then - printf "estimate_target_size: ratio = %.3f\n%!" ratio; - - (* (4) *) - let fs_free = - sum ( - List.map ( - function - (* On filesystems supported by fstrim, assume we can save all - * the free space. - *) - | { mp_vfs = "ext2"|"ext3"|"ext4"|"xfs"; mp_statvfs = s } -> - s.G.bfree *^ s.G.bsize - - (* fstrim is only supported on NTFS very recently, and has a - * lot of limitations. So make the safe assumption for now - * that it's not going to work. - *) - | { mp_vfs = "ntfs" } -> 0L - - (* For other filesystems, sorry we can't free anything :-/ *) - | _ -> 0L - ) mpstats - ) in - if verbose () then - printf "estimate_target_size: fs_free = %Ld [%s]\n%!" - fs_free (human_size fs_free); - let scaled_saving = Int64.of_float (Int64.to_float fs_free *. ratio) in - if verbose () then - printf "estimate_target_size: scaled_saving = %Ld [%s]\n%!" - scaled_saving (human_size scaled_saving); - - (* (5) *) - let targets = List.map ( - fun ({ target_overlay = ov } as t) -> - let size = ov.ov_virtual_size in - let proportion = - Int64.to_float size /. Int64.to_float source_total_size in - let estimated_size = - size -^ Int64.of_float (proportion *. Int64.to_float scaled_saving) in - if verbose () then - printf "estimate_target_size: %s: %Ld [%s]\n%!" - ov.ov_sd estimated_size (human_size estimated_size); - { t with target_estimated_size = Some estimated_size } - ) targets in - - targets - ) - (* Update the target_actual_size field in the target structure. *) and actual_target_size target = { target with target_actual_size = du target.target_file } -- 2.4.3

Factor out copying the overlays to final disk images into a separate function. Also move a couple of funcitons called therein ahead for easier navigation. Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 262 ++++++++++++++++++++++++++++++------------------------------- 1 file changed, 131 insertions(+), 131 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index b3c1a44..1424bf1 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -505,6 +505,136 @@ let target_bus_assignment source targets guestcaps = target_buses +(* There's no OCaml binding for st_blocks, so run coreutils 'du' + * to get the used size in bytes. + *) +let du filename = + let cmd = sprintf "du --block-size=1 %s | awk '{print $1}'" (quote filename) in + let lines = external_command cmd in + (* Ignore errors because we want to avoid failures after copying. *) + match lines with + | line::_ -> (try Some (Int64.of_string line) with _ -> None) + | [] -> None + +(* Update the target_actual_size field in the target structure. *) +let actual_target_size target = + { target with target_actual_size = du target.target_file } + +let delete_target_on_exit = ref true + +let copy_targets targets input (output:Types.output) output_alloc = + (* Copy the source to the output. *) + at_exit (fun () -> + if !delete_target_on_exit then ( + List.iter ( + fun t -> try unlink t.target_file with _ -> () + ) targets + ) + ); + let nr_disks = List.length targets in + mapi ( + fun i t -> + message (f_"Copying disk %d/%d to %s (%s)") + (i+1) nr_disks t.target_file t.target_format; + if verbose () then printf "%s%!" (string_of_target t); + + (* We noticed that qemu sometimes corrupts the qcow2 file on + * exit. This only seemed to happen with lazy_refcounts was + * used. The symptom was that the header wasn't written back + * to the disk correctly and the file appeared to have no + * backing file. Just sanity check this here. + *) + let overlay_file = t.target_overlay.ov_overlay_file in + if not ((new G.guestfs ())#disk_has_backing_file overlay_file) then + error (f_"internal error: qemu corrupted the overlay file"); + + (* Give the input module a chance to adjust the parameters + * of the overlay/backing file. This allows us to increase + * the readahead parameter when copying (see RHBZ#1151033 and + * RHBZ#1153589 for the gruesome details). + *) + input#adjust_overlay_parameters t.target_overlay; + + (* It turns out that libguestfs's disk creation code is + * considerably more flexible and easier to use than + * qemu-img, so create the disk explicitly using libguestfs + * then pass the 'qemu-img convert -n' option so qemu reuses + * the disk. + * + * Also we allow the output mode to actually create the disk + * image. This lets the output mode set ownership and + * permissions correctly if required. + *) + (* What output preallocation mode should we use? *) + let preallocation = + match t.target_format, output_alloc with + | "raw", Sparse -> Some "sparse" + | "raw", Preallocated -> Some "full" + | "qcow2", Sparse -> Some "off" (* ? *) + | "qcow2", Preallocated -> Some "metadata" + | _ -> None (* ignore -oa flag for other formats *) in + let compat = + match t.target_format with "qcow2" -> Some "1.1" | _ -> None in + output#disk_create + t.target_file t.target_format t.target_overlay.ov_virtual_size + ?preallocation ?compat; + + let cmd = + sprintf "qemu-img convert%s -n -f qcow2 -O %s %s %s" + (if not (quiet ()) then " -p" else "") + (quote t.target_format) (quote overlay_file) + (quote t.target_file) in + if verbose () then printf "%s\n%!" cmd; + let start_time = gettimeofday () in + if Sys.command cmd <> 0 then + error (f_"qemu-img command failed, see earlier errors"); + let end_time = gettimeofday () in + + (* Calculate the actual size on the target, returns an updated + * target structure. + *) + let t = actual_target_size t in + + (* If verbose, print the virtual and real copying rates. *) + let elapsed_time = end_time -. start_time in + if verbose () && elapsed_time > 0. then ( + let mbps size time = + Int64.to_float size /. 1024. /. 1024. *. 10. /. time + in + + printf "virtual copying rate: %.1f M bits/sec\n%!" + (mbps t.target_overlay.ov_virtual_size elapsed_time); + + match t.target_actual_size with + | None -> () + | Some actual -> + printf "real copying rate: %.1f M bits/sec\n%!" + (mbps actual elapsed_time) + ); + + (* If verbose, find out how close the estimate was. This is + * for developer information only - so we can increase the + * accuracy of the estimate. + *) + if verbose () then ( + match t.target_estimated_size, t.target_actual_size with + | None, None | None, Some _ | Some _, None | Some _, Some 0L -> () + | Some estimate, Some actual -> + let pc = + 100. *. Int64.to_float estimate /. Int64.to_float actual + -. 100. in + printf "%s: estimate %Ld (%s) versus actual %Ld (%s): %.1f%%" + t.target_overlay.ov_sd + estimate (human_size estimate) + actual (human_size actual) + pc; + if pc < 0. then printf " ! ESTIMATE TOO LOW !"; + printf "\n%!"; + ); + + t + ) targets + let rec main () = (* Handle the command line. *) let input, output, @@ -569,124 +699,9 @@ let rec main () = let target_firmware = get_target_firmware inspect guestcaps source output in let target_buses = target_bus_assignment source targets guestcaps in - - let delete_target_on_exit = ref true in - let targets = if not do_copy then targets - else ( - (* Copy the source to the output. *) - at_exit (fun () -> - if !delete_target_on_exit then ( - List.iter ( - fun t -> try unlink t.target_file with _ -> () - ) targets - ) - ); - let nr_disks = List.length targets in - mapi ( - fun i t -> - message (f_"Copying disk %d/%d to %s (%s)") - (i+1) nr_disks t.target_file t.target_format; - if verbose () then printf "%s%!" (string_of_target t); - - (* We noticed that qemu sometimes corrupts the qcow2 file on - * exit. This only seemed to happen with lazy_refcounts was - * used. The symptom was that the header wasn't written back - * to the disk correctly and the file appeared to have no - * backing file. Just sanity check this here. - *) - let overlay_file = t.target_overlay.ov_overlay_file in - if not ((new G.guestfs ())#disk_has_backing_file overlay_file) then - error (f_"internal error: qemu corrupted the overlay file"); - - (* Give the input module a chance to adjust the parameters - * of the overlay/backing file. This allows us to increase - * the readahead parameter when copying (see RHBZ#1151033 and - * RHBZ#1153589 for the gruesome details). - *) - input#adjust_overlay_parameters t.target_overlay; - - (* It turns out that libguestfs's disk creation code is - * considerably more flexible and easier to use than - * qemu-img, so create the disk explicitly using libguestfs - * then pass the 'qemu-img convert -n' option so qemu reuses - * the disk. - * - * Also we allow the output mode to actually create the disk - * image. This lets the output mode set ownership and - * permissions correctly if required. - *) - (* What output preallocation mode should we use? *) - let preallocation = - match t.target_format, output_alloc with - | "raw", Sparse -> Some "sparse" - | "raw", Preallocated -> Some "full" - | "qcow2", Sparse -> Some "off" (* ? *) - | "qcow2", Preallocated -> Some "metadata" - | _ -> None (* ignore -oa flag for other formats *) in - let compat = - match t.target_format with "qcow2" -> Some "1.1" | _ -> None in - output#disk_create - t.target_file t.target_format t.target_overlay.ov_virtual_size - ?preallocation ?compat; - - let cmd = - sprintf "qemu-img convert%s -n -f qcow2 -O %s %s %s" - (if not (quiet ()) then " -p" else "") - (quote t.target_format) (quote overlay_file) - (quote t.target_file) in - if verbose () then printf "%s\n%!" cmd; - let start_time = gettimeofday () in - if Sys.command cmd <> 0 then - error (f_"qemu-img command failed, see earlier errors"); - let end_time = gettimeofday () in - - (* Calculate the actual size on the target, returns an updated - * target structure. - *) - let t = actual_target_size t in - - (* If verbose, print the virtual and real copying rates. *) - let elapsed_time = end_time -. start_time in - if verbose () && elapsed_time > 0. then ( - let mbps size time = - Int64.to_float size /. 1024. /. 1024. *. 10. /. time - in - - printf "virtual copying rate: %.1f M bits/sec\n%!" - (mbps t.target_overlay.ov_virtual_size elapsed_time); - - match t.target_actual_size with - | None -> () - | Some actual -> - printf "real copying rate: %.1f M bits/sec\n%!" - (mbps actual elapsed_time) - ); - - (* If verbose, find out how close the estimate was. This is - * for developer information only - so we can increase the - * accuracy of the estimate. - *) - if verbose () then ( - match t.target_estimated_size, t.target_actual_size with - | None, None | None, Some _ | Some _, None | Some _, Some 0L -> () - | Some estimate, Some actual -> - let pc = - 100. *. Int64.to_float estimate /. Int64.to_float actual - -. 100. in - printf "%s: estimate %Ld (%s) versus actual %Ld (%s): %.1f%%" - t.target_overlay.ov_sd - estimate (human_size estimate) - actual (human_size actual) - pc; - if pc < 0. then printf " ! ESTIMATE TOO LOW !"; - printf "\n%!"; - ); - - t - ) targets - ) (* do_copy *) in + else copy_targets targets input output output_alloc in (* Create output metadata. *) message (f_"Creating output metadata"); @@ -905,19 +920,4 @@ and do_fstrim g no_trim inspect = ) ) fses -(* Update the target_actual_size field in the target structure. *) -and actual_target_size target = - { target with target_actual_size = du target.target_file } - -(* There's no OCaml binding for st_blocks, so run coreutils 'du' - * to get the used size in bytes. - *) -and du filename = - let cmd = sprintf "du --block-size=1 %s | awk '{print $1}'" (quote filename) in - let lines = external_command cmd in - (* Ignore errors because we want to avoid failures after copying. *) - match lines with - | line::_ -> (try Some (Int64.of_string line) with _ -> None) - | [] -> None - let () = run_main_and_handle_errors main -- 2.4.3

This makes it easier to navigate within the file (a tribute to C). Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 160 ++++++++++++++++++++++++++++++------------------------------- 1 file changed, 80 insertions(+), 80 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index 7888f0e..554b30d 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -646,85 +646,7 @@ let preserve_overlays overlays src_name = printf (f_"Overlay saved as %s [--debug-overlays]\n") saved_filename ) overlays -let rec main () = - (* Handle the command line. *) - let input, output, - debug_gc, debug_overlays, do_copy, network_map, no_trim, - output_alloc, output_format, output_name, print_source, root_choice = - Cmdline.parse_cmdline () in - - (* Print the version, easier than asking users to tell us. *) - if verbose () then - printf "%s: %s %s (%s)\n%!" - prog Config.package_name Config.package_version Config.host_cpu; - - if debug_gc then - at_exit (fun () -> Gc.compact()); - - 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 - - (* Open the guestfs handle. *) - message (f_"Opening the overlay"); - let g = new G.guestfs () in - if trace () then g#set_trace true; - if verbose () then g#set_verbose true; - g#set_network true; - List.iter ( - fun ({ov_overlay_file = overlay_file}) -> - g#add_drive_opts overlay_file - ~format:"qcow2" ~cachemode:"unsafe" ~discard:"besteffort" - ~copyonread:true - ) overlays; - - g#launch (); - - (* Inspection - this also mounts up the filesystems. *) - message (f_"Inspecting the overlay"); - let inspect = inspect_source g root_choice in - - let mpstats = get_mpstats g in - check_free_space mpstats; - check_target_free_space mpstats source targets output; - - let keep_serial_console = output#keep_serial_console in - let guestcaps = do_convert g inspect source keep_serial_console in - - g#umount_all (); - - if no_trim <> ["*"] && (do_copy || debug_overlays) then ( - (* Doing fstrim on all the filesystems reduces the transfer size - * because unused blocks are marked in the overlay and thus do - * not have to be copied. - *) - message (f_"Mapping filesystem data to avoid copying unused and blank areas"); - do_fstrim g no_trim inspect; - ); - - message (f_"Closing the overlay"); - g#umount_all (); - g#shutdown (); - g#close (); - - let target_firmware = get_target_firmware inspect guestcaps source output in - let target_buses = target_bus_assignment source targets guestcaps 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; - - 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. *) - -and inspect_source g root_choice = +let inspect_source g root_choice = let roots = g#inspect_os () in let roots = Array.to_list roots in @@ -865,7 +787,7 @@ and inspect_source g root_choice = (* Perform the fstrim. The trimming bit is easy. Dealing with the * [--no-trim] parameter .. not so much. *) -and do_fstrim g no_trim inspect = +let do_fstrim (g:G.guestfs) no_trim inspect = (* Get all filesystems. *) let fses = g#list_filesystems () in @@ -921,4 +843,82 @@ and do_fstrim g no_trim inspect = ) ) fses +let main () = + (* Handle the command line. *) + let input, output, + debug_gc, debug_overlays, do_copy, network_map, no_trim, + output_alloc, output_format, output_name, print_source, root_choice = + Cmdline.parse_cmdline () in + + (* Print the version, easier than asking users to tell us. *) + if verbose () then + printf "%s: %s %s (%s)\n%!" + prog Config.package_name Config.package_version Config.host_cpu; + + if debug_gc then + at_exit (fun () -> Gc.compact()); + + 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 + + (* Open the guestfs handle. *) + message (f_"Opening the overlay"); + let g = new G.guestfs () in + if trace () then g#set_trace true; + if verbose () then g#set_verbose true; + g#set_network true; + List.iter ( + fun ({ov_overlay_file = overlay_file}) -> + g#add_drive_opts overlay_file + ~format:"qcow2" ~cachemode:"unsafe" ~discard:"besteffort" + ~copyonread:true + ) overlays; + + g#launch (); + + (* Inspection - this also mounts up the filesystems. *) + message (f_"Inspecting the overlay"); + let inspect = inspect_source g root_choice in + + let mpstats = get_mpstats g in + check_free_space mpstats; + check_target_free_space mpstats source targets output; + + let keep_serial_console = output#keep_serial_console in + let guestcaps = do_convert g inspect source keep_serial_console in + + g#umount_all (); + + if no_trim <> ["*"] && (do_copy || debug_overlays) then ( + (* Doing fstrim on all the filesystems reduces the transfer size + * because unused blocks are marked in the overlay and thus do + * not have to be copied. + *) + message (f_"Mapping filesystem data to avoid copying unused and blank areas"); + do_fstrim g no_trim inspect; + ); + + message (f_"Closing the overlay"); + g#umount_all (); + g#shutdown (); + g#close (); + + let target_firmware = get_target_firmware inspect guestcaps source output in + let target_buses = target_bus_assignment source targets guestcaps 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; + + 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. *) + let () = run_main_and_handle_errors main -- 2.4.3

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index 9a21abc..e3a9cc1 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -843,6 +843,14 @@ let do_fstrim (g:G.guestfs) no_trim inspect = ) ) fses +let open_guestfs () = + (* Open the guestfs handle. *) + let g = new G.guestfs () in + if trace () then g#set_trace true; + if verbose () then g#set_verbose true; + g#set_network true; + g + let main () = (* Handle the command line. *) let input, output, @@ -863,12 +871,8 @@ let main () = let overlays = create_overlays source.s_disks in let targets = init_targets overlays source output output_format in - (* Open the guestfs handle. *) message (f_"Opening the overlay"); - let g = new G.guestfs () in - if trace () then g#set_trace true; - if verbose () then g#set_verbose true; - g#set_network true; + let g = open_guestfs () in List.iter ( fun ({ov_overlay_file = overlay_file}) -> g#add_drive_opts overlay_file -- 2.4.3

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> --- changes from v1: - make use of the preceding refactoring to simplify this patch, and avoid using possibly undefined objects on in_place paths. Note that I did *not* make it a totally separate scenario to avoid marginalizing it and because it shares a lot with the original one. - split out doc and test - put the new option in alphabetical order WRT others v2v/cmdline.ml | 7 ++++++- v2v/v2v.ml | 45 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/v2v/cmdline.ml b/v2v/cmdline.ml index eaf57dc..8383ce8 100644 --- a/v2v/cmdline.ml +++ b/v2v/cmdline.ml @@ -32,6 +32,7 @@ let parse_cmdline () = let do_copy = ref true in let input_conn = ref "" in let input_format = ref "" in + let in_place = ref false in let machine_readable = ref false in let output_conn = ref "" in let output_format = ref "" in @@ -147,6 +148,7 @@ let parse_cmdline () = "-ic", Arg.Set_string input_conn, "uri " ^ s_"Libvirt URI"; "-if", Arg.Set_string input_format, "format " ^ s_"Input format (for -i disk)"; + "--in-place", Arg.Set in_place, " " ^ s_"Only tune the guest in the input VM"; "--short-options", Arg.Unit display_short_options, " " ^ s_"List short options"; "--long-options", Arg.Unit display_long_options, " " ^ s_"List long options"; "--machine-readable", Arg.Set machine_readable, " " ^ s_"Make output machine readable"; @@ -217,6 +219,7 @@ read the man page virt-v2v(1). let input_conn = match !input_conn with "" -> None | s -> Some s in let input_format = match !input_format with "" -> None | s -> Some s 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 @@ -305,6 +308,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 -> @@ -385,6 +390,6 @@ read the man page virt-v2v(1). vmtype output_alloc in input, output, - debug_gc, debug_overlays, do_copy, network_map, no_trim, + debug_gc, 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 1228316..88bbbaa 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -860,10 +860,21 @@ let populate_overlays (g:G.guestfs) overlays = ~copyonread:true ) overlays +let 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 + let main () = (* Handle the command line. *) let input, output, - debug_gc, debug_overlays, do_copy, network_map, no_trim, + debug_gc, debug_overlays, do_copy, in_place, network_map, no_trim, output_alloc, output_format, output_name, print_source, root_choice = Cmdline.parse_cmdline () in @@ -877,24 +888,35 @@ let 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 + let overlays = + if not in_place then create_overlays source.s_disks + else [] in + let targets = + if not in_place then init_targets overlays source output output_format + else [] in - message (f_"Opening the overlay"); + let guestfs_kind = + if not in_place then "overlay" else "source VM" in + message (f_"Opening the %s") guestfs_kind; let g = open_guestfs () in - populate_overlays g overlays; + + if not in_place then populate_overlays g overlays + else populate_disks g source.s_disks; g#launch (); (* Inspection - this also mounts up the filesystems. *) - message (f_"Inspecting the overlay"); + message (f_"Inspecting the %s") guestfs_kind; let inspect = inspect_source g root_choice in let mpstats = get_mpstats g in check_free_space mpstats; - check_target_free_space mpstats source targets output; + if not in_place then + check_target_free_space mpstats source targets output; - let keep_serial_console = output#keep_serial_console in + let keep_serial_console = + if not in_place then output#keep_serial_console + else true in let guestcaps = do_convert g inspect source keep_serial_console in if no_trim <> ["*"] && (do_copy || debug_overlays) then ( @@ -906,9 +928,14 @@ let main () = do_fstrim g no_trim inspect; ); - message (f_"Closing the overlay"); + message (f_"Closing the %s") guestfs_kind; g#close (); + if in_place then ( + message (f_"Finishing off"); + exit 0 + ); + let target_firmware = get_target_firmware inspect guestcaps source output in let target_buses = target_bus_assignment source targets guestcaps in let targets = -- 2.4.3

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- changes from v1: - split out from code and doc - put the new option in alphabetical order WRT others v2v/Makefile.am | 1 + v2v/test-v2v-in-place.sh | 101 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) create mode 100755 v2v/test-v2v-in-place.sh diff --git a/v2v/Makefile.am b/v2v/Makefile.am index e957a15..35df563 100644 --- a/v2v/Makefile.am +++ b/v2v/Makefile.am @@ -232,6 +232,7 @@ TESTS += \ test-v2v-cdrom.sh \ test-v2v-i-ova.sh \ test-v2v-i-disk.sh \ + test-v2v-in-place.sh \ test-v2v-machine-readable.sh \ test-v2v-networks-and-bridges.sh \ test-v2v-no-copy.sh \ diff --git a/v2v/test-v2v-in-place.sh b/v2v/test-v2v-in-place.sh new file mode 100755 index 0000000..bd10aa2 --- /dev/null +++ b/v2v/test-v2v-in-place.sh @@ -0,0 +1,101 @@ +#!/bin/bash - +# libguestfs virt-v2v test script +# Copyright (C) 2014 Red Hat Inc. +# Copyright (C) 2015 Parallels IP Holdings GmbH. +# +# 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 --in-place. + +unset CDPATH +export LANG=C +set -e + +if [ -n "$SKIP_TEST_V2V_IN_PLACE_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 + +# You shouldn't be running the tests as root anyway, but in this case +# it's especially bad because we don't want to start creating guests +# or storage pools in the system namespace. +if [ "$(id -u)" -eq 0 ]; then + echo "$0: test skipped because you're running tests as root. Don't do that!" + exit 77 +fi + +here_dir="$(cd $(dirname $0) && pwd)" +guests_dir="$(cd $here_dir/../tests/guests && pwd)" + +img_base="$guests_dir/windows.img" +if ! test -f $img_base || ! test -s $img_base; then + echo "$0: test skipped because phony Windows image was not created" + exit 77 +fi + +virt_tools_data_dir=${VIRT_TOOLS_DATA_DIR:-/usr/share/virt-tools} +if ! test -r $virt_tools_data_dir/rhsrvany.exe; then + echo "$0: test skipped because rhsrvany.exe is not installed" + exit 77 +fi + +img="$here_dir/test-v2v-in-place.qcow2" +rm -f $img +qemu-img create -f qcow2 -b $img_base -o compat=1.1,backing_fmt=raw $img +md5="$(md5sum $img_base)" + +libvirt_xml="$here_dir/test-v2v-in-place.xml" +rm -f $libvirt_xml +n=windows-overlay +cat > $libvirt_xml <<EOF +<node> + <domain type='test'> + <name>$n</name> + <memory>1048576</memory> + <os> + <type>hvm</type> + <boot dev='hd'/> + </os> + <devices> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='$img'/> + <target dev='vda' bus='virtio'/> + </disk> + </devices> + </domain> +</node> +EOF + +$VG virt-v2v --debug-gc -i libvirt -ic "test://$libvirt_xml" $n --in-place + +# Test some aspects of the target disk image. +guestfish --ro -a $img -i <<EOF + is-dir "/Program Files/Red Hat/Firstboot" + is-file "/Program Files/Red Hat/Firstboot/firstboot.bat" + is-dir "/Program Files/Red Hat/Firstboot/scripts" + is-dir "/Windows/Drivers/VirtIO" +EOF + +# Test the base image remained untouched +test "$md5" = "$(md5sum $img_base)" + +# Clean up. +rm -r $img -- 2.4.3

Opening the source VM and amending the properties in its internal representation in accordance with command-line options fit nicely into two isolated functions. Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 124 ++++++++++++++++++++++++++++++++----------------------------- 1 file changed, 65 insertions(+), 59 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index fe16131..564c5da 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -55,65 +55,8 @@ let rec main () = printf "%s: %s %s (%s)\n%!" prog Config.package_name Config.package_version Config.host_cpu; - message (f_"Opening the source %s") input#as_options; - let source = input#source () in - - (* Print source and stop. *) - if print_source then ( - printf (f_"Source guest information (--print-source option):\n"); - printf "\n"; - printf "%s\n" (string_of_source source); - exit 0 - ); - - if verbose () then printf "%s%!" (string_of_source source); - - (match source.s_hypervisor with - | OtherHV hv -> - warning (f_"unknown source hypervisor ('%s') in metadata") hv - | _ -> () - ); - - assert (source.s_name <> ""); - assert (source.s_memory > 0L); - assert (source.s_vcpu >= 1); - if source.s_disks = [] then - error (f_"source has no hard disks!"); - List.iter ( - fun disk -> - assert (disk.s_qemu_uri <> ""); - ) source.s_disks; - - (* Map source name. *) - let source = - match output_name with - | None -> source - (* Note the s_orig_name field retains the original name in case we - * need it for some reason. - *) - | Some name -> { source with s_name = name } in - - (* Map networks and bridges. *) - let source = - let { s_nics = nics } = source in - let nics = List.map ( - fun ({ s_vnet_type = t; s_vnet = vnet } as nic) -> - try - (* Look for a --network or --bridge parameter which names this - * network/bridge (eg. --network in:out). - *) - let new_name = List.assoc (t, vnet) network_map in - { nic with s_vnet = new_name } - with Not_found -> - try - (* Not found, so look for a default mapping (eg. --network out). *) - let new_name = List.assoc (t, "") network_map in - { nic with s_vnet = new_name } - with Not_found -> - (* Not found, so return the original NIC unchanged. *) - nic - ) nics in - { source with s_nics = nics } in + let source = open_source input print_source in + let source = amend_source source output_name network_map in (* Create a qcow2 v3 overlay to protect the source image(s). There * is a specific reason to use the newer qcow2 variant: Because the @@ -454,6 +397,69 @@ let rec main () = message (f_"Finishing off"); delete_target_on_exit := false (* Don't delete target on exit. *) +and open_source input print_source = + message (f_"Opening the source %s") input#as_options; + let source = input#source () in + + (* Print source and stop. *) + if print_source then ( + printf (f_"Source guest information (--print-source option):\n"); + printf "\n"; + printf "%s\n" (string_of_source source); + exit 0 + ); + + if verbose () then printf "%s%!" (string_of_source source); + + (match source.s_hypervisor with + | OtherHV hv -> + warning (f_"unknown source hypervisor ('%s') in metadata") hv + | _ -> () + ); + + assert (source.s_name <> ""); + assert (source.s_memory > 0L); + assert (source.s_vcpu >= 1); + if source.s_disks = [] then + error (f_"source has no hard disks!"); + List.iter ( + fun disk -> + assert (disk.s_qemu_uri <> ""); + ) source.s_disks; + + source + +and amend_source source output_name network_map = + (* Map source name. *) + let source = + match output_name with + | None -> source + (* Note the s_orig_name field retains the original name in case we + * need it for some reason. + *) + | Some name -> { source with s_name = name } in + + (* Map networks and bridges. *) + let nics = List.map ( + fun ({ s_vnet_type = t; s_vnet = vnet } as nic) -> + try + (* Look for a --network or --bridge parameter which names this + * network/bridge (eg. --network in:out). + *) + let new_name = List.assoc (t, vnet) network_map in + { nic with s_vnet = new_name } + with Not_found -> + try + (* Not found, so look for a default mapping (eg. --network out). *) + let new_name = List.assoc (t, "") network_map in + { nic with s_vnet = new_name } + with Not_found -> + (* Not found, so return the original NIC unchanged. *) + nic + ) source.s_nics in + + { source with s_nics = nics } + and inspect_source g root_choice = let roots = g#inspect_os () in let roots = Array.to_list roots in -- 2.4.3

Iterating over source disks and creating temporary overlays for easy rollback fits nicely into a separate function. In addition, determining their size doesn't need to wait until the guestfs is launched: the size can be obtained via disk_virtual_size() method. Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 91 +++++++++++++++++++++++++++++--------------------------------- 1 file changed, 42 insertions(+), 49 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index 564c5da..155eb83 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -57,40 +57,7 @@ let rec main () = let source = open_source input print_source in let source = amend_source source output_name network_map in - - (* Create a qcow2 v3 overlay to protect the source image(s). There - * is a specific reason to use the newer qcow2 variant: Because the - * L2 table can store zero clusters efficiently, and because - * discarded blocks are stored as zero clusters, this should allow us - * to fstrim/blkdiscard and avoid copying significant parts of the - * data over the wire. - *) - message (f_"Creating an overlay to protect the source from being modified"); - let overlay_dir = (new Guestfs.guestfs ())#get_cachedir () in - let overlays = - List.map ( - fun ({ s_qemu_uri = qemu_uri; s_format = format } as source) -> - let overlay_file = - Filename.temp_file ~temp_dir:overlay_dir "v2vovl" ".qcow2" in - unlink_on_exit overlay_file; - - let options = - "compat=1.1" ^ - (match format with None -> "" - | Some fmt -> ",backing_fmt=" ^ fmt) in - let cmd = - sprintf "qemu-img create -q -f qcow2 -b %s -o %s %s" - (quote qemu_uri) (quote options) overlay_file in - if verbose () then printf "%s\n%!" cmd; - if Sys.command cmd <> 0 then - error (f_"qemu-img command failed, see earlier errors"); - - (* Sanity check created overlay (see below). *) - if not ((new G.guestfs ())#disk_has_backing_file overlay_file) then - error (f_"internal error: qemu-img did not create overlay with backing file"); - - overlay_file, source - ) source.s_disks in + let overlays = create_overlays source.s_disks in (* Open the guestfs handle. *) message (f_"Opening the overlay"); @@ -100,7 +67,7 @@ let rec main () = if verbose () then g#set_verbose true; g#set_network true; List.iter ( - fun (overlay_file, _) -> + fun ({ov_overlay_file = overlay_file}) -> g#add_drive_opts overlay_file ~format:"qcow2" ~cachemode:"unsafe" ~discard:"besteffort" ~copyonread:true @@ -108,20 +75,6 @@ let rec main () = g#launch (); - (* Create the list of overlays structs. Query each disk for its - * virtual size, and fill in a few other fields. - *) - let overlays = - mapi ( - fun i (overlay_file, source) -> - let sd = "sd" ^ drive_name i in - let dev = "/dev/" ^ sd in - let vsize = g#blockdev_getsize64 dev in - - { ov_overlay_file = overlay_file; ov_sd = sd; - ov_virtual_size = vsize; ov_source = source } - ) overlays in - (* Work out where we will write the final output. Do this early * just so we can display errors to the user before doing too much * work. @@ -385,6 +338,7 @@ let rec main () = (* Save overlays if --debug-overlays option was used. *) if debug_overlays then ( + let overlay_dir = (new Guestfs.guestfs ())#get_cachedir () in List.iter ( fun ov -> let saved_filename = @@ -460,6 +414,45 @@ and amend_source source output_name network_map = { source with s_nics = nics } +and create_overlays src_disks = + (* Create a qcow2 v3 overlay to protect the source image(s). There + * is a specific reason to use the newer qcow2 variant: Because the + * L2 table can store zero clusters efficiently, and because + * discarded blocks are stored as zero clusters, this should allow us + * to fstrim/blkdiscard and avoid copying significant parts of the + * data over the wire. + *) + message (f_"Creating an overlay to protect the source from being modified"); + let overlay_dir = (new Guestfs.guestfs ())#get_cachedir () in + List.mapi ( + fun i ({ s_qemu_uri = qemu_uri; s_format = format } as source) -> + let overlay_file = + Filename.temp_file ~temp_dir:overlay_dir "v2vovl" ".qcow2" in + unlink_on_exit overlay_file; + + let options = + "compat=1.1" ^ + (match format with None -> "" + | Some fmt -> ",backing_fmt=" ^ fmt) in + let cmd = + sprintf "qemu-img create -q -f qcow2 -b %s -o %s %s" + (quote qemu_uri) (quote options) overlay_file in + if verbose () then printf "%s\n%!" cmd; + if Sys.command cmd <> 0 then + error (f_"qemu-img command failed, see earlier errors"); + + (* Sanity check created overlay (see below). *) + if not ((new G.guestfs ())#disk_has_backing_file overlay_file) then + error (f_"internal error: qemu-img did not create overlay with backing file"); + + let sd = "sd" ^ drive_name i in + + let vsize = (new G.guestfs ())#disk_virtual_size overlay_file in + + { ov_overlay_file = overlay_file; ov_sd = sd; + ov_virtual_size = vsize; ov_source = source } + ) src_disks + and inspect_source g root_choice = let roots = g#inspect_os () in let roots = Array.to_list roots in -- 2.4.3

Besides, it doesn't need guestfs handle open so move this step earlier in the process. Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 77 ++++++++++++++++++++++++++++++++------------------------------ 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index 155eb83..4257b8d 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -58,6 +58,7 @@ 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 (* Open the guestfs handle. *) message (f_"Opening the overlay"); @@ -75,43 +76,6 @@ let rec main () = g#launch (); - (* Work out where we will write the final output. Do this early - * just so we can display errors to the user before doing too much - * work. - *) - message (f_"Initializing the target %s") output#as_options; - let targets = - List.map ( - fun ov -> - (* What output format should we use? *) - let format = - match output_format, ov.ov_source.s_format with - | Some format, _ -> format (* -of overrides everything *) - | None, Some format -> format (* same as backing format *) - | None, None -> - error (f_"disk %s (%s) has no defined format.\n\nThe input metadata did not define the disk format (eg. raw/qcow2/etc) of this disk, and so virt-v2v will try to autodetect the format when reading it.\n\nHowever because the input format was not defined, we do not know what output format you want to use. You have two choices: either define the original format in the source metadata, or use the '-of' option to force the output format") ov.ov_sd ov.ov_source.s_qemu_uri in - - (* What really happens here is that the call to #disk_create - * below fails if the format is not raw or qcow2. We would - * have to extend libguestfs to support further formats, which - * is trivial, but we'd want to check that the files being - * created by qemu-img really work. In any case, fail here, - * early, not below, later. - *) - if format <> "raw" && format <> "qcow2" then - error (f_"output format should be 'raw' or 'qcow2'.\n\nUse the '-of <format>' option to select a different output format for the converted guest.\n\nOther output formats are not supported at the moment, although might be considered in future."); - - (* output#prepare_targets will fill in the target_file field. - * estimate_target_size will fill in the target_estimated_size field. - * actual_target_size will fill in the target_actual_size field. - *) - { target_file = ""; target_format = format; - target_estimated_size = None; - target_actual_size = None; - target_overlay = ov } - ) overlays in - let targets = output#prepare_targets source targets in - (* Inspection - this also mounts up the filesystems. *) message (f_"Inspecting the overlay"); let inspect = inspect_source g root_choice in @@ -453,6 +417,45 @@ and create_overlays src_disks = ov_virtual_size = vsize; ov_source = source } ) src_disks +and init_targets overlays source output output_format = + (* Work out where we will write the final output. Do this early + * just so we can display errors to the user before doing too much + * work. + *) + message (f_"Initializing the target %s") output#as_options; + let targets = + List.map ( + fun ov -> + (* What output format should we use? *) + let format = + match output_format, ov.ov_source.s_format with + | Some format, _ -> format (* -of overrides everything *) + | None, Some format -> format (* same as backing format *) + | None, None -> + error (f_"disk %s (%s) has no defined format.\n\nThe input metadata did not define the disk format (eg. raw/qcow2/etc) of this disk, and so virt-v2v will try to autodetect the format when reading it.\n\nHowever because the input format was not defined, we do not know what output format you want to use. You have two choices: either define the original format in the source metadata, or use the '-of' option to force the output format") ov.ov_sd ov.ov_source.s_qemu_uri in + + (* What really happens here is that the call to #disk_create + * below fails if the format is not raw or qcow2. We would + * have to extend libguestfs to support further formats, which + * is trivial, but we'd want to check that the files being + * created by qemu-img really work. In any case, fail here, + * early, not below, later. + *) + if format <> "raw" && format <> "qcow2" then + error (f_"output format should be 'raw' or 'qcow2'.\n\nUse the '-of <format>' option to select a different output format for the converted guest.\n\nOther output formats are not supported at the moment, although might be considered in future."); + + (* output#prepare_targets will fill in the target_file field. + * estimate_target_size will fill in the target_estimated_size field. + * actual_target_size will fill in the target_actual_size field. + *) + { target_file = ""; target_format = format; + target_estimated_size = None; + target_actual_size = None; + target_overlay = ov } + ) overlays in + + output#prepare_targets source targets + and inspect_source g root_choice = let roots = g#inspect_os () in let roots = Array.to_list roots in -- 2.4.3

Factor the size checks out to separate functions. Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 53 ++++++++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index 4257b8d..a2b6f52 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -80,32 +80,9 @@ let rec main () = message (f_"Inspecting the overlay"); let inspect = inspect_source g root_choice in - (* The guest free disk space check and the target free space - * estimation both require statvfs information from mountpoints, so - * get that information first. - *) - let mpstats = List.map ( - fun (dev, path) -> - let statvfs = g#statvfs path in - let vfs = g#vfs_type dev in - { mp_dev = dev; mp_path = path; mp_statvfs = statvfs; mp_vfs = vfs } - ) (g#mountpoints ()) in - - if verbose () then ( - (* This is useful for debugging speed / fstrim issues. *) - printf "mpstats:\n"; - List.iter (print_mpstat Pervasives.stdout) mpstats - ); - - (* Check there is enough free space to perform conversion. *) - message (f_"Checking for sufficient free disk space in the guest"); + let mpstats = get_mpstats g in check_free_space mpstats; - - (* Estimate space required on target for each disk. Note this is a max. *) - message (f_"Estimating space required on target for each disk"); - let targets = estimate_target_size mpstats targets in - - output#check_target_free_space source targets; + check_target_free_space mpstats source targets output; (* Conversion. *) let guestcaps = @@ -594,11 +571,29 @@ and inspect_source g root_choice = if verbose () then printf "%s%!" (string_of_inspect inspect); inspect +and get_mpstats g = + (* Collect statvfs information from the guest mountpoints. *) + let mpstats = List.map ( + fun (dev, path) -> + let statvfs = g#statvfs path in + let vfs = g#vfs_type dev in + { mp_dev = dev; mp_path = path; mp_statvfs = statvfs; mp_vfs = vfs } + ) (g#mountpoints ()) in + + if verbose () then ( + (* This is useful for debugging speed / fstrim issues. *) + printf "mpstats:\n"; + List.iter (print_mpstat Pervasives.stdout) mpstats + ); + + mpstats + (* Conversion can fail if there is no space on the guest filesystems * (RHBZ#1139543). To avoid this situation, check there is some * headroom. Mainly we care about the root filesystem. *) and check_free_space mpstats = + message (f_"Checking for sufficient free disk space in the guest"); List.iter ( fun { mp_path = mp; mp_statvfs = { G.bfree = bfree; blocks = blocks; bsize = bsize } } -> @@ -816,6 +811,14 @@ and estimate_target_size mpstats targets = targets ) +and check_target_free_space mpstats source targets output = + + (* Estimate space required on target for each disk. Note this is a max. *) + message (f_"Estimating space required on target for each disk"); + let targets = estimate_target_size mpstats targets in + + output#check_target_free_space source targets + (* Update the target_actual_size field in the target structure. *) and actual_target_size target = { target with target_actual_size = du target.target_file } -- 2.4.3

Factor out perfoming the actual convertion of the guest, which includes determinig the appropriate guest os specific conversion module and running its conversion routine. Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 59 ++++++++++++++++++++++++++++++----------------------------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index a2b6f52..633c29f 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -84,35 +84,8 @@ let rec main () = check_free_space mpstats; check_target_free_space mpstats source targets output; - (* Conversion. *) - let guestcaps = - (match inspect.i_product_name with - | "unknown" -> - message (f_"Converting the guest to run on KVM") - | prod -> - message (f_"Converting %s to run on KVM") prod - ); - - (* RHEV doesn't support serial console so remove any on conversion. *) - let keep_serial_console = output#keep_serial_console in - - let conversion_name, convert = - try Modules_list.find_convert_module inspect - with Not_found -> - error (f_"virt-v2v is unable to convert this guest type (%s/%s)") - inspect.i_type inspect.i_distro in - if verbose () then printf "picked conversion module %s\n%!" conversion_name; - let guestcaps = convert ~keep_serial_console g inspect source in - if verbose () then printf "%s%!" (string_of_guestcaps guestcaps); - guestcaps in - - (* Did we manage to install virtio drivers? *) - if not (quiet ()) then ( - if guestcaps.gcaps_block_bus = Virtio_blk then - info (f_"This guest has virtio drivers installed.") - else - info (f_"This guest does not have virtio drivers installed."); - ); + let keep_serial_console = output#keep_serial_console in + let guestcaps = do_convert g inspect source keep_serial_console in g#umount_all (); @@ -819,6 +792,34 @@ and check_target_free_space mpstats source targets output = output#check_target_free_space source targets +and do_convert g inspect source keep_serial_console = + (* Conversion. *) + (match inspect.i_product_name with + | "unknown" -> + message (f_"Converting the guest to run on KVM") + | prod -> + message (f_"Converting %s to run on KVM") prod + ); + + let conversion_name, convert = + try Modules_list.find_convert_module inspect + with Not_found -> + error (f_"virt-v2v is unable to convert this guest type (%s/%s)") + inspect.i_type inspect.i_distro in + if verbose () then printf "picked conversion module %s\n%!" conversion_name; + let guestcaps = convert ~keep_serial_console g inspect source in + if verbose () then printf "%s%!" (string_of_guestcaps guestcaps); + + (* Did we manage to install virtio drivers? *) + if not (quiet ()) then ( + if guestcaps.gcaps_block_bus = Virtio_blk then + info (f_"This guest has virtio drivers installed.") + else + info (f_"This guest does not have virtio drivers installed."); + ); + + guestcaps + (* Update the target_actual_size field in the target structure. *) and actual_target_size target = { target with target_actual_size = du target.target_file } -- 2.4.3

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 45 +++++++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 20 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index 633c29f..afffde2 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -103,26 +103,7 @@ let rec main () = g#shutdown (); g#close (); - (* Does the guest require UEFI on the target? *) - message (f_"Checking if the guest needs BIOS or UEFI to boot"); - let target_firmware = - match source.s_firmware with - | BIOS -> TargetBIOS - | UEFI -> TargetUEFI - | UnknownFirmware -> - if inspect.i_uefi then TargetUEFI else TargetBIOS in - let supported_firmware = output#supported_firmware in - if not (List.mem target_firmware supported_firmware) then - error (f_"this guest cannot run on the target, because the target does not support %s firmware (supported firmware on target: %s)") - (string_of_target_firmware target_firmware) - (String.concat " " - (List.map string_of_target_firmware supported_firmware)); - - output#check_target_firmware guestcaps target_firmware; - - (match target_firmware with - | TargetBIOS -> () - | TargetUEFI -> info (f_"This guest requires UEFI on the target to boot.")); + 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 @@ -820,6 +801,30 @@ and do_convert g inspect source keep_serial_console = guestcaps +and get_target_firmware inspect guestcaps source output = + (* Does the guest require UEFI on the target? *) + message (f_"Checking if the guest needs BIOS or UEFI to boot"); + let target_firmware = + match source.s_firmware with + | BIOS -> TargetBIOS + | UEFI -> TargetUEFI + | UnknownFirmware -> + if inspect.i_uefi then TargetUEFI else TargetBIOS in + let supported_firmware = output#supported_firmware in + if not (List.mem target_firmware supported_firmware) then + error (f_"this guest cannot run on the target, because the target does not support %s firmware (supported firmware on target: %s)") + (string_of_target_firmware target_firmware) + (String.concat " " + (List.map string_of_target_firmware supported_firmware)); + + output#check_target_firmware guestcaps target_firmware; + + (match target_firmware with + | TargetBIOS -> () + | TargetUEFI -> info (f_"This guest requires UEFI on the target to boot.")); + + target_firmware + (* Update the target_actual_size field in the target structure. *) and actual_target_size target = { target with target_actual_size = du target.target_file } -- 2.4.3

Factor out copying the overlays to final disk images into a separate function. Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 227 +++++++++++++++++++++++++++++++------------------------------ 1 file changed, 114 insertions(+), 113 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index afffde2..703038c 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -110,121 +110,9 @@ let rec main () = if verbose () then printf "%s%!" (string_of_target_buses target_buses); - let delete_target_on_exit = ref true in - let targets = if not do_copy then targets - else ( - (* Copy the source to the output. *) - at_exit (fun () -> - if !delete_target_on_exit then ( - List.iter ( - fun t -> try unlink t.target_file with _ -> () - ) targets - ) - ); - let nr_disks = List.length targets in - mapi ( - fun i t -> - message (f_"Copying disk %d/%d to %s (%s)") - (i+1) nr_disks t.target_file t.target_format; - if verbose () then printf "%s%!" (string_of_target t); - - (* We noticed that qemu sometimes corrupts the qcow2 file on - * exit. This only seemed to happen with lazy_refcounts was - * used. The symptom was that the header wasn't written back - * to the disk correctly and the file appeared to have no - * backing file. Just sanity check this here. - *) - let overlay_file = t.target_overlay.ov_overlay_file in - if not ((new G.guestfs ())#disk_has_backing_file overlay_file) then - error (f_"internal error: qemu corrupted the overlay file"); - - (* Give the input module a chance to adjust the parameters - * of the overlay/backing file. This allows us to increase - * the readahead parameter when copying (see RHBZ#1151033 and - * RHBZ#1153589 for the gruesome details). - *) - input#adjust_overlay_parameters t.target_overlay; - - (* It turns out that libguestfs's disk creation code is - * considerably more flexible and easier to use than - * qemu-img, so create the disk explicitly using libguestfs - * then pass the 'qemu-img convert -n' option so qemu reuses - * the disk. - * - * Also we allow the output mode to actually create the disk - * image. This lets the output mode set ownership and - * permissions correctly if required. - *) - (* What output preallocation mode should we use? *) - let preallocation = - match t.target_format, output_alloc with - | ("raw"|"qcow2"), Sparse -> Some "sparse" - | ("raw"|"qcow2"), Preallocated -> Some "full" - | _ -> None (* ignore -oa flag for other formats *) in - let compat = - match t.target_format with "qcow2" -> Some "1.1" | _ -> None in - output#disk_create - t.target_file t.target_format t.target_overlay.ov_virtual_size - ?preallocation ?compat; - - let cmd = - sprintf "qemu-img convert%s -n -f qcow2 -O %s %s %s" - (if not (quiet ()) then " -p" else "") - (quote t.target_format) (quote overlay_file) - (quote t.target_file) in - if verbose () then printf "%s\n%!" cmd; - let start_time = gettimeofday () in - if Sys.command cmd <> 0 then - error (f_"qemu-img command failed, see earlier errors"); - let end_time = gettimeofday () in - - (* Calculate the actual size on the target, returns an updated - * target structure. - *) - let t = actual_target_size t in - - (* If verbose, print the virtual and real copying rates. *) - let elapsed_time = end_time -. start_time in - if verbose () && elapsed_time > 0. then ( - let mbps size time = - Int64.to_float size /. 1024. /. 1024. *. 10. /. time - in - - printf "virtual copying rate: %.1f M bits/sec\n%!" - (mbps t.target_overlay.ov_virtual_size elapsed_time); - - match t.target_actual_size with - | None -> () - | Some actual -> - printf "real copying rate: %.1f M bits/sec\n%!" - (mbps actual elapsed_time) - ); - - (* If verbose, find out how close the estimate was. This is - * for developer information only - so we can increase the - * accuracy of the estimate. - *) - if verbose () then ( - match t.target_estimated_size, t.target_actual_size with - | None, None | None, Some _ | Some _, None | Some _, Some 0L -> () - | Some estimate, Some actual -> - let pc = - 100. *. Int64.to_float estimate /. Int64.to_float actual - -. 100. in - printf "%s: estimate %Ld (%s) versus actual %Ld (%s): %.1f%%" - t.target_overlay.ov_sd - estimate (human_size estimate) - actual (human_size actual) - pc; - if pc < 0. then printf " ! ESTIMATE TOO LOW !"; - printf "\n%!"; - ); - - t - ) targets - ) (* do_copy *) in + else copy_targets targets input output output_alloc in (* Create output metadata. *) message (f_"Creating output metadata"); @@ -825,6 +713,119 @@ and get_target_firmware inspect guestcaps source output = target_firmware +and delete_target_on_exit = ref true + +and copy_targets targets input output output_alloc = + (* Copy the source to the output. *) + at_exit (fun () -> + if !delete_target_on_exit then ( + List.iter ( + fun t -> try unlink t.target_file with _ -> () + ) targets + ) + ); + let nr_disks = List.length targets in + mapi ( + fun i t -> + message (f_"Copying disk %d/%d to %s (%s)") + (i+1) nr_disks t.target_file t.target_format; + if verbose () then printf "%s%!" (string_of_target t); + + (* We noticed that qemu sometimes corrupts the qcow2 file on + * exit. This only seemed to happen with lazy_refcounts was + * used. The symptom was that the header wasn't written back + * to the disk correctly and the file appeared to have no + * backing file. Just sanity check this here. + *) + let overlay_file = t.target_overlay.ov_overlay_file in + if not ((new G.guestfs ())#disk_has_backing_file overlay_file) then + error (f_"internal error: qemu corrupted the overlay file"); + + (* Give the input module a chance to adjust the parameters + * of the overlay/backing file. This allows us to increase + * the readahead parameter when copying (see RHBZ#1151033 and + * RHBZ#1153589 for the gruesome details). + *) + input#adjust_overlay_parameters t.target_overlay; + + (* It turns out that libguestfs's disk creation code is + * considerably more flexible and easier to use than + * qemu-img, so create the disk explicitly using libguestfs + * then pass the 'qemu-img convert -n' option so qemu reuses + * the disk. + * + * Also we allow the output mode to actually create the disk + * image. This lets the output mode set ownership and + * permissions correctly if required. + *) + (* What output preallocation mode should we use? *) + let preallocation = + match t.target_format, output_alloc with + | ("raw"|"qcow2"), Sparse -> Some "sparse" + | ("raw"|"qcow2"), Preallocated -> Some "full" + | _ -> None (* ignore -oa flag for other formats *) in + let compat = + match t.target_format with "qcow2" -> Some "1.1" | _ -> None in + output#disk_create + t.target_file t.target_format t.target_overlay.ov_virtual_size + ?preallocation ?compat; + + let cmd = + sprintf "qemu-img convert%s -n -f qcow2 -O %s %s %s" + (if not (quiet ()) then " -p" else "") + (quote t.target_format) (quote overlay_file) + (quote t.target_file) in + if verbose () then printf "%s\n%!" cmd; + let start_time = gettimeofday () in + if Sys.command cmd <> 0 then + error (f_"qemu-img command failed, see earlier errors"); + let end_time = gettimeofday () in + + (* Calculate the actual size on the target, returns an updated + * target structure. + *) + let t = actual_target_size t in + + (* If verbose, print the virtual and real copying rates. *) + let elapsed_time = end_time -. start_time in + if verbose () && elapsed_time > 0. then ( + let mbps size time = + Int64.to_float size /. 1024. /. 1024. *. 10. /. time + in + + printf "virtual copying rate: %.1f M bits/sec\n%!" + (mbps t.target_overlay.ov_virtual_size elapsed_time); + + match t.target_actual_size with + | None -> () + | Some actual -> + printf "real copying rate: %.1f M bits/sec\n%!" + (mbps actual elapsed_time) + ); + + (* If verbose, find out how close the estimate was. This is + * for developer information only - so we can increase the + * accuracy of the estimate. + *) + if verbose () then ( + match t.target_estimated_size, t.target_actual_size with + | None, None | None, Some _ | Some _, None | Some _, Some 0L -> () + | Some estimate, Some actual -> + let pc = + 100. *. Int64.to_float estimate /. Int64.to_float actual + -. 100. in + printf "%s: estimate %Ld (%s) versus actual %Ld (%s): %.1f%%" + t.target_overlay.ov_sd + estimate (human_size estimate) + actual (human_size actual) + pc; + if pc < 0. then printf " ! ESTIMATE TOO LOW !"; + printf "\n%!"; + ); + + t + ) targets + (* Update the target_actual_size field in the target structure. *) and actual_target_size target = { target with target_actual_size = du target.target_file } -- 2.4.3

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index 703038c..cc36422 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -119,17 +119,7 @@ let rec main () = output#create_metadata source targets target_buses guestcaps inspect target_firmware; - (* Save overlays if --debug-overlays option was used. *) - if debug_overlays then ( - let overlay_dir = (new Guestfs.guestfs ())#get_cachedir () in - List.iter ( - fun ov -> - let saved_filename = - sprintf "%s/%s-%s.qcow2" overlay_dir source.s_name ov.ov_sd in - rename ov.ov_overlay_file saved_filename; - printf (f_"Overlay saved as %s [--debug-overlays]\n") saved_filename - ) overlays - ); + 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. *) @@ -916,4 +906,15 @@ and target_bus_assignment source targets guestcaps = target_ide_bus = !ide_bus; target_scsi_bus = !scsi_bus } +and preserve_overlays overlays src_name = + (* Save overlays if --debug-overlays option was used. *) + let overlay_dir = (new Guestfs.guestfs ())#get_cachedir () in + List.iter ( + fun ov -> + let saved_filename = + sprintf "%s/%s-%s.qcow2" overlay_dir src_name ov.ov_sd in + rename ov.ov_overlay_file saved_filename; + printf (f_"Overlay saved as %s [--debug-overlays]\n") saved_filename + ) overlays + let () = run_main_and_handle_errors main -- 2.4.3

umount_all() and shutdown() are performed by guestfs automatically on close(), so drop explicit calls to them right before close(). Also umount_all() in the middle of processing doesn't look justified so drop it, too. (The only step following it is do_fstrim() which does mounting/umounting on its own). Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index cc36422..c28905d 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -87,8 +87,6 @@ let rec main () = let keep_serial_console = output#keep_serial_console in let guestcaps = do_convert g inspect source keep_serial_console in - g#umount_all (); - if no_trim <> ["*"] && (do_copy || debug_overlays) then ( (* Doing fstrim on all the filesystems reduces the transfer size * because unused blocks are marked in the overlay and thus do @@ -99,8 +97,6 @@ let rec main () = ); message (f_"Closing the overlay"); - g#umount_all (); - g#shutdown (); g#close (); let target_firmware = get_target_firmware inspect guestcaps source output in -- 2.4.3

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/v2v.ml | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index c28905d..23bd708 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -60,19 +60,9 @@ let rec main () = let overlays = create_overlays source.s_disks in let targets = init_targets overlays source output output_format in - (* Open the guestfs handle. *) message (f_"Opening the overlay"); - let g = new G.guestfs () in - g#set_identifier "v2v"; - if trace () then g#set_trace true; - if verbose () then g#set_verbose true; - g#set_network true; - List.iter ( - fun ({ov_overlay_file = overlay_file}) -> - g#add_drive_opts overlay_file - ~format:"qcow2" ~cachemode:"unsafe" ~discard:"besteffort" - ~copyonread:true - ) overlays; + let g = open_guestfs () in + populate_overlays g overlays; g#launch (); @@ -261,6 +251,24 @@ and init_targets overlays source output output_format = output#prepare_targets source targets +and open_guestfs () = + (* Open the guestfs handle. *) + let g = new G.guestfs () in + g#set_identifier "v2v"; + if trace () then g#set_trace true; + if verbose () then g#set_verbose true; + g#set_network true; + g + +and populate_overlays (g:G.guestfs) overlays = + (* Populate guestfs handle with qcow2 overlays. *) + List.iter ( + fun ({ov_overlay_file = overlay_file}) -> + g#add_drive_opts overlay_file + ~format:"qcow2" ~cachemode:"unsafe" ~discard:"besteffort" + ~copyonread:true + ) overlays + and inspect_source g root_choice = let roots = g#inspect_os () in let roots = Array.to_list roots in -- 2.4.3

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

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/Makefile.am | 1 + v2v/test-v2v-in-place.sh | 119 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100755 v2v/test-v2v-in-place.sh diff --git a/v2v/Makefile.am b/v2v/Makefile.am index b5becba..0ed1ace 100644 --- a/v2v/Makefile.am +++ b/v2v/Makefile.am @@ -256,6 +256,7 @@ TESTS += \ test-v2v-cdrom.sh \ test-v2v-i-ova.sh \ test-v2v-i-disk.sh \ + test-v2v-in-place.sh \ test-v2v-machine-readable.sh \ test-v2v-networks-and-bridges.sh \ test-v2v-no-copy.sh \ diff --git a/v2v/test-v2v-in-place.sh b/v2v/test-v2v-in-place.sh new file mode 100755 index 0000000..f685ddb --- /dev/null +++ b/v2v/test-v2v-in-place.sh @@ -0,0 +1,119 @@ +#!/bin/bash - +# libguestfs virt-v2v test script +# Copyright (C) 2014 Red Hat Inc. +# Copyright (C) 2015 Parallels IP Holdings GmbH. +# +# 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 --in-place. + +unset CDPATH +export LANG=C +set -e + +if [ -n "$SKIP_TEST_V2V_IN_PLACE_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 + +abs_top_builddir="$(cd ..; pwd)" + +img_base="$abs_top_builddir/tests/guests/windows.img" +if ! test -f $img_base || ! test -s $img_base; then + echo "$0: test skipped because phony Windows image was not created" + exit 77 +fi + +export VIRT_TOOLS_DATA_DIR="$PWD/fake-virt-tools" +export VIRTIO_WIN="$PWD/fake-virtio-win" + +d=$PWD/test-v2v-in-place.d +rm -rf $d +mkdir $d + +img="$d/test.qcow2" +rm -f $img +qemu-img create -f qcow2 -b $img_base -o compat=1.1,backing_fmt=raw $img +md5="$(md5sum $img_base)" + +libvirt_xml="$d/test.xml" +rm -f $libvirt_xml +n=windows-overlay +cat > $libvirt_xml <<EOF +<node> + <domain type='test'> + <name>$n</name> + <memory>1048576</memory> + <os> + <type>hvm</type> + <boot dev='hd'/> + </os> + <devices> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='$img'/> + <target dev='vda' bus='virtio'/> + </disk> + </devices> + </domain> +</node> +EOF + +$VG virt-v2v --debug-gc -i libvirt -ic "test://$libvirt_xml" $n --in-place + +# Test that the drivers have been copied over into the guest +script="$d/test.fish" +expected="$d/expected" +response="$d/response" + +mktest () +{ + local cmd="$1" exp="$2" + + echo "echo '$cmd'" >> "$script" + echo "$cmd" >> "$expected" + + echo "$cmd" >> "$script" + echo "$exp" >> "$expected" +} + +:> "$script" +:> "$expected" + +firstboot_dir="/Program Files/Red Hat/Firstboot" +mktest "is-dir \"$firstboot_dir\"" true +mktest "is-file \"$firstboot_dir/firstboot.bat\"" true +mktest "is-dir \"$firstboot_dir/scripts\"" true +virtio_dir="/Windows/Drivers/VirtIO" +mktest "is-dir \"$virtio_dir\"" true +for drv in netkvm qxl vioscsi viostor; do + for sfx in cat inf sys; do + mktest "is-file \"$virtio_dir/$drv.$sfx\"" true + done +done + +guestfish --ro -a "$img" -i < "$script" > "$response" +diff -u "$expected" "$response" + +# Test the base image remained untouched +test "$md5" = "$(md5sum $img_base)" + +# Clean up. +rm -r $d -- 2.4.3

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 | 82 ++++++++++++++++++++++++++++++++++++++++++---------------- 2 files changed, 65 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 ff4060d..333ece0 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,25 @@ 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 + + (match conversion_mode with + | Copying _ -> message (f_"Opening the overlay") + | In_place -> message (f_"Opening the source VM") + ); + 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,7 +89,11 @@ 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 guestcaps = do_convert g inspect source keep_serial_console in @@ -88,31 +109,39 @@ let rec main () = do_fstrim g no_trim inspect; ); - message (f_"Closing the overlay"); + (match conversion_mode with + | Copying _ -> message (f_"Closing the overlay") + | In_place -> message (f_"Closing the source VM") + ); g#umount_all (); g#shutdown (); 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; @@ -264,7 +293,7 @@ and open_guestfs () = g#set_network true; g -and populate_overlays (g:G.guestfs) overlays = +and populate_overlays g overlays = (* Populate guestfs handle with qcow2 overlays. *) List.iter ( fun ({ov_overlay_file = overlay_file}) -> @@ -273,6 +302,13 @@ and populate_overlays (g:G.guestfs) overlays = ~copyonread:true ) overlays +and populate_disks g src_disks = + List.iter ( + fun ({s_qemu_uri = qemu_uri; s_format = format}) -> + g#add_drive_opts qemu_uri ?format ~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

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- v2v/Makefile.am | 1 + v2v/test-v2v-in-place.sh | 119 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+) create mode 100755 v2v/test-v2v-in-place.sh diff --git a/v2v/Makefile.am b/v2v/Makefile.am index b5becba..0ed1ace 100644 --- a/v2v/Makefile.am +++ b/v2v/Makefile.am @@ -256,6 +256,7 @@ TESTS += \ test-v2v-cdrom.sh \ test-v2v-i-ova.sh \ test-v2v-i-disk.sh \ + test-v2v-in-place.sh \ test-v2v-machine-readable.sh \ test-v2v-networks-and-bridges.sh \ test-v2v-no-copy.sh \ diff --git a/v2v/test-v2v-in-place.sh b/v2v/test-v2v-in-place.sh new file mode 100755 index 0000000..f685ddb --- /dev/null +++ b/v2v/test-v2v-in-place.sh @@ -0,0 +1,119 @@ +#!/bin/bash - +# libguestfs virt-v2v test script +# Copyright (C) 2014 Red Hat Inc. +# Copyright (C) 2015 Parallels IP Holdings GmbH. +# +# 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 --in-place. + +unset CDPATH +export LANG=C +set -e + +if [ -n "$SKIP_TEST_V2V_IN_PLACE_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 + +abs_top_builddir="$(cd ..; pwd)" + +img_base="$abs_top_builddir/tests/guests/windows.img" +if ! test -f $img_base || ! test -s $img_base; then + echo "$0: test skipped because phony Windows image was not created" + exit 77 +fi + +export VIRT_TOOLS_DATA_DIR="$PWD/fake-virt-tools" +export VIRTIO_WIN="$PWD/fake-virtio-win" + +d=$PWD/test-v2v-in-place.d +rm -rf $d +mkdir $d + +img="$d/test.qcow2" +rm -f $img +qemu-img create -f qcow2 -b $img_base -o compat=1.1,backing_fmt=raw $img +md5="$(md5sum $img_base)" + +libvirt_xml="$d/test.xml" +rm -f $libvirt_xml +n=windows-overlay +cat > $libvirt_xml <<EOF +<node> + <domain type='test'> + <name>$n</name> + <memory>1048576</memory> + <os> + <type>hvm</type> + <boot dev='hd'/> + </os> + <devices> + <disk type='file' device='disk'> + <driver name='qemu' type='qcow2'/> + <source file='$img'/> + <target dev='vda' bus='virtio'/> + </disk> + </devices> + </domain> +</node> +EOF + +$VG virt-v2v --debug-gc -i libvirt -ic "test://$libvirt_xml" $n --in-place + +# Test that the drivers have been copied over into the guest +script="$d/test.fish" +expected="$d/expected" +response="$d/response" + +mktest () +{ + local cmd="$1" exp="$2" + + echo "echo '$cmd'" >> "$script" + echo "$cmd" >> "$expected" + + echo "$cmd" >> "$script" + echo "$exp" >> "$expected" +} + +:> "$script" +:> "$expected" + +firstboot_dir="/Program Files/Red Hat/Firstboot" +mktest "is-dir \"$firstboot_dir\"" true +mktest "is-file \"$firstboot_dir/firstboot.bat\"" true +mktest "is-dir \"$firstboot_dir/scripts\"" true +virtio_dir="/Windows/Drivers/VirtIO" +mktest "is-dir \"$virtio_dir\"" true +for drv in netkvm qxl vioscsi viostor; do + for sfx in cat inf sys; do + mktest "is-file \"$virtio_dir/$drv.$sfx\"" true + done +done + +guestfish --ro -a "$img" -i < "$script" > "$response" +diff -u "$expected" "$response" + +# Test the base image remained untouched +test "$md5" = "$(md5sum $img_base)" + +# Clean up. +rm -r $d -- 2.4.3