On Thu, Apr 30, 2020 at 11:20 AM piliu <piliu(a)redhat.com> wrote:
On 04/26/2020 06:29 PM, Kairui Song wrote:
> This reverts commit 6dee286467e5a697d25148712a110da1a720ac96.
>
> There are reports that NFSv3 is failing after this commit, and after
> more debug, I found NFSv4 may not work properly if
> "nfs4_disable_idmapping" is set to 0.
>
> Besides, kdump.sh runs after dracut's pre-pivot and clean up hook, many
> dracut module will install hooks to kill some running services, so
> kdump.sh may have the risk of failing to setup certain kinds of dump
> targets that requires the service to be running.
>
> Dracut ensures the configured mount points are ready before pre-pivot,
> and after pre-pivot, any further mounting operation may not work as
> expected.
>
> Although there is no report of other type of dump target failure except
> NFSv3, we better revert this, to avoid other potential risk, and wait
> for a proper fix for that systemd/kernel issue.
Agree.
Acked-by: Pingfan Liu <piliu(a)redhat.com>
Hi Pingfan,
Thank you for your Ack, but after a second thought, maybe it's better
to partially revert this commit? Only revert the part in mkdumprd and
keep the change in kdump-lib-initramfs.sh.
I'll send a V2 later.
>
> Else, this may bring more trouble for further development.
> ---
> kdump-lib-initramfs.sh | 30 +++++-------------------------
> mkdumprd | 5 ++---
> 2 files changed, 7 insertions(+), 28 deletions(-)
>
> diff --git a/kdump-lib-initramfs.sh b/kdump-lib-initramfs.sh
> index a7c0bf9..2b5da16 100755
> --- a/kdump-lib-initramfs.sh
> +++ b/kdump-lib-initramfs.sh
> @@ -97,31 +97,16 @@ get_kdump_confs()
> # dump_fs <mount point| device>
> dump_fs()
> {
> - local _do_umount=""
> +
> local _dev=$(findmnt -k -f -n -r -o SOURCE $1)
> local _mp=$(findmnt -k -f -n -r -o TARGET $1)
> local _op=$(findmnt -k -f -n -r -o OPTIONS $1)
>
> + echo "kdump: dump target is $_dev"
> +
> if [ -z "$_mp" ]; then
> - _dev=$(findmnt -s -f -n -r -o SOURCE $1)
> - _mp=$(findmnt -s -f -n -r -o TARGET $1)
> - _op=$(findmnt -s -f -n -r -o OPTIONS $1)
> -
> - if [ -n "$_dev" ] && [ -n "$_mp" ]; then
> - echo "kdump: dump target $_dev is not mounted, trying to
mount..."
> - mkdir -p $_mp
> - mount -o $_op $_dev $_mp
> -
> - if [ $? -ne 0 ]; then
> - echo "kdump: mounting failed (mount point: $_mp, option:
$_op)"
> - return 1
> - fi
> - _do_umount=1
> - else
> - echo "kdump: error: Dump target $_dev is not usable"
> - fi
> - else
> - echo "kdump: dump target is $_dev"
> + echo "kdump: error: Dump target $_dev is not mounted."
> + return 1
> fi
>
> # Remove -F in makedumpfile case. We don't want a flat format dump here.
> @@ -146,11 +131,6 @@ dump_fs()
> sync
>
> echo "kdump: saving vmcore complete"
> -
> - if [ $_do_umount ]; then
> - umount $_mp || echo "kdump: warn: failed to umount target"
> - fi
> -
> # improper kernel cmdline can cause the failure of echo, we can ignore this
kind of failure
> return 0
> }
> diff --git a/mkdumprd b/mkdumprd
> index 35f5eed..cedf536 100644
> --- a/mkdumprd
> +++ b/mkdumprd
> @@ -77,13 +77,12 @@ to_mount() {
> fi
> # mount fs target as rw in 2nd kernel
> _options=$(echo $_options | sed 's/\(^\|,\)ro\($\|,\)/\1rw\2/g')
> - # filter out 'noauto' here, it will be force appended later, avoid
duplication
> + # with 'noauto' in fstab nfs and non-root disk mount will fail in 2nd
> + # kernel, filter it out here.
> _options=$(echo $_options | sed 's/\(^\|,\)noauto\($\|,\)/\1/g')
> # drop nofail or nobootwait
> _options=$(echo $_options | sed 's/\(^\|,\)nofail\($\|,\)/\1/g')
> _options=$(echo $_options | sed 's/\(^\|,\)nobootwait\($\|,\)/\1/g')
> - # only mount the dump target when needed.
> - _options="$_options,noauto"
>
> _mntopts="$_target $_fstype $_options"
> #for non-nfs _dev converting to use udev persistent name
>
--
Best Regards,
Kairui Song