Libguestfs - Jun 2022 - [v2v PATCH v2 1/4] output/create_libvirt_xml: wire up the QEMU guest agent

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2028764
v1:       https://listman.redhat.com/archives/libguestfs/2022-June/029082.html

Please see the "Notes" section on each patch for the updates in the v2
series.

Thanks
Laszlo

Laszlo Ersek (4):
  output/create_libvirt_xml: wire up the QEMU guest agent
  windows_virtio: remove "install_linux_tools"
  convert_linux: extract qemu-guest-agent package name
  convert_linux: install the QEMU guest agent with a firstboot script

 common                       |   2 +-
 convert/convert_linux.ml     | 107 ++++++++++++++++++--
 convert/linux.ml             |  35 -------
 convert/linux.mli            |  11 --
 convert/windows_virtio.ml    |  42 --------
 convert/windows_virtio.mli   |   4 -
 output/create_libvirt_xml.ml |  11 ++
 tests/test-v2v-i-ova.xml     |   4 +
 8 files changed, 116 insertions(+), 100 deletions(-)

-- 
2.19.1.3.g30247aa5d201

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 at redhat.com>
---

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 at redhat.com>
Acked-by: Richard W.M. Jones <rjones at redhat.com>
---

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

In commit a30383e35d34 ("v2v: linux: do not install qemu-guest-agent if
already installed", 2019-09-20), the name of the package providing the
QEMU guest agent was hard-coded as "qemu-guest-agent", regardless of
distro family. Turns out this is actually correct (and may have been
intentional, only it was not specifically documented): in all OS families
currently recognized by our "family" function (`RHEL_family,
`ALT_family,
`SUSE_family, `Debian_family), the *binary* package is indeed called
"qemu-guest-agent":

- https://brewweb.engineering.redhat.com/brew/packageinfo?packageID=47646
-
http://rpmfind.net/linux/rpm2html/search.php?query=qemu-guest-agent&submit=Search+...&system=&arch-
https://packages.altlinux.org/en/sisyphus/srpms/qemu/
-
https://packages.debian.org/search?keywords=qemu-guest-agent&searchon=names&suite=all&section=all

As a way of documenting this, extract the mapping to a new helper function
named "qga_pkg_of_family".

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2028764
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
Reviewed-by: Richard W.M. Jones <rjones at redhat.com>
---

Notes:
    v2:
    - pick up Rich's R-b

 convert/convert_linux.ml | 33 +++++++++++++++-----
 1 file changed, 25 insertions(+), 8 deletions(-)

diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml
index 79462aa1e7ae..2ddbc07aa86a 100644
--- a/convert/convert_linux.ml
+++ b/convert/convert_linux.ml
@@ -56,6 +56,16 @@ let convert (g : G.guestfs) source inspect
keep_serial_console _      | "debian" | "ubuntu" |
"linuxmint" | "kalilinux" -> `Debian_family
     | _ -> assert false in
 
+  (* map the OS family name to the qemu-guest-agent package name *)
+  let qga_pkg_of_family +    function
+    | `RHEL_family
+    | `ALT_family
+    | `SUSE_family
+    | `Debian_family -> Some "qemu-guest-agent"
+    | _ -> None
+  in
+
   assert (inspect.i_package_format = "rpm" ||
inspect.i_package_format = "deb");
 
   (* Fail early if i_apps is empty.  Certain steps such as kernel
@@ -539,14 +549,21 @@ 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
-      (* FIXME -- install qemu-guest-agent here *)
-      ()
+    match qga_pkg_of_family family with
+    | None -> warning (f_"The name of the package that provides the
QEMU Guest \
+                          Agent for this guest OS is unknown.  The guest agent
\
+                          will not be installed.  Please consider reporting a \
+                          bug according to the BUGS section of the virt-v2v(1)
\
+                          manual.")
+    | Some qga_pkg ->
+        let has_qemu_guest_agent +          List.exists (
+            fun { G.app2_name = name } ->
+              name = qga_pkg
+          ) inspect.i_apps in
+        if not has_qemu_guest_agent then
+          (* FIXME -- install qemu-guest-agent here *)
+          ()
 
   and configure_kernel ()      (* Previously this function would try to install
kernels, but we
-- 
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 at redhat.com>
---

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]:
    
    > #!/bin/sh
    > if conn=$(nmcli networking connectivity); then
    >   tries=0
    >   while
    >     test $tries -lt 30 &&
    >     test full != "$conn"
    >   do
    >     sleep 1
    >     tries=$((tries + 1))
    >     conn=$(nmcli networking connectivity)
    >   done
    > elif systemctl -q is-active systemd-networkd; then
    >   /usr/lib/systemd/systemd-networkd-wait-online \
    >     -q --timeout=30
    > fi
    
    - 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 Mon, Jun 13, 2022 at 07:01:31PM +0200, Laszlo Ersek
wrote:
> Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2028764
> v1:      
https://listman.redhat.com/archives/libguestfs/2022-June/029082.html
> 
> Please see the "Notes" section on each patch for the updates in
the v2
> series.
Series looks good now, thanks.

Rich.
> Thanks
> Laszlo
> 
> Laszlo Ersek (4):
>   output/create_libvirt_xml: wire up the QEMU guest agent
>   windows_virtio: remove "install_linux_tools"
>   convert_linux: extract qemu-guest-agent package name
>   convert_linux: install the QEMU guest agent with a firstboot script
> 
>  common                       |   2 +-
>  convert/convert_linux.ml     | 107 ++++++++++++++++++--
>  convert/linux.ml             |  35 -------
>  convert/linux.mli            |  11 --
>  convert/windows_virtio.ml    |  42 --------
>  convert/windows_virtio.mli   |   4 -
>  output/create_libvirt_xml.ml |  11 ++
>  tests/test-v2v-i-ova.xml     |   4 +
>  8 files changed, 116 insertions(+), 100 deletions(-)
> 
> -- 
> 2.19.1.3.g30247aa5d201
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://listman.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
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org