On 03/12/14 at 10:25am, Vivek Goyal wrote:
On Wed, Mar 12, 2014 at 05:59:27PM +0800, Baoquan He wrote:
> When user does not specify dump target explicitly, it's better to
> dump to the "path" specified. That means after dump user enter into
> 1st kernel, can find vmcore in the "path". If that path is in root
> fs, vmcore is stored in root fs. If separate disk is mounted on
> any tier of "path", we just dump vmcore into the left path on the
> left separate disk.
>
> E.g in kdump.conf
> path /mnt/nfs
>
> in mount info,
> /dev/vdb on /mnt type ext4 (rw,relatime,seclabel,data=ordered)
>
> Then vmcore will be saved in /nfs of /dev/vdb.
Hi Bao,
What happens if /mnt/nfs is an NFS mount? I think logically if NFS volume
is mounted on /mnt/xyz, we should be able to take care of that too? This
would be similar to the case where root is NFS mounted and we still can
dump to /var/crash directory over NFS.
Yeah, NFS need be considered. Will include it in v2.
>
> In this patch, pass mount info to dracut in this case if separate
> disk is mounted on any tier of "path".
>
> Signed-off-by: Baoquan He <bhe(a)redhat.com>
> ---
> mkdumprd | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/mkdumprd b/mkdumprd
> index 241a426..d792b42 100644
> --- a/mkdumprd
> +++ b/mkdumprd
> @@ -355,12 +355,12 @@ get_block_dump_target()
> [ -b "$_target" ] && echo $(to_dev_name $_target)
> }
>
> -# If no dump disk is specified make sure /var/crash is not mounted on a
> -# separate disk.
> -check_block_dump_target()
> +#handle the case user does not specify the dump target explicitly
> +handle_default_dump_target()
> {
> local _target
> local _mntpoint
> + local _ret
>
> _target=$(get_user_configured_dump_disk)
> [ -n "$_target" ] && return
> @@ -368,11 +368,16 @@ check_block_dump_target()
> _target=$(get_root_fs_device)
> if [ -b "$_target" ]; then
Why do we need to make sure that rootfs deivce is a block device?
If NFS cared, it should look like:
if [ -n "$_target" ]; then
will change.
> mkdir -p $SAVE_PATH
> - _mntpoint=`df $SAVE_PATH | tail -1 | awk '{print $NF}'`
> + _ret=$?
> + if [ $_ret -ne 0 ]; then
> + perror_exit "mkdir failed on $SAVE_PATH"
> + fi
I think we should resuse mkdir_save_path_fs() here. So first we need
to set dump disk and path relative to disk and then call
mkdir_save_path_fs(). This will also take care of handling read only
mounted disks.
I can't. Because "df" need be executed on existing path. E.g if /dev/vdb
is mounted on /mnt, and the "path" is /mnt/nfs. Then "mkdir -p
/mnt/nfs"
need be executed firstly, otherwise "df /mnt/nfs" will fail if /mnt/nfs
does not exist.
> + _mntpoint=$(df $SAVE_PATH | tail -1 | awk '{print $NF}')
> if [ "$_mntpoint" != "/" ]; then
> - perror "No dump target specified. Default dump target is rootfs
block device."
> - perror "But dump path $SAVE_PATH is not backed by rootfs block
device. "
> - perror_exit "Either explicitly specify a dump target or specify a
dump path backed by rootfs block device"
> + SAVE_PATH=$(echo $SAVE_PATH | sed "s,$_mntpoint,,")
> + _target=$(findmnt -k -f -n -o SOURCE $_mntpoint)
I guess when you did "df" previously on $SAVE_PATH, that will also give
you mounted disk. So we can save _target there itself.
_mntpoint=$(df $SAVE_PATH | tail -1 | awk '{print $NF}')
_target=$(df $SAVE_PATH | tail -1 | awk '{print $1}')
Yeah, this can be improved. Will do.
> + add_mount "$_target"
> + check_size fs $_target
> fi
> fi
> }
> @@ -500,8 +505,6 @@ check_crypt()
> return 1
> }
>
> -check_block_dump_target
> -
> if ! check_resettable; then
> exit 1
> fi
> @@ -521,6 +524,8 @@ if [ "$(uname -m)" = "s390x" ]; then
> add_dracut_module "znet"
> fi
>
> +handle_default_dump_target
> +
I think any handling of default dump target should happpen after we have
parsed the file. We seem to be doing some things in wrong order.
Performing various checks and actions even before we parse the file.
So we should first parse the file, save relevant state and then see if
user has specified a dump target or not. And if user has not specified
a dump target, then set the dump target, path and add_mount() accordingly.
In logic, this should be better. Will change.
Baoquan
Thanks
Thanks
Vivek