Laszlo Ersek
2022-Aug-17 14:47 UTC
[Libguestfs] [v2v PATCH] convert_linux: start the QEMU guest agent in a distro-specific way
The current command "service <package-name> start" does not
apply to
RHEL-6; the service name ("qemu-ga") differs from the package name
("qemu-guest-agent") there.
Overhaul the logic -- detach the command from the package name; cover the
RHEL, ALT, SUSE and Debian families separately. Remove the "chkconfig"
command, as in all tested / investigated cases, it is unnecessary.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2028764
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
convert/convert_linux.ml | 56 ++++++++++++++++++++++++++++------------
1 file changed, 40 insertions(+), 16 deletions(-)
diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml
index 2aaa438e42ac..b8e9ad15e22d 100644
--- a/convert/convert_linux.ml
+++ b/convert/convert_linux.ml
@@ -66,6 +66,34 @@ let convert (g : G.guestfs) source inspect
keep_serial_console _ | _ -> None
in
+ let qga_svc_start_cmd family distro major + match family, distro, major
with
+ | `RHEL_family, ( "rhel" | "centos" |
"scientificlinux" | "redhat-based" |
+ "oraclelinux" ), 6 ->
+ (* https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c52 *)
+ Some "service qemu-ga start"
+
+ | `RHEL_family, _, _ ->
+ (* https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c52 *)
+ Some "systemctl start qemu-guest-agent"
+
+ | `ALT_family, _, _ ->
+ (* https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c45 *)
+ Some "systemctl start qemu-guest-agent"
+
+ | `SUSE_family, _, _ ->
+ (* https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c51 *)
+ None
+
+ | `Debian_family, _, _ ->
+ (* https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c42 *)
+ Some "service qemu-guest-agent start"
+
+ | _ ->
+ (* should never be called when "qga_pkg_of_family" returns None
*)
+ assert false
+ in
+
assert (inspect.i_package_format = "rpm" ||
inspect.i_package_format = "deb");
(* Fail early if i_apps is empty. Certain steps such as kernel
@@ -615,23 +643,19 @@ let convert (g : G.guestfs) source inspect
keep_serial_console _ \ \ 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.
+ (* On all the distro families covered by
"qga_pkg_of_family" and
+ * "qga_svc_start_cmd", the QEMU guest agent service is
always
+ * enabled by package installation for *subsequent* boots. Package
+ * installation may or may not enable the service for the current
+ * (i.e., first) boot, however, so try that here manually.
*)
- fbs "start qga"
- (sprintf "#!/bin/sh\n\
- service %s start\n\
- chkconfig %s on\n" qga_pkg qga_pkg)
+ match qga_svc_start_cmd family inspect.i_distro
inspect.i_major_version
+ with
+ | None -> ()
+ | Some start_cmd ->
+ fbs "start qga"
+ (sprintf "#!/bin/sh\n\
+ %s\n" start_cmd)
with
| Guest_packages.Unknown_package_manager msg
| Guest_packages.Unimplemented_package_manager msg ->
Richard W.M. Jones
2022-Aug-17 16:03 UTC
[Libguestfs] [v2v PATCH] convert_linux: start the QEMU guest agent in a distro-specific way
On Wed, Aug 17, 2022 at 04:47:36PM +0200, Laszlo Ersek wrote:> The current command "service <package-name> start" does not apply to > RHEL-6; the service name ("qemu-ga") differs from the package name > ("qemu-guest-agent") there. > > Overhaul the logic -- detach the command from the package name; cover the > RHEL, ALT, SUSE and Debian families separately. Remove the "chkconfig" > command, as in all tested / investigated cases, it is unnecessary. > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2028764 > Signed-off-by: Laszlo Ersek <lersek at redhat.com> > --- > convert/convert_linux.ml | 56 ++++++++++++++++++++++++++++------------ > 1 file changed, 40 insertions(+), 16 deletions(-) > > diff --git a/convert/convert_linux.ml b/convert/convert_linux.ml > index 2aaa438e42ac..b8e9ad15e22d 100644 > --- a/convert/convert_linux.ml > +++ b/convert/convert_linux.ml > @@ -66,6 +66,34 @@ let convert (g : G.guestfs) source inspect keep_serial_console _ > | _ -> None > in > > + let qga_svc_start_cmd family distro major > + match family, distro, major with > + | `RHEL_family, ( "rhel" | "centos" | "scientificlinux" | "redhat-based" | > + "oraclelinux" ), 6 ->Shouldn't this be: ... , i when i <= 6 -> ?> + (* https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c52 *) > + Some "service qemu-ga start" > + > + | `RHEL_family, _, _ -> > + (* https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c52 *) > + Some "systemctl start qemu-guest-agent" > + > + | `ALT_family, _, _ -> > + (* https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c45 *) > + Some "systemctl start qemu-guest-agent" > + > + | `SUSE_family, _, _ -> > + (* https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c51 *) > + None > + > + | `Debian_family, _, _ -> > + (* https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c42 *) > + Some "service qemu-guest-agent start" > + > + | _ -> > + (* should never be called when "qga_pkg_of_family" returns None *) > + assert false > + in > + > assert (inspect.i_package_format = "rpm" || inspect.i_package_format = "deb"); > > (* Fail early if i_apps is empty. Certain steps such as kernel > @@ -615,23 +643,19 @@ let convert (g : G.guestfs) source inspect keep_serial_console _ > \ \ 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. > + (* On all the distro families covered by "qga_pkg_of_family" and > + * "qga_svc_start_cmd", the QEMU guest agent service is always > + * enabled by package installation for *subsequent* boots. Package > + * installation may or may not enable the service for the current > + * (i.e., first) boot, however, so try that here manually. > *) > - fbs "start qga" > - (sprintf "#!/bin/sh\n\ > - service %s start\n\ > - chkconfig %s on\n" qga_pkg qga_pkg) > + match qga_svc_start_cmd family inspect.i_distro inspect.i_major_version > + with > + | None -> () > + | Some start_cmd -> > + fbs "start qga" > + (sprintf "#!/bin/sh\n\ > + %s\n" start_cmd) > with > | Guest_packages.Unknown_package_manager msg > | Guest_packages.Unimplemented_package_manager msg ->If you wanted to reduce the long range dependency between qga_pkg_of_family and qga_svc_start_cmd (and hence the assert) you could make a qga_something function that returns both the package name and the service start command as a pair of strings (or None). It could make the code more maintainable long term. It all looks fine, so ACK either way. 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