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 restrictin, that if you once rolled back to an
older checkpoint, you no longer can roll"forward" to a younger one.

What is also 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.

4 files changed, 78 insertions, 30 deletions

diff --git a/libnm-core/nm-dbus-interface.h b/libnm-core/nm-dbus-interface.h
index b2e448dfa3..392d28432f 100644
--- a/libnm-core/nm-dbus-interface.h
+++ b/libnm-core/nm-dbus-interface.h
@@ -842,6 +842,18 @@ typedef enum {
  *   delete any new connection added after the checkpoint (Since: 1.6)
  * @NM_CHECKPOINT_CREATE_FLAG_DISCONNECT_NEW_DEVICES: upon rollback,
  *   disconnect any new device appeared after the checkpoint (Since: 1.6)
+ * @NM_CHECKPOINT_CREATE_FLAG_ALLOW_OVERLAPPING: by default, creating
+ *   a checkpoint fails if there are already existing checkoints that
+ *   reference the same devices. With this flag, creation of such
+ *   checkpoints is allowed, however, if an older checkpoint
+ *   that references overlapping devices gets rolled back, it will
+ *   automatically destroy this checkpoint during rollback. This
+ *   allows to create several overlappig checkpoints in parallel,
+ *   and rollback to them at will. With the special case that
+ *   rolling back to an older checkpoint will invalidate all
+ *   overlapping younger checkpoints. This opts-in that the
+ *   checkpoint can be automatically destroyed by the rollback
+ *   of an older checkpoint. (Since: 1.12)
  *
  * The flags for CheckpointCreate call
  *
@@ -852,6 +864,7 @@ typedef enum { /*< skip >*/
 	NM_CHECKPOINT_CREATE_FLAG_DESTROY_ALL                 = 0x01,
 	NM_CHECKPOINT_CREATE_FLAG_DELETE_NEW_CONNECTIONS      = 0x02,
 	NM_CHECKPOINT_CREATE_FLAG_DISCONNECT_NEW_DEVICES      = 0x04,
+	NM_CHECKPOINT_CREATE_FLAG_ALLOW_OVERLAPPING           = 0x08,
 } NMCheckpointCreateFlags;
 
 /**
diff --git a/src/nm-checkpoint-manager.c b/src/nm-checkpoint-manager.c
index 1086a0cb46..173eaf5a48 100644
--- a/src/nm-checkpoint-manager.c
+++ b/src/nm-checkpoint-manager.c
@@ -63,7 +63,7 @@ notify_checkpoints (NMCheckpointManager *self) {
 }
 
 static void
-destroy_checkpoint (NMCheckpointManager *self, NMCheckpoint *checkpoint)
+destroy_checkpoint (NMCheckpointManager *self, NMCheckpoint *checkpoint, gboolean log_destroy)
 {
 	nm_assert (NM_IS_CHECKPOINT (checkpoint));
 	nm_assert (nm_dbus_object_is_exported (NM_DBUS_OBJECT (checkpoint)));
@@ -73,6 +73,9 @@ destroy_checkpoint (NMCheckpointManager *self, NMCheckpoint *checkpoint)
 
 	c_list_unlink (&checkpoint->checkpoints_lst);
 
+	if (log_destroy)
+		nm_checkpoint_log_destroy (checkpoint);
+
 	notify_checkpoints (self);
 
 	nm_dbus_object_unexport (NM_DBUS_OBJECT (checkpoint));
@@ -83,9 +86,26 @@ static GVariant *
 rollback_checkpoint (NMCheckpointManager *self, NMCheckpoint *checkpoint)
 {
 	GVariant *result;
+	const CList *iter;
+
+	nm_assert (c_list_contains (&self->checkpoints_lst_head, &checkpoint->checkpoints_lst));
+
+	/* we destroy first all overlapping checkpoints that are younger/newer. */
+	for (iter = checkpoint->checkpoints_lst.next;
+	     iter != &self->checkpoints_lst_head;
+	     ) {
+		NMCheckpoint *cp = c_list_entry (iter, NMCheckpoint, checkpoints_lst);
+
+		iter = iter->next;
+		if (nm_checkpoint_includes_devices_of (cp, checkpoint)) {
+			/* the younger checkpoint has overlapping devices and gets obsoleted.
+			 * Destroy it. */
+			destroy_checkpoint (self, cp, TRUE);
+		}
+	}
 
 	result = nm_checkpoint_rollback (checkpoint);
-	destroy_checkpoint (self, checkpoint);
+	destroy_checkpoint (self, checkpoint, FALSE);
 	return result;
 }
 
@@ -99,19 +119,6 @@ rollback_timeout_cb (NMCheckpoint *checkpoint,
 	result = rollback_checkpoint (self, checkpoint);
 }
 
