Commit 2deaa539 authored by Thomas Haller's avatar Thomas Haller

libnm-util: normalize virtual_iface_name in NMSettings

Some type-specific NMSetting implementations (bond, bridge, team, vlan)
have their own 'interface-name' property. This property will be
deprecated in favour of 'interface-name' in NMSettingConnection.

Change verify() and normalize() to check that the redundant
values match and repair/normalize the properties.

Force the virtual interface name of the type-specific setting to be
equal to NMSettingConnection:interface_name. This way, the depreacted
field stays valid and backward compatible.

NMSettingInfiniband is special, because it does not have a backing
property for the interface name, although it implements
get_virtual_iface_name(). To account for this, some special handling
is needed in order not to change the behaviour of get_virtual_iface_name().
Signed-off-by: Thomas Haller's avatarThomas Haller <thaller@redhat.com>
parent de5656a5
......@@ -579,6 +579,58 @@ nm_connection_diff (NMConnection *a,
return *out_settings ? FALSE : TRUE;
}
static gboolean
_normalize_virtual_iface_name (NMConnection *self)
{
NMConnectionPrivate *priv = NM_CONNECTION_GET_PRIVATE (self);
GHashTableIter h_iter;
NMSetting *setting;
NMSettingConnection *s_con;
const char *interface_name;
char *virtual_iface_name = NULL;
gboolean was_modified = FALSE;
const char *prop_name = NULL;
/* search for settings that might need normalization of the interface name. */
g_hash_table_iter_init (&h_iter, priv->settings);
while ( !prop_name
&& g_hash_table_iter_next (&h_iter, NULL, (void **) &setting)) {
if (NM_IS_SETTING_BOND (setting))
prop_name = NM_SETTING_BOND_INTERFACE_NAME;
else if (NM_IS_SETTING_BRIDGE (setting))
prop_name = NM_SETTING_BRIDGE_INTERFACE_NAME;
else if (NM_IS_SETTING_TEAM (setting))
prop_name = NM_SETTING_TEAM_INTERFACE_NAME;
else if (NM_IS_SETTING_VLAN (setting))
prop_name = NM_SETTING_VLAN_INTERFACE_NAME;
}
if (!prop_name)
return FALSE;
s_con = nm_connection_get_setting_connection (self);
g_return_val_if_fail (s_con, FALSE);
interface_name = nm_setting_connection_get_interface_name (s_con);
/* read the potential virtual_iface_name from the setting. */
g_object_get (setting, prop_name, &virtual_iface_name, NULL);
if (g_strcmp0 (interface_name, virtual_iface_name) != 0) {
if (interface_name) {
/* interface_name is set and overwrites the virtual_iface_name. */
g_object_set (setting, prop_name, interface_name, NULL);
} else {
/* interface in NMSettingConnection must be set. */
g_object_set (s_con, NM_SETTING_CONNECTION_INTERFACE_NAME, virtual_iface_name, NULL);
}
was_modified = TRUE;
}
g_free (virtual_iface_name);
return was_modified;
}
/**
* nm_connection_verify:
* @connection: the #NMConnection to verify
......@@ -792,7 +844,7 @@ nm_connection_normalize (NMConnection *connection,
* We only do this, after verifying that the connection contains no un-normalizable
* errors, because in that case we rather fail without touching the settings. */
/* TODO: no normalizations implemented yet */
was_modified |= _normalize_virtual_iface_name (connection);
/* Verify anew. */
success = _nm_connection_verify (connection, error);
......
......@@ -491,24 +491,6 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
const char *arp_ip_target = NULL;
const char *primary;
if (!priv->interface_name || !strlen(priv->interface_name)) {
g_set_error_literal (error,
NM_SETTING_BOND_ERROR,
NM_SETTING_BOND_ERROR_MISSING_PROPERTY,
_("property is missing"));
g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_INTERFACE_NAME);
return FALSE;
}
if (!nm_utils_iface_valid_name (priv->interface_name)) {
g_set_error_literal (error,
NM_SETTING_BOND_ERROR,
NM_SETTING_BOND_ERROR_INVALID_PROPERTY,
_("property is invalid"));
g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_INTERFACE_NAME);
return FALSE;
}
g_hash_table_iter_init (&iter, priv->options);
while (g_hash_table_iter_next (&iter, (gpointer) &key, (gpointer) &value)) {
if (!value[0] || !nm_setting_bond_validate_option (key, value)) {
......@@ -688,7 +670,13 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
}
}
return TRUE;
return _nm_setting_verify_deprecated_virtual_iface_name (
priv->interface_name, FALSE,
NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_INTERFACE_NAME,
NM_SETTING_BOND_ERROR,
NM_SETTING_BOND_ERROR_INVALID_PROPERTY,
NM_SETTING_BOND_ERROR_MISSING_PROPERTY,
all_settings, error);
}
static const char *
......
......@@ -280,25 +280,6 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
{
NMSettingBridgePrivate *priv = NM_SETTING_BRIDGE_GET_PRIVATE (setting);
if (!priv->interface_name || !strlen(priv->interface_name)) {
g_set_error_literal (error,
NM_SETTING_BRIDGE_ERROR,
NM_SETTING_BRIDGE_ERROR_MISSING_PROPERTY,
_("property is missing"));
g_prefix_error (error, "%s.%s: ", NM_SETTING_BRIDGE_SETTING_NAME, NM_SETTING_BRIDGE_INTERFACE_NAME);
return FALSE;
}
if (!nm_utils_iface_valid_name (priv->interface_name)) {
g_set_error (error,
NM_SETTING_BRIDGE_ERROR,
NM_SETTING_BRIDGE_ERROR_INVALID_PROPERTY,
_("'%s' is not a valid interface name"),
priv->interface_name);
g_prefix_error (error, "%s.%s: ", NM_SETTING_BRIDGE_SETTING_NAME, NM_SETTING_BRIDGE_INTERFACE_NAME);
return FALSE;
}
if (priv->mac_address && priv->mac_address->len != ETH_ALEN) {
g_set_error_literal (error,
NM_SETTING_BRIDGE_ERROR,
......@@ -336,7 +317,13 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
error))
return FALSE;
return TRUE;
return _nm_setting_verify_deprecated_virtual_iface_name (
priv->interface_name, FALSE,
NM_SETTING_BRIDGE_SETTING_NAME, NM_SETTING_BRIDGE_INTERFACE_NAME,
NM_SETTING_BRIDGE_ERROR,
NM_SETTING_BRIDGE_ERROR_INVALID_PROPERTY,
NM_SETTING_BRIDGE_ERROR_MISSING_PROPERTY,
all_settings, error);
}
static const char *
......
......@@ -796,29 +796,35 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
return FALSE;
}
/* If the connection has a virtual interface name, it must match
* the connection setting's interface name.
/* FIXME: previously, verify() set the NMSettingConnection:interface_name property,
* thus modifying the setting. verify() should not do this, but keep this not to change
* behaviour.
*/
for (iter = all_settings; iter; iter = iter->next) {
const char *virtual_iface;
virtual_iface = nm_setting_get_virtual_iface_name (iter->data);
if (virtual_iface) {
if (priv->interface_name) {
if (strcmp (priv->interface_name, virtual_iface) != 0) {
g_set_error (error,
NM_SETTING_CONNECTION_ERROR,
NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY,
_("'%s' doesn't match the virtual interface name '%s'"),
priv->interface_name, virtual_iface);
g_prefix_error (error, "%s.%s: ",
NM_SETTING_CONNECTION_SETTING_NAME,
NM_SETTING_CONNECTION_INTERFACE_NAME);
return FALSE;
if (!priv->interface_name) {
for (iter = all_settings; iter; iter = iter->next) {
NMSetting *s_current = iter->data;
char *virtual_iface_name = NULL;
if (NM_IS_SETTING_BOND (s_current))
g_object_get (s_current, NM_SETTING_BOND_INTERFACE_NAME, &virtual_iface_name, NULL);
else if (NM_IS_SETTING_BRIDGE (s_current))
g_object_get (s_current, NM_SETTING_BRIDGE_INTERFACE_NAME, &virtual_iface_name, NULL);
else if (NM_IS_SETTING_TEAM (s_current))
g_object_get (s_current, NM_SETTING_TEAM_INTERFACE_NAME, &virtual_iface_name, NULL);
else if (NM_IS_SETTING_VLAN (s_current))
g_object_get (s_current, NM_SETTING_VLAN_INTERFACE_NAME, &virtual_iface_name, NULL);
/* For NMSettingInfiniband, virtual_iface_name has no backing field.
* No need to set the (unset) interface_name to the default value.
**/
if (virtual_iface_name) {
if (nm_utils_iface_valid_name (virtual_iface_name)) {
/* found a new interface name. */
priv->interface_name = virtual_iface_name;
break;
}
} else
priv->interface_name = g_strdup (virtual_iface);
break;
g_free (virtual_iface_name);
}
}
}
......
......@@ -194,6 +194,7 @@ get_virtual_iface_name (NMSetting *setting)
static gboolean
verify (NMSetting *setting, GSList *all_settings, GError **error)
{
NMSettingConnection *s_con;
NMSettingInfinibandPrivate *priv = NM_SETTING_INFINIBAND_GET_PRIVATE (setting);
if (priv->mac_address && priv->mac_address->len != INFINIBAND_ALEN) {
......@@ -249,6 +250,45 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
}
}
s_con = NM_SETTING_CONNECTION (nm_setting_find_in_list (all_settings, NM_SETTING_CONNECTION_SETTING_NAME));
if (s_con) {
const char *interface_name = nm_setting_connection_get_interface_name (s_con);
if (!interface_name)
;
else if (!nm_utils_iface_valid_name (interface_name)) {
/* report the error for NMSettingConnection:interface-name, because
* it's that property that is invalid -- although we currently verify()
* NMSettingInfiniband.
**/
g_set_error (error,
NM_SETTING_CONNECTION_ERROR,
NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY,
_("'%s' is not a valid interface name"),
interface_name);
g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_INTERFACE_NAME);
return FALSE;
} else {
if (priv->p_key != -1) {
if (!priv->virtual_iface_name)
priv->virtual_iface_name = g_strdup_printf ("%s.%04x", priv->parent, priv->p_key);
if (strcmp (interface_name, priv->virtual_iface_name) != 0) {
/* We don't support renaming software infiniband devices. Later we might, but
* for now just reject such connections.
**/
g_set_error (error,
NM_SETTING_CONNECTION_ERROR,
NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY,
_("interface name of software infiniband device must be '%s' or unset (instead it is '%s')"),
priv->virtual_iface_name, interface_name);
g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_INTERFACE_NAME);
return FALSE;
}
}
}
}
return TRUE;
}
......
......@@ -108,6 +108,16 @@ NMSetting *nm_setting_find_in_list (GSList *settings_list, const char *setting_n
const char *nm_setting_ip4_config_get_address_label (NMSettingIP4Config *setting, guint32 i);
gboolean nm_setting_ip4_config_add_address_with_label (NMSettingIP4Config *setting, NMIP4Address *address, const char *label);
NMSettingVerifyResult _nm_setting_verify_deprecated_virtual_iface_name (const char *interface_name,
gboolean allow_missing,
const char *setting_name,
const char *setting_property,
GQuark error_quark,
gint e_invalid_property,
gint e_missing_property,
GSList *all_settings,
GError **error);
NMSettingVerifyResult _nm_setting_verify (NMSetting *setting,
GSList *all_settings,
GError **error);
......
......@@ -134,25 +134,13 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
{
NMSettingTeamPrivate *priv = NM_SETTING_TEAM_GET_PRIVATE (setting);
if (!priv->interface_name || !strlen(priv->interface_name)) {
g_set_error_literal (error,
NM_SETTING_TEAM_ERROR,
NM_SETTING_TEAM_ERROR_MISSING_PROPERTY,
_("property is missing"));
g_prefix_error (error, "%s.%s: ", NM_SETTING_TEAM_SETTING_NAME, NM_SETTING_TEAM_INTERFACE_NAME);
return FALSE;
}
if (!nm_utils_iface_valid_name (priv->interface_name)) {
g_set_error_literal (error,
NM_SETTING_TEAM_ERROR,
NM_SETTING_TEAM_ERROR_INVALID_PROPERTY,
_("property is invalid"));
g_prefix_error (error, "%s.%s: ", NM_SETTING_TEAM_SETTING_NAME, NM_SETTING_TEAM_INTERFACE_NAME);
return FALSE;
}
return TRUE;
return _nm_setting_verify_deprecated_virtual_iface_name (
priv->interface_name, FALSE,
NM_SETTING_TEAM_SETTING_NAME, NM_SETTING_TEAM_INTERFACE_NAME,
NM_SETTING_TEAM_ERROR,
NM_SETTING_TEAM_ERROR_INVALID_PROPERTY,
NM_SETTING_TEAM_ERROR_MISSING_PROPERTY,
all_settings, error);
}
static const char *
......
......@@ -537,20 +537,6 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
s_wired = iter->data;
}
/* If interface_name is specified, it must be a valid interface name. We
* don't check that it matches parent and/or id, because we allowing
* renaming vlans to arbitrary names.
*/
if (priv->interface_name && !nm_utils_iface_valid_name (priv->interface_name)) {
g_set_error (error,
NM_SETTING_VLAN_ERROR,
NM_SETTING_VLAN_ERROR_INVALID_PROPERTY,
_("'%s' is not a valid interface name"),
priv->interface_name);
g_prefix_error (error, "%s.%s: ", NM_SETTING_VLAN_SETTING_NAME, NM_SETTING_VLAN_INTERFACE_NAME);
return FALSE;
}
if (priv->parent) {
if (nm_utils_is_uuid (priv->parent)) {
/* If we have an NMSettingConnection:master with slave-type="vlan",
......@@ -582,7 +568,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
priv->parent);
g_prefix_error (error, "%s.%s: ", NM_SETTING_VLAN_SETTING_NAME, NM_SETTING_VLAN_PARENT);
return FALSE;
}
}
} else {
/* If parent is NULL, the parent must be specified via
* NMSettingWired:mac-address.
......@@ -609,7 +595,17 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
return FALSE;
}
return TRUE;
/* If interface_name is specified, it must be a valid interface name. We
* don't check that it matches parent and/or id, because we allow
* renaming vlans to arbitrary names.
*/
return _nm_setting_verify_deprecated_virtual_iface_name (
priv->interface_name, TRUE,
NM_SETTING_VLAN_SETTING_NAME, NM_SETTING_VLAN_INTERFACE_NAME,
NM_SETTING_VLAN_ERROR,
NM_SETTING_VLAN_ERROR_INVALID_PROPERTY,
NM_SETTING_VLAN_ERROR_MISSING_PROPERTY,
all_settings, error);
}
static const char *
......
......@@ -24,6 +24,7 @@
*/
#include <string.h>
#include <glib/gi18n.h>
#include "nm-setting.h"
#include "nm-setting-private.h"
......@@ -1270,6 +1271,87 @@ nm_setting_get_virtual_iface_name (NMSetting *setting)
return NULL;
}
NMSettingVerifyResult
_nm_setting_verify_deprecated_virtual_iface_name (const char *interface_name,
gboolean allow_missing,
const char *setting_name,
const char *setting_property,
GQuark error_quark,
gint e_invalid_property,
gint e_missing_property,
GSList *all_settings,
GError **error)
{
NMSettingConnection *s_con;
const char *con_name;
s_con = NM_SETTING_CONNECTION (nm_setting_find_in_list (all_settings, NM_SETTING_CONNECTION_SETTING_NAME));
con_name = s_con ? nm_setting_connection_get_interface_name (s_con) : NULL;
if (!interface_name && !con_name) {
if (allow_missing)
return NM_SETTING_VERIFY_SUCCESS;
g_set_error_literal (error,
NM_SETTING_CONNECTION_ERROR,
NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY,
_("property is missing"));
g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_INTERFACE_NAME);
return NM_SETTING_VERIFY_ERROR;
}
if (!con_name && !nm_utils_iface_valid_name (interface_name)) {
/* the interface_name is invalid, we cannot normalize it. Only do this if !con_name,
* because if con_name is set, it can overwrite interface_name. */
g_set_error_literal (error,
error_quark,
e_invalid_property,
_("property is invalid"));
g_prefix_error (error, "%s.%s: ", setting_name, setting_property);
return NM_SETTING_VERIFY_ERROR;
}
if (!con_name) {
/* NMSettingConnection has interface not set, it should be normalized to interface_name */
g_set_error_literal (error,
NM_SETTING_CONNECTION_ERROR,
NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY,
_("property is missing"));
g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_INTERFACE_NAME);
return NM_SETTING_VERIFY_NORMALIZABLE;
}
if (!nm_utils_iface_valid_name (con_name)) {
/* NMSettingConnection:interface_name is invalid, we cannot normalize it. */
g_set_error_literal (error,
NM_SETTING_CONNECTION_ERROR,
NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY,
_("property is invalid"));
g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_INTERFACE_NAME);
return NM_SETTING_VERIFY_ERROR;
}
if (!interface_name) {
/* Normalize by setting NMSettingConnection:interface_name. */
g_set_error_literal (error,
error_quark,
e_missing_property,
_("property is missing"));
g_prefix_error (error, "%s.%s: ", setting_name, setting_property);
return NM_SETTING_VERIFY_NORMALIZABLE;
}
if (strcmp (con_name, interface_name) != 0) {
/* con_name and interface_name are different. It can be normalized by setting interface_name
* to con_name. */
g_set_error_literal (error,
error_quark,
e_missing_property,
_("property is invalid"));
g_prefix_error (error, "%s.%s: ", setting_name, setting_property);
/* we would like to make this a NORMALIZEABLE_ERROR, but that might
* break older connections. */
return NM_SETTING_VERIFY_NORMALIZABLE;
}
return NM_SETTING_VERIFY_SUCCESS;
}
/*****************************************************************************/
static void
......
......@@ -2527,6 +2527,76 @@ test_connection_verify_sets_interface_name (void)
g_object_unref (con);
}
/*
* Test normalization of interface-name
**/
static void
test_connection_normalize_virtual_iface_name (void)
{
NMConnection *con;
NMSettingConnection *s_con;
NMSettingVlan *s_vlan;
NMSetting *setting;
GError *error = NULL;
gboolean success;
const char *IFACE_NAME = "iface";
const char *IFACE_VIRT = "iface-X";
gboolean modified = FALSE;
con = nm_connection_new ();
setting = nm_setting_ip4_config_new ();
g_object_set (setting,
NM_SETTING_IP4_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_AUTO,
NULL);
nm_connection_add_setting (con, setting);
setting = nm_setting_ip6_config_new ();
g_object_set (setting,
NM_SETTING_IP6_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_AUTO,
NM_SETTING_IP6_CONFIG_MAY_FAIL, TRUE,
NULL);
nm_connection_add_setting (con, setting);
s_con = (NMSettingConnection *) nm_setting_connection_new ();
g_object_set (G_OBJECT (s_con),
NM_SETTING_CONNECTION_ID, "test1",
NM_SETTING_CONNECTION_UUID, "22001632-bbb4-4616-b277-363dce3dfb5b",
NM_SETTING_CONNECTION_TYPE, NM_SETTING_VLAN_SETTING_NAME,
NM_SETTING_CONNECTION_INTERFACE_NAME, IFACE_NAME,
NULL);
s_vlan = (NMSettingVlan *) nm_setting_vlan_new ();
g_object_set (G_OBJECT (s_vlan),
NM_SETTING_VLAN_INTERFACE_NAME, IFACE_VIRT,
NM_SETTING_VLAN_PARENT, "eth0",
NULL);
nm_connection_add_setting (con, NM_SETTING (s_con));
nm_connection_add_setting (con, NM_SETTING (s_vlan));
g_assert_cmpstr (nm_connection_get_interface_name (con), ==, IFACE_NAME);
g_assert_cmpstr (nm_setting_vlan_get_interface_name (s_vlan), ==, IFACE_VIRT);
/* for backward compatiblity, normalizes the interface name */
success = nm_connection_verify (con, &error);
g_assert (success && !error);
g_assert_cmpstr (nm_connection_get_interface_name (con), ==, IFACE_NAME);
g_assert_cmpstr (nm_setting_vlan_get_interface_name (s_vlan), ==, IFACE_VIRT);
success = nm_connection_normalize (con, NULL, &modified, &error);
g_assert (success && !error);
g_assert (modified);
g_assert_cmpstr (nm_connection_get_interface_name (con), ==, IFACE_NAME);
g_assert_cmpstr (nm_setting_vlan_get_interface_name (s_vlan), ==, IFACE_NAME);
success = nm_connection_verify (con, &error);
g_assert (success && !error);
g_object_unref (con);
}
NMTST_DEFINE ();
int main (int argc, char **argv)
......@@ -2565,6 +2635,7 @@ int main (int argc, char **argv)
test_connection_replace_settings_from_connection ();
test_connection_new_from_hash ();
test_connection_verify_sets_interface_name ();
test_connection_normalize_virtual_iface_name ();
test_setting_connection_permissions_helpers ();
test_setting_connection_permissions_property ();
......
......@@ -14,6 +14,7 @@ libnm-util/crypto.c
libnm-util/crypto_gnutls.c
libnm-util/crypto_nss.c
libnm-util/nm-connection.c
libnm-util/nm-setting.c
libnm-util/nm-setting-8021x.c
libnm-util/nm-setting-adsl.c
libnm-util/nm-setting-bluetooth.c
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment