On 04/13/15 at 04:29pm, Baoquan He wrote:
On 04/10/15 at 09:31pm, Minfei Huang wrote:
> Any block device can be mounted multiply on the different mount point.
> Once a mount point is mounted in bind mode, the general mount point can
> be unmounted. Thus kdump would not find the general mount point to
> handle the path.
>
> The mount point, which is as general mount point, will be got by
> "fintmnt" previously. But the mntpoint may be incorrect, if the mntpoint
> is bind mount.
>
> In order to fix it to support bind mounted in atomic, we will add the
> judgement to comfirm the mntpoint is not bind mount.
you mean add a judgement to check whether the mntpoint is
bind mount? I didn't get you.
Yes.
In order to fix it to support bind mounted in atomic, we will add a
judgement to comfirm the mntpoint is bind mounted, or not.
>
> For general mount, returning path is like following, if we use
> "findmnt". The returning is same as "findmnt -v".
>
> -bash-4.2# findmnt /var | tail -n 1 | awk '{print $2}'
> /dev/mapper/atomicos-root
>
> But for bind mount, returning path is like following, if we use
> "fintmnt".
> -bash-4.2# findmnt /var | tail -n 1 | awk '{print $2}'
> /dev/mapper/atomicos-root[/ostree/deploy/rhel-atomic-host/var]
>
> Use "findmnt -v" is like this:
> -bash-4.2# findmnt -v /var | tail -n 1 | awk '{print $2}'
> /dev/mapper/atomicos-root
>
> So we can determine the bind mount, if the returning is different
> between "findmnt" and "findmnt -v".
>
> Signed-off-by: Minfei Huang <mhuang(a)redhat.com>
> ---
> kdump-lib.sh | 25 ++++++++++++++++++++++++-
> mkdumprd | 8 ++------
> 2 files changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/kdump-lib.sh b/kdump-lib.sh
> index f0a61f9..ad58638 100755
> --- a/kdump-lib.sh
> +++ b/kdump-lib.sh
> @@ -133,9 +133,32 @@ get_fs_type_from_target()
> echo $(findmnt -k -f -n -r -o FSTYPE $1)
> }
>
> +# input: device path
> +# output: the general mount point
> +# find the general mount point, not the bind mounted point in atomic
> +# As general system, Use the previous code
> get_mntpoint_from_target()
> {
> - echo $(findmnt -k -f -n -r -o TARGET $1)
> + if is_atomic; then
> + for _mnt in $(findmnt -k -n -r -o TARGET $1)
> + do
> + if ! is_bind_mount $_mnt; then
> + echo $_mnt
> + return
> + fi
> + done
> +
> + # the device can be umounted the general mount point, if one of the mount
point is bind mounted
> + # For example:
> + # mount /dev/sda /mnt/
> + # mount -o bind /mnt/var /var
> + # umount /mnt
> + # So we can not get the general mount point according to above codeflow
I don't like a big block of comment inside function. Could it be put
above function name?
I am scared by a big function with many lines of code usually. Please
make code clean.
Otherwise I am fine with the code change in this patch.
Ok. I will add the comment at the head of function name.
Thanks
Minfei
Thanks
Baoquan
> + echo "Mount $1 firstly, without the bind mode" >&2
> + exit 1
> + else
> + echo $(findmnt -k -f -n -r -o TARGET $1)
> + fi
> }
>
> # get_option_value <option_name>
> diff --git a/mkdumprd b/mkdumprd
> index 7c572a7..14ec70f 100644
> --- a/mkdumprd
> +++ b/mkdumprd
> @@ -103,7 +103,7 @@ to_mount() {
> local _dev=$1 _source _target _fstype _options _mntopts _pdev
>
> _source=$(findmnt -k -f -n -r -o SOURCE $_dev)
> - _target=$(findmnt -k -f -n -r -o TARGET $_dev)
> + _target=$(get_mntpoint_from_target $_dev)
> # mount under /sysroot if dump to root disk or mount under
> #/kdumproot/$_target in other cases in 2nd kernel. systemd
> #will be in charge to umount it.
> @@ -147,10 +147,6 @@ to_mount() {
> echo "$_pdev $_mntopts"
> }
>
> -to_mount_point() {
> - echo $(findmnt -k -f -n -r -o TARGET $1)
> -}
> -
> is_readonly_mount() {
> local _mnt
> _mnt=$(findmnt -k -f -n -r -o OPTIONS $1)
> @@ -204,7 +200,7 @@ mkdir_save_path_ssh()
> #Function: get_fs_size
> #$1=dump target
> get_fs_size() {
> - local _mnt=$(to_mount_point $1)
> + local _mnt=$(get_mntpoint_from_target $1)
> echo -n $(df -P "${_mnt}/$SAVE_PATH"|tail -1|awk '{print
$4}')
> }
>
> --
> 1.9.3
>