Re: [Libguestfs] [PATCH v3 3/3] v2v: linux: install QEMU-GA (RHBZ#1619665)

On Wed, 7 Nov 2018 15:46:49 +0000 "Richard W.M. Jones" <rjones(a)redhat.com&gt; wrote: > On Wed, Nov 07, 2018 at 12:53:20PM +0100, Tomáš Golembiovský wrote: > > Signed-off-by: Tomáš Golembiovský <tgolembi(a)redhat.com&gt; > > --- > > v2v/convert_linux.ml | 2 ++ > > v2v/windows_virtio.ml | 30 ++++++++++++++++++++++++++++++ > > v2v/windows_virtio.mli | 4 ++++ > > 3 files changed, 36 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 da02b6c4e..223e7661b 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_from_virtio_win 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 I should make the message more generic, like "failed to install guest tools". > So this was going to be my question about this. Here we've decided to > effectively ignore failure to install qemu-ga. Should it be an error? If error is "printed in red" then possibly yes but that's about it. My opinion is that we should not fail conversion because of this. It is not a critical component (it does not prevent the guest from booting). > What's the chance that for a supported guest it might fail? Do the > packages have complex dependencies? It is unlikely on EL, but on Debian there is a dependency (on glib) which may fail. Also the guest OS may be in an inconsistent state which prevents installation of packages and this may fail. IMHO we're not in a position to deny user conversion of such guest as it may be totally functional otherwise.