The environment variable entries in /proc/[pid]/environ are separated by null bytes instead of by spaces. Update the sed regex to fix this issue.
Note this patch also fixes a issue which is kdumpctl would try to reset crashkernel even osbuild has provided custom crashkernel value.
Fixes: ddd428a ("set up kernel crashkernel for osbuild in kernel hook") Signed-off-by: Coiby Xu coxu@redhat.com --- kdumpctl | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 8107487..b32fac0 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1578,7 +1578,7 @@ reset_crashkernel_after_update()
_is_osbuild() { - [[ $(sed -n -E 's/.*(^|\s)container=(\S*).*/\2/p' < /proc/1/environ) == bwrap-osbuild ]] + [[ $(sed -n -E 's/.*(^|\x00)container=([^\x00]*).*/\2/p' < /proc/1/environ) == bwrap-osbuild ]] }
reset_crashkernel_for_installed_kernel() @@ -1590,8 +1590,10 @@ reset_crashkernel_for_installed_kernel() exit 1 fi
- if _is_osbuild && ! grep -qs crashkernel= /etc/kernel/cmdline; then - reset_crashkernel "--kernel=$_installed_kernel" + if _is_osbuild; then + if ! grep -qs crashkernel= /etc/kernel/cmdline; then + reset_crashkernel "--kernel=$_installed_kernel" + fi return fi
On Wed, Jan 19, 2022 at 2:33 PM Coiby Xu coxu@redhat.com wrote:
The environment variable entries in /proc/[pid]/environ are separated by null bytes instead of by spaces. Update the sed regex to fix this issue.
Note this patch also fixes a issue which is kdumpctl would try to reset crashkernel even osbuild has provided custom crashkernel value.
Fixes: ddd428a ("set up kernel crashkernel for osbuild in kernel hook") Signed-off-by: Coiby Xu coxu@redhat.com
kdumpctl | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 8107487..b32fac0 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1578,7 +1578,7 @@ reset_crashkernel_after_update()
_is_osbuild() {
[[ $(sed -n -E 's/.*(^|\s)container=(\S*).*/\2/p' < /proc/1/environ) == bwrap-osbuild ]]
[[ $(sed -n -E 's/.*(^|\x00)container=([^\x00]*).*/\2/p' < /proc/1/environ) == bwrap-osbuild ]]
It looks right, but could you input some data, so I can be more sure about it.
}
reset_crashkernel_for_installed_kernel() @@ -1590,8 +1590,10 @@ reset_crashkernel_for_installed_kernel() exit 1 fi
if _is_osbuild && ! grep -qs crashkernel= /etc/kernel/cmdline; thenreset_crashkernel "--kernel=$_installed_kernel"
if _is_osbuild; thenif ! grep -qs crashkernel= /etc/kernel/cmdline; thenreset_crashkernel "--kernel=$_installed_kernel"fi return fi-- 2.31.1
Else, LGTM,
Reviewed-by: Pingfan Liu piliu@redhat.com
On Wed, Jan 19, 2022 at 04:49:45PM +0800, Pingfan Liu wrote:
On Wed, Jan 19, 2022 at 2:33 PM Coiby Xu coxu@redhat.com wrote:
The environment variable entries in /proc/[pid]/environ are separated by null bytes instead of by spaces. Update the sed regex to fix this issue.
Note this patch also fixes a issue which is kdumpctl would try to reset crashkernel even osbuild has provided custom crashkernel value.
Fixes: ddd428a ("set up kernel crashkernel for osbuild in kernel hook") Signed-off-by: Coiby Xu coxu@redhat.com
kdumpctl | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 8107487..b32fac0 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1578,7 +1578,7 @@ reset_crashkernel_after_update()
_is_osbuild() {
[[ $(sed -n -E 's/.*(^|\s)container=(\S*).*/\2/p' < /proc/1/environ) == bwrap-osbuild ]]
[[ $(sed -n -E 's/.*(^|\x00)container=([^\x00]*).*/\2/p' < /proc/1/environ) == bwrap-osbuild ]]It looks right, but could you input some data, so I can be more sure about it.
Sure, I'll rewrite this function in order to add some unit tests which I guess will be the data your need.
}
reset_crashkernel_for_installed_kernel() @@ -1590,8 +1590,10 @@ reset_crashkernel_for_installed_kernel() exit 1 fi
if _is_osbuild && ! grep -qs crashkernel= /etc/kernel/cmdline; thenreset_crashkernel "--kernel=$_installed_kernel"
if _is_osbuild; thenif ! grep -qs crashkernel= /etc/kernel/cmdline; thenreset_crashkernel "--kernel=$_installed_kernel"fi return fi-- 2.31.1
Else, LGTM,
Reviewed-by: Pingfan Liu piliu@redhat.com
Thanks for reviewing this patch!
Hi Coiby,
On Wed, 19 Jan 2022 14:33:25 +0800 Coiby Xu coxu@redhat.com wrote:
The environment variable entries in /proc/[pid]/environ are separated by null bytes instead of by spaces. Update the sed regex to fix this issue.
Note this patch also fixes a issue which is kdumpctl would try to reset crashkernel even osbuild has provided custom crashkernel value.
Fixes: ddd428a ("set up kernel crashkernel for osbuild in kernel hook") Signed-off-by: Coiby Xu coxu@redhat.com
Thanks for catching this
Reviewed-by: Philipp Rudo prudo@redhat.com
kdumpctl | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 8107487..b32fac0 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1578,7 +1578,7 @@ reset_crashkernel_after_update()
_is_osbuild() {
- [[ $(sed -n -E 's/.*(^|\s)container=(\S*).*/\2/p' < /proc/1/environ) == bwrap-osbuild ]]
- [[ $(sed -n -E 's/.*(^|\x00)container=([^\x00]*).*/\2/p' < /proc/1/environ) == bwrap-osbuild ]]
}
reset_crashkernel_for_installed_kernel() @@ -1590,8 +1590,10 @@ reset_crashkernel_for_installed_kernel() exit 1 fi
- if _is_osbuild && ! grep -qs crashkernel= /etc/kernel/cmdline; then
reset_crashkernel "--kernel=$_installed_kernel"
- if _is_osbuild; then
if ! grep -qs crashkernel= /etc/kernel/cmdline; thenreset_crashkernel "--kernel=$_installed_kernel"fi
Hi Coiby,
I think I found an other problem related to this location...
On Wed, 19 Jan 2022 14:33:25 +0800 Coiby Xu coxu@redhat.com wrote:
The environment variable entries in /proc/[pid]/environ are separated by null bytes instead of by spaces. Update the sed regex to fix this issue.
Note this patch also fixes a issue which is kdumpctl would try to reset crashkernel even osbuild has provided custom crashkernel value.
Fixes: ddd428a ("set up kernel crashkernel for osbuild in kernel hook") Signed-off-by: Coiby Xu coxu@redhat.com
kdumpctl | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 8107487..b32fac0 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1578,7 +1578,7 @@ reset_crashkernel_after_update()
_is_osbuild() {
- [[ $(sed -n -E 's/.*(^|\s)container=(\S*).*/\2/p' < /proc/1/environ) == bwrap-osbuild ]]
- [[ $(sed -n -E 's/.*(^|\x00)container=([^\x00]*).*/\2/p' < /proc/1/environ) == bwrap-osbuild ]]
}
reset_crashkernel_for_installed_kernel() @@ -1590,8 +1590,10 @@ reset_crashkernel_for_installed_kernel() exit 1 fi
- if _is_osbuild && ! grep -qs crashkernel= /etc/kernel/cmdline; then
reset_crashkernel "--kernel=$_installed_kernel"
- if _is_osbuild; then
if ! grep -qs crashkernel= /etc/kernel/cmdline; thenreset_crashkernel "--kernel=$_installed_kernel"
When parsing the --kernel option in reset_crashkernel it calls _valid_grubby_kernel_path to verify the given path. But that function relies on grubby being installed which needn't be the case when _is_osbuild is true.
Or am I missing something?
Thanks Philipp
fi
Hi Philipp,
On Wed, Jan 19, 2022 at 04:48:57PM +0100, Philipp Rudo wrote:
Hi Coiby,
I think I found an other problem related to this location...
On Wed, 19 Jan 2022 14:33:25 +0800 Coiby Xu coxu@redhat.com wrote:
The environment variable entries in /proc/[pid]/environ are separated by null bytes instead of by spaces. Update the sed regex to fix this issue.
Note this patch also fixes a issue which is kdumpctl would try to reset crashkernel even osbuild has provided custom crashkernel value.
Fixes: ddd428a ("set up kernel crashkernel for osbuild in kernel hook") Signed-off-by: Coiby Xu coxu@redhat.com
kdumpctl | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 8107487..b32fac0 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1578,7 +1578,7 @@ reset_crashkernel_after_update()
_is_osbuild() {
- [[ $(sed -n -E 's/.*(^|\s)container=(\S*).*/\2/p' < /proc/1/environ) == bwrap-osbuild ]]
- [[ $(sed -n -E 's/.*(^|\x00)container=([^\x00]*).*/\2/p' < /proc/1/environ) == bwrap-osbuild ]]
}
reset_crashkernel_for_installed_kernel() @@ -1590,8 +1590,10 @@ reset_crashkernel_for_installed_kernel() exit 1 fi
- if _is_osbuild && ! grep -qs crashkernel= /etc/kernel/cmdline; then
reset_crashkernel "--kernel=$_installed_kernel"
- if _is_osbuild; then
if ! grep -qs crashkernel= /etc/kernel/cmdline; thenreset_crashkernel "--kernel=$_installed_kernel"When parsing the --kernel option in reset_crashkernel it calls _valid_grubby_kernel_path to verify the given path. But that function relies on grubby being installed which needn't be the case when _is_osbuild is true.
Thanks for raising up this concern!
Since kexec-tools recommends grubby, grubby has been there when the kernel hook 92-crashkernel.install is being called.
Btw, I was wrong that kernel recommends kexec-tools but somehow kernel is installed after kexec-tools for osbuild [1]. The order is also the same when I run, dnf install kernel-core kexec-tools --installroot=/ROOTDIR
I'm yet to find out why it's so.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=2024976#c5
Or am I missing something?
Thanks Philipp
fi
On Thu, 20 Jan 2022 14:43:08 +0800 Coiby Xu coxu@redhat.com wrote:
Hi Philipp,
On Wed, Jan 19, 2022 at 04:48:57PM +0100, Philipp Rudo wrote:
Hi Coiby,
I think I found an other problem related to this location...
On Wed, 19 Jan 2022 14:33:25 +0800 Coiby Xu coxu@redhat.com wrote:
The environment variable entries in /proc/[pid]/environ are separated by null bytes instead of by spaces. Update the sed regex to fix this issue.
Note this patch also fixes a issue which is kdumpctl would try to reset crashkernel even osbuild has provided custom crashkernel value.
Fixes: ddd428a ("set up kernel crashkernel for osbuild in kernel hook") Signed-off-by: Coiby Xu coxu@redhat.com
kdumpctl | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/kdumpctl b/kdumpctl index 8107487..b32fac0 100755 --- a/kdumpctl +++ b/kdumpctl @@ -1578,7 +1578,7 @@ reset_crashkernel_after_update()
_is_osbuild() {
- [[ $(sed -n -E 's/.*(^|\s)container=(\S*).*/\2/p' < /proc/1/environ) == bwrap-osbuild ]]
- [[ $(sed -n -E 's/.*(^|\x00)container=([^\x00]*).*/\2/p' < /proc/1/environ) == bwrap-osbuild ]]
}
reset_crashkernel_for_installed_kernel() @@ -1590,8 +1590,10 @@ reset_crashkernel_for_installed_kernel() exit 1 fi
- if _is_osbuild && ! grep -qs crashkernel= /etc/kernel/cmdline; then
reset_crashkernel "--kernel=$_installed_kernel"
- if _is_osbuild; then
if ! grep -qs crashkernel= /etc/kernel/cmdline; thenreset_crashkernel "--kernel=$_installed_kernel"When parsing the --kernel option in reset_crashkernel it calls _valid_grubby_kernel_path to verify the given path. But that function relies on grubby being installed which needn't be the case when _is_osbuild is true.
Thanks for raising up this concern!
Since kexec-tools recommends grubby, grubby has been there when the kernel hook 92-crashkernel.install is being called.
true, I missed that. I think I also mixed up osbuild and ostree...
Btw, I was wrong that kernel recommends kexec-tools but somehow kernel is installed after kexec-tools for osbuild [1]. The order is also the same when I run, dnf install kernel-core kexec-tools --installroot=/ROOTDIR
I'm yet to find out why it's so.
I imagine that this could also be some special handling for the kernel in dnf. Usually you want to have all programs that are included into the initramfs to be installed before the kernel. Otherwise the initramfs could still contain the old, buggy version of a program even with the new, fixed version being installed. But keeping track of all such dependencies sounds pretty difficult. Always installing the kernel last would be a simple solution for that problem.
Thanks Philipp
[1] https://bugzilla.redhat.com/show_bug.cgi?id=2024976#c5
Or am I missing something?
Thanks Philipp
fi
On Thu, Jan 20, 2022 at 04:25:19PM +0100, Philipp Rudo wrote:
On Thu, 20 Jan 2022 14:43:08 +0800
[..]
Btw, I was wrong that kernel recommends kexec-tools but somehow kernel is installed after kexec-tools for osbuild [1]. The order is also the same when I run, dnf install kernel-core kexec-tools --installroot=/ROOTDIR
I'm yet to find out why it's so.
I imagine that this could also be some special handling for the kernel in dnf. Usually you want to have all programs that are included into the initramfs to be installed before the kernel. Otherwise the initramfs could still contain the old, buggy version of a program even with the new, fixed version being installed. But keeping track of all such dependencies sounds pretty difficult. Always installing the kernel last would be a simple solution for that problem.
kernel is indeed a special case for dnf and I can't help laughing out loud when happening to read this commit message of the dnf repo,
commit 4e625f935c725893b2db7b5719febd7c4d42b44e Author: Seth Vidal skvidal@linux.duke.edu Date: Sun Jun 9 21:03:21 2002 +0000
ridiculous special case for the kernel. I think I'm going to cry
But it turns out the installation order doesn't matter for our osbuild case because posttrans scriptlet by definition means it would be executed as the last step of a transaction i.e. it would be executed afer all packages have been installed. So after both kexec-tools and kernel* packages are installed, The posttrans scriptlet of the kernel-core package would invoke kernel-install which in turn would run the kernel hooks in /usr/lib/kernel/install.d/ including 92-crashkernel.install,
$ rpm -q --scripts -p kernel-core-5.16.2-200.fc35.x86_64.rpm ... posttrans scriptlet (using /bin/sh): /bin/kernel-install add 5.16.2-200.fc35.x86_64 /lib/modules/5.16.2-200.fc35.x86_64/vmlinuz || exit $?
Btw, although kernel is installed after kexec-tooks, kernel-core is actually installed before kexec-tools,
Installing : kernel-core-5.14.0-39.el9.x86_64 151/163 Running scriptlet: kernel-core-5.14.0-39.el9.x86_64 151/163 ... Running scriptlet: kexec-tools-2.0.23-2.fc36.x86_64 162/163 Installing : kexec-tools-2.0.23-2.fc36.x86_64 162/163 Running scriptlet: kexec-tools-2.0.23-2.fc36.x86_64 162/163 Created symlink /etc/systemd/system/multi-user.target.want
Installing : kernel-5.14.0-39.el9.x86_64 163/163 ... Running scriptlet: kernel-core-5.14.0-39.el9.x86_64 163/163
Fortunately, kernel-core's posttrans scriptlet is guaranteed to be run after all packages have been installed.
[2] https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#orderi...
Thanks Philipp
-
Coiby Xu -
Philipp Rudo -
Pingfan Liu