From 7a14f19e0937bc7b8546dfa361f92319d5d8a351 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Oct 2015 09:52:48 +0200 Subject: [PATCH 1/2] libnm: always serialize NMSettingVlan:flags property for D-Bus We changed the default value for the NMSettingVlan:flags from 0 to 1 (NM_VLAN_FLAG_REORDER_HEADERS). That means, we will no longer serialize a value of 1 over D-Bus. This breaks older libnm clients, which treat a missing flags property as the old default (0). -- old clients here means: clients that still use an older version of libnm or clients that don't use libnm, but depend on the previous default value in the D-Bus API. Enforce to always serialize the flags properties. This workaround has almost no downsides except that for new clients we serialize more then absolutely necessary. But it ensures that old clients still receive the proper value. https://bugzilla.redhat.com/show_bug.cgi?id=1250225 Fixes: 687b6515980c08cdbb9734bd112a594166c4d6dd --- libnm-core/nm-setting-vlan.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/libnm-core/nm-setting-vlan.c b/libnm-core/nm-setting-vlan.c index a1750c52cc..6fd4e33748 100644 --- a/libnm-core/nm-setting-vlan.c +++ b/libnm-core/nm-setting-vlan.c @@ -558,6 +558,12 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) return TRUE; } +static GVariant * +_override_flags_get (NMSetting *setting, const char *property) +{ + return g_variant_new_uint32 (nm_setting_vlan_get_flags ((NMSettingVlan *) setting)); +} + static GSList * priority_strv_to_maplist (NMVlanPriorityMap map, char **strv) { @@ -753,6 +759,11 @@ nm_setting_vlan_class_init (NMSettingVlanClass *setting_class) G_PARAM_CONSTRUCT | NM_SETTING_PARAM_INFERRABLE | G_PARAM_STATIC_STRINGS)); + _nm_setting_class_override_property (parent_class, NM_SETTING_VLAN_FLAGS, + NULL, + _override_flags_get, + NULL, + NULL); /** * NMSettingVlan:ingress-priority-map: From 21674d5bfbd7858f2a19037bfbde4f3eea8beaa6 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Oct 2015 10:02:48 +0200 Subject: [PATCH 2/2] libnm: treat missing NMSettingVlan:flags property as old default value We changed the default value of MSettingVlan:flags from 0 to 1 (NM_VLAN_FLAG_REORDER_HEADERS). That means, that old libnm clients will not serialize 0 (their default). This change broke the D-Bus API. The D-Bus API allows to omit a value when meaning the default value. That means, we cannot change the default value (in the D-Bus API!) without breaking previous assumptions. A newer libnm version should treat a missing flags argument as the old default value and thus preserve the original default value (in the D-Bus API). This has the downside that for the future we will continue to treat a missing value as the old default value (0), and in order to get the new default value (1), the client must explicitly set the flags. We also must restore the original default value in libnm-glib. libnm-glib does not support _nm_setting_class_override_property() and thus it must keep thinking that the default value for the GObject property continues to be 0. Otherwise, it would not serialize a 1, which a new libnm would now interpret as 0. https://bugzilla.redhat.com/show_bug.cgi?id=1250225 Fixes: 687b6515980c08cdbb9734bd112a594166c4d6dd --- libnm-core/nm-setting-vlan.c | 19 ++++++++++++++++++- libnm-util/nm-setting-vlan.c | 2 +- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/libnm-core/nm-setting-vlan.c b/libnm-core/nm-setting-vlan.c index 6fd4e33748..99e433651b 100644 --- a/libnm-core/nm-setting-vlan.c +++ b/libnm-core/nm-setting-vlan.c @@ -564,6 +564,18 @@ _override_flags_get (NMSetting *setting, const char *property) return g_variant_new_uint32 (nm_setting_vlan_get_flags ((NMSettingVlan *) setting)); } +static void +_override_flags_not_set (NMSetting *setting, + GVariant *connection_dict, + const char *property) +{ + /* we changed the default value for FLAGS. When an older client + * doesn't serialize the property, we assume it is the old default. */ + g_object_set (G_OBJECT (setting), + NM_SETTING_VLAN_FLAGS, (NMVlanFlags) 0, + NULL); +} + static GSList * priority_strv_to_maplist (NMVlanPriorityMap map, char **strv) { @@ -742,6 +754,11 @@ nm_setting_vlan_class_init (NMSettingVlanClass *setting_class) * output packet headers), %NM_VLAN_FLAG_GVRP (use of the GVRP protocol), * and %NM_VLAN_FLAG_LOOSE_BINDING (loose binding of the interface to its * master device's operating state). + * + * The default value of this property is NM_VLAN_FLAG_REORDER_HEADERS, + * but it used to be 0. To preserve backward compatibility, the default-value + * in the D-Bus API continues to be 0 and a missing property on D-Bus + * is still considered as 0. **/ /* ---ifcfg-rh--- * property: flags @@ -763,7 +780,7 @@ nm_setting_vlan_class_init (NMSettingVlanClass *setting_class) NULL, _override_flags_get, NULL, - NULL); + _override_flags_not_set); /** * NMSettingVlan:ingress-priority-map: diff --git a/libnm-util/nm-setting-vlan.c b/libnm-util/nm-setting-vlan.c index c3d4bb58f1..52676d47f0 100644 --- a/libnm-util/nm-setting-vlan.c +++ b/libnm-util/nm-setting-vlan.c @@ -805,7 +805,7 @@ nm_setting_vlan_class_init (NMSettingVlanClass *setting_class) g_object_class_install_property (object_class, PROP_FLAGS, g_param_spec_uint (NM_SETTING_VLAN_FLAGS, "", "", - 0, G_MAXUINT32, NM_VLAN_FLAG_REORDER_HEADERS, + 0, G_MAXUINT32, 0, G_PARAM_READWRITE | G_PARAM_CONSTRUCT | NM_SETTING_PARAM_INFERRABLE |