The intent (even before RHBZ#2028764) has been to install the QEMU guest agent in the converted domain unconditionally. Therefore, in order for the GA to be actually accessible from the host side, augment the libvirt output module with a "guest agent connection" also unconditionally. For starters, the domain needs a virtio-serial device. Then there must be a port on the device that (in the guest) the GA identifies by name, and that (on the host) is exposed as a listening socket (usually in the unix address family). The adress of that port (usually a pathname, i.e., for a unix domain socket) is then passed to whatever host-side application wants to talk to the GA. The minimal domain XML fragment for that ("minimal" for our purposes) is <controller type='virtio-serial' model='virtio'> <channel type='unix'> <target type='virtio' name='org.qemu.guest_agent.0'/> </channel> The "controller" element is needed because "controller/@model" is where we regulate "virtio" vs. "virtio-transitional". Everything else is filled in by libvirt. Notably, libvirt (a) creates and binds the unix domain socket itself (usually "/var/lib/libvirt/qemu/channel/target/DOMAIN/org.qemu.guest_agent.0"), (b) passes the file descriptor to QEMU, and (c) figures out the socket pathname for commands such as virsh domfsinfo DOMAIN virsh domhostname DOMAIN --source agent virsh domifaddr DOMAIN --source agent virsh guestinfo DOMAIN For QEMU, the corresponding options would be -chardev socket,id=agent,server=on,wait=off,path=/tmp/DOMAIN-agent \ -device virtio-serial-pci,id=vioserial \ -device virtserialport,bus=vioserial.0,nr=1,chardev=agent,name=org.qemu.guest_agent.0 \ Note the "path=/tmp/DOMAIN-agent" property of "-chardev"; virt-v2v would have to generate that (in place of the "fd=nnnn" property that libvirt passes to QEMU). Omit extending the QEMU output module for now, as the QGA protocol is based on JSON, and one needs "virsh" or "virt-manager" (or another management application interface) anyway, for efficiently exchanging messages with QGA. I don't know of end-user tools that directly connect to "/tmp/DOMAIN-agent". Don't modify the RHV and OpenStack outputs either; both of these management products likely configure the virtio-serial device automatically, for the agent access. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2028764 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- Notes: v2: - no change output/create_libvirt_xml.ml | 11 +++++++++++ tests/test-v2v-i-ova.xml | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/output/create_libvirt_xml.ml b/output/create_libvirt_xml.ml index 68d0a90918f3..531a4f75bf3e 100644 --- a/output/create_libvirt_xml.ml +++ b/output/create_libvirt_xml.ml @@ -524,6 +524,17 @@ let create_libvirt_xml ?pool source inspect e "console" ["type", "pty"] []; ]; + (* Given that we install the QEMU Guest Agent for both Linux and Windows + * guests unconditionally, create the virtio-serial device that's needed for + * communication between the host and the agent. + *) + List.push_back_list devices [ + e "controller" ["type", "virtio-serial"; "model", virtio_model] []; + e "channel" ["type", "unix"] [ + e "target" ["type", "virtio"; "name", "org.qemu.guest_agent.0"] [] + ] + ]; + List.push_back_list body [ e "devices" [] !devices; ]; diff --git a/tests/test-v2v-i-ova.xml b/tests/test-v2v-i-ova.xml index 6b8cda62f051..da1db473e53c 100644 --- a/tests/test-v2v-i-ova.xml +++ b/tests/test-v2v-i-ova.xml @@ -49,5 +49,9 @@ <input type='tablet' bus='usb'/> <input type='mouse' bus='ps2'/> <console type='pty'/> + <controller type='virtio-serial' model='virtio'/> + <channel type='unix'> + <target type='virtio' name='org.qemu.guest_agent.0'/> + </channel> </devices> </domain> -- 2.19.1.3.g30247aa5d201

"Windows_virtio.install_linux_tools" has never really worked in practice; we've never managed to get the right content into the right location on the virtio-win ISO. Later patches in this series will install the qemu guest agent in a firstboot script, using the guest's own package manager. For now, for ease of review, only remove "Windows_virtio.install_linux_tools", and its dependencies that now become unused: - Linux.architecture_string - Linux.binary_package_extension - Linux.install_local Salvage the comment (at the outermost call site) that failure to install QGA is not fatal. This will be relevant for exception handling in the subsequent patches. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2028764 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; Acked-by: Richard W.M. Jones <rjones(a)redhat.com&gt; --- Notes: v2: - pick up Rich's A-b convert/linux.mli | 11 ----- convert/windows_virtio.mli | 4 -- convert/convert_linux.ml | 4 +- convert/linux.ml | 35 ---------------- convert/windows_virtio.ml | 42 -------------------- 5 files changed, 3 insertions(+), 93 deletions(-) diff --git a/convert/linux.mli b/convert/linux.mli index 856ffe3c95ab..57898310597c 100644 --- a/convert/linux.mli +++ b/convert/linux.mli @@ -23,9 +23,6 @@ val augeas_reload : Guestfs.guestfs -> unit additional debugging information about parsing problems that augeas found. *) -val install_local: Guestfs.guestfs -> Types.inspect -> string list -> unit -(** Install package(s). *) - val remove : Guestfs.guestfs -> Types.inspect -> string list -> unit (** Uninstall package(s). *) @@ -38,11 +35,3 @@ val is_file_owned : Guestfs.guestfs -> Types.inspect -> string -> bool val is_package_manager_save_file : string -> bool (** Return true if the filename is something like [*.rpmsave], ie. a package manager save-file. *) - -val binary_package_extension : Types.inspect -> string -(** Return the extension typically used for binary packages in the - specified package format. *) - -val architecture_string : Types.inspect -> string -(** Return the architecture string typically used for binary packages - in the specified package format, and for the specified distro. *) diff --git a/convert/windows_virtio.mli b/convert/windows_virtio.mli index a92cc01da6df..73ec95c71839 100644 --- a/convert/windows_virtio.mli +++ b/convert/windows_virtio.mli @@ -38,10 +38,6 @@ val install_drivers reflecting what devices are now required by the guest, either virtio devices if we managed to install those, or legacy devices if we didn't. *) -val install_linux_tools : Guestfs.guestfs -> Types.inspect -> unit -(** 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. *) - val copy_qemu_ga : Guestfs.guestfs -> Types.inspect -> string list (** copy MSIs (idealy just one) with QEMU Guest Agent to Windows guest. The MSIs are not installed by this function. *) diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml index 5660494973b1..79462aa1e7ae 100644 --- a/convert/convert_linux.ml +++ b/convert/convert_linux.ml @@ -538,13 +538,15 @@ let convert (g : G.guestfs) source inspect keep_serial_console _ = ) and install_linux_tools () = + (* It is not fatal if we fail to install the QEMU guest agent. *) let has_qemu_guest_agent = List.exists ( fun { G.app2_name = name } -> name = "qemu-guest-agent" ) inspect.i_apps in if not has_qemu_guest_agent then - Windows_virtio.install_linux_tools g inspect + (* FIXME -- install qemu-guest-agent here *) + () and configure_kernel () = (* Previously this function would try to install kernels, but we diff --git a/convert/linux.ml b/convert/linux.ml index e2908d02be28..f9acd63f9b82 100644 --- a/convert/linux.ml +++ b/convert/linux.ml @@ -33,20 +33,6 @@ let augeas_reload g = g#aug_load (); debug_augeas_errors g -let rec install_local g { i_package_format = package_format } packages = - if packages <> [] then ( - match package_format with - | "rpm" -> - let cmd = [ "rpm"; "--upgrade"; "-v" ] @ 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) - (* Reload Augeas in case anything changed. *) - augeas_reload g - ) - let rec remove g inspect packages = if packages <> [] then ( do_remove g inspect packages; @@ -187,24 +173,3 @@ let is_package_manager_save_file filename = (* Recognized suffixes of package managers. *) let suffixes = [ ".dpkg-old"; ".dpkg-new"; ".rpmsave"; ".rpmnew"; ] in List.exists (Filename.check_suffix filename) suffixes - -let binary_package_extension { i_package_format = package_format } = - match package_format with - | "deb" -> "deb" - | "rpm" -> "rpm" - | format -> - error (f_"don’t know what is the extension of binary packages using %s") - format - -let architecture_string { i_package_format = package_format; i_arch = arch; - i_distro = distro } = - match package_format, distro, arch with - | "deb", _, "x86_64" -> "amd64" - | "deb", _, a -> a - | "rpm", ("sles"|"suse-based"|"opensuse"), "i386" -> "i586" - | "rpm", ("sles"|"suse-based"|"opensuse"), a -> a - | "rpm", _, "i386" -> "i686" - | "rpm", _, a -> a - | format, distro, arch -> - error (f_"don’t know what is the architecture string of %s using %s on %s") - arch format distro diff --git a/convert/windows_virtio.ml b/convert/windows_virtio.ml index 5254322c7d28..a27cd6a5432e 100644 --- a/convert/windows_virtio.ml +++ b/convert/windows_virtio.ml @@ -113,48 +113,6 @@ let rec install_drivers ((g, _) as reg) inspect = virtio_rng_supported, virtio_ballon_supported, isa_pvpanic_supported, virtio_socket_supported) ) -and install_linux_tools g inspect = - let os = - match inspect.i_distro with - | "fedora" -> Some "fc28" - | "rhel" | "centos" | "scientificlinux" | "redhat-based" - | "oraclelinux" -> - (* map 6 -> "el6" etc. *) - if inspect.i_major_version >= 6 then - Some (sprintf "el%d" inspect.i_major_version) - else - None - | "sles" | "suse-based" | "opensuse" -> Some "lp151" - | _ -> None in - - match os with - | None -> () - | Some os -> - let src_path = "linux" // os in - let dst_path = "/var/tmp" in - let pkg_arch = Linux.architecture_string inspect in - let pkg_ext = Linux.binary_package_extension inspect in - let package_suffixes = [ - sprintf ".%s.%s" pkg_arch pkg_ext; - sprintf "_%s.%s" pkg_arch pkg_ext; - ] in - let package_filter path _ = - List.exists (String.is_suffix path) package_suffixes - in - debug "locating packages in %s" src_path; - let packages = - copy_from_virtio_win g inspect src_path dst_path - package_filter - (fun () -> ()) in - debug "done copying %d files" (List.length packages); - let packages = List.map ((//) dst_path) packages in - try - Linux.install_local g inspect packages; - if packages <> [] then - info (f_"QEMU Guest Agent installed for this guest."); - 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 -- 2.19.1.3.g30247aa5d201

Register a firstboot script, for installing the guest agent with the guest's own package manager -- that is, "Guest_packages.install_command". For installing the package, network connectivity is required. Check it first with "nmcli" (also checking whether NetworkManager is running), then with "systemd-networkd-wait-online" (dependent on systemd-networkd). Note that NetworkManager and systemd-networkd are never supposed to be enabled at the same time. The source domain's SELinux policy may not allow our firstboot service to execute the package's installation scripts (if any). For that reason, temporarily disable SELinux around package installation. After installation, register another script for launching the agent. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2028764 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- Notes: v2: - drop the "pkg_mgmt" helper variable; replace the call to "g#inspect_get_package_management" with the already populated "inspect.i_package_management" [Rich] - replace "sleep 60" with an open-coded "nmcli + sleep" loop, plus systemd-networkd-wait-online [Rich]: - refresh submodule checkout against now-upstream libguestfs-common commit 9e990f3e4530 ("mlcustomize: factor out pkg install/update/uninstall from guestfs-tools", 2022-06-09) convert/convert_linux.ml | 78 +++++++++++++++++++- common | 2 +- 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml index 2ddbc07aa86a..59d143bdda4b 100644 --- a/convert/convert_linux.ml +++ b/convert/convert_linux.ml @@ -562,8 +562,82 @@ let convert (g : G.guestfs) source inspect keep_serial_console _ = name = qga_pkg ) inspect.i_apps in if not has_qemu_guest_agent then - (* FIXME -- install qemu-guest-agent here *) - () + try + let inst_cmd = Guest_packages.install_command [qga_pkg] + inspect.i_package_management in + + (* Use only the portable filename character set in this. *) + let selinux_enforcing = "/root/virt-v2v-fb-selinux-enforcing" + and timeout = 30 in + let fbs = + Firstboot.add_firstboot_script g inspect.i_root + in + info (f_"The QEMU Guest Agent will be installed for this guest at \ + first boot."); + + (* Wait for the network to come online in the guest (best effort). + *) + fbs "wait online" + (sprintf "#!/bin/sh\n\ + if conn=$(nmcli networking connectivity); then\n\ + \ \ tries=0\n\ + \ \ while\n\ + \ \ \ \ test $tries -lt %d &&\n\ + \ \ \ \ test full != \"$conn\"\n\ + \ \ do\n\ + \ \ \ \ sleep 1\n\ + \ \ \ \ tries=$((tries + 1))\n\ + \ \ \ \ conn=$(nmcli networking connectivity)\n\ + \ \ done\n\ + elif systemctl -q is-active systemd-networkd; then\n\ + \ \ /usr/lib/systemd/systemd-networkd-wait-online \\\n\ + \ \ \ \ -q --timeout=%d\n\ + fi\n" timeout timeout); + + (* Disable SELinux temporarily around package installation. Refer to + * <https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c7> and + * <https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c8>;. + *) + fbs "setenforce 0" + (sprintf "#!/bin/sh\n\ + rm -f %s\n\ + if command -v getenforce >/dev/null &&\n\ + \ \ test Enforcing = \"$(getenforce)\"\n\ + then\n\ + \ \ touch %s\n\ + \ \ setenforce 0\n\ + fi\n" selinux_enforcing selinux_enforcing); + fbs "install qga" inst_cmd; + fbs "setenforce restore" + (sprintf "#!/bin/sh\n\ + if test -f %s; then\n\ + \ \ setenforce 1\n\ + \ \ rm -f %s\n\ + fi\n" selinux_enforcing selinux_enforcing); + + (* Start the agent now and at subsequent boots. The following + * commands should work on both sysvinit distros / distro versions + * (regardless of "/etc/rc.d/" vs. "/etc/init.d/" being the scheme + * in use) and systemd distros (via redirection to systemctl). + * + * On distros where the chkconfig command is redirected to + * systemctl, the chkconfig command is likely superfluous. That's + * because on systemd distros, the QGA package comes with such + * runtime dependencies / triggers that the presence of the + * virtio-serial port named "org.qemu.guest_agent.0" automatically + * starts the agent during (second and later) boots. However, even + * on such distros, the chkconfig command should do no harm. + *) + fbs "start qga" + (sprintf "#!/bin/sh\n\ + service %s start\n\ + chkconfig %s on\n" qga_pkg qga_pkg) + with + | Guest_packages.Unknown_package_manager msg + | Guest_packages.Unimplemented_package_manager msg -> + warning (f_"The QEMU Guest Agent will not be installed. The \ + install command for package ‘%s’ could not be created: \ + %s.") qga_pkg msg and configure_kernel () = (* Previously this function would try to install kernels, but we diff --git a/common b/common index 48527b8768d7..9e990f3e4530 160000 --- a/common +++ b/common @@ -1 +1 @@ -Subproject commit 48527b8768d7e010552beb62500a46ad940bca9a +Subproject commit 9e990f3e4530df3708d176bc50e0bc68cf07d3ff -- 2.19.1.3.g30247aa5d201

On 06/13/22 19:33, Richard W.M. Jones wrote: Nope :) The first (original) problem is that I very much dislike when we put scripts and other verbatim texts (multiline strings) to the very left in the OCaml source; it breaks up the otherwise great indentation of the OCaml code. Therefore I finish the lines in the string with \ -- subsequently, OCaml throws away (from the string) the newline character, plus *all* leading whitespace on the next line. "All whitespace" is the second problem then; I want the compiler to throw away those space chars that are meant for the OCaml identation, but not those that indent the embedded shell script! Therefore, I need to escape at least the very first space character of the shell script's indentation. In theory, the further escapes would not be necessary; so for example rather than \ \ while\n\ I should be able to write \ while\n\ But I kind of disliked this discrepancy. It seemed more consistent to say: "OCaml indentation (to be stripped): no escapes; shell script indentation (not to be stripped): escaped". Thanks! Laszlo