libnm: merge branch 'th/bond-stable-strdict-order'

6 files changed, 154 insertions, 73 deletions

diff --git a/libnm-core/nm-setting-bond.c b/libnm-core/nm-setting-bond.c
index e85d1564ea..44219bfe23 100644
--- a/libnm-core/nm-setting-bond.c
+++ b/libnm-core/nm-setting-bond.c
@@ -34,6 +34,8 @@
 #include "nm-setting-infiniband.h"
 #include "nm-core-internal.h"
 
+/*****************************************************************************/
+
 /**
  * SECTION:nm-setting-bond
  * @short_description: Describes connection properties for bonds
@@ -42,15 +44,7 @@
  * necessary for bond connections.
  **/
 
-G_DEFINE_TYPE_WITH_CODE (NMSettingBond, nm_setting_bond, NM_TYPE_SETTING,
-                         _nm_register_setting (BOND, NM_SETTING_PRIORITY_HW_BASE))
-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 {
-	GHashTable *options;
-} NMSettingBondPrivate;
+/*****************************************************************************/
 
 enum {
 	PROP_0,
@@ -59,6 +53,19 @@ enum {
 };
 
 typedef struct {
+	GHashTable *options;
+	NMUtilsNamedValue *options_idx_cache;
+} NMSettingBondPrivate;
+
+G_DEFINE_TYPE_WITH_CODE (NMSettingBond, nm_setting_bond, NM_TYPE_SETTING,
+                         _nm_register_setting (BOND, NM_SETTING_PRIORITY_HW_BASE))
+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 {
 	const char *opt;
 	const char *val;
 	guint opt_type;
@@ -104,18 +111,7 @@ static const BondDefault defaults[] = {
 	{ NM_SETTING_BOND_OPTION_LP_INTERVAL,      "1",          NM_BOND_OPTION_TYPE_INT, 1, G_MAXINT },
 };
 
-/**
- * nm_setting_bond_new:
- *
- * Creates a new #NMSettingBond object with default values.
- *
- * Returns: (transfer full): the new empty #NMSettingBond object
- **/
-NMSetting *
-nm_setting_bond_new (void)
-{
-	return (NMSetting *) g_object_new (NM_TYPE_SETTING_BOND, NULL);
-}
+/*****************************************************************************/
 
 /**
  * nm_setting_bond_get_num_options:
@@ -163,26 +159,38 @@ nm_setting_bond_get_option (NMSettingBond *setting,
                             const char **out_value)
 {
 	NMSettingBondPrivate *priv;
-	GList *keys;
-	const char *_key = NULL, *_value = NULL;
+	guint i, len;
+	GHashTableIter iter;
+	const char *key, *value;
 
 	g_return_val_if_fail (NM_IS_SETTING_BOND (setting), FALSE);
 
 	priv = NM_SETTING_BOND_GET_PRIVATE (setting);
 
-	if (idx >= nm_setting_bond_get_num_options (setting))
+	len = g_hash_table_size (priv->options);
+	if (idx >= len)
 		return FALSE;
 
-	keys = g_hash_table_get_keys (priv->options);
-	_key = g_list_nth_data (keys, idx);
-	_value = g_hash_table_lookup (priv->options, _key);
+	if (!G_UNLIKELY (priv->options_idx_cache)) {
+		NMUtilsNamedValue *options;
+
+		i = 0;
+		options = g_new (NMUtilsNamedValue, len);
+		g_hash_table_iter_init (&iter, priv->options);
+		while (g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &value)) {
+			options[i].name = key;
+			options[i].value_str = value;
+			i++;
+		}
+		nm_assert (i == len);
 
-	if (out_name)
-		*out_name = _key;
-	if (out_value)
-		*out_value = _value;
+		g_qsort_with_data (options, len, sizeof (options[0]),
+		                   nm_utils_named_entry_cmp_with_data, NULL);
+		priv->options_idx_cache = options;
+	}
 
-	g_list_free (keys);
+	NM_SET_OUT (out_name, priv->options_idx_cache[idx].name);
+	NM_SET_OUT (out_value, priv->options_idx_cache[idx].value_str);
 	return TRUE;
 }
 
@@ -362,6 +370,7 @@ nm_setting_bond_add_option (NMSettingBond *setting,
 
 	priv = NM_SETTING_BOND_GET_PRIVATE (setting);
 
+	nm_clear_g_free (&priv->options_idx_cache);
 	g_hash_table_insert (priv->options, g_strdup (name), g_strdup (value));
 
 	if (   !strcmp (name, NM_SETTING_BOND_OPTION_MIIMON)
@@ -395,6 +404,7 @@ gboolean
 nm_setting_bond_remove_option (NMSettingBond *setting,
                                const char *name)
 {
+	NMSettingBondPrivate *priv;
 	gboolean found;
 
 	g_return_val_if_fail (NM_IS_SETTING_BOND (setting), FALSE);
@@ -402,7 +412,10 @@ nm_setting_bond_remove_option (NMSettingBond *setting,
 	if (!nm_setting_bond_validate_option (name, NULL))
 		return FALSE;
 
-	found = g_hash_table_remove (NM_SETTING_BOND_GET_PRIVATE (setting)->options, name);
+	priv = NM_SETTING_BOND_GET_PRIVATE (setting);
+
+	nm_clear_g_free (&priv->options_idx_cache);
+	found = g_hash_table_remove (priv->options, name);
 	if (found)
 		g_object_notify (G_OBJECT (setting), NM_SETTING_BOND_OPTIONS);
 	return found;
@@ -514,6 +527,8 @@ _nm_setting_bond_mode_from_string (const char *str)
 	return NM_BOND_MODE_UNKNOWN;
 }
 
+/*****************************************************************************/
+
 #define BIT(x) (1 << (x))
 
 static const struct {
@@ -812,6 +827,8 @@ verify (NMSetting *setting, NMConnection *connection, GError **error)
 	return TRUE;
 }
 
+/*****************************************************************************/
+
 static gboolean
 options_hash_match (NMSettingBond *s_bond,
                     GHashTable *options1,
@@ -887,25 +904,22 @@ compare_property (NMSetting *setting,
 	return 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);
-
-	priv->options = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, g_free);
-
-	/* Default values: */
-	nm_setting_bond_add_option (setting, NM_SETTING_BOND_OPTION_MODE, "balance-rr");
-}
+/*****************************************************************************/
 
 static void
-finalize (GObject *object)
+get_property (GObject *object, guint prop_id,
+              GValue *value, GParamSpec *pspec)
 {
 	NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (object);
 
-	g_hash_table_destroy (priv->options);
-
-	G_OBJECT_CLASS (nm_setting_bond_parent_class)->finalize (object);
+	switch (prop_id) {
+	case PROP_OPTIONS:
+		g_value_take_boxed (value, _nm_utils_copy_strdict (priv->options));
+		break;
+	default:
+		G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
+		break;
+	}
 }
 
 static void
@@ -916,6 +930,7 @@ set_property (GObject *object, guint prop_id,
 
 	switch (prop_id) {
 	case PROP_OPTIONS:
+		nm_clear_g_free (&priv->options_idx_cache);
 		g_hash_table_unref (priv->options);
 		priv->options = _nm_utils_copy_strdict (g_value_get_boxed (value));
 		break;
@@ -925,20 +940,41 @@ set_property (GObject *object, guint prop_id,
 	}
 }
 
+/*****************************************************************************/
+
 static void
-get_property (GObject *object, guint prop_id,
-              GValue *value, GParamSpec *pspec)
+nm_setting_bond_init (NMSettingBond *setting)
+{
+	NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (setting);
+
+	priv->options = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, g_free);
+
+	/* Default values: */
+	nm_setting_bond_add_option (setting, NM_SETTING_BOND_OPTION_MODE, "balance-rr");
+}
+
+/**
+ * nm_setting_bond_new:
+ *
+ * Creates a new #NMSettingBond object with default values.
+ *
+ * Returns: (transfer full): the new empty #NMSettingBond object
+ **/
+NMSetting *
+nm_setting_bond_new (void)
+{
+	return (NMSetting *) g_object_new (NM_TYPE_SETTING_BOND, NULL);
+}
+
+static void
+finalize (GObject *object)
 {
 	NMSettingBondPrivate *priv = NM_SETTING_BOND_GET_PRIVATE (object);
 
-	switch (prop_id) {
-	case PROP_OPTIONS:
-		g_value_take_boxed (value, _nm_utils_copy_strdict (priv->options));
-		break;
-	default:
-		G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
-		break;
-	}
+	nm_clear_g_free (&priv->options_idx_cache);
+	g_hash_table_destroy (priv->options);
+
+	G_OBJECT_CLASS (nm_setting_bond_parent_class)->finalize (object);
 }
 
 static void
diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c
index c3001ab5d1..ef72011393 100644
--- a/libnm-core/nm-utils.c
+++ b/libnm-core/nm-utils.c
@@ -490,17 +490,45 @@ _nm_utils_strdict_to_dbus (const GValue *prop_value)
 {
 	GHashTable *hash;
 	GHashTableIter iter;
-	gpointer key, value;
+	const char *key, *value;
 	GVariantBuilder builder;
+	guint i, len;
 
 	g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{ss}"));
+
 	hash = g_value_get_boxed (prop_value);
-	if (hash) {
-		g_hash_table_iter_init (&iter, hash);
-		while (g_hash_table_iter_next (&iter, &key, &value))
-			g_variant_builder_add (&builder, "{ss}", key, value);
+	if (!hash)
+		goto out;
+	len = g_hash_table_size (hash);
+	if (!len)
+		goto out;
+
+	g_hash_table_iter_init (&iter, hash);
+	if (!g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &value))
+		nm_assert_not_reached ();
+
+	if (len == 1)
+		g_variant_builder_add (&builder, "{ss}", key, value);
+	else {
+		gs_free NMUtilsNamedValue *idx = NULL;
+
+		idx = g_new (NMUtilsNamedValue, len);
+		i = 0;
+		do {
+			idx[i].name = key;
+			idx[i].value_str = value;
+			i++;
+		} while (g_hash_table_iter_next (&iter, (gpointer *) &key, (gpointer *) &value));
+		nm_assert (i == len);
+
+		g_qsort_with_data (idx, len, sizeof (idx[0]),
+		                   nm_utils_named_entry_cmp_with_data, NULL);
+
+		for (i = 0; i < len; i++)
+			g_variant_builder_add (&builder, "{ss}", idx[i].name, idx[i].value_str);
 	}
 
+out:
 	return g_variant_builder_end (&builder);
 }
 
diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h
index a092a6b780..1e80b35a93 100644
--- a/shared/nm-utils/nm-shared-utils.h
+++ b/shared/nm-utils/nm-shared-utils.h
@@ -407,6 +407,26 @@ char *nm_utils_str_utf8safe_escape_take (char *str, NMUtilsStrUtf8SafeFlags flag
 
 /*****************************************************************************/
 
+typedef struct {
+	const char *name;
+} NMUtilsNamedEntry;
+
+typedef struct {
+	union {
+		NMUtilsNamedEntry named_entry;
+		const char *name;
+	};
+	union {
+		const char *value_str;
+		gconstpointer value_ptr;
+	};
+} NMUtilsNamedValue;
+
+#define nm_utils_named_entry_cmp           nm_strcmp_p
+#define nm_utils_named_entry_cmp_with_data nm_strcmp_p_with_data
+
+/*****************************************************************************/
+
 #define NM_UTILS_NS_PER_SECOND  ((gint64) 1000000000)
 #define NM_UTILS_NS_PER_MSEC    ((gint64) 1000000)
 #define NM_UTILS_NS_TO_MSEC_CEIL(nsec)      (((nsec) + (NM_UTILS_NS_PER_MSEC - 1)) / NM_UTILS_NS_PER_MSEC)
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
index 9292640960..c277cb2ad5 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
@@ -1311,21 +1311,14 @@ write_bond_setting (NMConnection *connection, shvarFile *ifcfg, gboolean *wired,
 	num_opts = nm_setting_bond_get_num_options (s_bond);
 	if (num_opts) {
 		nm_auto_free_gstring GString *str = NULL;
-		gs_free const char **options = NULL;
-		const char *value;
+		const char *name, *value;
 
 		str = g_string_sized_new (64);
-		options = g_new (const char *, num_opts);
-		for (i = 0; i < num_opts; i++)
-			nm_setting_bond_get_option (s_bond, i, &options[i], &value);
-		g_qsort_with_data (options, num_opts, sizeof (const char *),
-		                   nm_strcmp_p_with_data, NULL);
-
 		for (i = 0; i < num_opts; i++) {
 			if (str->len)
 				g_string_append_c (str, ' ');
-			value = nm_setting_bond_get_option_by_name (s_bond, options[i]);
-			g_string_append_printf (str, "%s=%s", options[i], value);
+			nm_setting_bond_get_option (s_bond, i, &name, &value);
+			g_string_append_printf (str, "%s=%s", name, value);
 		}
 
 		svSetValueStr (ifcfg, "BONDING_OPTS", str->str);
diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-Test_Write_Bond_Main.cexpected b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-Test_Write_Bond_Main.cexpected
index 854d24905f..5d81dfefb9 100644
--- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-Test_Write_Bond_Main.cexpected
+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-Test_Write_Bond_Main.cexpected
@@ -1,4 +1,4 @@
-BONDING_OPTS=mode=balance-rr
+BONDING_OPTS="downdelay=5 miimon=100 mode=balance-rr updelay=10"
 TYPE=Bond
 BONDING_MASTER=yes
 PROXY_METHOD=none
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 17b6ca4940..10a0f68396 100644
--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
@@ -8058,6 +8058,10 @@ test_write_bond_main (void)
 	s_bond = (NMSettingBond *) nm_setting_bond_new ();
 	nm_connection_add_setting (connection, NM_SETTING (s_bond));
 
+	nm_setting_bond_add_option (s_bond, NM_SETTING_BOND_OPTION_DOWNDELAY, "5");
+	nm_setting_bond_add_option (s_bond, NM_SETTING_BOND_OPTION_UPDELAY, "10");
+	nm_setting_bond_add_option (s_bond, NM_SETTING_BOND_OPTION_MIIMON, "100");
+
 	/* IP4 setting */
 	s_ip4 = (NMSettingIPConfig *) nm_setting_ip4_config_new ();
 	nm_connection_add_setting (connection, NM_SETTING (s_ip4));