Warn user to reboot when reverting a snapset with in-use origin devices

snapm/_snapm.py

@@ -21,6 +21,7 @@
 import math
 import logging
 import re
+import os
 
 _log = logging.getLogger(__name__)
 
@@ -73,6 +74,7 @@
 SNAPSET_UUID = "UUID"
 SNAPSET_STATUS = "Status"
 SNAPSET_AUTOACTIVATE = "Autoactivate"
+SNAPSET_BOOTABLE = "Bootable"
 SNAPSET_BOOT_ENTRIES = "BootEntries"
 SNAPSET_SNAPSHOT_ENTRY = "SnapshotEntry"
 SNAPSET_REVERT_ENTRY = "RevertEntry"
@@ -196,6 +198,13 @@ class SnapmExistsError(SnapmError):
     """
 
 
+class SnapmBusyError(SnapmError):
+    """
+    A resouce needed by the current command is already in use: for e.g.
+    a snapshot merge is in progress for a previous snapshot set.
+    """
+
+
 class SnapmPathError(SnapmError):
     """
     An invalid path was supplied, for example attempting to snapshot
@@ -647,7 +656,8 @@ def __str__(self):
             f"{SNAPSET_TIME}:             {datetime.fromtimestamp(self.timestamp)}\n"
             f"{SNAPSET_UUID}:             {self.uuid}\n"
             f"{SNAPSET_STATUS}:           {str(self.status)}\n"
-            f"{SNAPSET_AUTOACTIVATE}:     {'yes' if self.autoactivate else 'no'}"
+            f"{SNAPSET_AUTOACTIVATE}:     {'yes' if self.autoactivate else 'no'}\n"
+            f"{SNAPSET_BOOTABLE}:         {'yes' if self.boot_entry is not None else 'no'}"
         )
         if self.boot_entry or self.revert_entry:
             snapset_str += f"\n{SNAPSET_BOOT_ENTRIES}:"
@@ -673,6 +683,8 @@ def as_dict(self, members=False):
         pmap[SNAPSET_TIME] = self.time
         pmap[SNAPSET_UUID] = str(self.uuid)
         pmap[SNAPSET_STATUS] = str(self.status)
+        pmap[SNAPSET_AUTOACTIVATE] = self.autoactivate
+        pmap[SNAPSET_BOOTABLE] = self.boot_entry is not None
 
         if self.boot_entry or self.revert_entry:
             pmap[SNAPSET_BOOT_ENTRIES] = {}
@@ -785,6 +797,18 @@ def autoactivate(self, value):
                     err,
                 )
 
+    @property
+    def origin_mounted(self):
+        """
+        Test whether the origin volumes for this ``SnapshotSet`` are currently
+        mounted and in use.
+
+        :returns: ``True`` if any of the snaphots belonging to this
+                  ``SnapshotSet`` are currently mounted, or ``False``
+                  otherwise.
+        """
+        return any([s.origin_mounted for s in self.snapshots])
+
     def snapshot_by_mount_point(self, mount_point):
         """
         Return the snapshot corresponding to ``mount_point``.
@@ -995,6 +1019,22 @@ def autoactivate(self):
         """
         raise NotImplementedError
 
