Hi Philipp,
On Thu, Aug 11, 2022 at 12:24 AM Philipp Rudo <prudo(a)redhat.com> wrote:
Hi Tao,
On Mon, 8 Aug 2022 19:52:23 +0800
Tao Liu <ltao(a)redhat.com> wrote:
> We need to check if a directory or a device is lvm2 thinp target.
>
> First, we use get_block_dump_target() to convert dump path into
> block device, then we check if the device is lvm2 thinp target by
> cmd lvs.
>
> is_lvm2_thinp_device is now located in kdump-lib-initramfs.sh, for it
> will be used in 2nd kernel. is_lvm2_thinp_dump_target is located in
> kdump-lib.sh, for it is only used in 1st kernel, and it has dependencies
> which exist in kdump-lib.sh.
>
> Signed-off-by: Tao Liu <ltao(a)redhat.com>
> ---
> kdump-lib-initramfs.sh | 9 +++++++++
> kdump-lib.sh | 10 ++++++++++
> 2 files changed, 19 insertions(+)
>
> diff --git a/kdump-lib-initramfs.sh b/kdump-lib-initramfs.sh
> index 84e6bf7..bcf9927 100755
> --- a/kdump-lib-initramfs.sh
> +++ b/kdump-lib-initramfs.sh
> @@ -131,3 +131,12 @@ is_fs_dump_target()
> {
> [ -n "$(kdump_get_conf_val "ext[234]\|xfs\|btrfs\|minix")"
]
> }
> +
> +is_lvm2_thinp_device()
> +{
> + _device_path=$1
> + _lvm2_thin_device=$(lvm lvs -S 'lv_layout=sparse &&
lv_layout=thin' \
> + --nosuffix --noheadings -o vg_name,lv_name "$_device_path"
2>/dev/null)
> +
> + [ -n "$_lvm2_thin_device" ] && return $?
you can drop the '&& return $?'. Bash automatically does that on
function exit.
Agreed
> +}
> \ No newline at end of file
> diff --git a/kdump-lib.sh b/kdump-lib.sh
> index f7b659e..f32b802 100755
> --- a/kdump-lib.sh
> +++ b/kdump-lib.sh
> @@ -117,6 +117,16 @@ is_dump_to_rootfs()
> [[ $(kdump_get_conf_val 'failure_action\|default') == dump_to_rootfs
]]
> }
>
> +is_lvm2_thinp_dump_target()
> +{
> + _target=$(get_block_dump_target)
> + if [ -n "$_target" ]; then
> + is_lvm2_thinp_device "$_target"
> + else
> + return 1
> + fi
> +}
similar to above. You can simply use
[ -n "$_target" ] && is_lvm2_thinp_device "$_target"
Agreed.
or maybe even drop the -n "$_target" when you can guarantee
that
is_lvm2_thinp_device returns false for an empty target.
Thanks
Philipp
> +
> get_failure_action_target()
> {
> local _target