From 1a6e3e0a57d682609d7148786ea3384552f04a84 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 9 Sep 2016 15:17:28 +0200 Subject: manager: add nm_manager_get_device_paths() --- src/nm-manager.c | 22 ++++++++++++++++++++++ src/nm-manager.h | 1 + 2 files changed, 23 insertions(+) diff --git a/src/nm-manager.c b/src/nm-manager.c index dfbe10c049..8967031ea2 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2288,6 +2288,28 @@ 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_connection_device (NMManager *self, NMConnection *connection) diff --git a/src/nm-manager.h b/src/nm-manager.h index fb97951ce3..9e0b4e3e79 100644 --- a/src/nm-manager.h +++ b/src/nm-manager.h @@ -91,6 +91,7 @@ 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 5754a05605a8c9b70c7a7d82bd540d375486a89d Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 7 Sep 2016 17:47:26 +0200 Subject: core: allow passing an applied connection to nm_act_request_new() This is useful for the checkpoint/restore functionality to revert both the applied and the settings connections. --- src/nm-activation-request.c | 6 +++++- src/nm-activation-request.h | 1 + src/nm-active-connection.c | 27 +++++++++++++++++++++++---- src/nm-active-connection.h | 1 + src/nm-checkpoint.c | 1 + src/nm-manager.c | 18 +++++++++++++++--- src/nm-manager.h | 1 + src/nm-policy.c | 3 +++ 8 files changed, 50 insertions(+), 8 deletions(-) diff --git a/src/nm-activation-request.c b/src/nm-activation-request.c index 22fa5b6910..6da89432a8 100644 --- a/src/nm-activation-request.c +++ b/src/nm-activation-request.c @@ -462,17 +462,20 @@ master_failed (NMActiveConnection *self) * nm_act_request_new: * * @settings_connection: (allow-none): the connection to activate @device with + * @applied_connection: (allow-none): the applied connection * @specific_object: the object path of the specific object (ie, WiFi access point, * etc) that will be used to activate @connection and @device * @subject: the #NMAuthSubject representing the requestor of the activation * @device: the device/interface to configure according to @connection * - * Creates a new device-based activation request. + * Creates a new device-based activation request. If an applied connection is + * supplied, it shall not be modified by the caller afterwards. * * Returns: the new activation request on success, %NULL on error. */ NMActRequest * nm_act_request_new (NMSettingsConnection *settings_connection, + NMConnection *applied_connection, const char *specific_object, NMAuthSubject *subject, NMDevice *device) @@ -482,6 +485,7 @@ nm_act_request_new (NMSettingsConnection *settings_connection, g_return_val_if_fail (NM_IS_AUTH_SUBJECT (subject), NULL); return (NMActRequest *) g_object_new (NM_TYPE_ACT_REQUEST, + NM_ACTIVE_CONNECTION_INT_APPLIED_CONNECTION, applied_connection, NM_ACTIVE_CONNECTION_INT_SETTINGS_CONNECTION, settings_connection, NM_ACTIVE_CONNECTION_INT_DEVICE, device, NM_ACTIVE_CONNECTION_SPECIFIC_OBJECT, specific_object, diff --git a/src/nm-activation-request.h b/src/nm-activation-request.h index 1bd004994e..5a170d8976 100644 --- a/src/nm-activation-request.h +++ b/src/nm-activation-request.h @@ -46,6 +46,7 @@ typedef struct { GType nm_act_request_get_type (void); NMActRequest *nm_act_request_new (NMSettingsConnection *settings_connection, + NMConnection *applied_connection, const char *specific_object, NMAuthSubject *subject, NMDevice *device); diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index 1844587c67..503c53191b 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -90,6 +90,7 @@ NM_GOBJECT_PROPERTIES_DEFINE (NMActiveConnection, PROP_MASTER, PROP_INT_SETTINGS_CONNECTION, + PROP_INT_APPLIED_CONNECTION, PROP_INT_DEVICE, PROP_INT_SUBJECT, PROP_INT_MASTER, @@ -951,6 +952,14 @@ constructed (GObject *object) G_OBJECT_CLASS (nm_active_connection_parent_class)->constructed (object); + if (!priv->applied_connection && priv->settings_connection) { + priv->applied_connection = + nm_simple_connection_new_clone ((NMConnection *) priv->settings_connection); + } + + if (priv->applied_connection) + nm_connection_clear_secrets (priv->applied_connection); + _LOGD ("constructed (%s, version-id %llu)", G_OBJECT_TYPE_NAME (self), (long long unsigned) priv->version_id); g_return_if_fail (priv->subject); @@ -964,16 +973,20 @@ set_property (GObject *object, guint prop_id, NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (self); const char *tmp; NMSettingsConnection *con; + NMConnection *acon; switch (prop_id) { case PROP_INT_SETTINGS_CONNECTION: /* construct-only */ con = g_value_get_object (value); - if (con) { + if (con) _set_settings_connection (self, con); - priv->applied_connection = nm_simple_connection_new_clone ((NMConnection *) priv->settings_connection); - nm_connection_clear_secrets (priv->applied_connection); - } + break; + case PROP_INT_APPLIED_CONNECTION: + /* construct-only */ + acon = g_value_get_object (value); + if (acon) + priv->applied_connection = g_object_ref (acon); break; case PROP_INT_DEVICE: /* construct-only */ @@ -1259,6 +1272,12 @@ nm_active_connection_class_init (NMActiveConnectionClass *ac_class) G_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY | G_PARAM_STATIC_STRINGS); + obj_properties[PROP_INT_APPLIED_CONNECTION] = + g_param_spec_object (NM_ACTIVE_CONNECTION_INT_APPLIED_CONNECTION, "", "", + NM_TYPE_CONNECTION, + G_PARAM_WRITABLE | G_PARAM_CONSTRUCT_ONLY | + G_PARAM_STATIC_STRINGS); + obj_properties[PROP_INT_DEVICE] = g_param_spec_object (NM_ACTIVE_CONNECTION_INT_DEVICE, "", "", NM_TYPE_DEVICE, diff --git a/src/nm-active-connection.h b/src/nm-active-connection.h index a66f8e9ffc..42e45a2bf1 100644 --- a/src/nm-active-connection.h +++ b/src/nm-active-connection.h @@ -50,6 +50,7 @@ /* Internal non-exported properties */ #define NM_ACTIVE_CONNECTION_INT_SETTINGS_CONNECTION "int-settings-connection" +#define NM_ACTIVE_CONNECTION_INT_APPLIED_CONNECTION "int-applied-connection" #define NM_ACTIVE_CONNECTION_INT_DEVICE "int-device" #define NM_ACTIVE_CONNECTION_INT_SUBJECT "int-subject" #define NM_ACTIVE_CONNECTION_INT_MASTER "int-master" diff --git a/src/nm-checkpoint.c b/src/nm-checkpoint.c index 605e0700be..f2d573574a 100644 --- a/src/nm-checkpoint.c +++ b/src/nm-checkpoint.c @@ -178,6 +178,7 @@ nm_checkpoint_rollback (NMCheckpoint *self) if (!nm_manager_activate_connection (priv->manager, connection, NULL, + NULL, device, subject, &local_error)) { diff --git a/src/nm-manager.c b/src/nm-manager.c index 8967031ea2..9d536e358b 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -64,6 +64,7 @@ static gboolean add_device (NMManager *self, NMDevice *device, GError **error); static NMActiveConnection *_new_active_connection (NMManager *self, NMConnection *connection, + NMConnection *applied, const char *specific_object, NMDevice *device, NMAuthSubject *subject, @@ -1745,7 +1746,7 @@ assume_connection (NMManager *self, NMDevice *device, NMSettingsConnection *conn g_return_val_if_fail (nm_device_get_state (device) >= NM_DEVICE_STATE_DISCONNECTED, FALSE); subject = nm_auth_subject_new_internal (); - active = _new_active_connection (self, NM_CONNECTION (connection), NULL, device, subject, &error); + active = _new_active_connection (self, NM_CONNECTION (connection), NULL, NULL, device, subject, &error); g_object_unref (subject); if (!active) { @@ -2634,6 +2635,7 @@ ensure_master_active_connection (NMManager *self, master_ac = nm_manager_activate_connection (self, candidate, NULL, + NULL, master_device, subject, error); @@ -2680,6 +2682,7 @@ ensure_master_active_connection (NMManager *self, master_ac = nm_manager_activate_connection (self, master_connection, NULL, + NULL, candidate, subject, error); @@ -2804,6 +2807,7 @@ autoconnect_slaves (NMManager *self, nm_manager_activate_connection (self, slave_connection, NULL, + NULL, nm_manager_get_best_device_for_connection (self, NM_CONNECTION (slave_connection), FALSE), subject, &local_err); @@ -2963,7 +2967,7 @@ _internal_activate_device (NMManager *self, NMActiveConnection *active, GError * return FALSE; } - parent_ac = nm_manager_activate_connection (self, parent_con, NULL, parent, subject, error); + parent_ac = nm_manager_activate_connection (self, parent_con, NULL, NULL, parent, subject, error); if (!parent_ac) { g_prefix_error (error, "%s failed to activate parent: ", nm_device_get_iface (device)); return FALSE; @@ -3145,6 +3149,7 @@ _new_vpn_active_connection (NMManager *self, static NMActiveConnection * _new_active_connection (NMManager *self, NMConnection *connection, + NMConnection *applied, const char *specific_object, NMDevice *device, NMAuthSubject *subject, @@ -3184,6 +3189,7 @@ _new_active_connection (NMManager *self, } return (NMActiveConnection *) nm_act_request_new (settings_connection, + applied, specific_object, subject, device); @@ -3234,6 +3240,7 @@ _internal_activation_auth_done (NMActiveConnection *active, * nm_manager_activate_connection(): * @self: the #NMManager * @connection: the #NMSettingsConnection to activate on @device + * @applied: (allow-none): the applied connection to activate on @device * @specific_object: the specific object path, if any, for the activation * @device: the #NMDevice to activate @connection on * @subject: the subject which requested activation @@ -3243,7 +3250,8 @@ _internal_activation_auth_done (NMActiveConnection *active, * @subject should be the subject of the activation that triggered this * one, or if this is an autoconnect request, a new internal subject. * The returned #NMActiveConnection is owned by the Manager and should be - * referenced by the caller if the caller continues to use it. + * referenced by the caller if the caller continues to use it. If @applied + * is supplied, it shall not be modified by the caller afterwards. * * Returns: (transfer none): the new #NMActiveConnection that tracks * activation of @connection on @device @@ -3251,6 +3259,7 @@ _internal_activation_auth_done (NMActiveConnection *active, NMActiveConnection * nm_manager_activate_connection (NMManager *self, NMSettingsConnection *connection, + NMConnection *applied, const char *specific_object, NMDevice *device, NMAuthSubject *subject, @@ -3296,6 +3305,7 @@ nm_manager_activate_connection (NMManager *self, active = _new_active_connection (self, NM_CONNECTION (connection), + applied, specific_object, device, subject, @@ -3550,6 +3560,7 @@ impl_manager_activate_connection (NMManager *self, active = _new_active_connection (self, NM_CONNECTION (connection), + NULL, specific_object_path, device, subject, @@ -3757,6 +3768,7 @@ impl_manager_add_and_activate_connection (NMManager *self, active = _new_active_connection (self, connection, + NULL, specific_object_path, device, subject, diff --git a/src/nm-manager.h b/src/nm-manager.h index 9e0b4e3e79..90041ccd7d 100644 --- a/src/nm-manager.h +++ b/src/nm-manager.h @@ -105,6 +105,7 @@ char * nm_manager_get_connection_iface (NMManager *self, NMActiveConnection *nm_manager_activate_connection (NMManager *manager, NMSettingsConnection *connection, + NMConnection *applied_connection, const char *specific_object, NMDevice *device, NMAuthSubject *subject, diff --git a/src/nm-policy.c b/src/nm-policy.c index 5fa46259cd..0e437d9d4c 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -724,6 +724,7 @@ auto_activate_device (gpointer user_data) subject = nm_auth_subject_new_internal (); if (!nm_manager_activate_connection (priv->manager, best_connection, + NULL, specific_object, data->device, subject, @@ -1117,6 +1118,7 @@ activate_secondary_connections (NMPolicy *self, nm_connection_get_id (connection), nm_connection_get_uuid (connection)); ac = nm_manager_activate_connection (priv->manager, settings_con, + NULL, nm_exported_object_get_path (NM_EXPORTED_OBJECT (req)), device, nm_active_connection_get_subject (NM_ACTIVE_CONNECTION (req)), @@ -1520,6 +1522,7 @@ vpn_connection_retry_after_failure (NMVpnConnection *vpn, NMPolicy *self) connection, NULL, NULL, + NULL, nm_active_connection_get_subject (ac), &error)) { _LOGW (LOGD_DEVICE, "VPN '%s' reconnect failed: %s", -- cgit v1.2.1 From 637e31bc1fea0f49608f60a6bc799d613e25a845 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 7 Sep 2016 17:47:17 +0200 Subject: checkpoint: better handle unmanaged and unrealized devices In order to better restore the previous system state, allow the inclusion of unmanaged devices in a checkpoint and try to revert to the old state taking also the realized/managed state into account. --- src/nm-checkpoint.c | 71 +++++++++++++++++++++++++++++++++-------------------- 1 file changed, 45 insertions(+), 26 deletions(-) diff --git a/src/nm-checkpoint.c b/src/nm-checkpoint.c index f2d573574a..aebc25649a 100644 --- a/src/nm-checkpoint.c +++ b/src/nm-checkpoint.c @@ -56,6 +56,9 @@ typedef struct { char *original_dev_path; NMDevice *device; NMConnection *connection; + NMDeviceState state; + bool realized:1; + bool unmanaged_explicit:1; } DeviceCheckpoint; typedef struct { @@ -124,17 +127,45 @@ nm_checkpoint_rollback (NMCheckpoint *self) guint32 result = NM_ROLLBACK_RESULT_OK; const char *con_path; - _LOGD ("rollback: restoring state of device %s", nm_device_get_iface (device)); - - if (!nm_device_is_real (device)) { - result = NM_ROLLBACK_RESULT_ERR_NO_DEVICE; - _LOGD ("rollback: device is not realized"); + _LOGD ("rollback: restoring device %s (state %d, realized %d, explicitly unmanaged %d)", + nm_device_get_iface (device), + (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) { + if (nm_device_is_software (device)) { + /* 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; + } + } goto next_dev; } - if (nm_device_get_state (device) <= NM_DEVICE_STATE_UNMANAGED) { - result = NM_ROLLBACK_RESULT_ERR_DEVICE_UNMANAGED; - _LOGD ("rollback: device is unmanaged"); +activate: + if (dev_checkpoint->state == NM_DEVICE_STATE_UNMANAGED) { + if ( nm_device_get_state (device) != NM_DEVICE_STATE_UNMANAGED + || dev_checkpoint->unmanaged_explicit) { + _LOGD ("rollback: explicitly unmanage device"); + nm_device_set_unmanaged_by_flags_queue (device, + NM_UNMANAGED_USER_EXPLICIT, + TRUE, + NM_DEVICE_STATE_REASON_NOW_UNMANAGED); + } goto next_dev; } @@ -216,30 +247,18 @@ device_checkpoint_create (NMDevice *device, DeviceCheckpoint *dev_checkpoint; NMConnection *connection; const char *path; - - if (!nm_device_is_real (device)) { - g_set_error (error, - NM_MANAGER_ERROR, - NM_MANAGER_ERROR_INVALID_ARGUMENTS, - "device '%s' is not realized", - nm_device_get_iface (device)); - return NULL; - } - - if (nm_device_get_state (device) <= NM_DEVICE_STATE_UNMANAGED) { - g_set_error (error, - NM_MANAGER_ERROR, - NM_MANAGER_ERROR_INVALID_ARGUMENTS, - "device '%s' is unmanaged", - nm_device_get_iface (device)); - return NULL; - } + gboolean unmanaged_explicit; path = nm_exported_object_get_path (NM_EXPORTED_OBJECT (device)); + unmanaged_explicit = !!nm_device_get_unmanaged_flags (device, + NM_UNMANAGED_USER_EXPLICIT); dev_checkpoint = g_slice_new0 (DeviceCheckpoint); dev_checkpoint->device = g_object_ref (device); dev_checkpoint->original_dev_path = g_strdup (path); + dev_checkpoint->state = nm_device_get_state (device); + dev_checkpoint->realized = nm_device_is_real (device); + dev_checkpoint->unmanaged_explicit = unmanaged_explicit; connection = nm_device_get_applied_connection (device); if (connection) -- cgit v1.2.1 From c76e2188772525c00873b9a2796121c7044cc899 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 7 Sep 2016 17:47:19 +0200 Subject: checkpoint: use UUID instead of path to match connections The path can change, while the UUID should univocally identify the connection. --- src/nm-checkpoint.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/nm-checkpoint.c b/src/nm-checkpoint.c index aebc25649a..22360250bb 100644 --- a/src/nm-checkpoint.c +++ b/src/nm-checkpoint.c @@ -125,7 +125,7 @@ nm_checkpoint_rollback (NMCheckpoint *self) while (g_hash_table_iter_next (&iter, (gpointer *) &device, (gpointer *) &dev_checkpoint)) { gs_unref_object NMAuthSubject *subject = NULL; guint32 result = NM_ROLLBACK_RESULT_OK; - const char *con_path; + const char *con_uuid; _LOGD ("rollback: restoring device %s (state %d, realized %d, explicitly unmanaged %d)", nm_device_get_iface (device), @@ -173,14 +173,14 @@ activate: /* The device had an active connection, check if the * connection still exists * */ - con_path = nm_connection_get_path (dev_checkpoint->connection); - connection = nm_settings_get_connection_by_path (nm_settings_get(), con_path); + con_uuid = nm_connection_get_uuid (dev_checkpoint->connection); + connection = nm_settings_get_connection_by_uuid (nm_settings_get (), con_uuid); if (connection) { /* If the connection is still there, restore its content * and save it * */ - _LOGD ("rollback: connection %s still exists", con_path); + _LOGD ("rollback: connection %s still exists", con_uuid); nm_connection_replace_settings_from_connection (NM_CONNECTION (connection), dev_checkpoint->connection); @@ -190,7 +190,7 @@ activate: NULL); } else { /* The connection was deleted, recreate it */ - _LOGD ("rollback: adding connection %s again", con_path); + _LOGD ("rollback: adding connection %s again", con_uuid); connection = nm_settings_add_connection (nm_settings_get (), dev_checkpoint->connection, -- cgit v1.2.1 From 8b9ea0b7c6254c62eaaf76e00ca2cfb79829cf53 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Wed, 7 Sep 2016 17:47:21 +0200 Subject: checkpoint: consider all devices when an empty list is passed First, consider all devices and not only realized and managed ones when an empty list is passed. Also, move the list evaluation to the checkpoint manager, since the check for device conflicts is done there. Fixes: 3e09aed2a09fab11f66b8228e48dc8f732c65cce --- introspection/nm-manager.xml | 2 +- src/nm-checkpoint-manager.c | 13 ++++++++++--- src/nm-checkpoint.c | 26 -------------------------- 3 files changed, 11 insertions(+), 30 deletions(-) diff --git a/introspection/nm-manager.xml b/introspection/nm-manager.xml index 0537e86533..1c29936285 100644 --- a/introspection/nm-manager.xml +++ b/introspection/nm-manager.xml @@ -209,7 +209,7 @@