Re: [Libguestfs] [v2v PATCH 4/4] convert_linux: install the QEMU guest agent with a firstboot script

On Mon, Jun 06, 2022 at 04:19:41PM +0200, Laszlo Ersek wrote: > 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; for lack of > a universal "wait online" dependency expression, register "sleep 60" as a > preceding firstboot script. > > 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; > --- > convert/convert_linux.ml | 73 +++++++++++++++++++- > common | 2 +- > 2 files changed, 72 insertions(+), 3 deletions(-) > > diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml > index 2ddbc07aa86a..1dcd7abbcee2 100644 > --- a/convert/convert_linux.ml > +++ b/convert/convert_linux.ml > @@ -562,8 +562,77 @@ 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 *) > - () > + let pkg_mgmt = g#inspect_get_package_management inspect.i_root in You can just use: inspect.i_package_management to avoid round-tripping through the appliance for information that we already have locally.

> + try > + let inst_cmd = Guest_packages.install_command [qga_pkg] pkg_mgmt in > + > + (* Use only the portable filename character set in this. *) > + let selinux_enforcing = "/root/virt-v2v-fb-selinux-enforcing" 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."); > + > + (* Waiting for the guest to go "online" is very complicated. On > + * systemd-based distros, our firstboot service could perhaps depend > + * on "systemd-networkd-wait-online.service" or > + * "NetworkManager-wait-online.service". But not all Linux distros > + * are based on systemd, and even with systemd, a guest could use > + * systemd-networkd vs. NetworkManager... So just do what we do in > + * various Windows firstboot scripts: wait. We could cut the waiting > + * short by pinging a well-known IP address, or by resolving a > + * well-known FQDN, but those might qualify as "phoning home"... So > + * just wait. > + *) > + fbs "wait online" > + "#!/bin/sh\n\ > + sleep 60\n"; Hmmm. virt-p2v uses: nm-online -t 30 Apparently systemd-networkd (which I've never knowingly used) has another command: /usr/lib/systemd/systemd-networkd-wait-online which ought to do the same thing. (It defaults to 120 second timeout.)

Can we not do something like: nm-online -t 30 ||: /usr/lib/systemd/systemd-networkd-wait-online ||: as a best effort? Seems a lot better idea than sleeping. Long term we really ought to add this as a feature of the Firstboot module, ie. a parameter which allows a firstboot script to be flagged as needing the network, and we'll insert the appropriate code for those scripts automatically.

> + (* 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); Sounds horrible! But if that's what is needed ... > + (* 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 The patch seems fine in general, but definitely want to get rid of that sleep.