From fe82c3a37ae5f65845065d0c04be7e324ff64ab1 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 24 Jun 2022 00:29:58 +0200 Subject: [PATCH 01/12] libnmc-setting: fix default suggestions for some options These are just plain wrong. --- src/libnmc-setting/nm-meta-setting-desc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libnmc-setting/nm-meta-setting-desc.c b/src/libnmc-setting/nm-meta-setting-desc.c index fd7418eff0..cc15eab776 100644 --- a/src/libnmc-setting/nm-meta-setting-desc.c +++ b/src/libnmc-setting/nm-meta-setting-desc.c @@ -5139,7 +5139,7 @@ static const NMMetaPropertyInfo *const property_infos_BRIDGE[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_STP, .is_cli_option = TRUE, .property_alias = "stp", - .prompt = N_("Enable STP [no]"), + .prompt = N_("Enable STP [yes]"), .property_type = &_pt_gobject_bool, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_PRIORITY, @@ -5221,7 +5221,7 @@ static const NMMetaPropertyInfo *const property_infos_BRIDGE[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_MULTICAST_SNOOPING, .is_cli_option = TRUE, .property_alias = "multicast-snooping", - .prompt = N_("Enable IGMP snooping [no]"), + .prompt = N_("Enable IGMP snooping [yes]"), .property_type = &_pt_gobject_bool, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_MULTICAST_STARTUP_QUERY_COUNT, -- GitLab From 0cb971d1d6c7e267727cfaf26276b4cfc73685fb Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 24 Jun 2022 00:30:04 +0200 Subject: [PATCH 02/12] nmcli/connections: pass allow_reset to check_and_set() callback Like the regular set_option() handler, the special ones also need to know whether to reset an option or keep the value. --- src/nmcli/connections.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index 4d9f662bf5..787ea25c9f 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -52,6 +52,7 @@ typedef struct _OptionInfo { NMConnection *connection, const struct _OptionInfo *option, const char *value, + gboolean allow_reset, GError **error); CompEntryFunc generator_func; } OptionInfo; @@ -4375,7 +4376,7 @@ set_option(NmCli *nmc, NULL, NULL); if (option && option->check_and_set) { - return option->check_and_set(nmc, connection, option, value, error); + return option->check_and_set(nmc, connection, option, value, allow_reset, error); } else if (value || allow_reset) { return set_property(nmc->client, connection, @@ -4506,6 +4507,7 @@ set_connection_type(NmCli *nmc, NMConnection *con, const OptionInfo *option, const char *value, + gboolean allow_reset, GError **error) { const NMMetaSettingValidPartItem *const *type_settings; @@ -4516,6 +4518,8 @@ set_connection_type(NmCli *nmc, value = check_valid_name_toplevel(value, &slave_type, &local); if (!value) { + if (!allow_reset) + return TRUE; g_set_error(error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, @@ -4570,12 +4574,15 @@ set_connection_iface(NmCli *nmc, NMConnection *con, const OptionInfo *option, const char *value, + gboolean allow_reset, GError **error) { if (value) { /* Special value of '*' means no specific interface name */ if (nm_streq(value, "*")) value = NULL; + } else if (!allow_reset) { + return TRUE; } return set_property(nmc->client, @@ -4592,6 +4599,7 @@ set_connection_master(NmCli *nmc, NMConnection *con, const OptionInfo *option, const char *value, + gboolean allow_reset, GError **error) { const GPtrArray *connections; @@ -4602,6 +4610,8 @@ set_connection_master(NmCli *nmc, g_return_val_if_fail(s_con, FALSE); if (!value) { + if (!allow_reset) + return TRUE; g_set_error_literal(error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, @@ -4637,6 +4647,7 @@ set_bond_option(NmCli *nmc, NMConnection *con, const OptionInfo *option, const char *value, + gboolean allow_reset, GError **error) { NMSettingBond *s_bond; @@ -4681,6 +4692,7 @@ set_bond_monitoring_mode(NmCli *nmc, NMConnection *con, const OptionInfo *option, const char *value, + gboolean allow_reset, GError **error) { NMSettingBond *s_bond; @@ -4721,6 +4733,7 @@ set_bluetooth_type(NmCli *nmc, NMConnection *con, const OptionInfo *option, const char *value, + gboolean allow_reset, GError **error) { NMSetting *setting; @@ -4769,6 +4782,7 @@ set_ip4_address(NmCli *nmc, NMConnection *con, const OptionInfo *option, const char *value, + gboolean allow_reset, GError **error) { NMSettingIPConfig *s_ip4; @@ -4796,6 +4810,7 @@ set_ip6_address(NmCli *nmc, NMConnection *con, const OptionInfo *option, const char *value, + gboolean allow_reset, GError **error) { NMSettingIPConfig *s_ip6; -- GitLab From d51140d2abcde0a77b1bbc5d7be664161f498060 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 24 Jun 2022 00:30:04 +0200 Subject: [PATCH 03/12] nmcli/connections: do not remove a bond option unless reset is allowed If we're setting an option with no value given and no reset allowed, let's just set the default value. --- src/nmcli/connections.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index 787ea25c9f..eda68139bb 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -4651,7 +4651,6 @@ set_bond_option(NmCli *nmc, GError **error) { NMSettingBond *s_bond; - gboolean success; gs_free char *name = NULL; char *p; @@ -4665,26 +4664,25 @@ set_bond_option(NmCli *nmc, } if (nm_str_is_empty(value)) { - nm_setting_bond_remove_option(s_bond, name); - success = TRUE; - } else - success = _nm_meta_setting_bond_add_option(NM_SETTING(s_bond), name, value, error); - - if (!success) - return FALSE; + if (allow_reset) { + nm_setting_bond_remove_option(s_bond, name); + return TRUE; + } + } else { + if (!_nm_meta_setting_bond_add_option(NM_SETTING(s_bond), name, value, error)) + return FALSE; + } - if (success) { - if (nm_streq(name, NM_SETTING_BOND_OPTION_MODE)) { - value = nmc_bond_validate_mode(value, error); - if (nm_streq(value, "active-backup")) { - enable_options(NM_SETTING_BOND_SETTING_NAME, - NM_SETTING_BOND_OPTIONS, - NM_MAKE_STRV("primary")); - } + if (nm_streq(name, NM_SETTING_BOND_OPTION_MODE)) { + value = nm_setting_bond_get_option_by_name(s_bond, name); + if (nm_streq(value, "active-backup")) { + enable_options(NM_SETTING_BOND_SETTING_NAME, + NM_SETTING_BOND_OPTIONS, + NM_MAKE_STRV("primary")); } } - return success; + return TRUE; } static gboolean -- GitLab From a5e099d0085c86954f6821bd4aa07faee09f9591 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 24 Jun 2022 00:30:04 +0200 Subject: [PATCH 04/12] nmcli/connections: allow empty lists with "--ask c add" The interactive add is not too enthusiastic about not providing a value in a list. That is before on getting an empty line in ask_option() we take a shortcut instead of dispatching to set_option(). That way we skip setting the PROPERTY_INF_FLAG_DISABLED flag, causing the option to be included in questionnaire_one_optional()'s info list. There's no reason to avoid calling set_option() if we don't get a value; set_option() handles NULL value just fine. $ nmcli -a c add Connection type: dummy There is 1 optional setting for General settings. Do you want to provide it? (yes/no) [yes] Interface name [*]: lala There are 2 optional settings for IPv4 protocol. Do you want to provide them? (yes/no) [yes] You can specify this option more than once. Press when you're done. IPv4 address (IP[/plen]) [none]: You can specify this option more than once. Press when you're done. IPv4 address (IP[/plen]) [none]: You can specify this option more than once. Press when you're done. IPv4 address (IP[/plen]) [none]: ... --- src/nmcli/connections.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index eda68139bb..7208abcc75 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -5591,8 +5591,6 @@ ask_option(NmCli *nmc, NMConnection *connection, const NMMetaAbstractInfo *abstr again: value = nmc_readline(&nmc->nmc_config, "%s", prompt); - if (multi && !value) - return; if (!set_option(nmc, connection, abstract_info, value, FALSE, &error)) { g_printerr("%s\n", error->message); -- GitLab From 6fee8aa454d18fb9147fe229fe0cf6120ed41020 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 24 Jun 2022 00:30:04 +0200 Subject: [PATCH 05/12] nmcli/connections: make opts argument to enable_options() optional This makes things slightly less annoying when dealing with options that map nicely to properties (unlike bridge options). --- src/nmcli/connections.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index 7208abcc75..73d34452ad 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -4174,11 +4174,16 @@ enable_options(const char *setting_name, const char *property, const char *const for (i = 0; i < nm_meta_property_typ_data_bond.nested_len; i++) { const NMMetaNestedPropertyInfo *bi = &nm_meta_property_typ_data_bond.nested[i]; - if (bi->base.inf_flags & NM_META_PROPERTY_INF_FLAG_DONT_ASK && bi->base.property_alias - && g_strv_contains(opts, bi->base.property_alias)) + if (opts) { + if (!bi->base.property_alias || !g_strv_contains(opts, bi->base.property_alias)) + continue; + } + + if (bi->base.inf_flags & NM_META_PROPERTY_INF_FLAG_DONT_ASK) { _dynamic_options_set((const NMMetaAbstractInfo *) bi, PROPERTY_INF_FLAG_ENABLED, PROPERTY_INF_FLAG_ENABLED); + } } return; } @@ -4186,11 +4191,16 @@ enable_options(const char *setting_name, const char *property, const char *const if (!property_info->is_cli_option) g_return_if_reached(); - if (property_info->inf_flags & NM_META_PROPERTY_INF_FLAG_DONT_ASK - && property_info->property_alias && g_strv_contains(opts, property_info->property_alias)) + if (opts) { + if (!property_info->property_alias || !g_strv_contains(opts, property_info->property_alias)) + return; + } + + if (property_info->inf_flags & NM_META_PROPERTY_INF_FLAG_DONT_ASK) { _dynamic_options_set((const NMMetaAbstractInfo *) property_info, PROPERTY_INF_FLAG_ENABLED, PROPERTY_INF_FLAG_ENABLED); + } } /* @@ -4513,7 +4523,6 @@ set_connection_type(NmCli *nmc, const NMMetaSettingValidPartItem *const *type_settings; const NMMetaSettingValidPartItem *const *slv_settings; GError *local = NULL; - const char *master[] = {"master", NULL}; const char *slave_type = NULL; value = check_valid_name_toplevel(value, &slave_type, &local); @@ -4539,7 +4548,7 @@ set_connection_type(NmCli *nmc, error)) { return FALSE; } - enable_options(NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_MASTER, master); + enable_options(NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_MASTER, NULL); } /* ifname is mandatory for all connection types except virtual ones (bond, team, bridge, vlan) */ -- GitLab From cf62f0e3a15c71cb036f272c6b1d3793e6acd5c1 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 24 Jun 2022 00:30:04 +0200 Subject: [PATCH 06/12] nmcli/connections: make enable_options() always enable an option --- src/nmcli/connections.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index 73d34452ad..4d646798de 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -4179,11 +4179,9 @@ enable_options(const char *setting_name, const char *property, const char *const continue; } - if (bi->base.inf_flags & NM_META_PROPERTY_INF_FLAG_DONT_ASK) { - _dynamic_options_set((const NMMetaAbstractInfo *) bi, - PROPERTY_INF_FLAG_ENABLED, - PROPERTY_INF_FLAG_ENABLED); - } + _dynamic_options_set((const NMMetaAbstractInfo *) bi, + PROPERTY_INF_FLAG_ENABLED | PROPERTY_INF_FLAG_DISABLED, + PROPERTY_INF_FLAG_ENABLED); } return; } @@ -4196,11 +4194,9 @@ enable_options(const char *setting_name, const char *property, const char *const return; } - if (property_info->inf_flags & NM_META_PROPERTY_INF_FLAG_DONT_ASK) { - _dynamic_options_set((const NMMetaAbstractInfo *) property_info, - PROPERTY_INF_FLAG_ENABLED, - PROPERTY_INF_FLAG_ENABLED); - } + _dynamic_options_set((const NMMetaAbstractInfo *) property_info, + PROPERTY_INF_FLAG_ENABLED | PROPERTY_INF_FLAG_DISABLED, + PROPERTY_INF_FLAG_ENABLED); } /* -- GitLab From 69e65a9b0eec02ee41a3b6f0f6605eed57890106 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 24 Jun 2022 00:30:04 +0200 Subject: [PATCH 07/12] nmcli/connections: make sure the connection has a type We use it before we validate the connection, thus need to check if it's actually there. --- src/nmcli/connections.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index 4d646798de..92613739bb 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -5615,7 +5615,9 @@ connection_get_base_meta_setting_type(NMConnection *connection) const NMMetaSettingInfoEditor *editor; connection_type = nm_connection_get_connection_type(connection); - nm_assert(connection_type); + if (!connection_type) + return NM_META_SETTING_TYPE_UNKNOWN; + base_setting = nm_connection_get_setting_by_name(connection, connection_type); nm_assert(base_setting); editor = nm_meta_setting_info_editor_find_by_setting(base_setting); @@ -5671,10 +5673,15 @@ questionnaire_mandatory(NmCli *nmc, NMConnection *connection) NMMetaSettingType s, base; /* First ask connection properties */ - questionnaire_mandatory_ask_setting(nmc, connection, NM_META_SETTING_TYPE_CONNECTION); + while (1) { + base = connection_get_base_meta_setting_type(connection); + if (base != NM_META_SETTING_TYPE_UNKNOWN) + break; + enable_options(NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE, NULL); + questionnaire_mandatory_ask_setting(nmc, connection, NM_META_SETTING_TYPE_CONNECTION); + } /* Ask properties of the base setting */ - base = connection_get_base_meta_setting_type(connection); questionnaire_mandatory_ask_setting(nmc, connection, base); /* Remaining settings */ -- GitLab From ad7ac866dbf57e1c06bf427f9aa48fea3460dfd4 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 24 Jun 2022 00:30:04 +0200 Subject: [PATCH 08/12] nmcli/connections: don't ask to ask with --ask This is slightly annoying: $ nmcli -a c add type ethernet There is 1 optional setting for General settings. No point in asking if there's just one option. Just ask right away: $ nmcli -a c add type ethernet Interface name: --- src/nmcli/connections.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index 92613739bb..5b167738f7 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -5696,16 +5696,14 @@ want_provide_opt_args(const NmcConfig *nmc_config, const char *type, guint num) { gs_free char *answer = NULL; + /* Don't ask to ask. */ + if (num == 1) + return TRUE; + /* Ask for optional arguments. */ - g_print(ngettext("There is %d optional setting for %s.\n", - "There are %d optional settings for %s.\n", - num), - (int) num, - type); - answer = nmc_readline( - nmc_config, - ngettext("Do you want to provide it? %s", "Do you want to provide them? %s", num), - prompt_yes_no(TRUE, NULL)); + g_print(_("There are %d optional settings for %s.\n"), (int) num, type); + answer = + nmc_readline(nmc_config, _("Do you want to provide them? %s"), prompt_yes_no(TRUE, NULL)); nm_strstrip(answer); return !answer || matches(answer, WORD_YES); } -- GitLab From b171dcec0df459b2e11873de51c95681957eb27f Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 24 Jun 2022 00:30:04 +0200 Subject: [PATCH 09/12] nmcli/connections: use the current value in default in ask_option() For new connections, this ensures the value in square brackets on interactive add are always correct. Apart from that, this allows us to initialize some non-default values before asking (such as making up an interface name for some software devices), and inform the user about what we picked: Interface name [nm-bridge]: --- src/libnmc-setting/nm-meta-setting-desc.c | 75 +++++++++++++---------- src/libnmc-setting/nm-meta-setting-desc.h | 2 +- src/nmcli/connections.c | 17 ++++- 3 files changed, 57 insertions(+), 37 deletions(-) diff --git a/src/libnmc-setting/nm-meta-setting-desc.c b/src/libnmc-setting/nm-meta-setting-desc.c index cc15eab776..0952c016cf 100644 --- a/src/libnmc-setting/nm-meta-setting-desc.c +++ b/src/libnmc-setting/nm-meta-setting-desc.c @@ -4456,7 +4456,8 @@ static const NMMetaNestedPropertyInfo meta_nested_property_infos_bond[] = { .property_name = NM_SETTING_BOND_OPTIONS, .property_alias = "primary", .inf_flags = NM_META_PROPERTY_INF_FLAG_DONT_ASK, - .prompt = N_("Bonding primary interface [none]"), + .prompt = N_("Bonding primary interface"), + .def_hint = "[none]", ) }, { @@ -4472,7 +4473,8 @@ static const NMMetaNestedPropertyInfo meta_nested_property_infos_bond[] = { .property_name = NM_SETTING_BOND_OPTIONS, .property_alias = "miimon", .inf_flags = NM_META_PROPERTY_INF_FLAG_DONT_ASK, - .prompt = N_("Bonding miimon [100]"), + .prompt = N_("Bonding miimon"), + .def_hint = "[100]", ) }, { @@ -4480,7 +4482,8 @@ static const NMMetaNestedPropertyInfo meta_nested_property_infos_bond[] = { .property_name = NM_SETTING_BOND_OPTIONS, .property_alias = "downdelay", .inf_flags = NM_META_PROPERTY_INF_FLAG_DONT_ASK, - .prompt = N_("Bonding downdelay [0]"), + .prompt = N_("Bonding downdelay"), + .def_hint = "[0]", ) }, { @@ -4488,7 +4491,8 @@ static const NMMetaNestedPropertyInfo meta_nested_property_infos_bond[] = { .property_name = NM_SETTING_BOND_OPTIONS, .property_alias = "updelay", .inf_flags = NM_META_PROPERTY_INF_FLAG_DONT_ASK, - .prompt = N_("Bonding updelay [0]"), + .prompt = N_("Bonding updelay"), + .def_hint = "[0]", ) }, { @@ -4496,7 +4500,8 @@ static const NMMetaNestedPropertyInfo meta_nested_property_infos_bond[] = { .property_name = NM_SETTING_BOND_OPTIONS, .property_alias = "arp-interval", .inf_flags = NM_META_PROPERTY_INF_FLAG_DONT_ASK, - .prompt = N_("Bonding arp-interval [0]"), + .prompt = N_("Bonding arp-interval"), + .def_hint = "[0]", ) }, { @@ -4504,7 +4509,8 @@ static const NMMetaNestedPropertyInfo meta_nested_property_infos_bond[] = { .property_name = NM_SETTING_BOND_OPTIONS, .property_alias = "arp-ip-target", .inf_flags = NM_META_PROPERTY_INF_FLAG_DONT_ASK, - .prompt = N_("Bonding arp-ip-target [none]"), + .prompt = N_("Bonding arp-ip-target"), + .def_hint = "[none]", ) }, { @@ -4512,7 +4518,8 @@ static const NMMetaNestedPropertyInfo meta_nested_property_infos_bond[] = { .property_name = NM_SETTING_BOND_OPTIONS, .property_alias = "lacp-rate", .inf_flags = NM_META_PROPERTY_INF_FLAG_DONT_ASK, - .prompt = N_("LACP rate ('slow' or 'fast') [slow]"), + .prompt = N_("LACP rate (slow/fast)"), + .def_hint = "[slow]", ) }, }; @@ -5121,7 +5128,7 @@ static const NMMetaPropertyInfo *const property_infos_BOND_PORT[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_BOND_PORT_QUEUE_ID, .is_cli_option = TRUE, .property_alias = "queue-id", - .prompt = N_("Queue ID [0]"), + .prompt = N_("Queue ID"), .property_type = &_pt_gobject_int, ), NULL @@ -5139,37 +5146,37 @@ static const NMMetaPropertyInfo *const property_infos_BRIDGE[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_STP, .is_cli_option = TRUE, .property_alias = "stp", - .prompt = N_("Enable STP [yes]"), + .prompt = N_("Enable STP"), .property_type = &_pt_gobject_bool, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_PRIORITY, .is_cli_option = TRUE, .property_alias = "priority", - .prompt = N_("STP priority [32768]"), + .prompt = N_("STP priority"), .property_type = &_pt_gobject_int, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_FORWARD_DELAY, .is_cli_option = TRUE, .property_alias = "forward-delay", - .prompt = N_("Forward delay [15]"), + .prompt = N_("Forward delay"), .property_type = &_pt_gobject_int, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_HELLO_TIME, .is_cli_option = TRUE, .property_alias = "hello-time", - .prompt = N_("Hello time [2]"), + .prompt = N_("Hello time"), .property_type = &_pt_gobject_int, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_MAX_AGE, .is_cli_option = TRUE, .property_alias = "max-age", - .prompt = N_("Max age [20]"), + .prompt = N_("Max age"), .property_type = &_pt_gobject_int, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_AGEING_TIME, .is_cli_option = TRUE, .property_alias = "ageing-time", - .prompt = N_("MAC address ageing time [300]"), + .prompt = N_("MAC address ageing time"), .property_type = &_pt_gobject_int, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_GROUP_ADDRESS, @@ -5179,7 +5186,7 @@ static const NMMetaPropertyInfo *const property_infos_BRIDGE[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_GROUP_FORWARD_MASK, .is_cli_option = TRUE, .property_alias = "group-forward-mask", - .prompt = N_("Group forward mask [0]"), + .prompt = N_("Group forward mask"), .property_type = &_pt_gobject_int, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_MULTICAST_HASH_MAX, @@ -5221,7 +5228,7 @@ static const NMMetaPropertyInfo *const property_infos_BRIDGE[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_MULTICAST_SNOOPING, .is_cli_option = TRUE, .property_alias = "multicast-snooping", - .prompt = N_("Enable IGMP snooping [yes]"), + .prompt = N_("Enable IGMP snooping"), .property_type = &_pt_gobject_bool, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_MULTICAST_STARTUP_QUERY_COUNT, @@ -5279,19 +5286,19 @@ static const NMMetaPropertyInfo *const property_infos_BRIDGE_PORT[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_PORT_PRIORITY, .is_cli_option = TRUE, .property_alias = "priority", - .prompt = N_("Bridge port priority [32]"), + .prompt = N_("Bridge port priority"), .property_type = &_pt_gobject_int, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_PORT_PATH_COST, .is_cli_option = TRUE, .property_alias = "path-cost", - .prompt = N_("Bridge port STP path cost [100]"), + .prompt = N_("Bridge port STP path cost"), .property_type = &_pt_gobject_int, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_PORT_HAIRPIN_MODE, .is_cli_option = TRUE, .property_alias = "hairpin", - .prompt = N_("Hairpin [no]"), + .prompt = N_("Hairpin"), .property_type = &_pt_gobject_bool, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_BRIDGE_PORT_VLANS, @@ -5878,7 +5885,7 @@ static const NMMetaPropertyInfo *const property_infos_INFINIBAND[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_INFINIBAND_MTU, .is_cli_option = TRUE, .property_alias = "mtu", - .prompt = N_("MTU [auto]"), + .prompt = N_("MTU"), .property_type = &_pt_gobject_mtu, .property_typ_data = DEFINE_PROPERTY_TYP_DATA_SUBTYPE (mtu, .get_fcn = MTU_GET_FCN (NMSettingInfiniband, nm_setting_infiniband_get_mtu), @@ -6510,7 +6517,7 @@ static const NMMetaPropertyInfo *const property_infos_MACSEC[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_MACSEC_ENCRYPT, .is_cli_option = TRUE, .property_alias = "encrypt", - .prompt = N_("Enable encryption [yes]"), + .prompt = N_("Enable encryption"), .property_type = &_pt_gobject_bool, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_MACSEC_MKA_CAK, @@ -6532,7 +6539,7 @@ static const NMMetaPropertyInfo *const property_infos_MACSEC[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_MACSEC_PORT, .is_cli_option = TRUE, .property_alias = "port", - .prompt = N_("SCI port [1]"), + .prompt = N_("SCI port"), .property_type = &_pt_gobject_int, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_MACSEC_VALIDATION, @@ -6579,7 +6586,7 @@ static const NMMetaPropertyInfo *const property_infos_MACVLAN[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_MACVLAN_TAP, .is_cli_option = TRUE, .property_alias = "tap", - .prompt = N_("Tap [no]"), + .prompt = N_("Tap"), .property_type = &_pt_gobject_bool, ), NULL @@ -6655,7 +6662,7 @@ static const NMMetaPropertyInfo *const property_infos_OLPC_MESH[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_OLPC_MESH_CHANNEL, .is_cli_option = TRUE, .property_alias = "channel", - .prompt = N_("OLPC Mesh channel [1]"), + .prompt = N_("OLPC Mesh channel"), .property_type = DEFINE_PROPERTY_TYPE ( .get_fcn = _get_fcn_gobject, .set_fcn = _set_fcn_olpc_mesh_channel, @@ -6881,7 +6888,7 @@ static const NMMetaPropertyInfo *const property_infos_PROXY[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_PROXY_BROWSER_ONLY, .is_cli_option = TRUE, .property_alias = "browser-only", - .prompt = N_("Browser only [no]"), + .prompt = N_("Browser only"), .property_type = &_pt_gobject_bool ), PROPERTY_INFO_WITH_DESC (NM_SETTING_PROXY_PAC_URL, @@ -7320,19 +7327,19 @@ static const NMMetaPropertyInfo *const property_infos_TUN[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_TUN_PI, .is_cli_option = TRUE, .property_alias = "pi", - .prompt = N_("Enable PI [no]"), + .prompt = N_("Enable PI"), .property_type = &_pt_gobject_bool, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_TUN_VNET_HDR, .is_cli_option = TRUE, .property_alias = "vnet-hdr", - .prompt = N_("Enable VNET header [no]"), + .prompt = N_("Enable VNET header"), .property_type = &_pt_gobject_bool, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_TUN_MULTI_QUEUE, .is_cli_option = TRUE, .property_alias = "multi-queue", - .prompt = N_("Enable multi queue [no]"), + .prompt = N_("Enable multi queue"), .property_type = &_pt_gobject_bool, ), NULL @@ -7458,7 +7465,7 @@ static const NMMetaPropertyInfo *const property_infos_VRF[] = { .is_cli_option = TRUE, .property_alias = "table", .inf_flags = NM_META_PROPERTY_INF_FLAG_REQD, - .prompt = N_("Table [0]"), + .prompt = N_("Table"), .property_type = &_pt_gobject_int, ), NULL @@ -7496,19 +7503,19 @@ static const NMMetaPropertyInfo *const property_infos_VXLAN[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_VXLAN_SOURCE_PORT_MIN, .is_cli_option = TRUE, .property_alias = "source-port-min", - .prompt = N_("Minimum source port [0]"), + .prompt = N_("Minimum source port"), .property_type = &_pt_gobject_int, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_VXLAN_SOURCE_PORT_MAX, .is_cli_option = TRUE, .property_alias = "source-port-max", - .prompt = N_("Maximum source port [0]"), + .prompt = N_("Maximum source port"), .property_type = &_pt_gobject_int, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_VXLAN_DESTINATION_PORT, .is_cli_option = TRUE, .property_alias = "destination-port", - .prompt = N_("Destination port [8472]"), + .prompt = N_("Destination port"), .property_type = &_pt_gobject_int, ), PROPERTY_INFO_WITH_DESC (NM_SETTING_VXLAN_TOS, @@ -7640,7 +7647,7 @@ static const NMMetaPropertyInfo *const property_infos_WIRED[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_WIRED_MTU, .is_cli_option = TRUE, .property_alias = "mtu", - .prompt = N_("MTU [auto]"), + .prompt = N_("MTU"), .property_type = &_pt_gobject_mtu, .property_typ_data = DEFINE_PROPERTY_TYP_DATA_SUBTYPE (mtu, .get_fcn = MTU_GET_FCN (NMSettingWired, nm_setting_wired_get_mtu), @@ -7834,7 +7841,7 @@ static const NMMetaPropertyInfo *const property_infos_WIRELESS[] = { PROPERTY_INFO_WITH_DESC (NM_SETTING_WIRELESS_MTU, .is_cli_option = TRUE, .property_alias = "mtu", - .prompt = N_("MTU [auto]"), + .prompt = N_("MTU"), .property_type = &_pt_gobject_mtu, .property_typ_data = DEFINE_PROPERTY_TYP_DATA_SUBTYPE (mtu, .get_fcn = MTU_GET_FCN (NMSettingWireless, nm_setting_wireless_get_mtu), diff --git a/src/libnmc-setting/nm-meta-setting-desc.h b/src/libnmc-setting/nm-meta-setting-desc.h index b08d4c08c9..ca04457131 100644 --- a/src/libnmc-setting/nm-meta-setting-desc.h +++ b/src/libnmc-setting/nm-meta-setting-desc.h @@ -24,7 +24,7 @@ struct _NMDevice; "(" NM_SETTING_ADSL_ENCAPSULATION_VCMUX "/" NM_SETTING_ADSL_ENCAPSULATION_LLC ") [none]" #define NM_META_TEXT_PROMPT_CON_TYPE N_("Connection type") -#define NM_META_TEXT_PROMPT_IFNAME N_("Interface name [*]") +#define NM_META_TEXT_PROMPT_IFNAME N_("Interface name") #define NM_META_TEXT_PROMPT_VPN_TYPE N_("VPN type") #define NM_META_TEXT_PROMPT_MASTER N_("Master") diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index 5b167738f7..967d1d2482 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -5575,17 +5575,30 @@ ask_option(NmCli *nmc, NMConnection *connection, const NMMetaAbstractInfo *abstr GError *error = NULL; gs_free char *prompt = NULL; gboolean multi; + const char *setting_name, *property_name; const char *opt_prompt, *opt_def_hint; + gs_free char *def_hint = NULL; + gs_free char *property_val = NULL; NMMetaPropertyInfFlags inf_flags; + NMSetting *setting; _meta_abstract_get(abstract_info, NULL, - NULL, - NULL, + &setting_name, + &property_name, NULL, &inf_flags, &opt_prompt, &opt_def_hint); + + if (!opt_def_hint) { + setting = nm_connection_get_setting_by_name(connection, setting_name); + if (setting) + property_val = nmc_setting_get_property_parsable(setting, property_name, NULL); + if (property_val) + opt_def_hint = def_hint = g_strdup_printf("[%s]", property_val); + } + prompt = g_strjoin("", gettext(opt_prompt), opt_def_hint ? " " : "", opt_def_hint ?: "", ": ", NULL); -- GitLab From e3fa6dfd7f02c648732be811d540aa7dbcf4ffdc Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 24 Jun 2022 00:30:04 +0200 Subject: [PATCH 10/12] nmcli/connections: factor out code run after new connection's type is set After the connection's type is set, some bookkeeping is necessary for the interactive (--ask) mode: appropriate setting need to be added and options enabled. Currently it happens in an option setter; which runs when the "type" options is present on the command line, or the value is set in a response to interactive mode: $ nmcli --ask c add type team $ nmcli --ask c add Connection type: team But not when the property is set directly: $ nmcli --ask c add connection.type team ** nm:ERROR:src/nmcli/connections.c:5648:connection_get_base_meta_setting_type: assertion failed: (base_setting) Bail out! nm:ERROR:src/nmcli/connections.c:5648:connection_get_base_meta_setting_type: assertion failed: (base_setting) Aborted (core dumped) This doesn't fix the issue -- a followup commit (hopefully) will. --- src/nmcli/connections.c | 54 +++++++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 21 deletions(-) diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index 967d1d2482..ae2cd71d61 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -4508,6 +4508,36 @@ gen_func_bond_lacp_rate(const char *text, int state) /*****************************************************************************/ +static gboolean +enable_type_settings_and_options(NMConnection *con, GError **error) +{ + const NMMetaSettingValidPartItem *const *type_settings; + const NMMetaSettingValidPartItem *const *slv_settings; + NMSettingConnection *s_con; + + s_con = nm_connection_get_setting_connection(con); + g_return_val_if_fail(s_con, FALSE); + + if (nm_setting_connection_get_slave_type(s_con)) + enable_options(NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_MASTER, NULL); + + if (NM_IN_STRSET(nm_setting_connection_get_connection_type(s_con), + NM_SETTING_BOND_SETTING_NAME, + NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_BRIDGE_SETTING_NAME, + NM_SETTING_VLAN_SETTING_NAME)) { + disable_options(NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_INTERFACE_NAME); + } + + if (!con_settings(con, &type_settings, &slv_settings, error)) + return FALSE; + + ensure_settings(con, slv_settings); + ensure_settings(con, type_settings); + + return TRUE; +} + static gboolean set_connection_type(NmCli *nmc, NMConnection *con, @@ -4516,10 +4546,8 @@ set_connection_type(NmCli *nmc, gboolean allow_reset, GError **error) { - const NMMetaSettingValidPartItem *const *type_settings; - const NMMetaSettingValidPartItem *const *slv_settings; - GError *local = NULL; - const char *slave_type = NULL; + GError *local = NULL; + const char *slave_type = NULL; value = check_valid_name_toplevel(value, &slave_type, &local); if (!value) { @@ -4544,16 +4572,6 @@ set_connection_type(NmCli *nmc, error)) { return FALSE; } - enable_options(NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_MASTER, NULL); - } - - /* ifname is mandatory for all connection types except virtual ones (bond, team, bridge, vlan) */ - if (NM_IN_STRSET(value, - NM_SETTING_BOND_SETTING_NAME, - NM_SETTING_TEAM_SETTING_NAME, - NM_SETTING_BRIDGE_SETTING_NAME, - NM_SETTING_VLAN_SETTING_NAME)) { - disable_options(NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_INTERFACE_NAME); } if (!set_property(nmc->client, @@ -4565,13 +4583,7 @@ set_connection_type(NmCli *nmc, error)) return FALSE; - if (!con_settings(con, &type_settings, &slv_settings, error)) - return FALSE; - - ensure_settings(con, slv_settings); - ensure_settings(con, type_settings); - - return TRUE; + return enable_type_settings_and_options(con, error); } static gboolean -- GitLab From 647e2553627d38f6a7c254d162cc9ea697c662f1 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 24 Jun 2022 00:30:04 +0200 Subject: [PATCH 11/12] nmcli/connections: make sure the connection has a base setting Do the same bookkeeping as would happen upon setting the "type" option when the connection has a connection.type set upon its addition. Otherwise the --ask mode is sad: $ nmcli --ask c add connection.type team ** nm:ERROR:src/nmcli/connections.c:5648:connection_get_base_meta_setting_type: assertion failed: (base_setting) Bail out! nm:ERROR:src/nmcli/connections.c:5648:connection_get_base_meta_setting_type: assertion failed: (base_setting) Aborted (core dumped) --- src/nmcli/connections.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index ae2cd71d61..3370277ae2 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -5891,6 +5891,12 @@ read_properties: if (nmc->complete) goto finish; + if (!enable_type_settings_and_options(connection, &error)) { + g_string_assign(nmc->return_text, error->message); + nmc->return_value = error->code; + goto finish; + } + /* Now ask user for the rest of the mandatory options. */ if (nmc->ask) questionnaire_mandatory(nmc, connection); -- GitLab From cd2945f223e00200684d1d906142a06106ff3fba Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 24 Jun 2022 00:30:04 +0200 Subject: [PATCH 12/12] nmcli/connections: fix setting ifname with "--ask c add" We almost always do the wrong thing in interactive add: The software devices generally require an interactive name, but we don't insist of asking for them; treating them as optional: $ nmcli -a c add type dummy There is 1 optional setting for General settings. Do you want to provide it? (yes/no) [yes] For some interface types (bridges, bonds, ...) we make up a name, presumably for historical reasons. But we don't give the user an option to modify them: $ nmcli -a c add type bridge There are 9 optional settings for Bridge device. Do you want to provide them? (yes/no) [yes] This fixes the above use cases -- still set the default, but be sure to ask: $ nmcli -a c add type dummy Interface name: $ nmcli -a c add type bridge Interface name [nm-bridge1]: Beautiful. --- src/nmcli/connections.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index 3370277ae2..34012d8622 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -4509,7 +4509,7 @@ gen_func_bond_lacp_rate(const char *text, int state) /*****************************************************************************/ static gboolean -enable_type_settings_and_options(NMConnection *con, GError **error) +enable_type_settings_and_options(NmCli *nmc, NMConnection *con, GError **error) { const NMMetaSettingValidPartItem *const *type_settings; const NMMetaSettingValidPartItem *const *slv_settings; @@ -4522,11 +4522,20 @@ enable_type_settings_and_options(NMConnection *con, GError **error) enable_options(NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_MASTER, NULL); if (NM_IN_STRSET(nm_setting_connection_get_connection_type(s_con), + NM_SETTING_BLUETOOTH_SETTING_NAME, NM_SETTING_BOND_SETTING_NAME, - NM_SETTING_TEAM_SETTING_NAME, NM_SETTING_BRIDGE_SETTING_NAME, - NM_SETTING_VLAN_SETTING_NAME)) { - disable_options(NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_INTERFACE_NAME); + NM_SETTING_DUMMY_SETTING_NAME, + NM_SETTING_OVS_BRIDGE_SETTING_NAME, + NM_SETTING_OVS_PATCH_SETTING_NAME, + NM_SETTING_OVS_PORT_SETTING_NAME, + NM_SETTING_TEAM_SETTING_NAME, + NM_SETTING_VETH_SETTING_NAME, + NM_SETTING_VRF_SETTING_NAME, + NM_SETTING_WIREGUARD_SETTING_NAME)) { + enable_options(NM_SETTING_CONNECTION_SETTING_NAME, + NM_SETTING_CONNECTION_INTERFACE_NAME, + NULL); } if (!con_settings(con, &type_settings, &slv_settings, error)) @@ -4535,6 +4544,9 @@ enable_type_settings_and_options(NMConnection *con, GError **error) ensure_settings(con, slv_settings); ensure_settings(con, type_settings); + /* For some software connection types we generate the interface name for the user. */ + set_default_interface_name(nmc, s_con); + return TRUE; } @@ -4583,7 +4595,7 @@ set_connection_type(NmCli *nmc, error)) return FALSE; - return enable_type_settings_and_options(con, error); + return enable_type_settings_and_options(nmc, con, error); } static gboolean @@ -5891,7 +5903,7 @@ read_properties: if (nmc->complete) goto finish; - if (!enable_type_settings_and_options(connection, &error)) { + if (!enable_type_settings_and_options(nmc, connection, &error)) { g_string_assign(nmc->return_text, error->message); nmc->return_value = error->code; goto finish; @@ -5932,9 +5944,6 @@ read_properties: } } - /* For some software connection types we generate the interface name for the user. */ - set_default_interface_name(nmc, s_con); - /* Now see if there's something optional that needs to be asked for. * Keep asking until there's no more things to ask for. */ do { -- GitLab