Hi Dan, On Thu, 2013-11-14 at 19:57 -0600, Dan Williams wrote: > I've pushed my experiments here to dcbw/bondopts, enough that it > compiles and passes 'make check' in libnm-util, although we certainly > don't have enough testcase coverage of bonding to make sure it works. > > My main attempts were to use the normal GObject property ParamSpecs for > min/max validation and all defaults, and to reduce some of the legacy > property complexity by just exposing all the properties in the old > 'options' property. > > Thoughts?
I like it. I was looking at it all and changed the parts, where I think it should be different. Patch attached. Thomas
From ce640b2637130d045f79bec564479fbf41b0cb65 Mon Sep 17 00:00:00 2001 From: Thomas Haller <[email protected]> Date: Fri, 15 Nov 2013 11:56:57 +0100 Subject: [PATCH 1/1] fixup! bond: add proper properties to NMSettingBond, deprecate "options" [WIP] Signed-off-by: Thomas Haller <[email protected]> --- libnm-util/nm-setting-bond.c | 385 +++++++++++++++++++++++++++++++------------ 1 file changed, 280 insertions(+), 105 deletions(-) diff --git a/libnm-util/nm-setting-bond.c b/libnm-util/nm-setting-bond.c index 1177451..e1160ac 100644 --- a/libnm-util/nm-setting-bond.c +++ b/libnm-util/nm-setting-bond.c @@ -63,100 +63,103 @@ nm_setting_bond_error_quark (void) return quark; } G_DEFINE_TYPE_WITH_CODE (NMSettingBond, nm_setting_bond, NM_TYPE_SETTING, _nm_register_setting (NM_SETTING_BOND_SETTING_NAME, g_define_type_id, 1, NM_SETTING_BOND_ERROR)) NM_SETTING_REGISTER_TYPE (NM_TYPE_SETTING_BOND) #define NM_SETTING_BOND_GET_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o), NM_TYPE_SETTING_BOND, NMSettingBondPrivate)) typedef struct { char *interface_name; + char *mode; - guint miimon; - guint downdelay; - guint updelay; - guint arp_interval; + int miimon; + int downdelay; + int updelay; + int arp_interval; char **arp_ip_target; char *arp_validate; char *primary; char *primary_reselect; char *fail_over_mac; + int use_carrier; char *ad_select; char *xmit_hash_policy; - gboolean use_carrier; - guint resend_igmp; + int resend_igmp; 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_ARP_VALIDATE, PROP_PRIMARY, PROP_PRIMARY_RESELECT, PROP_FAIL_OVER_MAC, PROP_USE_CARRIER, PROP_AD_SELECT, PROP_XMIT_HASH_POLICY, PROP_RESEND_IGMP, PROP_OPTIONS, LAST_PROP }; #define _FIRST_LEGACY_PROP PROP_MODE #define _LAST_LEGACY_PROP PROP_RESEND_IGMP enum { + TYPE_NONE, /* must be 0, so that it is the default in props if not explicitly set. */ TYPE_INT, TYPE_STR, TYPE_BOTH, TYPE_IP, TYPE_IFNAME, }; typedef struct { guint opt_type; const char *legacy_name; - const char *list[10]; + const char *list[7]; + const char *list_sentinel[1]; /* dummy, to NULL terminate the previous 'list' array */ GParamSpec *pspec; char *defval; } BondProperty; -static BondProperty props[ LAST_PROP + 1 ] = { +static BondProperty props[LAST_PROP] = { [PROP_MODE] = { TYPE_BOTH, NM_SETTING_BOND_OPTION_MODE, { "balance-rr", "active-backup", "balance-xor", "broadcast", "802.3ad", "balance-tlb", "balance-alb" } }, [PROP_MIIMON] = { TYPE_INT, NM_SETTING_BOND_OPTION_MIIMON }, [PROP_DOWNDELAY] = { TYPE_INT, NM_SETTING_BOND_OPTION_DOWNDELAY }, [PROP_UPDELAY] = { TYPE_INT, NM_SETTING_BOND_OPTION_UPDELAY }, [PROP_ARP_INTERVAL] = { TYPE_INT, NM_SETTING_BOND_OPTION_ARP_INTERVAL }, [PROP_ARP_IP_TARGET] = { TYPE_IP, NM_SETTING_BOND_OPTION_ARP_IP_TARGET }, [PROP_ARP_VALIDATE] = { TYPE_BOTH, NULL, { "none", "active", "backup", "all" } }, - [PROP_PRIMARY] = { TYPE_IFNAME, NULL }, + [PROP_PRIMARY] = { TYPE_IFNAME }, [PROP_PRIMARY_RESELECT] = { TYPE_BOTH, NULL, { "always", "better", "failure" } }, [PROP_FAIL_OVER_MAC] = { TYPE_BOTH, NULL, { "none", "active", "follow" } }, [PROP_USE_CARRIER] = { TYPE_INT }, [PROP_AD_SELECT] = { TYPE_BOTH, NULL, { "stable", "bandwidth", "count" } }, [PROP_XMIT_HASH_POLICY] = { TYPE_STR, NULL, { "layer2", "layer2+3", "layer3+4", "encap2+3", "encap3+4" } }, [PROP_RESEND_IGMP] = { TYPE_INT }, }; /** * nm_setting_bond_new: * @@ -402,92 +405,106 @@ nm_setting_bond_get_xmit_hash_policy (NMSettingBond *setting) * Returns: the #NMSettingBond:resend-igmp property of the setting * * Since: 0.9.10 **/ guint nm_setting_bond_get_resend_igmp (NMSettingBond *setting) { g_return_val_if_fail (NM_IS_SETTING_BOND (setting), 0); return NM_SETTING_BOND_GET_PRIVATE (setting)->resend_igmp; } /*****************************************************************************/ static BondProperty * -find_property (const char *name, const char **out_new_name, guint *out_idx) +find_property_by_pspec (const GParamSpec *pspec, guint *out_idx, gboolean legacy_only) { - guint i; + guint i = legacy_only ? _FIRST_LEGACY_PROP : PROP_0 + 1; + guint end = legacy_only ? _LAST_LEGACY_PROP + 1 : LAST_PROP; + + g_return_val_if_fail (pspec != NULL, NULL); + + for (; i < end; i++) { + if (props[i].pspec == pspec) { + if (out_idx) + *out_idx = i; + return &props[i]; + } + } + if (out_idx) + *out_idx = LAST_PROP; + return NULL; +} + +/* Depending on legacy_only, this only finds properties that are exposed as legacy options. */ +static BondProperty * +find_property_by_name (const char *name, guint *out_idx, gboolean legacy_only) +{ + guint i = legacy_only ? _FIRST_LEGACY_PROP : PROP_0 + 1; + guint end = legacy_only ? _LAST_LEGACY_PROP + 1 : LAST_PROP; g_return_val_if_fail (name != NULL, NULL); - for (i = _FIRST_LEGACY_PROP; i <= _LAST_LEGACY_PROP; i++) { + for (; i < end; i++) { const char *new_name = g_param_spec_get_name (props[i].pspec); if (strcmp (name, new_name) == 0 || g_strcmp0 (name, props[i].legacy_name) == 0) { - if (out_new_name) - *out_new_name = new_name; if (out_idx) *out_idx = i; return &props[i]; } } + if (out_idx) + *out_idx = LAST_PROP; return NULL; } /* For a new property or legacy option name, returns the new property name */ static const char * -get_property_name (const char *name, const BondProperty **out_prop) +get_property_name (const BondProperty *prop) { - const char *new_name = NULL; - - *out_prop = find_property (name, &new_name, NULL); - return *out_prop ? new_name : NULL; + return prop ? g_param_spec_get_name (prop->pspec) : NULL; } /* For a new property or legacy option name, returns the legacy option name */ static const char * -get_legacy_name (GParamSpec *pspec) +get_legacy_name (const BondProperty *prop) { - const BondProperty *prop; - const char *new_name = NULL; - - prop = find_property (g_param_spec_get_name (pspec), &new_name, NULL); - if (prop) - return prop->legacy_name ? prop->legacy_name : new_name; - - return NULL; + if (!prop) + return NULL; + return prop->legacy_name ? prop->legacy_name : g_param_spec_get_name (prop->pspec); } /** * nm_setting_bond_get_num_options: * @setting: the #NMSettingBond * * Returns the number of options that are set in the legacy * #NMSettingBond:options property. This does not include other bond * properties which are not included in #NMSettingBond:options. * * Returns: the number of legacy bonding options * * Deprecated: use the option-specific getters instead. **/ guint32 nm_setting_bond_get_num_options (NMSettingBond *setting) { g_return_val_if_fail (NM_IS_SETTING_BOND (setting), 0); - return _LAST_LEGACY_PROP - _FIRST_LEGACY_PROP; + return _LAST_LEGACY_PROP - _FIRST_LEGACY_PROP + 1; } /** * 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_name: (out): on return, the 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 name 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(), @@ -495,118 +512,119 @@ nm_setting_bond_get_num_options (NMSettingBond *setting) * such as during option iteration. * * 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) * * Deprecated: use the option-specific getters instead. **/ gboolean nm_setting_bond_get_option (NMSettingBond *setting, guint32 idx, const char **out_name, const char **out_value) { NMSettingBondPrivate *priv; - const char *legacy_name, *value; + const char *legacy_name = NULL, *value = NULL; + gboolean result = FALSE; g_return_val_if_fail (NM_IS_SETTING_BOND (setting), FALSE); priv = NM_SETTING_BOND_GET_PRIVATE (setting); + if (idx >= _LAST_LEGACY_PROP - _FIRST_LEGACY_PROP) + goto out; idx += _FIRST_LEGACY_PROP; - g_return_val_if_fail (idx <= _LAST_LEGACY_PROP, FALSE); - legacy_name = get_legacy_name (props[idx].pspec); + legacy_name = get_legacy_name (&props[idx]); g_assert (legacy_name); - value = g_hash_table_lookup (priv->options, legacy_name); - if (!value) - return FALSE; + result = g_hash_table_lookup_extended (priv->options, legacy_name, NULL, (void **) &value); + g_assert (result); +out: if (out_name) *out_name = legacy_name; if (out_value) *out_value = value; - return TRUE; + return result; } static gboolean int_from_string (const char *s, glong *out_num) { - guint i; - - for (i = 0; i < strlen (s); i++) { - if (!g_ascii_isdigit (s[i]) && s[i] != '-') - return FALSE; - } + long int n; + if (!s) + return FALSE; errno = 0; - *out_num = strtol (s, NULL, 10); - return errno ? FALSE : TRUE; + n = strtol (s, NULL, 10); + if (out_num) + *out_num = n; + return !!errno; } static gboolean -validate_int (const BondProperty *prop, const char *value) +validate_int (const BondProperty *prop, const char *value, int *out_num) { GParamSpecInt *ispec; glong num = 0; + gboolean success = FALSE; - if (!G_IS_PARAM_SPEC_INT (prop->pspec)) - return FALSE; - if (!value) - return FALSE; + g_assert (G_IS_PARAM_SPEC_INT (prop->pspec)); if (!int_from_string (value, &num)) - return FALSE; + goto out; ispec = G_PARAM_SPEC_INT (prop->pspec); - return (num >= ispec->minimum && num <= ispec->maximum); + success = (num >= ispec->minimum && num <= ispec->maximum); +out: + if (out_num) + *out_num = success ? num : 0; + return success; } static gboolean validate_list (const BondProperty *prop, const char *value) { - guint i; - - if (!value) - return FALSE; + const char *const*ptr; - for (i = 0; i < G_N_ELEMENTS (prop->list) && prop->list[i]; i++) { - if (g_strcmp0 (prop->list[i], value) == 0) - return TRUE; + if (value) { + for (ptr = prop->list; *ptr; ptr++) { + if (strcmp (*ptr, value) == 0) + return TRUE; + } } - - /* empty validation list means all values pass */ - return prop->list[0] == NULL ? TRUE : FALSE; + return FALSE; } +/* by making it a macro, we don't have to worry about using glong as type of idx (otherwise, we would have to check for integer overflow too. */ +#define IS_VALID_LIST_INDEX(prop, idx) ( ((idx) >= 0) && ((idx) < g_strv_length ((char **) (prop)->list)) ) + static gboolean validate_both (const BondProperty *prop, const char *value) { glong num = -1; - g_assert (prop->list); - if (!value) return FALSE; if (validate_list (prop, value)) return TRUE; if (!int_from_string (value, &num)) return FALSE; /* Ensure number is within bounds of string list */ - return num >= 0 && num < G_N_ELEMENTS (prop->list); + return IS_VALID_LIST_INDEX (prop, num); } static char ** parse_ip (const char *value, gboolean warn_on_error) { char **ips, **iter; struct in_addr addr; if (!value || !value[0]) { /* missing value is valid, we just NULL instead of an empty array. */ return NULL; } ips = g_strsplit_set (value, ",", 0); for (iter = ips; *iter; iter++) { @@ -651,253 +669,280 @@ validate_ifname (const char *value) } return nm_utils_iface_valid_name (value); } /* Checks whether @value is is a valid value for @prop. * * Returns: TRUE, if the @value is valid for the given name. * If @value is NULL, false will be returned. **/ static gboolean validate_property (const BondProperty *prop, const char *value) { switch (prop->opt_type) { case TYPE_INT: - return validate_int (prop, value); + return validate_int (prop, value, NULL); case TYPE_STR: return validate_list (prop, value); case TYPE_BOTH: return validate_both (prop, value); case TYPE_IP: return validate_ip (value); case TYPE_IFNAME: return validate_ifname (value); + case TYPE_NONE: default: g_assert_not_reached(); } return FALSE; } /** * nm_setting_bond_get_option_by_name: * @setting: the #NMSettingBond * @name: the option name for which to retrieve the value * * Returns the value associated with the bonding option specified by * @name, if it exists. * * 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 * * Deprecated: use the option-specific getters instead. **/ const char * nm_setting_bond_get_option_by_name (NMSettingBond *setting, const char *name) { + NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (setting); + const char *value; + gboolean result; + BondProperty *prop; + g_return_val_if_fail (NM_IS_SETTING_BOND (setting), NULL); - return g_hash_table_lookup (NM_SETTING_BOND_GET_PRIVATE (setting)->options, name); + result = g_hash_table_lookup_extended (priv->options, name, NULL, (void **) &value); + if (result) + return value; + + /* Try to lookup by alias name (new name vs. legacy name)... */ + prop = find_property_by_name (name, NULL, TRUE); + if (!prop) + return NULL; + + /* 'name' is a valid property name, but we did not find it in the options hash. + * Since every element *must* be in the options hash, this can only mean, that + * the user tried to lookup by new_name for items that have a different + * legacy_name. Support lookup by this alias. */ + result = g_hash_table_lookup_extended (priv->options, get_legacy_name (prop), NULL, (void **) &value); + g_assert (result); + + return value; } /** * nm_setting_bond_add_option: * @setting: the #NMSettingBond * @name: name for the option * @value: value for the option * * Add an option to the table. The option is compared to an internal list * of allowed options. Option names may contain only alphanumeric characters - * (ie [a-zA-Z0-9]). Adding a new name replaces any existing name/value pair + * (ie [a-zA-Z0-9_]). Adding a new name replaces any existing name/value pair * that may already exist. * * The order of how to set several options is relevant because there are options * that conflict with each other. * * Returns: %TRUE if the option was valid and was added to the internal option * list, %FALSE if it was not. * * Deprecated: use the option-specific properties instead. **/ gboolean nm_setting_bond_add_option (NMSettingBond *setting, const char *name, const char *value) { NMSettingBondPrivate *priv; GObject *object = G_OBJECT (setting); const BondProperty *prop = NULL; const char *prop_name; glong num = 0; g_return_val_if_fail (NM_IS_SETTING_BOND (setting), FALSE); priv = NM_SETTING_BOND_GET_PRIVATE (setting); - prop_name = get_property_name (name, &prop); + prop = find_property_by_name (name, NULL, TRUE); + prop_name = get_property_name (prop); if (!prop_name) return FALSE; if (!validate_property (prop, value)) return FALSE; g_object_freeze_notify (object); switch (prop->opt_type) { case TYPE_INT: - if (!int_from_string (value, &num)) - return FALSE; + if (int_from_string (value, &num)) + g_assert_not_reached (); g_object_set (object, prop_name, (gint) num, NULL); break; case TYPE_BOTH: { const char *str_value = value; /* Might be an integer-as-string; find the string */ - if (!validate_list (prop, value)) { - if (!int_from_string (value, &num)) - return FALSE; - /* Be paranoid although it's already been checked by validate_property() */ - g_assert (num >= 0 && num < G_N_ELEMENTS (prop->list)); + if (int_from_string (value, &num)) { + /* FIXME: do we really want to coerce the value? verify() currently accepts numeric values + * for the TYPE_BOTH items. NMDeviceBond has to cope with the ambiguity of the options + * names anyway. It might be better, to support the same names as the kernel does, + * including numeric values. Also, when reading the value from sysfs, we will also + * encounter numeric values (so, either we ~always~ coerce -> set_property), or not at all. + **/ str_value = prop->list[num]; } g_object_set (object, prop_name, str_value, NULL); break; } case TYPE_IFNAME: case TYPE_STR: g_object_set (object, prop_name, value, NULL); break; case TYPE_IP: { char **ip = parse_ip (value, TRUE); g_object_set (object, prop_name, ip, NULL); g_strfreev (ip); break; } + case TYPE_NONE: default: g_assert_not_reached (); break; } g_object_thaw_notify (object); return TRUE; } /** * nm_setting_bond_remove_option: * @setting: the #NMSettingBond * @name: name of the option to remove * * Remove the bonding option referenced by @name from the internal option - * list. + * list. As the option list is deprected, you can no longer actually remove + * an item from the option hash. Removing it is now equivalent to resetting + * the default value. * * Returns: %TRUE if the option was found and removed from the internal option * list, %FALSE if it was not. * * Deprecated: use the option-specific properties instead. **/ gboolean nm_setting_bond_remove_option (NMSettingBond *setting, const char *name) { GObject *object = G_OBJECT (setting); NMSettingBondPrivate *priv; const BondProperty *prop; const char *prop_name; GValue defval = G_VALUE_INIT; g_return_val_if_fail (NM_IS_SETTING_BOND (setting), FALSE); priv = NM_SETTING_BOND_GET_PRIVATE (setting); - prop_name = get_property_name (name, &prop); + prop = find_property_by_name (name, NULL, TRUE); + prop_name = get_property_name (prop); if (!prop_name) return FALSE; g_object_freeze_notify (object); - /* we don't really remove the property, instead we reset the default value. */ + /* We don't really remove the property, instead we reset the default value. + * Thus, the number of options is always constant (all of them) and the option + * hash always contains every (legacy) option. */ g_value_init (&defval, prop->pspec->value_type); g_param_value_set_default (prop->pspec, &defval); g_object_set_property (object, prop_name, &defval); g_value_unset (&defval); g_object_thaw_notify (object); return TRUE; } /** * nm_setting_bond_get_valid_options: * @setting: the #NMSettingBond * * Returns a list of valid bond options. * * Returns: (transfer none): a %NULL-terminated array of strings of valid bond options. * * Deprecated: the valid options are defined by the #NMSettingBond * properties. **/ const char ** nm_setting_bond_get_valid_options (NMSettingBond *setting) { - static const char *array[LAST_PROP + 1] = { NULL }; + static const char *array[_LAST_LEGACY_PROP - _FIRST_LEGACY_PROP + 1 + 1] = { NULL }; /* initialize the array once */ if (G_UNLIKELY (array[0] == NULL)) { guint prop, i; - for (prop = _FIRST_LEGACY_PROP, i = 0; prop <= _LAST_LEGACY_PROP; prop++, i++) { - array[i] = props[prop].legacy_name ? - props[prop].legacy_name : g_param_spec_get_name (props[prop].pspec); - } + for (prop = _FIRST_LEGACY_PROP, i = 0; prop <= _LAST_LEGACY_PROP; prop++, i++) + array[i] = get_legacy_name (&props[prop]); } return array; } /** * nm_setting_bond_get_option_default: * @setting: the #NMSettingBond * @name: the name of the option * * Returns: the value of the bond option if not overridden by an entry in * the #NMSettingBond:options property. * * Deprecated: Use the default values of the option-specific properties. **/ const char * nm_setting_bond_get_option_default (NMSettingBond *setting, const char *name) { BondProperty *prop; GValue defval = G_VALUE_INIT; guint idx = 0; g_return_val_if_fail (NM_IS_SETTING_BOND (setting), NULL); - prop = find_property (name, NULL, &idx); + prop = find_property_by_name (name, &idx, TRUE); + if (!prop) return NULL; if (G_UNLIKELY (prop->defval == NULL)) { - if (idx == PROP_ARP_IP_TARGET) - prop->defval = ""; - else { - g_value_init (&defval, prop->pspec->value_type); - g_param_value_set_default (prop->pspec, &defval); - prop->defval = g_strdup_value_contents (&defval); - g_value_unset (&defval); - } + g_value_init (&defval, prop->pspec->value_type); + g_param_value_set_default (prop->pspec, &defval); + prop->defval = g_strdup_value_contents (&defval); + g_value_unset (&defval); + g_return_val_if_fail (prop->defval, NULL); } return prop->defval; } /*****************************************************************/ static void set_properties_from_hash (NMSettingBond *self, GHashTable *options) { const char *value; guint i; g_object_freeze_notify (G_OBJECT (self)); /* Set each property to the value given by @options, or if not present @@ -968,69 +1013,69 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) NM_SETTING_BOND_MODE); g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_MODE); return FALSE; } if (!validate_property (&props[PROP_MODE], priv->mode)) { g_set_error (error, NM_SETTING_BOND_ERROR, NM_SETTING_BOND_ERROR_INVALID_PROPERTY, _("'%s' is not a valid value for '%s'"), priv->mode, NM_SETTING_BOND_MODE); g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_MODE); return FALSE; } /* Make sure mode is compatible with other settings */ - if ( strcmp (priv->mode, "balance-alb") == 0 - || strcmp (priv->mode, "balance-tlb") == 0) { + if (__NM_SETTING_BOND_MODE_IS_balance_alb (priv->mode) || + __NM_SETTING_BOND_MODE_IS_balance_tlb (priv->mode)) { if (priv->arp_interval > 0) { g_set_error (error, NM_SETTING_BOND_ERROR, NM_SETTING_BOND_ERROR_INVALID_PROPERTY, _("'%s=%s' is incompatible with '%s > 0'"), NM_SETTING_BOND_OPTION_MODE, priv->mode, NM_SETTING_BOND_OPTION_ARP_INTERVAL); g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_ARP_INTERVAL); return FALSE; } } - if (strcmp (priv->mode, "active-backup") == 0) { + if (__NM_SETTING_BOND_MODE_IS_active_backup (priv->mode)) { if (priv->primary && !nm_utils_iface_valid_name (priv->primary)) { g_set_error (error, NM_SETTING_BOND_ERROR, NM_SETTING_BOND_ERROR_INVALID_PROPERTY, _("'%s' is not a valid interface name"), priv->primary); g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_PRIMARY); return FALSE; } } else { if (priv->primary) { g_set_error (error, NM_SETTING_BOND_ERROR, NM_SETTING_BOND_ERROR_INVALID_PROPERTY, _("'%s' is only valid for '%s=%s'"), NM_SETTING_BOND_PRIMARY, NM_SETTING_BOND_MODE, "active-backup"); g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_PRIMARY); return FALSE; } } if (nm_setting_find_in_list (all_settings, NM_SETTING_INFINIBAND_SETTING_NAME)) { - if (strcmp (priv->mode, "active-backup") != 0) { + if (__NM_SETTING_BOND_MODE_IS_active_backup (priv->mode)) { g_set_error (error, NM_SETTING_BOND_ERROR, NM_SETTING_BOND_ERROR_INVALID_PROPERTY, _("'%s=%s' is not a valid configuration for '%s'"), NM_SETTING_BOND_OPTION_MODE, priv->mode, NM_SETTING_INFINIBAND_SETTING_NAME); g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_MODE); return FALSE; } } if (priv->miimon == 0) { /* updelay and downdelay can only be used with miimon */ if (priv->updelay > 0) { g_set_error (error, @@ -1156,61 +1201,189 @@ verify (NMSetting *setting, GSList *all_settings, GError **error) g_prefix_error (error, "%s.%s: ", NM_SETTING_BOND_SETTING_NAME, NM_SETTING_BOND_XMIT_HASH_POLICY); return FALSE; } return TRUE; } static const char * get_virtual_iface_name (NMSetting *setting) { NMSettingBond *self = NM_SETTING_BOND (setting); return nm_setting_bond_get_interface_name (self); } +static gboolean +compare_property (NMSetting *setting, + NMSetting *other, + const GParamSpec *prop_spec, + NMSettingCompareFlags flags) +{ + BondProperty *prop = find_property_by_pspec (prop_spec, NULL, TRUE); + gboolean result = FALSE; + + if (!prop) + goto CHAIN; + + switch (prop->opt_type) { + case TYPE_BOTH: { + char *a0, *b0; + const char *a, *b; + + g_object_get (setting, prop_spec->name, &a0, NULL); + g_object_get (setting, prop_spec->name, &b0, NULL); + + a = a0; + b = b0; + if (!a || !b) + result = (a == b); + else if (strcmp (a, b) == 0) + result = TRUE; + else { + int idx; + + if (validate_int (prop, a, &idx)) { + if (!IS_VALID_LIST_INDEX (prop, idx)) + goto BOTH_FINISHED; + a = prop->list[idx]; + } + if (validate_int (prop, b, &idx)) { + if (!IS_VALID_LIST_INDEX (prop, idx)) + goto BOTH_FINISHED; + b = prop->list[idx]; + } + result = (strcmp (a, b) == 0); + } + +BOTH_FINISHED: + g_free (a0); + g_free (b0); + return result; + } + case TYPE_IP: { + char **a, **b; + struct in_addr *addr_a = NULL, *addr_b = NULL; + + g_object_get (setting, prop_spec->name, &a, NULL); + g_object_get (setting, prop_spec->name, &b, NULL); + + if (!a || !a[0] || !b || !b[0]) + result = ((a ? a[0] : NULL) == (b ? b[0] : NULL)); + else { + /* both arrays are not empty. We compare them by + * converting every item into a struct in_addr and looking + * whether we find it in the other array (and vice versa). + * If one of the addresses cannot be converted, the result + * is always false (because nothing compares to an invalid property). + **/ + + guint i, j; + guint alen = g_strv_length (a); + guint blen = g_strv_length (b); + + /* convert all strings to struct in_addr */ + addr_a = g_new (struct in_addr, alen); + for (i = 0; i < alen; i++) { + if (!inet_aton (a[i], &addr_a[i])) + goto IP_FINISHED; + } + + addr_b = g_new (struct in_addr, blen); + for (i = 0; i < blen; i++) { + if (!inet_aton (b[i], &addr_b[i])) + goto IP_FINISHED; + } + + /* ensure that we find every address in the other array too */ + for (i = 0; i < alen; i++) { + for (j = 0; j < blen; j++) { + if (addr_a[i].s_addr == addr_b[j].s_addr) + break; + } + if (j >= blen) + goto IP_FINISHED; + } + + for (i = 0; i < blen; i++) { + for (j = 0; j < alen; j++) { + if (addr_b[i].s_addr == addr_a[j].s_addr) + break; + } + if (j >= alen) + goto IP_FINISHED; + } + result = TRUE; + } +IP_FINISHED: + g_free (addr_a); + g_free (addr_b); + g_strfreev (a); + g_strfreev (b); + return result; + } + default: + /* other types shall be compared by the default implementation. */ + goto CHAIN; + } + +CHAIN: + return NM_SETTING_CLASS (nm_setting_bond_parent_class)->compare_property (setting, other, prop_spec, flags); +} + + 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, NULL); - priv->options = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); + priv->options = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, g_free); +} + +static void +constructed (GObject *object) +{ + G_OBJECT_CLASS (nm_setting_bond_parent_class)->constructed (object); + + g_assert (g_hash_table_size (NM_SETTING_BOND_GET_PRIVATE (object)->options) == (_LAST_LEGACY_PROP - _FIRST_LEGACY_PROP + 1)); } static void finalize (GObject *object) { NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (object); + g_assert (g_hash_table_size (priv->options) == (_LAST_LEGACY_PROP - _FIRST_LEGACY_PROP + 1)); + g_free (priv->interface_name); g_free (priv->mode); g_free (priv->primary); g_strfreev (priv->arp_ip_target); g_hash_table_destroy (priv->options); G_OBJECT_CLASS (nm_setting_bond_parent_class)->finalize (object); } static void set_property (GObject *object, guint prop_id, const GValue *value, GParamSpec *pspec) { NMSettingBond *setting = NM_SETTING_BOND (object); NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (object); - const char *legacy_name = get_legacy_name (pspec); + BondProperty *prop = find_property_by_pspec (pspec, NULL, TRUE); char *legacy_value = NULL; switch (prop_id) { case PROP_INTERFACE_NAME: g_free (priv->interface_name); priv->interface_name = g_value_dup_string (value); break; case PROP_MODE: g_free (priv->mode); priv->mode = g_value_dup_string (value); legacy_value = g_value_dup_string (value); break; case PROP_MIIMON: priv->miimon = g_value_get_int (value); legacy_value = g_strdup_printf ("%u", g_value_get_int (value)); @@ -1266,32 +1439,32 @@ set_property (GObject *object, guint prop_id, priv->xmit_hash_policy = g_value_dup_string (value); legacy_value = g_value_dup_string (value); break; case PROP_RESEND_IGMP: priv->resend_igmp = g_value_get_int (value); legacy_value = g_strdup_printf ("%u", g_value_get_int (value)); break; case PROP_OPTIONS: set_properties_from_hash (setting, g_value_get_boxed (value)); break; default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; } - if (legacy_value) { - g_hash_table_insert (priv->options, g_strdup (legacy_name), legacy_value); + if (prop) { + g_hash_table_insert (priv->options, (void *)get_legacy_name (prop), legacy_value); g_object_notify (object, NM_SETTING_BOND_OPTIONS); } } static void get_property (GObject *object, guint prop_id, GValue *value, GParamSpec *pspec) { NMSettingBond *setting = NM_SETTING_BOND (object); NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (object); switch (prop_id) { case PROP_INTERFACE_NAME: g_value_set_string (value, nm_setting_bond_get_interface_name (setting)); break; @@ -1344,34 +1517,36 @@ get_property (GObject *object, guint prop_id, G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec); break; } } static void nm_setting_bond_class_init (NMSettingBondClass *setting_class) { GObjectClass *object_class = G_OBJECT_CLASS (setting_class); NMSettingClass *parent_class = NM_SETTING_CLASS (setting_class); guint i; g_type_class_add_private (setting_class, sizeof (NMSettingBondPrivate)); /* virtual methods */ - object_class->set_property = set_property; - object_class->get_property = get_property; - object_class->finalize = finalize; - parent_class->verify = verify; + object_class->set_property = set_property; + object_class->get_property = get_property; + object_class->constructed = constructed; + object_class->finalize = finalize; + parent_class->verify = verify; + parent_class->compare_property = compare_property; parent_class->get_virtual_iface_name = get_virtual_iface_name; /* Properties */ /** * NMSettingBond:interface-name: * * The name of the virtual in-kernel bonding network interface **/ props[PROP_INTERFACE_NAME].pspec = g_param_spec_string (NM_SETTING_BOND_INTERFACE_NAME, "InterfaceName", "The name of the virtual in-kernel bonding network interface", NULL, G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE); @@ -1454,31 +1629,31 @@ nm_setting_bond_class_init (NMSettingBondClass *setting_class) G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE); /** * NMSettingBond:arp-ip-target: * * An array of IPv4 addresses to ping when using ARP-based link monitoring. * This only has an effect when #NMSettingBond:arp-interval is also set. * * Since: 0.9.10 **/ props[PROP_ARP_IP_TARGET].pspec = g_param_spec_boxed (NM_SETTING_BOND_ARP_IP_TARGET, "ARP IP target", "ARP monitoring target IP addresses", G_TYPE_STRV, - G_PARAM_READWRITE | NM_SETTING_PARAM_SERIALIZE); + G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_SERIALIZE); /** * NMSettingBond:arp-validate: * * Specifies whether or not ARP probes and replies should be * validated in the active-backup mode. One of * 'none', 'active', 'backup', 'all'. * * Since: 0.9.10 **/ props[PROP_ARP_VALIDATE].pspec = g_param_spec_string (NM_SETTING_BOND_ARP_VALIDATE, "arp-validate", "Specifies whether or not ARP probes and replies should " "be validate in the active-backup mode", -- 1.8.4.1.559.gdb9bdfb
signature.asc
Description: This is a digitally signed message part
_______________________________________________ networkmanager-list mailing list [email protected] https://mail.gnome.org/mailman/listinfo/networkmanager-list
