On 12/05/14 at 10:19am, Vivek Goyal wrote:
On Fri, Dec 05, 2014 at 01:52:25PM +0800, WANG Chao wrote:
> "default" option only takes reboot/halt/poweroff/shell/dump_to_rootfs as
> its config value. Otherwise kdump service fails early.
>
> Signed-off-by: WANG Chao <chaowang(a)redhat.com>
> ---
> kdumpctl | 22 +++++++++++++++++++++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/kdumpctl b/kdumpctl
> index ee3214a..2bee385 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -226,6 +226,23 @@ backup_initrd()
> fi
> }
>
> +check_config_default_action()
> +{
> + local config_opt="default"
Has config_opt not been set in outer loop? Can we use the same instead of
defining another local variable with same name?
Apart from this minor nit, patch looks good.
I think I'll hard coded "default" in the error message and discard
"config_opt".
Thanks for review.
WANG Chao
Vivek
> + local config_val=$1
> +
> + case "$config_val" in
> + reboot|halt|poweroff|shell|dump_to_rootfs)
> + ;;
> + *)
> + echo "Invalid kdump config value for option $config_opt."
> + return 1
> + ;;
> + esac
> +
> + return 0
> +}
> +
> check_config()
> {
> local nr
> @@ -242,7 +259,7 @@ 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|force_rebuild|dracut_args|fence_kdump_args|fence_kdump_nodes)
> [ -z "$config_val" ] && {
> echo "Invalid kdump config value for option $config_opt."
> return 1;
> @@ -252,6 +269,9 @@ check_config()
> echo "Deprecated kdump config option: $config_opt. Refer to kdump.conf
manpage for alternatives."
> return 1
> ;;
> + default)
> + check_config_default_action "$config_val" || return 1
> + ;;
> *)
> echo "Invalid kdump config option $config_opt"
> return 1;
> --
> 1.9.3