[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@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 ->
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@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
On 08/17/22 18:03, Richard W.M. Jones wrote:
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@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 ->
No, I checked RHEL5, and it had never shipped the guest agent: https://bugzilla.redhat.com/show_bug.cgi?id=2028764#c52 Most probably, upstream QEMA had not introduced QGA by then. In fact, RHEL-6's QEMU version is based on upstream 0.12, which is (IIRC) the very first version of upstream QEMU that integrated KVM support; so that was when we switched from "kvm" to "qemu" in userspace. So even if upstream QEMU had introduced QGA before 0.12, RHEL5 would not have used it.
?
+ (* 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.
I considered this, but opted out of it, because the call to "qga_svc_start_cmd" depends on additional conditions relative to the dependencies of the call to "qga_pkg_of_family": (1) If the package is already installed, we don't need the startup command; (2) if the package management system is unknown or unimplemented in v2v, we don't need the startup command. I don't think it would be a huge waste of CPU cycles, I just conceptually don't like computing something that we might throw away later. (Unless precomputing can significantly improve performace later on, of course.) ... We can still refactor it later, should we ever need to extend both functions with a new OS pattern.
It all looks fine, so ACK either way.
Thanks; I'll go ahead and merge it, and backport it too, in order to let QE start testing it soon. Laszlo
participants (2)
-
Laszlo Ersek -
Richard W.M. Jones