From 6ca5986e45bb891f9b06324270cee7c0818e4d8f Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Tue, 3 Jul 2018 19:15:46 +0100 Subject: [PATCH] v2v: Move per-target size stats into a separate mutable sub-struct. This change avoids us threading the targets structure awkwardly through several functions, and also makes it clear from the code that the targets struct does not change after its creation in init_targets (but the stats change). Just code refactoring. --- v2v/create_ovf.ml | 5 ++-- v2v/types.ml | 12 ++++++--- v2v/types.mli | 27 ++++++++++++++------ v2v/v2v.ml | 63 ++++++++++++++++++++++------------------------- 4 files changed, 61 insertions(+), 46 deletions(-) diff --git a/v2v/create_ovf.ml b/v2v/create_ovf.ml index 9e0c772fd..c1cc3c066 100644 --- a/v2v/create_ovf.ml +++ b/v2v/create_ovf.ml @@ -796,7 +796,8 @@ and add_disks targets guestcaps output_alloc sd_uuid image_uuids vol_uuids in let size_gb = bytes_to_gb ov.ov_virtual_size in let actual_size_gb, is_estimate = - match t.target_actual_size, t.target_estimated_size with + let ts = t.target_stats in + match ts.target_actual_size, ts.target_estimated_size with | Some actual_size, _ -> Some (bytes_to_gb actual_size), false (* In the --no-copy case the target file does not exist. In * that case we use the estimated size. @@ -824,7 +825,7 @@ and add_disks targets guestcaps output_alloc sd_uuid image_uuids vol_uuids "ovf:id", vol_uuid; "ovf:description", generated_by; ] in - (match t.target_actual_size with + (match t.target_stats.target_actual_size with | None -> () | Some actual_size -> List.push_back attrs ("ovf:size", Int64.to_string actual_size) diff --git a/v2v/types.ml b/v2v/types.ml index da1192ec3..05c9a2b1f 100644 --- a/v2v/types.ml +++ b/v2v/types.ml @@ -302,24 +302,30 @@ overlay virtual disk size: %Ld type target = { target_file : target_file; target_format : string; - target_estimated_size : int64 option; - target_actual_size : int64 option; + target_stats : target_stats; target_overlay : overlay; } and target_file = | TargetFile of string | TargetURI of string +and target_stats = { + mutable target_estimated_size : int64 option; + mutable target_actual_size : int64 option; +} let string_of_target t = sprintf " target file: %s target format: %s target estimated size: %s + target actual size: %s " (match t.target_file with | TargetFile s -> "[file] " ^ s | TargetURI s -> "[qemu] " ^ s) t.target_format - (match t.target_estimated_size with + (match t.target_stats.target_estimated_size with + | None -> "None" | Some i -> Int64.to_string i) + (match t.target_stats.target_actual_size with | None -> "None" | Some i -> Int64.to_string i) type target_firmware = TargetBIOS | TargetUEFI diff --git a/v2v/types.mli b/v2v/types.mli index c6724cd20..10d05c2ac 100644 --- a/v2v/types.mli +++ b/v2v/types.mli @@ -38,9 +38,11 @@ │source│ │struct│ └──┬───┘ + │ source.s_disks + │ │ ┌───────┐ ┌───────┐ ┌───────┐ └────┤ disk1 ├──┤ disk2 ├──┤ disk3 │ Source disks - └───▲───┘ └───▲───┘ └───▲───┘ (source.s_disks) + └───▲───┘ └───▲───┘ └───▲───┘ │ │ │ │ │ │ overlay.ov_source ┌───┴───┐ ┌───┴───┐ ┌───┴───┐ @@ -50,6 +52,11 @@ │ │ │ target.target_overlay ┌───┴───┐ ┌───┴───┐ ┌───┴───┐ │ targ1 ├──┤ targ2 ├──┤ targ3 │ Target disks + └───┬───┘ └───┬───┘ └───┬───┘ + │ │ │ target.target_stats + │ │ │ + ┌───▼───┐ ┌───▼───┐ ┌───▼───┐ + │ stat1 │ │ stat2 │ │ stat3 │ Mutable per-target stats └───────┘ └───────┘ └───────┘ v} *) @@ -197,12 +204,7 @@ type target = { target_file : target_file; (** Destination file or QEMU URI. *) target_format : string; (** Destination format (eg. -of option). *) - (* Note that the estimate is filled in by core v2v.ml code before - * copying starts, and the actual size is filled in after copying - * (but may not be filled in if [--no-copy] so don't rely on it). - *) - target_estimated_size : int64 option; (** Est. max. space taken on target. *) - target_actual_size : int64 option; (** Actual size on target. *) + target_stats : target_stats; (** Size stats for this disk. *) target_overlay : overlay; (** Link back to the overlay disk. *) } @@ -212,6 +214,17 @@ and target_file = | TargetFile of string (** Target is a file. *) | TargetURI of string (** Target is a QEMU URI. *) +and target_stats = { + (* Note that the estimate is filled in by core v2v.ml code before + * copying starts, and the actual size is filled in after copying + * (but may not be filled in if [--no-copy] so don't rely on it). + *) + mutable target_estimated_size : int64 option; + (** Est. max. space taken on target. *) + mutable target_actual_size : int64 option; + (** Actual size on target. *) +} + val string_of_target : target -> string (** {2 Guest firmware} *) diff --git a/v2v/v2v.ml b/v2v/v2v.ml index 80d4ebb18..5bef9b5ea 100644 --- a/v2v/v2v.ml +++ b/v2v/v2v.ml @@ -116,7 +116,7 @@ let rec main () = let conversion_mode = match conversion_mode with | Copying (overlays, targets) -> - let targets = check_target_free_space mpstats source targets output in + check_target_free_space mpstats source targets output; Copying (overlays, targets) | In_place -> In_place in @@ -179,9 +179,8 @@ let rec main () = guestcaps in debug "%s" (string_of_target_buses target_buses); - let targets = - if not cmdline.do_copy then targets - else copy_targets cmdline targets input output in + if cmdline.do_copy then + copy_targets cmdline targets input output; (* Create output metadata. *) message (f_"Creating output metadata"); @@ -363,13 +362,18 @@ and init_targets cmdline output source overlays = if cmdline.compressed && format <> "qcow2" then error (f_"the --compressed flag is only allowed when the output format is qcow2 (-of qcow2)"); - (* 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. + (* output#prepare_targets below will fill in the target_file field. + * + * Function 'estimate_target_size' replaces the + * target_stats.target_estimated_size field. + * Function 'actual_target_size' may replace the + * target_stats.target_actual_size field. *) { target_file = TargetFile ""; target_format = format; - target_estimated_size = None; - target_actual_size = None; + target_stats = { + target_estimated_size = None; + target_actual_size = None; + }; target_overlay = ov } ) overlays in @@ -547,9 +551,7 @@ and estimate_target_size mpstats targets = debug "estimate_target_size: source_total_size = %Ld [%s]" source_total_size (human_size source_total_size); - if source_total_size = 0L then (* Avoid divide by zero error. *) - targets - else ( + if source_total_size > 0L then ( (* Avoids divide by zero error below. *) (* (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 @@ -583,8 +585,8 @@ and estimate_target_size mpstats targets = scaled_saving (human_size scaled_saving); (* (5) *) - let targets = List.map ( - fun ({ target_overlay = ov } as t) -> + List.iter ( + fun { target_overlay = ov; target_stats = ts } -> let size = ov.ov_virtual_size in let proportion = Int64.to_float size /. Int64.to_float source_total_size in @@ -592,19 +594,15 @@ and estimate_target_size mpstats targets = size -^ Int64.of_float (proportion *. Int64.to_float scaled_saving) in debug "estimate_target_size: %s: %Ld [%s]" ov.ov_sd estimated_size (human_size estimated_size); - { t with target_estimated_size = Some estimated_size } - ) targets in - - targets + ts.target_estimated_size <- Some estimated_size + ) targets ) (* Estimate space required on target for each disk. Note this is a max. *) and check_target_free_space mpstats source targets output = 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; - targets + estimate_target_size mpstats targets; + output#check_target_free_space source targets (* Conversion. *) and do_convert g inspect source output rcaps = @@ -679,7 +677,7 @@ and copy_targets cmdline targets input output = ) ); let nr_disks = List.length targets in - List.mapi ( + List.iteri ( fun i t -> (match t.target_file with | TargetFile s -> @@ -755,10 +753,8 @@ and copy_targets cmdline targets input output = 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 + (* Calculate the actual size on the target. *) + actual_target_size t; (* If verbose, print the virtual and real copying rates. *) let elapsed_time = end_time -. start_time in @@ -770,7 +766,7 @@ and copy_targets cmdline targets input output = eprintf "virtual copying rate: %.1f M bits/sec\n%!" (mbps t.target_overlay.ov_virtual_size elapsed_time); - match t.target_actual_size with + match t.target_stats.target_actual_size with | None -> () | Some actual -> eprintf "real copying rate: %.1f M bits/sec\n%!" @@ -782,7 +778,8 @@ and copy_targets cmdline targets input output = * accuracy of the estimate. *) if verbose () then ( - match t.target_estimated_size, t.target_actual_size with + let ts = t.target_stats in + match ts.target_estimated_size, ts.target_actual_size with | None, None | None, Some _ | Some _, None | Some _, Some 0L -> () | Some estimate, Some actual -> let pc = @@ -795,9 +792,7 @@ and copy_targets cmdline targets input output = pc; if pc < 0. then eprintf " ! ESTIMATE TOO LOW !"; eprintf "\n%!"; - ); - - t + ) ) targets (* Update the target_actual_size field in the target structure. *) @@ -808,8 +803,8 @@ and actual_target_size target = (* Ignore errors because we want to avoid failures after copying. *) try Some (du filename) with Failure _ | Invalid_argument _ -> None in - { target with target_actual_size = size } - | TargetURI _ -> target + target.target_stats.target_actual_size <- size + | TargetURI _ -> () (* Save overlays if --debug-overlays option was used. *) and preserve_overlays overlays src_name =