[PATCH 1/2] Tell nmcli to not escape colon when getting the path of connection profile

[Coiby Xu]

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2151504

When a NetworManager connection profile contains a colon in the name,
"nmcli --get-values UUID,FILENAME" by default would escape the colon
because a colon is also used for separating the values. In this case,
99kdumpbase fails to get the correct connection profile path,
    kdumpctl[5439]: cp: cannot stat '/run/NetworkManager/system-connections/static-52\:54\:01.nmconnection': No such file or directory
    kdumpctl[5440]: sed: can't read /tmp/1977-DRACUT_KDUMP_NM/ifcfg-static-52-54-01: No such file or directory
    kdumpctl[5449]: dracut-install: ERROR: installing '/tmp/1977-DRACUT_KDUMP_NM/ifcfg-static-52-54-01' to '/etc/NetworkManager/system-connections/ifcfg-static-52-54-01'

As a result, dumping vmcore to a remote nfs would fail.

In our case of getting connection profile path, there is no need to escape the
colon so pass "-escape no" to nmcli,

[root@localhost ~]# nmcli --get-values UUID,FILENAME c show
    659e09c1-a6bd-3549-9be4-a07a1a9a8ffd:/etc/NetworkManager/system-connections/aa:bb.nmconnection

[root@localhost ~]# nmcli -escape no --get-values UUID,FILENAME c show
    659e09c1-a6bd-3549-9be4-a07a1a9a8ffd:/etc/NetworkManager/system-connections/aa:bb.nmconnection

Suggested-by: Beniamino Galvani 
Reported-by: Martin Pitt 
Signed-off-by: Coiby Xu 
---
 dracut-module-setup.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
