2016-07-01 12:11:01 +02:00
|
|
|
/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
|
|
|
|
|
/* NetworkManager -- Network link manager
|
|
|
|
|
*
|
|
|
|
|
* This program is free software; you can redistribute it and/or modify
|
|
|
|
|
* it under the terms of the GNU General Public License as published by
|
|
|
|
|
* the Free Software Foundation; either version 2 of the License, or
|
|
|
|
|
* (at your option) any later version.
|
|
|
|
|
*
|
|
|
|
|
* This program is distributed in the hope that it will be useful,
|
|
|
|
|
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
|
|
|
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
|
|
|
|
* GNU General Public License for more details.
|
|
|
|
|
*
|
|
|
|
|
* You should have received a copy of the GNU General Public License along
|
|
|
|
|
* with this program; if not, write to the Free Software Foundation, Inc.,
|
|
|
|
|
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
|
|
|
|
|
*
|
|
|
|
|
* Copyright (C) 2016 Red Hat, Inc.
|
|
|
|
|
*/
|
|
|
|
|
|
|
|
|
|
#include "nm-default.h"
|
2016-09-29 13:49:01 +02:00
|
|
|
|
2016-07-01 12:11:01 +02:00
|
|
|
#include "nm-checkpoint.h"
|
|
|
|
|
|
2017-03-05 01:00:52 +01:00
|
|
|
#include "nm-active-connection.h"
|
2018-03-29 08:30:26 +02:00
|
|
|
#include "nm-act-request.h"
|
2016-07-01 12:11:01 +02:00
|
|
|
#include "nm-auth-subject.h"
|
|
|
|
|
#include "nm-core-utils.h"
|
|
|
|
|
#include "nm-dbus-interface.h"
|
2016-11-21 00:43:52 +01:00
|
|
|
#include "devices/nm-device.h"
|
2016-07-01 12:11:01 +02:00
|
|
|
#include "nm-manager.h"
|
2016-11-21 00:43:52 +01:00
|
|
|
#include "settings/nm-settings.h"
|
|
|
|
|
#include "settings/nm-settings-connection.h"
|
2016-07-01 12:11:01 +02:00
|
|
|
#include "nm-simple-connection.h"
|
|
|
|
|
#include "nm-utils.h"
|
|
|
|
|
|
2016-09-29 13:49:01 +02:00
|
|
|
/*****************************************************************************/
|
2016-07-01 12:11:01 +02:00
|
|
|
|
|
|
|
|
typedef struct {
|
|
|
|
|
char *original_dev_path;
|
2019-03-15 18:56:23 +01:00
|
|
|
char *original_dev_name;
|
2019-03-27 16:55:00 +01:00
|
|
|
NMDeviceType dev_type;
|
2016-07-01 12:11:01 +02:00
|
|
|
NMDevice *device;
|
2016-09-07 17:47:28 +02:00
|
|
|
NMConnection *applied_connection;
|
|
|
|
|
NMConnection *settings_connection;
|
2017-03-05 01:00:52 +01:00
|
|
|
guint64 ac_version_id;
|
2016-09-07 17:47:17 +02:00
|
|
|
NMDeviceState state;
|
2019-03-15 18:56:23 +01:00
|
|
|
bool is_software:1;
|
2016-09-07 17:47:17 +02:00
|
|
|
bool realized:1;
|
core: improve and fix keeping connection active based on "connection.permissions"
By setting "connection.permissions", a profile is restricted to a
particular user.
That means for example, that another user cannot see, modify, delete,
activate or deactivate the profile. It also means, that the profile
will only autoconnect when the user is logged in (has a session).
Note that root is always able to activate the profile. Likewise, the
user is also allowed to manually activate the own profile, even if no
session currently exists (which can easily happen with `sudo`).
When the user logs out (the session goes away), we want do disconnect
the profile, however there are conflicting goals here:
1) if the profile was activate by root user, then logging out the user
should not disconnect the profile. The patch fixes that by not
binding the activation to the connection, if the activation is done
by the root user.
2) if the profile was activated by the owner when it had no session,
then it should stay alive until the user logs in (once) and logs
out again. This is already handled by the previous commit.
Yes, this point is odd. If you first do
$ sudo -u $OTHER_USER nmcli connection up $PROFILE
the profile activates despite not having a session. If you then
$ ssh guest@localhost nmcli device
you'll still see the profile active. However, the moment the SSH session
ends, a session closes and the profile disconnects. It's unclear, how to
solve that any better. I think, a user who cares about this, should not
activate the profile without having a session in the first place.
There are quite some special cases, in particular with internal
activations. In those cases we need to decide whether to bind the
activation to the profile's visibility.
Also, expose the "bind" setting in the D-Bus API. Note, that in the future
this flag may be modified via D-Bus API. Like we may also add related API
that allows to tweak the lifetime of the activation.
Also, I think we broke handling of connection visiblity with 37e8c53eeed
"core: Introduce helper class to track connection keep alive". This
should be fixed now too, with improved behavior.
Fixes: 37e8c53eeed579fe34a68819cd12f3295d581394
https://bugzilla.redhat.com/show_bug.cgi?id=1530977
2018-11-21 13:30:16 +01:00
|
|
|
bool activation_lifetime_bound_to_profile_visiblity:1;
|
2017-07-05 13:47:32 +02:00
|
|
|
NMUnmanFlagOp unmanaged_explicit;
|
2018-03-28 17:18:04 +02:00
|
|
|
NMActivationReason activation_reason;
|
2019-03-13 11:21:02 +01:00
|
|
|
gulong dev_exported_change_id;
|
2016-07-01 12:11:01 +02:00
|
|
|
} DeviceCheckpoint;
|
|
|
|
|
|
checkpoint: allow resetting the rollback timeout via D-Bus
This allows to adjust the timeout of an existing checkpoint.
The main usecase of checkpoints, is to have a fail-safe when
configuring the network remotely. By allowing to reset the timeout,
the user can perform a series of actions, and keep bumping the
timeout. That way, the entire series is still guarded by the same
checkpoint, but the user can start with short timeout, and
re-adjust the timeout as he goes along.
The libnm API only implements the async form (at least for now).
Sync methods are fundamentally wrong with D-Bus, and it's probably
not needed. Also, follow glib convenction, where the async form
doesn't have the _async name suffix. Also, accept a D-Bus path
as argument, not a NMCheckpoint instance. The libnm API should
not be more restricted than the underlying D-Bus API. It would
be cumbersome to require the user to lookup the NMCheckpoint
instance first, especially since libnm doesn't provide an efficient
or convenient lookup-by-path method. On the other hand, retrieving
the path from a NMCheckpoint instance is always possible.
2018-03-28 08:09:56 +02:00
|
|
|
NM_GOBJECT_PROPERTIES_DEFINE (NMCheckpoint,
|
2016-09-29 13:49:01 +02:00
|
|
|
PROP_DEVICES,
|
|
|
|
|
PROP_CREATED,
|
|
|
|
|
PROP_ROLLBACK_TIMEOUT,
|
|
|
|
|
);
|
|
|
|
|
|
2018-03-27 12:45:23 +02:00
|
|
|
struct _NMCheckpointPrivate {
|
2016-07-01 12:11:01 +02:00
|
|
|
/* properties */
|
|
|
|
|
GHashTable *devices;
|
2019-03-27 16:55:00 +01:00
|
|
|
GPtrArray *removed_devices;
|
2018-03-27 19:02:15 +02:00
|
|
|
gint64 created_at_ms;
|
|
|
|
|
guint32 rollback_timeout_s;
|
|
|
|
|
guint timeout_id;
|
|
|
|
|
/* private members */
|
2016-07-01 12:11:01 +02:00
|
|
|
NMManager *manager;
|
2016-09-23 09:37:42 +02:00
|
|
|
NMCheckpointCreateFlags flags;
|
|
|
|
|
GHashTable *connection_uuids;
|
2019-03-27 16:55:00 +01:00
|
|
|
gulong dev_removed_id;
|
2018-03-27 19:02:15 +02:00
|
|
|
|
|
|
|
|
NMCheckpointTimeoutCallback timeout_cb;
|
|
|
|
|
gpointer timeout_data;
|
2016-07-01 12:11:01 +02:00
|
|
|
};
|
|
|
|
|
|
2016-09-29 13:49:01 +02:00
|
|
|
struct _NMCheckpointClass {
|
core/dbus: rework D-Bus implementation to use lower layer GDBusConnection API
Previously, we used the generated GDBusInterfaceSkeleton types and glued
them via the NMExportedObject base class to our NM types. We also used
GDBusObjectManagerServer.
Don't do that anymore. The resulting code was more complicated despite (or
because?) using generated classes. It was hard to understand, complex, had
ordering-issues, and had a runtime and memory overhead.
This patch refactors this entirely and uses the lower layer API GDBusConnection
directly. It replaces the generated code, GDBusInterfaceSkeleton, and
GDBusObjectManagerServer. All this is now done by NMDbusObject and NMDBusManager
and static descriptor instances of type GDBusInterfaceInfo.
This adds a net plus of more then 1300 lines of hand written code. I claim
that this implementation is easier to understand. Note that previously we
also required extensive and complex glue code to bind our objects to the
generated skeleton objects. Instead, now glue our objects directly to
GDBusConnection. The result is more immediate and gets rid of layers of
code in between.
Now that the D-Bus glue us more under our control, we can address issus and
bottlenecks better, instead of adding code to bend the generated skeletons
to our needs.
Note that the current implementation now only supports one D-Bus connection.
That was effectively the case already, although there were places (and still are)
where the code pretends it could also support connections from a private socket.
We dropped private socket support mainly because it was unused, untested and
buggy, but also because GDBusObjectManagerServer could not export the same
objects on multiple connections. Now, it would be rather straight forward to
fix that and re-introduce ObjectManager on each private connection. But this
commit doesn't do that yet, and the new code intentionally supports only one
D-Bus connection.
Also, the D-Bus startup was simplified. There is no retry, either nm_dbus_manager_start()
succeeds, or it detects the initrd case. In the initrd case, bus manager never tries to
connect to D-Bus. Since the initrd scenario is not yet used/tested, this is good enough
for the moment. It could be easily extended later, for example with polling whether the
system bus appears (like was done previously). Also, restart of D-Bus daemon isn't
supported either -- just like before.
Note how NMDBusManager now implements the ObjectManager D-Bus interface
directly.
Also, this fixes race issues in the server, by no longer delaying
PropertiesChanged signals. NMExportedObject would collect changed
properties and send the signal out in idle_emit_properties_changed()
on idle. This messes up the ordering of change events w.r.t. other
signals and events on the bus. Note that not only NMExportedObject
messed up the ordering. Also the generated code would hook into
notify() and process change events in and idle handle, exhibiting the
same ordering issue too.
No longer do that. PropertiesChanged signals will be sent right away
by hooking into dispatch_properties_changed(). This means, changing
a property in quick succession will no longer be combined and is
guaranteed to emit signals for each individual state. Quite possibly
we emit now more PropertiesChanged signals then before.
However, we are now able to group a set of changes by using standard
g_object_freeze_notify()/g_object_thaw_notify(). We probably should
make more use of that.
Also, now that our signals are all handled in the right order, we
might find places where we still emit them in the wrong order. But that
is then due to the order in which our GObjects emit signals, not due
to an ill behavior of the D-Bus glue. Possibly we need to identify
such ordering issues and fix them.
Numbers (for contrib/rpm --without debug on x86_64):
- the patch changes the code size of NetworkManager by
- 2809360 bytes
+ 2537528 bytes (-9.7%)
- Runtime measurements are harder because there is a large variance
during testing. In other words, the numbers are not reproducible.
Currently, the implementation performs no caching of GVariants at all,
but it would be rather simple to add it, if that turns out to be
useful.
Anyway, without strong claim, it seems that the new form tends to
perform slightly better. That would be no surprise.
$ time (for i in {1..1000}; do nmcli >/dev/null || break; echo -n .; done)
- real 1m39.355s
+ real 1m37.432s
$ time (for i in {1..2000}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects > /dev/null || break; echo -n .; done)
- real 0m26.843s
+ real 0m25.281s
- Regarding RSS size, just looking at the processes in similar
conditions, doesn't give a large difference. On my system they
consume about 19MB RSS. It seems that the new version has a
slightly smaller RSS size.
- 19356 RSS
+ 18660 RSS
2018-02-26 13:51:52 +01:00
|
|
|
NMDBusObjectClass parent;
|
2016-09-29 13:49:01 +02:00
|
|
|
};
|
2016-07-01 12:11:01 +02:00
|
|
|
|
core/dbus: rework D-Bus implementation to use lower layer GDBusConnection API
Previously, we used the generated GDBusInterfaceSkeleton types and glued
them via the NMExportedObject base class to our NM types. We also used
GDBusObjectManagerServer.
Don't do that anymore. The resulting code was more complicated despite (or
because?) using generated classes. It was hard to understand, complex, had
ordering-issues, and had a runtime and memory overhead.
This patch refactors this entirely and uses the lower layer API GDBusConnection
directly. It replaces the generated code, GDBusInterfaceSkeleton, and
GDBusObjectManagerServer. All this is now done by NMDbusObject and NMDBusManager
and static descriptor instances of type GDBusInterfaceInfo.
This adds a net plus of more then 1300 lines of hand written code. I claim
that this implementation is easier to understand. Note that previously we
also required extensive and complex glue code to bind our objects to the
generated skeleton objects. Instead, now glue our objects directly to
GDBusConnection. The result is more immediate and gets rid of layers of
code in between.
Now that the D-Bus glue us more under our control, we can address issus and
bottlenecks better, instead of adding code to bend the generated skeletons
to our needs.
Note that the current implementation now only supports one D-Bus connection.
That was effectively the case already, although there were places (and still are)
where the code pretends it could also support connections from a private socket.
We dropped private socket support mainly because it was unused, untested and
buggy, but also because GDBusObjectManagerServer could not export the same
objects on multiple connections. Now, it would be rather straight forward to
fix that and re-introduce ObjectManager on each private connection. But this
commit doesn't do that yet, and the new code intentionally supports only one
D-Bus connection.
Also, the D-Bus startup was simplified. There is no retry, either nm_dbus_manager_start()
succeeds, or it detects the initrd case. In the initrd case, bus manager never tries to
connect to D-Bus. Since the initrd scenario is not yet used/tested, this is good enough
for the moment. It could be easily extended later, for example with polling whether the
system bus appears (like was done previously). Also, restart of D-Bus daemon isn't
supported either -- just like before.
Note how NMDBusManager now implements the ObjectManager D-Bus interface
directly.
Also, this fixes race issues in the server, by no longer delaying
PropertiesChanged signals. NMExportedObject would collect changed
properties and send the signal out in idle_emit_properties_changed()
on idle. This messes up the ordering of change events w.r.t. other
signals and events on the bus. Note that not only NMExportedObject
messed up the ordering. Also the generated code would hook into
notify() and process change events in and idle handle, exhibiting the
same ordering issue too.
No longer do that. PropertiesChanged signals will be sent right away
by hooking into dispatch_properties_changed(). This means, changing
a property in quick succession will no longer be combined and is
guaranteed to emit signals for each individual state. Quite possibly
we emit now more PropertiesChanged signals then before.
However, we are now able to group a set of changes by using standard
g_object_freeze_notify()/g_object_thaw_notify(). We probably should
make more use of that.
Also, now that our signals are all handled in the right order, we
might find places where we still emit them in the wrong order. But that
is then due to the order in which our GObjects emit signals, not due
to an ill behavior of the D-Bus glue. Possibly we need to identify
such ordering issues and fix them.
Numbers (for contrib/rpm --without debug on x86_64):
- the patch changes the code size of NetworkManager by
- 2809360 bytes
+ 2537528 bytes (-9.7%)
- Runtime measurements are harder because there is a large variance
during testing. In other words, the numbers are not reproducible.
Currently, the implementation performs no caching of GVariants at all,
but it would be rather simple to add it, if that turns out to be
useful.
Anyway, without strong claim, it seems that the new form tends to
perform slightly better. That would be no surprise.
$ time (for i in {1..1000}; do nmcli >/dev/null || break; echo -n .; done)
- real 1m39.355s
+ real 1m37.432s
$ time (for i in {1..2000}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects > /dev/null || break; echo -n .; done)
- real 0m26.843s
+ real 0m25.281s
- Regarding RSS size, just looking at the processes in similar
conditions, doesn't give a large difference. On my system they
consume about 19MB RSS. It seems that the new version has a
slightly smaller RSS size.
- 19356 RSS
+ 18660 RSS
2018-02-26 13:51:52 +01:00
|
|
|
G_DEFINE_TYPE (NMCheckpoint, nm_checkpoint, NM_TYPE_DBUS_OBJECT)
|
2016-07-01 12:11:01 +02:00
|
|
|
|
2018-03-27 12:45:23 +02:00
|
|
|
#define NM_CHECKPOINT_GET_PRIVATE(self) _NM_GET_PRIVATE_PTR (self, NMCheckpoint, NM_IS_CHECKPOINT)
|
2016-07-01 12:11:01 +02:00
|
|
|
|
2016-09-29 13:49:01 +02:00
|
|
|
/*****************************************************************************/
|
|
|
|
|
|
|
|
|
|
#define _NMLOG_PREFIX_NAME "checkpoint"
|
|
|
|
|
#define _NMLOG_DOMAIN LOGD_CORE
|
|
|
|
|
|
|
|
|
|
#define _NMLOG(level, ...) \
|
|
|
|
|
G_STMT_START { \
|
|
|
|
|
if (nm_logging_enabled (level, _NMLOG_DOMAIN)) { \
|
|
|
|
|
char __prefix[32]; \
|
|
|
|
|
\
|
|
|
|
|
if (self) \
|
|
|
|
|
g_snprintf (__prefix, sizeof (__prefix), "%s[%p]", ""_NMLOG_PREFIX_NAME"", (self)); \
|
|
|
|
|
else \
|
|
|
|
|
g_strlcpy (__prefix, _NMLOG_PREFIX_NAME, sizeof (__prefix)); \
|
2017-03-01 10:20:01 +00:00
|
|
|
_nm_log ((level), (_NMLOG_DOMAIN), 0, NULL, NULL, \
|
2016-09-29 13:49:01 +02:00
|
|
|
"%s: " _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \
|
|
|
|
|
__prefix _NM_UTILS_MACRO_REST(__VA_ARGS__)); \
|
|
|
|
|
} \
|
|
|
|
|
} G_STMT_END
|
|
|
|
|
|
|
|
|
|
/*****************************************************************************/
|
2016-07-01 12:11:01 +02:00
|
|
|
|
checkpoint: allow overlapping checkpoints
Introduce a new flag NM_CHECKPOINT_CREATE_FLAG_ALLOW_OVERLAPPING
that allows the creation of overlapping checkpoints. Before, and
by default, checkpoints that reference a same device conflict,
and creating such a checkpoint failed.
Now, allow this. But during rollback automatically destroy all
overlapping checkpoints that were created after the checkpoint
that is about to rollback.
With this, you can create a series of checkpoints, and rollback them
individually. With the restriction, that if you once rolled back to an
older checkpoint, you no longer can roll"forward" to a younger one.
What this implies and what is new here, is that the checkpoint might be
automatically destroyed by NetworkManager before the timeout expires. When
the user later would try to manually destroy/rollback such a checkpoint, it
would fail because the checkpoint no longer exists.
2018-03-28 07:06:10 +02:00
|
|
|
void
|
|
|
|
|
nm_checkpoint_log_destroy (NMCheckpoint *self)
|
|
|
|
|
{
|
|
|
|
|
_LOGI ("destroy %s", nm_dbus_object_get_path (NM_DBUS_OBJECT (self)));
|
|
|
|
|
}
|
|
|
|
|
|
2018-03-27 19:02:15 +02:00
|
|
|
void
|
|
|
|
|
nm_checkpoint_set_timeout_callback (NMCheckpoint *self,
|
|
|
|
|
NMCheckpointTimeoutCallback callback,
|
|
|
|
|
gpointer user_data)
|
2016-07-01 12:11:01 +02:00
|
|
|
{
|
2018-03-27 19:02:15 +02:00
|
|
|
NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE (self);
|
2016-07-01 12:11:01 +02:00
|
|
|
|
2018-03-27 19:02:15 +02:00
|
|
|
/* in glib world, we would have a GSignal for this. But as there
|
|
|
|
|
* is only one subscriber, it's simpler to just set and unset(!)
|
|
|
|
|
* the callback this way. */
|
|
|
|
|
priv->timeout_cb = callback;
|
|
|
|
|
priv->timeout_data = user_data;
|
2016-07-01 12:11:01 +02:00
|
|
|
}
|
|
|
|
|
|
checkpoint: allow overlapping checkpoints
Introduce a new flag NM_CHECKPOINT_CREATE_FLAG_ALLOW_OVERLAPPING
that allows the creation of overlapping checkpoints. Before, and
by default, checkpoints that reference a same device conflict,
and creating such a checkpoint failed.
Now, allow this. But during rollback automatically destroy all
overlapping checkpoints that were created after the checkpoint
that is about to rollback.
With this, you can create a series of checkpoints, and rollback them
individually. With the restriction, that if you once rolled back to an
older checkpoint, you no longer can roll"forward" to a younger one.
What this implies and what is new here, is that the checkpoint might be
automatically destroyed by NetworkManager before the timeout expires. When
the user later would try to manually destroy/rollback such a checkpoint, it
would fail because the checkpoint no longer exists.
2018-03-28 07:06:10 +02:00
|
|
|
NMDevice *
|
|
|
|
|
nm_checkpoint_includes_devices (NMCheckpoint *self, NMDevice *const*devices, guint n_devices)
|
|
|
|
|
{
|
|
|
|
|
NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE (self);
|
|
|
|
|
guint i;
|
|
|
|
|
|
|
|
|
|
for (i = 0; i < n_devices; i++) {
|
|
|
|
|
if (g_hash_table_contains (priv->devices, devices[i]))
|
|
|
|
|
return devices[i];
|
|
|
|
|
}
|
|
|
|
|
return NULL;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
NMDevice *
|
|
|
|
|
nm_checkpoint_includes_devices_of (NMCheckpoint *self, NMCheckpoint *cp_for_devices)
|
2016-07-01 12:11:01 +02:00
|
|
|
{
|
|
|
|
|
NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE (self);
|
checkpoint: allow overlapping checkpoints
Introduce a new flag NM_CHECKPOINT_CREATE_FLAG_ALLOW_OVERLAPPING
that allows the creation of overlapping checkpoints. Before, and
by default, checkpoints that reference a same device conflict,
and creating such a checkpoint failed.
Now, allow this. But during rollback automatically destroy all
overlapping checkpoints that were created after the checkpoint
that is about to rollback.
With this, you can create a series of checkpoints, and rollback them
individually. With the restriction, that if you once rolled back to an
older checkpoint, you no longer can roll"forward" to a younger one.
What this implies and what is new here, is that the checkpoint might be
automatically destroyed by NetworkManager before the timeout expires. When
the user later would try to manually destroy/rollback such a checkpoint, it
would fail because the checkpoint no longer exists.
2018-03-28 07:06:10 +02:00
|
|
|
NMCheckpointPrivate *priv2 = NM_CHECKPOINT_GET_PRIVATE (cp_for_devices);
|
|
|
|
|
GHashTableIter iter;
|
|
|
|
|
NMDevice *device;
|
2016-07-01 12:11:01 +02:00
|
|
|
|
checkpoint: allow overlapping checkpoints
Introduce a new flag NM_CHECKPOINT_CREATE_FLAG_ALLOW_OVERLAPPING
that allows the creation of overlapping checkpoints. Before, and
by default, checkpoints that reference a same device conflict,
and creating such a checkpoint failed.
Now, allow this. But during rollback automatically destroy all
overlapping checkpoints that were created after the checkpoint
that is about to rollback.
With this, you can create a series of checkpoints, and rollback them
individually. With the restriction, that if you once rolled back to an
older checkpoint, you no longer can roll"forward" to a younger one.
What this implies and what is new here, is that the checkpoint might be
automatically destroyed by NetworkManager before the timeout expires. When
the user later would try to manually destroy/rollback such a checkpoint, it
would fail because the checkpoint no longer exists.
2018-03-28 07:06:10 +02:00
|
|
|
g_hash_table_iter_init (&iter, priv2->devices);
|
|
|
|
|
while (g_hash_table_iter_next (&iter, (gpointer *) &device, NULL)) {
|
|
|
|
|
if (g_hash_table_contains (priv->devices, device))
|
|
|
|
|
return device;
|
|
|
|
|
}
|
|
|
|
|
return NULL;
|
2016-07-01 12:11:01 +02:00
|
|
|
}
|
|
|
|
|
|
2017-03-05 01:00:52 +01:00
|
|
|
static NMSettingsConnection *
|
|
|
|
|
find_settings_connection (NMCheckpoint *self,
|
|
|
|
|
DeviceCheckpoint *dev_checkpoint,
|
|
|
|
|
gboolean *need_update,
|
|
|
|
|
gboolean *need_activation)
|
|
|
|
|
{
|
|
|
|
|
NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE (self);
|
2017-11-23 21:30:09 +01:00
|
|
|
NMActiveConnection *active;
|
settings: use delegation instead of inheritance for NMSettingsConnection and NMConnection
NMConnection is an interface, which is implemented by the types
NMSimpleConnection (libnm-core), NMSettingsConnection (src) and
NMRemoteConnection (libnm).
NMSettingsConnection does a lot of things already:
1) it "is-a" NMDBusObject and exports the API of a connection profile
on D-Bus
2) it interacts with NMSettings and contains functionality
for tracking the profiles.
3) it is the base-class of types like NMSKeyfileConnection and
NMIfcfgConnection. These handle how the profile is persisted
on disk.
4) it implements NMConnection interface, to itself track the
settings of the profile.
3) and 4) would be better implemented via delegation than inheritance.
Address 4) and don't let NMSettingsConnection implemente the NMConnection
interface. Instead, a settings-connection references now a NMSimpleConnection
instance, to which it delegates for keeping the actual profiles.
Advantages:
- by delegating, there is a clearer separation of what
NMSettingsConnection does. For example, in C we often required
casts from NMSettingsConnection to NMConnection. NMConnection
is a very trivial object with very little logic. When we have
a NMConnection instance at hand, it's good to know that it is
*only* that simple instead of also being an entire
NMSettingsConnection instance.
The main purpose of this patch is to simplify the code by separating
the NMConnection from the NMSettingsConnection. We should generally
be aware whether we handle a NMSettingsConnection or a trivial
NMConnection instance. Now, because NMSettingsConnection no longer
"is-a" NMConnection, this distinction is apparent.
- NMConnection is implemented as an interface and we create
NMSimpleConnection instances whenever we need a real instance.
In GLib, interfaces have a performance overhead, that we needlessly
pay all the time. With this change, we no longer require
NMConnection to be an interface. Thus, in the future we could compile
a version of libnm-core for the daemon, where NMConnection is not an
interface but a GObject implementation akin to NMSimpleConnection.
- In the previous implementation, we cannot treat NMConnection immutable
and copy-on-write.
For example, when NMDevice needs a snapshot of the activated
profile as applied-connection, all it can do is clone the entire
NMSettingsConnection as a NMSimpleConnection.
Likewise, when we get a NMConnection instance and want to keep
a reference to it, we cannot do that, because we never know
who also references and modifies the instance.
By separating NMSettingsConnection we could in the future have
NMConnection immutable and copy-on-write, to avoid all unnecessary
clones.
2018-08-11 11:08:17 +02:00
|
|
|
NMSettingsConnection *sett_conn;
|
2017-03-05 01:00:52 +01:00
|
|
|
const char *uuid, *ac_uuid;
|
2017-11-23 21:30:09 +01:00
|
|
|
const CList *tmp_clist;
|
2017-03-05 01:00:52 +01:00
|
|
|
|
|
|
|
|
*need_activation = FALSE;
|
|
|
|
|
*need_update = FALSE;
|
|
|
|
|
|
|
|
|
|
uuid = nm_connection_get_uuid (dev_checkpoint->settings_connection);
|
settings: use delegation instead of inheritance for NMSettingsConnection and NMConnection
NMConnection is an interface, which is implemented by the types
NMSimpleConnection (libnm-core), NMSettingsConnection (src) and
NMRemoteConnection (libnm).
NMSettingsConnection does a lot of things already:
1) it "is-a" NMDBusObject and exports the API of a connection profile
on D-Bus
2) it interacts with NMSettings and contains functionality
for tracking the profiles.
3) it is the base-class of types like NMSKeyfileConnection and
NMIfcfgConnection. These handle how the profile is persisted
on disk.
4) it implements NMConnection interface, to itself track the
settings of the profile.
3) and 4) would be better implemented via delegation than inheritance.
Address 4) and don't let NMSettingsConnection implemente the NMConnection
interface. Instead, a settings-connection references now a NMSimpleConnection
instance, to which it delegates for keeping the actual profiles.
Advantages:
- by delegating, there is a clearer separation of what
NMSettingsConnection does. For example, in C we often required
casts from NMSettingsConnection to NMConnection. NMConnection
is a very trivial object with very little logic. When we have
a NMConnection instance at hand, it's good to know that it is
*only* that simple instead of also being an entire
NMSettingsConnection instance.
The main purpose of this patch is to simplify the code by separating
the NMConnection from the NMSettingsConnection. We should generally
be aware whether we handle a NMSettingsConnection or a trivial
NMConnection instance. Now, because NMSettingsConnection no longer
"is-a" NMConnection, this distinction is apparent.
- NMConnection is implemented as an interface and we create
NMSimpleConnection instances whenever we need a real instance.
In GLib, interfaces have a performance overhead, that we needlessly
pay all the time. With this change, we no longer require
NMConnection to be an interface. Thus, in the future we could compile
a version of libnm-core for the daemon, where NMConnection is not an
interface but a GObject implementation akin to NMSimpleConnection.
- In the previous implementation, we cannot treat NMConnection immutable
and copy-on-write.
For example, when NMDevice needs a snapshot of the activated
profile as applied-connection, all it can do is clone the entire
NMSettingsConnection as a NMSimpleConnection.
Likewise, when we get a NMConnection instance and want to keep
a reference to it, we cannot do that, because we never know
who also references and modifies the instance.
By separating NMSettingsConnection we could in the future have
NMConnection immutable and copy-on-write, to avoid all unnecessary
clones.
2018-08-11 11:08:17 +02:00
|
|
|
sett_conn = nm_settings_get_connection_by_uuid (nm_settings_get (), uuid);
|
2017-03-05 01:00:52 +01:00
|
|
|
|
settings: use delegation instead of inheritance for NMSettingsConnection and NMConnection
NMConnection is an interface, which is implemented by the types
NMSimpleConnection (libnm-core), NMSettingsConnection (src) and
NMRemoteConnection (libnm).
NMSettingsConnection does a lot of things already:
1) it "is-a" NMDBusObject and exports the API of a connection profile
on D-Bus
2) it interacts with NMSettings and contains functionality
for tracking the profiles.
3) it is the base-class of types like NMSKeyfileConnection and
NMIfcfgConnection. These handle how the profile is persisted
on disk.
4) it implements NMConnection interface, to itself track the
settings of the profile.
3) and 4) would be better implemented via delegation than inheritance.
Address 4) and don't let NMSettingsConnection implemente the NMConnection
interface. Instead, a settings-connection references now a NMSimpleConnection
instance, to which it delegates for keeping the actual profiles.
Advantages:
- by delegating, there is a clearer separation of what
NMSettingsConnection does. For example, in C we often required
casts from NMSettingsConnection to NMConnection. NMConnection
is a very trivial object with very little logic. When we have
a NMConnection instance at hand, it's good to know that it is
*only* that simple instead of also being an entire
NMSettingsConnection instance.
The main purpose of this patch is to simplify the code by separating
the NMConnection from the NMSettingsConnection. We should generally
be aware whether we handle a NMSettingsConnection or a trivial
NMConnection instance. Now, because NMSettingsConnection no longer
"is-a" NMConnection, this distinction is apparent.
- NMConnection is implemented as an interface and we create
NMSimpleConnection instances whenever we need a real instance.
In GLib, interfaces have a performance overhead, that we needlessly
pay all the time. With this change, we no longer require
NMConnection to be an interface. Thus, in the future we could compile
a version of libnm-core for the daemon, where NMConnection is not an
interface but a GObject implementation akin to NMSimpleConnection.
- In the previous implementation, we cannot treat NMConnection immutable
and copy-on-write.
For example, when NMDevice needs a snapshot of the activated
profile as applied-connection, all it can do is clone the entire
NMSettingsConnection as a NMSimpleConnection.
Likewise, when we get a NMConnection instance and want to keep
a reference to it, we cannot do that, because we never know
who also references and modifies the instance.
By separating NMSettingsConnection we could in the future have
NMConnection immutable and copy-on-write, to avoid all unnecessary
clones.
2018-08-11 11:08:17 +02:00
|
|
|
if (!sett_conn)
|
2017-03-05 01:00:52 +01:00
|
|
|
return NULL;
|
|
|
|
|
|
|
|
|
|
/* Now check if the connection changed, ... */
|
|
|
|
|
if (!nm_connection_compare (dev_checkpoint->settings_connection,
|
settings: use delegation instead of inheritance for NMSettingsConnection and NMConnection
NMConnection is an interface, which is implemented by the types
NMSimpleConnection (libnm-core), NMSettingsConnection (src) and
NMRemoteConnection (libnm).
NMSettingsConnection does a lot of things already:
1) it "is-a" NMDBusObject and exports the API of a connection profile
on D-Bus
2) it interacts with NMSettings and contains functionality
for tracking the profiles.
3) it is the base-class of types like NMSKeyfileConnection and
NMIfcfgConnection. These handle how the profile is persisted
on disk.
4) it implements NMConnection interface, to itself track the
settings of the profile.
3) and 4) would be better implemented via delegation than inheritance.
Address 4) and don't let NMSettingsConnection implemente the NMConnection
interface. Instead, a settings-connection references now a NMSimpleConnection
instance, to which it delegates for keeping the actual profiles.
Advantages:
- by delegating, there is a clearer separation of what
NMSettingsConnection does. For example, in C we often required
casts from NMSettingsConnection to NMConnection. NMConnection
is a very trivial object with very little logic. When we have
a NMConnection instance at hand, it's good to know that it is
*only* that simple instead of also being an entire
NMSettingsConnection instance.
The main purpose of this patch is to simplify the code by separating
the NMConnection from the NMSettingsConnection. We should generally
be aware whether we handle a NMSettingsConnection or a trivial
NMConnection instance. Now, because NMSettingsConnection no longer
"is-a" NMConnection, this distinction is apparent.
- NMConnection is implemented as an interface and we create
NMSimpleConnection instances whenever we need a real instance.
In GLib, interfaces have a performance overhead, that we needlessly
pay all the time. With this change, we no longer require
NMConnection to be an interface. Thus, in the future we could compile
a version of libnm-core for the daemon, where NMConnection is not an
interface but a GObject implementation akin to NMSimpleConnection.
- In the previous implementation, we cannot treat NMConnection immutable
and copy-on-write.
For example, when NMDevice needs a snapshot of the activated
profile as applied-connection, all it can do is clone the entire
NMSettingsConnection as a NMSimpleConnection.
Likewise, when we get a NMConnection instance and want to keep
a reference to it, we cannot do that, because we never know
who also references and modifies the instance.
By separating NMSettingsConnection we could in the future have
NMConnection immutable and copy-on-write, to avoid all unnecessary
clones.
2018-08-11 11:08:17 +02:00
|
|
|
nm_settings_connection_get_connection (sett_conn),
|
2017-03-05 01:00:52 +01:00
|
|
|
NM_SETTING_COMPARE_FLAG_EXACT)) {
|
|
|
|
|
_LOGT ("rollback: settings connection %s changed", uuid);
|
|
|
|
|
*need_update = TRUE;
|
|
|
|
|
*need_activation = TRUE;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
/* ... is active, ... */
|
2017-11-23 21:30:09 +01:00
|
|
|
nm_manager_for_each_active_connection (priv->manager, active, tmp_clist) {
|
2017-03-05 01:00:52 +01:00
|
|
|
ac_uuid = nm_settings_connection_get_uuid (nm_active_connection_get_settings_connection (active));
|
|
|
|
|
if (nm_streq (uuid, ac_uuid)) {
|
|
|
|
|
_LOGT ("rollback: connection %s is active", uuid);
|
|
|
|
|
break;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2017-11-23 21:30:09 +01:00
|
|
|
if (!active) {
|
2017-03-05 01:00:52 +01:00
|
|
|
_LOGT ("rollback: connection %s is not active", uuid);
|
|
|
|
|
*need_activation = TRUE;
|
settings: use delegation instead of inheritance for NMSettingsConnection and NMConnection
NMConnection is an interface, which is implemented by the types
NMSimpleConnection (libnm-core), NMSettingsConnection (src) and
NMRemoteConnection (libnm).
NMSettingsConnection does a lot of things already:
1) it "is-a" NMDBusObject and exports the API of a connection profile
on D-Bus
2) it interacts with NMSettings and contains functionality
for tracking the profiles.
3) it is the base-class of types like NMSKeyfileConnection and
NMIfcfgConnection. These handle how the profile is persisted
on disk.
4) it implements NMConnection interface, to itself track the
settings of the profile.
3) and 4) would be better implemented via delegation than inheritance.
Address 4) and don't let NMSettingsConnection implemente the NMConnection
interface. Instead, a settings-connection references now a NMSimpleConnection
instance, to which it delegates for keeping the actual profiles.
Advantages:
- by delegating, there is a clearer separation of what
NMSettingsConnection does. For example, in C we often required
casts from NMSettingsConnection to NMConnection. NMConnection
is a very trivial object with very little logic. When we have
a NMConnection instance at hand, it's good to know that it is
*only* that simple instead of also being an entire
NMSettingsConnection instance.
The main purpose of this patch is to simplify the code by separating
the NMConnection from the NMSettingsConnection. We should generally
be aware whether we handle a NMSettingsConnection or a trivial
NMConnection instance. Now, because NMSettingsConnection no longer
"is-a" NMConnection, this distinction is apparent.
- NMConnection is implemented as an interface and we create
NMSimpleConnection instances whenever we need a real instance.
In GLib, interfaces have a performance overhead, that we needlessly
pay all the time. With this change, we no longer require
NMConnection to be an interface. Thus, in the future we could compile
a version of libnm-core for the daemon, where NMConnection is not an
interface but a GObject implementation akin to NMSimpleConnection.
- In the previous implementation, we cannot treat NMConnection immutable
and copy-on-write.
For example, when NMDevice needs a snapshot of the activated
profile as applied-connection, all it can do is clone the entire
NMSettingsConnection as a NMSimpleConnection.
Likewise, when we get a NMConnection instance and want to keep
a reference to it, we cannot do that, because we never know
who also references and modifies the instance.
By separating NMSettingsConnection we could in the future have
NMConnection immutable and copy-on-write, to avoid all unnecessary
clones.
2018-08-11 11:08:17 +02:00
|
|
|
return sett_conn;
|
2017-03-05 01:00:52 +01:00
|
|
|
}
|
|
|
|
|
|
|
|
|
|
/* ... or if the connection was reactivated/reapplied */
|
|
|
|
|
if (nm_active_connection_version_id_get (active) != dev_checkpoint->ac_version_id) {
|
|
|
|
|
_LOGT ("rollback: active connection version id of %s changed", uuid);
|
|
|
|
|
*need_activation = TRUE;
|
|
|
|
|
}
|
|
|
|
|
|
settings: use delegation instead of inheritance for NMSettingsConnection and NMConnection
NMConnection is an interface, which is implemented by the types
NMSimpleConnection (libnm-core), NMSettingsConnection (src) and
NMRemoteConnection (libnm).
NMSettingsConnection does a lot of things already:
1) it "is-a" NMDBusObject and exports the API of a connection profile
on D-Bus
2) it interacts with NMSettings and contains functionality
for tracking the profiles.
3) it is the base-class of types like NMSKeyfileConnection and
NMIfcfgConnection. These handle how the profile is persisted
on disk.
4) it implements NMConnection interface, to itself track the
settings of the profile.
3) and 4) would be better implemented via delegation than inheritance.
Address 4) and don't let NMSettingsConnection implemente the NMConnection
interface. Instead, a settings-connection references now a NMSimpleConnection
instance, to which it delegates for keeping the actual profiles.
Advantages:
- by delegating, there is a clearer separation of what
NMSettingsConnection does. For example, in C we often required
casts from NMSettingsConnection to NMConnection. NMConnection
is a very trivial object with very little logic. When we have
a NMConnection instance at hand, it's good to know that it is
*only* that simple instead of also being an entire
NMSettingsConnection instance.
The main purpose of this patch is to simplify the code by separating
the NMConnection from the NMSettingsConnection. We should generally
be aware whether we handle a NMSettingsConnection or a trivial
NMConnection instance. Now, because NMSettingsConnection no longer
"is-a" NMConnection, this distinction is apparent.
- NMConnection is implemented as an interface and we create
NMSimpleConnection instances whenever we need a real instance.
In GLib, interfaces have a performance overhead, that we needlessly
pay all the time. With this change, we no longer require
NMConnection to be an interface. Thus, in the future we could compile
a version of libnm-core for the daemon, where NMConnection is not an
interface but a GObject implementation akin to NMSimpleConnection.
- In the previous implementation, we cannot treat NMConnection immutable
and copy-on-write.
For example, when NMDevice needs a snapshot of the activated
profile as applied-connection, all it can do is clone the entire
NMSettingsConnection as a NMSimpleConnection.
Likewise, when we get a NMConnection instance and want to keep
a reference to it, we cannot do that, because we never know
who also references and modifies the instance.
By separating NMSettingsConnection we could in the future have
NMConnection immutable and copy-on-write, to avoid all unnecessary
clones.
2018-08-11 11:08:17 +02:00
|
|
|
return sett_conn;
|
2017-03-05 01:00:52 +01:00
|
|
|
}
|
|
|
|
|
|
2019-03-27 13:07:24 +01:00
|
|
|
static gboolean
|
|
|
|
|
restore_and_activate_connection (NMCheckpoint *self,
|
|
|
|
|
DeviceCheckpoint *dev_checkpoint)
|
|
|
|
|
{
|
|
|
|
|
NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE (self);
|
|
|
|
|
NMSettingsConnection *connection;
|
|
|
|
|
gs_unref_object NMAuthSubject *subject = NULL;
|
|
|
|
|
GError *local_error = NULL;
|
|
|
|
|
gboolean need_update, need_activation;
|
|
|
|
|
|
|
|
|
|
connection = find_settings_connection (self,
|
|
|
|
|
dev_checkpoint,
|
|
|
|
|
&need_update,
|
|
|
|
|
&need_activation);
|
|
|
|
|
if (connection) {
|
|
|
|
|
if (need_update) {
|
|
|
|
|
_LOGD ("rollback: updating connection %s",
|
|
|
|
|
nm_settings_connection_get_uuid (connection));
|
|
|
|
|
nm_settings_connection_update (connection,
|
|
|
|
|
dev_checkpoint->settings_connection,
|
|
|
|
|
NM_SETTINGS_CONNECTION_PERSIST_MODE_DISK,
|
|
|
|
|
NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE,
|
|
|
|
|
"checkpoint-rollback",
|
|
|
|
|
NULL);
|
|
|
|
|
}
|
|
|
|
|
} else {
|
|
|
|
|
/* The connection was deleted, recreate it */
|
|
|
|
|
_LOGD ("rollback: adding connection %s again",
|
|
|
|
|
nm_connection_get_uuid (dev_checkpoint->settings_connection));
|
|
|
|
|
|
|
|
|
|
connection = nm_settings_add_connection (nm_settings_get (),
|
|
|
|
|
dev_checkpoint->settings_connection,
|
|
|
|
|
TRUE,
|
|
|
|
|
&local_error);
|
|
|
|
|
if (!connection) {
|
|
|
|
|
_LOGD ("rollback: connection add failure: %s", local_error->message);
|
|
|
|
|
g_clear_error (&local_error);
|
|
|
|
|
return FALSE;
|
|
|
|
|
}
|
2019-03-27 16:55:00 +01:00
|
|
|
|
|
|
|
|
/* If the device is software, a brand new NMDevice may have been created */
|
|
|
|
|
if ( dev_checkpoint->is_software
|
|
|
|
|
&& !dev_checkpoint->device) {
|
|
|
|
|
dev_checkpoint->device = nm_manager_get_device (priv->manager,
|
|
|
|
|
dev_checkpoint->original_dev_name,
|
|
|
|
|
dev_checkpoint->dev_type);
|
|
|
|
|
nm_g_object_ref (dev_checkpoint->device);
|
|
|
|
|
}
|
2019-03-27 13:07:24 +01:00
|
|
|
need_activation = TRUE;
|
|
|
|
|
}
|
|
|
|
|
|
2019-03-27 16:55:00 +01:00
|
|
|
if (!dev_checkpoint->device) {
|
|
|
|
|
_LOGD ("rollback: device cannot be restored");
|
|
|
|
|
return FALSE;
|
|
|
|
|
}
|
|
|
|
|
|
2019-03-27 13:07:24 +01:00
|
|
|
if (need_activation) {
|
|
|
|
|
_LOGD ("rollback: reactivating connection %s",
|
|
|
|
|
nm_settings_connection_get_uuid (connection));
|
|
|
|
|
subject = nm_auth_subject_new_internal ();
|
|
|
|
|
|
|
|
|
|
/* Disconnect the device if needed. This necessary because now
|
|
|
|
|
* the manager prevents the reactivation of the same connection by
|
|
|
|
|
* an internal subject. */
|
|
|
|
|
if ( nm_device_get_state (dev_checkpoint->device) > NM_DEVICE_STATE_DISCONNECTED
|
|
|
|
|
&& nm_device_get_state (dev_checkpoint->device) < NM_DEVICE_STATE_DEACTIVATING) {
|
|
|
|
|
nm_device_state_changed (dev_checkpoint->device,
|
|
|
|
|
NM_DEVICE_STATE_DEACTIVATING,
|
|
|
|
|
NM_DEVICE_STATE_REASON_NEW_ACTIVATION);
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if (!nm_manager_activate_connection (priv->manager,
|
|
|
|
|
connection,
|
|
|
|
|
dev_checkpoint->applied_connection,
|
|
|
|
|
NULL,
|
|
|
|
|
dev_checkpoint->device,
|
|
|
|
|
subject,
|
|
|
|
|
NM_ACTIVATION_TYPE_MANAGED,
|
|
|
|
|
dev_checkpoint->activation_reason,
|
|
|
|
|
dev_checkpoint->activation_lifetime_bound_to_profile_visiblity
|
|
|
|
|
? NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY
|
|
|
|
|
: NM_ACTIVATION_STATE_FLAG_NONE,
|
|
|
|
|
&local_error)) {
|
|
|
|
|
_LOGW ("rollback: reactivation of connection %s/%s failed: %s",
|
|
|
|
|
nm_settings_connection_get_id (connection),
|
|
|
|
|
nm_settings_connection_get_uuid (connection),
|
|
|
|
|
local_error->message);
|
|
|
|
|
g_clear_error (&local_error);
|
|
|
|
|
return FALSE;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
return TRUE;
|
|
|
|
|
}
|
|
|
|
|
|
2016-07-01 12:11:01 +02:00
|
|
|
GVariant *
|
|
|
|
|
nm_checkpoint_rollback (NMCheckpoint *self)
|
|
|
|
|
{
|
|
|
|
|
NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE (self);
|
|
|
|
|
DeviceCheckpoint *dev_checkpoint;
|
|
|
|
|
GHashTableIter iter;
|
|
|
|
|
NMDevice *device;
|
|
|
|
|
GVariantBuilder builder;
|
2019-03-27 16:55:00 +01:00
|
|
|
uint i;
|
2016-07-01 12:11:01 +02:00
|
|
|
|
core/dbus: rework D-Bus implementation to use lower layer GDBusConnection API
Previously, we used the generated GDBusInterfaceSkeleton types and glued
them via the NMExportedObject base class to our NM types. We also used
GDBusObjectManagerServer.
Don't do that anymore. The resulting code was more complicated despite (or
because?) using generated classes. It was hard to understand, complex, had
ordering-issues, and had a runtime and memory overhead.
This patch refactors this entirely and uses the lower layer API GDBusConnection
directly. It replaces the generated code, GDBusInterfaceSkeleton, and
GDBusObjectManagerServer. All this is now done by NMDbusObject and NMDBusManager
and static descriptor instances of type GDBusInterfaceInfo.
This adds a net plus of more then 1300 lines of hand written code. I claim
that this implementation is easier to understand. Note that previously we
also required extensive and complex glue code to bind our objects to the
generated skeleton objects. Instead, now glue our objects directly to
GDBusConnection. The result is more immediate and gets rid of layers of
code in between.
Now that the D-Bus glue us more under our control, we can address issus and
bottlenecks better, instead of adding code to bend the generated skeletons
to our needs.
Note that the current implementation now only supports one D-Bus connection.
That was effectively the case already, although there were places (and still are)
where the code pretends it could also support connections from a private socket.
We dropped private socket support mainly because it was unused, untested and
buggy, but also because GDBusObjectManagerServer could not export the same
objects on multiple connections. Now, it would be rather straight forward to
fix that and re-introduce ObjectManager on each private connection. But this
commit doesn't do that yet, and the new code intentionally supports only one
D-Bus connection.
Also, the D-Bus startup was simplified. There is no retry, either nm_dbus_manager_start()
succeeds, or it detects the initrd case. In the initrd case, bus manager never tries to
connect to D-Bus. Since the initrd scenario is not yet used/tested, this is good enough
for the moment. It could be easily extended later, for example with polling whether the
system bus appears (like was done previously). Also, restart of D-Bus daemon isn't
supported either -- just like before.
Note how NMDBusManager now implements the ObjectManager D-Bus interface
directly.
Also, this fixes race issues in the server, by no longer delaying
PropertiesChanged signals. NMExportedObject would collect changed
properties and send the signal out in idle_emit_properties_changed()
on idle. This messes up the ordering of change events w.r.t. other
signals and events on the bus. Note that not only NMExportedObject
messed up the ordering. Also the generated code would hook into
notify() and process change events in and idle handle, exhibiting the
same ordering issue too.
No longer do that. PropertiesChanged signals will be sent right away
by hooking into dispatch_properties_changed(). This means, changing
a property in quick succession will no longer be combined and is
guaranteed to emit signals for each individual state. Quite possibly
we emit now more PropertiesChanged signals then before.
However, we are now able to group a set of changes by using standard
g_object_freeze_notify()/g_object_thaw_notify(). We probably should
make more use of that.
Also, now that our signals are all handled in the right order, we
might find places where we still emit them in the wrong order. But that
is then due to the order in which our GObjects emit signals, not due
to an ill behavior of the D-Bus glue. Possibly we need to identify
such ordering issues and fix them.
Numbers (for contrib/rpm --without debug on x86_64):
- the patch changes the code size of NetworkManager by
- 2809360 bytes
+ 2537528 bytes (-9.7%)
- Runtime measurements are harder because there is a large variance
during testing. In other words, the numbers are not reproducible.
Currently, the implementation performs no caching of GVariants at all,
but it would be rather simple to add it, if that turns out to be
useful.
Anyway, without strong claim, it seems that the new form tends to
perform slightly better. That would be no surprise.
$ time (for i in {1..1000}; do nmcli >/dev/null || break; echo -n .; done)
- real 1m39.355s
+ real 1m37.432s
$ time (for i in {1..2000}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects > /dev/null || break; echo -n .; done)
- real 0m26.843s
+ real 0m25.281s
- Regarding RSS size, just looking at the processes in similar
conditions, doesn't give a large difference. On my system they
consume about 19MB RSS. It seems that the new version has a
slightly smaller RSS size.
- 19356 RSS
+ 18660 RSS
2018-02-26 13:51:52 +01:00
|
|
|
_LOGI ("rollback of %s", nm_dbus_object_get_path (NM_DBUS_OBJECT (self)));
|
2016-07-01 12:11:01 +02:00
|
|
|
g_variant_builder_init (&builder, G_VARIANT_TYPE ("a{su}"));
|
|
|
|
|
|
2019-03-27 16:55:00 +01:00
|
|
|
/* Start creating removed devices (if any and if possible) */
|
|
|
|
|
if (priv->removed_devices) {
|
|
|
|
|
for (i = 0; i < priv->removed_devices->len; i++) {
|
|
|
|
|
guint32 result = NM_ROLLBACK_RESULT_OK;
|
|
|
|
|
|
|
|
|
|
dev_checkpoint = priv->removed_devices->pdata[i];
|
|
|
|
|
_LOGD ("rollback: restoring removed device %s (state %d, realized %d, explicitly unmanaged %d)",
|
|
|
|
|
dev_checkpoint->original_dev_name,
|
|
|
|
|
(int) dev_checkpoint->state,
|
|
|
|
|
dev_checkpoint->realized,
|
|
|
|
|
dev_checkpoint->unmanaged_explicit);
|
|
|
|
|
|
|
|
|
|
if (dev_checkpoint->applied_connection) {
|
|
|
|
|
if (!restore_and_activate_connection (self, dev_checkpoint))
|
|
|
|
|
result = NM_ROLLBACK_RESULT_ERR_FAILED;
|
|
|
|
|
}
|
|
|
|
|
g_variant_builder_add (&builder, "{su}", dev_checkpoint->original_dev_path, result);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2016-07-01 12:11:01 +02:00
|
|
|
/* Start rolling-back each device */
|
|
|
|
|
g_hash_table_iter_init (&iter, priv->devices);
|
|
|
|
|
while (g_hash_table_iter_next (&iter, (gpointer *) &device, (gpointer *) &dev_checkpoint)) {
|
|
|
|
|
guint32 result = NM_ROLLBACK_RESULT_OK;
|
|
|
|
|
|
2016-09-07 17:47:17 +02:00
|
|
|
_LOGD ("rollback: restoring device %s (state %d, realized %d, explicitly unmanaged %d)",
|
2019-03-15 18:56:23 +01:00
|
|
|
dev_checkpoint->original_dev_name,
|
2016-09-07 17:47:17 +02:00
|
|
|
(int) dev_checkpoint->state,
|
|
|
|
|
dev_checkpoint->realized,
|
|
|
|
|
dev_checkpoint->unmanaged_explicit);
|
|
|
|
|
|
|
|
|
|
if (nm_device_is_real (device)) {
|
|
|
|
|
if (!dev_checkpoint->realized) {
|
|
|
|
|
_LOGD ("rollback: device was not realized, unmanage it");
|
|
|
|
|
nm_device_set_unmanaged_by_flags_queue (device,
|
|
|
|
|
NM_UNMANAGED_USER_EXPLICIT,
|
|
|
|
|
TRUE,
|
|
|
|
|
NM_DEVICE_STATE_REASON_NOW_UNMANAGED);
|
|
|
|
|
goto next_dev;
|
|
|
|
|
}
|
|
|
|
|
} else {
|
|
|
|
|
if (dev_checkpoint->realized) {
|
2019-03-15 18:56:23 +01:00
|
|
|
if (dev_checkpoint->is_software) {
|
2016-09-07 17:47:17 +02:00
|
|
|
/* try to recreate software device */
|
|
|
|
|
_LOGD ("rollback: software device not realized, will re-activate");
|
|
|
|
|
goto activate;
|
|
|
|
|
} else {
|
|
|
|
|
_LOGD ("rollback: device is not realized");
|
|
|
|
|
result = NM_ROLLBACK_RESULT_ERR_FAILED;
|
|
|
|
|
}
|
|
|
|
|
}
|
2016-07-01 12:11:01 +02:00
|
|
|
goto next_dev;
|
|
|
|
|
}
|
|
|
|
|
|
2017-07-05 13:47:32 +02:00
|
|
|
/* Manage the device again if needed */
|
|
|
|
|
if ( nm_device_get_unmanaged_flags (device, NM_UNMANAGED_USER_EXPLICIT)
|
|
|
|
|
&& dev_checkpoint->unmanaged_explicit != NM_UNMAN_FLAG_OP_SET_UNMANAGED) {
|
|
|
|
|
_LOGD ("rollback: restore unmanaged user-explicit");
|
|
|
|
|
nm_device_set_unmanaged_by_flags_queue (device,
|
|
|
|
|
NM_UNMANAGED_USER_EXPLICIT,
|
|
|
|
|
dev_checkpoint->unmanaged_explicit,
|
|
|
|
|
NM_DEVICE_STATE_REASON_NOW_MANAGED);
|
|
|
|
|
}
|
|
|
|
|
|
2016-09-07 17:47:17 +02:00
|
|
|
if (dev_checkpoint->state == NM_DEVICE_STATE_UNMANAGED) {
|
|
|
|
|
if ( nm_device_get_state (device) != NM_DEVICE_STATE_UNMANAGED
|
2017-07-05 13:47:32 +02:00
|
|
|
|| dev_checkpoint->unmanaged_explicit == NM_UNMAN_FLAG_OP_SET_UNMANAGED) {
|
2016-09-07 17:47:17 +02:00
|
|
|
_LOGD ("rollback: explicitly unmanage device");
|
|
|
|
|
nm_device_set_unmanaged_by_flags_queue (device,
|
|
|
|
|
NM_UNMANAGED_USER_EXPLICIT,
|
|
|
|
|
TRUE,
|
|
|
|
|
NM_DEVICE_STATE_REASON_NOW_UNMANAGED);
|
|
|
|
|
}
|
2016-07-01 12:11:01 +02:00
|
|
|
goto next_dev;
|
|
|
|
|
}
|
|
|
|
|
|
2019-03-15 19:03:24 +01:00
|
|
|
activate:
|
2016-09-07 17:47:28 +02:00
|
|
|
if (dev_checkpoint->applied_connection) {
|
2019-03-27 13:07:24 +01:00
|
|
|
if (!restore_and_activate_connection (self, dev_checkpoint)) {
|
|
|
|
|
result = NM_ROLLBACK_RESULT_ERR_FAILED;
|
|
|
|
|
goto next_dev;
|
2016-07-01 12:11:01 +02:00
|
|
|
}
|
|
|
|
|
} else {
|
|
|
|
|
/* The device was initially disconnected, deactivate any existing connection */
|
|
|
|
|
_LOGD ("rollback: disconnecting device");
|
|
|
|
|
|
|
|
|
|
if ( nm_device_get_state (device) > NM_DEVICE_STATE_DISCONNECTED
|
|
|
|
|
&& nm_device_get_state (device) < NM_DEVICE_STATE_DEACTIVATING) {
|
|
|
|
|
nm_device_state_changed (device,
|
|
|
|
|
NM_DEVICE_STATE_DEACTIVATING,
|
|
|
|
|
NM_DEVICE_STATE_REASON_USER_REQUESTED);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
next_dev:
|
|
|
|
|
g_variant_builder_add (&builder, "{su}", dev_checkpoint->original_dev_path, result);
|
|
|
|
|
}
|
|
|
|
|
|
2016-09-23 09:37:42 +02:00
|
|
|
if (NM_FLAGS_HAS (priv->flags, NM_CHECKPOINT_CREATE_FLAG_DELETE_NEW_CONNECTIONS)) {
|
|
|
|
|
NMSettingsConnection *con;
|
2017-02-03 14:15:16 +01:00
|
|
|
gs_free NMSettingsConnection **list = NULL;
|
2016-09-23 09:37:42 +02:00
|
|
|
|
|
|
|
|
g_return_val_if_fail (priv->connection_uuids, NULL);
|
2017-11-22 12:46:58 +01:00
|
|
|
list = nm_settings_get_connections_clone (nm_settings_get (), NULL,
|
|
|
|
|
NULL, NULL,
|
|
|
|
|
nm_settings_connection_cmp_autoconnect_priority_p_with_data, NULL);
|
2016-09-23 09:37:42 +02:00
|
|
|
|
2017-02-03 14:15:16 +01:00
|
|
|
for (i = 0; list[i]; i++) {
|
|
|
|
|
con = list[i];
|
2016-09-23 09:37:42 +02:00
|
|
|
if (!g_hash_table_contains (priv->connection_uuids,
|
|
|
|
|
nm_settings_connection_get_uuid (con))) {
|
2017-03-05 01:00:52 +01:00
|
|
|
_LOGD ("rollback: deleting new connection %s",
|
|
|
|
|
nm_settings_connection_get_uuid (con));
|
2017-10-19 11:22:14 +02:00
|
|
|
nm_settings_connection_delete (con, NULL);
|
2016-09-23 09:37:42 +02:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if (NM_FLAGS_HAS (priv->flags, NM_CHECKPOINT_CREATE_FLAG_DISCONNECT_NEW_DEVICES)) {
|
2018-03-29 08:52:45 +02:00
|
|
|
const CList *tmp_lst;
|
2016-09-23 09:37:42 +02:00
|
|
|
NMDeviceState state;
|
core: track devices in manager via embedded CList
Instead of using a GSList for tracking the devices, use a CList.
I think a CList is in most cases the more suitable data structure
then GSList:
- you can find out in O(1) whether the object is linked. That
is nice, for example to assert in NMDevice's destructor that
the object was unlinked, and we will use that later in
nm_manager_get_device_by_path().
- you can unlink the element in O(1) and you can unlink the
element without having access to the link's head
- Contrary to GSList, this does not require an extra slice
allocation for the link node. It quite possibliy consumes
slightly less memory because the CList structure is embedded
in a struct that we already allocate. Even if slice allocation
would be perfect to only consume 2*sizeof(gpointer) for the link
note, it would at most be as-good as CList. Quite possibly,
there is an overhead though.
- CList possibly has better memory locality, because the link
structure and the data are close to each other.
Something which could be seen as disavantage, is that with CList
one device can only be tracked in one NMManager instance at a time.
But that is fine. There exists only one NMManager instance for now,
and even if we would ever introduce multiple managers, we probably
would not associate one NMDevice instance with multiple managers.
The advantages are arguably not huge, but CList is IMHO clearly the
more suited data structure. No need to stick to a suboptimal data
structure for the job. Refactor it.
2018-03-23 21:51:07 +01:00
|
|
|
|
2018-03-29 08:52:45 +02:00
|
|
|
nm_manager_for_each_device (priv->manager, device, tmp_lst) {
|
core: track devices in manager via embedded CList
Instead of using a GSList for tracking the devices, use a CList.
I think a CList is in most cases the more suitable data structure
then GSList:
- you can find out in O(1) whether the object is linked. That
is nice, for example to assert in NMDevice's destructor that
the object was unlinked, and we will use that later in
nm_manager_get_device_by_path().
- you can unlink the element in O(1) and you can unlink the
element without having access to the link's head
- Contrary to GSList, this does not require an extra slice
allocation for the link node. It quite possibliy consumes
slightly less memory because the CList structure is embedded
in a struct that we already allocate. Even if slice allocation
would be perfect to only consume 2*sizeof(gpointer) for the link
note, it would at most be as-good as CList. Quite possibly,
there is an overhead though.
- CList possibly has better memory locality, because the link
structure and the data are close to each other.
Something which could be seen as disavantage, is that with CList
one device can only be tracked in one NMManager instance at a time.
But that is fine. There exists only one NMManager instance for now,
and even if we would ever introduce multiple managers, we probably
would not associate one NMDevice instance with multiple managers.
The advantages are arguably not huge, but CList is IMHO clearly the
more suited data structure. No need to stick to a suboptimal data
structure for the job. Refactor it.
2018-03-23 21:51:07 +01:00
|
|
|
if (g_hash_table_contains (priv->devices, device))
|
|
|
|
|
continue;
|
|
|
|
|
state = nm_device_get_state (device);
|
|
|
|
|
if ( state > NM_DEVICE_STATE_DISCONNECTED
|
|
|
|
|
&& state < NM_DEVICE_STATE_DEACTIVATING) {
|
|
|
|
|
_LOGD ("rollback: disconnecting new device %s", nm_device_get_iface (device));
|
|
|
|
|
nm_device_state_changed (device,
|
|
|
|
|
NM_DEVICE_STATE_DEACTIVATING,
|
|
|
|
|
NM_DEVICE_STATE_REASON_USER_REQUESTED);
|
2016-09-23 09:37:42 +02:00
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
}
|
|
|
|
|
|
2016-07-01 12:11:01 +02:00
|
|
|
return g_variant_new ("(a{su})", &builder);
|
|
|
|
|
}
|
|
|
|
|
|
2019-03-27 16:55:00 +01:00
|
|
|
static void
|
|
|
|
|
device_checkpoint_destroy (gpointer data)
|
|
|
|
|
{
|
|
|
|
|
DeviceCheckpoint *dev_checkpoint = data;
|
|
|
|
|
|
|
|
|
|
nm_clear_g_signal_handler (dev_checkpoint->device, &dev_checkpoint->dev_exported_change_id);
|
|
|
|
|
g_clear_object (&dev_checkpoint->applied_connection);
|
|
|
|
|
g_clear_object (&dev_checkpoint->settings_connection);
|
|
|
|
|
g_clear_object (&dev_checkpoint->device);
|
|
|
|
|
g_free (dev_checkpoint->original_dev_path);
|
|
|
|
|
g_free (dev_checkpoint->original_dev_name);
|
|
|
|
|
|
|
|
|
|
g_slice_free (DeviceCheckpoint, dev_checkpoint);
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
static void
|
|
|
|
|
_move_dev_to_removed_devices (NMDevice *device,
|
|
|
|
|
NMCheckpoint *checkpoint)
|
|
|
|
|
{
|
|
|
|
|
NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE (checkpoint);
|
|
|
|
|
DeviceCheckpoint *dev_checkpoint;
|
|
|
|
|
|
|
|
|
|
g_return_if_fail (device);
|
|
|
|
|
|
|
|
|
|
dev_checkpoint = g_hash_table_lookup (priv->devices, device);
|
|
|
|
|
if (!dev_checkpoint)
|
|
|
|
|
return;
|
|
|
|
|
|
|
|
|
|
g_hash_table_steal (priv->devices, dev_checkpoint->device);
|
|
|
|
|
nm_clear_g_signal_handler (dev_checkpoint->device,
|
|
|
|
|
&dev_checkpoint->dev_exported_change_id);
|
|
|
|
|
g_clear_object (&dev_checkpoint->device);
|
|
|
|
|
|
|
|
|
|
if (!priv->removed_devices)
|
|
|
|
|
priv->removed_devices = g_ptr_array_new_with_free_func ((GDestroyNotify) device_checkpoint_destroy);
|
|
|
|
|
g_ptr_array_add (priv->removed_devices, dev_checkpoint);
|
|
|
|
|
|
|
|
|
|
_notify (checkpoint, PROP_DEVICES);
|
|
|
|
|
}
|
|
|
|
|
|
2019-03-13 11:21:02 +01:00
|
|
|
static void
|
|
|
|
|
_dev_exported_changed (NMDBusObject *obj,
|
|
|
|
|
NMCheckpoint *checkpoint)
|
|
|
|
|
{
|
2019-03-27 16:55:00 +01:00
|
|
|
|
|
|
|
|
_move_dev_to_removed_devices (NM_DEVICE (obj), checkpoint);
|
2019-03-13 11:21:02 +01:00
|
|
|
}
|
|
|
|
|
|
2016-07-01 12:11:01 +02:00
|
|
|
static DeviceCheckpoint *
|
2019-03-13 11:21:02 +01:00
|
|
|
device_checkpoint_create (NMCheckpoint *checkpoint, NMDevice *device)
|
2016-07-01 12:11:01 +02:00
|
|
|
{
|
|
|
|
|
DeviceCheckpoint *dev_checkpoint;
|
2016-09-07 17:47:28 +02:00
|
|
|
NMConnection *applied_connection;
|
|
|
|
|
NMSettingsConnection *settings_connection;
|
2016-07-01 12:11:01 +02:00
|
|
|
const char *path;
|
2017-03-05 01:00:52 +01:00
|
|
|
NMActRequest *act_request;
|
2016-07-01 12:11:01 +02:00
|
|
|
|
2018-03-27 19:26:11 +02:00
|
|
|
nm_assert (NM_IS_DEVICE (device));
|
2018-03-27 19:02:15 +02:00
|
|
|
nm_assert (nm_device_is_real (device));
|
2018-03-27 19:26:11 +02:00
|
|
|
|
core/dbus: rework D-Bus implementation to use lower layer GDBusConnection API
Previously, we used the generated GDBusInterfaceSkeleton types and glued
them via the NMExportedObject base class to our NM types. We also used
GDBusObjectManagerServer.
Don't do that anymore. The resulting code was more complicated despite (or
because?) using generated classes. It was hard to understand, complex, had
ordering-issues, and had a runtime and memory overhead.
This patch refactors this entirely and uses the lower layer API GDBusConnection
directly. It replaces the generated code, GDBusInterfaceSkeleton, and
GDBusObjectManagerServer. All this is now done by NMDbusObject and NMDBusManager
and static descriptor instances of type GDBusInterfaceInfo.
This adds a net plus of more then 1300 lines of hand written code. I claim
that this implementation is easier to understand. Note that previously we
also required extensive and complex glue code to bind our objects to the
generated skeleton objects. Instead, now glue our objects directly to
GDBusConnection. The result is more immediate and gets rid of layers of
code in between.
Now that the D-Bus glue us more under our control, we can address issus and
bottlenecks better, instead of adding code to bend the generated skeletons
to our needs.
Note that the current implementation now only supports one D-Bus connection.
That was effectively the case already, although there were places (and still are)
where the code pretends it could also support connections from a private socket.
We dropped private socket support mainly because it was unused, untested and
buggy, but also because GDBusObjectManagerServer could not export the same
objects on multiple connections. Now, it would be rather straight forward to
fix that and re-introduce ObjectManager on each private connection. But this
commit doesn't do that yet, and the new code intentionally supports only one
D-Bus connection.
Also, the D-Bus startup was simplified. There is no retry, either nm_dbus_manager_start()
succeeds, or it detects the initrd case. In the initrd case, bus manager never tries to
connect to D-Bus. Since the initrd scenario is not yet used/tested, this is good enough
for the moment. It could be easily extended later, for example with polling whether the
system bus appears (like was done previously). Also, restart of D-Bus daemon isn't
supported either -- just like before.
Note how NMDBusManager now implements the ObjectManager D-Bus interface
directly.
Also, this fixes race issues in the server, by no longer delaying
PropertiesChanged signals. NMExportedObject would collect changed
properties and send the signal out in idle_emit_properties_changed()
on idle. This messes up the ordering of change events w.r.t. other
signals and events on the bus. Note that not only NMExportedObject
messed up the ordering. Also the generated code would hook into
notify() and process change events in and idle handle, exhibiting the
same ordering issue too.
No longer do that. PropertiesChanged signals will be sent right away
by hooking into dispatch_properties_changed(). This means, changing
a property in quick succession will no longer be combined and is
guaranteed to emit signals for each individual state. Quite possibly
we emit now more PropertiesChanged signals then before.
However, we are now able to group a set of changes by using standard
g_object_freeze_notify()/g_object_thaw_notify(). We probably should
make more use of that.
Also, now that our signals are all handled in the right order, we
might find places where we still emit them in the wrong order. But that
is then due to the order in which our GObjects emit signals, not due
to an ill behavior of the D-Bus glue. Possibly we need to identify
such ordering issues and fix them.
Numbers (for contrib/rpm --without debug on x86_64):
- the patch changes the code size of NetworkManager by
- 2809360 bytes
+ 2537528 bytes (-9.7%)
- Runtime measurements are harder because there is a large variance
during testing. In other words, the numbers are not reproducible.
Currently, the implementation performs no caching of GVariants at all,
but it would be rather simple to add it, if that turns out to be
useful.
Anyway, without strong claim, it seems that the new form tends to
perform slightly better. That would be no surprise.
$ time (for i in {1..1000}; do nmcli >/dev/null || break; echo -n .; done)
- real 1m39.355s
+ real 1m37.432s
$ time (for i in {1..2000}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects > /dev/null || break; echo -n .; done)
- real 0m26.843s
+ real 0m25.281s
- Regarding RSS size, just looking at the processes in similar
conditions, doesn't give a large difference. On my system they
consume about 19MB RSS. It seems that the new version has a
slightly smaller RSS size.
- 19356 RSS
+ 18660 RSS
2018-02-26 13:51:52 +01:00
|
|
|
path = nm_dbus_object_get_path (NM_DBUS_OBJECT (device));
|
2016-07-01 12:11:01 +02:00
|
|
|
|
|
|
|
|
dev_checkpoint = g_slice_new0 (DeviceCheckpoint);
|
|
|
|
|
dev_checkpoint->device = g_object_ref (device);
|
|
|
|
|
dev_checkpoint->original_dev_path = g_strdup (path);
|
2019-03-15 18:56:23 +01:00
|
|
|
dev_checkpoint->original_dev_name = g_strdup (nm_device_get_iface (device));
|
2019-03-27 16:55:00 +01:00
|
|
|
dev_checkpoint->dev_type = nm_device_get_device_type (device);
|
2016-09-07 17:47:17 +02:00
|
|
|
dev_checkpoint->state = nm_device_get_state (device);
|
2019-03-15 18:56:23 +01:00
|
|
|
dev_checkpoint->is_software = nm_device_is_software (device);
|
2016-09-07 17:47:17 +02:00
|
|
|
dev_checkpoint->realized = nm_device_is_real (device);
|
2019-03-13 11:21:02 +01:00
|
|
|
dev_checkpoint->dev_exported_change_id = g_signal_connect (device,
|
|
|
|
|
NM_DBUS_OBJECT_EXPORTED_CHANGED,
|
|
|
|
|
G_CALLBACK (_dev_exported_changed),
|
|
|
|
|
checkpoint);
|
2017-07-05 13:47:32 +02:00
|
|
|
|
|
|
|
|
if (nm_device_get_unmanaged_mask (device, NM_UNMANAGED_USER_EXPLICIT)) {
|
2018-03-29 08:30:26 +02:00
|
|
|
dev_checkpoint->unmanaged_explicit = !!nm_device_get_unmanaged_flags (device,
|
|
|
|
|
NM_UNMANAGED_USER_EXPLICIT);
|
2017-07-05 13:47:32 +02:00
|
|
|
} else
|
|
|
|
|
dev_checkpoint->unmanaged_explicit = NM_UNMAN_FLAG_OP_FORGET;
|
2016-07-01 12:11:01 +02:00
|
|
|
|
2018-03-29 08:30:26 +02:00
|
|
|
act_request = nm_device_get_act_request (device);
|
|
|
|
|
if (act_request) {
|
|
|
|
|
settings_connection = nm_act_request_get_settings_connection (act_request);
|
|
|
|
|
applied_connection = nm_act_request_get_applied_connection (act_request);
|
2016-09-07 17:47:28 +02:00
|
|
|
|
2018-03-29 08:30:26 +02:00
|
|
|
dev_checkpoint->applied_connection = nm_simple_connection_new_clone (applied_connection);
|
settings: use delegation instead of inheritance for NMSettingsConnection and NMConnection
NMConnection is an interface, which is implemented by the types
NMSimpleConnection (libnm-core), NMSettingsConnection (src) and
NMRemoteConnection (libnm).
NMSettingsConnection does a lot of things already:
1) it "is-a" NMDBusObject and exports the API of a connection profile
on D-Bus
2) it interacts with NMSettings and contains functionality
for tracking the profiles.
3) it is the base-class of types like NMSKeyfileConnection and
NMIfcfgConnection. These handle how the profile is persisted
on disk.
4) it implements NMConnection interface, to itself track the
settings of the profile.
3) and 4) would be better implemented via delegation than inheritance.
Address 4) and don't let NMSettingsConnection implemente the NMConnection
interface. Instead, a settings-connection references now a NMSimpleConnection
instance, to which it delegates for keeping the actual profiles.
Advantages:
- by delegating, there is a clearer separation of what
NMSettingsConnection does. For example, in C we often required
casts from NMSettingsConnection to NMConnection. NMConnection
is a very trivial object with very little logic. When we have
a NMConnection instance at hand, it's good to know that it is
*only* that simple instead of also being an entire
NMSettingsConnection instance.
The main purpose of this patch is to simplify the code by separating
the NMConnection from the NMSettingsConnection. We should generally
be aware whether we handle a NMSettingsConnection or a trivial
NMConnection instance. Now, because NMSettingsConnection no longer
"is-a" NMConnection, this distinction is apparent.
- NMConnection is implemented as an interface and we create
NMSimpleConnection instances whenever we need a real instance.
In GLib, interfaces have a performance overhead, that we needlessly
pay all the time. With this change, we no longer require
NMConnection to be an interface. Thus, in the future we could compile
a version of libnm-core for the daemon, where NMConnection is not an
interface but a GObject implementation akin to NMSimpleConnection.
- In the previous implementation, we cannot treat NMConnection immutable
and copy-on-write.
For example, when NMDevice needs a snapshot of the activated
profile as applied-connection, all it can do is clone the entire
NMSettingsConnection as a NMSimpleConnection.
Likewise, when we get a NMConnection instance and want to keep
a reference to it, we cannot do that, because we never know
who also references and modifies the instance.
By separating NMSettingsConnection we could in the future have
NMConnection immutable and copy-on-write, to avoid all unnecessary
clones.
2018-08-11 11:08:17 +02:00
|
|
|
dev_checkpoint->settings_connection = nm_simple_connection_new_clone (nm_settings_connection_get_connection (settings_connection));
|
|
|
|
|
dev_checkpoint->ac_version_id = nm_active_connection_version_id_get (NM_ACTIVE_CONNECTION (act_request));
|
|
|
|
|
dev_checkpoint->activation_reason = nm_active_connection_get_activation_reason (NM_ACTIVE_CONNECTION (act_request));
|
core: improve and fix keeping connection active based on "connection.permissions"
By setting "connection.permissions", a profile is restricted to a
particular user.
That means for example, that another user cannot see, modify, delete,
activate or deactivate the profile. It also means, that the profile
will only autoconnect when the user is logged in (has a session).
Note that root is always able to activate the profile. Likewise, the
user is also allowed to manually activate the own profile, even if no
session currently exists (which can easily happen with `sudo`).
When the user logs out (the session goes away), we want do disconnect
the profile, however there are conflicting goals here:
1) if the profile was activate by root user, then logging out the user
should not disconnect the profile. The patch fixes that by not
binding the activation to the connection, if the activation is done
by the root user.
2) if the profile was activated by the owner when it had no session,
then it should stay alive until the user logs in (once) and logs
out again. This is already handled by the previous commit.
Yes, this point is odd. If you first do
$ sudo -u $OTHER_USER nmcli connection up $PROFILE
the profile activates despite not having a session. If you then
$ ssh guest@localhost nmcli device
you'll still see the profile active. However, the moment the SSH session
ends, a session closes and the profile disconnects. It's unclear, how to
solve that any better. I think, a user who cares about this, should not
activate the profile without having a session in the first place.
There are quite some special cases, in particular with internal
activations. In those cases we need to decide whether to bind the
activation to the profile's visibility.
Also, expose the "bind" setting in the D-Bus API. Note, that in the future
this flag may be modified via D-Bus API. Like we may also add related API
that allows to tweak the lifetime of the activation.
Also, I think we broke handling of connection visiblity with 37e8c53eeed
"core: Introduce helper class to track connection keep alive". This
should be fixed now too, with improved behavior.
Fixes: 37e8c53eeed579fe34a68819cd12f3295d581394
https://bugzilla.redhat.com/show_bug.cgi?id=1530977
2018-11-21 13:30:16 +01:00
|
|
|
dev_checkpoint->activation_lifetime_bound_to_profile_visiblity = NM_FLAGS_HAS (nm_active_connection_get_state_flags (NM_ACTIVE_CONNECTION (act_request)),
|
|
|
|
|
NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY);
|
2016-09-07 17:47:28 +02:00
|
|
|
}
|
2016-07-01 12:11:01 +02:00
|
|
|
|
|
|
|
|
return dev_checkpoint;
|
|
|
|
|
}
|
|
|
|
|
|
2018-03-27 19:02:15 +02:00
|
|
|
static gboolean
|
|
|
|
|
_timeout_cb (gpointer user_data)
|
|
|
|
|
{
|
|
|
|
|
NMCheckpoint *self = user_data;
|
|
|
|
|
NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE (self);
|
|
|
|
|
|
|
|
|
|
priv->timeout_id = 0;
|
|
|
|
|
|
|
|
|
|
if (priv->timeout_cb)
|
|
|
|
|
priv->timeout_cb (self, priv->timeout_data);
|
|
|
|
|
|
|
|
|
|
/* beware, @self likely got destroyed! */
|
|
|
|
|
return G_SOURCE_REMOVE;
|
|
|
|
|
}
|
|
|
|
|
|
checkpoint: allow resetting the rollback timeout via D-Bus
This allows to adjust the timeout of an existing checkpoint.
The main usecase of checkpoints, is to have a fail-safe when
configuring the network remotely. By allowing to reset the timeout,
the user can perform a series of actions, and keep bumping the
timeout. That way, the entire series is still guarded by the same
checkpoint, but the user can start with short timeout, and
re-adjust the timeout as he goes along.
The libnm API only implements the async form (at least for now).
Sync methods are fundamentally wrong with D-Bus, and it's probably
not needed. Also, follow glib convenction, where the async form
doesn't have the _async name suffix. Also, accept a D-Bus path
as argument, not a NMCheckpoint instance. The libnm API should
not be more restricted than the underlying D-Bus API. It would
be cumbersome to require the user to lookup the NMCheckpoint
instance first, especially since libnm doesn't provide an efficient
or convenient lookup-by-path method. On the other hand, retrieving
the path from a NMCheckpoint instance is always possible.
2018-03-28 08:09:56 +02:00
|
|
|
void
|
|
|
|
|
nm_checkpoint_adjust_rollback_timeout (NMCheckpoint *self, guint32 add_timeout)
|
|
|
|
|
{
|
|
|
|
|
guint32 rollback_timeout_s;
|
|
|
|
|
gint64 now_ms, add_timeout_ms, rollback_timeout_ms;
|
|
|
|
|
|
|
|
|
|
NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE (self);
|
|
|
|
|
|
|
|
|
|
nm_clear_g_source (&priv->timeout_id);
|
|
|
|
|
|
|
|
|
|
if (add_timeout == 0)
|
|
|
|
|
rollback_timeout_s = 0;
|
|
|
|
|
else {
|
|
|
|
|
now_ms = nm_utils_get_monotonic_timestamp_ms ();
|
|
|
|
|
add_timeout_ms = ((gint64) add_timeout) * 1000;
|
|
|
|
|
rollback_timeout_ms = (now_ms - priv->created_at_ms) + add_timeout_ms;
|
|
|
|
|
|
|
|
|
|
/* round to nearest integer second. Since NM_CHECKPOINT_ROLLBACK_TIMEOUT is
|
|
|
|
|
* in units seconds, it will be able to exactly express the timeout. */
|
|
|
|
|
rollback_timeout_s = NM_MIN ((rollback_timeout_ms + 500) / 1000, (gint64) G_MAXUINT32);
|
|
|
|
|
|
|
|
|
|
/* we expect the timeout to be positive, because add_timeout_ms is positive.
|
|
|
|
|
* We cannot accept a zero, because it means "infinity". */
|
|
|
|
|
nm_assert (rollback_timeout_s > 0);
|
|
|
|
|
|
|
|
|
|
priv->timeout_id = g_timeout_add (NM_MIN (add_timeout_ms, (gint64) G_MAXUINT32),
|
|
|
|
|
_timeout_cb,
|
|
|
|
|
self);
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
if (rollback_timeout_s != priv->rollback_timeout_s) {
|
|
|
|
|
priv->rollback_timeout_s = rollback_timeout_s;
|
|
|
|
|
_notify (self, PROP_ROLLBACK_TIMEOUT);
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
2016-09-29 13:49:01 +02:00
|
|
|
/*****************************************************************************/
|
|
|
|
|
|
|
|
|
|
static void
|
|
|
|
|
get_property (GObject *object, guint prop_id,
|
|
|
|
|
GValue *value, GParamSpec *pspec)
|
|
|
|
|
{
|
|
|
|
|
NMCheckpoint *self = NM_CHECKPOINT (object);
|
|
|
|
|
NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE (self);
|
|
|
|
|
|
|
|
|
|
switch (prop_id) {
|
|
|
|
|
case PROP_DEVICES:
|
2018-03-29 08:18:59 +02:00
|
|
|
nm_dbus_utils_g_value_set_object_path_from_hash (value,
|
|
|
|
|
priv->devices,
|
|
|
|
|
FALSE);
|
2016-09-29 13:49:01 +02:00
|
|
|
break;
|
|
|
|
|
case PROP_CREATED:
|
2018-03-27 19:02:15 +02:00
|
|
|
g_value_set_int64 (value,
|
|
|
|
|
nm_utils_monotonic_timestamp_as_boottime (priv->created_at_ms,
|
|
|
|
|
NM_UTILS_NS_PER_MSEC));
|
2016-09-29 13:49:01 +02:00
|
|
|
break;
|
|
|
|
|
case PROP_ROLLBACK_TIMEOUT:
|
2018-03-27 19:02:15 +02:00
|
|
|
g_value_set_uint (value, priv->rollback_timeout_s);
|
2016-09-29 13:49:01 +02:00
|
|
|
break;
|
|
|
|
|
default:
|
|
|
|
|
G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
|
|
|
|
|
break;
|
|
|
|
|
}
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
/*****************************************************************************/
|
|
|
|
|
|
2016-07-01 12:11:01 +02:00
|
|
|
static void
|
|
|
|
|
nm_checkpoint_init (NMCheckpoint *self)
|
|
|
|
|
{
|
2018-03-27 12:45:23 +02:00
|
|
|
NMCheckpointPrivate *priv;
|
|
|
|
|
|
|
|
|
|
priv = G_TYPE_INSTANCE_GET_PRIVATE (self, NM_TYPE_CHECKPOINT, NMCheckpointPrivate);
|
|
|
|
|
|
|
|
|
|
self->_priv = priv;
|
|
|
|
|
|
|
|
|
|
c_list_init (&self->checkpoints_lst);
|
2016-07-01 12:11:01 +02:00
|
|
|
|
2017-11-15 16:06:43 +01:00
|
|
|
priv->devices = g_hash_table_new_full (nm_direct_hash, NULL,
|
2016-07-01 12:11:01 +02:00
|
|
|
NULL, device_checkpoint_destroy);
|
|
|
|
|
}
|
|
|
|
|
|
2019-03-27 16:55:00 +01:00
|
|
|
static void
|
|
|
|
|
_device_removed (NMManager *manager, NMDevice *device, gpointer user_data)
|
|
|
|
|
{
|
|
|
|
|
_move_dev_to_removed_devices (device, NM_CHECKPOINT (user_data));
|
|
|
|
|
}
|
|
|
|
|
|
2016-07-01 12:11:01 +02:00
|
|
|
NMCheckpoint *
|
2018-03-27 19:02:15 +02:00
|
|
|
nm_checkpoint_new (NMManager *manager, GPtrArray *devices, guint32 rollback_timeout_s,
|
2018-03-28 07:08:54 +02:00
|
|
|
NMCheckpointCreateFlags flags)
|
2016-07-01 12:11:01 +02:00
|
|
|
{
|
|
|
|
|
NMCheckpoint *self;
|
|
|
|
|
NMCheckpointPrivate *priv;
|
2016-09-23 09:37:42 +02:00
|
|
|
NMSettingsConnection *const *con;
|
2018-03-27 19:02:15 +02:00
|
|
|
gint64 rollback_timeout_ms;
|
2016-07-01 12:11:01 +02:00
|
|
|
guint i;
|
|
|
|
|
|
|
|
|
|
g_return_val_if_fail (manager, NULL);
|
|
|
|
|
g_return_val_if_fail (devices, NULL);
|
2018-03-28 07:08:54 +02:00
|
|
|
g_return_val_if_fail (devices->len > 0, NULL);
|
2016-07-01 12:11:01 +02:00
|
|
|
|
|
|
|
|
self = g_object_new (NM_TYPE_CHECKPOINT, NULL);
|
|
|
|
|
|
|
|
|
|
priv = NM_CHECKPOINT_GET_PRIVATE (self);
|
|
|
|
|
priv->manager = manager;
|
2018-03-27 19:02:15 +02:00
|
|
|
priv->rollback_timeout_s = rollback_timeout_s;
|
|
|
|
|
priv->created_at_ms = nm_utils_get_monotonic_timestamp_ms ();
|
2016-09-23 09:37:42 +02:00
|
|
|
priv->flags = flags;
|
|
|
|
|
|
2018-03-27 19:02:15 +02:00
|
|
|
if (rollback_timeout_s != 0) {
|
|
|
|
|
rollback_timeout_ms = ((gint64) rollback_timeout_s) * 1000;
|
|
|
|
|
priv->timeout_id = g_timeout_add (NM_MIN (rollback_timeout_ms, (gint64) G_MAXUINT32),
|
|
|
|
|
_timeout_cb,
|
|
|
|
|
self);
|
|
|
|
|
}
|
|
|
|
|
|
2016-09-23 09:37:42 +02:00
|
|
|
if (NM_FLAGS_HAS (flags, NM_CHECKPOINT_CREATE_FLAG_DELETE_NEW_CONNECTIONS)) {
|
2017-10-13 16:12:35 +02:00
|
|
|
priv->connection_uuids = g_hash_table_new_full (nm_str_hash, g_str_equal, g_free, NULL);
|
2016-09-23 09:37:42 +02:00
|
|
|
for (con = nm_settings_get_connections (nm_settings_get (), NULL); *con; con++) {
|
|
|
|
|
g_hash_table_add (priv->connection_uuids,
|
|
|
|
|
g_strdup (nm_settings_connection_get_uuid (*con)));
|
|
|
|
|
}
|
|
|
|
|
}
|
2016-07-01 12:11:01 +02:00
|
|
|
|
|
|
|
|
for (i = 0; i < devices->len; i++) {
|
2018-03-27 19:26:11 +02:00
|
|
|
NMDevice *device = devices->pdata[i];
|
|
|
|
|
|
2019-03-13 11:21:02 +01:00
|
|
|
/* As long as the check point instance exists, it will keep a reference
|
|
|
|
|
* to the device also if the device gets removed (by rmmod or by deleting
|
|
|
|
|
* a connection profile for a software device). */
|
2018-03-27 19:26:11 +02:00
|
|
|
g_hash_table_insert (priv->devices,
|
|
|
|
|
device,
|
2019-03-13 11:21:02 +01:00
|
|
|
device_checkpoint_create (self, device));
|
2016-07-01 12:11:01 +02:00
|
|
|
}
|
|
|
|
|
|
2019-03-27 16:55:00 +01:00
|
|
|
priv->dev_removed_id = g_signal_connect (priv->manager,
|
|
|
|
|
NM_MANAGER_DEVICE_REMOVED,
|
|
|
|
|
G_CALLBACK (_device_removed),
|
|
|
|
|
self);
|
2016-07-01 12:11:01 +02:00
|
|
|
return self;
|
|
|
|
|
}
|
|
|
|
|
|
|
|
|
|
static void
|
|
|
|
|
dispose (GObject *object)
|
|
|
|
|
{
|
|
|
|
|
NMCheckpoint *self = NM_CHECKPOINT (object);
|
|
|
|
|
NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE (self);
|
|
|
|
|
|
2018-03-27 12:45:23 +02:00
|
|
|
nm_assert (c_list_is_empty (&self->checkpoints_lst));
|
|
|
|
|
|
2016-07-01 12:11:01 +02:00
|
|
|
g_clear_pointer (&priv->devices, g_hash_table_unref);
|
2016-09-23 09:37:42 +02:00
|
|
|
g_clear_pointer (&priv->connection_uuids, g_hash_table_unref);
|
2019-03-27 16:55:00 +01:00
|
|
|
nm_clear_pointer (&priv->removed_devices, g_ptr_array_unref);
|
|
|
|
|
|
|
|
|
|
nm_clear_g_signal_handler (priv->manager, &priv->dev_removed_id);
|
2016-07-01 12:11:01 +02:00
|
|
|
|
2018-03-27 19:02:15 +02:00
|
|
|
nm_clear_g_source (&priv->timeout_id);
|
|
|
|
|
|
2016-07-01 12:11:01 +02:00
|
|
|
G_OBJECT_CLASS (nm_checkpoint_parent_class)->dispose (object);
|
|
|
|
|
}
|
|
|
|
|
|
core/dbus: rework D-Bus implementation to use lower layer GDBusConnection API
Previously, we used the generated GDBusInterfaceSkeleton types and glued
them via the NMExportedObject base class to our NM types. We also used
GDBusObjectManagerServer.
Don't do that anymore. The resulting code was more complicated despite (or
because?) using generated classes. It was hard to understand, complex, had
ordering-issues, and had a runtime and memory overhead.
This patch refactors this entirely and uses the lower layer API GDBusConnection
directly. It replaces the generated code, GDBusInterfaceSkeleton, and
GDBusObjectManagerServer. All this is now done by NMDbusObject and NMDBusManager
and static descriptor instances of type GDBusInterfaceInfo.
This adds a net plus of more then 1300 lines of hand written code. I claim
that this implementation is easier to understand. Note that previously we
also required extensive and complex glue code to bind our objects to the
generated skeleton objects. Instead, now glue our objects directly to
GDBusConnection. The result is more immediate and gets rid of layers of
code in between.
Now that the D-Bus glue us more under our control, we can address issus and
bottlenecks better, instead of adding code to bend the generated skeletons
to our needs.
Note that the current implementation now only supports one D-Bus connection.
That was effectively the case already, although there were places (and still are)
where the code pretends it could also support connections from a private socket.
We dropped private socket support mainly because it was unused, untested and
buggy, but also because GDBusObjectManagerServer could not export the same
objects on multiple connections. Now, it would be rather straight forward to
fix that and re-introduce ObjectManager on each private connection. But this
commit doesn't do that yet, and the new code intentionally supports only one
D-Bus connection.
Also, the D-Bus startup was simplified. There is no retry, either nm_dbus_manager_start()
succeeds, or it detects the initrd case. In the initrd case, bus manager never tries to
connect to D-Bus. Since the initrd scenario is not yet used/tested, this is good enough
for the moment. It could be easily extended later, for example with polling whether the
system bus appears (like was done previously). Also, restart of D-Bus daemon isn't
supported either -- just like before.
Note how NMDBusManager now implements the ObjectManager D-Bus interface
directly.
Also, this fixes race issues in the server, by no longer delaying
PropertiesChanged signals. NMExportedObject would collect changed
properties and send the signal out in idle_emit_properties_changed()
on idle. This messes up the ordering of change events w.r.t. other
signals and events on the bus. Note that not only NMExportedObject
messed up the ordering. Also the generated code would hook into
notify() and process change events in and idle handle, exhibiting the
same ordering issue too.
No longer do that. PropertiesChanged signals will be sent right away
by hooking into dispatch_properties_changed(). This means, changing
a property in quick succession will no longer be combined and is
guaranteed to emit signals for each individual state. Quite possibly
we emit now more PropertiesChanged signals then before.
However, we are now able to group a set of changes by using standard
g_object_freeze_notify()/g_object_thaw_notify(). We probably should
make more use of that.
Also, now that our signals are all handled in the right order, we
might find places where we still emit them in the wrong order. But that
is then due to the order in which our GObjects emit signals, not due
to an ill behavior of the D-Bus glue. Possibly we need to identify
such ordering issues and fix them.
Numbers (for contrib/rpm --without debug on x86_64):
- the patch changes the code size of NetworkManager by
- 2809360 bytes
+ 2537528 bytes (-9.7%)
- Runtime measurements are harder because there is a large variance
during testing. In other words, the numbers are not reproducible.
Currently, the implementation performs no caching of GVariants at all,
but it would be rather simple to add it, if that turns out to be
useful.
Anyway, without strong claim, it seems that the new form tends to
perform slightly better. That would be no surprise.
$ time (for i in {1..1000}; do nmcli >/dev/null || break; echo -n .; done)
- real 1m39.355s
+ real 1m37.432s
$ time (for i in {1..2000}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects > /dev/null || break; echo -n .; done)
- real 0m26.843s
+ real 0m25.281s
- Regarding RSS size, just looking at the processes in similar
conditions, doesn't give a large difference. On my system they
consume about 19MB RSS. It seems that the new version has a
slightly smaller RSS size.
- 19356 RSS
+ 18660 RSS
2018-02-26 13:51:52 +01:00
|
|
|
static const NMDBusInterfaceInfoExtended interface_info_checkpoint = {
|
|
|
|
|
.parent = NM_DEFINE_GDBUS_INTERFACE_INFO_INIT (
|
|
|
|
|
NM_DBUS_INTERFACE_CHECKPOINT,
|
|
|
|
|
.signals = NM_DEFINE_GDBUS_SIGNAL_INFOS (
|
|
|
|
|
&nm_signal_info_property_changed_legacy,
|
|
|
|
|
),
|
|
|
|
|
.properties = NM_DEFINE_GDBUS_PROPERTY_INFOS (
|
|
|
|
|
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Devices", "ao", NM_CHECKPOINT_DEVICES),
|
|
|
|
|
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("Created", "x", NM_CHECKPOINT_CREATED),
|
|
|
|
|
NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE_L ("RollbackTimeout", "u", NM_CHECKPOINT_ROLLBACK_TIMEOUT),
|
|
|
|
|
),
|
|
|
|
|
),
|
|
|
|
|
.legacy_property_changed = TRUE,
|
|
|
|
|
};
|
|
|
|
|
|
2016-07-01 12:11:01 +02:00
|
|
|
static void
|
|
|
|
|
nm_checkpoint_class_init (NMCheckpointClass *checkpoint_class)
|
|
|
|
|
{
|
|
|
|
|
GObjectClass *object_class = G_OBJECT_CLASS (checkpoint_class);
|
core/dbus: rework D-Bus implementation to use lower layer GDBusConnection API
Previously, we used the generated GDBusInterfaceSkeleton types and glued
them via the NMExportedObject base class to our NM types. We also used
GDBusObjectManagerServer.
Don't do that anymore. The resulting code was more complicated despite (or
because?) using generated classes. It was hard to understand, complex, had
ordering-issues, and had a runtime and memory overhead.
This patch refactors this entirely and uses the lower layer API GDBusConnection
directly. It replaces the generated code, GDBusInterfaceSkeleton, and
GDBusObjectManagerServer. All this is now done by NMDbusObject and NMDBusManager
and static descriptor instances of type GDBusInterfaceInfo.
This adds a net plus of more then 1300 lines of hand written code. I claim
that this implementation is easier to understand. Note that previously we
also required extensive and complex glue code to bind our objects to the
generated skeleton objects. Instead, now glue our objects directly to
GDBusConnection. The result is more immediate and gets rid of layers of
code in between.
Now that the D-Bus glue us more under our control, we can address issus and
bottlenecks better, instead of adding code to bend the generated skeletons
to our needs.
Note that the current implementation now only supports one D-Bus connection.
That was effectively the case already, although there were places (and still are)
where the code pretends it could also support connections from a private socket.
We dropped private socket support mainly because it was unused, untested and
buggy, but also because GDBusObjectManagerServer could not export the same
objects on multiple connections. Now, it would be rather straight forward to
fix that and re-introduce ObjectManager on each private connection. But this
commit doesn't do that yet, and the new code intentionally supports only one
D-Bus connection.
Also, the D-Bus startup was simplified. There is no retry, either nm_dbus_manager_start()
succeeds, or it detects the initrd case. In the initrd case, bus manager never tries to
connect to D-Bus. Since the initrd scenario is not yet used/tested, this is good enough
for the moment. It could be easily extended later, for example with polling whether the
system bus appears (like was done previously). Also, restart of D-Bus daemon isn't
supported either -- just like before.
Note how NMDBusManager now implements the ObjectManager D-Bus interface
directly.
Also, this fixes race issues in the server, by no longer delaying
PropertiesChanged signals. NMExportedObject would collect changed
properties and send the signal out in idle_emit_properties_changed()
on idle. This messes up the ordering of change events w.r.t. other
signals and events on the bus. Note that not only NMExportedObject
messed up the ordering. Also the generated code would hook into
notify() and process change events in and idle handle, exhibiting the
same ordering issue too.
No longer do that. PropertiesChanged signals will be sent right away
by hooking into dispatch_properties_changed(). This means, changing
a property in quick succession will no longer be combined and is
guaranteed to emit signals for each individual state. Quite possibly
we emit now more PropertiesChanged signals then before.
However, we are now able to group a set of changes by using standard
g_object_freeze_notify()/g_object_thaw_notify(). We probably should
make more use of that.
Also, now that our signals are all handled in the right order, we
might find places where we still emit them in the wrong order. But that
is then due to the order in which our GObjects emit signals, not due
to an ill behavior of the D-Bus glue. Possibly we need to identify
such ordering issues and fix them.
Numbers (for contrib/rpm --without debug on x86_64):
- the patch changes the code size of NetworkManager by
- 2809360 bytes
+ 2537528 bytes (-9.7%)
- Runtime measurements are harder because there is a large variance
during testing. In other words, the numbers are not reproducible.
Currently, the implementation performs no caching of GVariants at all,
but it would be rather simple to add it, if that turns out to be
useful.
Anyway, without strong claim, it seems that the new form tends to
perform slightly better. That would be no surprise.
$ time (for i in {1..1000}; do nmcli >/dev/null || break; echo -n .; done)
- real 1m39.355s
+ real 1m37.432s
$ time (for i in {1..2000}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects > /dev/null || break; echo -n .; done)
- real 0m26.843s
+ real 0m25.281s
- Regarding RSS size, just looking at the processes in similar
conditions, doesn't give a large difference. On my system they
consume about 19MB RSS. It seems that the new version has a
slightly smaller RSS size.
- 19356 RSS
+ 18660 RSS
2018-02-26 13:51:52 +01:00
|
|
|
NMDBusObjectClass *dbus_object_class = NM_DBUS_OBJECT_CLASS (checkpoint_class);
|
2016-07-01 12:11:01 +02:00
|
|
|
|
2018-03-27 12:45:23 +02:00
|
|
|
g_type_class_add_private (object_class, sizeof (NMCheckpointPrivate));
|
|
|
|
|
|
2018-03-13 10:14:06 +01:00
|
|
|
dbus_object_class->export_path = NM_DBUS_EXPORT_PATH_NUMBERED (NM_DBUS_PATH"/Checkpoint");
|
core/dbus: rework D-Bus implementation to use lower layer GDBusConnection API
Previously, we used the generated GDBusInterfaceSkeleton types and glued
them via the NMExportedObject base class to our NM types. We also used
GDBusObjectManagerServer.
Don't do that anymore. The resulting code was more complicated despite (or
because?) using generated classes. It was hard to understand, complex, had
ordering-issues, and had a runtime and memory overhead.
This patch refactors this entirely and uses the lower layer API GDBusConnection
directly. It replaces the generated code, GDBusInterfaceSkeleton, and
GDBusObjectManagerServer. All this is now done by NMDbusObject and NMDBusManager
and static descriptor instances of type GDBusInterfaceInfo.
This adds a net plus of more then 1300 lines of hand written code. I claim
that this implementation is easier to understand. Note that previously we
also required extensive and complex glue code to bind our objects to the
generated skeleton objects. Instead, now glue our objects directly to
GDBusConnection. The result is more immediate and gets rid of layers of
code in between.
Now that the D-Bus glue us more under our control, we can address issus and
bottlenecks better, instead of adding code to bend the generated skeletons
to our needs.
Note that the current implementation now only supports one D-Bus connection.
That was effectively the case already, although there were places (and still are)
where the code pretends it could also support connections from a private socket.
We dropped private socket support mainly because it was unused, untested and
buggy, but also because GDBusObjectManagerServer could not export the same
objects on multiple connections. Now, it would be rather straight forward to
fix that and re-introduce ObjectManager on each private connection. But this
commit doesn't do that yet, and the new code intentionally supports only one
D-Bus connection.
Also, the D-Bus startup was simplified. There is no retry, either nm_dbus_manager_start()
succeeds, or it detects the initrd case. In the initrd case, bus manager never tries to
connect to D-Bus. Since the initrd scenario is not yet used/tested, this is good enough
for the moment. It could be easily extended later, for example with polling whether the
system bus appears (like was done previously). Also, restart of D-Bus daemon isn't
supported either -- just like before.
Note how NMDBusManager now implements the ObjectManager D-Bus interface
directly.
Also, this fixes race issues in the server, by no longer delaying
PropertiesChanged signals. NMExportedObject would collect changed
properties and send the signal out in idle_emit_properties_changed()
on idle. This messes up the ordering of change events w.r.t. other
signals and events on the bus. Note that not only NMExportedObject
messed up the ordering. Also the generated code would hook into
notify() and process change events in and idle handle, exhibiting the
same ordering issue too.
No longer do that. PropertiesChanged signals will be sent right away
by hooking into dispatch_properties_changed(). This means, changing
a property in quick succession will no longer be combined and is
guaranteed to emit signals for each individual state. Quite possibly
we emit now more PropertiesChanged signals then before.
However, we are now able to group a set of changes by using standard
g_object_freeze_notify()/g_object_thaw_notify(). We probably should
make more use of that.
Also, now that our signals are all handled in the right order, we
might find places where we still emit them in the wrong order. But that
is then due to the order in which our GObjects emit signals, not due
to an ill behavior of the D-Bus glue. Possibly we need to identify
such ordering issues and fix them.
Numbers (for contrib/rpm --without debug on x86_64):
- the patch changes the code size of NetworkManager by
- 2809360 bytes
+ 2537528 bytes (-9.7%)
- Runtime measurements are harder because there is a large variance
during testing. In other words, the numbers are not reproducible.
Currently, the implementation performs no caching of GVariants at all,
but it would be rather simple to add it, if that turns out to be
useful.
Anyway, without strong claim, it seems that the new form tends to
perform slightly better. That would be no surprise.
$ time (for i in {1..1000}; do nmcli >/dev/null || break; echo -n .; done)
- real 1m39.355s
+ real 1m37.432s
$ time (for i in {1..2000}; do busctl call org.freedesktop.NetworkManager /org/freedesktop org.freedesktop.DBus.ObjectManager GetManagedObjects > /dev/null || break; echo -n .; done)
- real 0m26.843s
+ real 0m25.281s
- Regarding RSS size, just looking at the processes in similar
conditions, doesn't give a large difference. On my system they
consume about 19MB RSS. It seems that the new version has a
slightly smaller RSS size.
- 19356 RSS
+ 18660 RSS
2018-02-26 13:51:52 +01:00
|
|
|
dbus_object_class->interface_infos = NM_DBUS_INTERFACE_INFOS (&interface_info_checkpoint);
|
2016-07-01 12:11:01 +02:00
|
|
|
|
|
|
|
|
object_class->dispose = dispose;
|
|
|
|
|
object_class->get_property = get_property;
|
|
|
|
|
|
|
|
|
|
obj_properties[PROP_DEVICES] =
|
|
|
|
|
g_param_spec_boxed (NM_CHECKPOINT_DEVICES, "", "",
|
|
|
|
|
G_TYPE_STRV,
|
|
|
|
|
G_PARAM_READABLE |
|
|
|
|
|
G_PARAM_STATIC_STRINGS);
|
|
|
|
|
|
|
|
|
|
obj_properties[PROP_CREATED] =
|
|
|
|
|
g_param_spec_int64 (NM_CHECKPOINT_CREATED, "", "",
|
|
|
|
|
G_MININT64, G_MAXINT64, 0,
|
|
|
|
|
G_PARAM_READABLE |
|
|
|
|
|
G_PARAM_STATIC_STRINGS);
|
|
|
|
|
|
|
|
|
|
obj_properties[PROP_ROLLBACK_TIMEOUT] =
|
|
|
|
|
g_param_spec_uint (NM_CHECKPOINT_ROLLBACK_TIMEOUT, "", "",
|
|
|
|
|
0, G_MAXUINT32, 0,
|
|
|
|
|
G_PARAM_READABLE |
|
|
|
|
|
G_PARAM_STATIC_STRINGS);
|
|
|
|
|
|
|
|
|
|
g_object_class_install_properties (object_class, _PROPERTY_ENUMS_LAST, obj_properties);
|
|
|
|
|
}
|
