libnm-core: normalize invalid bridge|team slave-port settings

Having a bridge-port/team-port setting for a connection that
has a different slave-type makes no sense. Such a configuration
shall be considered invalid, and be fixed by normalization.

Note that there is already a normalization the other way around,
when you omit the "slave-type" but a "master" and one(!) port-type
setting is present, the slave-type is automatically determined
based on the port-type.

The use of this is of course to modify an existing slave connection
to make it a non-slave. Then the invalid port settings should be
automatically removed.

Previously, ifcfg-rh writer would write the "BRIDGING_OPTS" setting
without a "BRIDGE". The reader would then (correctly) ignore the
bridge-port. Avoid that altogehter, by requiring the connection to
strictly verify.
This commit is contained in:
Thomas Haller 2017-03-01 13:50:59 +01:00
parent e0252e7a75
commit 670e088efe
4 changed files with 77 additions and 17 deletions

View file

@ -132,15 +132,15 @@ nm_connection_add_setting (NMConnection *connection, NMSetting *setting)
* Removes the #NMSetting with the given #GType from the #NMConnection. This
* operation dereferences the #NMSetting object.
**/
void
nm_connection_remove_setting (NMConnection *connection, GType setting_type)
gboolean
_nm_connection_remove_setting (NMConnection *connection, GType setting_type)
{
NMConnectionPrivate *priv;
NMSetting *setting;
const char *setting_name;
g_return_if_fail (NM_IS_CONNECTION (connection));
g_return_if_fail (g_type_is_a (setting_type, NM_TYPE_SETTING));
g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE);
g_return_val_if_fail (g_type_is_a (setting_type, NM_TYPE_SETTING), FALSE);
priv = NM_CONNECTION_GET_PRIVATE (connection);
setting_name = g_type_name (setting_type);
@ -149,7 +149,23 @@ nm_connection_remove_setting (NMConnection *connection, GType setting_type)
g_signal_handlers_disconnect_by_func (setting, setting_changed_cb, connection);
g_hash_table_remove (priv->settings, setting_name);
g_signal_emit (connection, signals[CHANGED], 0);
return TRUE;
}
return FALSE;
}
/**
* nm_connection_remove_setting:
* @connection: a #NMConnection
* @setting_type: the #GType of the setting object to remove
*
* Removes the #NMSetting with the given #GType from the #NMConnection. This
* operation dereferences the #NMSetting object.
**/
void
nm_connection_remove_setting (NMConnection *connection, GType setting_type)
{
_nm_connection_remove_setting (connection, setting_type);
}
/**
@ -994,6 +1010,26 @@ _normalize_required_settings (NMConnection *self, GHashTable *parameters)
return FALSE;
}
static gboolean
_normalize_invalid_slave_port_settings (NMConnection *self, GHashTable *parameters)
{
NMSettingConnection *s_con = nm_connection_get_setting_connection (self);
const char *slave_type;
gboolean changed = FALSE;
slave_type = nm_setting_connection_get_slave_type (s_con);
if ( !nm_streq0 (slave_type, NM_SETTING_BRIDGE_SETTING_NAME)
&& _nm_connection_remove_setting (self, NM_TYPE_SETTING_BRIDGE_PORT))
changed = TRUE;
if ( !nm_streq0 (slave_type, NM_SETTING_TEAM_SETTING_NAME)
&& _nm_connection_remove_setting (self, NM_TYPE_SETTING_TEAM_PORT))
changed = TRUE;
return changed;
}
/**
* nm_connection_verify:
* @connection: the #NMConnection to verify
@ -1242,6 +1278,7 @@ nm_connection_normalize (NMConnection *connection,
was_modified |= _normalize_connection_type (connection);
was_modified |= _normalize_connection_slave_type (connection);
was_modified |= _normalize_required_settings (connection, parameters);
was_modified |= _normalize_invalid_slave_port_settings (connection, parameters);
was_modified |= _normalize_ip_config (connection, parameters);
was_modified |= _normalize_ethernet_link_neg (connection);
was_modified |= _normalize_infiniband_mtu (connection, parameters);

View file

@ -136,6 +136,8 @@ typedef enum {
NMSettingVerifyResult _nm_connection_verify (NMConnection *connection, GError **error);
gboolean _nm_connection_remove_setting (NMConnection *connection, GType setting_type);
NMConnection *_nm_simple_connection_new_from_dbus (GVariant *dict,
NMSettingParseFlags parse_flags,
GError **error);

View file

@ -857,7 +857,7 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
{
NMSettingConnectionPrivate *priv = NM_SETTING_CONNECTION_GET_PRIVATE (setting);
gboolean is_slave;
const char *slave_setting_type = NULL;
const char *slave_setting_type;
NMSetting *normerr_base_type = NULL;
const char *normerr_slave_setting_type = NULL;
const char *normerr_missing_slave_type = NULL;
@ -958,16 +958,17 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
}
is_slave = FALSE;
if (priv->slave_type)
slave_setting_type = NULL;
if (priv->slave_type) {
is_slave = _nm_setting_slave_type_is_valid (priv->slave_type, &slave_setting_type);
if (priv->slave_type && !is_slave) {
g_set_error (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_PROPERTY,
_("Unknown slave type '%s'"), priv->slave_type);
g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE);
return FALSE;
if (!is_slave) {
g_set_error (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_PROPERTY,
_("Unknown slave type '%s'"), priv->slave_type);
g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE);
return FALSE;
}
}
if (is_slave) {
@ -1063,6 +1064,24 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
return NM_SETTING_VERIFY_NORMALIZABLE_ERROR;
}
if (connection) {
gboolean has_bridge_port = FALSE;
if ( ( !nm_streq0 (priv->slave_type, NM_SETTING_BRIDGE_SETTING_NAME)
&& (has_bridge_port = !!nm_connection_get_setting_by_name (connection, NM_SETTING_BRIDGE_PORT_SETTING_NAME)))
|| ( !nm_streq0 (priv->slave_type, NM_SETTING_TEAM_SETTING_NAME)
&& nm_connection_get_setting_by_name (connection, NM_SETTING_TEAM_PORT_SETTING_NAME))) {
g_set_error (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_SETTING,
_("A slave connection with '%s' set to '%s' cannot have a '%s' setting"),
NM_SETTING_CONNECTION_SLAVE_TYPE, priv->slave_type ?: "",
has_bridge_port ? NM_SETTING_BRIDGE_PORT_SETTING_NAME : NM_SETTING_TEAM_PORT_SETTING_NAME);
g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE);
return NM_SETTING_VERIFY_NORMALIZABLE_ERROR;
}
}
return TRUE;
}

View file

@ -1928,10 +1928,12 @@ test_clear_master (void)
g_assert_cmpstr (nm_setting_connection_get_master (s_con), ==, NULL);
g_assert_cmpstr (nm_setting_connection_get_slave_type (s_con), ==, NULL);
nmtst_assert_connection_verifies_after_normalization (connection, 0, 0);
/* 4. update the connection on disk */
_writer_update_connection_FIXME (connection,
TEST_SCRATCH_DIR "/network-scripts/",
testfile);
_writer_update_connection (connection,
TEST_SCRATCH_DIR "/network-scripts/",
testfile);
keyfile = utils_get_keys_path (testfile);
g_assert (!g_file_test (keyfile, G_FILE_TEST_EXISTS));