On 25/08/2016:11:06:30 PM, Xunlei Pang wrote:
On 2016/08/25 at 22:15, Pratyush Anand wrote:
> 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 <dyoung(a)redhat.com>
>> Signed-off-by: Xunlei Pang <xlpang(a)redhat.com>
>> ---
>> 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.
is_raw_dump_target() can't be specified by "dracut_args", so it is fine.
As for "is_fs_dump_target", its only callsite is
"is_user_configured_dump_target()",
where I added "is_mount_in_dracut_args", so I think it is fine too.
Yes, you are right! It is safe as long as is_fs_dump_target() is not called from
any other place.
Reviewed-by: Pratyush Anand <panand(a)redhat.com>
>
> >>
> >> 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)
>
> Yes, this is better, thanks for the review.
>
> Regards,
> Xunlei
>
> >> +}
> >> +
> >> +# $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