From c90118ff602114f6c86ff916bc036c8d917f93c4 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 8 Nov 2017 18:57:59 +0100 Subject: libnm: don't invert order in array properties The order of elements in array properties was inverted when reconstructing the array. Keep the original order from D-Bus. --- libnm/nm-object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm/nm-object.c b/libnm/nm-object.c index ef3358627e..c344a08dca 100644 --- a/libnm/nm-object.c +++ b/libnm/nm-object.c @@ -572,7 +572,7 @@ object_created (GObject *obj, const char *path, gpointer user_data) object_class->object_creation_failed (odata->self, path); } - odata->objects[--odata->remaining] = obj ? g_object_ref (obj) : NULL; + odata->objects[odata->length - odata->remaining--] = obj ? g_object_ref (obj) : NULL; object_property_maybe_complete (odata->self); } -- cgit v1.2.1 From 66d048023c780f6581a568b46efa29b1fadd7dbc Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Sat, 21 Oct 2017 16:05:09 +0200 Subject: checkpoint: specify path of already existing checkpoint on error --- src/nm-checkpoint-manager.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/nm-checkpoint-manager.c b/src/nm-checkpoint-manager.c index 033c11cc43..46d6a14019 100644 --- a/src/nm-checkpoint-manager.c +++ b/src/nm-checkpoint-manager.c @@ -175,10 +175,12 @@ nm_checkpoint_manager_create (NMCheckpointManager *self, if (!NM_FLAGS_HAS (flags, NM_CHECKPOINT_CREATE_FLAG_DESTROY_ALL)) { for (i = 0; i < devices->len; i++) { device = devices->pdata[i]; - if (find_checkpoint_for_device (self, device)) { + checkpoint = find_checkpoint_for_device (self, device); + if (checkpoint) { g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_INVALID_ARGUMENTS, - "a checkpoint for device '%s' already exists", - nm_device_get_iface (device)); + "device '%s' is already included in checkpoint %s", + nm_device_get_iface (device), + nm_exported_object_get_path (NM_EXPORTED_OBJECT (checkpoint))); return NULL; } } -- cgit v1.2.1 From 974f21eca37934eb035ad214121190d280895cca Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 7 Nov 2017 16:36:33 +0100 Subject: checkpoint: don't include unrealized devices Don't include unrealized devices in checkpoint because, as the name says, they are not real. While at it, remove nm_manager_get_device_paths() as it is no longer used. --- src/nm-checkpoint-manager.c | 18 +++++++++++++++++- src/nm-manager.c | 22 ---------------------- src/nm-manager.h | 1 - 3 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/nm-checkpoint-manager.c b/src/nm-checkpoint-manager.c index 46d6a14019..6da220c4cc 100644 --- a/src/nm-checkpoint-manager.c +++ b/src/nm-checkpoint-manager.c @@ -153,7 +153,23 @@ nm_checkpoint_manager_create (NMCheckpointManager *self, manager = GET_MANAGER (self); if (!device_paths || !device_paths[0]) { - device_paths_free = nm_manager_get_device_paths (manager); + const char *device_path; + const GSList *iter; + GPtrArray *paths; + + paths = g_ptr_array_new (); + for (iter = nm_manager_get_devices (manager); + iter; + iter = g_slist_next (iter)) { + device = NM_DEVICE (iter->data); + if (!nm_device_is_real (device)) + continue; + device_path = nm_exported_object_get_path (NM_EXPORTED_OBJECT (device)); + if (device_path) + g_ptr_array_add (paths, (gpointer) device_path); + } + g_ptr_array_add (paths, NULL); + device_paths_free = (const char **) g_ptr_array_free (paths, FALSE); device_paths = (const char *const *) device_paths_free; } else if (NM_FLAGS_HAS (flags, NM_CHECKPOINT_CREATE_FLAG_DISCONNECT_NEW_DEVICES)) { g_set_error_literal (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_INVALID_ARGUMENTS, diff --git a/src/nm-manager.c b/src/nm-manager.c index 45afc7f131..2766e86aed 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2589,28 +2589,6 @@ nm_manager_get_devices (NMManager *manager) return NM_MANAGER_GET_PRIVATE (manager)->devices; } -const char ** -nm_manager_get_device_paths (NMManager *self) -{ - const GSList *devices, *iter; - GPtrArray *paths; - const char *path; - - g_return_val_if_fail (NM_IS_MANAGER (self), NULL); - devices = NM_MANAGER_GET_PRIVATE (self)->devices; - paths = g_ptr_array_new (); - - for (iter = devices; iter; iter = g_slist_next (iter)) { - path = nm_exported_object_get_path (NM_EXPORTED_OBJECT (iter->data)); - if (path) - g_ptr_array_add (paths, (gpointer) path); - } - - g_ptr_array_add (paths, NULL); - - return (const char **) g_ptr_array_free (paths, FALSE); -} - static NMDevice * nm_manager_get_best_device_for_connection (NMManager *self, NMConnection *connection, diff --git a/src/nm-manager.h b/src/nm-manager.h index 622edb5b54..418c5ea972 100644 --- a/src/nm-manager.h +++ b/src/nm-manager.h @@ -95,7 +95,6 @@ void nm_manager_write_device_state (NMManager *manager); /* Device handling */ const GSList * nm_manager_get_devices (NMManager *manager); -const char ** nm_manager_get_device_paths (NMManager *self); NMDevice * nm_manager_get_device_by_ifindex (NMManager *manager, int ifindex); -- cgit v1.2.1 From a034a917e68c615a4b5b2002a87ae0f35d6599bf Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Sat, 21 Oct 2017 16:05:12 +0200 Subject: checkpoint: track checkpoints in a list Checkpoints will be exported over D-Bus and they must be presented in a predictable order. Keep them in a list ordered by creation time. --- src/nm-checkpoint-manager.c | 68 +++++++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 27 deletions(-) diff --git a/src/nm-checkpoint-manager.c b/src/nm-checkpoint-manager.c index 6da220c4cc..92872c45f9 100644 --- a/src/nm-checkpoint-manager.c +++ b/src/nm-checkpoint-manager.c @@ -29,12 +29,14 @@ #include "nm-exported-object.h" #include "nm-manager.h" #include "nm-utils.h" +#include "nm-utils/c-list.h" /*****************************************************************************/ struct _NMCheckpointManager { NMManager *_manager; GHashTable *checkpoints; + CList list; guint rollback_timeout_id; }; @@ -56,33 +58,43 @@ struct _NMCheckpointManager { /*****************************************************************************/ +typedef struct { + CList list; + NMCheckpoint *checkpoint; +} CheckpointItem; + static void update_rollback_timeout (NMCheckpointManager *self); static void -checkpoint_destroy (gpointer checkpoint) +item_destroy (gpointer data) { - nm_exported_object_unexport (NM_EXPORTED_OBJECT (checkpoint)); - g_object_unref (G_OBJECT (checkpoint)); + CheckpointItem *item = data; + + c_list_unlink (&item->list); + nm_exported_object_unexport (NM_EXPORTED_OBJECT (item->checkpoint)); + g_object_unref (G_OBJECT (item->checkpoint)); + g_slice_free (CheckpointItem, item); } static gboolean rollback_timeout_cb (NMCheckpointManager *self) { - NMCheckpoint *checkpoint; - GHashTableIter iter; + CheckpointItem *item, *safe; GVariant *result; gint64 ts, now; + const char *path; now = nm_utils_get_monotonic_timestamp_ms (); - g_hash_table_iter_init (&iter, self->checkpoints); - while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &checkpoint)) { - ts = nm_checkpoint_get_rollback_ts (checkpoint); + c_list_for_each_entry_safe (item, safe, &self->list, list) { + ts = nm_checkpoint_get_rollback_ts (item->checkpoint); if (ts && ts <= now) { - result = nm_checkpoint_rollback (checkpoint); + result = nm_checkpoint_rollback (item->checkpoint); if (result) g_variant_unref (result); - g_hash_table_iter_remove (&iter); + path = nm_exported_object_get_path (NM_EXPORTED_OBJECT (item->checkpoint)); + if (!g_hash_table_remove (self->checkpoints, path)) + nm_assert_not_reached(); } } @@ -95,13 +107,11 @@ rollback_timeout_cb (NMCheckpointManager *self) static void update_rollback_timeout (NMCheckpointManager *self) { - NMCheckpoint *checkpoint; - GHashTableIter iter; + CheckpointItem *item; gint64 ts, delta, next = G_MAXINT64; - g_hash_table_iter_init (&iter, self->checkpoints); - while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &checkpoint)) { - ts = nm_checkpoint_get_rollback_ts (checkpoint); + c_list_for_each_entry (item, &self->list, list) { + ts = nm_checkpoint_get_rollback_ts (item->checkpoint); if (ts && ts < next) next = ts; } @@ -120,13 +130,11 @@ update_rollback_timeout (NMCheckpointManager *self) static NMCheckpoint * find_checkpoint_for_device (NMCheckpointManager *self, NMDevice *device) { - GHashTableIter iter; - NMCheckpoint *checkpoint; + CheckpointItem *item; - g_hash_table_iter_init (&iter, self->checkpoints); - while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &checkpoint)) { - if (nm_checkpoint_includes_device (checkpoint, device)) - return checkpoint; + c_list_for_each_entry (item, &self->list, list) { + if (nm_checkpoint_includes_device (item->checkpoint, device)) + return item->checkpoint; } return NULL; @@ -141,6 +149,7 @@ nm_checkpoint_manager_create (NMCheckpointManager *self, { NMManager *manager; NMCheckpoint *checkpoint; + CheckpointItem *item; const char * const *path; gs_unref_ptrarray GPtrArray *devices = NULL; NMDevice *device; @@ -212,9 +221,13 @@ nm_checkpoint_manager_create (NMCheckpointManager *self, nm_exported_object_export (NM_EXPORTED_OBJECT (checkpoint)); checkpoint_path = nm_exported_object_get_path (NM_EXPORTED_OBJECT (checkpoint)); + item = g_slice_new0 (CheckpointItem); + item->checkpoint = checkpoint; + c_list_link_tail (&self->list, &item->list); + if (!nm_g_hash_table_insert (self->checkpoints, (gpointer) checkpoint_path, - checkpoint)) + item)) g_return_val_if_reached (NULL); update_rollback_timeout (self); @@ -263,21 +276,21 @@ nm_checkpoint_manager_rollback (NMCheckpointManager *self, GVariant **results, GError **error) { - NMCheckpoint *cp; + CheckpointItem *item; g_return_val_if_fail (self, FALSE); g_return_val_if_fail (checkpoint_path && checkpoint_path[0] == '/', FALSE); g_return_val_if_fail (results, FALSE); g_return_val_if_fail (!error || !*error, FALSE); - cp = g_hash_table_lookup (self->checkpoints, checkpoint_path); - if (!cp) { + item = g_hash_table_lookup (self->checkpoints, checkpoint_path); + if (!item) { g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_FAILED, "checkpoint %s does not exist", checkpoint_path); return FALSE; } - *results = nm_checkpoint_rollback (cp); + *results = nm_checkpoint_rollback (item->checkpoint); g_hash_table_remove (self->checkpoints, checkpoint_path); return TRUE; @@ -302,7 +315,8 @@ nm_checkpoint_manager_new (NMManager *manager) * instance. */ self->_manager = manager; self->checkpoints = g_hash_table_new_full (nm_str_hash, g_str_equal, - NULL, checkpoint_destroy); + NULL, item_destroy); + c_list_init (&self->list); return self; } -- cgit v1.2.1 From dece9f9ddaf9f73c5f6b62ccde6b42d39e20cc0a Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Sat, 21 Oct 2017 16:05:14 +0200 Subject: core: export checkpoint list over D-Bus --- introspection/org.freedesktop.NetworkManager.xml | 7 ++++ src/nm-checkpoint-manager.c | 44 ++++++++++++++++++++++-- src/nm-checkpoint-manager.h | 4 ++- src/nm-manager.c | 16 ++++++++- src/nm-manager.h | 1 + 5 files changed, 68 insertions(+), 4 deletions(-) diff --git a/introspection/org.freedesktop.NetworkManager.xml b/introspection/org.freedesktop.NetworkManager.xml index dceea7f21f..26a618c1bf 100644 --- a/introspection/org.freedesktop.NetworkManager.xml +++ b/introspection/org.freedesktop.NetworkManager.xml @@ -269,6 +269,13 @@ --> + + +