Re: [PATCH 5/8] bonding: Implement bonding slaves



On Fri, 2011-11-04 at 19:12 -0500, Dan Williams wrote:
> On Tue, 2011-10-18 at 13:48 +0200, Thomas Graf wrote:
> > Adds a MASTER= directive to ifcfg-rh allowing a connection to define
> > itself as bonding slave.
> > 
> > Adds a connection property "master" which contains the in-kernel device
> > name or UUID of the master connection.
> > 
> > Adds a connection property "slave-type" which defines the type of slave
> > this connection represents. Currently this is only set by bonding but
> > eventually this will be used by VLAN and bridging.
> 
> Could we drop this property and just use the 'type' setting of the
> connection to determine what type of slave this is?  ie, if 'master' is
> set and 'type == 'bond'' then this is a bond slave, no?  It seems like
> most of the time the 'slave-type' value will correspond to the same
> value as 'type'.  Thoughts?

After re-reading the patches, disregard this...  slaves won't be
type=bond since that's what the master is; the slave would just be
type=ethernet or whatever (well, until we get vlans on top of bonds on
top of bridges :)

Dan

> Dan
> 
> > Enforces that no bonding slave connection has any IPv4 or IPv6
> > configuration set.
> > 
> > Changes make_ip4_setting() to take a universal flag indicating whether
> > to allow disabling ip4 config or not and use it for both, ip6 and
> > bonding special case.
> > 
> > Signed-off-by: Thomas Graf <tgraf redhat com>
> > ---
> >  libnm-util/libnm-util.ver              |    3 +
> >  libnm-util/nm-setting-connection.c     |  140 ++++++++++++++++++++++++++++++++
> >  libnm-util/nm-setting-connection.h     |   11 +++-
> >  src/settings/plugins/ifcfg-rh/reader.c |   38 +++++++--
> >  4 files changed, 185 insertions(+), 7 deletions(-)
> > 
> > diff --git a/libnm-util/libnm-util.ver b/libnm-util/libnm-util.ver
> > index 0ee5917..b4d6912 100644
> > --- a/libnm-util/libnm-util.ver
> > +++ b/libnm-util/libnm-util.ver
> > @@ -197,11 +197,14 @@ global:
> >  	nm_setting_connection_get_connection_type;
> >  	nm_setting_connection_get_id;
> >  	nm_setting_connection_get_num_permissions;
> > +	nm_setting_connection_get_master;
> >  	nm_setting_connection_get_permission;
> > +	nm_setting_connection_get_slave_type;
> >  	nm_setting_connection_get_read_only;
> >  	nm_setting_connection_get_timestamp;
> >  	nm_setting_connection_get_type;
> >  	nm_setting_connection_get_uuid;
> > +	nm_setting_connection_is_slave_type;
> >  	nm_setting_connection_new;
> >  	nm_setting_connection_permissions_user_allowed;
> >  	nm_setting_connection_remove_permission;
> > diff --git a/libnm-util/nm-setting-connection.c b/libnm-util/nm-setting-connection.c
> > index e9030b1..ede63fb 100644
> > --- a/libnm-util/nm-setting-connection.c
> > +++ b/libnm-util/nm-setting-connection.c
> > @@ -72,6 +72,7 @@ nm_setting_connection_error_get_type (void)
> >  			ENUM_ENTRY (NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY, "InvalidProperty"),
> >  			ENUM_ENTRY (NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY, "MissingProperty"),
> >  			ENUM_ENTRY (NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND, "TypeSettingNotFound"),
> > +			ENUM_ENTRY (NM_SETTING_CONNECTION_ERROR_IP_CONFIG_FOUND, "IpConfigFound"),
> >  			{ 0, 0, 0 }
> >  		};
> >  		etype = g_enum_register_static ("NMSettingConnectionError", values);
> > @@ -97,6 +98,8 @@ typedef struct {
> >  	char *id;
> >  	char *uuid;
> >  	char *type;
> > +	char *master;
> > +	char *slave_type;
> >  	GSList *permissions; /* list of Permission structs */
> >  	gboolean autoconnect;
> >  	guint64 timestamp;
> > @@ -112,6 +115,8 @@ enum {
> >  	PROP_AUTOCONNECT,
> >  	PROP_TIMESTAMP,
> >  	PROP_READ_ONLY,
> > +	PROP_MASTER,
> > +	PROP_SLAVE_TYPE,
> >  
> >  	LAST_PROP
> >  };
> > @@ -478,6 +483,59 @@ nm_setting_connection_get_read_only (NMSettingConnection *setting)
> >  	return NM_SETTING_CONNECTION_GET_PRIVATE (setting)->read_only;
> >  }
> >  
> > +/**
> > + * nm_setting_connection_get_master:
> > + * @setting: the #NMSettingConnection
> > + *
> > + * Returns the #NMSettingConnection:master property of the connection.
> > + *
> > + * Returns: Name of the master device
> > + */
> > +const char *
> > +nm_setting_connection_get_master (NMSettingConnection *setting)
> > +{
> > +	g_return_val_if_fail (NM_IS_SETTING_CONNECTION (setting), NULL);
> > +
> > +	return NM_SETTING_CONNECTION_GET_PRIVATE (setting)->master;
> > +}
> > +
> > +/**
> > + * nm_setting_connection_get_slave_type:
> > + * @setting: the #NMSettingConnection
> > + *
> > + * Returns the #NMSettingConnection:slave-type property of the connection.
> > + *
> > + * Returns: Type of the slave device
> > + */
> > +const char *
> > +nm_setting_connection_get_slave_type (NMSettingConnection *setting)
> > +{
> > +	g_return_val_if_fail (NM_IS_SETTING_CONNECTION (setting), NULL);
> > +
> > +	return NM_SETTING_CONNECTION_GET_PRIVATE (setting)->slave_type;
> > +}
> > +
> > +/**
> > + * nm_setting_connection_is_slave_type:
> > + * @setting: the #NMSettingConnection
> > + * @type: the name of the slave type
> > + *
> > + * Returns: TRUE if connection is of specified slave type
> > + */
> > +gboolean
> > +nm_setting_connection_is_slave_type (NMSettingConnection *setting,
> > +				     const char *type)
> > +{
> > +	NMSettingConnectionPrivate *priv = NM_SETTING_CONNECTION_GET_PRIVATE (setting);
> > +
> > +	g_return_val_if_fail (NM_IS_SETTING_CONNECTION (setting), FALSE);
> > +
> > +	if (!priv->slave_type)
> > +		return FALSE;
> > +
> > +	return !strcmp (priv->slave_type, type);
> > +}
> > +
> >  static gint
> >  find_setting_by_name (gconstpointer a, gconstpointer b)
> >  {
> > @@ -559,6 +617,46 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
> >  		return FALSE;
> >  	}
> >  
> > +	/*
> > +	 * Bonding: Slaves are not allowed to have any IP configuration.
> > +	 */
> > +	if (priv->slave_type && all_settings &&
> > +	    !strcmp(priv->slave_type, NM_SETTING_BOND_SETTING_NAME)) {
> > +		GSList *list;
> > +		
> > +		list = g_slist_find_custom (all_settings, NM_SETTING_IP4_CONFIG_SETTING_NAME,
> > +					    find_setting_by_name);
> > +		if (list) {
> > +			NMSettingIP4Config *s_ip4 = g_slist_nth_data (list, 0);
> > +			g_assert (s_ip4);
> > +
> > +			if (strcmp (nm_setting_ip4_config_get_method (s_ip4),
> > +				    NM_SETTING_IP4_CONFIG_METHOD_DISABLED)) {
> > +				g_set_error (error,
> > +					     NM_SETTING_CONNECTION_ERROR,
> > +					     NM_SETTING_CONNECTION_ERROR_IP_CONFIG_FOUND,
> > +					     "No IP configuration allowed for bonding slave");
> > +				return FALSE;
> > +			}
> > +		}
> > +
> > +		list = g_slist_find_custom (all_settings, NM_SETTING_IP6_CONFIG_SETTING_NAME,
> > +					    find_setting_by_name);
> > +		if (list) {
> > +			NMSettingIP6Config *s_ip6 = g_slist_nth_data (list, 0);
> > +			g_assert (s_ip6);
> > +
> > +			if (strcmp (nm_setting_ip6_config_get_method (s_ip6),
> > +				    NM_SETTING_IP6_CONFIG_METHOD_IGNORE)) {
> > +				g_set_error (error,
> > +					     NM_SETTING_CONNECTION_ERROR,
> > +					     NM_SETTING_CONNECTION_ERROR_IP_CONFIG_FOUND,
> > +					     "No IPv6 configuration allowed for bonding slave");
> > +				return FALSE;
> > +			}
> > +		}
> > +	}
> > +
> >  	return TRUE;
> >  }
> >  
> > @@ -591,6 +689,8 @@ finalize (GObject *object)
> >  	g_free (priv->id);
> >  	g_free (priv->uuid);
> >  	g_free (priv->type);
> > +	g_free (priv->master);
> > +	g_free (priv->slave_type);
> >  	nm_utils_slist_free (priv->permissions, (GDestroyNotify) permission_free);
> >  
> >  	G_OBJECT_CLASS (nm_setting_connection_parent_class)->finalize (object);
> > @@ -644,6 +744,14 @@ set_property (GObject *object, guint prop_id,
> >  	case PROP_READ_ONLY:
> >  		priv->read_only = g_value_get_boolean (value);
> >  		break;
> > +	case PROP_MASTER:
> > +		g_free (priv->master);
> > +		priv->master = g_value_dup_string (value);
> > +		break;
> > +	case PROP_SLAVE_TYPE:
> > +		g_free (priv->slave_type);
> > +		priv->slave_type = g_value_dup_string (value);
> > +		break;
> >  	default:
> >  		G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
> >  		break;
> > @@ -689,6 +797,12 @@ get_property (GObject *object, guint prop_id,
> >  	case PROP_READ_ONLY:
> >  		g_value_set_boolean (value, nm_setting_connection_get_read_only (setting));
> >  		break;
> > +	case PROP_MASTER:
> > +		g_value_set_string (value, nm_setting_connection_get_master (setting));
> > +		break;
> > +	case PROP_SLAVE_TYPE:
> > +		g_value_set_string (value, nm_setting_connection_get_slave_type (setting));
> > +		break;
> >  	default:
> >  		G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
> >  		break;
> > @@ -876,4 +990,30 @@ nm_setting_connection_class_init (NMSettingConnectionClass *setting_class)
> >  	                      "cannot yet write updated connections back out.",
> >  	                      FALSE,
> >  	                      G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE | NM_SETTING_PARAM_FUZZY_IGNORE));
> > +
> > +	/**
> > +	 * NMSettingConnection:master:
> > +	 *
> > +	 * Name of the master deviec or UUID of the master connection
> > +	 **/
> > +	g_object_class_install_property
> > +		(object_class, PROP_MASTER,
> > +		 g_param_spec_string (NM_SETTING_CONNECTION_MASTER,
> > +						  "Master",
> > +						  "Name of the master device or UUID of the master connection",
> > +						  NULL,
> > +						  G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE | NM_SETTING_PARAM_FUZZY_IGNORE));
> > +
> > +	/**
> > +	 * NMSettingConnection:slave-type:
> > +	 *
> > +	 * Type of slave device or NULL if of not a slave.
> > +	 **/
> > +	g_object_class_install_property
> > +		(object_class, PROP_SLAVE_TYPE,
> > +		 g_param_spec_string (NM_SETTING_CONNECTION_SLAVE_TYPE,
> > +						  "Slave-Type",
> > +						  "Type of slave device or NULL if not a slave",
> > +						  NULL,
> > +						  G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE | NM_SETTING_PARAM_FUZZY_IGNORE));
> >  }
> > diff --git a/libnm-util/nm-setting-connection.h b/libnm-util/nm-setting-connection.h
> > index 1ec5bf1..d82049c 100644
> > --- a/libnm-util/nm-setting-connection.h
> > +++ b/libnm-util/nm-setting-connection.h
> > @@ -49,6 +49,8 @@ G_BEGIN_DECLS
> >   * @NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND: the #NMSetting object
> >   *   referenced by the setting name contained in the
> >   *   #NMSettingConnection:type property was not present in the #NMConnection
> > + * @NM_SETTING_CONNECTION_ERROR_IP_CONFIG_FOUND: ip configuration is not
> > + *   allowed to be present.
> >   *
> >   * Describes errors that may result from operations involving a
> >   * #NMSettingConnection.
> > @@ -59,7 +61,8 @@ typedef enum
> >  	NM_SETTING_CONNECTION_ERROR_UNKNOWN = 0,
> >  	NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY,
> >  	NM_SETTING_CONNECTION_ERROR_MISSING_PROPERTY,
> > -	NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND
> > +	NM_SETTING_CONNECTION_ERROR_TYPE_SETTING_NOT_FOUND,
> > +	NM_SETTING_CONNECTION_ERROR_IP_CONFIG_FOUND,
> >  } NMSettingConnectionError;
> >  
> >  #define NM_TYPE_SETTING_CONNECTION_ERROR (nm_setting_connection_error_get_type ()) 
> > @@ -75,6 +78,8 @@ GQuark nm_setting_connection_error_quark (void);
> >  #define NM_SETTING_CONNECTION_TIMESTAMP   "timestamp"
> >  #define NM_SETTING_CONNECTION_READ_ONLY   "read-only"
> >  #define NM_SETTING_CONNECTION_PERMISSIONS "permissions"
> > +#define NM_SETTING_CONNECTION_MASTER      "master"
> > +#define NM_SETTING_CONNECTION_SLAVE_TYPE  "slave-type"
> >  
> >  /**
> >   * NMSettingConnection:
> > @@ -119,6 +124,10 @@ gboolean    nm_setting_connection_add_permission       (NMSettingConnection *set
> >                                                          const char *detail);
> >  void        nm_setting_connection_remove_permission    (NMSettingConnection *setting,
> >                                                          guint32 idx);
> > +const char *nm_setting_connection_get_master           (NMSettingConnection *setting);
> > +gboolean    nm_setting_connection_is_slave_type        (NMSettingConnection *setting,
> > +							const char *type);
> > +const char *nm_setting_connection_get_slave_type       (NMSettingConnection *setting);
> >  
> >  G_END_DECLS
> >  
> > diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c
> > index a327e79..cee4100 100644
> > --- a/src/settings/plugins/ifcfg-rh/reader.c
> > +++ b/src/settings/plugins/ifcfg-rh/reader.c
> > @@ -139,6 +139,14 @@ make_connection_setting (const char *file,
> >  	              svTrueValue (ifcfg, "ONBOOT", TRUE),
> >  	              NULL);
> >  
> > +	value = svGetValue (ifcfg, "MASTER", FALSE);
> > +	if (value) {
> > +		g_object_set (s_con, NM_SETTING_CONNECTION_MASTER, value, NULL);
> > +		g_object_set (s_con, NM_SETTING_CONNECTION_SLAVE_TYPE,
> > +			      NM_SETTING_BOND_SETTING_NAME, NULL);
> > +		g_free (value);
> > +	}
> > +
> >  	value = svGetValue (ifcfg, "USERS", FALSE);
> >  	if (value) {
> >  		char **items, **iter;
> > @@ -1116,7 +1124,7 @@ static NMSetting *
> >  make_ip4_setting (shvarFile *ifcfg,
> >                    const char *network_file,
> >                    const char *iscsiadm_path,
> > -                  gboolean valid_ip6_config,
> > +                  gboolean can_disable_ip4,
> >                    GError **error)
> >  {
> >  	NMSettingIP4Config *s_ip4 = NULL;
> > @@ -1231,7 +1239,7 @@ make_ip4_setting (shvarFile *ifcfg,
> >  		    && !tmp_ip4_0 && !tmp_prefix_0 && !tmp_netmask_0
> >  		    && !tmp_ip4_1 && !tmp_prefix_1 && !tmp_netmask_1
> >  		    && !tmp_ip4_2 && !tmp_prefix_2 && !tmp_netmask_2) {
> > -			if (valid_ip6_config)
> > +			if (can_disable_ip4)
> >  				/* Nope, no IPv4 */
> >  				method = NM_SETTING_IP4_CONFIG_METHOD_DISABLED;
> >  			else
> > @@ -3471,6 +3479,21 @@ bond_connection_from_ifcfg (const char *file,
> >  }
> >  
> >  static gboolean
> > +disabling_ip4_config_allowed (NMConnection *connection)
> > +{
> > +	NMSettingConnection *s_con;
> > +
> > +	s_con = nm_connection_get_setting_connection (connection);
> > +	g_assert (s_con);
> > +
> > +	/* bonding slaves are allowed to have no ip configuration */
> > +	if (nm_setting_connection_is_slave_type (s_con, NM_SETTING_BOND_SETTING_NAME))
> > +		return TRUE;
> > +
> > +	return FALSE;
> > +}
> > +
> > +static gboolean
> >  is_bond_device (const char *name, shvarFile *parsed)
> >  {
> >  	g_return_val_if_fail (name != NULL, FALSE);
> > @@ -3508,7 +3531,7 @@ connection_from_file (const char *filename,
> >  	NMSetting *s_ip4, *s_ip6;
> >  	const char *ifcfg_name = NULL;
> >  	gboolean nm_controlled = TRUE;
> > -	gboolean ip6_used = FALSE;
> > +	gboolean can_disable_ip4 = FALSE;
> >  	GError *error = NULL;
> >  	guint32 ignore_reason = IGNORE_REASON_NONE;
> >  
> > @@ -3677,10 +3700,13 @@ connection_from_file (const char *filename,
> >  		nm_connection_add_setting (connection, s_ip6);
> >  		method = nm_setting_ip6_config_get_method (NM_SETTING_IP6_CONFIG (s_ip6));
> >  		if (method && strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_IGNORE))
> > -			ip6_used = TRUE;
> > +			can_disable_ip4 = TRUE;
> >  	}
> >  
> > -	s_ip4 = make_ip4_setting (parsed, network_file, iscsiadm_path, ip6_used, &error);
> > +	if (disabling_ip4_config_allowed (connection))
> > +		can_disable_ip4 = TRUE;
> > +
> > +	s_ip4 = make_ip4_setting (parsed, network_file, iscsiadm_path, can_disable_ip4, &error);
> >  	if (error) {
> >  		g_object_unref (connection);
> >  		connection = NULL;
> > @@ -3697,7 +3723,7 @@ connection_from_file (const char *filename,
> >  	    && !g_ascii_strcasecmp (bootproto, "ibft")) {
> >  		NMSettingConnection *s_con;
> >  
> > -		s_con = (NMSettingConnection *) nm_connection_get_setting (connection, NM_TYPE_SETTING_CONNECTION);
> > +		s_con = nm_connection_get_setting_connection (connection);
> >  		g_assert (s_con);
> >  
> >  		g_object_set (G_OBJECT (s_con), NM_SETTING_CONNECTION_READ_ONLY, TRUE, NULL);
> 
> 
> _______________________________________________
> networkmanager-list mailing list
> networkmanager-list gnome org
> http://mail.gnome.org/mailman/listinfo/networkmanager-list




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]