-static NMCheckpoint *
-find_checkpoint_for_device (NMCheckpointManager *self, NMDevice *device)
-{
-	NMCheckpoint *checkpoint;
-
-	c_list_for_each_entry (checkpoint, &self->checkpoints_lst_head, checkpoints_lst) {
-		if (nm_checkpoint_includes_device (checkpoint, device))
-			return checkpoint;
-	}
-
-	return NULL;
-}
-
 NMCheckpoint *
 nm_checkpoint_manager_create (NMCheckpointManager *self,
                               const char *const *device_paths,
@@ -123,7 +130,6 @@ nm_checkpoint_manager_create (NMCheckpointManager *self,
 	NMCheckpoint *checkpoint;
 	gs_unref_ptrarray GPtrArray *devices = NULL;
 	NMDevice *device;
-	guint i;
 
 	g_return_val_if_fail (self, FALSE);
 	g_return_val_if_fail (!error || !*error, FALSE);
@@ -169,11 +175,12 @@ nm_checkpoint_manager_create (NMCheckpointManager *self,
 		return NULL;
 	}
 
-	if (!NM_FLAGS_HAS (flags, NM_CHECKPOINT_CREATE_FLAG_DESTROY_ALL)) {
-		for (i = 0; i < devices->len; i++) {
-			device = devices->pdata[i];
-			checkpoint = find_checkpoint_for_device (self, device);
-			if (checkpoint) {
+	if (NM_FLAGS_HAS (flags, NM_CHECKPOINT_CREATE_FLAG_DESTROY_ALL))
+		nm_checkpoint_manager_destroy_all (self);
+	else if (!NM_FLAGS_HAS (flags, NM_CHECKPOINT_CREATE_FLAG_ALLOW_OVERLAPPING)) {
+		c_list_for_each_entry (checkpoint, &self->checkpoints_lst_head, checkpoints_lst) {
+			device = nm_checkpoint_includes_devices (checkpoint, (NMDevice *const*) devices->pdata, devices->len);
+			if (device) {
 				g_set_error (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_INVALID_ARGUMENTS,
 				             "device '%s' is already included in checkpoint %s",
 				             nm_device_get_iface (device),
@@ -185,9 +192,6 @@ nm_checkpoint_manager_create (NMCheckpointManager *self,
 
 	checkpoint = nm_checkpoint_new (manager, devices, rollback_timeout, flags);
 
-	if (NM_FLAGS_HAS (flags, NM_CHECKPOINT_CREATE_FLAG_DESTROY_ALL))
-		nm_checkpoint_manager_destroy_all (self);
-
 	nm_dbus_object_export (NM_DBUS_OBJECT (checkpoint));
 
 	nm_checkpoint_set_timeout_callback (checkpoint, rollback_timeout_cb, self);
@@ -204,7 +208,7 @@ nm_checkpoint_manager_destroy_all (NMCheckpointManager *self)
 	g_return_if_fail (self);
 
 	while ((checkpoint = c_list_first_entry (&self->checkpoints_lst_head, NMCheckpoint, checkpoints_lst)))
-		destroy_checkpoint (self, checkpoint);
+		destroy_checkpoint (self, checkpoint, TRUE);
 }
 
 gboolean
@@ -232,7 +236,7 @@ nm_checkpoint_manager_destroy (NMCheckpointManager *self,
 		return FALSE;
 	}
 
-	destroy_checkpoint (self, checkpoint);
+	destroy_checkpoint (self, checkpoint, TRUE);
 	return TRUE;
 }
 
diff --git a/src/nm-checkpoint.c b/src/nm-checkpoint.c
index 57d5da89e5..a93bdf5113 100644
--- a/src/nm-checkpoint.c
+++ b/src/nm-checkpoint.c
@@ -101,6 +101,12 @@ G_DEFINE_TYPE (NMCheckpoint, nm_checkpoint, NM_TYPE_DBUS_OBJECT)
 /*****************************************************************************/
 
 void
+nm_checkpoint_log_destroy (NMCheckpoint *self)
+{
+	_LOGI ("destroy %s", nm_dbus_object_get_path (NM_DBUS_OBJECT (self)));
+}
+
+void
 nm_checkpoint_set_timeout_callback (NMCheckpoint *self,
                                     NMCheckpointTimeoutCallback callback,
                                     gpointer user_data)
@@ -114,12 +120,33 @@ nm_checkpoint_set_timeout_callback (NMCheckpoint *self,
 	priv->timeout_data = user_data;
 }
 
-gboolean
-nm_checkpoint_includes_device (NMCheckpoint *self, NMDevice *device)
+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)
 {
 	NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE (self);
+	NMCheckpointPrivate *priv2 = NM_CHECKPOINT_GET_PRIVATE (cp_for_devices);
+	GHashTableIter iter;
+	NMDevice *device;
 
-	return g_hash_table_contains (priv->devices, device);
+	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;
 }
 
 static NMSettingsConnection *
diff --git a/src/nm-checkpoint.h b/src/nm-checkpoint.h
index 49783ca533..e342650c23 100644
--- a/src/nm-checkpoint.h
+++ b/src/nm-checkpoint.h
@@ -53,11 +53,15 @@ NMCheckpoint *nm_checkpoint_new (NMManager *manager, GPtrArray *devices, guint32
 typedef void (*NMCheckpointTimeoutCallback) (NMCheckpoint *self,
                                              gpointer user_data);
 
+void nm_checkpoint_log_destroy (NMCheckpoint *self);
+
 void nm_checkpoint_set_timeout_callback (NMCheckpoint *self,
                                          NMCheckpointTimeoutCallback callback,
                                          gpointer user_data);
 
-gboolean nm_checkpoint_includes_device (NMCheckpoint *checkpoint, NMDevice *device);
 GVariant *nm_checkpoint_rollback (NMCheckpoint *self);
 
+NMDevice *nm_checkpoint_includes_devices (NMCheckpoint *self, NMDevice *const*devices, guint n_devices);
+NMDevice *nm_checkpoint_includes_devices_of (NMCheckpoint *self, NMCheckpoint *cp_for_devices);
+
 #endif /* __NETWORKMANAGER_CHECKPOINT_H__ */