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

[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 <bhe at 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 at lists.fedoraproject.org
> https://lists.fedoraproject.org/mailman/listinfo/kexec