On Fri, Sep 3, 2021 at 4:50 PM Coiby Xu <coxu(a)redhat.com> wrote:
On Thu, Aug 19, 2021 at 07:39:41PM +0800, Kairui Song wrote:
>nmcli expects multiple parameters, but get_nmcli_value_by_field only
>accepts two params and depends on shell word splitting to split the
>_nm_show_cmd into multiple params, which is very fragile.
>
>By switching the param order, the function can be greatly simplified and
>multiple params can be used properly.
>
>Signed-off-by: Kairui Song <kasong(a)redhat.com>
>---
> dracut-module-setup.sh | 10 +++++-----
> kdump-lib.sh | 10 +++-------
> 2 files changed, 8 insertions(+), 12 deletions(-)
>
>diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
>index 74b969cb..3a8b0e65 100755
>--- a/dracut-module-setup.sh
>+++ b/dracut-module-setup.sh
>@@ -91,7 +91,7 @@ kdump_setup_dns() {
> local _nameserver _dns _tmp array
> local _dnsfile=${initdir}/etc/cmdline.d/42dns.conf
>
>- _tmp=$(get_nmcli_value_by_field "$_nm_show_cmd" "IP4.DNS")
>+ _tmp=$(get_nmcli_value_by_field "IP4.DNS" "$_nm_show_cmd")
> # shellcheck disable=SC2206
> array=(${_tmp//|/ })
> if [[ ${array[*]} ]]; then
>@@ -384,7 +384,7 @@ kdump_setup_bond() {
> done
> echo -n " bond=$_netdev:${_slaves%,}" >>
"${initdir}/etc/cmdline.d/42bond.conf"
>
>- _bondoptions=$(get_nmcli_value_by_field "$_nm_show_cmd"
"bond.options")
>+ _bondoptions=$(get_nmcli_value_by_field "bond.options"
"$_nm_show_cmd")
>
> if [[ -z $_bondoptions ]]; then
> dwarning "Failed to get bond configuration via nmlci output. Now try
sourcing ifcfg script."
>@@ -482,9 +482,9 @@ kdump_setup_znet() {
> local NETTYPE
> local SUBCHANNELS
>
>- NETTYPE=$(get_nmcli_value_by_field "$_nmcli_cmd"
"${s390_prefix}nettype")
>- SUBCHANNELS=$(get_nmcli_value_by_field "$_nmcli_cmd"
"${s390_prefix}subchannels")
>- _options=$(get_nmcli_value_by_field "$_nmcli_cmd"
"${s390_prefix}options")
>+ NETTYPE=$(get_nmcli_value_by_field "${s390_prefix}nettype"
"$_nmcli_cmd")
>+ SUBCHANNELS=$(get_nmcli_value_by_field "${s390_prefix}subchannels"
"$_nmcli_cmd")
>+ _options=$(get_nmcli_value_by_field "${s390_prefix}options"
"$_nmcli_cmd")
>
> if [[ -z $NETTYPE || -z $SUBCHANNELS || -z $_options ]]; then
> dwarning "Failed to get znet configuration via nmlci output. Now try
sourcing ifcfg script."
>diff --git a/kdump-lib.sh b/kdump-lib.sh
>index 4e9a8293..2e003dd8 100755
>--- a/kdump-lib.sh
>+++ b/kdump-lib.sh
>@@ -275,6 +275,7 @@ get_hwaddr()
>
>
> # Get value by a field using "nmcli -g"
>+# Usage: get_nmcli_value_by_field <field> <nmcli command>
> #
> # "nmcli --get-values" allows us to retrive value(s) by field, for
example,
> # nmcli --get-values <field> connection show
/org/freedesktop/NetworkManager/ActiveConnection/1
>@@ -285,12 +286,7 @@ get_hwaddr()
> # bond.options "mode=balance-rr"
> get_nmcli_value_by_field()
> {
>- local _nm_show_cmd=$1
>- local _field=$2
>-
>- local val=$(LANG=C nmcli --get-values $_field $_nm_show_cmd)
>-
>- echo -n "$val"
>+ LANG=C nmcli --get-values "$@"
> }
>
> # Get nmcli connection apath (a D-Bus active connection path ) by ifname
>@@ -302,7 +298,7 @@ get_nmcli_connection_apath_by_ifname()
> local _ifname=$1
> local _nm_show_cmd="device show $_ifname"
>
>- local _apath=$(get_nmcli_value_by_field "$_nm_show_cmd"
"GENERAL.CON-PATH")
>+ local _apath=$(get_nmcli_value_by_field "GENERAL.CON-PATH"
"$_nm_show_cmd")
This won't work because "$_nm_show_cmd" is passed to nmcli as a single
parameter in get_nmcli_value_by_field,
$ nmcli --get-values GENERAL.CON-PATH "device show tun0"
Error: argument 'device show tun0' not understood. Try passing --help instead.
But "[PATCH v2 43/49] kdump-lib.sh: fix word splitting of nmcli params"
later fixes this issue. Maybe "[PATCH v2 43/49]" should be squashed with
this patch considering that "[PATCH v2 43/49]" also explains how "shell
word splitting is very fragile", a.k.a, the connection name could have space.
Good idea, let me squash them together, I didn't check this one
carefully because later patch will rewrite part of the code, leave a
broken commit here is not a good idea.
>
> echo -n "$_apath"
> }
>--
>2.31.1
>_______________________________________________
>kexec mailing list -- kexec(a)lists.fedoraproject.org
>To unsubscribe send an email to kexec-leave(a)lists.fedoraproject.org
>Fedora Code of Conduct:
https://docs.fedoraproject.org/en-US/project/code-of-conduct/
>List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
>List Archives:
https://lists.fedoraproject.org/archives/list/kexec@lists.fedoraproject.org
>Do not reply to spam on the list, report it:
https://pagure.io/fedora-infrastructure
--
Best regards,
Coiby
--
Best Regards,
Kairui Song