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 <tg...@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);


_______________________________________________
networkmanager-list mailing list
networkmanager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list

Reply via email to