Hi Dave,
On Thu, Apr 6, 2017 at 11:40 AM, Dave Young <dyoung(a)redhat.com> wrote:
Hi Bhupesh,
On 04/06/17 at 01:55pm, Dave Young wrote:
> On 04/05/17 at 12:44am, Bhupesh Sharma wrote:
> > This patch introduces the 'force_no_rebuild' option
> > inside the 'kdump.conf' and its handling inside the 'kdumpctl'
> > script.
> >
> > There might be several use cases, where a system admin
> > decides that he doesn't need to rebuild the kdump initrd
> > and wants to use an existing version of the same. In such cases,
> > he can set the 'force_no_rebuild' option inside 'kdump.conf'
> > to 1, to force the 'kdumpctl' script not to rebuild the kdump
> > initrd.
> >
> > This change also handles existing issues with kdump initrd
> > rebuild, when the /boot (and/or /root) are set as read-only
> > by a system admin. In such cases, this patch proposes the
> > following approach to decide as to whether the kdump initrd needs to be
> > rebuild:
> >
> > -------------------------------------------------------------------------
> > | Options |Val| /boot is RO | /boot is WRITABLE |
> > | | | | |
> > -------------------------------------------------------------------------
> > | force_rebuild | 0 |WARN that kdump initrd |kdumpctl decides if|
> > | | |will not be rebuilt |kdump initrd |
> > | | |but proceed with |rebuild in required|
> > | | |loading the kdump | |
> > | | |kernel | |
> > -------------------------------------------------------------------------
> > | force_rebuild | 1 |Bail out with |Proceed with kdump |
> > | | |an ERROR |initrd rebuild |
> > -------------------------------------------------------------------------
> > | force_no_rebuild | 0 |Check force_rebuild value and act |
> > | | |accordingly |
> > -------------------------------------------------------------------------
> > | force_no_rebuild | 1 |WARN that kdump initrd |Do not rebuild |
> > | | |will not be rebuilt |kdump initrd |
> > | | |but proceed with | |
> > | | |loading the kdump | |
> > | | |kernel | |
> > -------------------------------------------------------------------------
>
> Although this option is introduced because of the readonly /boot issue,
> that can be mentioned in a few sentences, a new bool option
> force_no_rebuild is easy to understand, it seems not necessary to list all
> of those cases in such a table.
I still do not understand why we will care about /boot for this option,
how about do something like this?
Please refer to the comments I posted on the comments shared by
Pratyush and Xunlei - in v2 will remove most of the /boot handling.
I propose to only enhance the error message when /boot is RO and we
have neither force_no_rebuild nor force_rebuils set to 1, in which
case probably the user might find it useful to be informed that he
might find it useful to set orce_no_rebuild = 1
diff --git a/kdumpctl b/kdumpctl
index 4d68be0..7f03b9d 100755
--- a/kdumpctl
+++ b/kdumpctl
@@ -354,11 +354,13 @@ check_config()
case "$config_opt" in
\#* | "")
;;
-
raw|ext2|ext3|ext4|minix|btrfs|xfs|nfs|ssh|sshkey|path|core_collector|kdump_post|kdump_pre|extra_bins|extra_modules|default|force_rebuild|dracut_args|fence_kdump_args|fence_kdump_nodes)
+
raw|ext2|ext3|ext4|minix|btrfs|xfs|nfs|ssh|sshkey|path|core_collector|kdump_post|kdump_pre|extra_bins|extra_modules|default|force_rebuild|force_no_rebuild|dracut_args|fence_kdump_args|fence_kdump_nodes)
[ -z "$config_val" ] && {
echo "Invalid kdump config value for option
$config_opt."
return 1;
}
+ ## if both force_rebuild and force_no_rebuild equal to 1 then
print error and return 1
+ ...
;;
net|options|link_delay|disk_timeout|debug_mem_level|blacklist)
echo "Deprecated kdump config option: $config_opt. Refer to
kdump.conf manpage for alternatives."
@@ -611,6 +613,8 @@ check_rebuild()
local _force_rebuild force_rebuild="0"
local ret system_modified="0"
+ #if force_no_rebuild equal to 1 then do nothing and return 0
+ ...
check_boot_dir
if [ -z "$KDUMP_KERNELVER" ]; then
Looks similar to what I have in v2 (which I am about to send out),
with a minor change being the additional information to the
user/system admin when the /boot is RO and neither force_no_rebuild
and force_rebuild are set.
Regards,
Bhupesh