fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-03-05 07:40:34 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

fixup! libnm/client: don't reset properties when interface goes away

Browse source 
We want that when a D-Bus interface get removed (which in practice
means, the D-Bus object got removed altogether), that the properties are
frozen in time. Previously, they were reset to the default, which isn't
very useful.

... Except for the complex properties that refer other objects. We need
to break the reference cycles.

Previously, the patch only checked for the D-Bus types, and special
handled "o" and "ao" types. That seems wrong to me.

1) at the point where the check was performed, we didn't
  yet check whether the variant has the expected signature. If (due
  to a bug or an API break) the type changes, we still want to reset
  the property.
  For that, we would have to call_obj_handle_dbus_prop_changes() and
  not shortcut.

2) we have handlers, I think those handlers should be asked to handle
  the removal, and not the calling code decide that it's not necessary.

2b) nm_active_connection_get_specific_object_path() is D-Bus type "o",
  but the libnm property is a plain string. For this we should also
  freeze the value.

2b) is actually the only exception, where it really makes a difference.
Maybe that is a weak justification for this large patch, compared to the "simpler"
variant before. I think the "simpler" variant is however a bit "wrong", and
doing the wrong thing is usually simpler.
This commit is contained in:
Thomas Haller 2022-09-06 15:41:12 +02:00
parent 02f7fb2cdd
commit 56580882e3
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
6 changed files with 88 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/libnm-client-impl/nm-access-point.c
View file

