Skip to content

Commit 886f98e

Browse files
committed
feat: implement revertable snapshot support with version validation
Add support for creating revert boot entries when snapm is available. This allows users to boot into a special mode that automatically merges snapshots back into their origin volumes for disaster recovery. The revertable parameter now properly validates snapm availability and version support, failing explicitly rather than being silently ignored. Changes: - Add SNAPM_REVERT_PARAM_MIN_VERSION constant (0.5.0) - Add has_revert_parameter() version check function - Implement revertable parameter handling with conflict detection - Add explicit version validation for bootable and revertable features - Fail with ERROR_REVERTABLE_NOT_SUPPORTED when snapm is unavailable or version doesn't support revert parameter - Pass bootable/revert parameters to snapm create_snapshot_set via kwargs pattern for clean version-conditional support - Propagate revertable parameter through validation layer - Add ERROR_REVERTABLE_NOT_SUPPORTED and ERROR_REVERTABLE_CONFLICT status codes - Add debug logging for create_snapshot_set calls The implementation mirrors the existing bootable parameter handling, supporting both global-level and snapset-level configuration with conflict detection. Both parameters now validate properly instead of silently ignoring unsupported features. Signed-off-by: Todd Gill <tgill@redhat.com>
1 parent 2ee1d35 commit 886f98e

4 files changed

Lines changed: 93 additions & 12 deletions

File tree

module_utils/snapshot_lsr/consts.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ class SnapshotStatus:
6161
ERROR_SNAPM_INTERNAL_ERROR = 42
6262
ERROR_BOOTABLE_NOT_SUPPORTED = 43
6363
ERROR_BOOTABLE_CONFLICT = 44
64+
ERROR_REVERTABLE_NOT_SUPPORTED = 45
65+
ERROR_REVERTABLE_CONFLICT = 46
6466

6567

6668
COMMAND_ENV = {"LC_ALL": "C", "LVM_COMMAND_PROFILE": "lvmdbusd"}

module_utils/snapshot_lsr/lvm.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1235,6 +1235,20 @@ def snapshot_set(module, snapset_json, check_mode):
12351235
changed,
12361236
)
12371237

1238+
# If this function has been called with revertable snapshot requested, return error
1239+
# because snapm is required for revertable snapshots.
1240+
if any(
1241+
(
1242+
snapset_json.get("snapshot_lvm_revertable", False),
1243+
snapset_json.get("revertable", False),
1244+
)
1245+
):
1246+
return (
1247+
SnapshotStatus.ERROR_REVERTABLE_NOT_SUPPORTED,
1248+
"Revertable snapshots are not supported without snapm",
1249+
changed,
1250+
)
1251+
12381252
rc, message = verify_snapset_source_lvs_exist(module, snapset_json)
12391253
if rc != SnapshotStatus.SNAPSHOT_OK:
12401254
return rc, message, changed

module_utils/snapshot_lsr/snapmgr.py

Lines changed: 71 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@
3535
# Minimum version of snapm that supports the boot parameter
3636
# in the create_snapshot_set function.
3737
SNAPM_BOOT_PARAM_MIN_VERSION = "0.4.3"
38+
# Minimum version of snapm that supports the revert parameter
39+
# in the create_snapshot_set function.
40+
SNAPM_REVERT_PARAM_MIN_VERSION = "0.5.0"
3841

3942

4043
# NOTE: Because of PEP632, we cannot use distutils.
@@ -73,6 +76,16 @@ def has_boot_parameter():
7376
return True
7477

7578

79+
def has_revert_parameter():
80+
81+
if not snapshot_manager_imported or lsr_parse_version(
82+
snapm.__version__
83+
) < lsr_parse_version(SNAPM_REVERT_PARAM_MIN_VERSION):
84+
return False
85+
86+
return True
87+
88+
7689
def mgr_get_snapshot_lv(module, origin_vg, origin_lv, snapshot_set):
7790

7891
for snapshot in snapshot_set[0].snapshots:
@@ -202,6 +215,7 @@ def mgr_check_verify_lvs_set(manager, module, snapset_json):
202215

203216
def mgr_snapshot_cmd(module, module_args, snapset_json):
204217
bootable = None
218+
revertable = None
205219
snapset_name = snapset_json["name"]
206220
logger.info("mgr_snapshot_cmd: %s", snapset_name)
207221
changed = False
@@ -237,8 +251,48 @@ def mgr_snapshot_cmd(module, module_args, snapset_json):
237251
"changed": False,
238252
}
239253

