From 90c02cafdc7a6076bb6a07dc19f29eef768ca63f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Klime=C5=A1?= Date: Tue, 11 Mar 2014 12:10:00 +0100 Subject: [PATCH 1/2] cli: allow specifying 'master' for slaves as connection ID (rh #1057494) nmcli con add type *-slave ifname em1 master 'master' property of 'connection' setting has to be either interface name or connection UUID of master connection. However, to make nmcli more convenient for users, we also allow specifying connection name and translating it to UUID automatically. https://bugzilla.redhat.com/show_bug.cgi?id=1057494 --- cli/src/connections.c | 81 ++++++++++++++++++++++++++++++++++++------- man/nmcli.1.in | 6 ++-- 2 files changed, 72 insertions(+), 15 deletions(-) diff --git a/cli/src/connections.c b/cli/src/connections.c index 011a4031b3..20451e8727 100644 --- a/cli/src/connections.c +++ b/cli/src/connections.c @@ -386,9 +386,9 @@ usage_connection_add (void) " [updelay ]\n" " [arp-interval ]\n" " [arp-ip-target ]\n\n" - " bond-slave: master \n\n" + " bond-slave: master \n\n" " team: [config |]\n\n" - " team-slave: master \n" + " team-slave: master \n" " [config |]\n\n" " bridge: [stp yes|no]\n" " [priority ]\n" @@ -396,7 +396,7 @@ usage_connection_add (void) " [hello-time <1-10>]\n" " [max-age <6-40>]\n" " [ageing-time <0-1000000>]\n\n" - " bridge-slave: master \n" + " bridge-slave: master \n" " [priority <0-63>]\n" " [path-cost <1-65535>]\n" " [hairpin yes|no]\n\n" @@ -2743,6 +2743,54 @@ unique_master_iface_ifname (GSList *list, return new_name; } +/* verify_master_for_slave: + * @connections: list af all connections + * @master: UUID, ifname or ID of the master connection + * @type: virtual connection type (bond, team, bridge, ...) + * + * Check whether master is a valid interface name, UUID or ID of some @type connection. + * First UUID and ifname are checked. If they don't match, ID is checked + * and replaced by UUID on a match. + * + * Returns: identifier of master connection if found, %NULL otherwise + */ +static const char * +verify_master_for_slave (GSList *connections, + const char *master, + const char *type) +{ + NMConnection *connection; + NMSettingConnection *s_con; + const char *con_type, *id, *uuid, *ifname; + GSList *iterator = connections; + const char *found_by_id = NULL; + const char *out_master = NULL; + + while (iterator) { + connection = NM_CONNECTION (iterator->data); + s_con = nm_connection_get_setting_connection (connection); + g_assert (s_con); + con_type = nm_setting_connection_get_connection_type (s_con); + if (g_strcmp0 (con_type, type) != 0) { + iterator = g_slist_next (iterator); + continue; + } + id = nm_connection_get_id (connection); + uuid = nm_connection_get_uuid (connection); + ifname = nm_connection_get_virtual_iface_name (connection); + if ( g_strcmp0 (master, uuid) == 0 + || g_strcmp0 (master, ifname) == 0) { + out_master = master; + break; + } + if (!found_by_id && g_strcmp0 (master, id) == 0) + found_by_id = uuid; + + iterator = g_slist_next (iterator); + } + return out_master ? out_master : found_by_id; +} + static gboolean bridge_prop_string_to_uint (const char *str, const char *nmc_arg, @@ -4343,6 +4391,7 @@ cleanup_bond: /* Build up the settings required for 'bond-slave' */ const char *master = NULL; char *master_ask = NULL; + const char *checked_master = NULL; const char *type = NULL; nmc_arg_t exp_args[] = { {"master", TRUE, &master, !ask}, {"type", TRUE, &type, FALSE}, @@ -4358,6 +4407,10 @@ cleanup_bond: _("Error: 'master' is required.")); return FALSE; } + /* Verify master argument */ + checked_master = verify_master_for_slave (all_connections, master, NM_SETTING_BOND_SETTING_NAME); + if (!checked_master) + printf (_("Warning: master='%s' doesn't refer to any existing profile.\n"), master); if (type) printf (_("Warning: 'type' is currently ignored. " @@ -4366,7 +4419,7 @@ cleanup_bond: /* Change properties in 'connection' setting */ g_object_set (s_con, NM_SETTING_CONNECTION_TYPE, NM_SETTING_WIRED_SETTING_NAME, - NM_SETTING_CONNECTION_MASTER, master, + NM_SETTING_CONNECTION_MASTER, checked_master ? checked_master : master, NM_SETTING_CONNECTION_SLAVE_TYPE, NM_SETTING_BOND_SETTING_NAME, NULL); @@ -4431,6 +4484,7 @@ cleanup_team: gboolean success = FALSE; const char *master = NULL; char *master_ask = NULL; + const char *checked_master = NULL; const char *type = NULL; const char *config_c = NULL; char *config = NULL; @@ -4450,6 +4504,10 @@ cleanup_team: _("Error: 'master' is required.")); return FALSE; } + /* Verify master argument */ + checked_master = verify_master_for_slave (all_connections, master, NM_SETTING_TEAM_SETTING_NAME); + if (!checked_master) + printf (_("Warning: master='%s' doesn't refer to any existing profile.\n"), master); /* Also ask for all optional arguments if '--ask' is specified. */ config = g_strdup (config_c); @@ -4475,7 +4533,7 @@ cleanup_team: /* Change properties in 'connection' setting */ g_object_set (s_con, NM_SETTING_CONNECTION_TYPE, NM_SETTING_WIRED_SETTING_NAME, - NM_SETTING_CONNECTION_MASTER, master, + NM_SETTING_CONNECTION_MASTER, checked_master ? checked_master : master, NM_SETTING_CONNECTION_SLAVE_TYPE, NM_SETTING_TEAM_SETTING_NAME, NULL); @@ -4611,6 +4669,7 @@ cleanup_bridge: gboolean success = FALSE; const char *master = NULL; char *master_ask = NULL; + const char *checked_master = NULL; const char *type = NULL; const char *priority_c = NULL; char *priority = NULL; @@ -4637,12 +4696,10 @@ cleanup_bridge: _("Error: 'master' is required.")); return FALSE; } - if (!nm_utils_is_uuid (master) && !nm_utils_iface_valid_name (master)) { - g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, - _("Error: 'master': '%s' is not valid UUID nor interface."), - master); - goto cleanup_bridge_slave; - } + /* Verify master argument */ + checked_master = verify_master_for_slave (all_connections, master, NM_SETTING_BRIDGE_SETTING_NAME); + if (!checked_master) + printf (_("Warning: master='%s' doesn't refer to any existing profile.\n"), master); if (type) printf (_("Warning: 'type' is currently ignored. " @@ -4681,7 +4738,7 @@ cleanup_bridge: /* Change properties in 'connection' setting */ g_object_set (s_con, NM_SETTING_CONNECTION_TYPE, NM_SETTING_WIRED_SETTING_NAME, - NM_SETTING_CONNECTION_MASTER, master, + NM_SETTING_CONNECTION_MASTER, checked_master ? checked_master : master, NM_SETTING_CONNECTION_SLAVE_TYPE, NM_SETTING_BRIDGE_SETTING_NAME, NULL); diff --git a/man/nmcli.1.in b/man/nmcli.1.in index 0292fa5257..bc3b942cf1 100644 --- a/man/nmcli.1.in +++ b/man/nmcli.1.in @@ -521,7 +521,7 @@ Note: use quotes around \fB*\fP to suppress shell expansion. .RS .TP .B bond-slave: -.IP "\fImaster \fP" 42 +.IP "\fImaster \fP" 42 \(en name of bond master interface .RE .RS @@ -533,7 +533,7 @@ Note: use quotes around \fB*\fP to suppress shell expansion. .RS .TP .B team-slave: -.IP "\fImaster \fP" 42 +.IP "\fImaster \fP" 42 \(en name of team master interface .IP "\fI[config |]\fP" 42 \(en JSON configuration for team @@ -557,7 +557,7 @@ Note: use quotes around \fB*\fP to suppress shell expansion. .RS .TP .B bridge-slave: -.IP "\fImaster \fP" 42 +.IP "\fImaster \fP" 42 \(en name of bridge master interface .IP "\fI[priority <0-63>]\fP" 42 \(en STP priority of this slave (default: 32) From d7e1ec9183a10d303dcb583c29ebc7cd4b7858e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Klime=C5=A1?= Date: Tue, 11 Mar 2014 13:21:50 +0100 Subject: [PATCH 2/2] cli: accept prefix "ifname/", "uuid/" or "id/" for 'master' argument nmcli con add type team-slave ifname em2 master team-master0 nmcli con add type team-slave ifname em2 master id/team-master0 It helps to disambiguate values for cases where they may overlap, e.g. "team0" -> "ifname/team0" or "id/team0" --- cli/src/connections.c | 62 ++++++++++++++++++++++++++++++++++--------- man/nmcli.1.in | 9 ++++--- 2 files changed, 56 insertions(+), 15 deletions(-) diff --git a/cli/src/connections.c b/cli/src/connections.c index 20451e8727..d31bff8f69 100644 --- a/cli/src/connections.c +++ b/cli/src/connections.c @@ -2743,6 +2743,28 @@ unique_master_iface_ifname (GSList *list, return new_name; } +static const char * +_strip_master_prefix (const char *master, const char *(**func)(NMConnection *)) +{ + if (!master) + return NULL; + + if (g_str_has_prefix (master, "ifname/")) { + master = master + strlen ("ifname/"); + if (func) + *func = nm_connection_get_virtual_iface_name; + } else if (g_str_has_prefix (master, "uuid/")) { + master = master + strlen ("uuid/"); + if (func) + *func = nm_connection_get_uuid; + } else if (g_str_has_prefix (master, "id/")) { + master = master + strlen ("id/"); + if (func) + *func = nm_connection_get_id; + } + return master; +} + /* verify_master_for_slave: * @connections: list af all connections * @master: UUID, ifname or ID of the master connection @@ -2765,7 +2787,12 @@ verify_master_for_slave (GSList *connections, GSList *iterator = connections; const char *found_by_id = NULL; const char *out_master = NULL; + const char *(*func) (NMConnection *) = NULL; + if (!master) + return NULL; + + master = _strip_master_prefix (master, &func); while (iterator) { connection = NM_CONNECTION (iterator->data); s_con = nm_connection_get_setting_connection (connection); @@ -2775,16 +2802,27 @@ verify_master_for_slave (GSList *connections, iterator = g_slist_next (iterator); continue; } - id = nm_connection_get_id (connection); - uuid = nm_connection_get_uuid (connection); - ifname = nm_connection_get_virtual_iface_name (connection); - if ( g_strcmp0 (master, uuid) == 0 - || g_strcmp0 (master, ifname) == 0) { - out_master = master; - break; + if (func) { + /* There was a prefix; only compare to that type. */ + if (g_strcmp0 (master, func (connection)) == 0) { + if (func == nm_connection_get_id) + out_master = nm_connection_get_uuid (connection); + else + out_master = master; + break; + } + } else { + id = nm_connection_get_id (connection); + uuid = nm_connection_get_uuid (connection); + ifname = nm_connection_get_virtual_iface_name (connection); + if ( g_strcmp0 (master, uuid) == 0 + || g_strcmp0 (master, ifname) == 0) { + out_master = master; + break; + } + if (!found_by_id && g_strcmp0 (master, id) == 0) + found_by_id = uuid; } - if (!found_by_id && g_strcmp0 (master, id) == 0) - found_by_id = uuid; iterator = g_slist_next (iterator); } @@ -4419,7 +4457,7 @@ cleanup_bond: /* Change properties in 'connection' setting */ g_object_set (s_con, NM_SETTING_CONNECTION_TYPE, NM_SETTING_WIRED_SETTING_NAME, - NM_SETTING_CONNECTION_MASTER, checked_master ? checked_master : master, + NM_SETTING_CONNECTION_MASTER, checked_master ? checked_master : _strip_master_prefix (master, NULL), NM_SETTING_CONNECTION_SLAVE_TYPE, NM_SETTING_BOND_SETTING_NAME, NULL); @@ -4533,7 +4571,7 @@ cleanup_team: /* Change properties in 'connection' setting */ g_object_set (s_con, NM_SETTING_CONNECTION_TYPE, NM_SETTING_WIRED_SETTING_NAME, - NM_SETTING_CONNECTION_MASTER, checked_master ? checked_master : master, + NM_SETTING_CONNECTION_MASTER, checked_master ? checked_master : _strip_master_prefix (master, NULL), NM_SETTING_CONNECTION_SLAVE_TYPE, NM_SETTING_TEAM_SETTING_NAME, NULL); @@ -4738,7 +4776,7 @@ cleanup_bridge: /* Change properties in 'connection' setting */ g_object_set (s_con, NM_SETTING_CONNECTION_TYPE, NM_SETTING_WIRED_SETTING_NAME, - NM_SETTING_CONNECTION_MASTER, checked_master ? checked_master : master, + NM_SETTING_CONNECTION_MASTER, checked_master ? checked_master : _strip_master_prefix (master, NULL), NM_SETTING_CONNECTION_SLAVE_TYPE, NM_SETTING_BRIDGE_SETTING_NAME, NULL); diff --git a/man/nmcli.1.in b/man/nmcli.1.in index bc3b942cf1..6eb6f1fac0 100644 --- a/man/nmcli.1.in +++ b/man/nmcli.1.in @@ -522,7 +522,8 @@ Note: use quotes around \fB*\fP to suppress shell expansion. .TP .B bond-slave: .IP "\fImaster \fP" 42 -\(en name of bond master interface +\(en master bond interface name, or connection UUID or ID of bond master connection profile. +The value can be prefixed with \fBifname/\fP, \fBuuid/\fP or \fBid/\fP to disambiguate it. .RE .RS .TP @@ -534,7 +535,8 @@ Note: use quotes around \fB*\fP to suppress shell expansion. .TP .B team-slave: .IP "\fImaster \fP" 42 -\(en name of team master interface +\(en master team interface name, or connection UUID or ID of team master connection profile. +The value can be prefixed with \fBifname/\fP, \fBuuid/\fP or \fBid/\fP to disambiguate it. .IP "\fI[config |]\fP" 42 \(en JSON configuration for team .RE @@ -558,7 +560,8 @@ Note: use quotes around \fB*\fP to suppress shell expansion. .TP .B bridge-slave: .IP "\fImaster \fP" 42 -\(en name of bridge master interface +\(en master bridge interface name, or connection UUID or ID of bridge master connection profile. +The value can be prefixed with \fBifname/\fP, \fBuuid/\fP or \fBid/\fP to disambiguate it. .IP "\fI[priority <0-63>]\fP" 42 \(en STP priority of this slave (default: 32) .IP "\fI[path-cost <1-65535>]\fP" 42