Tomáš Golembiovský
2018-Nov-06 10:44 UTC
[Libguestfs] [PATCH 0/3] Install QEMU-GA from oVirt guest tools ISO on Linux
This installs packages with QEMU Guest Agent when converting Linux machine. The packages should be available on guest tools ISO. The patches work "as-is" but probably deserve some more attention: - it is "abusing" Winows_virtio code but renaming/refactoring everything to remove "windows" from the name and use "guest tools" seems like a lot of unnecesary work - support for Debian is missing I don't know how to install the package only when all it's dependencies are already installed. dpkg cannot be used to check that (simulate the install). And attempting to install the package will leave it half-installed (dpkg cannot roll-back). Tomáš Golembiovský (3): v2v: refactor copy_drivers() in Windows_virtio v2v: linux: install packages v2v: linux: install QEMU-GA v2v/convert_linux.ml | 2 ++ v2v/linux.ml | 19 ++++++++++ v2v/linux.mli | 3 ++ v2v/windows_virtio.ml | 82 +++++++++++++++++++++++++++++++----------- v2v/windows_virtio.mli | 5 +++ 5 files changed, 91 insertions(+), 20 deletions(-) -- 2.19.0
Tomáš Golembiovský
2018-Nov-06 10:44 UTC
[Libguestfs] [PATCH 1/3] v2v: refactor copy_drivers() in Windows_virtio
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@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 ]; ] -(* 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. *) -and copy_drivers g inspect driverdir - let ret = ref false in +and copy_files g inspect srcdir destdir filter + + 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; - 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. -- 2.19.0
Tomáš Golembiovský
2018-Nov-06 10:44 UTC
[Libguestfs] [PATCH 2/3] v2v: linux: install packages
Install packages from local files without touching network. Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- v2v/linux.ml | 19 +++++++++++++++++++ v2v/linux.mli | 3 +++ 2 files changed, 22 insertions(+) diff --git a/v2v/linux.ml b/v2v/linux.ml index 177724e39..6a5cae512 100644 --- a/v2v/linux.ml +++ b/v2v/linux.ml @@ -31,6 +31,25 @@ let augeas_reload g g#aug_load (); debug_augeas_errors g +let rec install g inspect packages + if packages <> [] then ( + do_install g inspect packages; + (* Reload Augeas in case anything changed. *) + augeas_reload g + ) + +and do_install g { i_package_format = package_format } packages + assert (List.length packages > 0); + match package_format with + | "rpm" -> + let cmd = [ "yum"; "--assumeyes"; "install" ] @ packages in + let cmd = Array.of_list cmd in + ignore (g#command cmd) + + | format -> + error (f_"don’t know how to install packages using %s: packages: %s") + format (String.concat " " packages) + let rec remove g inspect packages if packages <> [] then ( do_remove g inspect packages; diff --git a/v2v/linux.mli b/v2v/linux.mli index 1c604665e..0036f4769 100644 --- a/v2v/linux.mli +++ b/v2v/linux.mli @@ -23,6 +23,9 @@ val augeas_reload : Guestfs.guestfs -> unit additional debugging information about parsing problems that augeas found. *) +val install: Guestfs.guestfs -> Types.inspect -> string list -> unit +(** Install pacakge(s). *) + val remove : Guestfs.guestfs -> Types.inspect -> string list -> unit (** Uninstall package(s). *) -- 2.19.0
Tomáš Golembiovský
2018-Nov-06 10:44 UTC
[Libguestfs] [PATCH 3/3] v2v: linux: install QEMU-GA
Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- v2v/convert_linux.ml | 2 ++ v2v/windows_virtio.ml | 30 ++++++++++++++++++++++++++++++ v2v/windows_virtio.mli | 5 +++++ 3 files changed, 37 insertions(+) diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml index e8c64ac1b..a1bafe91a 100644 --- a/v2v/convert_linux.ml +++ b/v2v/convert_linux.ml @@ -81,6 +81,8 @@ let convert (g : G.guestfs) inspect source output rcaps let rec do_convert () augeas_grub_configuration (); + Windows_virtio.install_linux_tools g inspect; + unconfigure_xen (); unconfigure_vbox (); unconfigure_vmware (); diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml index 91649694d..93b4d643e 100644 --- a/v2v/windows_virtio.ml +++ b/v2v/windows_virtio.ml @@ -27,6 +27,8 @@ open Regedit open Types open Utils +module G = Guestfs + let virtio_win try Sys.getenv "VIRTIO_WIN" with Not_found -> @@ -181,6 +183,34 @@ let rec install_drivers ((g, _) as reg) inspect rcaps virtio_rng_supported, virtio_ballon_supported, isa_pvpanic_supported) ) +and install_linux_tools g inspect + let os = match inspect.i_distro with + | "fedora" -> Some "fc28" + | "rhel" | "centos" | "scientificlinux" | "redhat-based" + | "oraclelinux" -> (match inspect.i_major_version with + | 6 -> Some "el6" + | 7 -> Some "el7" + | _ -> None) + | "sles" | "suse-based" | "opensuse" -> Some "lp151" + | _ -> None in + + match os with + | None -> + warning (f_"Don't know how to install guest tools on %s-%d") + inspect.i_distro inspect.i_major_version + | Some os -> + let src_path = "linux" // os in + let dst_path = "/var/tmp" in + debug "locating packages in %s" src_path; + let packages = copy_files g inspect src_path dst_path + (fun _ _ -> true) in + debug "done copying %d files" (List.length packages); + let packages = List.map ((//) dst_path) packages in + try + Linux.install g inspect packages; + with G.Error msg -> + warning (f_"Failed to install QEMU Guest Agent: %s") msg + and add_guestor_to_registry ((g, root) as reg) inspect drv_name drv_pciid let ddb_node = g#hivex_node_get_child root "DriverDatabase" in diff --git a/v2v/windows_virtio.mli b/v2v/windows_virtio.mli index 91b3ced45..4558d041b 100644 --- a/v2v/windows_virtio.mli +++ b/v2v/windows_virtio.mli @@ -42,6 +42,11 @@ val install_drivers (**/**) +val install_linux_tools : Guestfs.guestfs -> Types.inspect -> unit +(** [inspect] + installs QEMU Guest Agent on Linux guest OS from the driver directory or + driver ISO. It is not fatal if we fail to install the agent. *) + (* The following function is only exported for unit tests. *) module UNIT_TESTS : sig val virtio_iso_path_matches_guest_os : string -> Types.inspect -> bool -- 2.19.0
Richard W.M. Jones
2018-Nov-06 11:27 UTC
Re: [Libguestfs] [PATCH 1/3] v2v: refactor copy_drivers() in Windows_virtio
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@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
Richard W.M. Jones
2018-Nov-06 11:32 UTC
Re: [Libguestfs] [PATCH 2/3] v2v: linux: install packages
On Tue, Nov 06, 2018 at 11:44:14AM +0100, Tomáš Golembiovský wrote:> Install packages from local files without touching network.In fact, not limited to local files, but is limited to guests which use ‘yum’. So I think the function needs a better name unless you're planning to combine it with customize/customize_run.ml:guest_install_command (which would be overkill).> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> > --- > v2v/linux.ml | 19 +++++++++++++++++++ > v2v/linux.mli | 3 +++ > 2 files changed, 22 insertions(+) > > diff --git a/v2v/linux.ml b/v2v/linux.ml > index 177724e39..6a5cae512 100644 > --- a/v2v/linux.ml > +++ b/v2v/linux.ml > @@ -31,6 +31,25 @@ let augeas_reload g > g#aug_load (); > debug_augeas_errors g > > +let rec install g inspect packages > + if packages <> [] then ( > + do_install g inspect packages; > + (* Reload Augeas in case anything changed. *) > + augeas_reload g > + ) > + > +and do_install g { i_package_format = package_format } packagesIt's purely a matter of style, but it's also possible to nest functions, so: let install g inspect packages let do_install () ... in if packages <> [] then ( do_install (); ... Of course you could also inline do_install. Rich.> + assert (List.length packages > 0); > + match package_format with > + | "rpm" -> > + let cmd = [ "yum"; "--assumeyes"; "install" ] @ packages in > + let cmd = Array.of_list cmd in > + ignore (g#command cmd) > + > + | format -> > + error (f_"don’t know how to install packages using %s: packages: %s") > + format (String.concat " " packages) > + > let rec remove g inspect packages > if packages <> [] then ( > do_remove g inspect packages; > diff --git a/v2v/linux.mli b/v2v/linux.mli > index 1c604665e..0036f4769 100644 > --- a/v2v/linux.mli > +++ b/v2v/linux.mli > @@ -23,6 +23,9 @@ val augeas_reload : Guestfs.guestfs -> unit > additional debugging information about parsing problems > that augeas found. *) > > +val install: Guestfs.guestfs -> Types.inspect -> string list -> unit > +(** Install pacakge(s). *) > + > val remove : Guestfs.guestfs -> Types.inspect -> string list -> unit > (** Uninstall package(s). *) > > -- > 2.19.0 > > _______________________________________________ > Libguestfs mailing list > Libguestfs@redhat.com > https://www.redhat.com/mailman/listinfo/libguestfs-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Richard W.M. Jones
2018-Nov-06 11:36 UTC
Re: [Libguestfs] [PATCH 3/3] v2v: linux: install QEMU-GA
On Tue, Nov 06, 2018 at 11:44:15AM +0100, Tomáš Golembiovský wrote:> Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>The commit message should end with " (RHBZ#1619665)."> v2v/convert_linux.ml | 2 ++ > v2v/windows_virtio.ml | 30 ++++++++++++++++++++++++++++++ > v2v/windows_virtio.mli | 5 +++++ > 3 files changed, 37 insertions(+) > > diff --git a/v2v/convert_linux.ml b/v2v/convert_linux.ml > index e8c64ac1b..a1bafe91a 100644 > --- a/v2v/convert_linux.ml > +++ b/v2v/convert_linux.ml > @@ -81,6 +81,8 @@ let convert (g : G.guestfs) inspect source output rcaps > let rec do_convert () > augeas_grub_configuration (); > > + Windows_virtio.install_linux_tools g inspect;I see why we're using Windows_virtio, but ... it's surprising!> unconfigure_xen (); > unconfigure_vbox (); > unconfigure_vmware (); > diff --git a/v2v/windows_virtio.ml b/v2v/windows_virtio.ml > index 91649694d..93b4d643e 100644 > --- a/v2v/windows_virtio.ml > +++ b/v2v/windows_virtio.ml > @@ -27,6 +27,8 @@ open Regedit > open Types > open Utils > > +module G = Guestfs > + > let virtio_win > try Sys.getenv "VIRTIO_WIN" > with Not_found -> > @@ -181,6 +183,34 @@ let rec install_drivers ((g, _) as reg) inspect rcaps > virtio_rng_supported, virtio_ballon_supported, isa_pvpanic_supported) > ) > > +and install_linux_tools g inspect > + let os = match inspect.i_distro with > + | "fedora" -> Some "fc28" > + | "rhel" | "centos" | "scientificlinux" | "redhat-based" > + | "oraclelinux" -> (match inspect.i_major_version with > + | 6 -> Some "el6" > + | 7 -> Some "el7" > + | _ -> None) > + | "sles" | "suse-based" | "opensuse" -> Some "lp151" > + | _ -> None in > + > + match os with > + | None -> > + warning (f_"Don't know how to install guest tools on %s-%d")Use "don’t" here (lowercase and Unicode quote).> + inspect.i_distro inspect.i_major_version > + | Some os -> > + let src_path = "linux" // os in > + let dst_path = "/var/tmp" in > + debug "locating packages in %s" src_path; > + let packages = copy_files g inspect src_path dst_path > + (fun _ _ -> true) in > + debug "done copying %d files" (List.length packages); > + let packages = List.map ((//) dst_path) packages in > + try > + Linux.install g inspect packages; > + with G.Error msg -> > + warning (f_"Failed to install QEMU Guest Agent: %s") msg > + > and add_guestor_to_registry ((g, root) as reg) inspect drv_name drv_pciid > let ddb_node = g#hivex_node_get_child root "DriverDatabase" in > > diff --git a/v2v/windows_virtio.mli b/v2v/windows_virtio.mli > index 91b3ced45..4558d041b 100644 > --- a/v2v/windows_virtio.mli > +++ b/v2v/windows_virtio.mli > @@ -42,6 +42,11 @@ val install_drivers > > (**/**) > > +val install_linux_tools : Guestfs.guestfs -> Types.inspect -> unit > +(** [inspect] > + installs QEMU Guest Agent on Linux guest OS from the driver directory or > + driver ISO. It is not fatal if we fail to install the agent. *)This definition should be above the (**/**) mark, since that mark turns off ocamldoc and is used for undocumented functions such as the unit tests below.> (* The following function is only exported for unit tests. *) > module UNIT_TESTS : sig > val virtio_iso_path_matches_guest_os : string -> Types.inspect -> boolRich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Pino Toscano
2018-Nov-06 17:13 UTC
Re: [Libguestfs] [PATCH 2/3] v2v: linux: install packages
On Tuesday, 6 November 2018 11:44:14 CET Tomáš Golembiovský wrote:> Install packages from local files without touching network. > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> > ---In this case, better name it local_install/install_local (or something along these lines) to make it more clear it is not a "classic install".> +and do_install g { i_package_format = package_format } packages > + assert (List.length packages > 0); > + match package_format with > + | "rpm" -> > + let cmd = [ "yum"; "--assumeyes"; "install" ] @ packages in > + let cmd = Array.of_list cmd in > + ignore (g#command cmd)Note that i_package_format and i_package_management are different things. The code above breaks when the system is RPM-based, but does not use yum, so e.g. Fedora (supported), and ALT Linux (not supported). So either use i_package_format with low-level package managers (rpm/dpkg/etc), or use i_package_management with yum/dnf/etc. -- Pino Toscano
Maybe Matching Threads
- [PATCH v3 1/3] v2v: refactor copy_drivers() in Windows_virtio
- [PATCH v4 1/3] v2v: refactor copy_drivers() in Windows_virtio
- Re: [PATCH v3 1/3] v2v: refactor copy_drivers() in Windows_virtio
- [PATCH] v2v: don't fail when virtio-win does not have qemu-ga packages
- [PATCH 2/2] v2v: allow alternative directories for distributions