cli: fix sorting of active connections

CMP() is a confusing pattern. Sure enough, the sort order was wrong, for
example, `nmcli connection` would show

    $ nmcli -f STATE,UUID,DEVICE c
    STATE       UUID                                  DEVICE
    activating  3098c902-c59c-45f4-9e5a-e4cdb79cfe1b  nm-bond
    activated   e4fc23ac-54ab-4b1a-932a-ebed12c96d9b  eth1

("activating" shown before "activated").

With `nmcli device`, we sort with compare_devices(). This first sorts by
device state (with "connected" being sorted first). Only when the device
state is equal, we fallback to nmc_active_connection_cmp().  So with
`nmcli device` we usually get "connected" devices first, and we don't
really notice that there is a problem with nmc_active_connection_cmp().

On the other hand, `nmcli connection` likes to sort first via
nmc_active_connection_cmp(), which gets it wrong. Profiles in
"activating" state are sorted first. That's inconsistent with `nmcli
device`, but it's also not what is intended.

Fix that.

Note the change in the test output. Both eth1 and eth0 are connected to
to the same profile, but one "eth0" the active-connection's state is
DEACTIVATING, while on "eth1" it's ACTIVATED (but both device's states
are "CONNECTED"). That's why "eth1" is now sorted first (as desired).

Fixes: a1b25a47b0 ('cli: rework printing of `nmcli connection` for multiple active connections')
(cherry picked from commit 8e1330964d)
This commit is contained in:
Thomas Haller 2023-11-06 11:32:20 +01:00
parent 16f8fcfa0d
commit f623d556ca
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
2 changed files with 56 additions and 42 deletions

View file

@ -314,42 +314,56 @@ active_connection_get_state_ord(NMActiveConnection *active)
int
nmc_active_connection_cmp(NMActiveConnection *ac_a, NMActiveConnection *ac_b)
{
NMSettingIPConfig *s_ip;
NMRemoteConnection *conn;
NMSettingIPConfig *s_ip4_a;
NMSettingIPConfig *s_ip4_b;
NMSettingIPConfig *s_ip6_a;
NMSettingIPConfig *s_ip6_b;
NMRemoteConnection *conn_a;
NMRemoteConnection *conn_b;
NMIPConfig *da_ip;
NMIPConfig *db_ip;
int da_num_addrs;
int db_num_addrs;
int cmp = 0;
gint64 da_num_addrs;
gint64 db_num_addrs;
gboolean bool_a;
gboolean bool_b;
/* Non-active sort last. */
/* nmc_active_connection_cmp() sorts more-important ACs later. That means,
* - NULL comes first
* - then sorting by state (active_connection_get_state_ord()), with "activated" sorted last.
* - various properties of the AC.
*
* This is basically the inverse order of `nmcli connection`.
*/
/* Non-active (and NULL) sort first! */
NM_CMP_SELF(ac_a, ac_b);
NM_CMP_DIRECT(active_connection_get_state_ord(ac_b), active_connection_get_state_ord(ac_a));
NM_CMP_DIRECT(active_connection_get_state_ord(ac_a), active_connection_get_state_ord(ac_b));
conn_a = nm_active_connection_get_connection(ac_a);
conn_b = nm_active_connection_get_connection(ac_b);
s_ip6_a = conn_a ? nm_connection_get_setting_ip6_config(NM_CONNECTION(conn_a)) : NULL;
s_ip6_b = conn_b ? nm_connection_get_setting_ip6_config(NM_CONNECTION(conn_b)) : NULL;
/* Shared connections (likely hotspots) go on the top if possible */
conn = nm_active_connection_get_connection(ac_a);
s_ip = conn ? nm_connection_get_setting_ip6_config(NM_CONNECTION(conn)) : NULL;
if (s_ip
&& nm_streq(nm_setting_ip_config_get_method(s_ip), NM_SETTING_IP6_CONFIG_METHOD_SHARED))
cmp++;
conn = nm_active_connection_get_connection(ac_b);
s_ip = conn ? nm_connection_get_setting_ip6_config(NM_CONNECTION(conn)) : NULL;
if (s_ip
&& nm_streq(nm_setting_ip_config_get_method(s_ip), NM_SETTING_IP6_CONFIG_METHOD_SHARED))
cmp--;
NM_CMP_RETURN(cmp);
bool_a = (s_ip6_a
&& nm_streq(nm_setting_ip_config_get_method(s_ip6_a),
NM_SETTING_IP6_CONFIG_METHOD_SHARED));
bool_b = (s_ip6_b
&& nm_streq(nm_setting_ip_config_get_method(s_ip6_b),
NM_SETTING_IP6_CONFIG_METHOD_SHARED));
NM_CMP_DIRECT(bool_a, bool_b);
conn = nm_active_connection_get_connection(ac_a);
s_ip = conn ? nm_connection_get_setting_ip4_config(NM_CONNECTION(conn)) : NULL;
if (s_ip
&& nm_streq(nm_setting_ip_config_get_method(s_ip), NM_SETTING_IP4_CONFIG_METHOD_SHARED))
cmp++;
conn = nm_active_connection_get_connection(ac_b);
s_ip = conn ? nm_connection_get_setting_ip4_config(NM_CONNECTION(conn)) : NULL;
if (s_ip
&& nm_streq(nm_setting_ip_config_get_method(s_ip), NM_SETTING_IP4_CONFIG_METHOD_SHARED))
cmp--;
NM_CMP_RETURN(cmp);
s_ip4_a = conn_a ? nm_connection_get_setting_ip4_config(NM_CONNECTION(conn_a)) : NULL;
s_ip4_b = conn_b ? nm_connection_get_setting_ip4_config(NM_CONNECTION(conn_b)) : NULL;
bool_a = (s_ip4_a
&& nm_streq(nm_setting_ip_config_get_method(s_ip4_a),
NM_SETTING_IP4_CONFIG_METHOD_SHARED));
bool_b = (s_ip4_b
&& nm_streq(nm_setting_ip_config_get_method(s_ip4_b),
NM_SETTING_IP4_CONFIG_METHOD_SHARED));
NM_CMP_DIRECT(bool_a, bool_b);
/* VPNs go next */
NM_CMP_DIRECT(!!nm_active_connection_get_vpn(ac_a), !!nm_active_connection_get_vpn(ac_b));
@ -367,9 +381,9 @@ nmc_active_connection_cmp(NMActiveConnection *ac_a, NMActiveConnection *ac_b)
db_num_addrs = db_ip ? nm_ip_config_get_addresses(db_ip)->len : 0;
da_ip = nm_active_connection_get_ip6_config(ac_a);
da_num_addrs += da_ip ? nm_ip_config_get_addresses(da_ip)->len : 0;
da_num_addrs += (gint64) (da_ip ? nm_ip_config_get_addresses(da_ip)->len : 0u);
db_ip = nm_active_connection_get_ip6_config(ac_b);
db_num_addrs += db_ip ? nm_ip_config_get_addresses(db_ip)->len : 0;
db_num_addrs += (gint64) (db_ip ? nm_ip_config_get_addresses(db_ip)->len : 0u);
NM_CMP_DIRECT(da_num_addrs, db_num_addrs);
@ -1447,7 +1461,7 @@ get_ac_for_connection_cmp(gconstpointer pa, gconstpointer pb)
NMActiveConnection *ac_a = *((NMActiveConnection *const *) pa);
NMActiveConnection *ac_b = *((NMActiveConnection *const *) pb);
NM_CMP_RETURN(nmc_active_connection_cmp(ac_a, ac_b));
NM_CMP_RETURN(nmc_active_connection_cmp(ac_b, ac_a));
NM_CMP_DIRECT_STRCMP0(nm_active_connection_get_id(ac_a), nm_active_connection_get_id(ac_b));
NM_CMP_DIRECT_STRCMP0(nm_active_connection_get_connection_type(ac_a),
nm_active_connection_get_connection_type(ac_b));

View file

@ -2986,8 +2986,8 @@ returncode: 0
stdout: 1272 bytes
>>>
DEVICE TYPE STATE IP4-CONNECTIVITY IP6-CONNECTIVITY DBUS-PATH CONNECTION CON-UUID CON-PATH
eth0 ethernet connected unknown unknown /org/freedesktop/NetworkManager/Devices/1 ethernet UUID-ethernet-REPLACED-REPLACED-REPL /org/freedesktop/NetworkManager/ActiveConnection/1
eth1 ethernet connected unknown unknown /org/freedesktop/NetworkManager/Devices/2 ethernet UUID-ethernet-REPLACED-REPLACED-REPL /org/freedesktop/NetworkManager/ActiveConnection/2
eth0 ethernet connected unknown unknown /org/freedesktop/NetworkManager/Devices/1 ethernet UUID-ethernet-REPLACED-REPLACED-REPL /org/freedesktop/NetworkManager/ActiveConnection/1
wlan0 wifi unavailable unknown unknown /org/freedesktop/NetworkManager/Devices/3 -- -- --
wlan1 wifi unavailable unknown unknown /org/freedesktop/NetworkManager/Devices/4 -- -- --
wlan1 wifi unavailable unknown unknown /org/freedesktop/NetworkManager/Devices/5 -- -- --
@ -3001,8 +3001,8 @@ returncode: 0
stdout: 1279 bytes
>>>
DEVICE TYPE STATE IP4-CONNECTIVITY IP6-CONNECTIVITY DBUS-PATH CONNECTION CON-UUID CON-PATH
eth0 ethernet połączono nieznane nieznane /org/freedesktop/NetworkManager/Devices/1 ethernet UUID-ethernet-REPLACED-REPLACED-REPL /org/freedesktop/NetworkManager/ActiveConnection/1
eth1 ethernet połączono nieznane nieznane /org/freedesktop/NetworkManager/Devices/2 ethernet UUID-ethernet-REPLACED-REPLACED-REPL /org/freedesktop/NetworkManager/ActiveConnection/2
eth0 ethernet połączono nieznane nieznane /org/freedesktop/NetworkManager/Devices/1 ethernet UUID-ethernet-REPLACED-REPLACED-REPL /org/freedesktop/NetworkManager/ActiveConnection/1
wlan0 wifi niedostępne nieznane nieznane /org/freedesktop/NetworkManager/Devices/3 -- -- --
wlan1 wifi niedostępne nieznane nieznane /org/freedesktop/NetworkManager/Devices/4 -- -- --
wlan1 wifi niedostępne nieznane nieznane /org/freedesktop/NetworkManager/Devices/5 -- -- --
@ -3015,14 +3015,14 @@ lang: C
returncode: 0
stdout: 641 bytes
>>>
eth0: connected to ethernet
"eth0"
ethernet (virtual), C0:61:AE:26:4D:D7, hw
eth1: connected to ethernet
"eth1"
ethernet (virtual), 33:9A:18:81:EB:DC, hw
eth0: connected to ethernet
"eth0"
ethernet (virtual), C0:61:AE:26:4D:D7, hw
wlan0: unavailable
"wlan0"
wifi (virtual), DC:39:87:BA:3E:5D, hw
@ -3051,14 +3051,14 @@ lang: pl_PL.UTF-8
returncode: 0
stdout: 706 bytes
>>>
eth0: połączono do ethernet
"eth0"
ethernet (virtual), C0:61:AE:26:4D:D7, sprzęt
eth1: połączono do ethernet
"eth1"
ethernet (virtual), 33:9A:18:81:EB:DC, sprzęt
eth0: połączono do ethernet
"eth0"
ethernet (virtual), C0:61:AE:26:4D:D7, sprzęt
wlan0: niedostępne
"wlan0"
wifi (virtual), DC:39:87:BA:3E:5D, sprzęt