In preparation for some GUI investigation / work, I've run "make run-virt-p2v-in-a-vm" for the first time on my new RHEL-9.1 laptop. Some tweaks appear to be necessary (or at least useful). Laszlo Laszlo Ersek (4): ssh: shield virt-v2v from bash RC files make-disk: restrict kernel update bug work-around make-disk: replace here-docs with printfs for better indentation make-disk: inject EPEL9 repo in RHEL9 disk image ssh.c | 2 +- test-virt-p2v-ssh.sh | 2 +- virt-p2v-make-disk.in | 57 +++++++++++++++++++++++++++++++------------ 3 files changed, 44 insertions(+), 17 deletions(-)
Laszlo Ersek
2023-Jan-19 12:14 UTC
[Libguestfs] [p2v PATCH 1/4] ssh: shield virt-v2v from bash RC files
I don't have virt-v2v installed system-wide on my RHEL9 laptop, I rely on the various "./run" scripts to put everything that virt-p2v's "make check" requires on the PATH environment variable. However, "test-virt-p2v-nbdkit.sh" still breaks for me; it complains that "virt-v2v --version" cannot be executed by test_connection(), due to "virt-v2v" not being found. Prefixing the "virt-v2v --version" invocation in test_connection() with "echo \"$PATH\"; ", I've determined from "test-virt-p2v-nbdkit.sh.log" that the *non-appending* PATH=... variable assignment from my $HOME/.bashrc takes effect. It wipes out the PATH changes from the "./run" scripts, hiding virt-v2v. I've added the following snippet to my $HOME/.bashrc file, for debugging:> XXX=mess-$(date --rfc-3339=ns)--$$.log > /bin/pstree -a -A -l -n -p $$ >| "$HOME/tmp/$XXX"so that whichever shell read the RC file create a log file, named with a nanosecond-resolution timestamp and the shell's PID, and record the "path" in the process tree that lead to the shell. The snippet created the following two files:> mess-2023-01-18 09:33:49.896065330+01:00--36312.log > mess-2023-01-18 09:33:49.937365639+01:00--36312.logcontaining, respectively:> bash,36312 > `-pstree,36315 -a -A -l -n -p 36312and> bash,36312 --noediting --noprofile > `-pstree,36320 -a -A -l -n -p 36312Note that the PID of the shell is unchanged, but the pstree PID changes. This means that the same PID (same process) reads the bash RC file twice -- which can only be explained by the *image* of the process being replaced, from bash, to bash. So the problem happens in two places: - First, when we "exec" the interactive shell in "test-virt-p2v-ssh.sh", that is, our ssh "shim". Interactive *non-login* shells read the RC file, unless the "--norc" option is passed. - Second, when we "exec" bash from start_ssh(), on the remote machine. This invocation already passes the "--noprofile" option, but that has no effect. "--noprofile" prevents the shell from reading "$HOME/.bash_profile" when the shell is a *login* shell, regardless of whether it is interactive or not. Because the existent "--noprofile" option does not prevent the symptom, we can determine that the remote shell started by start_ssh() is a *non-login* shell, and that it's also interactive (otherwise it wouldn't read the RC file). Thus, we need to pass "--norc" here as well. (While I believe, based on the above, that "--noprofile" is superfluous, I'd like to avoid any potential regressions here, so I'm keeping "--noprofile" too.) Append "--norc" to both command lines. Signed-off-by: Laszlo Ersek <lersek at redhat.com> --- ssh.c | 2 +- test-virt-p2v-ssh.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ssh.c b/ssh.c index aeb57584cf55..513a20318359 100644 --- a/ssh.c +++ b/ssh.c @@ -469,7 +469,7 @@ start_ssh (unsigned spawn_flags, struct config *config, * We don't know how command line editing is set up * (https://bugzilla.redhat.com/1314244#c9). */ - if (mexp_printf (h, "exec bash --noediting --noprofile\n") == -1) { + if (mexp_printf (h, "exec bash --noediting --noprofile --norc\n") == -1) { set_ssh_mexp_error ("mexp_printf"); mexp_close (h); return NULL; diff --git a/test-virt-p2v-ssh.sh b/test-virt-p2v-ssh.sh index 8a14b71fbd4b..f8b86b539ffe 100755 --- a/test-virt-p2v-ssh.sh +++ b/test-virt-p2v-ssh.sh @@ -57,4 +57,4 @@ while true ; do done # Now run the interactive shell. -exec bash +exec bash --norc
Laszlo Ersek
2023-Jan-19 12:14 UTC
[Libguestfs] [p2v PATCH 2/4] make-disk: restrict kernel update bug work-around
Commit 2764abefaa14 ("make-disk: Don't upgrade the kernel to avoid kernel command line bug", 2022-03-24) replaced the "--update" option of virt-builder with an open-coded DNF update command, excluding the kernel packages from the update. Albeit a justified workaround, it is a bit too broad: it breaks on all guest OS images that don't have DNF, such as old RHEL, Debian and Ubuntu, etc. (In a subsequent patch, I'm going to refactor the Debian/Ubuntu branch, and for testing that, I first need eliminate the open-coded DNF command.) Refine the workaround as follows: - I've tested Fedora 33 and Fedora 37; they work without the workaround (they are not yet, and no longer, affected by the bug, respectively). - I've tested Fedora 34 and 36, they are affected. They need the workaround. - Fedora 35 is also broken, per <https://bugzilla.redhat.com/show_bug.cgi?id=1945835#c24>. - RHEL-9.0 and RHEL-9.1 *seem* to work fine without the workaround (that is, with commit 2764abefaa14 reverted). However, that's deceptive: in my testing, the original "--update" option does not bring in any new kernel packages, so even if the bug is still there, it is not triggered. Because RHEL-9 is based on Fedora 34, which is affected, assume that RHEL-9 is affected too. Signed-off-by: Laszlo Ersek <lersek at redhat.com> --- virt-p2v-make-disk.in | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/virt-p2v-make-disk.in b/virt-p2v-make-disk.in index 865a92847f8b..44ff0b45ac7a 100644 --- a/virt-p2v-make-disk.in +++ b/virt-p2v-make-disk.in @@ -48,6 +48,7 @@ eval set -- "$TEMP" output upload verbose+declare -a update_option=(--update) declare -a passthru usage () @@ -184,6 +185,19 @@ EOF --upload $tmpdir/p2v.conf:/etc/dracut.conf.d/ --run $tmpdir/post-install " + # Fedora 34 through 36, and presumably RHEL-9 (based on Fedora 34) + # suffer from a bug where a kernel image installation via libguestfs + # pollutes the just installed kernel's commandline in the boot loader + # config file. Irrelevant and bogus parameters from the appliance + # kernel's /proc/cmdline are copied over. This prevents the root + # filesystem from being mounted. Refer to + # <https://bugzilla.redhat.com/show_bug.cgi?id=1945835#c24> and/or + # commit 2764abefaa14. + case "$osversion" in + fedora-34|fedora-35|fedora-36|rhel-9.*) + update_option=(--run-command 'dnf -y update --exclude=kernel\*') + ;; + esac ;; debian-*|ubuntu-*) depsfile="$datadir/dependencies.debian" @@ -246,7 +260,7 @@ virt-builder "$osversion" \ $preinstall_args \ --hostname p2v.local \ --run-command 'hostname p2v.local' \ - --run-command 'dnf -y update --exclude=kernel\*' \ + "${update_option[@]}" \ --install "$install" \ --root-password password:p2v \ --upload "$datadir"/issue:/etc/issue \
Laszlo Ersek
2023-Jan-19 12:14 UTC
[Libguestfs] [p2v PATCH 3/4] make-disk: replace here-docs with printfs for better indentation
Here-documents break the indentation of the script; replace them with the "printf" shell utility. (We could use the <<- redirection operator for stripping leading TABs from the here-docs, but the script itself does not use TABs for indentation, so that would introduce a different kind of inconsistency.) Because we use a quoted word ('EOF') for our delimiter in the here-documents, variables in the here-docs are not expanded; stick with that. Use a sole '%s\n' format operand with each printf invocation: "[t]he /format/ operand shall be reused as often as necessary to satisfy the /argument/ operands" <https://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html>. (I've tried to regression-test this with the two latest Ubuntu images: ubuntu-18.04, ubuntu-20.04. The former wouldn't build, and the latter only booted to a GDM login screen. My build host is RHEL-9.1, so the kiosk is not actually expected to work. Any further investigation here is left to Ubuntu users. The Fedora 37 image works fine, even with the virt-p2v binary coming from RHEL-9.1.) Signed-off-by: Laszlo Ersek <lersek at redhat.com> --- virt-p2v-make-disk.in | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/virt-p2v-make-disk.in b/virt-p2v-make-disk.in index 44ff0b45ac7a..0bf7f0e0414f 100644 --- a/virt-p2v-make-disk.in +++ b/virt-p2v-make-disk.in @@ -170,15 +170,15 @@ xzcat "$virt_p2v_xz_binary" > "$virt_p2v_binary" case "$osversion" in centos-*|fedora-*|rhel-*|scientificlinux-*|oraclelinux-*) depsfile="$datadir/dependencies.redhat" - cat > $tmpdir/p2v.conf <<'EOF' -add_drivers+=" usb-storage " -EOF - cat > $tmpdir/post-install <<'EOF' -#!/bin/bash -# Rebuild the initramfs. -latest_version="$(cd /lib/modules; ls -1vr | head -1)" -dracut -v -f --kver $latest_version -EOF + printf '%s\n' \ + 'add_drivers+=" usb-storage "' \ + > $tmpdir/p2v.conf + printf '%s\n' \ + '#!/bin/bash' \ + '# Rebuild the initramfs.' \ + 'latest_version="$(cd /lib/modules; ls -1vr | head -1)"' \ + 'dracut -v -f --kver $latest_version' \ + > $tmpdir/post-install # Double quotes because we want $tmpdir to be expanded: extra_args=" --selinux-relabel @@ -201,11 +201,11 @@ EOF ;; debian-*|ubuntu-*) depsfile="$datadir/dependencies.debian" - cat > $tmpdir/policy-rc.d <<'EOF' -#!/bin/sh -# Avoid running daemons during the upgrade/installation -exit 101 -EOF + printf '%s\n' \ \ + '#!/bin/sh' \ + '# Avoid running daemons during the upgrade/installation' \ + 'exit 101' \ + > $tmpdir/policy-rc.d chmod +x $tmpdir/policy-rc.d # Double quotes because we want $tmpdir to be expanded: preinstall_args="
Laszlo Ersek
2023-Jan-19 12:14 UTC
[Libguestfs] [p2v PATCH 4/4] make-disk: inject EPEL9 repo in RHEL9 disk image
In RHEL9, "metacity" was replaced with "gnome-kiosk": https://bugzilla.redhat.com/show_bug.cgi?id=1951145#c4 https://bugzilla.redhat.com/show_bug.cgi?id=1956984#c1 gnome-kiosk has particular requirements for the main application: https://bugzilla.redhat.com/show_bug.cgi?id=1999060#c13 We don't know if that's compatible with our separately opened XTerm window(s). For now, let's stick with metacity, by pulling in EPEL9. We can work on RHEL9 purity (including gnome-kiosk fixes and/or compatibility) once a "standard" RHEL9-based image builder service becomes a realistic prospect. Signed-off-by: Laszlo Ersek <lersek at redhat.com> --- virt-p2v-make-disk.in | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/virt-p2v-make-disk.in b/virt-p2v-make-disk.in index 0bf7f0e0414f..63001debbf3e 100644 --- a/virt-p2v-make-disk.in +++ b/virt-p2v-make-disk.in @@ -48,6 +48,7 @@ eval set -- "$TEMP" output upload verbose+declare -a install_repo_packages_option declare -a update_option=(--update) declare -a passthru @@ -198,6 +199,17 @@ case "$osversion" in update_option=(--run-command 'dnf -y update --exclude=kernel\*') ;; esac + # For RHEL-9, we need to grab the metacity window manager from EPEL-9. + # Enable the EPEL-9 repository by installing the RPM containing the + # *.repo file(s) directly from the network. + case "$osversion" in + rhel-9.*) + install_repo_packages_option=( + --install + https://dl.fedoraproject.org/pub/epel/epel-release-latest-9.noarch.rpm + ) + ;; + esac ;; debian-*|ubuntu-*) depsfile="$datadir/dependencies.debian" @@ -260,6 +272,7 @@ virt-builder "$osversion" \ $preinstall_args \ --hostname p2v.local \ --run-command 'hostname p2v.local' \ + "${install_repo_packages_option[@]}" \ "${update_option[@]}" \ --install "$install" \ --root-password password:p2v \