DaanHoogland commented on code in PR #8067:
URL: https://github.com/apache/cloudstack/pull/8067#discussion_r1352554473
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/api/storage/LinstorRevertBackupSnapshotCommand.java:
##########
@@ -0,0 +1,12 @@
+package com.cloud.api.storage;
Review Comment:
license
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/api/storage/LinstorBackupSnapshotCommand.java:
##########
@@ -0,0 +1,12 @@
+package com.cloud.api.storage;
Review Comment:
license needed
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java:
##########
@@ -0,0 +1,128 @@
+package com.cloud.hypervisor.kvm.resource.wrapper;
Review Comment:
license
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorRevertBackupSnapshotCommandWrapper.java:
##########
@@ -0,0 +1,73 @@
+package com.cloud.hypervisor.kvm.resource.wrapper;
Review Comment:
license
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorRevertBackupSnapshotCommandWrapper.java:
##########
@@ -0,0 +1,73 @@
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import java.io.File;
+
+import com.cloud.agent.api.to.DataStoreTO;
+import com.cloud.api.storage.LinstorRevertBackupSnapshotCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePool;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.storage.Storage;
+import org.apache.cloudstack.storage.command.CopyCmdAnswer;
+import org.apache.cloudstack.storage.to.SnapshotObjectTO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.cloudstack.utils.qemu.QemuImg;
+import org.apache.cloudstack.utils.qemu.QemuImgFile;
+import org.apache.log4j.Logger;
+
+@ResourceWrapper(handles = LinstorRevertBackupSnapshotCommand.class)
+public final class LinstorRevertBackupSnapshotCommandWrapper
+ extends CommandWrapper<LinstorRevertBackupSnapshotCommand, CopyCmdAnswer,
LibvirtComputingResource>
+{
+ private static final Logger s_logger =
Logger.getLogger(LinstorRevertBackupSnapshotCommandWrapper.class);
+
+ @Override
+ public CopyCmdAnswer execute(LinstorRevertBackupSnapshotCommand cmd,
LibvirtComputingResource serverResource)
+ {
+ s_logger.debug("LinstorRevertBackupSnapshotCommandWrapper: " +
cmd.getSrcTO().getPath() + " -> " + cmd.getDestTO().getPath());
+ final SnapshotObjectTO src = (SnapshotObjectTO) cmd.getSrcTO();
+ final VolumeObjectTO dst = (VolumeObjectTO) cmd.getDestTO();
+ KVMStoragePool secondaryPool = null;
+ final KVMStoragePoolManager storagePoolMgr =
serverResource.getStoragePoolMgr();
+ KVMStoragePool linstorPool =
storagePoolMgr.getStoragePool(Storage.StoragePoolType.Linstor,
dst.getDataStore().getUuid());
+
+ if (linstorPool == null) {
+ return new CopyCmdAnswer("Unable to get linstor storage pool from
destination volume.");
+ }
+
+ try
+ {
+ final DataStoreTO srcDataStore = src.getDataStore();
+ File srcFile = new File(src.getPath());
+ secondaryPool = storagePoolMgr.getStoragePoolByURI(
+ srcDataStore.getUrl() + File.separator + srcFile.getParent());
+
+ final QemuImgFile srcQemuFile = new QemuImgFile(
+ secondaryPool.getLocalPath() + File.separator +
srcFile.getName(), QemuImg.PhysicalDiskFormat.QCOW2);
+ final QemuImg qemu = new QemuImg(cmd.getWaitInMillSeconds());
+ final QemuImgFile dstFile = new QemuImgFile(
+ linstorPool.getPhysicalDisk(dst.getPath()).getPath(),
+ QemuImg.PhysicalDiskFormat.RAW);
+ qemu.convert(srcQemuFile, dstFile);
+
+ final VolumeObjectTO dstVolume = new VolumeObjectTO();
+ dstVolume.setPath(dst.getPath());
+ return new CopyCmdAnswer(dstVolume);
+ } catch (final Exception e) {
+ final String error = String.format("Failed to revert snapshot with
id [%s] with a pool %s, due to %s",
+ cmd.getSrcTO().getId(),
cmd.getSrcTO().getDataStore().getUuid(), e.getMessage());
+ s_logger.error(error);
+ return new CopyCmdAnswer(cmd, e);
+ } finally {
+ if (secondaryPool != null) {
+ try {
+ secondaryPool.delete();
+ } catch (final Exception e) {
+ s_logger.debug("Failed to delete secondary storage", e);
+ }
+ }
Review Comment:
looks like a section from the prior cmd wrapper, mayby unify in a util
method?
##########
plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LinstorBackupSnapshotCommandWrapper.java:
##########
@@ -0,0 +1,128 @@
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import java.io.File;
+
+import com.cloud.agent.api.to.DataStoreTO;
+import com.cloud.agent.api.to.NfsTO;
+import com.cloud.api.storage.LinstorBackupSnapshotCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePool;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.resource.ResourceWrapper;
+import com.cloud.storage.Storage;
+import com.cloud.utils.script.Script;
+import org.apache.cloudstack.storage.command.CopyCmdAnswer;
+import org.apache.cloudstack.storage.to.SnapshotObjectTO;
+import org.apache.cloudstack.utils.qemu.QemuImg;
+import org.apache.cloudstack.utils.qemu.QemuImgFile;
+import org.apache.commons.io.FileUtils;
+import org.apache.log4j.Logger;
+import org.joda.time.Duration;
+
+@ResourceWrapper(handles = LinstorBackupSnapshotCommand.class)
+public final class LinstorBackupSnapshotCommandWrapper
+ extends CommandWrapper<LinstorBackupSnapshotCommand, CopyCmdAnswer,
LibvirtComputingResource>
+{
+ private static final Logger s_logger =
Logger.getLogger(LinstorBackupSnapshotCommandWrapper.class);
+
+ private String zfsSnapdev(boolean hide, String zfsUrl) {
+ Script script = new Script("/usr/bin/zfs", Duration.millis(5000));
+ script.add("set");
+ script.add("snapdev=" + (hide ? "hidden" : "visible"));
+ script.add(zfsUrl.substring(6)); // cutting zfs://
+ return script.execute();
+ }
+
+ private String qemuShrink(String path, long sizeByte, long timeout) {
+ Script qemuImg = new Script("qemu-img", Duration.millis(timeout));
+ qemuImg.add("resize");
+ qemuImg.add("--shrink");
+ qemuImg.add(path);
+ qemuImg.add("" + sizeByte);
+ return qemuImg.execute();
+ }
+
+ @Override
+ public CopyCmdAnswer execute(LinstorBackupSnapshotCommand cmd,
LibvirtComputingResource serverResource)
Review Comment:
can you disect this a bit to make it more readable (for instance extract the
init section, try - and finally clauses?
##########
plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/driver/LinstorPrimaryDataStoreDriverImpl.java:
##########
@@ -668,15 +688,39 @@ public void revertSnapshot(
return;
}
- String resultMsg;
+ final StoragePool pool = (StoragePool) volumeInfo.getDataStore();
+ final DevelopersApi linstorApi =
LinstorUtil.getLinstorAPI(pool.getHostAddress());
+ final String rscName = LinstorUtil.RSC_PREFIX + volumeInfo.getUuid();
+ String resultMsg = null;
try {
- final StoragePool pool = (StoragePool) snapshot.getDataStore();
- final String rscName = LinstorUtil.RSC_PREFIX +
volumeInfo.getUuid();
- final String snapName = LinstorUtil.RSC_PREFIX +
snapshot.getUuid();
- final DevelopersApi linstorApi =
LinstorUtil.getLinstorAPI(pool.getHostAddress());
-
- ApiCallRcList answers =
linstorApi.resourceSnapshotRollback(rscName, snapName);
- resultMsg = checkLinstorAnswers(answers);
+ if (snapshot.getDataStore().getRole() == DataStoreRole.Primary) {
+ final String snapName = LinstorUtil.RSC_PREFIX +
snapshot.getUuid();
+
+ ApiCallRcList answers =
linstorApi.resourceSnapshotRollback(rscName, snapName);
+ resultMsg = checkLinstorAnswers(answers);
+ } else if (snapshot.getDataStore().getRole() ==
DataStoreRole.Image) {
+ String value =
_configDao.getValue(Config.BackupSnapshotWait.toString());
+ int _backupsnapshotwait = NumbersUtil.parseInt(
+ value,
Integer.parseInt(Config.BackupSnapshotWait.getDefaultValue()));
+
+ LinstorRevertBackupSnapshotCommand cmd = new
LinstorRevertBackupSnapshotCommand(
+ snapshot.getTO(),
+ volumeInfo.getTO(),
+ _backupsnapshotwait,
+ VirtualMachineManager.ExecuteInSequence.value());
+
+ Optional<RemoteHostEndPoint> optEP = getDiskfullEP(linstorApi,
rscName);
+ if (optEP.isPresent()) {
+ Answer answer = optEP.get().sendMessage(cmd);
+ if (!answer.getResult()) {
+ resultMsg = answer.getDetails();
+ }
+ } else {
+ resultMsg = "Unable to get matching Linstor endpoint.";
+ }
+ } else {
+ resultMsg = "Linstor: Snapshot revert datastore not supported";
+ }
Review Comment:
can this go in (a) separate method(s)?
##########
plugins/storage/volume/linstor/src/main/java/org/apache/cloudstack/storage/datastore/util/LinstorConfigurationManager.java:
##########
@@ -0,0 +1,24 @@
+package org.apache.cloudstack.storage.datastore.util;
Review Comment:
license
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
