[p2v PATCH 0/4] RHEL9 build host tweaks
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(-)
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.log
containing, respectively:
bash,36312 `-pstree,36315 -a -A -l -n -p 36312
and
bash,36312 --noediting --noprofile `-pstree,36320 -a -A -l -n -p 36312
Note 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@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
On Thu, Jan 19, 2023 at 01:14:45PM +0100, Laszlo Ersek wrote:
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.log
containing, respectively:
bash,36312 `-pstree,36315 -a -A -l -n -p 36312
and
bash,36312 --noediting --noprofile `-pstree,36320 -a -A -l -n -p 36312
Note 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@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
Yes, this seems reasonable given that we were already using --noprofile. ACK. 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
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@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 \
On Thu, Jan 19, 2023 at 01:14:46PM +0100, Laszlo Ersek wrote:
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@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 \
ACK 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
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@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="
On Thu, Jan 19, 2023 at 01:14:47PM +0100, Laszlo Ersek wrote:
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@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="
Interesting - didn't know this would work ... $ printf '%s\n' 'abc' 'def' 123 abc def 123 It seems like it applies the format pattern to every parameter?! I verified this works both with bash printf and coreutils. bash info says: The FORMAT is reused as necessary to consume all of the ARGUMENTS. If the FORMAT requires more ARGUMENTS than are supplied, the extra format specifications behave as if a zero value or null string, as appropriate, had been supplied. The return value is zero on success, non-zero on failure. and almost the same text appears in coreutils info. So there we are. ACK! Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
On 1/19/23 19:56, Richard W.M. Jones wrote:
On Thu, Jan 19, 2023 at 01:14:47PM +0100, Laszlo Ersek wrote:
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@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="
Interesting - didn't know this would work ...
$ printf '%s\n' 'abc' 'def' 123 abc def 123
It seems like it applies the format pattern to every parameter?!
I verified this works both with bash printf and coreutils.
bash info says:
The FORMAT is reused as necessary to consume all of the ARGUMENTS. If the FORMAT requires more ARGUMENTS than are supplied, the extra format specifications behave as if a zero value or null string, as appropriate, had been supplied. The return value is zero on success, non-zero on failure.
and almost the same text appears in coreutils info.
POSIX even provides an informative EXAMPLES section that demonstrates the behavior when you have N > 1 conversion specifiers in the /format/ operand, but M > 0 /argument/ operands where M is not a whole multiple of N: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html#tag_2... printf "%5d%4d\n" 1 21 321 4321 54321 1 21 3214321 54321 0 There is no /argument/ operand for %4d in the third iteration, so 0 is substituted for it. That too happens by rule#9 <https://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html#tag_20_94_13>.
So there we are. ACK!
Series merged as commit range 2082cf98add8..f4c7ae6ba98f. Thanks! Laszlo
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@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 \
On Thu, Jan 19, 2023 at 01:14:48PM +0100, Laszlo Ersek wrote:
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@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 \
ACK, as discussed separately. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
participants (2)
-
Laszlo Ersek -
Richard W.M. Jones