Re: [Libguestfs] [PATCH 3/4] v2v: copy virtio drivers without guestfs handle leak

Refactor copying of virtio windows drivers into the guest so that the matching of the drivers to the guest os flavor and copying the files happens one next to the other in a single function, and no guestfs handle (nor any other irrelevant info) is leaked outside. Signed-off-by: Roman Kagan <rkagan(a)virtuozzo.com&gt; --- v2v/convert_windows.ml | 77 ++++++++++------- v2v/utils.ml | 224 ++++++++++++++++--------------------------------- 2 files changed, 118 insertions(+), 183 deletions(-) diff --git a/v2v/convert_windows.ml b/v2v/convert_windows.ml index 26609f2..b6e6c62 100644 --- a/v2v/convert_windows.ml +++ b/v2v/convert_windows.ml @@ -233,35 +233,54 @@ echo uninstalling Xen PV driver and copy_virtio_drivers driverdir = (* Copy the matching drivers to the driverdir. *) - - let drivers = find_virtio_win_drivers virtio_win in - - (* Filter out only drivers matching the current guest. *) - let drivers = - List.filter ( - fun { vwd_os_arch = arch; - vwd_os_major = os_major; vwd_os_minor = os_minor; - vwd_os_variant = os_variant } -> - arch = inspect.i_arch && - os_major = inspect.i_major_version && - os_minor = inspect.i_minor_version && - (match os_variant with - | Vwd_client -> inspect.i_product_variant = "Client" - | Vwd_not_client -> inspect.i_product_variant <> "Client" - | Vwd_any_variant -> true) - ) drivers in - - if verbose () then ( - printf "virtio-win driver files matching this guest:\n"; - List.iter print_virtio_win_driver_file drivers; - flush stdout - ); - - List.iter ( - fun driver -> - let content = driver.vwd_get_contents () in - g#write (driverdir // driver.vwd_filename) content - ) drivers + if is_directory virtio_win then ( + let cmd = sprintf "cd %s && find -type f" (quote virtio_win) in + let paths = external_command cmd in + List.iter ( + fun path -> + if (match_vio_path_with_os path inspect.i_arch + inspect.i_major_version inspect.i_minor_version + inspect.i_product_variant) then ( + let source = virtio_win // path in + let target = driverdir // (String.lowercase + (Filename.basename path)) in + if verbose () then + printf "Copying virtio driver bits: 'host:%s' -> '%s'\n" + source target; + + g#write target (read_whole_file source) + ) + ) paths + ) + else if is_regular_file virtio_win then ( + try + let g2 = new Guestfs.guestfs () in + if trace () then g2#set_trace true; + if verbose () then g2#set_verbose true; + g2#add_drive_opts virtio_win ~readonly:true; + g2#launch (); + let vio_root = "/" in + g2#mount_ro "/dev/sda" vio_root; + let paths = g2#find vio_root in + Array.iter ( + fun path -> + let source = vio_root // path in + if ((g2#is_file source ~followsymlinks:false) && + (match_vio_path_with_os path inspect.i_arch + inspect.i_major_version inspect.i_minor_version + inspect.i_product_variant)) then + let target = driverdir // (String.lowercase + (Filename.basename path)) in + if verbose () then + printf "Copying virtio driver bits: '%s:%s' -> '%s'\n" + virtio_win path target; + + g#write target (g2#read_file source) + ) paths; + g2#close() + with Guestfs.Error msg -> + error (f_"%s: cannot open virtio-win ISO file: %s") virtio_win msg + ) and install_virtio_drivers root current_cs = (* Copy the virtio drivers to the guest. *) diff --git a/v2v/utils.ml b/v2v/utils.ml index 4e6befc..7db4448 100644 --- a/v2v/utils.ml +++ b/v2v/utils.ml @@ -110,161 +110,77 @@ let find_uefi_firmware guest_arch = in loop files -(* Find virtio-win driver files from an unpacked or mounted virtio-win - * directory, or from a virtio-win.iso file. The location of drivers - varies between releases of virtio-win and also across Fedora and - RHEL so try to be robust to changes. - *) -type virtio_win_driver_file = { - (* Base filename, eg. "netkvm.sys". Always lowercase. *) - vwd_filename : string; - (* Return the contents of this file. *) - vwd_get_contents : unit -> string; - - (* Various fields that classify this driver: *) - - vwd_os_major : int; (* Windows version. *) - vwd_os_minor : int; - vwd_os_variant : vwd_os_variant; - vwd_os_arch : string; (* Architecture, eg "i386", "x86_64". *) - vwd_extension : string; (* File extension (lowercase), eg. "sys" "inf"*) - - (* Original source of file (for debugging only). *) - vwd_original_source : string; -} -and vwd_os_variant = Vwd_client | Vwd_not_client | Vwd_any_variant - -let print_virtio_win_driver_file vwd = - printf "%s [%d,%d,%s,%s,%s] from %s\n" - vwd.vwd_filename - vwd.vwd_os_major vwd.vwd_os_minor - (match vwd.vwd_os_variant with - | Vwd_client -> "client" | Vwd_not_client -> "not-client" - | Vwd_any_variant -> "any") - vwd.vwd_os_arch - vwd.vwd_extension - vwd.vwd_original_source - -let find_virtio_win_drivers virtio_win = - let is_regular_file path = (* NB: follows symlinks. *) - try (Unix.stat path).Unix.st_kind = Unix.S_REG - with Unix.Unix_error _ -> false - in - - let files = - if is_directory virtio_win then ( - let cmd = sprintf "cd %s && find -type f" (quote virtio_win) in - let paths = external_command cmd in - List.map ( - fun path -> - let abs_path = virtio_win // path in - (path, abs_path, - Filename.basename path, - fun () -> read_whole_file abs_path) - ) paths - ) - else if is_regular_file virtio_win then ( - try - let g = new Guestfs.guestfs () in - if trace () then g#set_trace true; - if verbose () then g#set_verbose true; - g#add_drive_opts virtio_win ~readonly:true; - g#launch (); - g#mount_ro "/dev/sda" "/"; - let paths = g#find "/" in - let paths = Array.to_list paths in - let paths = List.map ((^) "/") paths in - let paths = List.filter (g#is_file ~followsymlinks:false) paths in - List.map ( - fun path -> - let basename = - match last_part_of path '/' with - | Some x -> x - | None -> - error "v2v/find_virtio_win_drivers: missing '/' in %s" path in - (path, sprintf "%s:%s" virtio_win path, - basename, - fun () -> g#read_file path) - ) paths - with Guestfs.Error msg -> - error (f_"%s: cannot open virtio-win ISO file: %s") virtio_win msg - ) - else [] in +let is_regular_file path = (* NB: follows symlinks. *) + try (Unix.stat path).Unix.st_kind = Unix.S_REG + with Unix.Unix_error _ -> false - let files = - filter_map ( - fun (path, original_source, basename, get_contents) -> - try - (* Lowercased path, since the ISO may contain upper or lowercase - * path elements. XXX This won't work if paths contain non-ASCII. - *) - let lc_path = String.lowercase path in - let lc_basename = String.lowercase basename in - - let extension = - match last_part_of lc_basename '.' with - | Some x -> x - | None -> - error "v2v/find_virtio_win_drivers: missing '.' in %s" - lc_basename in - - (* Skip files without specific extensions. *) - if extension <> "cat" && extension <> "inf" && - extension <> "pdb" && extension <> "sys" then - raise Not_found; - - (* Using the full path, work out what version of Windows - * this driver is for. Paths can be things like: - * "NetKVM/2k12R2/amd64/netkvm.sys" or - * "./drivers/amd64/Win2012R2/netkvm.sys". - * Note we check lowercase paths. - *) - let pathelem elem = string_find lc_path ("/" ^ elem ^ "/") >= 0 in - let arch = - if pathelem "x86" || pathelem "i386" then "i386" - else if pathelem "amd64" then "x86_64" - else raise Not_found in - let os_major, os_minor, os_variant = - if pathelem "xp" || pathelem "winxp" then - (5, 1, Vwd_any_variant) - else if pathelem "2k3" || pathelem "win2003" then - (5, 2, Vwd_any_variant) - else if pathelem "vista" then - (6, 0, Vwd_client) - else if pathelem "2k8" || pathelem "win2008" then - (6, 0, Vwd_not_client) - else if pathelem "w7" || pathelem "win7" then - (6, 1, Vwd_client) - else if pathelem "2k8r2" || pathelem "win2008r2" then - (6, 1, Vwd_not_client) - else if pathelem "w8" || pathelem "win8" then - (6, 2, Vwd_client) - else if pathelem "2k12" || pathelem "win2012" then - (6, 2, Vwd_not_client) - else if pathelem "w8.1" || pathelem "win8.1" then - (6, 3, Vwd_client) - else if pathelem "2k12r2" || pathelem "win2012r2" then - (6, 3, Vwd_not_client) - else if pathelem "w10" || pathelem "win10" then - (10, 0, Vwd_client) - else - raise Not_found in - - Some { - vwd_filename = lc_basename; - vwd_get_contents = get_contents; - vwd_os_major = os_major; - vwd_os_minor = os_minor; - vwd_os_variant = os_variant; - vwd_os_arch = arch; - vwd_extension = extension; - vwd_original_source = original_source; - } - - with Not_found -> None - ) files in - - files +(* Given a path of a file relative to the root of the directory tree with + * virtio-win drivers, figure out if it's suitable for the specific Windows + * flavor. + *) +let match_vio_path_with_os path arch os_major os_minor os_variant = + try + (* Lowercased path, since the ISO may contain upper or lowercase path + * elements. XXX This won't work if paths contain non-ASCII. + *) + let lc_path = String.lowercase path in + let lc_basename = Filename.basename path in + + let extension = + match last_part_of lc_basename '.' with + | Some x -> x + | None -> raise Not_found + in + + (* Skip files without specific extensions. *) + let extensions = ["cat"; "inf"; "pdb"; "sys"] in + if (not (List.mem extension extensions)) then raise Not_found; + + (* Using the full path, work out what version of Windows + * this driver is for. Paths can be things like: + * "NetKVM/2k12R2/amd64/netkvm.sys" or + * "./drivers/amd64/Win2012R2/netkvm.sys". + * Note we check lowercase paths. + *) + let pathelem elem = string_find lc_path ("/" ^ elem ^ "/") >= 0 in + let p_arch = + if pathelem "x86" || pathelem "i386" then "i386" + else if pathelem "amd64" then "x86_64" + else raise Not_found in + + let is_client os_variant = os_variant = "Client" + and not_client os_variant = os_variant <> "Client" + and any_variant os_variant = true in + let p_os_major, p_os_minor, match_os_variant = + if pathelem "xp" || pathelem "winxp" then + (5, 1, any_variant) + else if pathelem "2k3" || pathelem "win2003" then + (5, 2, any_variant) + else if pathelem "vista" then + (6, 0, is_client) + else if pathelem "2k8" || pathelem "win2008" then + (6, 0, not_client) + else if pathelem "w7" || pathelem "win7" then + (6, 1, is_client) + else if pathelem "2k8r2" || pathelem "win2008r2" then + (6, 1, not_client) + else if pathelem "w8" || pathelem "win8" then + (6, 2, is_client) + else if pathelem "2k12" || pathelem "win2012" then + (6, 2, not_client) + else if pathelem "w8.1" || pathelem "win8.1" then + (6, 3, is_client) + else if pathelem "2k12r2" || pathelem "win2012r2" then + (6, 3, not_client) + else if pathelem "w10" || pathelem "win10" then + (10, 0, is_client) + else + raise Not_found in + + arch = p_arch && os_major = p_os_major && os_minor = p_os_minor && + match_os_variant os_variant + + with Not_found -> false let compare_app2_versions app1 app2 = let i = compare app1.Guestfs.app2_epoch app2.Guestfs.app2_epoch in