Libguestfs - Jun 2022 - [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 at redhat.com>
> ---
>  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.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
nbdkit - Flexible, fast NBD server with plugins
https://gitlab.com/nbdkit/nbdkit

On Tue, Jun 07, 2022 at 01:59:30PM +0100, Richard W.M. Jones
wrote:
> On Mon, Jun 06, 2022 at 04:19:41PM +0200, Laszlo Ersek wrote:
> > +            (* 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 ...
OK, now I caught up with the BZ comments, it really seems odd to me
that a service or script can run dnf, but that dnf doesn't transition
to the right SELinux context in order to do its work, but also dnf
doesn't fail immediately ("error: wrong context!") either.

However I don't know enough about SELinux to really understand whether
this is how it's supposed to work or not.

In reply to your other comment about --firstboot-install, it is
possible that this did work but has seen been broken by some change.
I don't believe we test it thoroughly anywhere.

Rich.

-- 
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

On 06/07/22 14:59, Richard W.M. Jones wrote:
> 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 at redhat.com>
>> ---
>>  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.
Ah great, will do that.
> 
>> +          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.)
It's horrible that these incantations are mostly impossible to figure
out :/ anyway, definitely an improvement over the static sleep, so I'll
use your suggestion from below:
> 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.
Let's do that when we have three (or at least two) clients for that feature.
> 
>> +            (* 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.
Yep, I'll rework that.

Thanks for the review!
Laszlo

On 06/07/22 14:59, Richard W.M. Jones wrote:
> 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.
After some investigation:

- NetworkManager and systemd-networkd-wait-online seem mutually
exclusive, and switching from the former to the latter (which is
obviously only possible on a systemd-based system) takes (mostly) one
"systemctl disable" and another "systemctl enable" command:

https://fedoraproject.org/wiki/Cloud/Network-Requirements#Switching_from_NetworkManager_to_systemd-networkd

- When one of { NetworkManager, systemd-networkd-wait-online } is
*disabled* (not running), and we invoke the "wait online" utility for
that (disabled) service, the utility will wait (uselessly) for the
timeout, and then report failure / timeout. I've tested this with
"systemd-networkd-wait-online" on my F35 workstation (where
NetworkManager is enabled -- the default), and with "nm-online" in a
RHEL9 guest, where I could disable NetworkManager without risks.

- This useless waiting (for NetworkManager that's not running) can be
disabled with the "-x" switch for nm-online, but I found no
corresponding switch for "systemd-networkd-wait-online". What works is
"systemctl is-active systemd-networkd", which is reasonable, because
"systemd-networkd" already depends on systemd.

Additionally the default timeout for nm-online seems to be 30 seconds
already.

So ultimately I believe we should do:

nm-online -x -q ||
(
  systemctl -q is-active systemd-networkd &&
  /usr/lib/systemd/systemd-networkd-wait-online -q --timeout=30
)

If the final exit status is nonzero, I think that's not a problem for
the firstboot script.

The whole command seems to make sense also when nm-online, systemctl, or
both, are missing (a missing command results in exit status 127).

Thanks
Laszlo