On 03/17/14 at 11:16am, Vivek Goyal wrote:
On Mon, Mar 17, 2014 at 03:49:19PM +0800, Baoquan He wrote:
[..]
> +default_dump_target_install_conf()
> +{
> + local _target _fstype
> + local _s _t
> + local _mntpoint
> + local _path _save_path
> +
> + is_user_configured_dump_target && return
> +
> + _save_path=$(grep ^path "/etc/kdump.conf"| cut -d' ' -f2)
> + [ -z "$_save_path" ] && _save_path="/var/crash"
Let us define default path in kdump-lib.sh and use that throughout the
kdump code (instead of hardcoding to /var/crash".
DEFAULT_PATH="/var/crash/"
Good idea, will do.
> +
> + _mntpoint=$(df $SAVE_PATH | tail -1 | awk '{print $NF}')
How about defining a function say get_mntpoint_from_path() in
kdump-lib.sh and use both here in mkdumprd.
will do.
> + _target=$(df $SAVE_PATH | tail -1 | awk '{print $1}')
How about defining a function say get_target_from_path() to find target from
path in kdump-lib.sh and use that in mkdurmprd too.
We are essentially repeating all the things we did in mkdumprd and I want
to share as much code as possible between the two.
ditto.
Here the reason why I don't abstract them into functions is they are
mostly only one line of code, and have been very simple and direct.
Surely we can wrap them like C functions do, maybe for shell script they
are very clear.
Anyway, I will wrap them into functions in kdump-lib.sh, and call them
here.
> + if [ "$_mntpoint" != "/" ]; then
> + _fstype=$(findmnt -k -f -n -r -o FSTYPE $_target)
how about get_fs_type_from_target() in kdump-lib.sh and reuse in mkdumprd.
ditto.
> +
> + if [ "$_fstype" = "nfs" ];then
> + kdump_install_net "$_target"
> + else
> + _target=$(kdump_to_udev_name $_target)
> + fi
Ok, we seem to have this functionality to convert to persistent names
both in mkdumprd as well as dracut-module-sh. How about coming up with
a common function in kudmp-lib.sh and use that at both the places. This
is a cleanup and can go in a separate patch.
> +
> + echo "$_fstype $_target" >> /tmp/$$-kdump.conf
> +
> + _path=${_save_path##"$_mntpoint"}
> + sed -i -e "s#$_save_path#$_path#" /tmp/$$-kdump.conf
> + fi
> +
> +}
> +
Thanks
Vivek