Re: [PATCH v3 1/4] Support special mount information via "dracut_args"

[Pratyush Anand]

Hi Xunlei,

On 25/08/2016:06:23:25 PM, Xunlei Pang wrote:
> There are some complaints about nfs kdump that users must mount
> nfs beforehand, which may cause some overhead to nfs server.
> For example, there're thounsands of diskless clients deployed with
> nfs dumping, each time the client is boot up, it will trigger
> kdump rebuilding so will mount nfs, thus resulting in thousands
> of nfs request concurrently imposed on the same nfs server.
>
> We introduce a new way of specifying mount information via the
> already-existent "dracut_args" directive(so avoid adding extra
> directives in /etc/kdump.conf), we will skip all the filesystem
> mounting and checking stuff for it. So it can be used in the
> above-mentioned nfs scenario to avoid severe nfs server overhead.
>
> Specifically, if there is any "--mount" information specified via
> "dracut_args" in /etc/kdump.conf, always use it as the final mount
> without any validation(mounting or checking like mount options,
> fs size, etc), so users are expected to ensure its correctness.
>
> NOTE:
> -Only one mount target is allowed using "dracut_args" globally.
> -Dracut will create <mountpoint> if it doesn't exist in kdump kernel,
>  <mountpoint> must be specified as an absolute path.
> -Users should do a test first and ensure it works because kdump does
>  not prepare the mount or check all the validity.
>
> Suggested-by: Dave Young 
> Signed-off-by: Xunlei Pang
>
> dracut-kdump.sh        |  4 ++++
>  dracut-module-setup.sh |  5 +++++
>  kdump-lib.sh           | 27 +++++++++++++++++++++++++--
>  kdumpctl               | 13 ++++++++++++-
>  4 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/dracut-kdump.sh b/dracut-kdump.sh
> index 4aab205..42ba37f 100755
> --- a/dracut-kdump.sh
> +++ b/dracut-kdump.sh
> @@ -146,6 +146,10 @@ read_kdump_conf()
>          # remove inline comments after the end of a directive.
>          config_val=$(strip_comments $config_val)
>          case "$config_opt" in
>    dracut_args)
>        config_val=$(get_dracut_args_target "$config_val")
>        [[ -n "$config_val" ]] && add_dump_code "dump_fs $config_val"
>        ;;
>    ext[234]|xfs|btrfs|minix|nfs)
>        add_dump_code "dump_fs $config_val"
>        ;;
>
> diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
> index f5c0218..68e0ff8 100755
> --- a/dracut-module-setup.sh
> +++ b/dracut-module-setup.sh
> @@ -450,6 +450,11 @@ kdump_install_conf() {
>          ssh|nfs)
>              kdump_install_net "$config_val"
>              ;;
>    dracut_args)
>        if [[ $(get_dracut_args_fstype "$config_val") = nfs* ]] ; then
>            kdump_install_net "$(get_dracut_args_target "$config_val")"
>        fi
>        ;;
>    kdump_pre|kdump_post|extra_bins)
>        dracut_install $config_val
>        ;;
>
> diff --git a/kdump-lib.sh b/kdump-lib.sh
> index 4567a05..da072d9 100755
> --- a/kdump-lib.sh
> +++ b/kdump-lib.sh
> @@ -23,7 +23,9 @@ is_ssh_dump_target()
>
> is_nfs_dump_target()
>  {
> grep -q "^nfs" /etc/kdump.conf
> grep -q "^nfs" /etc/kdump.conf || \
> [[ $(grep "^dracut_args .*--mount" /etc/kdump.conf | \
>    sed "s/.*--mount *.\(.*\)/\1/" | cut -d' ' -f3) = nfs* ]]
>
> }
>
> is_raw_dump_target()
> @@ -45,7 +47,8 @@ is_fs_dump_target()
>
> is_user_configured_dump_target()
>  {
> return $(is_ssh_dump_target || is_nfs_dump_target || is_raw_dump_target || is_fs_dump_target)
> return $(is_mount_in_dracut_args || is_ssh_dump_target || is_nfs_dump_target || \
>         is_raw_dump_target || is_fs_dump_target)
>
> }

I see, that in the cover  letter you have mentioned "including other supported
filesystems like ext[2-4]/xfs, etc". If we intend to support other file system
then, we will need to improve is_raw_dump_target(), is_fs_dump_target() etc as
well. Probably, we can skip complexity of other filesystem support in current
patch set, and can just rename is_mount_in_dracut_args as
is_nfs_mount_in_dracut_args.
> strip_comments()
> @@ -406,3 +409,23 @@ is_wdt_mod_omitted() {
>
> return $ret
>  }
>
> +# If "dracut_args" contains "--mount" information, use it
> +# directly without any check(users are expected to ensure
> +# its correctness).
> +is_mount_in_dracut_args()
> +{
> return $(grep ^dracut_args /etc/kdump.conf | grep -q "--mount")

may be
return $(grep -q "^dracut_args .*--mount" /etc/kdump.conf)
> +}
>
> +# $1: configuration value of "dracut_args".
> +get_dracut_args_fstype()
> +{
> echo $1 | grep "--mount" | sed "s/.*--mount .(.*)/\1/" | cut -d' ' -f3
>
> +}
>
> +# $1: configuration value of "dracut_args".
> +get_dracut_args_target()
> +{
> echo $1 | grep "--mount" | sed "s/.*--mount .(.*)/\1/" | cut -d' ' -f1
>
> +}
> diff --git a/kdumpctl b/kdumpctl
> index 0436bfc..4d83e69 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -236,12 +236,18 @@ check_config()
>  {
>    local nr
> nr=$(awk 'BEGIN{cnt=0} /^raw|^ssh[[:blank:]]|^nfs|^ext[234]|^xfs|^btrfs|^minix/{cnt++} END{print cnt}' $KDUMP_CONFIG_FILE)
>
> nr=$(awk 'BEGIN{cnt=0} /^raw|^ssh[[:blank:]]|^nfs|^ext[234]|^xfs|^btrfs|^minix|^dracut_args .*--mount/{cnt++} END{print cnt}' $KDUMP_CONFIG_FILE)
> [ $nr -gt 1 ] && {
> echo "More than one dump targets specified."
> return 1
> }
>
> nr=$(grep "^dracut_args .*--mount" $KDUMP_CONFIG_FILE | grep -o "--mount" | wc -l)
>
> [ $nr -gt 1 ] && {
> echo "More than one mount targets specified in \"dracut_args\"."
> return 1
>
> }
>
> while read config_opt config_val; do
>   # remove inline comments after the end of a directive.
>   config_val=$(strip_comments $config_val)
>
> @@ -365,6 +371,11 @@ check_dump_fs_modified()
>    local _new_dev _new_mntpoint _new_fstype
>    local _target _path _dracut_args
> # No need to check in case of mount target specified via "dracut_args".
> if is_mount_in_dracut_args; then
> return 0
> fi
> # No need to check in case of raw target.
>  # Currently we do not check also if ssh/nfs target is specified
>  if is_ssh_dump_target || is_nfs_dump_target || is_raw_dump_target; then
>
> --

~Pratyush