+    @property
+    def origin_mounted(self):
+        """
+        Test whether the origin volume for this ``Snapshot`` is currently
+        mounted and in use.
+
+        :returns: ``True`` if this snaphot's prigin is currently mounted
+                  or ``False`` otherwise.
+        """
+        with open("/proc/mounts") as mounts:
+            for line in mounts:
+                fields = line.split(" ")
+                if self.mount_point == fields[1]:
+                    return os.path.samefile(self.origin, fields[0])
+        return False
+
     def delete(self):
         """
         Delete this snapshot.
@@ -1061,6 +1101,7 @@ def set_autoactivate(self, auto=False):
     "SNAPSET_UUID",
     "SNAPSET_STATUS",
     "SNAPSET_AUTOACTIVATE",
+    "SNAPSET_BOOTABLE",
     "SNAPSET_BOOT_ENTRIES",
     "SNAPSET_SNAPSHOT_ENTRY",
     "SNAPSET_REVERT_ENTRY",
@@ -1090,6 +1131,7 @@ def set_autoactivate(self, auto=False):
     "SnapmNoSpaceError",
     "SnapmNoProviderError",
     "SnapmExistsError",
+    "SnapmBusyError",
     "SnapmPathError",
     "SnapmNotFoundError",
     "SnapmInvalidIdentifierError",

snapm/command.py

@@ -35,6 +35,7 @@
     SNAPSET_UUID,
     SNAPSET_STATUS,
     SNAPSET_AUTOACTIVATE,
+    SNAPSET_BOOTABLE,
     SNAPSET_BOOT_ENTRIES,
     SNAPSET_SNAPSHOT_ENTRY,
     SNAPSET_REVERT_ENTRY,
@@ -204,6 +205,15 @@ def _bool_to_yes_no(bval):
         REP_STR,
         lambda f, d: f.report_str(_bool_to_yes_no(d.autoactivate)),
     ),
+    FieldType(
+        PR_SNAPSET,
+        "bootable",
+        SNAPSET_BOOTABLE,
+        "Configured for snapshot boot",
+        8,
+        REP_STR,
+        lambda f, d: f.report_str(_bool_to_yes_no(d.boot_entry is not None)),
+    ),
     FieldType(
         PR_SNAPSET,
         "bootentry",
@@ -448,16 +458,14 @@ def rename_snapset(manager, old_name, new_name):
     return manager.rename_snapshot_set(old_name, new_name)
 
 
-def revert_snapset(manager, selection):
+def revert_snapset(manager, name=None, uuid=None):
     """
     Revert snapshot set matching selection criteria.
 
     :param manager: The manager context to use
     :param selection: Selection criteria for the snapshot set to revert.
     """
-    if not selection.is_single():
-        raise SnapmInvalidIdentifierError("Revert requires unique selection criteria")
-    return manager.revert_snapshot_sets(selection)
+    return manager.revert_snapshot_set(name=name, uuid=uuid)
 
 
 def show_snapshots(manager, selection=None, json=False):
@@ -702,9 +710,23 @@ def _revert_cmd(cmd_args):
     :returns: integer status code returned from ``main()``
     """
     manager = Manager()
-    select = Selection.from_cmd_args(cmd_args)
-    count = revert_snapset(manager, select)
-    _log_info("Set %d snapshot set%s for revert", count, "s" if count > 1 else "")
+    name = None
+    uuid = None
+
+    selection = Selection.from_cmd_args(cmd_args)
+    if not selection.is_single():
+        raise SnapmInvalidIdentifierError("Revert requires unique selection criteria")
+    if selection.name:
+        name = selection.name
+    elif select.name:
+        uuid = selection.uuid
+    else:
+        raise SnapmInvalidIdentifierError("Revert requires a snapset name or UUID")
+
+    snapset = revert_snapset(manager, name=name, uuid=uuid)
+    if snapset.origin_mounted and snapset.revert_entry:
+        print(f"Boot into '{snapset.revert_entry.title}' to continue")
+    _log_info("Started revert for snapset %s", snapset.name)
     return 0
 
 

snapm/manager/_manager.py

@@ -677,19 +677,19 @@ def rename_snapshot_set(self, old_name, new_name):
                 new_snapshots.append(new_snapshot)
             except SnapmError as err:
                 _log_error("Failed to rename snapshot %s: %s", snapshot.name, err)
-                revert_snapshots = []
+                rollback_snapshots = []
                 for new_snapshot in new_snapshots:
                     try:
-                        revert_snapshot = snapshot.rename(old_name)
-                        revert_snapshots.append(revert_snapshot)
+                        rollback_snapshot = snapshot.rename(old_name)
+                        rollback_snapshots.append(rollback_snapshot)
                     except SnapmError as err2:
                         _log_error(
-                            "Failed to revert snapshot rename on %s: %s",
+                            "Failed to rollback snapshot rename on %s: %s",
                             snapshot.name,
                             err2,
                         )
                 old_snapset = SnapshotSet(
-                    old_name, timestamp, snapshots + revert_snapshots
+                    old_name, timestamp, snapshots + rollback_snapshots
                 )
                 self.by_name[old_snapset.name] = old_snapset
                 self.by_uuid[str(old_snapset.uuid)] = old_snapset
@@ -741,6 +741,58 @@ def delete_snapshot_sets(self, selection):
         self._boot_cache.refresh_cache()
         return deleted
 
+    def revert_snapshot_set(self, name=None, uuid=None):
+        """
+        Revert snapshot sets matching selection criteria ``selection``.
+
+        Request to revert each snapshot origin within each snapshot set
+        to the state at the time the snapshot was taken.
+
+        :param selection: Selection criteria for snapshot sets to revert.
+        """
+        if name and uuid:
+            if by_name[name] != by_uuid[uuid]:
+                raise SnapmInvalidIdentifierError(
+                    f"Conflicting name and UUID: {str(uuid)} does not match '{name}'"
+                )
+            snapset = by_name[name]
+        if name is not None:
+            if name not in self.by_name:
+                raise SnapmNotFoundError(f"Could not find snapshot set named {name}")
+            snapset = self.by_name[name]
+        elif uuid is not None:
+            if uuid not in self.by_uuid:
+                raise SnapmNotFoundError(
+                    f"Could not find snapshot set with uuid {uuid}"
+                )
+            snapset = self.by_uuid[uuid]
+        else:
+            raise SnapmNotFoundError("A snapshot set name or UUID is required")
+
+        # Snapshot boot entry becomes invalid as soon as revert is initiated.
+        delete_snapset_boot_entry(snapset)
+
+        for snapshot in snapset.snapshots:
+            try:
+                snapshot.revert()
+            except SnapmError as err:
+                _log_error(
+                    "Failed to revert snapshot set member %s: %s",
+                    snapshot.name,
+                    err,
+                )
+                raise SnapmPluginError(
+                    f"Could not revert all snapshots for set {snapset.name}"
+                )
+        if snapset.origin_mounted:
+            _log_warn(
+                "Snaphot set %s origin is in use: reboot required to complete revert",
+                snapset.name,
+            )
+
+        self._boot_cache.refresh_cache()
+        return snapset
+
     def revert_snapshot_sets(self, selection):
         """
         Revert snapshot sets matching selection criteria ``selection``.
@@ -757,21 +809,8 @@ def revert_snapshot_sets(self, selection):
                 f"Could not find snapshot sets matching {selection}"
             )
         for snapset in sets:
-            delete_snapset_boot_entry(snapset)
-            for snapshot in snapset.snapshots:
-                try:
-                    snapshot.revert()
-                except SnapmError as err:
-                    _log_error(
-                        "Failed to revert snapshot set member %s: %s",
-                        snapshot.name,
-                        err,
-                    )
-                    raise SnapmPluginError(
-                        f"Could not revert all snapshots for set {snapset.name}"
-                    )
+            self.revert_snapshot_set(name=snapset.name)
             reverted += 1
-        self._boot_cache.refresh_cache()
         return reverted
 
     def activate_snapshot_sets(self, selection):

snapm/manager/plugins/lvm2.py

@@ -25,6 +25,7 @@
     SNAPM_DEBUG_PLUGINS,
     SnapmInvalidIdentifierError,
     SnapmCalloutError,
+    SnapmBusyError,
     SnapmNoSpaceError,
     SizePolicy,
     SnapStatus,
@@ -82,6 +83,7 @@
 LVM_ACTIVE_ATTR = "a"
 LVM_INVALID_ATTR = "I"
 LVM_LV_TYPE_DEFAULT = "-"
+LVM_LV_ORIGIN_MERGING = "O"
 LVM_SKIP_ACTIVATION_ATTR = "k"
 
 # lv_attr flag indexes
@@ -711,6 +713,10 @@ def can_snapshot(self, mount_point):
         lvs_dict = get_lvs_json_report(f"{vg_name}/{lv_name}")
         lv_report = lvs_dict[LVS_REPORT][0][LVS_LV][0]
         lv_attr = lv_report[LVS_LV_ATTR]
+        if lv_attr[0] == LVM_LV_ORIGIN_MERGING:
+            raise SnapmBusyError(
+                f"Snapshot revert is in progress for {vg_name}/{lv_name}"
+            )
         if lv_attr[0] != LVM_LV_TYPE_DEFAULT and lv_attr[0] != LVM_COW_ORIGIN_ATTR:
             return False
         return True
@@ -849,6 +855,10 @@ def can_snapshot(self, mount_point):
         lvs_dict = get_lvs_json_report(f"{vg_name}/{lv_name}")
         lv_report = lvs_dict[LVS_REPORT][0][LVS_LV][0]
         lv_attr = lv_report[LVS_LV_ATTR]
+        if lv_attr[0] == LVM_LV_ORIGIN_MERGING:
+            raise SnapmBusyError(
+                f"Snapshot revert is in progress for {vg_name}/{lv_name}"
+            )
         if lv_attr[0] != LVM_THIN_VOL_ATTR:
             return False
         return True