On 04/16/15 at 05:24pm, Baoquan He wrote:
On 04/10/15 at 09:31pm, Minfei Huang wrote:
> diff --git a/kdump-lib.sh b/kdump-lib.sh
> index ad58638..fbd2970 100755
> --- a/kdump-lib.sh
> +++ b/kdump-lib.sh
> @@ -181,7 +181,10 @@ make_absolute_save_path()
> local _mnt
>
> [ -n $_target ] && _mnt=$(get_mntpoint_from_target $1)
> - echo "${_mnt}/$SAVE_PATH"
> + _mnt="${_mnt}/$SAVE_PATH"
> +
> + # strip the duplicated "/"
> + echo "$_mnt" | tr -s /
> }
>
> check_save_path_fs()
> diff --git a/mkdumprd b/mkdumprd
> index 14ec70f..fcfc03c 100644
> --- a/mkdumprd
> +++ b/mkdumprd
> @@ -362,6 +362,19 @@ handle_default_dump_target()
>
> _mntpoint=$(get_mntpoint_from_path $SAVE_PATH)
> _target=$(get_target_from_path $SAVE_PATH)
> +
> + # if the system is atomic and mntpoint is bind mount, then we will get the real
path
> + # otherwise do nothing
Comment is necessary when you think code is very hard to understand
without it. Here the code can tell what it's doing obviously, so can you
remove above comment? I think it's unnecessary.
will remove it.
> + if is_atomic && is_bind_mount $_mntpoint; then
> + SAVE_PATH=${SAVE_PATH##"$_mntpoint"}
> + # the real dump path in the 2nd kernel, if the mount point is bind
mounted.
Here I am fine with it since it may note code reader.
> + SAVE_PATH=$(get_bind_mount_directory $_mntpoint)/$SAVE_PATH
> + _mntpoint=$(get_mntpoint_from_target $_target)
> +
> + # the absolute path in the 1st kernel
> + SAVE_PATH=$_mntpoint/$SAVE_PATH
> + fi
> +
> if [ "$_mntpoint" != "/" ]; then
> SAVE_PATH=${SAVE_PATH##"$_mntpoint"}
> _fstype=$(get_fs_type_from_target $_target)
> @@ -533,8 +546,21 @@ do
> if [ "$config_opt" = "nfs" ]; then
> add_dracut_module "nfs"
> fi
> +
> + _absolute_save_path=$(make_absolute_save_path $config_val)
> + # if the system is atomic and mntpoint is bind mount, then we will get the
real path
Here redundent comment too.
will remove it.
> + # otherwise do nothing
> + if is_atomic; then
> + _mntpoint=$(get_mntpoint_from_path $_absolute_save_path)
Could you make this block be consistent with above condition checking?
Seems it do the similar thing. If you can abstract them and wrap into a
function, that will make code simpler. But I don't insist on this.
Ok, I will move it at the head of the function.
Thanks
Minfei
> + if is_bind_mount $_mntpoint; then
> + SAVE_PATH=${_absolute_save_path##"$_mntpoint"}
> + # the real dump path in the 2nd kernel, if the mount point is bind
mounted.
> + SAVE_PATH=$(get_bind_mount_directory $_mntpoint)/$SAVE_PATH
> + fi
> + fi
> +
> add_mount "$config_val"
> - check_save_path_fs $(make_absolute_save_path $config_val)
> + check_save_path_fs $_absolute_save_path
> check_size fs $config_val
> ;;
> raw)
> --
> 1.9.3
>