[Libguestfs] [PATCH 1/3] v2v: refactor copy_drivers() in Windows_virtio
Richard W.M. Jones
rjones at redhat.com
Tue Nov 6 11:27:51 UTC 2018
On Tue, Nov 06, 2018 at 11:44:13AM +0100, Tomáš Golembiovský wrote:
> Changed the function to be more generic and renamed to copy_files.
> The only change in behavior is in produced debug messages.
>
> Signed-off-by: Tomáš Golembiovský <tgolembi at redhat.com>
> ---
> v2v/windows_virtio.ml | 52 ++++++++++++++++++++++++++-----------------
> 1 file changed, 32 insertions(+), 20 deletions(-)
>
> diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml
> index 9b45c76f5..91649694d 100644
> --- a/v2v/windows_virtio.ml
> +++ b/v2v/windows_virtio.ml
> @@ -250,32 +250,35 @@ and ddb_regedits inspect drv_name drv_pciid =
> [ "Configuration", REG_SZ drv_config ];
> ]
I think it's better to put the copy_drivers function above the new
copy_files function, since the rest of this file is following a
top-to-bottom order.
> -(* Copy the matching drivers to the driverdir; return true if any have
> - * been copied.
> +(* Copy all files from [srcdir] and all its subdirectories to the [destdir].
> + * The file list is filtered based on [filter] function. Return list of copied
> + * files.
> *)
This comment doesn't accurately describe what the function does, and I
don't think "copy_files" is a good name either (since it sounds
generic). As far as I can tell [srcdir] is relative to the tools
directory/ISO, so ‘copy_files g inspect "/" ...’ is not copying
everything on the host, but copying every file in the tools
directory/ISO.
It's probably better to call this function something like
‘copy_from_virtio_win’, and update the comment above to make it
clearer what the options mean.
> -and copy_drivers g inspect driverdir =
> - let ret = ref false in
> +and copy_files g inspect srcdir destdir filter =
> +
Extra blank line added here.
> + let ret = ref [] in
> if is_directory virtio_win then (
> - debug "windows: copy_drivers: source directory virtio_win %s" virtio_win;
> + let dir = virtio_win // srcdir in
> + debug "copy_files: guest tools source directory %s" dir;
In the debug messages, let's keep the "windows: " prefix (also applies
below).
> - let cmd = sprintf "cd %s && find -L -type f" (quote virtio_win) in
> + let cmd = sprintf "cd %s && find -L -type f" (quote dir) in
> let paths = external_command cmd in
> List.iter (
> fun path ->
> - if virtio_iso_path_matches_guest_os path inspect then (
> - let source = virtio_win // path in
> - let target = driverdir //
> - String.lowercase_ascii (Filename.basename path) in
> - debug "copying virtio driver bits: 'host:%s' -> '%s'"
> + if filter path inspect then (
> + let source = dir // path in
> + let target_name = String.lowercase_ascii (Filename.basename path) in
> + let target = destdir // target_name in
> + debug "copying guest tool bits: 'host:%s' -> '%s'"
> source target;
>
> g#write target (read_whole_file source);
> - ret := true
> + List.push_front target_name ret
> )
> ) paths
> )
> else if is_regular_file virtio_win then (
> - debug "windows: copy_drivers: source ISO virtio_win %s" virtio_win;
> + debug "copy_files: guest tools source ISO %s" virtio_win;
>
> try
> let g2 = open_guestfs ~identifier:"virtio_win" () in
> @@ -283,19 +286,20 @@ and copy_drivers g inspect driverdir =
> g2#launch ();
> let vio_root = "/" in
> g2#mount_ro "/dev/sda" vio_root;
> - let paths = g2#find vio_root in
> + let srcdir = vio_root // srcdir in
> + let paths = g2#find srcdir in
> Array.iter (
> fun path ->
> - let source = vio_root // path in
> + let source = srcdir // path in
> if g2#is_file source ~followsymlinks:false &&
> - virtio_iso_path_matches_guest_os path inspect then (
> - let target = driverdir //
> - String.lowercase_ascii (Filename.basename path) in
> - debug "copying virtio driver bits: '%s:%s' -> '%s'"
> + filter path inspect then (
> + let target_name = String.lowercase_ascii (Filename.basename path) in
> + let target = destdir // target_name in
> + debug "copying guest tool bits: '%s:%s' -> '%s'"
> virtio_win path target;
>
> g#write target (g2#read_file source);
> - ret := true
> + List.push_front target_name ret
> )
> ) paths;
> g2#close()
> @@ -304,6 +308,14 @@ and copy_drivers g inspect driverdir =
> );
> !ret
>
> +(* Copy the matching drivers to the driverdir; return true if any have
> + * been copied.
> + *)
> +and copy_drivers g inspect driverdir =
> + List.length (
> + copy_files g inspect "/" driverdir virtio_iso_path_matches_guest_os
> + ) > 0
> +
> (* 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 of the current guest.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html
More information about the Libguestfs
mailing list