[Patch v2] kdump fails loading if target is root fs by default while disk is mounted on save path

[Baoquan He]

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 
---
 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
+
+    #get rootfs device name
+    _target=$(findmnt -k -f -n -o SOURCE /)
+    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
+    fi
+        
+}
+
+check_block_dump_target
+
 if ! check_resettable; then
     exit 1
 fi

[Vivek Goyal]

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
>
> 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.
> #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.
> 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.
> 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
....

Thanks
Vivek

[Vivek Goyal]

On Fri, Feb 28, 2014 at 02:55:07PM -0500, Vivek Goyal wrote:

[..]
> "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"

Or we could say same thing with smaller message.

"No dump target specified. Default dump target is rootfs block device. But $SAVE_PATH is not backed by rootfs block device. Edit /etc/kdump.conf and either specify a dump target or change path so that it is backed by rootfs block device"

Thanks
Vivek

[Vivek Goyal]

On Fri, Feb 28, 2014 at 03:01:29PM -0500, Vivek Goyal wrote:
> On Fri, Feb 28, 2014 at 02:55:07PM -0500, Vivek Goyal wrote:
>
> [..]
> > "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"
>
> Or we could say same thing with smaller message.
>
> "No dump target specified. Default dump target is rootfs block device. But $SAVE_PATH is not backed by rootfs block device. Edit /etc/kdump.conf and either specify a dump target or change path so that it is backed by rootfs block device"

Edit /etc/kdump.conf is just one way of changing dump destination. In
theory one can use s-c-kdump to tweak dump destination/path. So how about
following.

"No dump target specified. Default dump target is rootfs block device. But $SAVE_PATH is not backed by rootfs block device. Either specify a dump target or speicy a dump path backed by rootfs block device"

Thanks
Vivek

[Vivek Goyal]

On Fri, Feb 28, 2014 at 03:06:36PM -0500, Vivek Goyal wrote:

[..]
> "No dump target specified. Default dump target is rootfs block device. But $SAVE_PATH is not backed by rootfs block device. Either specify a dump target or speicy a dump path backed by rootfs block device"

Ok, refining the message a little bit more.

"No dump target specified. Default dump target is rootfs block device. But dump path $SAVE_PATH is not backed by rootfs block device. Either explicitly specify a dump target or specify a dump path backed by rootfs block device"

Thanks
Vivek

[Baoquan He]

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
> >
> > 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
> [e-mail]
> https://lists.fedoraproject.org/mailman/listinfo/kexec