Thanks for reviewing this patch set.
On Tue, Feb 09, 2021 at 05:46:50PM +0800, Kairui Song wrote:
NETTYPE=$(grep s390-subchannels <<< $nmcli_out | sed
"s/.*:\s*//g")On
sed is a better way:)
Mon, Feb 8, 2021 at 2:14 PM Coiby Xu <coxu(a)redhat.com> wrote:
>
> Since Fedora 34, ifcfg has been deprecated. Read znet config from nmcli
> directly,
>
> $ nmcli c show enc8000
> connection.id: enc8000
> connection.uuid: 02fbf6c2-b5bf-4ced-8a41-99a46ade828c
> connection.stable-id: --
> connection.type: 802-3-ethernet
> connection.interface-name: enc8000
> ...
> 802-3-ethernet.s390-subchannels: 0.0.8000,0.0.8001,0.0.8002
> 802-3-ethernet.s390-nettype: qeth
> 802-3-ethernet.s390-options: layer2=1,portname=z-104,portno=0
>
> Reported-by: Jie Li <jieli(a)redhat.com>
> Signed-off-by: Coiby Xu <coxu(a)redhat.com>
> ---
> dracut-module-setup.sh | 31 +++++++++++++++++++++++++++----
> 1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
> index 21143b4..54e3fb5 100755
> --- a/dracut-module-setup.sh
> +++ b/dracut-module-setup.sh
> @@ -323,11 +323,34 @@ kdump_setup_znet() {
> local _options=""
> local _netdev=$1
>
> - source_ifcfg_file $_netdev
> -
> - for i in $OPTIONS; do
> - _options=${_options},$i
> + declare -A vals
Will "local -A vals" work here? If so please use a local statement to
avoid pollution namespace.
For bash, "declare -A var" would make a local variable. But "local
-A"
seems better in terms of readability.
> + fields=(subchannels nettype options)
> +
> + # 1. first read from nmcli output directly
> + # 802-3-ethernet.s390-subchannels: 0.0.8000,0.0.8001,0.0.8002
> + # 802-3-ethernet.s390-nettype: qeth
> + # 802-3-ethernet.s390-options: layer2=1,portname=z-104,portno=0
> +
> + nmcli_out=$(nmcli conn show $_netdev)
Also better add a local constraint to "nmcli_out" and "fields" and
other temp variables.
Oh, I forgot about them. Thanks for the reminder!
> + for field in "${fields[@]}"; do
> + val_raw=$(grep s390-$field <<< $nmcli_out | cut -d":"
-f2)
> + # remove leading and trailing spaces
> + vals[$field]=${val_raw//[[:space:]]}
> done
> +
> + NETTYPE=${vals[nettype]}
> + SUBCHANNELS=${vals[subchannels]}
> + options=${vals[options]}
How about not just:
NETTYPE=$(grep s390-nettype <<< $nmcli_out | sed "s/^.*:\s*//")
SUBCHANNELS=$(grep s390-subchannels <<< $nmcli_out | sed
"s/^.*:\s*//")
options=$(grep s390-options <<< $nmcli_out | sed "s/^.*:\s*//")
A bit longer line but get rids of the array and for loop.
My reason to use an array is parsing output from nmlci follows the same
logic. In case nmlci changes its output format in the future, we only
need to change one place.
> +
> + # 2. if failed to get SUBCHANNELS, read from ifcfg_file
> + if [[ -z "$SUBCHANNELS" ]]; then
> + source_ifcfg_file $_netdev
> +
> + for i in $OPTIONS; do
> + _options=${_options},$i
> + done
> + fi
> +
> echo rd.znet=${NETTYPE},${SUBCHANNELS}${_options}
rd.znet_ifname=$_netdev:${SUBCHANNELS} > ${initdir}/etc/cmdline.d/30znet.conf
> }
>
> --
> 2.30.0
>
--
Best Regards,
Kairui Song
--
Best regards,
Coiby