From 61d4bc62e01ebe7634e11a83576a833c03afbb90 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 10 Jul 2020 17:30:06 +0200 Subject: [PATCH] tui: fix default values for bond options in nmtui When configuring miimon settings, the updelay/downdelay fields with value zero may not be stored in the setting. For example: - have a profile with "mode=balance-rr,arp_interval=11,arp_ip_target=10.10.10.1,miimon=10" Switch the link monitoring mode to "MII" and press . Previously, the change of the link monitoring did not update the settings, and nothing was changed. - when loading settings, initialize all fields with the values from the settings, regardless whether they are currently visible or not. Otherwise, if you edit a profile with "mode=balance-rr,arp_interval=11,arp_ip_target=10.10.10.1,miimon=10" and switch link monitoring mode to "MII", the miimon setting was not initialized to 10. - accept empty bond settings, for example for updelay. In that case, initialize the text input to "0". Likewise, when the text entry is empty, set the bond option to the respective default. https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/488 --- clients/tui/nmt-page-bond.c | 115 +++++++++++++++++++----------------- 1 file changed, 61 insertions(+), 54 deletions(-) diff --git a/clients/tui/nmt-page-bond.c b/clients/tui/nmt-page-bond.c index 09dcdbc22a..e92b9f6c26 100644 --- a/clients/tui/nmt-page-bond.c +++ b/clients/tui/nmt-page-bond.c @@ -53,6 +53,14 @@ typedef struct { gboolean updating; } NmtPageBondPrivate; +/*****************************************************************************/ + +static void arp_ip_target_widget_changed (GObject *object, + GParamSpec *pspec, + gpointer user_data); + +/*****************************************************************************/ + NmtEditorPage * nmt_page_bond_new (NMConnection *conn, NmtDeviceEntry *deventry) @@ -98,8 +106,10 @@ bond_options_changed (GObject *object, NMSettingBond *s_bond = NM_SETTING_BOND (object); NmtPageBond *bond = NMT_PAGE_BOND (user_data); NmtPageBondPrivate *priv = NMT_PAGE_BOND_GET_PRIVATE (bond); + gs_free const char **ips = NULL; const char *val; - char **ips; + gboolean visible_mii; + NMBondMode mode; if (priv->updating) return; @@ -109,54 +119,47 @@ bond_options_changed (GObject *object, val = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_MODE); nmt_newt_popup_set_active_id (priv->mode, val); - if (!strcmp (val, "active-backup")) { - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->primary), TRUE); - val = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_PRIMARY); - nmt_newt_entry_set_text (priv->primary, val); - } else - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->primary), FALSE); + mode = _nm_setting_bond_mode_from_string (val ?: ""); + + val = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_PRIMARY); + nmt_newt_entry_set_text (priv->primary, val); + + nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->primary), mode == NM_BOND_MODE_ACTIVEBACKUP); if (priv->monitoring_mode == NMT_PAGE_BOND_MONITORING_UNKNOWN) { val = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_ARP_INTERVAL); - if (val && strcmp (val, "0") != 0) + if (_nm_utils_ascii_str_to_int64 (val, 10, 0, G_MAXINT, 0) > 0) priv->monitoring_mode = NMT_PAGE_BOND_MONITORING_ARP; else priv->monitoring_mode = NMT_PAGE_BOND_MONITORING_MII; } nmt_newt_popup_set_active (priv->monitoring, priv->monitoring_mode); - if (priv->monitoring_mode == NMT_PAGE_BOND_MONITORING_MII) { - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->miimon), TRUE); - val = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_MIIMON); - nmt_newt_entry_set_text (priv->miimon, val); + val = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_MIIMON); + nmt_newt_entry_set_text (priv->miimon, val ?: "0"); - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->updelay), TRUE); - val = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_UPDELAY); - nmt_newt_entry_set_text (priv->updelay, val); + val = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_UPDELAY); + nmt_newt_entry_set_text (priv->updelay, val ?: "0"); - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->downdelay), TRUE); - val = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_DOWNDELAY); - nmt_newt_entry_set_text (priv->downdelay, val); + val = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_DOWNDELAY); + nmt_newt_entry_set_text (priv->downdelay, val ?: "0"); - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->arp_interval), FALSE); - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->arp_ip_target), FALSE); - } else { - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->arp_interval), TRUE); - val = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_ARP_INTERVAL); - nmt_newt_entry_set_text (priv->arp_interval, val); + val = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_ARP_INTERVAL); + nmt_newt_entry_set_text (priv->arp_interval, val ?: "0"); - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->arp_ip_target), TRUE); - val = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_ARP_IP_TARGET); - ips = g_strsplit (val, ",", -1); - g_object_set (G_OBJECT (priv->arp_ip_target), - "strings", ips, - NULL); - g_strfreev (ips); + val = nm_setting_bond_get_option_by_name (s_bond, NM_SETTING_BOND_OPTION_ARP_IP_TARGET); + ips = nm_utils_bond_option_arp_ip_targets_split (val); + g_object_set (G_OBJECT (priv->arp_ip_target), + "strings", ips ?: NM_PTRARRAY_EMPTY (const char *), + NULL); - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->miimon), FALSE); - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->updelay), FALSE); - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->downdelay), FALSE); - } + visible_mii = (priv->monitoring_mode == NMT_PAGE_BOND_MONITORING_MII); + + nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->miimon), visible_mii); + nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->updelay), visible_mii); + nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->downdelay), visible_mii); + nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->arp_interval), !visible_mii); + nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->arp_ip_target), !visible_mii); priv->updating = FALSE; } @@ -210,7 +213,7 @@ _bond_add_option (NMSettingBond *s_bond, _nm_setting_bond_remove_options_arp_interval (s_bond); } -#define WIDGET_CHANGED_FUNC(widget, func, option) \ +#define WIDGET_CHANGED_FUNC(widget, func, option, dflt) \ static void \ widget ## _widget_changed (GObject *object, \ GParamSpec *pspec, \ @@ -218,20 +221,22 @@ widget ## _widget_changed (GObject *object, \ { \ NmtPageBond *bond = NMT_PAGE_BOND (user_data); \ NmtPageBondPrivate *priv = NMT_PAGE_BOND_GET_PRIVATE (bond); \ + const char *v; \ \ if (priv->updating) \ return; \ \ + v = func (priv->widget); \ priv->updating = TRUE; \ - _bond_add_option (priv->s_bond, option, func (priv->widget)); \ + _bond_add_option (priv->s_bond, option, v ?: dflt); \ priv->updating = FALSE; \ } -WIDGET_CHANGED_FUNC (primary, nmt_newt_entry_get_text, NM_SETTING_BOND_OPTION_PRIMARY) -WIDGET_CHANGED_FUNC (miimon, nmt_newt_entry_get_text, NM_SETTING_BOND_OPTION_MIIMON) -WIDGET_CHANGED_FUNC (updelay, nmt_newt_entry_get_text, NM_SETTING_BOND_OPTION_UPDELAY) -WIDGET_CHANGED_FUNC (downdelay, nmt_newt_entry_get_text, NM_SETTING_BOND_OPTION_DOWNDELAY) -WIDGET_CHANGED_FUNC (arp_interval, nmt_newt_entry_get_text, NM_SETTING_BOND_OPTION_ARP_INTERVAL) +WIDGET_CHANGED_FUNC (primary, nmt_newt_entry_get_text, NM_SETTING_BOND_OPTION_PRIMARY, NULL) +WIDGET_CHANGED_FUNC (miimon, nmt_newt_entry_get_text, NM_SETTING_BOND_OPTION_MIIMON, "0") +WIDGET_CHANGED_FUNC (updelay, nmt_newt_entry_get_text, NM_SETTING_BOND_OPTION_UPDELAY, "0") +WIDGET_CHANGED_FUNC (downdelay, nmt_newt_entry_get_text, NM_SETTING_BOND_OPTION_DOWNDELAY, "0") +WIDGET_CHANGED_FUNC (arp_interval, nmt_newt_entry_get_text, NM_SETTING_BOND_OPTION_ARP_INTERVAL, "0") static void mode_widget_changed (GObject *object, @@ -273,25 +278,27 @@ monitoring_widget_changed (GObject *object, { NmtPageBond *bond = NMT_PAGE_BOND (user_data); NmtPageBondPrivate *priv = NMT_PAGE_BOND_GET_PRIVATE (bond); + gboolean visible_mii; if (priv->updating) return; priv->monitoring_mode = nmt_newt_popup_get_active (priv->monitoring); - if (priv->monitoring_mode == NMT_PAGE_BOND_MONITORING_MII) { - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->miimon), TRUE); - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->updelay), TRUE); - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->downdelay), TRUE); + visible_mii = (priv->monitoring_mode == NMT_PAGE_BOND_MONITORING_MII); - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->arp_interval), FALSE); - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->arp_ip_target), FALSE); + nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->miimon), visible_mii); + nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->updelay), visible_mii); + nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->downdelay), visible_mii); + nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->arp_interval), !visible_mii); + nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->arp_ip_target), !visible_mii); + + if (visible_mii) { + miimon_widget_changed (NULL, NULL, bond); + updelay_widget_changed (NULL, NULL, bond); + downdelay_widget_changed (NULL, NULL, bond); } else { - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->arp_interval), TRUE); - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->arp_ip_target), TRUE); - - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->miimon), FALSE); - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->updelay), FALSE); - nmt_newt_widget_set_visible (NMT_NEWT_WIDGET (priv->downdelay), FALSE); + arp_interval_widget_changed (NULL, NULL, bond); + arp_ip_target_widget_changed (NULL, NULL, bond); } }