On 07/10/2017 at 10:53 AM, Dave Young wrote:
On 07/07/17 at 03:48pm, Xunlei Pang wrote:
> Resolves: bz1451717
>
https://bugzilla.redhat.com/1451717
>
> When there is any "rd.lvm.lv=X", "lvm" dracut module will
> try to recognize all the lvm volumes which is unnecessary
> and probably cause trouble for us.
>
> See
https://bugzilla.redhat.com/show_bug.cgi?id=1451717#c2
>
> Remove all the rd.lvm.lv=X inherited from the kernel cmdline,
> and generate the corresponding cmdline as needed for kdump.
> Because prepare_cmdline() is only used by kdump, we don't need
> to add any fadump judgement(also remove the existing judgement
> in passing).
>
> Currently, we don't handle "rd.lvm.vg=X", we can add it in
> when there is some bug reported in the future.
>
> Signed-off-by: Xunlei Pang <xlpang(a)redhat.com>
> ---
> v3->v4:
> -Made "GENERATED_LVM_CMDLINES" a local variable "lvm_cmdlines".
>
> kdumpctl | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/kdumpctl b/kdumpctl
> index 8c40564..3405530 100755
> --- a/kdumpctl
> +++ b/kdumpctl
> @@ -171,6 +171,46 @@ check_kdump_cpus()
> echo " try nr_cpus=$nr_min or larger instead"
> }
>
> +# Generate rd.lvm.lv=X for the kdump targets if any.
> +generate_lvm_cmdline()
> +{
> + local majmin=$1 dev
> +
> + [ -d "/sys/dev/block/$majmin/dm" ] || return 0
> + dev=/dev/mapper/$(< "/sys/dev/block/$majmin/dm/name")
> +
> + vg=$(lvm lvs --rows $dev -o vg_name --separator=* 2>/dev/null | cut -d
"*" -f 2)
> + lv=$(lvm lvs --rows $dev -o lv_name --separator=* 2>/dev/null | cut -d
"*" -f 2)
> + if [ -n "$vg" -a -n "$lv" ]; then
> + lvm_cmdlines="rd.lvm.lv=$vg/$lv $lvm_cmdlines"
> + fi
> +
> + return 0
> +}
> +
> +generate_lvm_cmdlines()
> +{
> + local lvm_cmdlines=""
> +
> + for_each_block_target_all generate_lvm_cmdline
Since lvm_cmdlines becomes a local variable, it is better to move
function generate_lvm_cmdline code into getnerate_lvm_cmdlines directly
It is less likely we need generate lvm_cmdline for a specific block
target.
Sorry for observed this late, I think this is the only comment for me to
this series, other than this, the whole series looks good to me.
Can you resend this patch only if agree with above?
Completely agree, thanks for noticing this, will resend this patch alone soon.
After waiting for several days I can merge the series if no other
comments from other people.
Yes, thanks for all your valuable suggestions!
Regards,
Xunlei
> +
> + echo "$lvm_cmdlines"
> +}
> +
> +# $1: function name
> +for_each_block_target_all()
> +{
> + local dev majmin
> +
> + for dev in $(get_kdump_targets); do
> + [ -b "$dev" ] || continue
> + majmin=$(get_maj_min $dev)
> + check_block_and_slaves_all $1 $majmin
> + done
> +
> + return 0
> +}
> +
> # This function performs a series of edits on the command line.
> # Store the final result in global $KDUMP_COMMANDLINE.
> prepare_cmdline()
> @@ -190,15 +230,19 @@ prepare_cmdline()
>
> # Always remove "root=X", as we now explicitly generate all kinds
> # of dump target mount information including root fs. But we can
> - # not remove it in case of fadump or "default dump_to_rootfs".
> + # not remove it in case of "default dump_to_rootfs".
> #
> # We do this before KDUMP_COMMANDLINE_APPEND, if one really cares
> # about it(e.g. for debug purpose), then can pass "root=X" using
> # KDUMP_COMMANDLINE_APPEND.
> - if [ $DEFAULT_DUMP_MODE != "fadump" ] && ! is_dump_to_rootfs;
then
> + if ! is_dump_to_rootfs; then
> cmdline=`remove_cmdline_param "$cmdline" root`
> fi
>
> + # Remove all the inherited rd.lvm.lv=X and generate those as needed.
> + cmdline=`remove_cmdline_param "$cmdline" rd.lvm.lv`
> + cmdline="${cmdline} $(generate_lvm_cmdlines)"
> +
> cmdline="${cmdline} ${KDUMP_COMMANDLINE_APPEND}"
>
> id=`get_bootcpu_initial_apicid`
> --
> 1.8.3.1
> _______________________________________________
> kexec mailing list -- kexec(a)lists.fedoraproject.org
> To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
Thanks
Dave