Re: [PATCH] bonding: Convert individual bonding options to a dict
- From: Dan Williams <dcbw redhat com>
- To: Thomas Graf <tgraf redhat com>
- Cc: networkmanager-list gnome org
- Subject: Re: [PATCH] bonding: Convert individual bonding options to a dict
- Date: Wed, 15 Feb 2012 21:50:42 -0600
On Tue, 2012-02-14 at 14:05 +0100, Thomas Graf wrote:
> Removes all bonding options properties and adds a "options" dict to hold
> them all. Accessible via accessor functions. ifcfg interface is
> unchanged.
Applied with a few changes; instead of _by_key() use _by_name() since
that's more appropriate, despite what I chose for the s390 options 3
years ago :) Second add #defines for the recognized options to reduce
the possibility of mis-spelling them then in the code at some point and
thus failing silently; better to catch that at compile time.
Thanks!
Dan
> Signed-off-by: Thomas Graf <tgraf redhat com>
> ---
> libnm-util/libnm-util.ver | 13 +-
> libnm-util/nm-setting-bond.c | 424 ++++++++------------
> libnm-util/nm-setting-bond.h | 25 +-
> src/nm-system.c | 31 +-
> src/settings/plugins/ifcfg-rh/reader.c | 14 +-
> .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 6 +-
> 6 files changed, 204 insertions(+), 309 deletions(-)
>
> diff --git a/libnm-util/libnm-util.ver b/libnm-util/libnm-util.ver
> index cd676fe..d691ec1 100644
> --- a/libnm-util/libnm-util.ver
> +++ b/libnm-util/libnm-util.ver
> @@ -176,14 +176,13 @@ global:
> nm_setting_bluetooth_new;
> nm_setting_bond_error_get_type;
> nm_setting_bond_error_quark;
> - nm_setting_bond_get_arp_interval;
> - nm_setting_bond_get_arp_ip_target;
> - nm_setting_bond_get_downdelay;
> - nm_setting_bond_get_interface_name;
> - nm_setting_bond_get_miimon;
> - nm_setting_bond_get_mode;
> nm_setting_bond_get_type;
> - nm_setting_bond_get_updelay;
> + nm_setting_bond_get_interface_name;
> + nm_setting_bond_get_num_options;
> + nm_setting_bond_get_option;
> + nm_setting_bond_get_option_by_key;
> + nm_setting_bond_add_option;
> + nm_setting_bond_remove_option;
> nm_setting_bond_new;
> nm_setting_cdma_error_get_type;
> nm_setting_cdma_error_quark;
> diff --git a/libnm-util/nm-setting-bond.c b/libnm-util/nm-setting-bond.c
> index 3aa9cf3..b5d13cb 100644
> --- a/libnm-util/nm-setting-bond.c
> +++ b/libnm-util/nm-setting-bond.c
> @@ -89,26 +89,21 @@ G_DEFINE_TYPE (NMSettingBond, nm_setting_bond, NM_TYPE_SETTING)
>
> typedef struct {
> char * interface_name;
> - char * mode;
> - guint32 miimon;
> - guint32 downdelay;
> - guint32 updelay;
> - guint32 arp_interval;
> - char * arp_ip_target;
> + GHashTable *options;
> } NMSettingBondPrivate;
>
> enum {
> PROP_0,
> PROP_INTERFACE_NAME,
> - PROP_MODE,
> - PROP_MIIMON,
> - PROP_DOWNDELAY,
> - PROP_UPDELAY,
> - PROP_ARP_INTERVAL,
> - PROP_ARP_IP_TARGET,
> + PROP_OPTIONS,
> LAST_PROP
> };
>
> +static const char *valid_opts[] = {
> + "mode", "miimon", "downdelay", "updelay", "arp_interval", "arp_ip_target",
> + NULL,
> +};
> +
> /**
> * nm_setting_bond_new:
> *
> @@ -137,87 +132,149 @@ nm_setting_bond_get_interface_name (NMSettingBond *setting)
> }
>
> /**
> - * nm_setting_bond_get_mode:
> + * nm_setting_bond_get_num_options:
> * @setting: the #NMSettingBond
> *
> - * Returns: the #NMSettingBond:mode property of the setting
> - **/
> -const char *
> -nm_setting_bond_get_mode (NMSettingBond *setting)
> -{
> - g_return_val_if_fail (NM_IS_SETTING_BOND (setting), 0);
> -
> - return NM_SETTING_BOND_GET_PRIVATE (setting)->mode;
> -}
> -
> -/**
> - * nm_setting_bond_get_miimon:
> - * @setting: the #NMSettingBond
> + * Returns the number of options that should be set for this bond when it
> + * is activated. This can be used to retrieve each option individually
> + * using nm_setting_bond_get_option().
> *
> - * Returns: the #NMSettingBond:miimon property of the setting
> + * Returns: the number of bonding options
> **/
> guint32
> -nm_setting_bond_get_miimon (NMSettingBond *setting)
> +nm_setting_bond_get_num_options (NMSettingBond *setting)
> {
> g_return_val_if_fail (NM_IS_SETTING_BOND (setting), 0);
>
> - return NM_SETTING_BOND_GET_PRIVATE (setting)->miimon;
> + return g_hash_table_size (NM_SETTING_BOND_GET_PRIVATE (setting)->options);
> }
>
> /**
> - * nm_setting_bond_get_downdelay:
> + * nm_setting_bond_get_option:
> * @setting: the #NMSettingBond
> + * @idx: index of the desired option, from 0 to
> + * nm_setting_bond_get_num_options() - 1
> + * @out_key: (out): on return, the key name of the bonding option; this
> + * value is owned by the setting and should not be modified
> + * @out_value: (out): on return, the value of the key of the bonding
> + * option; this value is owned by the setting and should not be modified
> + *
> + * Given an index, return the value of the bonding option at that index. indexes
> + * are *not* guaranteed to be static across modifications to options done by
> + * nm_setting_bond_add_option() and nm_setting_bond_remove_option(),
> + * and should not be used to refer to options except for short periods of time
> + * such as during option iteration.
> *
> - * Returns: the #NMSettingBond:downdelay property of the setting
> + * Returns: %TRUE on success if the index was valid and an option was found,
> + * %FALSE if the index was invalid (ie, greater than the number of options
> + * currently held by the setting)
> **/
> -guint32
> -nm_setting_bond_get_downdelay (NMSettingBond *setting)
> +gboolean
> +nm_setting_bond_get_option (NMSettingBond *setting,
> + guint32 idx,
> + const char **out_key,
> + const char **out_value)
> {
> - g_return_val_if_fail (NM_IS_SETTING_BOND (setting), 0);
> + NMSettingBondPrivate *priv;
> + guint32 num_keys;
> + GList *keys;
> + const char *_key = NULL, *_value = NULL;
> +
> + g_return_val_if_fail (NM_IS_SETTING_BOND (setting), FALSE);
> +
> + priv = NM_SETTING_BOND_GET_PRIVATE (setting);
> +
> + num_keys = nm_setting_bond_get_num_options (setting);
> + g_return_val_if_fail (idx < num_keys, FALSE);
> +
> + keys = g_hash_table_get_keys (priv->options);
> + _key = g_list_nth_data (keys, idx);
> + _value = g_hash_table_lookup (priv->options, _key);
>
> - return NM_SETTING_BOND_GET_PRIVATE (setting)->downdelay;
> + if (out_key)
> + *out_key = _key;
> + if (out_value)
> + *out_value = _value;
> +
> + return TRUE;
> }
>
> /**
> - * nm_setting_bond_get_updelay:
> + * nm_setting_bond_get_option_by_key:
> * @setting: the #NMSettingBond
> + * @key: the key for which to retrieve the value
> + *
> + * Returns the value associated with the bonding option specified by
> + * @key, if it exists.
> *
> - * Returns: the #NMSettingBond:updelay property of the setting
> + * Returns: the value, or NULL if the key/value pair was never added to the
> + * setting; the value is owned by the setting and must not be modified
> **/
> -guint32
> -nm_setting_bond_get_updelay (NMSettingBond *setting)
> +const char *
> +nm_setting_bond_get_option_by_key (NMSettingBond *setting,
> + const char *key)
> {
> - g_return_val_if_fail (NM_IS_SETTING_BOND (setting), 0);
> + g_return_val_if_fail (NM_IS_SETTING_BOND (setting), NULL);
> + g_return_val_if_fail (key != NULL, NULL);
> + g_return_val_if_fail (strlen (key), NULL);
>
> - return NM_SETTING_BOND_GET_PRIVATE (setting)->updelay;
> + return g_hash_table_lookup (NM_SETTING_BOND_GET_PRIVATE (setting)->options, key);
> }
>
> /**
> - * nm_setting_bond_get_arp_interval:
> + * nm_setting_bond_add_options:
> * @setting: the #NMSettingBond
> + * @key: key name for the option
> + * @value: value for the option
> *
> - * Returns: the #NMSettingBond:arp_interval property of the setting
> + * Add an option to the table. The option is compared to an internal list
> + * of allowed options. Key names may contain only alphanumeric characters
> + * (ie [a-zA-Z0-9]). Adding a new key replaces any existing key/value pair that
> + * may already exist.
> + *
> + * Returns: %TRUE if the option was valid and was added to the internal option
> + * list, %FALSE if it was not.
> **/
> -guint32
> -nm_setting_bond_get_arp_interval (NMSettingBond *setting)
> +gboolean nm_setting_bond_add_option (NMSettingBond *setting,
> + const char *key,
> + const char *value)
> {
> - g_return_val_if_fail (NM_IS_SETTING_BOND (setting), 0);
> + size_t value_len;
>
> - return NM_SETTING_BOND_GET_PRIVATE (setting)->arp_interval;
> + g_return_val_if_fail (NM_IS_SETTING_BOND (setting), FALSE);
> + g_return_val_if_fail (key != NULL, FALSE);
> + g_return_val_if_fail (strlen (key), FALSE);
> + g_return_val_if_fail (_nm_utils_string_in_list (key, valid_opts), FALSE);
> + g_return_val_if_fail (value != NULL, FALSE);
> +
> + value_len = strlen (value);
> + g_return_val_if_fail (value_len > 0 && value_len < 200, FALSE);
> +
> + g_hash_table_insert (NM_SETTING_BOND_GET_PRIVATE (setting)->options,
> + g_strdup (key), g_strdup (value));
> + return TRUE;
> }
>
> /**
> - * nm_setting_bond_get_arp_ip_target:
> + * nm_setting_bond_remove_options:
> * @setting: the #NMSettingBond
> + * @key: key name for the option to remove
> *
> - * Returns: the #NMSettingBond:arp_ip_target property of the setting
> + * Remove the bonding option referenced by @key from the internal option
> + * list.
> + *
> + * Returns: %TRUE if the option was found and removed from the internal option
> + * list, %FALSE if it was not.
> **/
> -const char *
> -nm_setting_bond_get_arp_ip_target (NMSettingBond *setting)
> +gboolean
> +nm_setting_bond_remove_option (NMSettingBond *setting,
> + const char *key)
> {
> - g_return_val_if_fail (NM_IS_SETTING_BOND (setting), 0);
> + g_return_val_if_fail (NM_IS_SETTING_BOND (setting), FALSE);
> + g_return_val_if_fail (key != NULL, FALSE);
> + g_return_val_if_fail (strlen (key), FALSE);
>
> - return NM_SETTING_BOND_GET_PRIVATE (setting)->arp_ip_target;
> + return g_hash_table_remove (NM_SETTING_BOND_GET_PRIVATE (setting)->options, key);
> }
>
> /*
> @@ -249,6 +306,8 @@ static gboolean
> verify (NMSetting *setting, GSList *all_settings, GError **error)
> {
> NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (setting);
> + GHashTableIter iter;
> + const char *key, *value;
> const char *valid_modes[] = { "balance-rr",
> "active-backup",
> "balance-xor",
> @@ -274,15 +333,29 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
> return FALSE;
> }
>
> - if (priv->mode && !_nm_utils_string_in_list (priv->mode, valid_modes)) {
> - g_set_error (error,
> - NM_SETTING_BOND_ERROR,
> - NM_SETTING_BOND_ERROR_INVALID_PROPERTY,
> - NM_SETTING_BOND_MODE);
> - return FALSE;
> - }
> + g_hash_table_iter_init (&iter, priv->options);
> + while (g_hash_table_iter_next (&iter, (gpointer) &key, (gpointer) &value)) {
> + if ( !_nm_utils_string_in_list (key, valid_opts)
> + || !strlen (value)
> + || (strlen (value) > 200)) {
> + g_set_error (error,
> + NM_SETTING_BOND_ERROR,
> + NM_SETTING_BOND_ERROR_INVALID_PROPERTY,
> + NM_SETTING_BOND_OPTIONS);
> + return FALSE;
> + }
> +
> + if (!g_strcmp0 (key, "mode")
> + && !_nm_utils_string_in_list (value, valid_modes)) {
> + g_set_error (error,
> + NM_SETTING_BOND_ERROR,
> + NM_SETTING_BOND_ERROR_INVALID_PROPERTY,
> + NM_SETTING_BOND_OPTIONS);
> + return FALSE;
> + }
>
> - /* XXX: Validate arp-ip-target */
> + /* XXX: Validate arp-ip-target */
> + }
>
> return TRUE;
> }
> @@ -298,9 +371,15 @@ get_virtual_iface_name (NMSetting *setting)
> static void
> nm_setting_bond_init (NMSettingBond *setting)
> {
> + NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (setting);
> +
> g_object_set (setting, NM_SETTING_NAME, NM_SETTING_BOND_SETTING_NAME,
> - NM_SETTING_BOND_MIIMON, 100, /* default: miimon=100 */
> NULL);
> +
> + priv->options = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
> +
> + /* Default values: */
> + nm_setting_bond_add_option (setting, "miimon", "100");
> }
>
> static void
> @@ -309,40 +388,34 @@ finalize (GObject *object)
> NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (object);
>
> g_free (priv->interface_name);
> - g_free (priv->mode);
> - g_free (priv->arp_ip_target);
> + g_hash_table_destroy (priv->options);
>
> G_OBJECT_CLASS (nm_setting_bond_parent_class)->finalize (object);
> }
>
> static void
> +copy_hash (gpointer key, gpointer value, gpointer user_data)
> +{
> + g_hash_table_insert ((GHashTable *) user_data, g_strdup (key), g_strdup (value));
> +}
> +
> +static void
> set_property (GObject *object, guint prop_id,
> const GValue *value, GParamSpec *pspec)
> {
> NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (object);
> + GHashTable *new_hash;
>
> switch (prop_id) {
> case PROP_INTERFACE_NAME:
> priv->interface_name = g_value_dup_string (value);
> break;
> - case PROP_MODE:
> - priv->mode = g_value_dup_string (value);
> - break;
> - case PROP_MIIMON:
> - priv->miimon = g_value_get_uint (value);
> - break;
> - case PROP_DOWNDELAY:
> - priv->downdelay = g_value_get_uint (value);
> - break;
> - case PROP_UPDELAY:
> - priv->updelay = g_value_get_uint (value);
> - break;
> - case PROP_ARP_INTERVAL:
> - priv->arp_interval = g_value_get_uint (value);
> - break;
> - case PROP_ARP_IP_TARGET:
> - g_free (priv->arp_ip_target);
> - priv->arp_ip_target = g_value_dup_string (value);
> + case PROP_OPTIONS:
> + /* Must make a deep copy of the hash table here... */
> + g_hash_table_remove_all (priv->options);
> + new_hash = g_value_get_boxed (value);
> + if (new_hash)
> + g_hash_table_foreach (new_hash, copy_hash, priv->options);
> break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
> @@ -354,30 +427,16 @@ static void
> get_property (GObject *object, guint prop_id,
> GValue *value, GParamSpec *pspec)
> {
> + NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (object);
> NMSettingBond *setting = NM_SETTING_BOND (object);
>
> switch (prop_id) {
> case PROP_INTERFACE_NAME:
> g_value_set_string (value, nm_setting_bond_get_interface_name (setting));
> break;
> - case PROP_MODE:
> - g_value_set_string (value, nm_setting_bond_get_mode (setting));
> - break;
> - case PROP_MIIMON:
> - g_value_set_uint (value, nm_setting_bond_get_miimon (setting));
> - break;
> - case PROP_DOWNDELAY:
> - g_value_set_uint (value, nm_setting_bond_get_downdelay (setting));
> - break;
> - case PROP_UPDELAY:
> - g_value_set_uint (value, nm_setting_bond_get_updelay (setting));
> - break;
> - case PROP_ARP_INTERVAL:
> - g_value_set_uint (value, nm_setting_bond_get_arp_interval (setting));
> - break;
> - case PROP_ARP_IP_TARGET:
> - g_value_set_string (value, nm_setting_bond_get_arp_ip_target (setting));
> - break;
> + case PROP_OPTIONS:
> + g_value_set_boxed (value, priv->options);
> + break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
> break;
> @@ -414,159 +473,18 @@ nm_setting_bond_class_init (NMSettingBondClass *setting_class)
> G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE));
>
> /**
> - * NMSettingBond:mode:
> - *
> - * Bonding policy
> - **/
> - g_object_class_install_property
> - (object_class, PROP_MODE,
> - g_param_spec_string (NM_SETTING_BOND_MODE,
> - "Mode",
> - "The bonding policy to use. One of 'balance-rr' (default), "
> - "'active-backup', 'balance-xor', 'broadcast', '802.3ad', "
> - "'balance-tlb', 'balance-alb'.",
> - NULL,
> - G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE));
> -
> - /**
> - * NMSettingBond:miimon:
> + * NMSettingBridge:options:
> *
> - * Specifies the MII link monitoring frequency in milliseconds.
> - * This determines how often the link state of each slave is
> - * inspected for link failures. A value of zero disables MII
> - * link monitoring. A value of 100 is a good starting point.
> - * The use_carrier option, below, affects how the link state is
> - * determined.
> + * Dictionary of key/value pairs of bridging options. Both keys
> + * and values must be strings. Key names must contain only
> + * alphanumeric characters (ie, [a-zA-Z0-9]).
> **/
> - g_object_class_install_property
> - (object_class, PROP_MIIMON,
> - g_param_spec_uint (NM_SETTING_BOND_MIIMON,
> - "MiiMon",
> - "Specifies the MII link monitoring frequency in milliseconds. "
> - "This determines how often the link state of each slave is "
> - "inspected for link failures. A value of zero disables MII "
> - "link monitoring. A value of 100 is a good starting point. "
> - "The use_carrier option, below, affects how the link state is "
> - "determined. The default value is 0.",
> - 0, G_MAXUINT32, 100,
> - G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE));
> -
> - /**
> - * NMSettingBond:downdelay:
> - *
> - * Specifies the time, in milliseconds, to wait before disabling
> - * a slave after a link failure has been detected. This option
> - * is only valid for the miimon link monitor. The downdelay
> - * value should be a multiple of the miimon value; if not, it
> - * will be rounded down to the nearest multiple. The default
> - * value is 0.
> - **/
> - g_object_class_install_property
> - (object_class, PROP_DOWNDELAY,
> - g_param_spec_uint (NM_SETTING_BOND_DOWNDELAY,
> - "DownDelay",
> - "Specifies the time, in milliseconds, to wait before disabling "
> - "a slave after a link failure has been detected. This option "
> - "is only valid for the miimon link monitor. The downdelay "
> - "value should be a multiple of the miimon value; if not, it "
> - "will be rounded down to the nearest multiple. The default "
> - "value is 0.",
> - 0, G_MAXUINT32, 0,
> - G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE));
> -
> - /**
> - * NMSettingBond:updelay:
> - *
> - * Specifies the time, in milliseconds, to wait before enabling a
> - * slave after a link recovery has been detected. This option is
> - * only valid for the miimon link monitor. The updelay value
> - * should be a multiple of the miimon value; if not, it will be
> - * rounded down to the nearest multiple. The default value is 0.
> - **/
> - g_object_class_install_property
> - (object_class, PROP_UPDELAY,
> - g_param_spec_uint (NM_SETTING_BOND_UPDELAY,
> - "UpDelay",
> - "Specifies the time, in milliseconds, to wait before enabling a "
> - "slave after a link recovery has been detected. This option is "
> - "only valid for the miimon link monitor. The updelay value "
> - "should be a multiple of the miimon value; if not, it will be "
> - "rounded down to the nearest multiple. The default value is 0.",
> - 0, G_MAXUINT32, 0,
> - G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE));
> -
> - /**
> - * NMSettingBond:arp_interval:
> - *
> - * Specifies the ARP link monitoring frequency in milliseconds.
> - *
> - * The ARP monitor works by periodically checking the slave
> - * devices to determine whether they have sent or received
> - * traffic recently (the precise criteria depends upon the
> - * bonding mode, and the state of the slave). Regular traffic is
> - * generated via ARP probes issued for the addresses specified by
> - * the arp-ip-target option.
> - *
> - * This behavior can be modified by the arp-validate option.
> - *
> - * If ARP monitoring is used in an etherchannel compatible mode
> - * (modes 0 and 2), the switch should be configured in a mode
> - * that evenly distributes packets across all links. If the
> - * switch is configured to distribute the packets in an XOR
> - * fashion, all replies from the ARP targets will be received on
> - * the same link which could cause the other team members to
> - * fail. ARP monitoring should not be used in conjunction with
> - * miimon. A value of 0 disables ARP monitoring. The default
> - * value is 0.
> - **/
> - g_object_class_install_property
> - (object_class, PROP_ARP_INTERVAL,
> - g_param_spec_uint (NM_SETTING_BOND_ARP_INTERVAL,
> - "ArpInterval",
> - "Specifies the ARP link monitoring frequency in milliseconds. "
> - "The ARP monitor works by periodically checking the slave "
> - "devices to determine whether they have sent or received "
> - "traffic recently (the precise criteria depends upon the "
> - "bonding mode, and the state of the slave). Regular traffic is "
> - "generated via ARP probes issued for the addresses specified by "
> - "the arp-ip-target option. "
> - "This behavior can be modified by the arp-validate option. "
> - "If ARP monitoring is used in an etherchannel compatible mode "
> - "(modes 0 and 2), the switch should be configured in a mode "
> - "that evenly distributes packets across all links. If the "
> - "switch is configured to distribute the packets in an XOR "
> - "fashion, all replies from the ARP targets will be received on "
> - "the same link which could cause the other team members to "
> - "fail. ARP monitoring should not be used in conjunction with "
> - "miimon. A value of 0 disables ARP monitoring. The default "
> - "value is 0.",
> - 0, G_MAXUINT32, 0,
> - G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE));
> -
> - /**
> - * NMSettingBond:arp_ip_target:
> - *
> - * Specifies the IP addresses to use as ARP monitoring peers when
> - * arp_interval is > 0. These are the targets of the ARP request
> - * sent to determine the health of the link to the targets.
> - * Specify these values in ddd.ddd.ddd.ddd format. Multiple IP
> - * addresses must be separated by a comma. At least one IP
> - * address must be given for ARP monitoring to function. The
> - * maximum number of targets that can be specified is 16. The
> - * default value is no IP addresses.
> - **/
> - g_object_class_install_property
> - (object_class, PROP_ARP_IP_TARGET,
> - g_param_spec_string (NM_SETTING_BOND_ARP_IP_TARGET,
> - "ArpIpTarget",
> - "Specifies the IP addresses to use as ARP monitoring peers when "
> - "arp-interval is > 0. These are the targets of the ARP request "
> - "sent to determine the health of the link to the targets. "
> - "Specify these values in ddd.ddd.ddd.ddd format. Multiple IP "
> - "addresses must be separated by a comma. At least one IP "
> - "address must be given for ARP monitoring to function. The "
> - "maximum number of targets that can be specified is 16. The "
> - "default value is no IP addresses.",
> - NULL,
> - G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE));
> + g_object_class_install_property
> + (object_class, PROP_OPTIONS,
> + _nm_param_spec_specialized (NM_SETTING_BOND_OPTIONS,
> + "Options",
> + "Dictionary of key/value pairs of bonding options. "
> + "Both keys and values must be strings.",
> + DBUS_TYPE_G_MAP_OF_STRING,
> + G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE));
> }
> diff --git a/libnm-util/nm-setting-bond.h b/libnm-util/nm-setting-bond.h
> index 6ef6992..49926df 100644
> --- a/libnm-util/nm-setting-bond.h
> +++ b/libnm-util/nm-setting-bond.h
> @@ -57,12 +57,7 @@ GType nm_setting_bond_error_get_type (void);
> GQuark nm_setting_bond_error_quark (void);
>
> #define NM_SETTING_BOND_INTERFACE_NAME "interface-name"
> -#define NM_SETTING_BOND_MODE "mode"
> -#define NM_SETTING_BOND_MIIMON "miimon"
> -#define NM_SETTING_BOND_DOWNDELAY "down-delay"
> -#define NM_SETTING_BOND_UPDELAY "up-delay"
> -#define NM_SETTING_BOND_ARP_INTERVAL "arp-interval"
> -#define NM_SETTING_BOND_ARP_IP_TARGET "arp-ip-target"
> +#define NM_SETTING_BOND_OPTIONS "options"
>
> typedef struct {
> NMSetting parent;
> @@ -82,12 +77,18 @@ GType nm_setting_bond_get_type (void);
>
> NMSetting * nm_setting_bond_new (void);
> const char * nm_setting_bond_get_interface_name (NMSettingBond *setting);
> -const char * nm_setting_bond_get_mode (NMSettingBond *setting);
> -guint32 nm_setting_bond_get_miimon (NMSettingBond *setting);
> -guint32 nm_setting_bond_get_downdelay (NMSettingBond *setting);
> -guint32 nm_setting_bond_get_updelay (NMSettingBond *setting);
> -guint32 nm_setting_bond_get_arp_interval (NMSettingBond *setting);
> -const char * nm_setting_bond_get_arp_ip_target (NMSettingBond *setting);
> +guint32 nm_setting_bond_get_num_options (NMSettingBond *setting);
> +gboolean nm_setting_bond_get_option (NMSettingBond *setting,
> + guint32 idx,
> + const char **out_key,
> + const char **out_value);
> +const char * nm_setting_bond_get_option_by_key (NMSettingBond *setting,
> + const char *key);
> +gboolean nm_setting_bond_add_option (NMSettingBond *setting,
> + const char *key,
> + const char *item);
> +gboolean nm_setting_bond_remove_option (NMSettingBond *setting,
> + const char *key);
>
> G_END_DECLS
>
> diff --git a/src/nm-system.c b/src/nm-system.c
> index 43f43cf..d0613ae 100644
> --- a/src/nm-system.c
> +++ b/src/nm-system.c
> @@ -1238,28 +1238,15 @@ set_bond_attr (const char *iface, const char *attr, const char *value)
> return ret;
> }
>
> -static gboolean
> -set_bond_attr_int (const char *iface, const char *attr,
> - guint32 value)
> -{
> - char buf[128];
> -
> - snprintf (buf, sizeof(buf), "%u", value);
> -
> - return set_bond_attr (iface, attr, buf);
> -}
> -
> gboolean
> nm_system_apply_bonding_config (NMSettingBond *s_bond)
> {
> - const char *name, *val;
> + const char *name;
> + guint32 i;
>
> name = nm_setting_bond_get_interface_name (s_bond);
> g_assert (name);
>
> - if ((val = nm_setting_bond_get_mode (s_bond)))
> - set_bond_attr (name, "mode", val);
> -
> /*
> * FIXME:
> *
> @@ -1278,13 +1265,15 @@ nm_system_apply_bonding_config (NMSettingBond *s_bond)
> * Not sure if this is actually being used and it seems dangerous as
> * the result is pretty much unforeseeable.
> */
> - if ((val = nm_setting_bond_get_arp_ip_target (s_bond)))
> - set_bond_attr (name, "arp_ip_target", val);
>
> - set_bond_attr_int (name, "miimon", nm_setting_bond_get_miimon (s_bond));
> - set_bond_attr_int (name, "downdelay", nm_setting_bond_get_downdelay (s_bond));
> - set_bond_attr_int (name, "updelay", nm_setting_bond_get_updelay (s_bond));
> - set_bond_attr_int (name, "arp_interval", nm_setting_bond_get_arp_interval (s_bond));
> + for (i = 0; i < nm_setting_bond_get_num_options (s_bond); i++) {
> + const char *key, *value;
> +
> + if (!nm_setting_bond_get_option (s_bond, i, &key, &value))
> + continue;
> +
> + set_bond_attr (name, key, value);
> + }
>
> return TRUE;
> }
> diff --git a/src/settings/plugins/ifcfg-rh/reader.c b/src/settings/plugins/ifcfg-rh/reader.c
> index 15a8629..0aa1e75 100644
> --- a/src/settings/plugins/ifcfg-rh/reader.c
> +++ b/src/settings/plugins/ifcfg-rh/reader.c
> @@ -3566,19 +3566,7 @@ handle_bond_option (NMSettingBond *s_bond,
> const char *key,
> const char *value)
> {
> - if (!g_strcmp0 (key, "mode"))
> - g_object_set (s_bond, NM_SETTING_BOND_MODE, value, NULL);
> - else if (!g_strcmp0 (key, "miimon"))
> - g_object_set (s_bond, NM_SETTING_BOND_MIIMON, strtoul(value, NULL, 0), NULL);
> - else if (!g_strcmp0 (key, "updelay"))
> - g_object_set (s_bond, NM_SETTING_BOND_UPDELAY, strtoul(value, NULL, 0), NULL);
> - else if (!g_strcmp0 (key, "downdelay"))
> - g_object_set (s_bond, NM_SETTING_BOND_DOWNDELAY, strtoul(value, NULL, 0), NULL);
> - else if (!g_strcmp0 (key, "arp_interval"))
> - g_object_set (s_bond, NM_SETTING_BOND_ARP_INTERVAL, strtoul(value, NULL, 0), NULL);
> - else if (!g_strcmp0 (key, "arp_ip_target"))
> - g_object_set (s_bond, NM_SETTING_BOND_ARP_IP_TARGET, value, NULL);
> - else
> + if (!nm_setting_bond_add_option (s_bond, key, value))
> PLUGIN_WARN (IFCFG_PLUGIN_NAME, " warning: invalid bonding option '%s'", key);
> }
>
> 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 3515b83..5f2ea8a 100644
> --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
> +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
> @@ -11817,9 +11817,9 @@ test_read_bond_main (void)
> "bond-main", "failed to verify %s: DEVICE=%s does not match bond0",
> TEST_IFCFG_BOND_MAIN, nm_setting_bond_get_interface_name (s_bond));
>
> - ASSERT (nm_setting_bond_get_miimon (s_bond) == 100,
> - "bond-main", "failed to verify %s: miimon=%d does not match 100",
> - TEST_IFCFG_BOND_MAIN, nm_setting_bond_get_miimon (s_bond));
> + ASSERT (g_strcmp0 (nm_setting_bond_get_option_by_key (s_bond, "miimon"), "100") == 0,
> + "bond-main", "failed to verify %s: miimon=%s does not match 100",
> + TEST_IFCFG_BOND_MAIN, nm_setting_bond_get_option_by_key (s_bond, "miimon"));
>
> g_free (unmanaged);
> g_free (keyfile);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
