Skip to content

Commit

Permalink
linstor: Fix ZFS snapshot backup
Browse files Browse the repository at this point in the history
Linstor plugin used the wrong zfs dataset path to hide/unhide
the snapshot device.
Also don't use the full path to the zfs binary.
  • Loading branch information
rp- committed Jan 20, 2025
1 parent 00c659b commit 05e1dd0
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
6 changes: 6 additions & 0 deletions plugins/storage/volume/linstor/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ All notable changes to Linstor CloudStack plugin will be documented in this file
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [2025-01-20]

### Fixed

- Volume snapshots on zfs used the wrong dataset path to hide/unhide snapdev

## [2024-12-13]

### Fixed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,17 @@ public final class LinstorBackupSnapshotCommandWrapper
{
private static final Logger s_logger = Logger.getLogger(LinstorBackupSnapshotCommandWrapper.class);

private static String zfsDatasetName(String zfsFullSnapshotUrl) {
String zfsFullPath = zfsFullSnapshotUrl.substring(6);
int atPos = zfsFullPath.indexOf('@');

Check warning on line 50 in plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java

View check run for this annotation

Codecov / codecov/patch

plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java#L49-L50

Added lines #L49 - L50 were not covered by tests
return atPos >= 0 ? zfsFullPath.substring(0, atPos) : zfsFullPath;
}

private String zfsSnapdev(boolean hide, String zfsUrl) {
Script script = new Script("/usr/bin/zfs", Duration.millis(5000));
Script script = new Script("zfs", Duration.millis(5000));

Check warning on line 55 in plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java

View check run for this annotation

Codecov / codecov/patch

plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java#L55

Added line #L55 was not covered by tests
script.add("set");
script.add("snapdev=" + (hide ? "hidden" : "visible"));
script.add(zfsUrl.substring(6)); // cutting zfs://
script.add(zfsDatasetName(zfsUrl)); // cutting zfs:// and @snapshotname

Check warning on line 58 in plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java

View check run for this annotation

Codecov / codecov/patch

plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java#L58

Added line #L58 was not covered by tests
return script.execute();
}

Expand Down Expand Up @@ -133,7 +139,7 @@ public CopyCmdAnswer execute(LinstorBackupSnapshotCommand cmd, LibvirtComputingR
s_logger.info("Src: " + srcPath + " | " + src.getName());
if (srcPath.startsWith("zfs://")) {
zfsHidden = true;
if (zfsSnapdev(false, srcPath) != null) {
if (zfsSnapdev(false, src.getPath()) != null) {
return new CopyCmdAnswer("Unable to unhide zfs snapshot device.");
}
srcPath = "/dev/" + srcPath.substring(6);
Expand Down

0 comments on commit 05e1dd0

Please sign in to comment.