On Wed, May 25, 2022 at 9:40 AM Coiby Xu <coxu(a)redhat.com> wrote:
On Tue, May 24, 2022 at 10:31:17PM +0800, Tao Liu wrote:
>Hi Coiby,
>
>On Wed, May 18, 2022 at 10:37 AM Tao Liu <ltao(a)redhat.com> wrote:
>>
>> On Wed, May 18, 2022 at 10:15 AM Coiby Xu <coxu(a)redhat.com> wrote:
>> >
>> > Hi Tao,
>> >
>> > kdump-lib-initramfs.sh is supposed to be POSIX-compatible. As caught by
>> > shellspec [1], we can't use local or arrays in POSIX sh. You may need
to
>> > fix other small issues caught by shellspec as well.
>>
>
>I see from your gitlab shellcheck results, there are plenty of errors, and I can
>reproduce them locally by "$ shellcheck *.sh spec/*.sh kdumpctl
mk*dumprd".
>
>Seems these checking errors existed for a long time, and I remember kasong
>had sent a big patch set to address the similar issue before. Just for
>curiosity,
>should the errors have been noticed then, or did we use a different shellcheck
>cmdline?
Kairui's huge patch only addressed some of the errors. There are still
many remaining shellcheck errors. My plan is to pay off the technical
debt gradually. As long we don't introduce new errors in new patches,
it should be fine.
OK, thanks for the explanation! I will try to solve them, since I'm
not quite familiar with shellcheck, it may take a while for me to
figure out the useful or false alarms.
Thanks,
Tao Liu
>
>Thanks,
>Tao Liu
>
>
>> OK, I think I will post 2 separate patches for this:
>>
>> one for fix POSIX-compatible issue specifically for this patch set.
>> The other for fix POSIX-compatible issues for the rest.
>>
>> Thanks,
>> Tao Liu
>>
>> >
>> > [1]
https://gitlab.com/coxu/fedora-kexec-tools/-/jobs/2461492476
>> >
>> > On Mon, May 16, 2022 at 11:25:51AM +0800, Tao Liu 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 in the output of
>> > >cmd lvs. If the device is lvm2 thinp, whose attribute given by
>> > >lvs will be '/^.{6}t'. And the device's full path can be
one of:
>> > >1) /dev/mapper/vg-thinp, 2) /dev/vg/thinp.
>> > >
>> > >Signed-off-by: Tao Liu <ltao(a)redhat.com>
>> > >---
>> > > kdump-lib-initramfs.sh | 24 ++++++++++++++++++++++++
>> > > 1 file changed, 24 insertions(+)
>> > >
>> > >diff --git a/kdump-lib-initramfs.sh b/kdump-lib-initramfs.sh
>> > >index 84e6bf7..149aa49 100755
>> > >--- a/kdump-lib-initramfs.sh
>> > >+++ b/kdump-lib-initramfs.sh
>> > >@@ -131,3 +131,27 @@ is_fs_dump_target()
>> > > {
>> > > [ -n "$(kdump_get_conf_val
"ext[234]\|xfs\|btrfs\|minix")" ]
>> > > }
>> > >+
>> > >+is_lvm2_thinp_device()
>> > >+{
>> > >+ local _device_path=$1
>> > >+ local _lvm2_thinp_volumes=($(lvs 2>/dev/null |
>> > >+ awk '{if ($3 ~ /^.{6}t/) {printf("%s-%s %s/%s
", $2, $1, $2, $1);}}'))
>> > >+
>> > >+ for _v in ${_lvm2_thinp_volumes[@]}; do
>> > >+ [[ "/dev/mapper/$_v" == $_device_path ||
>> > >+ "/dev/$_v" == $_device_path ]] &&
>> > >+ return 0
>> > >+ done
>> > >+ return 1
>> > >+}
>> > >+
>> > >+is_lvm2_thinp_dump_target()
>> > >+{
>> > >+ local _target=$(get_block_dump_target)
>> > >+ if [ -n $_target ]; then
>> > >+ is_lvm2_thinp_device $_target
>> > >+ else
>> > >+ return 1
>> > >+ fi
>> > >+}
>> > >\ No newline at end of file
>> > >--
>> > >2.33.1
>> > >
>> >
>> > --
>> > Best regards,
>> > Coiby
>> >
>
--
Best regards,
Coiby