On 04/20/15 at 04:17pm, Dave Young wrote:
On 04/20/15 at 12:00pm, Minfei Huang wrote:
> On 04/20/15 at 10:23am, Dave Young wrote:
> > On 04/20/15 at 09:43am, Dave Young wrote:
> > > On 04/17/15 at 04:26pm, Minfei Huang wrote:
> > > > kdump will raise the warning in Atomic, if the path is bind mounted
> > > > directory. The reason why causes this issue is kdump cannt parse the
> > > > bind mounted directory.
> > > >
> > > > To correct dumping target, we can construct the real dumping path in
> > > > Atomic, which contains two part, one bind mounted path, the other
> > > > specified dump target.
> > > >
> > > > Following is an example:
> > > > -bash-4.2# cat /etc/kdump.conf | grep ^path
> > > > path /var/crash
> > > >
> > > > -bash-4.2# findmnt /var | tail -n 1 | awk '{print $2}'
> > > > /dev/mapper/atomicos-root[/ostree/deploy/rhel-atomic-host/var]
> > > >
> > > > -bash-4.2# findmnt -v /var | tail -n 1 | awk '{print
$2}'
> > > > /dev/mapper/atomicos-root
> > > >
> > > > Then we can found it that the real path of dumping vmcore is
> > > > /ostree/deploy/rhel-atomic-host/var/crash.
> > > >
> > > > To fix this issue, we can replace the target path as the real path
which
> > > > is from above parsing.
> > > >
> > > > Signed-off-by: Minfei Huang <mhuang(a)redhat.com>
> > > > ---
> > > > kdump-lib.sh | 5 ++++-
> > > > mkdumprd | 22 +++++++++++++++++++++-
> > > > 2 files changed, 25 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/kdump-lib.sh b/kdump-lib.sh
> > > > index 5333fe4..2abb513 100755
> > > > --- a/kdump-lib.sh
> > > > +++ b/kdump-lib.sh
> > > > @@ -182,7 +182,10 @@ make_absolute_save_path()
> > > > local _mnt
> > > >
> > > > [ -n $_target ] && _mnt=$(get_mntpoint_from_target $1)
> > > > - echo "${_mnt}/$SAVE_PATH"
> > > > + _mnt="${_mnt}/$SAVE_PATH"
> > > > +
> > > > + # strip the duplicated "/"
> > > > + echo "$_mnt" | tr -s /
> > > > }
> > > >
> > > > check_save_path_fs()
> > > > diff --git a/mkdumprd b/mkdumprd
> > > > index 14ec70f..8cf3c7b 100644
> > > > --- a/mkdumprd
> > > > +++ b/mkdumprd
> > > > @@ -362,6 +362,17 @@ handle_default_dump_target()
> > > >
> > > > _mntpoint=$(get_mntpoint_from_path $SAVE_PATH)
> > > > _target=$(get_target_from_path $SAVE_PATH)
> > > > +
> > > > + if is_atomic && is_bind_mount $_mntpoint; then
> > > > + SAVE_PATH=${SAVE_PATH##"$_mntpoint"}
> > > > + # the real dump path in the 2nd kernel, if the mount point
is bind mounted.
> > > > + SAVE_PATH=$(get_bind_mount_directory $_mntpoint)/$SAVE_PATH
> > > > + _mntpoint=$(get_mntpoint_from_target $_target)
> > > > +
> > > > + # the absolute path in the 1st kernel
> > > > + SAVE_PATH=$_mntpoint/$SAVE_PATH
> > > > + fi
> > > > +
> > > > if [ "$_mntpoint" != "/" ]; then
> > > > SAVE_PATH=${SAVE_PATH##"$_mntpoint"}
> > > > _fstype=$(get_fs_type_from_target $_target)
> > > > @@ -533,8 +544,17 @@ do
> > > > if [ "$config_opt" = "nfs" ]; then
> > > > add_dracut_module "nfs"
> > > > fi
> > > > +
> > > > + _absolute_save_path=$(make_absolute_save_path $config_val)
> > >
> > > What the use of below section? They are redundant?
> >
> > Talked with Baoquan, SAVE_PATH is used in check_size function, it is not easy
> > to find. Moving functions to lib but still using global variables is bad to me
> >
>
> SAVE_PATH is used in mkdumprd only.
>
> Yes, I will do a cleanup after these patch set megered, and move the
> function make_absolute_save_path from kdump-lib.sh to mkdumprd.
>
No, it is get_size function which is using global $SAVE_PATH. What I thouht is
do not use global variable in lib function, instead we should pass it as arguments.
Sure.
Since the deadline is coming, I prefer to do it later. I will do a
cleanup to remove the global variable.
Thanks
Minfei
Thanks
Dave