colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

nmcli: connection: process port-type, type and controller first

Browse Source 
If the connection is a port we need to set the connection.port-type
property. Usually this property is guessed by nmcli depending on the
connection type or the chosen controller, so it doesn't need to be
specified by the user. However, if it is explicitly set by the user
we should not guess, but just use it.

When we process arguments like "controller" or "type" we call custom
functions like set_connection_controller that will guess the port-type
if needed. By processing port-type first, it will be set in the
connection by the time that these other properties are processed, so they
won't try to guess.

After port-type, process connection.type and connection.controller, as we
are usually capable of deducing the port-type from them. Type needs to
be processed first because some types like bond-slave or ovs-port have
only one possible port-type value so we must not try to guess from the
controller.

Fixes: c5324ed285 ('nmcli: streamline connection addition')
This commit is contained in:
Íñigo Huguet
2025-03-14 07:43:23 +01:00
committed by Íñigo Huguet
parent fcf304bbf1
commit bb850fda0e
3 changed files with 151 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files

181
src/nmcli/connections.c
View File

@@ -5229,19 +5229,101 @@ get_value(const char **value,
return TRUE;
}
static int
_copy_connection_properties(const char ***dst,
const char *const *src,
gboolean invert_match,
const char *const *options_list_match)
{
const char *option;
gboolean match;
int count = 0;
while (*src) {
option = (**src == '+' || **src == '-') ? *src + 1 : *src;
match = _nm_g_strv_contains(options_list_match, option);
match = invert_match ? !match : match;
if (match) {
*((*dst)++) = src[0];
*((*dst)++) = src[1];
count += 2;
}
src++;
if (*src) /* Might be the NULL termination, already */
src++;
}
return count;
}
gboolean
nmc_process_connection_properties(NmCli *nmc,
NMConnection *connection,
int *argc,
const char *const **argv,
int argc,
const char *const *argv,
gboolean allow_setting_removal,
GError **error)
{
gs_free const char **to_free = NULL;
if (argc == 0)
return TRUE;
/* First check if we have a port-type, as this would mean we will not
* have ip properties but possibly others, port-type specific.
* Then check connection.type and connection.controller, as port-type might
* be deduced from them.
* Don't reorder if we are doing CLI argument completion, as it might give
* unexpected results
*/
if (!nmc->complete) {
const char **dst;
dst = to_free = g_new(const char *, argc + 1);
argc = _copy_connection_properties(
&dst,
argv,
FALSE,
NM_MAKE_STRV(NM_SETTING_CONNECTION_SETTING_NAME "." NM_SETTING_CONNECTION_PORT_TYPE,
"port-type", /* alias */
NM_SETTING_CONNECTION_SETTING_NAME "." NM_SETTING_CONNECTION_SLAVE_TYPE,
"slave-type" /* alias */));
argc += _copy_connection_properties(&dst,
argv,
FALSE,
NM_MAKE_STRV(NM_SETTING_CONNECTION_SETTING_NAME
"." NM_SETTING_CONNECTION_TYPE,
"type" /* alias */));
argc += _copy_connection_properties(
&dst,
argv,
FALSE,
NM_MAKE_STRV(NM_SETTING_CONNECTION_SETTING_NAME "." NM_SETTING_CONNECTION_CONTROLLER,
"controller", /* alias */
NM_SETTING_CONNECTION_SETTING_NAME "." NM_SETTING_CONNECTION_MASTER,
"master" /* alias */));
argc += _copy_connection_properties(
&dst,
argv,
TRUE,
NM_MAKE_STRV(NM_SETTING_CONNECTION_SETTING_NAME "." NM_SETTING_CONNECTION_PORT_TYPE,
"port-type",
NM_SETTING_CONNECTION_SETTING_NAME "." NM_SETTING_CONNECTION_SLAVE_TYPE,
"slave-type",
NM_SETTING_CONNECTION_SETTING_NAME "." NM_SETTING_CONNECTION_TYPE,
"type",
NM_SETTING_CONNECTION_SETTING_NAME "." NM_SETTING_CONNECTION_CONTROLLER,
"controller",
NM_SETTING_CONNECTION_SETTING_NAME "." NM_SETTING_CONNECTION_MASTER,
"master"));
*dst = NULL; /* NULL terminated as expected by get_value() */
argv = to_free;
}
/* Go through arguments and set properties */
do {
while (argc) {
const NMMetaSettingValidPartItem *const *type_settings;
const NMMetaSettingValidPartItem *const *port_settings;
NMMetaAccessorModifier modifier;
@@ -5260,19 +5342,10 @@ nmc_process_connection_properties(NmCli *nmc,
ensure_settings(connection, port_settings);
ensure_settings(connection, type_settings);
if (*argc <= 0) {
g_set_error_literal(error,
NMCLI_ERROR,
NMC_RESULT_ERROR_USER_INPUT,
_("Error: <setting>.<property> argument is missing."));
return FALSE;
}
nm_assert(argv);
nm_assert(*argv);
nm_assert(**argv);
option_orig = **argv;
option_orig = *argv;
switch (option_orig[0]) {
case '+':
@@ -5294,15 +5367,15 @@ nmc_process_connection_properties(NmCli *nmc,
NMSetting *ss;
const char *setting_name;
(*argc)--;
(*argv)++;
argc--;
argv++;
if (*argc == 1 && nmc->complete) {
if (argc == 1 && nmc->complete) {
complete_existing_setting(nmc, connection, value);
return TRUE;
break;
}
if (!*argc) {
if (!argc) {
g_set_error_literal(error,
NMCLI_ERROR,
NMC_RESULT_ERROR_USER_INPUT,
@@ -5310,9 +5383,9 @@ nmc_process_connection_properties(NmCli *nmc,
return FALSE;
}
setting_name = **argv;
(*argc)--;
(*argv)++;
setting_name = *argv;
argc--;
argv++;
ss = is_setting_valid(connection, type_settings, port_settings, setting_name);
if (!ss) {
@@ -5342,7 +5415,7 @@ nmc_process_connection_properties(NmCli *nmc,
/* This seems like a <setting>.<property> (such as "connection.id" or "bond.mode"),
* optionally prefixed with "+| or "-". */
if (*argc == 1 && nmc->complete)
if (argc == 1 && nmc->complete)
complete_property_name(nmc, connection, modifier, option_sett, option_prop);
option_sett_expanded =
@@ -5358,14 +5431,14 @@ nmc_process_connection_properties(NmCli *nmc,
return FALSE;
}
(*argc)--;
(*argv)++;
if (!get_value(&value, argc, argv, option_orig, error))
argc--;
argv++;
if (!get_value(&value, &argc, &argv, option_orig, error))
return FALSE;
if (!*argc && nmc->complete) {
if (!argc && nmc->complete) {
complete_property(nmc, option_sett, option_prop, value ?: "", connection);
return TRUE;
break;
}
if (!set_property(nmc->client,
@@ -5447,7 +5520,7 @@ nmc_process_connection_properties(NmCli *nmc,
}
if (!chosen) {
if (*argc == 1 && nmc->complete) {
if (argc == 1 && nmc->complete) {
if (allow_setting_removal && g_str_has_prefix("remove", option))
nmc_print("remove\n");
complete_property_name(nmc, connection, modifier, option, NULL);
@@ -5460,21 +5533,20 @@ nmc_process_connection_properties(NmCli *nmc,
return FALSE;
}
if (*argc == 1 && nmc->complete)
if (argc == 1 && nmc->complete)
complete_property_name(nmc, connection, modifier, option, NULL);
(*argc)--;
(*argv)++;
if (!get_value(&value, argc, argv, option_orig, error))
argc--;
argv++;
if (!get_value(&value, &argc, &argv, option_orig, error))
return FALSE;
if (!*argc && nmc->complete)
if (!argc && nmc->complete)
complete_option(nmc, chosen, value ?: "", connection);
if (!set_option(nmc, connection, chosen, value, TRUE, error))
return FALSE;
} while (*argc);
}
return TRUE;
}
@@ -5911,6 +5983,7 @@ nmc_add_connection(NmCli *nmc, NMConnection *connection, gboolean temporary)
static void
do_connection_add(const NMCCommand *cmd, NmCli *nmc, int argc, const char *const *argv)
{
gs_unref_ptrarray GPtrArray *props;
gs_unref_object NMConnection *connection = NULL;
NMSettingConnection *s_con;
gs_free_error GError *error = NULL;
@@ -5929,16 +6002,21 @@ do_connection_add(const NMCCommand *cmd, NmCli *nmc, int argc, const char *const
s_con = (NMSettingConnection *) nm_setting_connection_new();
nm_connection_add_setting(connection, NM_SETTING(s_con));
read_properties:
g_clear_error(&error);
/* Get the arguments from the command line if any */
if (argc && !nmc_process_connection_properties(nmc, connection, &argc, &argv, FALSE, &error)) {
if (nm_streq0(*argv, "--") && !seen_dash_dash) {
props = g_ptr_array_new_full(sizeof(const char *) * (argc + 1), NULL);
while (argc) {
if (nm_streq0(*argv, "--")) {
/* This is for compatibility with older nmcli that required
* options and properties to be separated with "--" */
if (seen_dash_dash) {
g_string_printf(nmc->return_text,
_("Error: argument '--' can only be passed once"));
nmc->return_value = NMC_RESULT_ERROR_USER_INPUT;
goto finish;
} else {
seen_dash_dash = TRUE;
next_arg(nmc, &argc, &argv, NULL);
goto read_properties;
}
} else if (nm_streq0(*argv, "save")) {
/* It would be better if "save" was a separate argument and not
* mixed with properties, but there's not much we can do about it now. */
@@ -5951,16 +6029,31 @@ read_properties:
nmc->return_value = NMC_RESULT_ERROR_USER_INPUT;
goto finish;
}
g_clear_error(&error);
if (!nmc_string_to_bool(*argv, &save_bool, &error)) {
g_string_printf(nmc->return_text, _("Error: 'save': %s."), error->message);
nmc->return_value = NMC_RESULT_ERROR_USER_INPUT;
goto finish;
}
next_arg(nmc, &argc, &argv, NULL);
goto read_properties;
} else {
g_ptr_array_add(props, (gpointer) *argv);
argc--;
argv++;
if (argc > 0) {
g_ptr_array_add(props, (gpointer) *argv);
argc--;
argv++;
}
}
}
g_ptr_array_add(props, NULL); /* Must be NULL terminated */
if (!nmc_process_connection_properties(nmc,
connection,
props->len - 1,
(const char *const *) props->pdata,
FALSE,
&error)) {
g_string_assign(nmc->return_text, error->message);
nmc->return_value = error->code;
goto finish;
@@ -9251,7 +9344,7 @@ do_connection_modify(const NMCCommand *cmd, NmCli *nmc, int argc, const char *co
/* Don't insist on having argument if we're running in offline mode. */
if (!nmc->nmc_config.offline || argc > 0) {
if (!nmc_process_connection_properties(nmc, connection, &argc, &argv, TRUE, &error)) {
if (!nmc_process_connection_properties(nmc, connection, argc, argv, TRUE, &error)) {
g_string_assign(nmc->return_text, error->message);
nmc->return_value = error->code;
return;

4
src/nmcli/connections.h
View File

@@ -14,8 +14,8 @@ const char *nmc_connection_check_deprecated(NMConnection *c);
gboolean nmc_process_connection_properties(NmCli *nmc,
NMConnection *connection,
int *argc,
const char *const **argv,
int argc,
const char *const *args,
gboolean allow_remove_setting,
GError **error);

2
src/nmcli/devices.c
View File

@@ -2596,7 +2596,7 @@ modify_get_applied_cb(GObject *object, GAsyncResult *result, gpointer user_data)
argc = info->argc;
argv = (const char *const *) info->argv;
if (!nmc_process_connection_properties(info->nmc, connection, &argc, &argv, TRUE, &error)) {
if (!nmc_process_connection_properties(info->nmc, connection, argc, argv, TRUE, &error)) {
g_string_assign(nmc->return_text, error->message);
nmc->return_value = error->code;
quit();