index ab398414..c8015cb5 100755
--- a/dracut-module-setup.sh
+++ b/dracut-module-setup.sh
@@ -318,7 +318,9 @@ clone_and_modify_nmconnection() {
     # connection profile based on MAC address
     _match_nmconnection_by_mac "$_uuid" "$_dev"

-    _cloned_nmconnection_file_path=$(nmcli --get-values UUID,FILENAME connection show | sed -n "s/^${_uuid}://p")
+    # If a value contain ":", nmcli by default escape it with ":" because it
+    # also uses ":" as the delimiter to separate values. In our case, escaping is not needed.
+    _cloned_nmconnection_file_path=$(nmcli --escape no --get-values UUID,FILENAME connection show | sed -n "s/^${_uuid}://p")
     _tmp_nmconnection_file_path=$_DRACUT_KDUMP_NM_TMP_DIR/$(basename "$_nmconnection_file_path")
     cp "$_cloned_nmconnection_file_path" "$_tmp_nmconnection_file_path"
     # change uuid back to old value in case it's refered by other connection

[PATCH 2/2] Show how much time kdump has waited for the network to be ready

[Coiby Xu]

Relates: https://bugzilla.redhat.com/show_bug.cgi?id=2151504

Currently, when the network isn't ready, kdump would repeatedly print
the same info,

[   29.537230] kdump[671]: Bad kdump network destination: 192.123.1.21
    [   30.559418] kdump[679]: Bad kdump network destination: 192.123.1.21
    [   31.580189] kdump[687]: Bad kdump network destination: 192.123.1.21

This is not user-friendly and users may think kdump has got stuck. So
also show much time has waited for the network to be ready,

[   29.537230] kdump[671]: Bad kdump network destination: 192.123.1.21
    [   29.546258] kdump[673]: Waiting for network to be ready (50s / 10min)
    ...
    [   32.601434] kdump[695]: Bad kdump network destination: 192.123.1.21
    [   32.608967] kdump[697]: Waiting for network to be ready (56s / 10min)

Reported-by: Martin Pitt 
Signed-off-by: Coiby Xu 
---
 dracut-kdump.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dracut-kdump.sh b/dracut-kdump.sh
index 872e75ee..74b2b8ae 100755
--- a/dracut-kdump.sh
+++ b/dracut-kdump.sh
@@ -502,6 +502,8 @@ wait_online_network()
    	if _route=$(kdump_get_ip_route "$1" 2> /dev/null); then
    		printf "%s" "$_route"
    		return
+		else
+			dwarn "Waiting for network to be ready (${_loop}s / 10min)"
    	fi
    done

[PATCH 2/2] Show how much time kdump has waited for the network to be ready

[Philipp Rudo]

Hi Coiby,

On Wed, 29 Mar 2023 17:03:49 +0800
Coiby Xu  wrote:
> Relates: https://bugzilla.redhat.com/show_bug.cgi?id=2151504
>
> Currently, when the network isn't ready, kdump would repeatedly print
> the same info,
> [   29.537230] kdump[671]: Bad kdump network destination: 192.123.1.21
> [   30.559418] kdump[679]: Bad kdump network destination: 192.123.1.21
> [   31.580189] kdump[687]: Bad kdump network destination: 192.123.1.21
>
> This is not user-friendly and users may think kdump has got stuck. So
> also show much time has waited for the network to be ready,
> [   29.537230] kdump[671]: Bad kdump network destination: 192.123.1.21
> [   29.546258] kdump[673]: Waiting for network to be ready (50s / 10min)
> ...
> [   32.601434] kdump[695]: Bad kdump network destination: 192.123.1.21
> [   32.608967] kdump[697]: Waiting for network to be ready (56s / 10min)
>
> Reported-by: Martin Pitt 
> Signed-off-by: Coiby Xu
>
> dracut-kdump.sh | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/dracut-kdump.sh b/dracut-kdump.sh
> index 872e75ee..74b2b8ae 100755
> --- a/dracut-kdump.sh
> +++ b/dracut-kdump.sh
> @@ -502,6 +502,8 @@ wait_online_network()
>    	if _route=$(kdump_get_ip_route "$1" 2> /dev/null); then
>    		printf "%s" "$_route"
>    		return
> else
> 	dwarn "Waiting for network to be ready (${_loop}s / 10min)"
> fi
> done

The message makes totally sense. What I'm a little bit puzzled about is
that kdump_get_ip_route and wait_online_network will use different
log levels (derror vs. dwarn). But finding no route to the destination
is sort of expected behavior here so not actually an error. Not sure
what's the best thing to do here...

Anyway
Reviewed-by: Philipp Rudo

[PATCH 2/2] Show how much time kdump has waited for the network to be ready

[Coiby Xu]

On Mon, Apr 03, 2023 at 05:45:31PM +0200, Philipp Rudo wrote:
> Hi Coiby,
>
> On Wed, 29 Mar 2023 17:03:49 +0800
> Coiby Xu  wrote:
> > Relates: https://bugzilla.redhat.com/show_bug.cgi?id=2151504
> >
> > Currently, when the network isn't ready, kdump would repeatedly print
> > the same info,
> > [   29.537230] kdump[671]: Bad kdump network destination: 192.123.1.21
> > [   30.559418] kdump[679]: Bad kdump network destination: 192.123.1.21
> > [   31.580189] kdump[687]: Bad kdump network destination: 192.123.1.21
> >
> > This is not user-friendly and users may think kdump has got stuck. So
> > also show much time has waited for the network to be ready,
> > [   29.537230] kdump[671]: Bad kdump network destination: 192.123.1.21
> > [   29.546258] kdump[673]: Waiting for network to be ready (50s / 10min)
> > ...
> > [   32.601434] kdump[695]: Bad kdump network destination: 192.123.1.21
> > [   32.608967] kdump[697]: Waiting for network to be ready (56s / 10min)
> >
> > Reported-by: Martin Pitt 
> > Signed-off-by: Coiby Xu
> >
> > dracut-kdump.sh | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/dracut-kdump.sh b/dracut-kdump.sh
> > index 872e75ee..74b2b8ae 100755
> > --- a/dracut-kdump.sh
> > +++ b/dracut-kdump.sh
> > @@ -502,6 +502,8 @@ wait_online_network()
> >    	if _route=$(kdump_get_ip_route "$1" 2> /dev/null); then
> >    		printf "%s" "$_route"
> >    		return
> > else
> > 	dwarn "Waiting for network to be ready (${_loop}s / 10min)"
> > fi
> > done
>
> The message makes totally sense. What I'm a little bit puzzled about is
> that kdump_get_ip_route and wait_online_network will use different
> log levels (derror vs. dwarn). But finding no route to the destination
> is sort of expected behavior here so not actually an error. Not sure
> what's the best thing to do here...

Thanks for catching this inconsistency. I've sent a new version to
let the caller determine what to be printed.
> Anyway
> Reviewed-by: Philipp Rudo

[Philipp Rudo]

Hi Coiby,

On Wed, 29 Mar 2023 17:03:48 +0800
Coiby Xu  wrote:
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2151504
>
> When a NetworManager connection profile contains a colon in the name,
> "nmcli --get-values UUID,FILENAME" by default would escape the colon
> because a colon is also used for separating the values. In this case,
> 99kdumpbase fails to get the correct connection profile path,
>    kdumpctl[5439]: cp: cannot stat '/run/NetworkManager/system-connections/static-52\:54\:01.nmconnection': No such file or directory
>    kdumpctl[5440]: sed: can't read /tmp/1977-DRACUT_KDUMP_NM/ifcfg-static-52-54-01: No such file or directory
>    kdumpctl[5449]: dracut-install: ERROR: installing '/tmp/1977-DRACUT_KDUMP_NM/ifcfg-static-52-54-01' to '/etc/NetworkManager/system-connections/ifcfg-static-52-54-01'
>
> As a result, dumping vmcore to a remote nfs would fail.
>
> In our case of getting connection profile path, there is no need to escape the
> colon so pass "-escape no" to nmcli,
>
> [root@localhost ~]# nmcli --get-values UUID,FILENAME c show
>    659e09c1-a6bd-3549-9be4-a07a1a9a8ffd:/etc/NetworkManager/system-connections/aa:bb.nmconnection
>
> [root@localhost ~]# nmcli -escape no --get-values UUID,FILENAME c show
>    659e09c1-a6bd-3549-9be4-a07a1a9a8ffd:/etc/NetworkManager/system-connections/aa:bb.nmconnection
>
> Suggested-by: Beniamino Galvani 
> Reported-by: Martin Pitt 
> Signed-off-by: Coiby Xu
>
> dracut-module-setup.sh | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
> index ab398414..c8015cb5 100755
> --- a/dracut-module-setup.sh
> +++ b/dracut-module-setup.sh
> @@ -318,7 +318,9 @@ clone_and_modify_nmconnection() {
>      # connection profile based on MAC address
>      _match_nmconnection_by_mac "$_uuid" "$_dev"
> _cloned_nmconnection_file_path=$(nmcli --get-values UUID,FILENAME connection show | sed -n "s/^${_uuid}://p")
> # If a value contain ":", nmcli by default escape it with ":" because it
> # also uses ":" as the delimiter to separate values. In our case, escaping is not needed.
> _cloned_nmconnection_file_path=$(nmcli --escape no --get-values UUID,FILENAME connection show | sed -n "s/^${_uuid}://p")
> _tmp_nmconnection_file_path=$_DRACUT_KDUMP_NM_TMP_DIR/$(basename "$_nmconnection_file_path")
> cp "$_cloned_nmconnection_file_path" "$_tmp_nmconnection_file_path"
> # change uuid back to old value in case it's refered by other connection

Looks good to me
Reviewed-by: Philipp Rudo

[Coiby Xu]

On Mon, Apr 03, 2023 at 05:35:17PM +0200, Philipp Rudo wrote:
> Hi Coiby,
>
> On Wed, 29 Mar 2023 17:03:48 +0800
> Coiby Xu  wrote:
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2151504
> >
> > When a NetworManager connection profile contains a colon in the name,
> > "nmcli --get-values UUID,FILENAME" by default would escape the colon
> > because a colon is also used for separating the values. In this case,
> > 99kdumpbase fails to get the correct connection profile path,
> >    kdumpctl[5439]: cp: cannot stat '/run/NetworkManager/system-connections/static-52\:54\:01.nmconnection': No such file or directory
> >    kdumpctl[5440]: sed: can't read /tmp/1977-DRACUT_KDUMP_NM/ifcfg-static-52-54-01: No such file or directory
> >    kdumpctl[5449]: dracut-install: ERROR: installing '/tmp/1977-DRACUT_KDUMP_NM/ifcfg-static-52-54-01' to '/etc/NetworkManager/system-connections/ifcfg-static-52-54-01'
> >
> > As a result, dumping vmcore to a remote nfs would fail.
> >
> > In our case of getting connection profile path, there is no need to escape the
> > colon so pass "-escape no" to nmcli,
> >
> > [root@localhost ~]# nmcli --get-values UUID,FILENAME c show
> >    659e09c1-a6bd-3549-9be4-a07a1a9a8ffd:/etc/NetworkManager/system-connections/aa:bb.nmconnection
> >
> > [root@localhost ~]# nmcli -escape no --get-values UUID,FILENAME c show
> >    659e09c1-a6bd-3549-9be4-a07a1a9a8ffd:/etc/NetworkManager/system-connections/aa:bb.nmconnection
> >
> > Suggested-by: Beniamino Galvani 
> > Reported-by: Martin Pitt 
> > Signed-off-by: Coiby Xu
> >
> > dracut-module-setup.sh | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/dracut-module-setup.sh b/dracut-module-setup.sh
> > index ab398414..c8015cb5 100755
> > --- a/dracut-module-setup.sh
> > +++ b/dracut-module-setup.sh
> > @@ -318,7 +318,9 @@ clone_and_modify_nmconnection() {
> >      # connection profile based on MAC address
> >      _match_nmconnection_by_mac "$_uuid" "$_dev"
> > _cloned_nmconnection_file_path=$(nmcli --get-values UUID,FILENAME connection show | sed -n "s/^${_uuid}://p")
> > # If a value contain ":", nmcli by default escape it with ":" because it
> > # also uses ":" as the delimiter to separate values. In our case, escaping is not needed.
> > _cloned_nmconnection_file_path=$(nmcli --escape no --get-values UUID,FILENAME connection show | sed -n "s/^${_uuid}://p")
> > _tmp_nmconnection_file_path=$_DRACUT_KDUMP_NM_TMP_DIR/$(basename "$_nmconnection_file_path")
> > cp "$_cloned_nmconnection_file_path" "$_tmp_nmconnection_file_path"
> > # change uuid back to old value in case it's refered by other connection
>
> Looks good to me
> Reviewed-by: Philipp Rudo

Thanks for the review!