From 7279e7e150180a1196913a0fb141f67d637f7adc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 7 Jul 2014 17:05:10 +0200 Subject: libnm-core: normalize slave-type and slave-settings of connections Some NMSettingConnection:slave-type types require a matching slave #NMSetting. Add normalization of either the 'slave-type' property or the slave-setting. Also be more strict in NMSettingConnection:verify() to enforce an existing slave-setting depending on the slave-type. Signed-off-by: Thomas Haller --- libnm-core/nm-connection.c | 43 ++++++++++++++- libnm-core/nm-setting-bridge-port.c | 26 +++++++++ libnm-core/nm-setting-connection.c | 62 +++++++++++++++++----- libnm-core/nm-setting-connection.h | 3 ++ libnm-core/nm-setting-private.h | 3 ++ libnm-core/nm-setting-team-port.c | 24 +++++++++ libnm-core/nm-setting.c | 59 ++++++++++++++++++++ libnm-core/tests/test-general.c | 60 +++++++++++++++++++++ po/POTFILES.in | 1 + .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 7 ++- 10 files changed, 273 insertions(+), 15 deletions(-) diff --git a/libnm-core/nm-connection.c b/libnm-core/nm-connection.c index 549dc6527d..709fc20a64 100644 --- a/libnm-core/nm-connection.c +++ b/libnm-core/nm-connection.c @@ -25,7 +25,7 @@ #include #include #include "nm-connection.h" -#include "nm-utils.h" +#include "nm-utils-internal.h" #include "nm-dbus-glib-types.h" #include "nm-setting-private.h" @@ -580,6 +580,46 @@ _normalize_virtual_iface_name (NMConnection *self) return was_modified; } +static gboolean +_normalize_connection_slave_type (NMConnection *self) +{ + NMSettingConnection *s_con = nm_connection_get_setting_connection (self); + const char *slave_type, *port_type; + GSList *all_settings; + + if (!s_con) + return FALSE; + if (!nm_setting_connection_get_master (s_con)) + return FALSE; + + slave_type = nm_setting_connection_get_slave_type (s_con); + if (slave_type) { + if ( _nm_setting_slave_type_is_valid (slave_type, &port_type) + && port_type) { + NMSetting *s_port; + + s_port = nm_connection_get_setting_by_name (self, port_type); + if (!s_port) { + GType p_type = nm_setting_lookup_type (port_type); + + g_return_val_if_fail (p_type, FALSE); + nm_connection_add_setting (self, g_object_new (p_type, NULL)); + return TRUE; + } + } + } else { + all_settings = _nm_utils_hash_values_to_slist (NM_CONNECTION_GET_PRIVATE (self)->settings); + + if ((slave_type = _nm_setting_slave_type_detect_from_settings (all_settings, NULL))) { + g_object_set (s_con, NM_SETTING_CONNECTION_SLAVE_TYPE, slave_type, NULL); + g_slist_free (all_settings); + return TRUE; + } + g_slist_free (all_settings); + } + return FALSE; +} + static gboolean _normalize_ip_config (NMConnection *self, GHashTable *parameters) { @@ -837,6 +877,7 @@ nm_connection_normalize (NMConnection *connection, * errors, because in that case we rather fail without touching the settings. */ was_modified |= _normalize_virtual_iface_name (connection); + was_modified |= _normalize_connection_slave_type (connection); was_modified |= _normalize_ip_config (connection, parameters); /* Verify anew. */ diff --git a/libnm-core/nm-setting-bridge-port.c b/libnm-core/nm-setting-bridge-port.c index 243f4668dd..0bc1a1c828 100644 --- a/libnm-core/nm-setting-bridge-port.c +++ b/libnm-core/nm-setting-bridge-port.c @@ -159,6 +159,32 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) return FALSE; } + + if (all_settings) { + NMSettingConnection *s_con; + const char *slave_type; + + s_con = NM_SETTING_CONNECTION (_nm_setting_find_in_list_required (all_settings, + NM_SETTING_CONNECTION_SETTING_NAME, + error, NULL, NULL)); + if (!s_con) + return FALSE; + + slave_type = nm_setting_connection_get_slave_type (s_con); + if ( slave_type + && strcmp (slave_type, NM_SETTING_BRIDGE_SETTING_NAME)) { + g_set_error (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY, + _("A connection with a '%s' setting must have the slave-type set to '%s'. Instead it is '%s'"), + NM_SETTING_BRIDGE_PORT_SETTING_NAME, + NM_SETTING_BRIDGE_SETTING_NAME, + slave_type); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE); + return FALSE; + } + } + return TRUE; } diff --git a/libnm-core/nm-setting-connection.c b/libnm-core/nm-setting-connection.c index c359298250..fdfe5f7214 100644 --- a/libnm-core/nm-setting-connection.c +++ b/libnm-core/nm-setting-connection.c @@ -741,7 +741,11 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) { NMSettingConnectionPrivate *priv = NM_SETTING_CONNECTION_GET_PRIVATE (setting); gboolean is_slave; + const char *slave_setting_type = NULL; GSList *iter; + const char *normerr_slave_setting_type = NULL; + const char *normerr_missing_slave_type = NULL; + const char *normerr_missing_slave_type_port = NULL; if (!priv->id) { g_set_error_literal (error, @@ -862,10 +866,9 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) } } - is_slave = ( priv->slave_type - && ( !strcmp (priv->slave_type, NM_SETTING_BOND_SETTING_NAME) - || !strcmp (priv->slave_type, NM_SETTING_BRIDGE_SETTING_NAME) - || !strcmp (priv->slave_type, NM_SETTING_TEAM_SETTING_NAME))); + is_slave = FALSE; + 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, @@ -873,7 +876,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) NM_SETTING_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 NM_SETTING_VERIFY_ERROR; + return FALSE; } if (is_slave) { @@ -883,19 +886,54 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY, _("Slave connections need a valid '" NM_SETTING_CONNECTION_MASTER "' property")); g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_MASTER); - return NM_SETTING_VERIFY_ERROR; + return FALSE; } + if ( slave_setting_type + && all_settings /* only check for an existing slave-setting when having @all_settings */ + && !nm_setting_find_in_list (all_settings, slave_setting_type)) + normerr_slave_setting_type = slave_setting_type; } else { if (priv->master) { - g_set_error_literal (error, - NM_SETTING_CONNECTION_ERROR, - NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY, - _("Cannot set '" NM_SETTING_CONNECTION_MASTER "' without '" NM_SETTING_CONNECTION_SLAVE_TYPE "'")); - g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE); - return NM_SETTING_VERIFY_ERROR; + const char *slave_type; + NMSetting *s_port; + + if ( all_settings + && (slave_type = _nm_setting_slave_type_detect_from_settings (all_settings, &s_port))) { + normerr_missing_slave_type = slave_type; + normerr_missing_slave_type_port = nm_setting_get_name (s_port); + } else { + g_set_error_literal (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY, + _("Cannot set '" NM_SETTING_CONNECTION_MASTER "' without '" NM_SETTING_CONNECTION_SLAVE_TYPE "'")); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE); + return FALSE; + } } } + /* *** errors above here should be always fatal, below NORMALIZABLE_ERROR *** */ + + if (normerr_slave_setting_type) { + g_set_error (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_SLAVE_SETTING_NOT_FOUND, + _("slave-type '%s' requires a '%s' setting in the connection"), + priv->slave_type, normerr_slave_setting_type); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE); + return NM_SETTING_VERIFY_NORMALIZABLE_ERROR; + } + + if (normerr_missing_slave_type) { + g_set_error (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY, + _("Detect a slave connection with '" NM_SETTING_CONNECTION_MASTER "' set and a port type '%s'. '" NM_SETTING_CONNECTION_SLAVE_TYPE "' should be set to '%s'"), + normerr_missing_slave_type_port, normerr_missing_slave_type); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE); + return NM_SETTING_VERIFY_NORMALIZABLE_ERROR; + } + return TRUE; } diff --git a/libnm-core/nm-setting-connection.h b/libnm-core/nm-setting-connection.h index 7782567e39..169e2c18ee 100644 --- a/libnm-core/nm-setting-connection.h +++ b/libnm-core/nm-setting-connection.h @@ -52,6 +52,8 @@ G_BEGIN_DECLS * #NMSettingConnection:type property was not present in the #NMConnection * @NM_SETTING_CONNECTION_ERROR_IP_CONFIG_NOT_ALLOWED: ip configuration is not * allowed to be present. + * @NM_SETTING_CONNECTION_ERROR_SLAVE_SETTING_NOT_FOUND: the mandatory #NMSetting + * object for the slave is missing. The slave type depends on #NMSettingConnection:slave-type * * Describes errors that may result from operations involving a * #NMSettingConnection. @@ -64,6 +66,7 @@ typedef enum NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY, /*< nick=MissingProperty >*/ NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND, /*< nick=TypeSettingNotFound >*/ NM_SETTING_CONNECTION_ERROR_IP_CONFIG_NOT_ALLOWED, /*< nick=IpConfigNotAllowed >*/ + NM_SETTING_CONNECTION_ERROR_SLAVE_SETTING_NOT_FOUND, /*< nick=SlaveSettingNotFound >*/ } NMSettingConnectionError; #define NM_SETTING_CONNECTION_ERROR nm_setting_connection_error_quark () diff --git a/libnm-core/nm-setting-private.h b/libnm-core/nm-setting-private.h index e68d5a4278..316838a32b 100644 --- a/libnm-core/nm-setting-private.h +++ b/libnm-core/nm-setting-private.h @@ -110,4 +110,7 @@ NMSettingVerifyResult _nm_setting_verify (NMSetting *setting, GSList *all_settings, GError **error); +gboolean _nm_setting_slave_type_is_valid (const char *slave_type, const char **out_port_type); +const char * _nm_setting_slave_type_detect_from_settings (GSList *all_settings, NMSetting **out_s_port); + #endif /* NM_SETTING_PRIVATE_H */ diff --git a/libnm-core/nm-setting-team-port.c b/libnm-core/nm-setting-team-port.c index cb8d00c38f..e5cac6d61a 100644 --- a/libnm-core/nm-setting-team-port.c +++ b/libnm-core/nm-setting-team-port.c @@ -103,6 +103,30 @@ nm_setting_team_port_get_config (NMSettingTeamPort *setting) static gboolean verify (NMSetting *setting, GSList *all_settings, GError **error) { + if (all_settings) { + NMSettingConnection *s_con; + const char *slave_type; + + s_con = NM_SETTING_CONNECTION (_nm_setting_find_in_list_required (all_settings, + NM_SETTING_CONNECTION_SETTING_NAME, + error, NULL, NULL)); + if (!s_con) + return FALSE; + + slave_type = nm_setting_connection_get_slave_type (s_con); + if ( slave_type + && strcmp (slave_type, NM_SETTING_TEAM_SETTING_NAME)) { + g_set_error (error, + NM_SETTING_CONNECTION_ERROR, + NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY, + _("A connection with a '%s' setting must have the slave-type set to '%s'. Instead it is '%s'"), + NM_SETTING_TEAM_PORT_SETTING_NAME, + NM_SETTING_TEAM_SETTING_NAME, + slave_type); + g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_SLAVE_TYPE); + return FALSE; + } + } return TRUE; } diff --git a/libnm-core/nm-setting.c b/libnm-core/nm-setting.c index 830484b569..5fc0b64867 100644 --- a/libnm-core/nm-setting.c +++ b/libnm-core/nm-setting.c @@ -288,6 +288,65 @@ _nm_setting_compare_priority (gconstpointer a, gconstpointer b) /*************************************************************/ +gboolean +_nm_setting_slave_type_is_valid (const char *slave_type, const char **out_port_type) +{ + const char *port_type = NULL; + gboolean found = TRUE; + + if (!slave_type) + found = FALSE; + else if (!strcmp (slave_type, NM_SETTING_BOND_SETTING_NAME)) + ; + else if (!strcmp (slave_type, NM_SETTING_BRIDGE_SETTING_NAME)) + port_type = NM_SETTING_BRIDGE_PORT_SETTING_NAME; + else if (!strcmp (slave_type, NM_SETTING_TEAM_SETTING_NAME)) + port_type = NM_SETTING_TEAM_PORT_SETTING_NAME; + else + found = FALSE; + + if (out_port_type) + *out_port_type = port_type; + return found; +} + + +const char * +_nm_setting_slave_type_detect_from_settings (GSList *all_settings, NMSetting **out_s_port) +{ + GSList *iter; + const char *slave_type = NULL; + NMSetting *s_port = NULL; + + for (iter = all_settings; iter; iter = iter->next) { + NMSetting *s_iter = NM_SETTING (iter->data); + const char *name = nm_setting_get_name (s_iter); + const char *i_slave_type = NULL; + + if (!strcmp (name, NM_SETTING_BRIDGE_PORT_SETTING_NAME)) + i_slave_type = NM_SETTING_BRIDGE_SETTING_NAME; + else if (!strcmp (name, NM_SETTING_TEAM_PORT_SETTING_NAME)) + i_slave_type = NM_SETTING_TEAM_SETTING_NAME; + else + continue; + + if (slave_type) { + /* there are more then one matching port types, cannot detect the slave type. */ + slave_type = NULL; + s_port = NULL; + break; + } + slave_type = i_slave_type; + s_port = s_iter; + } + + if (out_s_port) + *out_s_port = s_port; + return slave_type; +} + +/*************************************************************/ + static void destroy_gvalue (gpointer data) { diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 0c660d0384..f9a7714b8c 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -2658,6 +2658,64 @@ test_connection_normalize_virtual_iface_name (void) g_object_unref (con); } +static void +test_connection_normalize_slave_type_1 (void) +{ + gs_unref_object NMConnection *con = NULL; + NMSettingConnection *s_con; + + con = nmtst_create_minimal_connection ("test_connection_normalize_slave_type_1", + "cc4cd5df-45dc-483e-b291-6b76c2338ecb", + NM_SETTING_WIRED_SETTING_NAME, &s_con); + + g_object_set (s_con, + NM_SETTING_CONNECTION_MASTER, "master0", + NM_SETTING_CONNECTION_SLAVE_TYPE, "invalid-type", + NULL); + + nmtst_assert_connection_unnormalizable (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY); + g_assert (!nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME)); + + g_object_set (s_con, + NM_SETTING_CONNECTION_SLAVE_TYPE, "bridge", + NULL); + + g_assert (!nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME)); + nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_SLAVE_SETTING_NOT_FOUND); + g_assert (nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME)); + g_assert_cmpstr (nm_setting_connection_get_slave_type (s_con), ==, NM_SETTING_BRIDGE_SETTING_NAME); +} + +static void +test_connection_normalize_slave_type_2 (void) +{ + gs_unref_object NMConnection *con = NULL; + NMSettingConnection *s_con; + + con = nmtst_create_minimal_connection ("test_connection_normalize_slave_type_2", + "40bea008-ca72-439a-946b-e65f827656f9", + NM_SETTING_WIRED_SETTING_NAME, &s_con); + + g_object_set (s_con, + NM_SETTING_CONNECTION_MASTER, "master0", + NM_SETTING_CONNECTION_SLAVE_TYPE, "invalid-type", + NULL); + + nmtst_assert_connection_unnormalizable (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY); + g_assert (!nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME)); + + g_object_set (s_con, + NM_SETTING_CONNECTION_SLAVE_TYPE, NULL, + NULL); + nm_connection_add_setting (con, nm_setting_bridge_port_new ()); + + g_assert (nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME)); + g_assert_cmpstr (nm_setting_connection_get_slave_type (s_con), ==, NULL); + nmtst_assert_connection_verifies_after_normalization (con, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY); + g_assert (nm_connection_get_setting_by_name (con, NM_SETTING_BRIDGE_PORT_SETTING_NAME)); + g_assert_cmpstr (nm_setting_connection_get_slave_type (s_con), ==, NM_SETTING_BRIDGE_SETTING_NAME); +} + NMTST_DEFINE (); int main (int argc, char **argv) @@ -2697,6 +2755,8 @@ int main (int argc, char **argv) test_connection_new_from_hash (); test_connection_verify_sets_interface_name (); test_connection_normalize_virtual_iface_name (); + test_connection_normalize_slave_type_1 (); + test_connection_normalize_slave_type_2 (); test_setting_connection_permissions_helpers (); test_setting_connection_permissions_property (); diff --git a/po/POTFILES.in b/po/POTFILES.in index 37fd6199a6..86a0d1834b 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -59,6 +59,7 @@ libnm-core/nm-setting-ip6-config.c libnm-core/nm-setting-olpc-mesh.c libnm-core/nm-setting-ppp.c libnm-core/nm-setting-pppoe.c +libnm-core/nm-setting-team-port.c libnm-core/nm-setting-vlan.c libnm-core/nm-setting-vpn.c libnm-core/nm-setting-wimax.c diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index 4b9c7aa675..d917159efe 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -14409,8 +14409,11 @@ test_read_team_port_empty_config (void) g_assert_cmpstr (nm_setting_connection_get_connection_type (s_con), ==, NM_SETTING_WIRED_SETTING_NAME); g_assert_cmpstr (nm_setting_connection_get_master (s_con), ==, "team0"); - /* Empty TEAM_PORT_CONFIG means no team-port setting */ - g_assert (nm_connection_get_setting_team_port (connection) == NULL); + /* Normalization adds a team-port setting */ + g_assert (nm_connection_get_setting_team_port (connection)); + + /* empty/missing config */ + g_assert (!nm_setting_team_port_get_config (nm_connection_get_setting_team_port (connection))); g_object_unref (connection); } -- cgit v1.2.1