254+
# Revertable global variable is set
255+
if module_args["snapshot_lvm_revertable"]:
256+
revertable = module_args["snapshot_lvm_revertable"]
257+
258+
# Global is not set, check the snapset
259+
if revertable is None:
260+
if "revertable" in snapset_json:
261+
revertable = snapset_json["revertable"]
262+
else:
263+
revertable = False
264+
else: # Global is set, check for conflict
265+
if (
266+
"revertable" in snapset_json
267+
and snapset_json["revertable"] is not None
268+
and revertable != snapset_json["revertable"]
269+
):
270+
return {
271+
"return_code": SnapshotStatus.ERROR_REVERTABLE_CONFLICT,
272+
"errors": "Conflicting values for revertable",
273+
"changed": False,
274+
}
275+
240276
source_list = mgr_get_source_list_for_create(volume_list)
241277

278+
# Check if bootable is requested but not supported by snapm version
279+
if bootable and not has_boot_parameter():
280+
return {
281+
"return_code": SnapshotStatus.ERROR_BOOTABLE_NOT_SUPPORTED,
282+
"errors": "Bootable snapshots require snapm version %s or later"
283+
% SNAPM_BOOT_PARAM_MIN_VERSION,
284+
"changed": False,
285+
}
286+
287+
# Check if revertable is requested but not supported by snapm version
288+
if revertable and not has_revert_parameter():
289+
return {
290+
"return_code": SnapshotStatus.ERROR_REVERTABLE_NOT_SUPPORTED,
291+
"errors": "Revertable snapshots require snapm version %s or later"
292+
% SNAPM_REVERT_PARAM_MIN_VERSION,
293+
"changed": False,
294+
}
295+
242296
if check_mode:
243297
return {
244298
"return_code": SnapshotStatus.SNAPSHOT_OK,
@@ -249,19 +303,24 @@ def mgr_snapshot_cmd(module, module_args, snapset_json):
249303
manager = snap_manager.Manager()
250304

251305
try:
306+
# Build kwargs for create_snapshot_set based on available parameters
307+
kwargs = {}
252308
if has_boot_parameter():
253-
manager.create_snapshot_set(
254-
snapset_name,
255-
source_list,
256-
SNAPM_DEFAULT_SIZE_POLICY,
257-
boot=bootable,
258-
)
259-
else:
260-
manager.create_snapshot_set(
261-
snapset_name,
262-
source_list,
263-
SNAPM_DEFAULT_SIZE_POLICY,
264-
)
309+
kwargs["boot"] = bootable
310+
if has_revert_parameter():
311+
kwargs["revert"] = revertable
312+
313+
logger.info(
314+
"DEBUG: Calling create_snapshot_set with snapset_name=%s, "
315+
"source_list=%s, default_size_policy=%s, kwargs=%s",
316+
snapset_name,
317+
source_list,
318+
SNAPM_DEFAULT_SIZE_POLICY,
319+
kwargs,
320+
)
321+
manager.create_snapshot_set(
322+
snapset_name, source_list, SNAPM_DEFAULT_SIZE_POLICY, **kwargs
323+
)
265324
changed = True
266325
except snapm.SnapmError as snap_err:
267326
# if the set already exists, return ok

module_utils/snapshot_lsr/validate.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ def get_json_from_args(module, module_args, vg_include):
3636
if module_args["snapshot_lvm_bootable"]:
3737
args_dict["bootable"] = module_args["snapshot_lvm_bootable"]
3838

39+
if module_args["snapshot_lvm_revertable"]:
40+
args_dict["revertable"] = module_args["snapshot_lvm_revertable"]
41+
3942
for vg, lv_list in vgs_lvs_iterator(
4043
module,
4144
module_args["snapshot_lvm_vg"],
@@ -341,6 +344,9 @@ def validate_snapset_json(cmd, module_args, verify_only):
341344
if module_args["snapshot_lvm_bootable"]:
342345
snapset_dict["snapshot_lvm_bootable"] = module_args["snapshot_lvm_bootable"]
343346

347+
if module_args["snapshot_lvm_revertable"]:
348+
snapset_dict["snapshot_lvm_revertable"] = module_args["snapshot_lvm_revertable"]
349+
344350
if cmd == SnapshotCommand.SNAPSHOT:
345351
rc, message = validate_json_request(snapset_dict, True)
346352
elif cmd == SnapshotCommand.CHECK and not verify_only:

0 commit comments

Comments
 (0)