On 02/28/14 at 02:55pm, Vivek Goyal wrote:
On Fri, Feb 28, 2014 at 06:44:03PM +0800, Baoquan He wrote:
> kdump now dumps vmcore to root partition by default in SAVE_PATCH
> directory, e.g /var/crash defaultly. This is problematic when another
> disk is mounted on /var or /var/crash, because the saved vmcore will
> he hidden after dump in 1st kernel. This also has the potential of
> blindly filling the root file system without a clue as to why.
>
> Now fix this by failing the loading of kdump kernel if dump target
> is root fs by default while different disk is mounted on save path.
>
> Signed-off-by: Baoquan He <bhe(a)redhat.com>
> ---
> mkdumprd | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/mkdumprd b/mkdumprd
> index bc002bc..d30b34c 100644
> --- a/mkdumprd
> +++ b/mkdumprd
> @@ -453,6 +453,34 @@ check_resettable()
> return 1
> }
>
> +check_block_dump_target()
> +{
> + local _target
> + local _mntpoint
> +
> + if is_ssh_dump_target || is_nfs_dump_target; then
> + return
> + fi
> +
> + _target=$(egrep "^ext[234]|^xfs|^btrfs|^minix|^raw" /etc/kdump.conf
2>/dev/null |awk '{print $2}')
> + [ -n "$_target" ] && return
> +
We are sharing lot of code with get_block_dump_target(). Instead of
copying code again how about writing smaller functions (in kdump-lib)
and use these at both the places. For example.
We could write a function says get_user_configured_dump_disk(). This will
return a dump target if user configured any of the file systems or raw
targets.
And above code could look lik.e
if get_user_configured_dump_disk;then
return
fi
And get_block_dump_target() could use this function too.
Yeah, this make sense to me, will do.
> + #get rootfs device name
> + _target=$(findmnt -k -f -n -o SOURCE /)
Similarly we could write get_root_fs_device() function and use it at both
the places. This will just return us the device used by root file system.
It's only one line of code, but I am fine with it, maybe later people
don't need reimplement it.
> + if [ -b "$_target" ]; then
> + mkdir -p $SAVE_PATH
> + _mntpoint=`df $SAVE_PATH | tail -1 | awk '{print $NF}'`
> + _target=`df $SAVE_PATH | tail -1 | awk '{print $1}'`
> + if [ "$_mntpoint" != "/" ]; then
> + perror "Currently SAVE_PATH is $SAVE_PATH, $_target is mounted on
$_mntpoint. While dump target is root filesystem by default."
> + perror_exit "Then vmcore will be hidden in 1st kernel after dump.
Please check /etc/kdump.conf clearly to remove confusion!"
> + fi
Oh, this message is going to be tricky. After spending lot of time, I
could think of following. See if it is concise and clear enough or not.
"No dump target specified. By default dump will be saved to rootfs block device in
location $SAVE_PATH. But $SAVE_PATH is not backed rootfs block device. Edit
/etc/kdump.conf and either specify a dump target or change to a path which
is backed by rootfs block device"
We also need to write a small section about this issue in
kexec-kdump-howto.txt file and explain what's the problem and what to do.
Will change based on your last version. And do you think we need tell
user what is exactly going on? E.g a disk is mounted on the SAVE_PATH,
just make them the essential detail.
> + fi
> +
> +}
> +
> +check_block_dump_target
> +
> if ! check_resettable; then
> exit 1
> fi
Current code organization looks bad. We seem to have intermixing function
definition and call to functions.
function1_definition
call function1
function2_definition
call function2
This is very confusing. One can't read what's the flow of script. Where
does it start. Can you please fix it. It should look like.
function1_definition
function2_definition
..
..
#Main Script
call function1
call function2
Yeah, will change.
Baoquan
Thanks
....
Thanks
Vivek
_______________________________________________
kexec mailing list
kexec(a)lists.fedoraproject.org
https://lists.fedoraproject.org/mailman/listinfo/kexec