@ -399,11 +399,15 @@ _notify_update_prop_hw_address(NMClient *client,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value)
{
NMAccessPoint *self = NM_ACCESS_POINT(dbobj->nmobj);
NMAccessPointPrivate *priv = NM_ACCESS_POINT_GET_PRIVATE(self);
if (is_removed)
return NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NONE;
g_free(priv->bssid);
priv->bssid = value ? g_variant_dup_string(value, NULL) : 0u;
_notify(self, PROP_HW_ADDRESS);

58
src/libnm-client-impl/nm-client.c
View file

@ -2364,6 +2364,7 @@ _nml_dbus_notify_update_prop_ignore(NMClient *self,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value)
{
return NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NONE;
@ -2374,11 +2375,15 @@ _nml_dbus_notify_update_prop_o(NMClient *self,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value)
{
const char *path = NULL;
NMRefString **p_property;
if (is_removed)
return NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NONE;
if (value)
path = g_variant_get_string(value, NULL);
@ -2409,9 +2414,22 @@ _obj_handle_dbus_prop_changes(NMClient *self,
const char *dbus_type_s;
const GParamSpec *param_spec;
NMLDBusNotifyUpdatePropFlags notify_update_prop_flags;
const gboolean is_removed = (!value);
nm_assert(G_IS_OBJECT(dbobj->nmobj));
#define _HANDLE_IS_REMOVED(is_removed) \
G_STMT_START \
{ \
/* The D-Bus interface was removed, when @value was originally NULL.
* In that case, we only want to reset complex properties ("o"
* and "ao"), which refer to other objects. Plain properties
* are frozen at the last value, and we do nothing about them */ \
if (is_removed) \
return; \
} \
G_STMT_END
if (value && !g_variant_is_of_type(value, meta_property->dbus_type)) {
NML_NMCLIENT_LOG_E(self,
"[%s] property %s.%s expected of type \"%s\" but is \"%s\". Ignore",
@ -2424,8 +2442,12 @@ _obj_handle_dbus_prop_changes(NMClient *self,
}
if (meta_property->notify_update_prop) {
notify_update_prop_flags =
meta_property->notify_update_prop(self, dbobj, meta_iface, dbus_property_idx, value);
notify_update_prop_flags = meta_property->notify_update_prop(self,
dbobj,
meta_iface,
dbus_property_idx,
is_removed,
value);
if (notify_update_prop_flags == NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NONE)
return;
@ -2451,6 +2473,7 @@ _obj_handle_dbus_prop_changes(NMClient *self,
switch (dbus_type_s[0]) {
case 'b':
nm_assert(dbus_type_s[1] == '\0');
_HANDLE_IS_REMOVED(is_removed);
if (value)
*((bool *) p_property) = g_variant_get_boolean(value);
else if (param_spec->value_type == G_TYPE_BOOLEAN)
@ -2462,6 +2485,7 @@ _obj_handle_dbus_prop_changes(NMClient *self,
break;
case 'y':
nm_assert(dbus_type_s[1] == '\0');
_HANDLE_IS_REMOVED(is_removed);
if (value)
*((guint8 *) p_property) = g_variant_get_byte(value);
else {
@ -2471,6 +2495,7 @@ _obj_handle_dbus_prop_changes(NMClient *self,
break;
case 'q':
nm_assert(dbus_type_s[1] == '\0');
_HANDLE_IS_REMOVED(is_removed);
if (value)
*((guint16 *) p_property) = g_variant_get_uint16(value);
else {
@ -2480,6 +2505,7 @@ _obj_handle_dbus_prop_changes(NMClient *self,
break;
case 'i':
nm_assert(dbus_type_s[1] == '\0');
_HANDLE_IS_REMOVED(is_removed);
if (value)
*((gint32 *) p_property) = g_variant_get_int32(value);
else if (param_spec->value_type == G_TYPE_INT)
@ -2491,6 +2517,7 @@ _obj_handle_dbus_prop_changes(NMClient *self,
break;
case 'u':
nm_assert(dbus_type_s[1] == '\0');
_HANDLE_IS_REMOVED(is_removed);
if (value)
*((guint32 *) p_property) = g_variant_get_uint32(value);
else {
@ -2500,6 +2527,7 @@ _obj_handle_dbus_prop_changes(NMClient *self,
break;
case 'x':
nm_assert(dbus_type_s[1] == '\0');
_HANDLE_IS_REMOVED(is_removed);
if (value)
*((gint64 *) p_property) = g_variant_get_int64(value);
else if (param_spec->value_type == G_TYPE_INT64)
@ -2511,6 +2539,7 @@ _obj_handle_dbus_prop_changes(NMClient *self,
break;
case 't':
nm_assert(dbus_type_s[1] == '\0');
_HANDLE_IS_REMOVED(is_removed);
if (value)
*((guint64 *) p_property) = g_variant_get_uint64(value);
else {
@ -2521,6 +2550,7 @@ _obj_handle_dbus_prop_changes(NMClient *self,
case 's':
nm_assert(dbus_type_s[1] == '\0');
nm_clear_g_free((char **) p_property);
_HANDLE_IS_REMOVED(is_removed);
if (value)
*((char **) p_property) = g_variant_dup_string(value, NULL);
else {
@ -2530,6 +2560,7 @@ _obj_handle_dbus_prop_changes(NMClient *self,
break;
case 'o':
nm_assert(dbus_type_s[1] == '\0');
/* Don't call _HANDLE_IS_REMOVED(), because we want to break reference cycles. */
notify_update_prop_flags = nml_dbus_property_o_notify(self,
p_property,
dbobj,
@ -2541,6 +2572,7 @@ _obj_handle_dbus_prop_changes(NMClient *self,
switch (dbus_type_s[1]) {
case 'y':
nm_assert(dbus_type_s[2] == '\0');
_HANDLE_IS_REMOVED(is_removed);
{
gconstpointer v;
gsize l;
@ -2563,6 +2595,8 @@ _obj_handle_dbus_prop_changes(NMClient *self,
nm_assert(dbus_type_s[2] == '\0');
nm_assert(param_spec->value_type == G_TYPE_STRV);
_HANDLE_IS_REMOVED(is_removed);
g_strfreev(*((char ***) p_property));
if (value)
*((char ***) p_property) = g_variant_dup_strv(value, NULL);
@ -2571,6 +2605,7 @@ _obj_handle_dbus_prop_changes(NMClient *self,
break;
case 'o':
nm_assert(dbus_type_s[2] == '\0');
/* Don't call _HANDLE_IS_REMOVED(), because we want to break reference cycles. */
notify_update_prop_flags = nml_dbus_property_ao_notify(self,
p_property,
dbobj,
@ -2630,16 +2665,7 @@ _obj_handle_dbus_iface_changes(NMClient *self,
if (is_removed) {
for (i_prop = 0; i_prop < db_iface_data->dbus_iface.meta->n_dbus_properties; i_prop++) {
const GVariantType *dbus_type =
db_iface_data->dbus_iface.meta->dbus_properties[i_prop].dbus_type;
/* Unset properties that can potentially contain objects, to release them,
* but keep the rest around, because it might still make sense to know what
* they were (e.g. when a device has been removed we'd like know what interface
* name it had, or keep the state to avoid spurious state change into UNKNOWN). */
if (g_variant_type_is_array(dbus_type)
|| g_variant_type_equal(dbus_type, G_VARIANT_TYPE_OBJECT_PATH))
_obj_handle_dbus_prop_changes(self, dbobj, db_iface_data, i_prop, NULL);
_obj_handle_dbus_prop_changes(self, dbobj, db_iface_data, i_prop, NULL);
}
} else {
while ((db_prop_data = c_list_first_entry(&db_iface_data->changed_prop_lst_head,
@ -6201,6 +6227,7 @@ _notify_update_prop_dns_manager_configuration(NMClient *self,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value)
{
NMClientPrivate *priv = NM_CLIENT_GET_PRIVATE(self);
@ -6209,6 +6236,9 @@ _notify_update_prop_dns_manager_configuration(NMClient *self,
nm_assert(G_OBJECT(self) == dbobj->nmobj);
if (is_removed)
return NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NONE;
if (value) {
GVariant *entry_var_tmp;
GVariantIter iter;
@ -6304,12 +6334,16 @@ _notify_update_prop_nm_capabilities(NMClient *self,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value)
{
NMClientPrivate *priv = NM_CLIENT_GET_PRIVATE(self);
nm_assert(G_OBJECT(self) == dbobj->nmobj);
if (is_removed)
return NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NONE;
nm_clear_g_free(&priv->nm.capabilities_arr);
priv->nm.capabilities_len = 0;

13
src/libnm-client-impl/nm-device.c
View file

@ -190,6 +190,7 @@ _notify_update_prop_state_reason(NMClient *client,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value)
{
NMDevice *self = NM_DEVICE(dbobj->nmobj);
@ -197,6 +198,9 @@ _notify_update_prop_state_reason(NMClient *client,
guint32 new_state = NM_DEVICE_STATE_UNKNOWN;
guint32 reason = NM_DEVICE_STATE_REASON_NONE;
if (is_removed)
return NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NONE;
/* We ignore the "State" property and the "StateChanged" signal of the device.
* This information is redundant to the "StateReason" property, and we rely
* on that one alone. In the best case, the information is identical. If it
@ -234,6 +238,7 @@ _notify_update_prop_lldp_neighbors(NMClient *client,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value)
{
NMDevice *self = NM_DEVICE(dbobj->nmobj);
@ -243,6 +248,9 @@ _notify_update_prop_lldp_neighbors(NMClient *client,
GVariantIter *attrs_iter;
GVariantIter iter;
if (is_removed)
return NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NONE;
new = g_ptr_array_new_with_free_func((GDestroyNotify) nm_lldp_neighbor_unref);
if (value) {
@ -1300,6 +1308,7 @@ _nm_device_notify_update_prop_hw_address(NMClient *client,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value)
{
NMDevice *self = NM_DEVICE(dbobj->nmobj);
@ -1307,6 +1316,9 @@ _nm_device_notify_update_prop_hw_address(NMClient *client,
gboolean is_new = (meta_iface == &_nml_dbus_meta_iface_nm_device);
gboolean changed = FALSE;
if (is_removed)
return NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NONE;
if (!is_new && priv->hw_address_is_new) {
/* once the instance is marked to honor the new property, the
* changed signal for the old variant gets ignored. */
@ -1364,6 +1376,7 @@ _nm_device_notify_update_prop_ports(NMClient *client,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value)
{
const NMLDBusMetaProperty *meta_property =

4
src/libnm-client-impl/nm-dhcp-config.c
View file

@ -34,11 +34,15 @@ _notify_update_prop_options(NMClient *client,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value)
{
NMDhcpConfig *self = NM_DHCP_CONFIG(dbobj->nmobj);
NMDhcpConfigPrivate *priv = NM_DHCP_CONFIG_GET_PRIVATE(self);
if (is_removed)
return NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NONE;
g_hash_table_remove_all(priv->options);
if (value) {

16
src/libnm-client-impl/nm-ip-config.c
View file

@ -55,6 +55,7 @@ _notify_update_prop_addresses(NMClient *client,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value)
{
NMIPConfig *self = NM_IP_CONFIG(dbobj->nmobj);
@ -64,6 +65,9 @@ _notify_update_prop_addresses(NMClient *client,
int addr_family = meta_iface == &_nml_dbus_meta_iface_nm_ip4config ? AF_INET : AF_INET6;
gboolean new_style;
if (is_removed)
return NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NONE;
new_style =
(((const char *) meta_iface->dbus_properties[dbus_property_idx].dbus_type)[2] == '{');
@ -95,6 +99,7 @@ _notify_update_prop_routes(NMClient *client,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value)
{
NMIPConfig *self = NM_IP_CONFIG(dbobj->nmobj);
@ -104,6 +109,9 @@ _notify_update_prop_routes(NMClient *client,
int addr_family = meta_iface == &_nml_dbus_meta_iface_nm_ip4config ? AF_INET : AF_INET6;
gboolean new_style;
if (is_removed)
return NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NONE;
new_style =
(((const char *) meta_iface->dbus_properties[dbus_property_idx].dbus_type)[2] == '{');
@ -135,6 +143,7 @@ _notify_update_prop_nameservers(NMClient *client,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value)
{
NMIPConfig *self = NM_IP_CONFIG(dbobj->nmobj);
@ -143,6 +152,9 @@ _notify_update_prop_nameservers(NMClient *client,
gboolean new_style = TRUE;
int addr_family = meta_iface == &_nml_dbus_meta_iface_nm_ip4config ? AF_INET : AF_INET6;
if (is_removed)
return NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NONE;
if (addr_family == AF_INET) {
new_style =
(((const char *) meta_iface->dbus_properties[dbus_property_idx].dbus_type)[1] == 'a');
@ -205,6 +217,7 @@ _notify_update_prop_wins_servers(NMClient *client,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value)
{
NMIPConfig *self = NM_IP_CONFIG(dbobj->nmobj);
@ -212,6 +225,9 @@ _notify_update_prop_wins_servers(NMClient *client,
gs_strfreev char **wins_servers_new = NULL;
gboolean new_style;
if (is_removed)
return NML_DBUS_NOTIFY_UPDATE_PROP_FLAGS_NONE;
new_style =
(((const char *) meta_iface->dbus_properties[dbus_property_idx].dbus_type)[1] == 's');

5
src/libnm-client-impl/nm-libnm-utils.h
View file

@ -325,12 +325,14 @@ NMLDBusNotifyUpdatePropFlags _nml_dbus_notify_update_prop_ignore(NMClient
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value);
NMLDBusNotifyUpdatePropFlags _nml_dbus_notify_update_prop_o(NMClient *client,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value);
NMLDBusNotifyUpdatePropFlags nml_dbus_property_ao_notify(NMClient *self,
@ -348,6 +350,7 @@ typedef struct {
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value);
guint16 prop_struct_offset;
@ -1043,12 +1046,14 @@ _nm_device_notify_update_prop_hw_address(NMClient *client,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value);
NMLDBusNotifyUpdatePropFlags _nm_device_notify_update_prop_ports(NMClient *client,
NMLDBusObject *dbobj,
const NMLDBusMetaIface *meta_iface,
guint dbus_property_idx,
gboolean is_removed,
GVariant *value);
/*****************************************************************************/