From c8392018ca19ae88997af2fb8a64a9eb762da394 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 24 Oct 2022 13:22:40 +0200 Subject: [PATCH] libnm: refactor to-dbus on the client skipping to serialize legacy properties We have 4 legacy properties ("ipv[46].addresses", "ipv[46].routes") that got replaced by newer variants ("ipv[46].address-data", "ipv[46].route-data"). When the client side of libnm (_nm_utils_is_manager_process) serializes those properties, it must only serialize the newer version. That is so that the forward/backward compatibility works as intended. Previously, there was the NM_SETTING_PARAM_LEGACY GObject property flag. That was fine, but not very clear. For one, the legacy part of those properties is only about D-Bus. In particular, they are not deprecated in libnm, keyfile, or nmcli. Thus the name wasn't very clear. Also, in the meantime we have more elaborate property meta data, that goes beyond the meta data of the GObject property. Move NM_SETTING_PARAM_LEGACY to NMSettInfoProperty.to_dbus_only_in_manager_process. I think, this is a better name. It's also right at ``` _nm_properties_override_gobj( properties_override, g_object_class_find_property(G_OBJECT_CLASS(setting_class), NM_SETTING_IP_CONFIG_ROUTES), NM_SETT_INFO_PROPERT_TYPE_DBUS(NM_G_VARIANT_TYPE("a(ayuayu)"), .to_dbus_fcn = ip6_routes_to_dbus, .compare_fcn = _nm_setting_ip_config_compare_fcn_routes, .from_dbus_fcn = ip6_routes_from_dbus, ), .to_dbus_only_in_manager_process = TRUE, .dbus_deprecated = TRUE, ); ``` that is, directly at the place where we describe how the D-Bus property behaves. --- src/libnm-core-impl/nm-setting-ip-config.c | 17 ++++++++--------- src/libnm-core-impl/nm-setting-ip4-config.c | 6 ++++-- src/libnm-core-impl/nm-setting-ip6-config.c | 6 ++++-- src/libnm-core-impl/nm-setting-private.h | 5 +++-- src/libnm-core-impl/nm-setting.c | 7 +++---- src/libnm-core-intern/nm-core-internal.h | 7 +++++++ 6 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-ip-config.c b/src/libnm-core-impl/nm-setting-ip-config.c index 78e8ece67e..7d21551853 100644 --- a/src/libnm-core-impl/nm-setting-ip-config.c +++ b/src/libnm-core-impl/nm-setting-ip-config.c @@ -6291,12 +6291,9 @@ nm_setting_ip_config_class_init(NMSettingIPConfigClass *klass) "", "", G_TYPE_PTR_ARRAY, - /* "addresses" is a legacy D-Bus property, because the - * "addresses" GObject property normally gets set from - * the "address-data" D-Bus property... - */ - G_PARAM_READWRITE | NM_SETTING_PARAM_INFERRABLE | NM_SETTING_PARAM_LEGACY - | G_PARAM_STATIC_STRINGS); + /* On D-Bus, "addresses" is deprecated for "address-data". */ + G_PARAM_READWRITE | NM_SETTING_PARAM_INFERRABLE + | NM_SETTING_PARAM_UNUSED1 | G_PARAM_STATIC_STRINGS); /** * NMSettingIPConfig:gateway: @@ -6318,6 +6315,8 @@ nm_setting_ip_config_class_init(NMSettingIPConfigClass *klass) "", "", NULL, + /* On D-Bus, the legacy property "addresses" contains the gateway. + * This was replaced by "address-data" and "gateway". */ G_PARAM_READWRITE | NM_SETTING_PARAM_INFERRABLE | G_PARAM_STATIC_STRINGS); /** @@ -6330,9 +6329,9 @@ nm_setting_ip_config_class_init(NMSettingIPConfigClass *klass) "", "", G_TYPE_PTR_ARRAY, - G_PARAM_READWRITE | NM_SETTING_PARAM_INFERRABLE | - /* See :addresses above Re: LEGACY */ - NM_SETTING_PARAM_LEGACY | G_PARAM_STATIC_STRINGS); + /* On D-Bus, "routes" is deprecated for "route-data". */ + G_PARAM_READWRITE | NM_SETTING_PARAM_INFERRABLE + | NM_SETTING_PARAM_UNUSED1 | G_PARAM_STATIC_STRINGS); /** * NMSettingIPConfig:route-metric: diff --git a/src/libnm-core-impl/nm-setting-ip4-config.c b/src/libnm-core-impl/nm-setting-ip4-config.c index a8c3aea75f..80b6d0ea5c 100644 --- a/src/libnm-core-impl/nm-setting-ip4-config.c +++ b/src/libnm-core-impl/nm-setting-ip4-config.c @@ -1001,7 +1001,8 @@ nm_setting_ip4_config_class_init(NMSettingIP4ConfigClass *klass) .to_dbus_fcn = ip4_addresses_to_dbus, .compare_fcn = _nm_setting_ip_config_compare_fcn_addresses, .from_dbus_fcn = ip4_addresses_from_dbus, ), - .dbus_deprecated = TRUE, ); + .to_dbus_only_in_manager_process = TRUE, + .dbus_deprecated = TRUE, ); _nm_properties_override_dbus( properties_override, "address-labels", @@ -1139,7 +1140,8 @@ nm_setting_ip4_config_class_init(NMSettingIP4ConfigClass *klass) .to_dbus_fcn = ip4_routes_to_dbus, .compare_fcn = _nm_setting_ip_config_compare_fcn_routes, .from_dbus_fcn = ip4_routes_from_dbus, ), - .dbus_deprecated = TRUE, ); + .to_dbus_only_in_manager_process = TRUE, + .dbus_deprecated = TRUE, ); /* ---dbus--- * property: route-data diff --git a/src/libnm-core-impl/nm-setting-ip6-config.c b/src/libnm-core-impl/nm-setting-ip6-config.c index 02bc85c295..16e96895f6 100644 --- a/src/libnm-core-impl/nm-setting-ip6-config.c +++ b/src/libnm-core-impl/nm-setting-ip6-config.c @@ -1010,7 +1010,8 @@ nm_setting_ip6_config_class_init(NMSettingIP6ConfigClass *klass) .to_dbus_fcn = ip6_addresses_to_dbus, .compare_fcn = _nm_setting_ip_config_compare_fcn_addresses, .from_dbus_fcn = ip6_addresses_from_dbus, ), - .dbus_deprecated = TRUE, ); + .to_dbus_only_in_manager_process = TRUE, + .dbus_deprecated = TRUE, ); /* ---dbus--- * property: address-data @@ -1129,7 +1130,8 @@ nm_setting_ip6_config_class_init(NMSettingIP6ConfigClass *klass) .to_dbus_fcn = ip6_routes_to_dbus, .compare_fcn = _nm_setting_ip_config_compare_fcn_routes, .from_dbus_fcn = ip6_routes_from_dbus, ), - .dbus_deprecated = TRUE, ); + .to_dbus_only_in_manager_process = TRUE, + .dbus_deprecated = TRUE, ); /* ---dbus--- * property: route-data diff --git a/src/libnm-core-impl/nm-setting-private.h b/src/libnm-core-impl/nm-setting-private.h index d38e889222..8c674053c7 100644 --- a/src/libnm-core-impl/nm-setting-private.h +++ b/src/libnm-core-impl/nm-setting-private.h @@ -318,8 +318,9 @@ typedef struct { */ #define NM_SETTING_PARAM_INFERRABLE (1 << (4 + G_PARAM_USER_SHIFT)) -/* This is a legacy property, which clients should not send to the daemon. */ -#define NM_SETTING_PARAM_LEGACY (1 << (5 + G_PARAM_USER_SHIFT)) +/* This flag has no meaning (anymore). It's only kept, because we used it + * on some older versions of libnm. */ +#define NM_SETTING_PARAM_UNUSED1 (1 << (5 + G_PARAM_USER_SHIFT)) /* When a connection is active and gets modified, usually the change * to the settings-connection does not propagate automatically to the diff --git a/src/libnm-core-impl/nm-setting.c b/src/libnm-core-impl/nm-setting.c index 46ed9ebb79..30fdcc0400 100644 --- a/src/libnm-core-impl/nm-setting.c +++ b/src/libnm-core-impl/nm-setting.c @@ -1737,12 +1737,11 @@ property_to_dbus(const NMSettInfoSetting *sett_info, || NM_FLAGS_HAS(property_info->param_spec->flags, G_PARAM_WRITABLE) || property_info->property_type == &nm_sett_info_propert_type_setting_name); + if (property_info->to_dbus_only_in_manager_process && !_nm_utils_is_manager_process) + return NULL; + if (property_info->param_spec && !ignore_flags && !NM_FLAGS_HAS(property_info->param_spec->flags, NM_SETTING_PARAM_TO_DBUS_IGNORE_FLAGS)) { - if (NM_FLAGS_HAS(property_info->param_spec->flags, NM_SETTING_PARAM_LEGACY) - && !_nm_utils_is_manager_process) - return NULL; - if (NM_FLAGS_HAS(property_info->param_spec->flags, NM_SETTING_PARAM_SECRET)) { NMSettingSecretFlags f = NM_SETTING_SECRET_FLAG_NONE; diff --git a/src/libnm-core-intern/nm-core-internal.h b/src/libnm-core-intern/nm-core-internal.h index 563811305c..21c994f033 100644 --- a/src/libnm-core-intern/nm-core-internal.h +++ b/src/libnm-core-intern/nm-core-internal.h @@ -827,6 +827,13 @@ struct _NMSettInfoProperty { * is the default. */ bool to_dbus_including_default : 1; + /* This indicates, that the property is deprecated (on D-Bus) for another property. + * See also _nm_setting_use_legacy_property() how that works. + * + * The to_dbus_fcn() will be skipped for such properties, if _nm_utils_is_manager_process + * is FALSE. */ + bool to_dbus_only_in_manager_process : 1; + /* Whether the property is deprecated. * * Note that we have various representations of profiles, e.g. on D-Bus, keyfile,