[PATCH 1/2] pass mount info to dracut when default target is a separate disk

[Baoquan He]

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 at 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