From 2027f25256be3fc4c74f18230793c35c7bc146e9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <joao@scclouds.com.br>
Date: Tue, 7 Jan 2025 10:29:06 -0300
Subject: [PATCH 01/11] File-based disk-only VM snapshot with KVM as hypervisor
---
.../agent/properties/AgentProperties.java | 8 +
.../com/cloud/vm/snapshot/VMSnapshot.java | 7 +-
.../CreateDiskOnlyVmSnapshotAnswer.java | 39 +
.../CreateDiskOnlyVmSnapshotCommand.java | 41 ++
.../DeleteDiskOnlyVmSnapshotCommand.java | 44 ++
.../MergeDiskOnlyVmSnapshotCommand.java | 55 ++
.../RevertDiskOnlyVmSnapshotAnswer.java | 38 +
.../RevertDiskOnlyVmSnapshotCommand.java | 50 ++
.../api/storage/SnapshotMergeTreeTO.java | 57 ++
.../cloud/vm/snapshot/dao/VMSnapshotDao.java | 2 +
.../vm/snapshot/dao/VMSnapshotDaoImpl.java | 8 +
.../datastore/db/SnapshotDataStoreDao.java | 2 +
.../db/SnapshotDataStoreDaoImpl.java | 6 +
.../vmsnapshot/DefaultVMSnapshotStrategy.java | 31 +-
...KvmFileBasedStorageVmSnapshotStrategy.java | 667 ++++++++++++++++++
.../vmsnapshot/StorageVMSnapshotStrategy.java | 23 +-
...ngine-storage-snapshot-storage-context.xml | 3 +
.../vmsnapshot/VMSnapshotStrategyTest.java | 6 +
.../kvm/resource/BlockCommitListener.java | 77 ++
.../resource/LibvirtComputingResource.java | 228 ++++++
...reateDiskOnlyVMSnapshotCommandWrapper.java | 198 ++++++
...eleteDiskOnlyVMSnapshotCommandWrapper.java | 58 ++
...MergeDiskOnlyVMSnapshotCommandWrapper.java | 150 ++++
...evertDiskOnlyVMSnapshotCommandWrapper.java | 111 +++
.../wrapper/LibvirtUtilitiesHelper.java | 10 +
.../kvm/storage/KVMStoragePool.java | 7 +
.../kvm/storage/KVMStorageProcessor.java | 79 +--
.../apache/cloudstack/utils/qemu/QemuImg.java | 40 ++
.../cloudstack/utils/qemu/QemuImgFile.java | 6 +
.../LibvirtComputingResourceTest.java | 162 +++++
.../kvm/storage/KVMStorageProcessorTest.java | 57 +-
.../cloud/storage/VolumeApiServiceImpl.java | 44 +-
.../vm/snapshot/VMSnapshotManagerImpl.java | 11 +-
.../storage/VolumeApiServiceImplTest.java | 97 ++-
ui/src/config/section/compute.js | 4 +-
35 files changed, 2262 insertions(+), 164 deletions(-)
create mode 100644 core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotAnswer.java
create mode 100644 core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotCommand.java
create mode 100644 core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java
create mode 100644 core/src/main/java/com/cloud/agent/api/storage/MergeDiskOnlyVmSnapshotCommand.java
create mode 100644 core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotAnswer.java
create mode 100644 core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java
create mode 100644 core/src/main/java/com/cloud/agent/api/storage/SnapshotMergeTreeTO.java
create mode 100644 engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
create mode 100644 plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BlockCommitListener.java
create mode 100644 plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java
create mode 100644 plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper.java
create mode 100644 plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMergeDiskOnlyVMSnapshotCommandWrapper.java
create mode 100644 plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertDiskOnlyVMSnapshotCommandWrapper.java
diff --git a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java
index feb1845d84b2..38bbc4b8a6a4 100644
--- a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java
+++ b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java
@@ -155,6 +155,14 @@ public class AgentProperties{
*/
public static final Property<Integer> CMDS_TIMEOUT = new Property<>("cmds.timeout", 7200);
+ /**
+ * The timeout (in seconds) for the snapshot merge operation, mainly used for classic volume snapshots and disk-only VM snapshots on file-based storage.<br>
+ * This configuration is only considered if libvirt.events.enabled is also true. <br>
+ * Data type: Integer.<br>
+ * Default value: <code>259200</code>
+ */
+ public static final Property<Integer> SNAPSHOT_MERGE_TIMEOUT = new Property<>("snapshot.merge.timeout", 60 * 60 * 72);
+
/**
* This parameter sets the VM migration speed (in mbps). The default value is -1,<br>
* which means that the agent will try to guess the speed of the guest network and consume all possible bandwidth.<br>
diff --git a/api/src/main/java/com/cloud/vm/snapshot/VMSnapshot.java b/api/src/main/java/com/cloud/vm/snapshot/VMSnapshot.java
index 3897df2d5e67..24e93af15621 100644
--- a/api/src/main/java/com/cloud/vm/snapshot/VMSnapshot.java
+++ b/api/src/main/java/com/cloud/vm/snapshot/VMSnapshot.java
@@ -31,7 +31,8 @@ public interface VMSnapshot extends ControlledEntity, Identity, InternalIdentity
enum State {
Allocated("The VM snapshot is allocated but has not been created yet."), Creating("The VM snapshot is being created."), Ready(
"The VM snapshot is ready to be used."), Reverting("The VM snapshot is being used to revert"), Expunging("The volume is being expunging"), Removed(
- "The volume is destroyed, and can't be recovered."), Error("The volume is in error state, and can't be recovered");
+ "The volume is destroyed, and can't be recovered."), Error("The volume is in error state, and can't be recovered"),
+ Hidden("The VM snapshot is hidden from the user and cannot be recovered.");
String _description;
@@ -60,6 +61,8 @@ public String getDescription() {
s_fsm.addTransition(Expunging, Event.ExpungeRequested, Expunging);
s_fsm.addTransition(Expunging, Event.OperationSucceeded, Removed);
s_fsm.addTransition(Expunging, Event.OperationFailed, Error);
+ s_fsm.addTransition(Expunging, Event.Hide, Hidden);
+ s_fsm.addTransition(Hidden, Event.ExpungeRequested, Expunging);
}
}
@@ -68,7 +71,7 @@ enum Type {
}
enum Event {
- CreateRequested, OperationFailed, OperationSucceeded, RevertRequested, ExpungeRequested,
+ CreateRequested, OperationFailed, OperationSucceeded, RevertRequested, ExpungeRequested, Hide,
}
@Override
diff --git a/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotAnswer.java b/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotAnswer.java
new file mode 100644
index 000000000000..4d61249c7cbc
--- /dev/null
+++ b/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotAnswer.java
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package com.cloud.agent.api.storage;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.Command;
+import com.cloud.utils.Pair;
+
+import java.util.Map;
+
+public class CreateDiskOnlyVmSnapshotAnswer extends Answer {
+
+ protected Map<String, Pair<Long, String>> mapVolumeToSnapshotSizeAndNewVolumePath;
+
+ public CreateDiskOnlyVmSnapshotAnswer(Command command, boolean success, String details, Map<String, Pair<Long, String>> mapVolumeToSnapshotSizeAndNewVolumePath) {
+ super(command, success, details);
+ this.mapVolumeToSnapshotSizeAndNewVolumePath = mapVolumeToSnapshotSizeAndNewVolumePath;
+ }
+
+ public Map<String, Pair<Long, String>> getMapVolumeToSnapshotSizeAndNewVolumePath() {
+ return mapVolumeToSnapshotSizeAndNewVolumePath;
+ }
+}
diff --git a/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotCommand.java b/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotCommand.java
new file mode 100644
index 000000000000..952bf0c971de
--- /dev/null
+++ b/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotCommand.java
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package com.cloud.agent.api.storage;
+
+
+import com.cloud.agent.api.VMSnapshotBaseCommand;
+import com.cloud.agent.api.VMSnapshotTO;
+import com.cloud.vm.VirtualMachine;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+
+import java.util.List;
+
+public class CreateDiskOnlyVmSnapshotCommand extends VMSnapshotBaseCommand {
+
+ protected VirtualMachine.State vmState;
+
+ public CreateDiskOnlyVmSnapshotCommand(String vmName, VMSnapshotTO snapshot, List<VolumeObjectTO> volumeTOs, String guestOSType, VirtualMachine.State vmState) {
+ super(vmName, snapshot, volumeTOs, guestOSType);
+ this.vmState = vmState;
+ }
+
+ public VirtualMachine.State getVmState() {
+ return vmState;
+ }
+}
diff --git a/core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java b/core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java
new file mode 100644
index 000000000000..368a506f2127
--- /dev/null
+++ b/core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java
@@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package com.cloud.agent.api.storage;
+
+import com.cloud.agent.api.Command;
+
+import com.cloud.agent.api.to.DataTO;
+
+
+import java.util.List;
+
+public class DeleteDiskOnlyVmSnapshotCommand extends Command {
+
+ List<DataTO> snapshots;
+
+ public DeleteDiskOnlyVmSnapshotCommand(List<DataTO> snapshots) {
+ this.snapshots = snapshots;
+ }
+
+ public List<DataTO> getSnapshots() {
+ return snapshots;
+ }
+
+ @Override
+ public boolean executeInSequence() {
+ return true;
+ }
+}
diff --git a/core/src/main/java/com/cloud/agent/api/storage/MergeDiskOnlyVmSnapshotCommand.java b/core/src/main/java/com/cloud/agent/api/storage/MergeDiskOnlyVmSnapshotCommand.java
new file mode 100644
index 000000000000..463189b8b0b8
--- /dev/null
+++ b/core/src/main/java/com/cloud/agent/api/storage/MergeDiskOnlyVmSnapshotCommand.java
@@ -0,0 +1,55 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package com.cloud.agent.api.storage;
+
+import com.cloud.agent.api.Command;
+import com.cloud.vm.VirtualMachine;
+
+import java.util.List;
+
+public class MergeDiskOnlyVmSnapshotCommand extends Command {
+
+ private List<SnapshotMergeTreeTO> snapshotMergeTreeToList;
+ private VirtualMachine.State vmState;
+ private String vmName;
+
+ public MergeDiskOnlyVmSnapshotCommand(List<SnapshotMergeTreeTO> snapshotMergeTreeToList, VirtualMachine.State vmState, String vmName) {
+ this.snapshotMergeTreeToList = snapshotMergeTreeToList;
+ this.vmState = vmState;
+ this.vmName = vmName;
+ }
+
+ public List<SnapshotMergeTreeTO> getSnapshotMergeTreeToList() {
+ return snapshotMergeTreeToList;
+ }
+
+ public VirtualMachine.State getVmState() {
+ return vmState;
+ }
+
+ public String getVmName() {
+ return vmName;
+ }
+
+ @Override
+ public boolean executeInSequence() {
+ return true;
+ }
+
+}
diff --git a/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotAnswer.java b/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotAnswer.java
new file mode 100644
index 000000000000..2ecf587d59d1
--- /dev/null
+++ b/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotAnswer.java
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package com.cloud.agent.api.storage;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.Command;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+
+import java.util.List;
+
+public class RevertDiskOnlyVmSnapshotAnswer extends Answer {
+ List<VolumeObjectTO> volumeObjectTos;
+
+ public RevertDiskOnlyVmSnapshotAnswer(Command cmd, List<VolumeObjectTO> volumeObjectTos) {
+ super(cmd, true, null);
+ this.volumeObjectTos = volumeObjectTos;
+ }
+
+ public List<VolumeObjectTO> getVolumeObjectTos() {
+ return volumeObjectTos;
+ }
+}
diff --git a/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java b/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java
new file mode 100644
index 000000000000..6effca7dfa07
--- /dev/null
+++ b/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package com.cloud.agent.api.storage;
+
+import com.cloud.agent.api.Command;
+import org.apache.cloudstack.storage.to.SnapshotObjectTO;
+
+import java.util.List;
+
+public class RevertDiskOnlyVmSnapshotCommand extends Command {
+
+ private List<SnapshotObjectTO> snapshotObjectTos;
+ private String vmName;
+
+ public RevertDiskOnlyVmSnapshotCommand(List<SnapshotObjectTO> snapshotObjectTos, String vmName) {
+ super();
+ this.snapshotObjectTos = snapshotObjectTos;
+ this.vmName = vmName;
+ }
+
+ public List<SnapshotObjectTO> getSnapshotObjectTos() {
+ return snapshotObjectTos;
+ }
+
+ public String getVmName() {
+ return vmName;
+ }
+
+ @Override
+ public boolean executeInSequence() {
+ return true;
+ }
+
+}
diff --git a/core/src/main/java/com/cloud/agent/api/storage/SnapshotMergeTreeTO.java b/core/src/main/java/com/cloud/agent/api/storage/SnapshotMergeTreeTO.java
new file mode 100644
index 000000000000..78f23105e192
--- /dev/null
+++ b/core/src/main/java/com/cloud/agent/api/storage/SnapshotMergeTreeTO.java
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package com.cloud.agent.api.storage;
+
+import com.cloud.agent.api.to.DataTO;
+import org.apache.commons.lang3.builder.ReflectionToStringBuilder;
+
+import java.util.List;
+
+public class SnapshotMergeTreeTO {
+ DataTO parent;
+ DataTO child;
+ List<DataTO> grandChildren;
+
+ public SnapshotMergeTreeTO(DataTO parent, DataTO child, List<DataTO> grandChildren) {
+ this.parent = parent;
+ this.child = child;
+ this.grandChildren = grandChildren;
+ }
+
+ public DataTO getParent() {
+ return parent;
+ }
+
+ public DataTO getChild() {
+ return child;
+ }
+
+ public List<DataTO> getGrandChildren() {
+ return grandChildren;
+ }
+
+ public void addGrandChild(DataTO grandChild) {
+ grandChildren.add(grandChild);
+ }
+
+ @Override
+ public String toString() {
+ return ReflectionToStringBuilder.toString(this);
+ }
+}
diff --git a/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDao.java b/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDao.java
index 0143aaa1e735..ace58167c0ed 100644
--- a/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDao.java
+++ b/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDao.java
@@ -27,6 +27,8 @@ public interface VMSnapshotDao extends GenericDao<VMSnapshotVO, Long>, StateDao<
List<VMSnapshotVO> findByVm(Long vmId);
+ List<VMSnapshotVO> findByVmAndByType(Long vmId, VMSnapshot.Type type);
+
List<VMSnapshotVO> listExpungingSnapshot();
List<VMSnapshotVO> listByInstanceId(Long vmId, VMSnapshot.State... status);
diff --git a/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDaoImpl.java
index 03a978f85469..d7af7292d2e1 100644
--- a/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDaoImpl.java
+++ b/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDaoImpl.java
@@ -80,6 +80,14 @@ public List<VMSnapshotVO> findByVm(Long vmId) {
return listBy(sc, null);
}
+ @Override
+ public List<VMSnapshotVO> findByVmAndByType(Long vmId, VMSnapshot.Type type) {
+ SearchCriteria<VMSnapshotVO> sc = AllFieldsSearch.create();
+ sc.setParameters("vm_id", vmId);
+ sc.setParameters("vm_snapshot_type", type);
+ return listBy(sc, null);
+ }
+
@Override
public List<VMSnapshotVO> listExpungingSnapshot() {
SearchCriteria<VMSnapshotVO> sc = ExpungingSnapshotSearch.create();
diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java
index 4cd29b465eeb..9e1458dfb583 100644
--- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java
+++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDao.java
@@ -52,6 +52,8 @@ public interface SnapshotDataStoreDao extends GenericDao<SnapshotDataStoreVO, Lo
SnapshotDataStoreVO findBySourceSnapshot(long snapshotId, DataStoreRole role);
+ SnapshotDataStoreVO findBySnapshotIdInAnyState(long snapshotId, DataStoreRole role);
+
List<SnapshotDataStoreVO> listDestroyed(long storeId);
List<SnapshotDataStoreVO> findBySnapshotId(long snapshotId);
diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java
index 5bf67eb38819..287dc91f0e21 100644
--- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java
+++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java
@@ -321,6 +321,12 @@ public SnapshotDataStoreVO findBySourceSnapshot(long snapshotId, DataStoreRole r
return findOneBy(sc);
}
+ @Override
+ public SnapshotDataStoreVO findBySnapshotIdInAnyState(long snapshotId, DataStoreRole role) {
+ SearchCriteria<SnapshotDataStoreVO> sc = createSearchCriteriaBySnapshotIdAndStoreRole(snapshotId, role);
+ return findOneBy(sc);
+ }
+
@Override
public List<SnapshotDataStoreVO> listAllByVolumeAndDataStore(long volumeId, DataStoreRole role) {
SearchCriteria<SnapshotDataStoreVO> sc = searchFilteringStoreIdEqStateEqStoreRoleEqIdEqUpdateCountEqSnapshotIdEqVolumeIdEq.create();
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
index 09f569e6f193..c035700f64d2 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
@@ -25,6 +25,9 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotOptions;
import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy;
@@ -100,6 +103,13 @@ public class DefaultVMSnapshotStrategy extends ManagerBase implements VMSnapshot
@Inject
PrimaryDataStoreDao primaryDataStoreDao;
+ @Inject
+ VMSnapshotDetailsDao vmSnapshotDetailsDao;
+
+ protected static final String KVM_FILE_BASED_STORAGE_SNAPSHOT = "kvmFileBasedStorageSnapshot";
+
+ protected static final String STORAGE_SNAPSHOT = "kvmStorageSnapshot";
+
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
String value = configurationDao.getValue("vmsnapshot.create.wait");
@@ -469,16 +479,35 @@ public boolean deleteVMSnapshotFromDB(VMSnapshot vmSnapshot, boolean unmanage) {
@Override
public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMemory) {
UserVmVO vm = userVmDao.findById(vmId);
- if (vm.getState() == State.Running && !snapshotMemory) {
+ if (State.Running.equals(vm.getState()) && (!snapshotMemory || vmHasKvmDiskOnlySnapshot(vm))) {
+ logger.debug("Default VM snapshot cannot handle VM snapshot for [{}] as it is running and its memory will not be affected.", vm);
return StrategyPriority.CANT_HANDLE;
}
List<VolumeVO> volumes = volumeDao.findByInstance(vmId);
for (VolumeVO volume : volumes) {
if (volume.getFormat() != ImageFormat.QCOW2) {
+ logger.debug("Default VM snapshot cannot handle VM snapshot for [{}] as it has a volume [{}] that is not in the QCOW2 format.", vm, volume);
return StrategyPriority.CANT_HANDLE;
}
}
return StrategyPriority.DEFAULT;
}
+
+ private boolean vmHasKvmDiskOnlySnapshot(UserVm vm) {
+ if (!Hypervisor.HypervisorType.KVM.equals(vm.getHypervisorType())) {
+ return false;
+ }
+
+ for (VMSnapshotVO vmSnapshotVO : vmSnapshotDao.findByVmAndByType(vm.getId(), VMSnapshot.Type.Disk)) {
+ List<VMSnapshotDetailsVO> vmSnapshotDetails = vmSnapshotDetailsDao.listDetails(vmSnapshotVO.getId());
+ if (vmSnapshotDetails.stream().
+ anyMatch(vmSnapshotDetailsVO -> vmSnapshotDetailsVO.getName().equals(KVM_FILE_BASED_STORAGE_SNAPSHOT) ||
+ vmSnapshotDetailsVO.getName().equals(STORAGE_SNAPSHOT))) {
+ return true;
+ }
+ }
+
+ return false;
+ }
}
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
new file mode 100644
index 000000000000..6463bf738a41
--- /dev/null
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
@@ -0,0 +1,667 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.cloudstack.storage.vmsnapshot;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.VMSnapshotTO;
+import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotAnswer;
+import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotCommand;
+import com.cloud.agent.api.storage.DeleteDiskOnlyVmSnapshotCommand;
+import com.cloud.agent.api.storage.MergeDiskOnlyVmSnapshotCommand;
+import com.cloud.agent.api.storage.RevertDiskOnlyVmSnapshotAnswer;
+import com.cloud.agent.api.storage.RevertDiskOnlyVmSnapshotCommand;
+import com.cloud.agent.api.storage.SnapshotMergeTreeTO;
+import com.cloud.agent.api.to.DataTO;
+import com.cloud.configuration.Resource;
+import com.cloud.event.EventTypes;
+import com.cloud.event.UsageEventUtils;
+import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.DataStoreRole;
+import com.cloud.storage.Snapshot;
+import com.cloud.storage.SnapshotVO;
+import com.cloud.storage.Storage;
+import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeVO;
+import com.cloud.user.ResourceLimitService;
+import com.cloud.uservm.UserVm;
+import com.cloud.utils.DateUtil;
+import com.cloud.utils.Pair;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.fsm.NoTransitionException;
+import com.cloud.vm.UserVmVO;
+import com.cloud.vm.VirtualMachine;
+import com.cloud.vm.snapshot.VMSnapshot;
+import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
+import com.cloud.vm.snapshot.VMSnapshotVO;
+import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
+import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotOptions;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.snapshot.SnapshotObject;
+import org.apache.cloudstack.storage.to.SnapshotObjectTO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.commons.collections.CollectionUtils;
+
+import javax.inject.Inject;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Objects;
+import java.util.stream.Collectors;
+
+public class KvmFileBasedStorageVmSnapshotStrategy extends StorageVMSnapshotStrategy {
+
+ private static final List<Storage.StoragePoolType> supportedStoragePoolTypes = List.of(Storage.StoragePoolType.Filesystem, Storage.StoragePoolType.NetworkFilesystem, Storage.StoragePoolType.SharedMountPoint);
+
+ @Inject
+ protected SnapshotDataStoreDao snapshotDataStoreDao;
+
+ @Inject
+ protected ResourceLimitService resourceLimitManager;
+
+ @Override
+ public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
+ Map<VolumeInfo, SnapshotObject> volumeInfoToSnapshotObjectMap = new HashMap<>();
+ try {
+ return takeVmSnapshotInternal(vmSnapshot, volumeInfoToSnapshotObjectMap);
+ } catch (CloudRuntimeException | NullPointerException | NoTransitionException ex) {
+ for (VolumeInfo volumeInfo : volumeInfoToSnapshotObjectMap.keySet()) {
+ volumeInfo.stateTransit(Volume.Event.OperationFailed);
+ SnapshotObject snapshot = volumeInfoToSnapshotObjectMap.get(volumeInfo);
+ try {
+ snapshot.processEvent(Snapshot.Event.OperationFailed);
+ } catch (NoTransitionException e) {
+ logger.error("Failed to change snapshot [{}] state due to [{}].", snapshot.getUuid(), e.getMessage(), e);
+ }
+ snapshot.processEvent(ObjectInDataStoreStateMachine.Event.OperationFailed);
+ }
+ try {
+ vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed);
+ } catch (NoTransitionException e) {
+ throw new CloudRuntimeException(e);
+ }
+ throw new CloudRuntimeException(ex);
+ }
+ }
+
+ @Override
+ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) {
+ logger.info("Starting VM snapshot delete process for snapshot [{}].", vmSnapshot.getUuid());
+ UserVmVO userVm = userVmDao.findById(vmSnapshot.getVmId());
+ VMSnapshotVO vmSnapshotBeingDeleted = (VMSnapshotVO) vmSnapshot;
+ Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshotBeingDeleted.getVmId());
+ long virtualSize = 0;
+ boolean isCurrent = vmSnapshotBeingDeleted.getCurrent();
+
+ transitStateWithoutThrow(vmSnapshotBeingDeleted, VMSnapshot.Event.ExpungeRequested);
+
+ List<VolumeObjectTO> volumeTOs = vmSnapshotHelper.getVolumeTOList(vmSnapshotBeingDeleted.getVmId());
+ List<VMSnapshotVO> snapshotChildren = vmSnapshotDao.listByParent(vmSnapshotBeingDeleted.getId());
+
+ long realSize = getVMSnapshotRealSize(vmSnapshotBeingDeleted);
+ int numberOfChildren = snapshotChildren.size();
+
+ List<SnapshotVO> volumeSnapshotVos = new ArrayList<>();
+ if (isCurrent && numberOfChildren == 0) {
+ volumeSnapshotVos = mergeCurrentDeltaOnSnapshot(vmSnapshotBeingDeleted, userVm, hostId, volumeTOs);
+ } else if (numberOfChildren == 0) {
+ logger.debug("Deleting VM snapshot [{}] as no snapshots/volumes depend on it.", vmSnapshot.getUuid());
+ volumeSnapshotVos = deleteSnapshot(vmSnapshotBeingDeleted, hostId);
+ mergeOldSiblingWithOldParentIfOldParentIsDead(vmSnapshotDao.findByIdIncludingRemoved(vmSnapshotBeingDeleted.getParent()), userVm, hostId, volumeTOs);
+ } else if (!isCurrent && numberOfChildren == 1) {
+ VMSnapshotVO childSnapshot = snapshotChildren.get(0);
+ volumeSnapshotVos = mergeSnapshots(vmSnapshotBeingDeleted, childSnapshot, userVm, volumeTOs, hostId);
+ }
+
+ for (SnapshotVO snapshotVO : volumeSnapshotVos) {
+ snapshotVO.setState(Snapshot.State.Destroyed);
+ snapshotDao.update(snapshotVO.getId(), snapshotVO);
+ }
+
+ for (VolumeObjectTO volumeTo : volumeTOs) {
+ publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_DELETE, vmSnapshotBeingDeleted, userVm, volumeTo);
+ virtualSize += volumeTo.getSize();
+ }
+
+ publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_OFF_PRIMARY, vmSnapshotBeingDeleted, userVm, realSize, virtualSize);
+
+ if (numberOfChildren > 1 || (isCurrent && numberOfChildren == 1)) {
+ transitStateWithoutThrow(vmSnapshotBeingDeleted, VMSnapshot.Event.Hide);
+ return true;
+ }
+
+ transitStateWithoutThrow(vmSnapshotBeingDeleted, VMSnapshot.Event.OperationSucceeded);
+
+ vmSnapshotDetailsDao.removeDetails(vmSnapshotBeingDeleted.getId());
+
+ vmSnapshotBeingDeleted.setRemoved(DateUtil.now());
+ vmSnapshotDao.update(vmSnapshotBeingDeleted.getId(), vmSnapshotBeingDeleted);
+
+ return true;
+ }
+
+ @Override
+ public boolean revertVMSnapshot(VMSnapshot vmSnapshot) {
+ UserVmVO userVm = userVmDao.findById(vmSnapshot.getVmId());
+ if (!userVm.getState().equals(VirtualMachine.State.Stopped)) {
+ throw new CloudRuntimeException("VM must be stopped to revert disk-only VM snapshot.");
+ }
+
+ VMSnapshotVO vmSnapshotBeingReverted = (VMSnapshotVO) vmSnapshot;
+
+ transitStateWithoutThrow(vmSnapshotBeingReverted, VMSnapshot.Event.RevertRequested);
+
+ Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshotBeingReverted.getVmId());
+ List<SnapshotDataStoreVO> volumeSnapshots = getVolumeSnapshotsAssociatedWithVmSnapshot(vmSnapshotBeingReverted);
+ List<SnapshotObjectTO> volumeSnapshotTos = volumeSnapshots.stream()
+ .map(snapshot -> (SnapshotObjectTO) snapshotDataFactory.getSnapshot(snapshot.getSnapshotId(), snapshot.getDataStoreId(), DataStoreRole.Primary).getTO())
+ .collect(Collectors.toList());
+
+ RevertDiskOnlyVmSnapshotCommand revertDiskOnlyVMSnapshotCommand = new RevertDiskOnlyVmSnapshotCommand(volumeSnapshotTos, userVm.getName());
+ Answer answer = agentMgr.easySend(hostId, revertDiskOnlyVMSnapshotCommand);
+
+ if (answer == null || !answer.getResult()) {
+ transitStateWithoutThrow(vmSnapshotBeingReverted, VMSnapshot.Event.OperationFailed);
+ logger.error(answer != null ? answer.getDetails() : String.format("Communication failure with host [%s].", hostId));
+ throw new CloudRuntimeException(String.format("Error reverting VM snapshot [%s].", vmSnapshot.getUuid()));
+ }
+
+ RevertDiskOnlyVmSnapshotAnswer revertDiskOnlyVMSnapshotAnswer = (RevertDiskOnlyVmSnapshotAnswer) answer;
+
+ for (VolumeObjectTO volumeObjectTo : revertDiskOnlyVMSnapshotAnswer.getVolumeObjectTos()) {
+ VolumeVO volumeVO = volumeDao.findById(volumeObjectTo.getVolumeId());
+ volumeVO.setPath(volumeObjectTo.getPath());
+ updateSizeIfNeeded(volumeSnapshots, volumeVO, volumeObjectTo);
+
+ volumeDao.update(volumeVO.getId(), volumeVO);
+ publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_REVERT, vmSnapshotBeingReverted, userVm, volumeObjectTo);
+ }
+
+ transitStateWithoutThrow(vmSnapshotBeingReverted, VMSnapshot.Event.OperationSucceeded);
+
+ VMSnapshotVO currentVmSnapshot = vmSnapshotDao.findCurrentSnapshotByVmId(userVm.getId());
+ currentVmSnapshot.setCurrent(false);
+ vmSnapshotBeingReverted.setCurrent(true);
+
+ vmSnapshotDao.update(currentVmSnapshot.getId(), currentVmSnapshot);
+ vmSnapshotDao.update(vmSnapshotBeingReverted.getId(), vmSnapshotBeingReverted);
+
+ mergeOldSiblingWithOldParentIfOldParentIsDead(currentVmSnapshot, userVm, hostId, vmSnapshotHelper.getVolumeTOList(userVm.getId()));
+ return true;
+ }
+
+ /**
+ * Updates the volume size if it changed due to the snapshot reversion.
+ * */
+ private void updateSizeIfNeeded(List<SnapshotDataStoreVO> volumeSnapshots, VolumeVO volumeVO, VolumeObjectTO volumeObjectTO) {
+ SnapshotDataStoreVO snapshotRef = volumeSnapshots.stream().filter(snapshotDataStoreVO -> snapshotDataStoreVO.getVolumeId() == volumeVO.getId()).
+ findFirst().
+ orElseThrow(() -> new CloudRuntimeException(String.format("Unable to map any snapshot to volume [%s].", volumeVO)));
+
+ if (volumeVO.getSize() == snapshotRef.getSize()) {
+ logger.debug("No need to update the volume size and launch a resize event as the snapshot [{}] and volume [{}] size are equal.", snapshotRef.getSnapshotId(), volumeVO.getUuid());
+ return;
+ }
+
+ resourceLimitManager.decrementResourceCount(volumeVO.getAccountId(), Resource.ResourceType.primary_storage, volumeVO.getSize() - snapshotRef.getSize());
+ volumeVO.setSize(snapshotRef.getSize());
+ volumeObjectTO.setSize(snapshotRef.getSize());
+ volumeDao.update(volumeVO.getId(), volumeVO);
+ UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_RESIZE, volumeVO.getAccountId(), volumeVO.getDataCenterId(), volumeVO.getId(), volumeVO.getName(),
+ volumeVO.getDiskOfferingId(), volumeVO.getTemplateId(), volumeVO.getSize(), Volume.class.getName(), volumeVO.getUuid());
+ }
+
+ private void mergeOldSiblingWithOldParentIfOldParentIsDead(VMSnapshotVO oldParent, UserVmVO userVm, Long hostId, List<VolumeObjectTO> volumeTOs) {
+ if (oldParent == null || oldParent.getRemoved() != null || !VMSnapshot.State.Hidden.equals(oldParent.getState())) {
+ return;
+ }
+
+ List<SnapshotVO> snapshotVos;
+
+ if (oldParent.getCurrent()) {
+ snapshotVos = mergeCurrentDeltaOnSnapshot(oldParent, userVm, hostId, volumeTOs);
+ } else {
+ List<VMSnapshotVO> oldSiblings = vmSnapshotDao.listByParent(oldParent.getId());
+
+ if (oldSiblings.size() > 1) {
+ logger.debug("The old snapshot [{}] is dead and still has more than one live snapshot. We will keep it on storage still.", oldParent.getUuid());
+ return;
+ }
+
+ if (oldSiblings.isEmpty()) {
+ logger.warn("The old snapshot [{}] is dead, but it only had one child. This is an inconsistency and should be analysed/reported.", oldParent.getUuid());
+ return;
+ }
+
+ VMSnapshotVO oldSibling = oldSiblings.get(0);
+ logger.debug("Merging VM snapshot [{}] with [{}] as the former was hidden and only the latter depends on it.", oldParent.getUuid(), oldSibling.getUuid());
+
+ snapshotVos = mergeSnapshots(oldParent, oldSibling, userVm, volumeTOs, hostId);
+ }
+
+ for (SnapshotVO snapshotVO : snapshotVos) {
+ snapshotVO.setState(Snapshot.State.Destroyed);
+ snapshotDao.update(snapshotVO.getId(), snapshotVO);
+ }
+
+ vmSnapshotDetailsDao.removeDetails(oldParent.getId());
+
+ oldParent.setRemoved(DateUtil.now());
+ vmSnapshotDao.update(oldParent.getId(), oldParent);
+
+ transitStateWithoutThrow(oldParent, VMSnapshot.Event.ExpungeRequested);
+ transitStateWithoutThrow(oldParent, VMSnapshot.Event.OperationSucceeded);
+ }
+
+ @Override
+ public StrategyPriority canHandle(VMSnapshot vmSnapshot) {
+ if (!VMSnapshot.State.Allocated.equals(vmSnapshot.getState())) {
+ List<VMSnapshotDetailsVO> vmSnapshotDetails = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), KVM_FILE_BASED_STORAGE_SNAPSHOT);
+ if (CollectionUtils.isEmpty(vmSnapshotDetails)) {
+ logger.debug("KVM file based storage VM snapshot strategy cannot handle [{}] as it is not a KVM file based storage VM snapshot.",
+ vmSnapshot.getUuid());
+ return StrategyPriority.CANT_HANDLE;
+ }
+ return StrategyPriority.HIGHEST;
+ }
+
+ long vmId = vmSnapshot.getVmId();
+ boolean memorySnapshot = VMSnapshot.Type.DiskAndMemory.equals(vmSnapshot.getType());
+ return canHandle(vmId, null, memorySnapshot);
+ }
+
+ @Override
+ public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMemory) {
+ VirtualMachine vm = userVmDao.findById(vmId);
+
+ String cantHandleLog = String.format("KVM file based storage VM snapshot strategy cannot handle VM snapshot for [%s]", vm);
+ if (snapshotMemory) {
+ logger.debug("{} as a snapshot with memory was requested.", cantHandleLog);
+ return StrategyPriority.CANT_HANDLE;
+ }
+
+ if (!Hypervisor.HypervisorType.KVM.equals(vm.getHypervisorType())) {
+ logger.debug("{} as the hypervisor is not KVM.", cantHandleLog);
+ return StrategyPriority.CANT_HANDLE;
+ }
+
+ if (CollectionUtils.isNotEmpty(vmSnapshotDao.findByVmAndByType(vmId, VMSnapshot.Type.DiskAndMemory))) {
+ logger.debug("{} as there is already a VM snapshot with disk and memory.", cantHandleLog);
+ return StrategyPriority.CANT_HANDLE;
+ }
+
+ List<VolumeVO> volumes = volumeDao.findByInstance(vmId);
+ for (VolumeVO volume : volumes) {
+ StoragePoolVO storagePoolVO = storagePool.findById(volume.getPoolId());
+ if (!supportedStoragePoolTypes.contains(storagePoolVO.getPoolType())) {
+ logger.debug("{} as the VM has a volume that is in a storage with unsupported type [{}].", cantHandleLog, storagePoolVO.getPoolType());
+ return StrategyPriority.CANT_HANDLE;
+ }
+ }
+
+ return StrategyPriority.HIGHEST;
+ }
+
+ private List<SnapshotVO> deleteSnapshot(VMSnapshotVO vmSnapshotVO, Long hostId) {
+ List<SnapshotDataStoreVO> volumeSnapshots = getVolumeSnapshotsAssociatedWithVmSnapshot(vmSnapshotVO);
+ List<DataTO> volumeSnapshotTOList = volumeSnapshots.stream()
+ .map(snapshotDataStoreVO -> snapshotDataFactory.getSnapshot(snapshotDataStoreVO.getSnapshotId(), snapshotDataStoreVO.getDataStoreId(), DataStoreRole.Primary).getTO())
+ .collect(Collectors.toList());
+
+ DeleteDiskOnlyVmSnapshotCommand deleteSnapshotCommand = new DeleteDiskOnlyVmSnapshotCommand(volumeSnapshotTOList);
+ Answer answer = agentMgr.easySend(hostId, deleteSnapshotCommand);
+ if (answer == null || !answer.getResult()) {
+ logger.error("Failed to delete VM snapshot [{}] due to {}.", vmSnapshotVO.getUuid(), answer != null ? answer.getDetails() : "Communication failure");
+ throw new CloudRuntimeException(String.format("Failed to delete VM snapshot [%s].", vmSnapshotVO.getUuid()));
+ }
+
+ logger.debug("Updating metadata of VM snapshot [{}].", vmSnapshotVO.getUuid());
+ List<SnapshotVO> snapshotVOList = new ArrayList<>();
+ for (SnapshotDataStoreVO snapshotDataStoreVO : volumeSnapshots) {
+ snapshotDataStoreDao.remove(snapshotDataStoreVO.getId());
+ snapshotVOList.add(snapshotDao.findById(snapshotDataStoreVO.getSnapshotId()));
+ }
+ return snapshotVOList;
+ }
+
+ private List<SnapshotVO> mergeSnapshots(VMSnapshotVO vmSnapshotVO, VMSnapshotVO childSnapshot, UserVmVO userVm, List<VolumeObjectTO> volumeObjectTOS, Long hostId) {
+ logger.debug("Merging VM snapshot [{}] with its child [{}].", vmSnapshotVO.getUuid(), childSnapshot.getUuid());
+
+ List<VMSnapshotVO> snapshotGrandChildren = vmSnapshotDao.listByParent(childSnapshot.getId());
+
+ if (userVm.getState().equals(VirtualMachine.State.Running) && !snapshotGrandChildren.isEmpty()) {
+ logger.debug("Removing VM snapshots that are part of the VM's [{}] current backing chain from the list of snapshots to be rebased.", userVm.getUuid());
+ removeCurrentBackingChainSnapshotFromVmSnapshotList(snapshotGrandChildren, userVm);
+ }
+
+ List<SnapshotMergeTreeTO> snapshotMergeTreeToList = generateSnapshotMergeTrees(vmSnapshotVO, childSnapshot, snapshotGrandChildren);
+
+ if (childSnapshot.getCurrent() && !userVm.getState().equals(VirtualMachine.State.Running)) {
+ for (VolumeObjectTO volumeObjectTO : volumeObjectTOS) {
+ snapshotMergeTreeToList.stream().filter(snapshotTree -> Objects.equals(((SnapshotObjectTO) snapshotTree.getParent()).getVolume().getId(), volumeObjectTO.getId()))
+ .findFirst()
+ .orElseThrow(() -> new CloudRuntimeException(String.format("Failed to find volume snapshot for volume [%s].", volumeObjectTO.getUuid())))
+ .addGrandChild(volumeObjectTO);
+ }
+ }
+
+ MergeDiskOnlyVmSnapshotCommand mergeDiskOnlyVMSnapshotCommand = new MergeDiskOnlyVmSnapshotCommand(snapshotMergeTreeToList, userVm.getState(), userVm.getName());
+ Answer answer = agentMgr.easySend(hostId, mergeDiskOnlyVMSnapshotCommand);
+ if (answer == null || !answer.getResult()) {
+ throw new CloudRuntimeException(String.format("Failed to merge VM snapshot [%s] due to %s.", vmSnapshotVO.getUuid(), answer != null ? answer.getDetails() : "Communication failure"));
+ }
+
+ logger.debug("Updating metadata of VM snapshot [{}] and its child [{}].", vmSnapshotVO.getUuid(), childSnapshot.getUuid());
+ List<SnapshotVO> snapshotVOList = new ArrayList<>();
+ for (SnapshotMergeTreeTO snapshotMergeTreeTO : snapshotMergeTreeToList) {
+ SnapshotObjectTO childTO = (SnapshotObjectTO) snapshotMergeTreeTO.getChild();
+ SnapshotObjectTO parentTO = (SnapshotObjectTO) snapshotMergeTreeTO.getParent();
+
+ SnapshotDataStoreVO childSnapshotDataStoreVO = snapshotDataStoreDao.findBySnapshotIdInAnyState(childTO.getId(), DataStoreRole.Primary);
+ childSnapshotDataStoreVO.setInstallPath(parentTO.getPath());
+ snapshotDataStoreDao.update(childSnapshotDataStoreVO.getId(), childSnapshotDataStoreVO);
+
+ snapshotDataStoreDao.expungeReferenceBySnapshotIdAndDataStoreRole(parentTO.getId(), childSnapshotDataStoreVO.getDataStoreId(), DataStoreRole.Primary);
+ snapshotVOList.add(snapshotDao.findById(parentTO.getId()));
+ }
+
+ childSnapshot.setParent(vmSnapshotVO.getParent());
+ vmSnapshotDao.update(childSnapshot.getId(), childSnapshot);
+
+ return snapshotVOList;
+ }
+
+ private List<SnapshotVO> mergeCurrentDeltaOnSnapshot(VMSnapshotVO vmSnapshotVo, UserVmVO userVmVO, Long hostId, List<VolumeObjectTO> volumeObjectTOS) {
+ logger.debug("Merging VM snapshot [{}] with the current volume delta.", vmSnapshotVo.getUuid());
+ List<SnapshotMergeTreeTO> snapshotMergeTreeTOList = new ArrayList<>();
+ List<SnapshotDataStoreVO> volumeSnapshots = getVolumeSnapshotsAssociatedWithVmSnapshot(vmSnapshotVo);
+
+ for (VolumeObjectTO volumeObjectTO : volumeObjectTOS) {
+ SnapshotDataStoreVO volumeParentSnapshot = volumeSnapshots.stream().filter(snapshot -> Objects.equals(snapshot.getVolumeId(), volumeObjectTO.getId()))
+ .findFirst()
+ .orElseThrow(() -> new CloudRuntimeException(String.format("Failed to find volume snapshot for volume [%s].", volumeObjectTO.getUuid())));
+ DataTO parentSnapshot = snapshotDataFactory.getSnapshot(volumeParentSnapshot.getSnapshotId(), volumeParentSnapshot.getDataStoreId(), DataStoreRole.Primary).getTO();
+ snapshotMergeTreeTOList.add(new SnapshotMergeTreeTO(parentSnapshot, volumeObjectTO, new ArrayList<>()));
+ }
+
+ MergeDiskOnlyVmSnapshotCommand mergeDiskOnlyVMSnapshotCommand = new MergeDiskOnlyVmSnapshotCommand(snapshotMergeTreeTOList, userVmVO.getState(), userVmVO.getName());
+
+ Answer answer = agentMgr.easySend(hostId, mergeDiskOnlyVMSnapshotCommand);
+ if (answer == null || !answer.getResult()) {
+ throw new CloudRuntimeException(String.format("Failed to delete VM snapshot [%s] due to %s.", vmSnapshotVo.getUuid(), answer != null ? answer.getDetails() : "Communication failure"));
+ }
+
+ logger.debug("Updating metadata of VM snapshot [{}].", vmSnapshotVo.getUuid());
+ List<SnapshotVO> snapshotVOList = new ArrayList<>();
+ for (SnapshotMergeTreeTO snapshotMergeTreeTO : snapshotMergeTreeTOList) {
+ VolumeObjectTO volumeObjectTO = (VolumeObjectTO) snapshotMergeTreeTO.getChild();
+ SnapshotObjectTO parentTO = (SnapshotObjectTO) snapshotMergeTreeTO.getParent();
+
+ VolumeVO volumeVO = volumeDao.findById(volumeObjectTO.getId());
+ volumeVO.setPath(parentTO.getPath());
+ volumeDao.update(volumeVO.getId(), volumeVO);
+
+ snapshotDataStoreDao.expungeReferenceBySnapshotIdAndDataStoreRole(parentTO.getId(), volumeVO.getPoolId(), DataStoreRole.Primary);
+ snapshotVOList.add(snapshotDao.findById(parentTO.getId()));
+ }
+
+ vmSnapshotVo.setCurrent(false);
+ if (vmSnapshotVo.getParent() != null) {
+ VMSnapshotVO parentSnapshot = vmSnapshotDao.findById(vmSnapshotVo.getParent());
+ parentSnapshot.setCurrent(true);
+ vmSnapshotDao.update(parentSnapshot.getId(), parentSnapshot);
+ }
+
+ return snapshotVOList;
+ }
+
+ /**
+ * Takes a disk-only VM snapshot, exceptions thrown will be caught deeper in the stack and treated there.
+ * @param vmSnapshot the definition of the VM Snapshot that will be created.
+ * @param volumeInfoToSnapshotObjectMap Empty map of VolumeInfo to SnapshotObject, will be populated within the method, used for treating the exceptions thrown.
+ * @return the VM Snapshot created.
+ * */
+ protected VMSnapshot takeVmSnapshotInternal(VMSnapshot vmSnapshot, Map<VolumeInfo, SnapshotObject> volumeInfoToSnapshotObjectMap) throws NoTransitionException {
+ UserVm userVm = userVmDao.findById(vmSnapshot.getVmId());
+
+ logger.info("Starting disk-only VM snapshot process for VM [{}].", userVm.getUuid());
+
+ Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshot.getVmId());
+ VMSnapshotVO vmSnapshotVO = (VMSnapshotVO) vmSnapshot;
+ List<VolumeObjectTO> volumeTOs = vmSnapshotHelper.getVolumeTOList(userVm.getId());
+
+ transitStateWithoutThrow(vmSnapshot, VMSnapshot.Event.CreateRequested);
+
+ VMSnapshotTO parentSnapshotTo = null;
+ VMSnapshotVO parentSnapshotVo = vmSnapshotDao.findCurrentSnapshotByVmId(userVm.getId());
+ if (parentSnapshotVo != null) {
+ parentSnapshotTo = vmSnapshotHelper.getSnapshotWithParents(parentSnapshotVo);
+ vmSnapshotVO.setParent(parentSnapshotTo.getId());
+ }
+
+ VMSnapshotOptions options = ((VMSnapshotVO) vmSnapshot).getOptions();
+ boolean quiesceVm = false;
+ if (options != null) {
+ quiesceVm = options.needQuiesceVM();
+ }
+
+ long virtualSize = createVolumeSnapshotMetadataAndCalculateVirtualSize(vmSnapshot, volumeInfoToSnapshotObjectMap, volumeTOs);
+
+ VMSnapshotTO target = new VMSnapshotTO(vmSnapshot.getId(), vmSnapshot.getName(), vmSnapshot.getType(), null, vmSnapshot.getDescription(), false, parentSnapshotTo, quiesceVm);
+
+ CreateDiskOnlyVmSnapshotCommand ccmd = new CreateDiskOnlyVmSnapshotCommand(userVm.getInstanceName(), target, volumeTOs, null, userVm.getState());
+
+ logger.info("Sending disk-only VM snapshot creation of VM Snapshot [{}] command for host [{}].", vmSnapshot.getUuid(), hostId);
+ Answer answer = agentMgr.easySend(hostId, ccmd);
+
+ if (answer != null && answer.getResult()) {
+ CreateDiskOnlyVmSnapshotAnswer createDiskOnlyVMSnapshotAnswer = (CreateDiskOnlyVmSnapshotAnswer) answer;
+ return processCreateVmSnapshotAnswer(vmSnapshot, volumeInfoToSnapshotObjectMap, createDiskOnlyVMSnapshotAnswer, userVm, vmSnapshotVO, virtualSize, parentSnapshotVo);
+ }
+
+ logger.error("Disk-only VM snapshot for VM [{}] failed{}.", userVm.getUuid(), answer != null ? " due to" + answer.getDetails() : "");
+ throw new CloudRuntimeException(String.format("Disk-only VM snapshot for VM [%s] failed.", userVm.getUuid()));
+ }
+
+ /**
+ * Updates the needed metadata of the given VM Snapshot and its associated volume snapshots.
+ * */
+ private VMSnapshot processCreateVmSnapshotAnswer(VMSnapshot vmSnapshot, Map<VolumeInfo, SnapshotObject> volumeInfoToSnapshotObjectMap, CreateDiskOnlyVmSnapshotAnswer answer, UserVm userVm, VMSnapshotVO vmSnapshotVO, long virtualSize, VMSnapshotVO parentSnapshotVo) throws NoTransitionException {
+ logger.debug("Processing CreateDiskOnlyVMSnapshotCommand answer for disk-only VM snapshot [{}].", vmSnapshot.getUuid());
+ Map<String, Pair<Long, String>> volumeUuidToSnapshotSizeAndNewVolumePathMap = answer.getMapVolumeToSnapshotSizeAndNewVolumePath();
+ long vmSnapshotSize = 0;
+
+ for (VolumeInfo volumeInfo : volumeInfoToSnapshotObjectMap.keySet()) {
+ VolumeVO volumeVO = (VolumeVO) volumeInfo.getVolume();
+ Pair<Long, String> snapSizeAndNewVolumePath = volumeUuidToSnapshotSizeAndNewVolumePathMap.get(volumeVO.getUuid());
+
+ SnapshotObject snapshot = volumeInfoToSnapshotObjectMap.get(volumeInfo);
+ snapshot.markBackedUp();
+
+ logger.debug("Updating metadata for volume [{}] and its corresponding snapshot [{}].", volumeVO, snapshot.getSnapshotVO());
+
+ SnapshotDataStoreVO snapshotDataStoreVO = snapshotDataStoreDao.findBySnapshotId(snapshot.getId()).get(0);
+ snapshotDataStoreVO.setInstallPath(volumeVO.getPath());
+ snapshotDataStoreVO.setPhysicalSize(snapSizeAndNewVolumePath.first());
+ snapshotDataStoreVO.setState(ObjectInDataStoreStateMachine.State.Ready);
+ snapshotDataStoreDao.update(snapshotDataStoreVO.getId(), snapshotDataStoreVO);
+
+ vmSnapshotSize += snapSizeAndNewVolumePath.first();
+
+ volumeVO.setPath(snapSizeAndNewVolumePath.second());
+ volumeDao.update(volumeVO.getId(), volumeVO);
+ volumeInfo.stateTransit(Volume.Event.OperationSucceeded);
+
+ vmSnapshotDetailsDao.persist(new VMSnapshotDetailsVO(vmSnapshot.getId(), KVM_FILE_BASED_STORAGE_SNAPSHOT, String.valueOf(snapshot.getId()), true));
+
+ publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_CREATE, vmSnapshot, userVm, (VolumeObjectTO) volumeInfo.getTO());
+ }
+
+ vmSnapshotVO.setCurrent(true);
+ vmSnapshotDao.persist(vmSnapshotVO);
+
+ if (parentSnapshotVo != null) {
+ parentSnapshotVo.setCurrent(false);
+ vmSnapshotDao.persist(parentSnapshotVo);
+ }
+
+ vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationSucceeded);
+
+ publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_ON_PRIMARY, vmSnapshot, userVm, vmSnapshotSize, virtualSize);
+
+ return vmSnapshot;
+ }
+
+ private long createVolumeSnapshotMetadataAndCalculateVirtualSize(VMSnapshot vmSnapshot, Map<VolumeInfo, SnapshotObject> volumeInfoToSnapshotObjectMap, List<VolumeObjectTO> volumeTOs) throws NoTransitionException {
+ long virtualSize = 0;
+ for (VolumeObjectTO volumeObjectTO : volumeTOs) {
+ VolumeInfo volumeInfo = volumeDataFactory.getVolume(volumeObjectTO.getId());
+ volumeInfo.stateTransit(Volume.Event.SnapshotRequested);
+ virtualSize += volumeInfo.getSize();
+
+ String snapshotName = String.format("%s_%s", vmSnapshot.getId(), volumeObjectTO.getUuid());
+ SnapshotVO snapshot = new SnapshotVO(volumeInfo.getDataCenterId(), volumeInfo.getAccountId(), volumeInfo.getDomainId(), volumeInfo.getId(),
+ volumeInfo.getDiskOfferingId(), snapshotName, (short) Snapshot.Type.GROUP.ordinal(), Snapshot.Type.GROUP.name(), volumeInfo.getSize(), volumeInfo.getMinIops(),
+ volumeInfo.getMaxIops(), Hypervisor.HypervisorType.KVM, null);
+
+ logger.debug("Creating snapshot metadata [{}] as part of the disk-only snapshot process for VM [{}].", snapshot, volumeObjectTO.getVmName());
+
+ snapshot = snapshotDao.persist(snapshot);
+
+ SnapshotInfo snapshotInfo = snapshotDataFactory.getSnapshot(snapshot.getId(), volumeInfo.getDataStore());
+ SnapshotObject snapshotOnPrimary = (SnapshotObject) snapshotInfo.getDataStore().create(snapshotInfo);
+
+ snapshotOnPrimary.processEvent(Snapshot.Event.CreateRequested);
+ snapshotOnPrimary.processEvent(ObjectInDataStoreStateMachine.Event.CreateOnlyRequested);
+
+ volumeInfoToSnapshotObjectMap.put(volumeInfo, snapshotOnPrimary);
+ }
+ return virtualSize;
+ }
+
+ private List<SnapshotMergeTreeTO> generateSnapshotMergeTrees(VMSnapshotVO parent, VMSnapshotVO child, List<VMSnapshotVO> grandChildren) throws NoSuchElementException {
+ logger.debug("Generating list of Snapshot Merge Trees for the merge process of VM Snapshot [{}].", parent.getUuid());
+
+ List<SnapshotMergeTreeTO> snapshotMergeTrees = new ArrayList<>();
+ List<SnapshotDataStoreVO> parentVolumeSnapshots = getVolumeSnapshotsAssociatedWithVmSnapshot(parent);
+ List<SnapshotDataStoreVO> childVolumeSnapshots = getVolumeSnapshotsAssociatedWithVmSnapshot(child);
+ List<SnapshotDataStoreVO> grandChildrenVolumeSnapshots = new ArrayList<>();
+
+ for (VMSnapshotVO grandChild : grandChildren) {
+ grandChildrenVolumeSnapshots.addAll(getVolumeSnapshotsAssociatedWithVmSnapshot(grandChild));
+ }
+
+ for (SnapshotDataStoreVO parentSnapshotDataStoreVO : parentVolumeSnapshots) {
+ DataTO parentTO = snapshotDataFactory.getSnapshot(parentSnapshotDataStoreVO.getSnapshotId(), parentSnapshotDataStoreVO.getDataStoreId(), DataStoreRole.Primary).getTO();
+
+ DataTO childTO = childVolumeSnapshots.stream()
+ .filter(childSnapshot -> Objects.equals(parentSnapshotDataStoreVO.getVolumeId(), childSnapshot.getVolumeId()))
+ .map(snapshotDataStoreVO -> snapshotDataFactory.getSnapshot(snapshotDataStoreVO.getSnapshotId(), snapshotDataStoreVO.getDataStoreId(), DataStoreRole.Primary).getTO())
+ .findFirst().orElseThrow(() -> new CloudRuntimeException(String.format("Could not find child snapshot of parent [%s].", parentSnapshotDataStoreVO.getSnapshotId())));
+
+ List<DataTO> grandChildrenTOList = grandChildrenVolumeSnapshots.stream()
+ .filter(grandChildSnapshot -> Objects.equals(parentSnapshotDataStoreVO.getVolumeId(), grandChildSnapshot.getVolumeId()))
+ .map(snapshotDataStoreVO -> snapshotDataFactory.getSnapshot(snapshotDataStoreVO.getSnapshotId(), snapshotDataStoreVO.getDataStoreId(), DataStoreRole.Primary).getTO())
+ .collect(Collectors.toList());
+
+ snapshotMergeTrees.add(new SnapshotMergeTreeTO(parentTO, childTO, grandChildrenTOList));
+ }
+
+ logger.debug("Generated the following list of Snapshot Merge Trees for the VM snapshot [{}]: [{}].", parent.getUuid(), snapshotMergeTrees);
+ return snapshotMergeTrees;
+ }
+
+ /**
+ * For a given {@code VMSnapshotVO}, populates the {@code associatedVolumeSnapshots} list with all the volume snapshots that are
+ * part of the VMSnapshot.
+ * @param vmSnapshot the VMSnapshotVO that will have its size calculated
+ * @return the list that will be populated with the volume snapshots associated with the VM snapshot.
+ * */
+ private List<SnapshotDataStoreVO> getVolumeSnapshotsAssociatedWithVmSnapshot(VMSnapshotVO vmSnapshot) {
+ List<SnapshotDataStoreVO> associatedVolumeSnapshots = new ArrayList<>();
+ List<VMSnapshotDetailsVO> snapshotDetailList = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), KVM_FILE_BASED_STORAGE_SNAPSHOT);
+ for (VMSnapshotDetailsVO vmSnapshotDetailsVO : snapshotDetailList) {
+ SnapshotDataStoreVO snapshot = snapshotDataStoreDao.findOneBySnapshotAndDatastoreRole(Long.parseLong(vmSnapshotDetailsVO.getValue()), DataStoreRole.Primary);
+ if (snapshot == null) {
+ throw new CloudRuntimeException(String.format("Could not find snapshot for VM snapshot [%s].", vmSnapshot.getUuid()));
+ }
+ associatedVolumeSnapshots.add(snapshot);
+ }
+ return associatedVolumeSnapshots;
+ }
+
+ /**
+ * For a given {@code VMSnapshotVO}, returns the real size of the snapshot.
+ * @param vmSnapshot the VMSnapshotVO that will have its size calculated
+ * */
+ private long getVMSnapshotRealSize(VMSnapshotVO vmSnapshot) {
+ long realSize = 0;
+ List<VMSnapshotDetailsVO> snapshotDetailList = vmSnapshotDetailsDao.findDetails(vmSnapshot.getId(), KVM_FILE_BASED_STORAGE_SNAPSHOT);
+ for (VMSnapshotDetailsVO vmSnapshotDetailsVO : snapshotDetailList) {
+ SnapshotDataStoreVO snapshot = snapshotDataStoreDao.findOneBySnapshotAndDatastoreRole(Long.parseLong(vmSnapshotDetailsVO.getValue()), DataStoreRole.Primary);
+ if (snapshot == null) {
+ throw new CloudRuntimeException(String.format("Could not find snapshot for VM snapshot [%s].", vmSnapshot.getUuid()));
+ }
+ realSize += snapshot.getPhysicalSize();
+ }
+ return realSize;
+ }
+
+ /**
+ * Given a list of VM snapshots, will remove any that are part of the current direct backing chain (all the direct ancestors of the current vm snapshot).
+ * This is done because, when using <a href="https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainBlockCommit">virDomainBlockCommit</a>}, Libvirt will maintain
+ * the current backing chain consistent; thus we only need to rebase the snapshots that are not on the current backing chain.
+ * */
+ private void removeCurrentBackingChainSnapshotFromVmSnapshotList(List<VMSnapshotVO> vmSnapshotList, UserVm userVm) {
+ VMSnapshotVO currentSnapshotVO = vmSnapshotDao.findCurrentSnapshotByVmId(vmSnapshotList.get(0).getVmId());
+ VMSnapshotTO currentSnapshotTO = vmSnapshotHelper.getSnapshotWithParents(currentSnapshotVO);
+
+ List<VMSnapshotTO> currentBranch = new ArrayList<>();
+ currentBranch.add(currentSnapshotTO);
+ VMSnapshotTO parent = currentSnapshotTO.getParent();
+ while (parent != null) {
+ currentBranch.add(parent);
+ parent = parent.getParent();
+ }
+
+ for (VMSnapshotVO vmSnapshotVO : vmSnapshotList) {
+ if (currentBranch.stream().anyMatch(currentBranchSnap -> Objects.equals(currentBranchSnap.getId(), vmSnapshotVO.getId()))) {
+ logger.trace("Removing snapshot [{}] from the list of VM snapshots of VM [{}] being rebased.", vmSnapshotVO.getUuid(), userVm.getUuid());
+ vmSnapshotList.remove(vmSnapshotVO);
+ return;
+ }
+ }
+ }
+
+ private void transitStateWithoutThrow(VMSnapshot vmSnapshot, VMSnapshot.Event event) {
+ try {
+ vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, event);
+ } catch (NoTransitionException e) {
+ String msg = String.format("Failed to change VM snapshot [%s] state with event [%s].", vmSnapshot, event.toString());
+ logger.error(msg, e);
+ throw new CloudRuntimeException(msg, e);
+ }
+ }
+}
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java
index ec73246851cd..6081bd3a1531 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/StorageVMSnapshotStrategy.java
@@ -95,8 +95,6 @@ public class StorageVMSnapshotStrategy extends DefaultVMSnapshotStrategy {
@Inject
VMSnapshotDetailsDao vmSnapshotDetailsDao;
- private static final String STORAGE_SNAPSHOT = "kvmStorageSnapshot";
-
@Override
public boolean configure(String name, Map<String, Object> params) throws ConfigurationException {
return super.configure(name, params);
@@ -356,12 +354,25 @@ public StrategyPriority canHandle(VMSnapshot vmSnapshot) {
@Override
public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMemory) {
+ UserVmVO vm = userVmDao.findById(vmId);
+ String cantHandleLog = String.format("Storage VM snapshot strategy cannot handle VM snapshot for [%s]", vm);
+
+ if (CollectionUtils.isNotEmpty(vmSnapshotDao.findByVmAndByType(vmId, VMSnapshot.Type.DiskAndMemory))) {
+ logger.debug("{} as it has VM snapshots with disk and memory.", cantHandleLog);
+ return StrategyPriority.CANT_HANDLE;
+ }
+
+ if (!VirtualMachine.State.Running.equals(vm.getState())) {
+ logger.debug("{} as the VM is not running.", cantHandleLog);
+ return StrategyPriority.CANT_HANDLE;
+ }
+
if (SnapshotManager.VmStorageSnapshotKvm.value() && !snapshotMemory) {
- UserVmVO vm = userVmDao.findById(vmId);
- if (vm.getState() == VirtualMachine.State.Running) {
- return StrategyPriority.HYPERVISOR;
- }
+ return StrategyPriority.HYPERVISOR;
}
+
+ logger.debug("{} as {}.", () -> cantHandleLog, () -> snapshotMemory ? "A VM snapshot with memory was requested" :
+ String.format("%s is false", SnapshotManager.VmStorageSnapshotKvm.key()));
return StrategyPriority.CANT_HANDLE;
}
diff --git a/engine/storage/snapshot/src/main/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml b/engine/storage/snapshot/src/main/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml
index fc561159c8ea..32ac6e9fc8b7 100644
--- a/engine/storage/snapshot/src/main/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml
+++ b/engine/storage/snapshot/src/main/resources/META-INF/cloudstack/storage/spring-engine-storage-snapshot-storage-context.xml
@@ -48,4 +48,7 @@
<bean id="ScaleIOVMSnapshotStrategy"
class="org.apache.cloudstack.storage.vmsnapshot.ScaleIOVMSnapshotStrategy" />
+ <bean id="KvmFileBasedStorageVmSnapshotStrategy"
+ class="org.apache.cloudstack.storage.vmsnapshot.KvmFileBasedStorageVmSnapshotStrategy" />
+
</beans>
diff --git a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyTest.java b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyTest.java
index 7bcfd4dda581..829ca5ade39b 100644
--- a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyTest.java
+++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyTest.java
@@ -25,6 +25,7 @@
import javax.inject.Inject;
+import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
import org.apache.cloudstack.engine.subsystem.api.storage.VMSnapshotStrategy;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
@@ -283,6 +284,11 @@ public VMSnapshotDao vmSnapshotDao() {
return Mockito.mock(VMSnapshotDao.class);
}
+ @Bean
+ public VMSnapshotDetailsDao vmSnapshotDetailsDao() {
+ return Mockito.mock(VMSnapshotDetailsDao.class);
+ }
+
@Bean
public ConfigurationDao configurationDao() {
return Mockito.mock(ConfigurationDao.class);
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BlockCommitListener.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BlockCommitListener.java
new file mode 100644
index 000000000000..d360aa481372
--- /dev/null
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BlockCommitListener.java
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package com.cloud.hypervisor.kvm.resource;
+
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.ThreadContext;
+import org.libvirt.Domain;
+import org.libvirt.LibvirtException;
+import org.libvirt.event.BlockJobListener;
+import org.libvirt.event.BlockJobStatus;
+import org.libvirt.event.BlockJobType;
+
+import java.util.concurrent.Semaphore;
+
+public class BlockCommitListener implements BlockJobListener {
+ private Semaphore semaphore;
+ private String result;
+ private String vmName;
+
+ private Logger logger;
+ private String logid;
+
+ protected BlockCommitListener(Semaphore semaphore, String vmName, String logid) {
+ this.semaphore = semaphore;
+ this.vmName = vmName;
+ this.logid = logid;
+ logger = LogManager.getLogger(getClass());
+ }
+
+ protected String getResult() {
+ return result;
+ }
+
+ @Override
+ public void onEvent(Domain domain, String diskPath, BlockJobType type, BlockJobStatus status) {
+ if (!BlockJobType.COMMIT.equals(type) && !BlockJobType.ACTIVE_COMMIT.equals(type)) {
+ return;
+ }
+
+ switch (status) {
+ case COMPLETED:
+ result = null;
+ semaphore.release();
+ return;
+ case READY:
+ try {
+ ThreadContext.put("logcontextid", logid);
+ logger.debug("Pivoting disk [{}] of VM [{}].", diskPath, vmName);
+ domain.blockJobAbort(diskPath, Domain.BlockJobAbortFlags.PIVOT);
+ } catch (LibvirtException ex) {
+ result = String.format("Failed to pivot disk due to [%s].", ex.getMessage());
+ semaphore.release();
+ }
+ return;
+ default:
+ result = String.format("Failed to block commit disk with status [%s].", status);
+ semaphore.release();
+ }
+ }
+}
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
index 7997983a3673..794db4b24517 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
@@ -29,6 +29,7 @@
import java.net.NetworkInterface;
import java.net.URI;
import java.net.URISyntaxException;
+import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
@@ -43,6 +44,8 @@
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Semaphore;
+import java.util.concurrent.TimeUnit;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
@@ -83,10 +86,12 @@
import org.apache.commons.lang3.builder.ToStringStyle;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.ThreadContext;
import org.apache.xerces.impl.xpath.regex.Match;
import org.joda.time.Duration;
import org.libvirt.Connect;
import org.libvirt.Domain;
+import org.libvirt.DomainBlockJobInfo;
import org.libvirt.DomainBlockStats;
import org.libvirt.DomainInfo;
import org.libvirt.DomainInfo.DomainState;
@@ -341,6 +346,8 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
public static final int LIBVIRT_CGROUPV2_WEIGHT_MIN = 2;
public static final int LIBVIRT_CGROUPV2_WEIGHT_MAX = 10000;
+ protected int snapshotMergeTimeout;
+
private String modifyVlanPath;
private String versionStringPath;
private String patchScriptPath;
@@ -526,6 +533,15 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
public static final String CGROUP_V2 = "cgroup2fs";
+ /**
+ * Virsh command to merge (blockcommit) snapshot into the base file.<br><br>
+ * 1st parameter: VM's name;<br>
+ * 2nd parameter: disk's label (target.dev tag from VM's XML);<br>
+ * 3rd parameter: the absolute path of the base file;
+ * 4th parameter: the flag '--delete', if Libvirt supports it. Libvirt started to support it on version <b>6.0.0</b>;
+ */
+ private static final String COMMAND_MERGE_SNAPSHOT = "virsh blockcommit %s %s --base %s --active --wait %s --pivot";
+
protected long getHypervisorLibvirtVersion() {
return hypervisorLibvirtVersion;
}
@@ -1141,6 +1157,8 @@ public boolean configure(final String name, final Map<String, Object> params) th
cmdsTimeout = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.CMDS_TIMEOUT) * 1000;
noMemBalloon = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_DISABLE);
+ snapshotMergeTimeout = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.SNAPSHOT_MERGE_TIMEOUT);
+ snapshotMergeTimeout = snapshotMergeTimeout > 0 ? snapshotMergeTimeout : AgentProperties.SNAPSHOT_MERGE_TIMEOUT.getDefaultValue();
manualCpuSpeed = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HOST_CPU_MANUAL_SPEED_MHZ);
@@ -4494,6 +4512,13 @@ public List<VmDiskStatsEntry> getVmDiskStat(final Connect conn, final String vmN
}
}
+ public DiskDef getDiskWithPathOfVolumeObjectTO(List<DiskDef> disks, VolumeObjectTO vol) {
+ return disks.stream()
+ .filter(diskDef -> diskDef.getDiskPath() != null && diskDef.getDiskPath().contains(vol.getPath()))
+ .findFirst()
+ .orElseThrow(() -> new CloudRuntimeException(String.format("Unable to find volume [%s].", vol.getUuid())));
+ }
+
protected String getDiskPathFromDiskDef(DiskDef disk) {
final String path = disk.getDiskPath();
if (path != null) {
@@ -5571,10 +5596,213 @@ public void cleanOldSecretsByDiskDef(Connect conn, List<DiskDef> disks) throws L
}
}
+ /**
+ * Retrieves the temporary path of the snapshot.
+ * @param diskPath Path of the disk to snapshot;
+ * @param snapshotName Snapshot name;
+ * @return the path of the disk replacing the disk with the snapshot.
+ */
+ public String getSnapshotTemporaryPath(String diskPath, String snapshotName) {
+ String[] diskPathSplitted = diskPath.split(File.separator);
+ diskPathSplitted[diskPathSplitted.length - 1] = snapshotName;
+ return String.join(File.separator, diskPathSplitted);
+ }
+
public static String generateSecretUUIDFromString(String seed) {
return UUID.nameUUIDFromBytes(seed.getBytes()).toString();
}
+ /**
+ * Merges the snapshot into base file.
+ *
+ * @param vm Domain of the VM;
+ * @param diskLabel Disk label to manage snapshot and base file;
+ * @param baseFilePath Path of the base file;
+ * @param topFilePath Path of the top file, if null, the active image is used;
+ * @param active Whether the snapshot being merged is the active image;
+ * @param snapshotName Name of the snapshot;
+ * @param volume VolumeObjectTO of the corresponding volume;
+ * @param conn Libvirt connection;
+ * @throws LibvirtException
+ */
+ public void mergeSnapshotIntoBaseFile(Domain vm, String diskLabel, String baseFilePath, String topFilePath, boolean active, String snapshotName, VolumeObjectTO volume,
+ Connect conn) throws LibvirtException {
+ if (AgentPropertiesFileHandler.getPropertyValue(AgentProperties.LIBVIRT_EVENTS_ENABLED)) {
+ mergeSnapshotIntoBaseFileWithEventsAndConfigurableTimeout(vm, diskLabel, baseFilePath, topFilePath, active, snapshotName, volume, conn);
+ } else {
+ mergeSnapshotIntoBaseFileWithoutEvents(vm, diskLabel, baseFilePath, snapshotName, volume, conn);
+ }
+ }
+
+ /**
+ * This method only works if LIBVIRT_EVENTS_ENABLED is true.
+ * */
+ protected void mergeSnapshotIntoBaseFileWithEventsAndConfigurableTimeout(Domain vm, String diskLabel, String baseFilePath, String topFilePath, boolean active, String snapshotName, VolumeObjectTO volume,
+ Connect conn) throws LibvirtException {
+ boolean isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit = LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(conn);
+ String vmName = vm.getName();
+
+ int commitFlags = 0;
+ if (isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit) {
+ commitFlags |= Domain.BlockCommitFlags.DELETE;
+ }
+ if (active) {
+ commitFlags |= Domain.BlockCommitFlags.ACTIVE;
+ }
+
+ Semaphore semaphore = getSemaphoreToWaitForMerge();
+ BlockCommitListener blockCommitListener = getBlockCommitListener(semaphore, vmName);
+ vm.addBlockJobListener(blockCommitListener);
+
+ logger.info("Starting block commit of snapshot [{}] of VM [{}]. Using parameters: diskLabel [{}]; baseFilePath [{}]; topFilePath [{}]; commitFlags [{}]", snapshotName,
+ vmName, diskLabel, baseFilePath, topFilePath, commitFlags);
+
+ vm.blockCommit(diskLabel, baseFilePath, topFilePath, 0, commitFlags);
+
+ Thread checkProgressThread = new Thread(() -> checkBlockCommitProgress(vm, diskLabel, vmName, snapshotName, topFilePath, baseFilePath));
+ checkProgressThread.start();
+
+ String errorMessage = String.format("the block commit of top file [%s] into base file [%s] for snapshot [%s] of VM [%s]." +
+ " The job will be left running to avoid data corruption, but ACS will return an error and volume [%s] will need to be normalized manually. If the commit" +
+ " involved the active image, the pivot will need to be manually done.", topFilePath, baseFilePath, snapshotName, vmName, volume);
+ try {
+ if (!semaphore.tryAcquire(snapshotMergeTimeout, TimeUnit.SECONDS)) {
+ throw new CloudRuntimeException("Timed out while waiting for " + errorMessage);
+ }
+ } catch (InterruptedException e) {
+ throw new CloudRuntimeException("Interrupted while waiting for " + errorMessage);
+ } finally {
+ vm.removeBlockJobListener(blockCommitListener);
+ }
+
+ String mergeResult = blockCommitListener.getResult();
+ try {
+ checkProgressThread.join();
+ } catch (InterruptedException ex) {
+ throw new CloudRuntimeException(String.format("Exception while running wait block commit task of snapshot [%s] and VM [%s].", snapshotName, vmName));
+ }
+
+ if (mergeResult != null) {
+ String commitError = String.format("Failed %s The failure occurred due to [%s].", errorMessage, mergeResult);
+ logger.error(commitError);
+ throw new CloudRuntimeException(commitError);
+ }
+
+ logger.info("Completed block commit of snapshot [{}] of VM [{}].", snapshotName, vmName);
+
+ manuallyDeleteUnusedSnapshotFile(isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit, topFilePath != null ? topFilePath : getSnapshotTemporaryPath(baseFilePath, snapshotName));
+ }
+
+ /**
+ * Merges the snapshot into base file to keep volume and VM behavior after stopping - starting.
+ * @param vm Domain of the VM;
+ * @param diskLabel Disk label to manage snapshot and base file;
+ * @param baseFilePath Path of the base file;
+ * @param snapshotName Name of the snapshot;
+ * @throws LibvirtException
+ */
+ protected void mergeSnapshotIntoBaseFileWithoutEvents(Domain vm, String diskLabel, String baseFilePath, String snapshotName, VolumeObjectTO volume, Connect conn) throws LibvirtException {
+ boolean isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit = LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(conn);
+ String vmName = vm.getName();
+ String mergeCommand = String.format(COMMAND_MERGE_SNAPSHOT, vmName, diskLabel, baseFilePath, isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit ? "--delete" : "");
+ String mergeResult = Script.runSimpleBashScript(mergeCommand);
+
+ if (mergeResult == null) {
+ logger.debug("Successfully merged snapshot [{}] into VM [{}] {} base file.", snapshotName, vmName, volume);
+ manuallyDeleteUnusedSnapshotFile(isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit, getSnapshotTemporaryPath(baseFilePath, snapshotName));
+ return;
+ }
+
+ String errorMsg = String.format("Failed to merge snapshot [%s] into VM [%s] %s base file. Command [%s] resulted in [%s]. If the VM is stopped and then started, it"
+ + " will start to write in the base file again. All changes made between the snapshot and the VM stop will be in the snapshot. If the VM is stopped, the snapshot must be"
+ + " merged into the base file manually.", snapshotName, vmName, volume, mergeCommand, mergeResult);
+
+ logger.warn("%s VM XML: [{}].", errorMsg, vm.getXMLDesc(0));
+ throw new CloudRuntimeException(errorMsg);
+ }
+
+ /**
+ * This was created to facilitate testing.
+ * */
+ protected BlockCommitListener getBlockCommitListener(Semaphore semaphore, String vmName) {
+ return new BlockCommitListener(semaphore, vmName, ThreadContext.get("logcontextid"));
+ }
+
+ /**
+ * This was created to facilitate testing.
+ * */
+ protected Semaphore getSemaphoreToWaitForMerge() {
+ return new Semaphore(0);
+ }
+
+ protected void checkBlockCommitProgress(Domain vm, String diskLabel, String vmName, String snapshotName, String topFilePath, String baseFilePath) {
+ int timeout = snapshotMergeTimeout;
+ DomainBlockJobInfo result;
+ long lastCommittedBytes = 0;
+ long endBytes = 0;
+ String partialLog = String.format("of top file [%s] into base file [%s] for snapshot [%s] of VM [%s]", topFilePath, baseFilePath, snapshotName, vmName);
+
+ while (timeout > 0) {
+ timeout -= 1;
+
+ try {
+ Thread.sleep(1000);
+ } catch (InterruptedException ex) {
+ logger.debug("Thread that was tracking the progress {} was interrupted.", partialLog, ex);
+ return;
+ }
+
+ try {
+ result = vm.getBlockJobInfo(diskLabel, 0);
+ } catch (LibvirtException ex) {
+ logger.warn("Exception while getting block job info {}: [{}].", partialLog, ex.getMessage(), ex);
+ return;
+ }
+
+ if (result == null || result.type == 0 && result.end == 0 && result.cur == 0) {
+ logger.debug("Block commit job {} has already finished.", partialLog);
+ return;
+ }
+
+ long currentCommittedBytes = result.cur;
+ if (currentCommittedBytes > lastCommittedBytes) {
+ logger.debug("The block commit {} is at [{}] of [{}].", partialLog, currentCommittedBytes, result.end);
+ }
+ lastCommittedBytes = currentCommittedBytes;
+ endBytes = result.end;
+ }
+ logger.warn("Block commit {} has timed out after waiting at least {} seconds. The progress of the operation was [{}] of [{}].", partialLog,
+ snapshotMergeTimeout, lastCommittedBytes, endBytes);
+ }
+
+ /**
+ * Manually deletes the unused snapshot file.<br/>
+ * This method is necessary due to Libvirt created the tag '--delete' on command 'virsh blockcommit' on version <b>1.2.9</b>, however it was only implemented on version
+ * <b>6.0.0</b>.
+ * @param snapshotPath The unused snapshot file to manually delete.
+ */
+ protected void manuallyDeleteUnusedSnapshotFile(boolean isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit, String snapshotPath) {
+ if (isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit) {
+ logger.debug("The current Libvirt's version supports the flag '--delete' on command 'virsh blockcommit', we will skip the manually deletion of the"
+ + " unused snapshot file [{}] as it already was automatically deleted.", snapshotPath);
+ return;
+ }
+
+ logger.debug("The current Libvirt's version does not supports the flag '--delete' on command 'virsh blockcommit', therefore we will manually delete the"
+ + " unused snapshot file [{}].", snapshotPath);
+
+ deleteIfExists(snapshotPath);
+ }
+
+ protected void deleteIfExists(String snapshotPath) {
+ try {
+ Files.deleteIfExists(Paths.get(snapshotPath));
+ logger.debug("Manually deleted unused snapshot file [{}].", snapshotPath);
+ } catch (IOException ex) {
+ throw new CloudRuntimeException(String.format("Unable to manually delete unused snapshot file [%s] due to [%s].", snapshotPath, ex.getMessage()));
+ }
+ }
+
public void setInterfaceDefQueueSettings(Map<String, String> details, Integer cpus, InterfaceDef interfaceDef) {
String nicMultiqueueNumber = details.get(VmDetailConstants.NIC_MULTIQUEUE_NUMBER);
if (nicMultiqueueNumber != null) {
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java
new file mode 100644
index 000000000000..84d17a1a1161
--- /dev/null
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java
@@ -0,0 +1,198 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.VMSnapshotTO;
+import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotAnswer;
+import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.hypervisor.kvm.resource.LibvirtVMDef;
+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.utils.Pair;
+import com.cloud.vm.VirtualMachine;
+import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
+import org.apache.cloudstack.storage.to.VolumeObjectTO;
+import org.apache.cloudstack.utils.qemu.QemuImg;
+import org.apache.cloudstack.utils.qemu.QemuImgException;
+import org.apache.cloudstack.utils.qemu.QemuImgFile;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.LibvirtException;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+
+@ResourceWrapper(handles = CreateDiskOnlyVmSnapshotCommand.class)
+public class LibvirtCreateDiskOnlyVMSnapshotCommandWrapper extends CommandWrapper<CreateDiskOnlyVmSnapshotCommand, Answer, LibvirtComputingResource> {
+
+ private static final String SNAPSHOT_XML = "<domainsnapshot>\n" +
+ "<name>%s</name>\n" +
+ "<memory snapshot='no'/>\n" +
+ "<disks> \n" +
+ "%s" +
+ "</disks> \n" +
+ "</domainsnapshot>";
+
+ private static final String TAG_DISK_SNAPSHOT = "<disk name='%s' snapshot='external'>\n" +
+ "<source file='%s'/>\n" +
+ "</disk>\n";
+
+ @Override
+ public Answer execute(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) {
+ VirtualMachine.State state = cmd.getVmState();
+
+ if (VirtualMachine.State.Running.equals(state)) {
+ return takeDiskOnlyVmSnapshotOfRunningVm(cmd, resource);
+ }
+
+ return takeDiskOnlyVmSnapshotOfStoppedVm(cmd, resource);
+ }
+
+ protected Answer takeDiskOnlyVmSnapshotOfRunningVm(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) {
+ String vmName = cmd.getVmName();
+ logger.info("Taking disk-only VM snapshot of running VM [{}].", vmName);
+
+ Domain dm = null;
+ try {
+ LibvirtUtilitiesHelper libvirtUtilitiesHelper = resource.getLibvirtUtilitiesHelper();
+ Connect conn = libvirtUtilitiesHelper.getConnection();
+ List<VolumeObjectTO> volumeObjectTOS = cmd.getVolumeTOs();
+ List<LibvirtVMDef.DiskDef> disks = resource.getDisks(conn, vmName);
+
+ dm = resource.getDomain(conn, vmName);
+
+ if (dm == null) {
+ return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, String.format("Creation of disk-only VM Snapshot failed as we could not find the VM [%s].", vmName), null);
+ }
+
+ VMSnapshotTO target = cmd.getTarget();
+ Pair<String, Map<String, Pair<Long, String>>> snapshotXmlAndVolumeToNewPathMap = createSnapshotXmlAndNewVolumePathMap(volumeObjectTOS, disks, target, resource);
+
+ dm.snapshotCreateXML(snapshotXmlAndVolumeToNewPathMap.first(), getFlagsToUseForRunningVmSnapshotCreation(target));
+
+ return new CreateDiskOnlyVmSnapshotAnswer(cmd, true, null, snapshotXmlAndVolumeToNewPathMap.second());
+ } catch (LibvirtException e) {
+ String errorMsg = String.format("Creation of disk-only VM snapshot for VM [%s] failed due to %s.", vmName, e.getMessage());
+ logger.error(errorMsg, e);
+ if (e.getMessage().contains("QEMU guest agent is not connected")) {
+ errorMsg = "QEMU guest agent is not connected. If the VM has been recently started, it might connect soon. Otherwise the VM does not have the" +
+ " guest agent installed; thus the QuiesceVM parameter is not supported.";
+ return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, errorMsg, null);
+ }
+ return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, e.getMessage(), null);
+ } finally {
+ if (dm != null) {
+ try {
+ dm.free();
+ } catch (LibvirtException l) {
+ logger.trace("Ignoring libvirt error.", l);
+ }
+ }
+ }
+ }
+
+ protected Answer takeDiskOnlyVmSnapshotOfStoppedVm(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) {
+ String vmName = cmd.getVmName();
+ logger.info("Taking disk-only VM snapshot of stopped VM [{}].", vmName);
+
+ Map<String, Pair<Long, String>> mapVolumeToSnapshotSizeAndNewVolumePath = new HashMap<>();
+
+ List<VolumeObjectTO> volumeObjectTos = cmd.getVolumeTOs();
+ KVMStoragePoolManager storagePoolMgr = resource.getStoragePoolMgr();
+ try {
+ for (VolumeObjectTO volumeObjectTO : volumeObjectTos) {
+ PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) volumeObjectTO.getDataStore();
+ KVMStoragePool kvmStoragePool = storagePoolMgr.getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid());
+
+ String snapshotPath = UUID.randomUUID().toString();
+ String snapshotFullPath = kvmStoragePool.getLocalPathFor(snapshotPath);
+ QemuImgFile newDelta = new QemuImgFile(snapshotFullPath, QemuImg.PhysicalDiskFormat.QCOW2);
+
+ String currentDeltaFullPath = kvmStoragePool.getLocalPathFor(volumeObjectTO.getPath());
+ QemuImgFile currentDelta = new QemuImgFile(currentDeltaFullPath, QemuImg.PhysicalDiskFormat.QCOW2);
+
+ QemuImg qemuImg = new QemuImg(0);
+
+ logger.debug("Creating new delta for volume [{}] as part of the disk-only VM snapshot process for VM [{}].", volumeObjectTO.getUuid(), vmName);
+ qemuImg.create(newDelta, currentDelta);
+
+ mapVolumeToSnapshotSizeAndNewVolumePath.put(volumeObjectTO.getUuid(), new Pair<>(getFileSize(currentDeltaFullPath), snapshotPath));
+ }
+ } catch (LibvirtException | QemuImgException e) {
+ logger.error("Exception while creating disk-only VM snapshot for VM [{}]. Deleting leftover deltas.", vmName, e);
+ for (VolumeObjectTO volumeObjectTO : volumeObjectTos) {
+ Pair<Long, String> volSizeAndNewPath = mapVolumeToSnapshotSizeAndNewVolumePath.get(volumeObjectTO.getUuid());
+ PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) volumeObjectTO.getDataStore();
+ KVMStoragePool kvmStoragePool = storagePoolMgr.getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid());
+
+ if (volSizeAndNewPath == null) {
+ continue;
+ }
+ try {
+ Files.deleteIfExists(Path.of(kvmStoragePool.getLocalPathFor(volSizeAndNewPath.second())));
+ } catch (IOException ex) {
+ logger.warn("Tried to delete leftover snapshot at [{}] failed.", volSizeAndNewPath.second(), ex);
+ }
+ }
+ return new Answer(cmd, e);
+ }
+
+ return new CreateDiskOnlyVmSnapshotAnswer(cmd, true, null, mapVolumeToSnapshotSizeAndNewVolumePath);
+ }
+
+ protected int getFlagsToUseForRunningVmSnapshotCreation(VMSnapshotTO target) {
+ int flags = target.getQuiescevm() ? Domain.SnapshotCreateFlags.QUIESCE : 0;
+ flags += Domain.SnapshotCreateFlags.DISK_ONLY +
+ Domain.SnapshotCreateFlags.ATOMIC +
+ Domain.SnapshotCreateFlags.NO_METADATA;
+ return flags;
+ }
+
+ protected Pair<String, Map<String, Pair<Long, String>>> createSnapshotXmlAndNewVolumePathMap(List<VolumeObjectTO> volumeObjectTOS, List<LibvirtVMDef.DiskDef> disks, VMSnapshotTO target, LibvirtComputingResource resource) {
+ StringBuilder stringBuilder = new StringBuilder();
+ Map<String, Pair<Long, String>> volumeObjectToNewPathMap = new HashMap<>();
+
+ for (VolumeObjectTO volumeObjectTO : volumeObjectTOS) {
+ LibvirtVMDef.DiskDef diskdef = resource.getDiskWithPathOfVolumeObjectTO(disks, volumeObjectTO);
+ String newPath = UUID.randomUUID().toString();
+ stringBuilder.append(String.format(TAG_DISK_SNAPSHOT, diskdef.getDiskLabel(), resource.getSnapshotTemporaryPath(diskdef.getDiskPath(), newPath)));
+
+ long snapSize = getFileSize(diskdef.getDiskPath());
+
+ volumeObjectToNewPathMap.put(volumeObjectTO.getUuid(), new Pair<>(snapSize, newPath));
+ }
+
+ String snapshotXml = String.format(SNAPSHOT_XML, target.getSnapshotName(), stringBuilder);
+ return new Pair<>(snapshotXml, volumeObjectToNewPathMap);
+ }
+
+ protected long getFileSize(String path) {
+ return new File(path).length();
+ }
+}
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper.java
new file mode 100644
index 000000000000..15df8627a8a7
--- /dev/null
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper.java
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.storage.DeleteDiskOnlyVmSnapshotCommand;
+import com.cloud.agent.api.to.DataTO;
+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 org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+
+@ResourceWrapper(handles = DeleteDiskOnlyVmSnapshotCommand.class)
+public class LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper extends CommandWrapper<DeleteDiskOnlyVmSnapshotCommand, Answer, LibvirtComputingResource> {
+
+ @Override
+ public Answer execute(DeleteDiskOnlyVmSnapshotCommand command, LibvirtComputingResource resource) {
+ List<DataTO> snapshotsToDelete = command.getSnapshots();
+ KVMStoragePoolManager storagePoolMgr = resource.getStoragePoolMgr();
+
+ for (DataTO snapshot : snapshotsToDelete) {
+ PrimaryDataStoreTO dataStoreTO = (PrimaryDataStoreTO) snapshot.getDataStore();
+ KVMStoragePool kvmStoragePool = storagePoolMgr.getStoragePool(dataStoreTO.getPoolType(), dataStoreTO.getUuid());
+
+ try {
+ String path = kvmStoragePool.getLocalPathFor(snapshot.getPath());
+ logger.debug("Deleting snapshot [{}] file [{}] as part of VM snapshot deletion.", snapshot.getId(), path);
+ Files.deleteIfExists(Path.of(path));
+ } catch (IOException e) {
+ return new Answer(command, e);
+ }
+ }
+ return new Answer(command, true, null);
+ }
+}
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMergeDiskOnlyVMSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMergeDiskOnlyVMSnapshotCommandWrapper.java
new file mode 100644
index 000000000000..ad0678080ed7
--- /dev/null
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMergeDiskOnlyVMSnapshotCommandWrapper.java
@@ -0,0 +1,150 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+
+import com.cloud.agent.api.storage.MergeDiskOnlyVmSnapshotCommand;
+import com.cloud.agent.api.storage.SnapshotMergeTreeTO;
+import com.cloud.agent.api.to.DataObjectType;
+import com.cloud.agent.api.to.DataTO;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.hypervisor.kvm.resource.LibvirtVMDef;
+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.utils.exception.CloudRuntimeException;
+import com.cloud.vm.VirtualMachine;
+import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
+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.QemuImgException;
+import org.apache.cloudstack.utils.qemu.QemuImgFile;
+import org.libvirt.Connect;
+import org.libvirt.Domain;
+import org.libvirt.LibvirtException;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+import java.util.stream.Collectors;
+
+@ResourceWrapper(handles = MergeDiskOnlyVmSnapshotCommand.class)
+public class LibvirtMergeDiskOnlyVMSnapshotCommandWrapper extends CommandWrapper<MergeDiskOnlyVmSnapshotCommand, Answer, LibvirtComputingResource> {
+
+ @Override
+ public Answer execute(MergeDiskOnlyVmSnapshotCommand command, LibvirtComputingResource serverResource) {
+ VirtualMachine.State vmState = command.getVmState();
+
+ try {
+ if (VirtualMachine.State.Running.equals(vmState)) {
+ return mergeDiskOnlySnapshotsForRunningVM(command, serverResource);
+ }
+ return mergeDiskOnlySnapshotsForStoppedVM(command, serverResource);
+ } catch (LibvirtException | QemuImgException | CloudRuntimeException ex) {
+ return new Answer(command, ex);
+ }
+ }
+
+ protected Answer mergeDiskOnlySnapshotsForStoppedVM(MergeDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) throws QemuImgException, LibvirtException {
+ QemuImg qemuImg = new QemuImg(resource.getCmdsTimeout());
+ KVMStoragePoolManager storageManager = resource.getStoragePoolMgr();
+
+ List<SnapshotMergeTreeTO> snapshotMergeTreeTOList = cmd.getSnapshotMergeTreeToList();
+
+ logger.debug("Merging disk-only snapshots for stopped VM [{}] using the following Snapshot Merge Trees [{}].", cmd.getVmName(), snapshotMergeTreeTOList);
+
+ for (SnapshotMergeTreeTO snapshotMergeTreeTO : snapshotMergeTreeTOList) {
+ DataTO parentTo = snapshotMergeTreeTO.getParent();
+ PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) parentTo.getDataStore();
+ KVMStoragePool storagePool = storageManager.getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid());
+ String childLocalPath = storagePool.getLocalPathFor(snapshotMergeTreeTO.getChild().getPath());
+
+ QemuImgFile parent = new QemuImgFile(storagePool.getLocalPathFor(parentTo.getPath()), QemuImg.PhysicalDiskFormat.QCOW2);
+ QemuImgFile child = new QemuImgFile(childLocalPath, QemuImg.PhysicalDiskFormat.QCOW2);
+
+ logger.debug("Committing child delta [{}] into parent snapshot [{}].", parentTo, snapshotMergeTreeTO.getChild());
+ qemuImg.commit(child, parent, true);
+
+ List<QemuImgFile> grandChildren = snapshotMergeTreeTO.getGrandChildren().stream()
+ .map(snapshotTo -> new QemuImgFile(storagePool.getLocalPathFor(snapshotTo.getPath()), QemuImg.PhysicalDiskFormat.QCOW2))
+ .collect(Collectors.toList());
+
+ logger.debug("Rebasing grandChildren [{}] into parent at [{}].", grandChildren, parent.getFileName());
+ for (QemuImgFile grandChild : grandChildren) {
+ qemuImg.rebase(grandChild, parent, parent.getFormat().toString(), false);
+ }
+
+ logger.debug("Deleting child at [{}] as it is useless.", childLocalPath);
+ try {
+ Files.deleteIfExists(Path.of(childLocalPath));
+ } catch (IOException e) {
+ return new Answer(cmd, e);
+ }
+ }
+ return new Answer(cmd, true, null);
+ }
+
+ protected Answer mergeDiskOnlySnapshotsForRunningVM(MergeDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) throws LibvirtException, QemuImgException {
+ String vmName = cmd.getVmName();
+ List<SnapshotMergeTreeTO> snapshotMergeTreeTOList = cmd.getSnapshotMergeTreeToList();
+
+ LibvirtUtilitiesHelper libvirtUtilitiesHelper = resource.getLibvirtUtilitiesHelper();
+ Connect conn = libvirtUtilitiesHelper.getConnection();
+ Domain domain = resource.getDomain(conn, vmName);
+ List<LibvirtVMDef.DiskDef> disks = resource.getDisks(conn, vmName);
+ KVMStoragePoolManager storageManager = resource.getStoragePoolMgr();
+ QemuImg qemuImg = new QemuImg(resource.getCmdsTimeout());
+
+ logger.debug("Merging disk-only snapshots for running VM [{}] using the following Snapshot Merge Trees [{}].", vmName, snapshotMergeTreeTOList);
+
+ for (SnapshotMergeTreeTO mergeTreeTO : snapshotMergeTreeTOList) {
+ DataTO childTO = mergeTreeTO.getChild();
+ SnapshotObjectTO parentSnapshotTO = (SnapshotObjectTO) mergeTreeTO.getParent();
+ VolumeObjectTO volumeObjectTO = parentSnapshotTO.getVolume();
+ KVMStoragePool storagePool = libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(volumeObjectTO, storageManager);
+
+ boolean active = DataObjectType.VOLUME.equals(childTO.getObjectType());
+ String label = resource.getDiskWithPathOfVolumeObjectTO(disks, volumeObjectTO).getDiskLabel();
+ String parentSnapshotLocalPath = storagePool.getLocalPathFor(parentSnapshotTO.getPath());
+ String childDeltaPath = storagePool.getLocalPathFor(childTO.getPath());
+
+ logger.debug("Found label [{}] for [{}]. Will merge delta at [{}] into delta at [{}].", label, volumeObjectTO, parentSnapshotLocalPath, childDeltaPath);
+
+ resource.mergeSnapshotIntoBaseFile(domain, label, parentSnapshotLocalPath, childDeltaPath, active, childTO.getPath(),
+ volumeObjectTO, conn);
+
+ QemuImgFile parent = new QemuImgFile(parentSnapshotLocalPath, QemuImg.PhysicalDiskFormat.QCOW2);
+
+ List<QemuImgFile> grandChildren = mergeTreeTO.getGrandChildren().stream()
+ .map(snapshotTo -> new QemuImgFile(storagePool.getLocalPathFor(snapshotTo.getPath()), QemuImg.PhysicalDiskFormat.QCOW2))
+ .collect(Collectors.toList());
+
+ logger.debug("Rebasing grandChildren [{}] into parent at [{}].", grandChildren, parentSnapshotLocalPath);
+ for (QemuImgFile grandChild : grandChildren) {
+ qemuImg.rebase(grandChild, parent, parent.getFormat().toString(), false);
+ }
+ }
+
+ return new Answer(cmd, true, null);
+ }
+}
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertDiskOnlyVMSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertDiskOnlyVMSnapshotCommandWrapper.java
new file mode 100644
index 000000000000..1aa79d48eec2
--- /dev/null
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertDiskOnlyVMSnapshotCommandWrapper.java
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.storage.RevertDiskOnlyVmSnapshotAnswer;
+import com.cloud.agent.api.storage.RevertDiskOnlyVmSnapshotCommand;
+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 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.QemuImgException;
+import org.apache.cloudstack.utils.qemu.QemuImgFile;
+import org.libvirt.LibvirtException;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+
+@ResourceWrapper(handles = RevertDiskOnlyVmSnapshotCommand.class)
+public class LibvirtRevertDiskOnlyVMSnapshotCommandWrapper extends CommandWrapper<RevertDiskOnlyVmSnapshotCommand, Answer, LibvirtComputingResource> {
+
+ @Override
+ public Answer execute(RevertDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) {
+ List<SnapshotObjectTO> snapshotObjectTos = cmd.getSnapshotObjectTos();
+
+ String vmName = cmd.getVmName();
+ logger.info("Reverting disk-only VM snapshot of VM [{}]", vmName);
+
+ KVMStoragePoolManager storagePoolMgr = resource.getStoragePoolMgr();
+ LibvirtUtilitiesHelper libvirtUtilitiesHelper = resource.getLibvirtUtilitiesHelper();
+
+ HashMap<SnapshotObjectTO, String> snapshotToNewDeltaPath = new HashMap<>();
+ try {
+ for (SnapshotObjectTO snapshotObjectTo : snapshotObjectTos) {
+ KVMStoragePool kvmStoragePool = libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(snapshotObjectTo, storagePoolMgr);
+
+ String deltaPath = libvirtUtilitiesHelper.generateUUIDName();
+ String deltaFullPath = kvmStoragePool.getLocalPathFor(deltaPath);
+ QemuImgFile newDelta = new QemuImgFile(deltaFullPath, QemuImg.PhysicalDiskFormat.QCOW2);
+
+ String snapshotFullPath = kvmStoragePool.getLocalPathFor(snapshotObjectTo.getPath());
+ QemuImgFile currentDelta = new QemuImgFile(snapshotFullPath, QemuImg.PhysicalDiskFormat.QCOW2);
+
+ QemuImg qemuImg = new QemuImg(0);
+
+ logger.debug("Creating new delta for volume [{}] as part of the disk-only VM snapshot revert process for VM [{}].", snapshotObjectTo.getVolume().getUuid(), vmName);
+ qemuImg.create(newDelta, currentDelta);
+ snapshotToNewDeltaPath.put(snapshotObjectTo, deltaPath);
+ }
+ } catch (LibvirtException | QemuImgException e) {
+ logger.error("Exception while reverting disk-only VM snapshot for VM [{}]. Deleting leftover deltas.", vmName, e);
+ for (SnapshotObjectTO snapshotObjectTo : snapshotObjectTos) {
+ String newPath = snapshotToNewDeltaPath.get(snapshotObjectTo);
+
+ if (newPath == null) {
+ continue;
+ }
+
+ KVMStoragePool kvmStoragePool = libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(snapshotObjectTo, storagePoolMgr);
+ try {
+ Files.deleteIfExists(Path.of(kvmStoragePool.getLocalPathFor(newPath)));
+ } catch (IOException ex) {
+ logger.warn("Tried to delete leftover snapshot at [{}] failed.", newPath, ex);
+ }
+ }
+ return new Answer(cmd, e);
+ }
+
+ List<VolumeObjectTO> volumeObjectTos = new ArrayList<>();
+ for (SnapshotObjectTO snapshotObjectTo : snapshotObjectTos) {
+ VolumeObjectTO volumeObjectTo = snapshotObjectTo.getVolume();
+
+ KVMStoragePool kvmStoragePool = libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(snapshotObjectTo, storagePoolMgr);
+
+ try {
+ Files.deleteIfExists(Path.of(kvmStoragePool.getLocalPathFor(volumeObjectTo.getPath())));
+ } catch (IOException ex) {
+ logger.warn("Got an error while trying to delete old volume delta [{}], there might be trash on storage [{}].", volumeObjectTo.getPath(),
+ kvmStoragePool.getUuid());
+ }
+ volumeObjectTo.setPath(snapshotToNewDeltaPath.get(snapshotObjectTo));
+ volumeObjectTos.add(volumeObjectTo);
+ }
+
+ return new RevertDiskOnlyVmSnapshotAnswer(cmd, volumeObjectTos);
+ }
+}
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUtilitiesHelper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUtilitiesHelper.java
index a2d161ac94bf..70e7f074c879 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUtilitiesHelper.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtUtilitiesHelper.java
@@ -22,6 +22,10 @@
import javax.naming.ConfigurationException;
+import com.cloud.agent.api.to.DataTO;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePool;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager;
+import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.LogManager;
import org.libvirt.Connect;
@@ -145,4 +149,10 @@ public static boolean isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(Con
result.second() ? "" : " does not"));
return result.second();
}
+
+ public KVMStoragePool getPrimaryPoolFromDataTo(DataTO dataTO, KVMStoragePoolManager storageManager) {
+ PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) dataTO.getDataStore();
+ return storageManager.getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid());
+ }
+
}
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java
index d7791310ff79..82603fab35e3 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java
@@ -16,6 +16,7 @@
// under the License.
package com.cloud.hypervisor.kvm.storage;
+import java.io.File;
import java.util.List;
import java.util.Map;
@@ -122,4 +123,10 @@ default LibvirtVMDef.DiskDef.BlockIOSize getSupportedPhysicalBlockSize() {
default void customizeLibvirtDiskDef(LibvirtVMDef.DiskDef disk) {
}
+
+
+ default String getLocalPathFor(String relativePath) {
+ return String.format("%s%s%s", getLocalPath(), File.separator, relativePath);
+ }
+
}
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
index cdb523a50fd6..13c7ae3c2a9f 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
@@ -121,7 +121,6 @@
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef.DeviceType;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef.DiscardType;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef.DiskProtocol;
-import com.cloud.hypervisor.kvm.resource.wrapper.LibvirtUtilitiesHelper;
import com.cloud.storage.JavaStorageLayer;
import com.cloud.storage.MigrationOptions;
import com.cloud.storage.ScopeType;
@@ -1720,15 +1719,6 @@ public Answer createVolume(final CreateObjectCommand cmd) {
*/
private static final String TAG_AVOID_DISK_FROM_SNAPSHOT = "<disk name='%s' snapshot='no' />";
- /**
- * Virsh command to merge (blockcommit) snapshot into the base file.<br><br>
- * 1st parameter: VM's name;<br>
- * 2nd parameter: disk's label (target.dev tag from VM's XML);<br>
- * 3rd parameter: the absolute path of the base file;
- * 4th parameter: the flag '--delete', if Libvirt supports it. Libvirt started to support it on version <b>6.0.0</b>;
- */
- private static final String COMMAND_MERGE_SNAPSHOT = "virsh blockcommit %s %s --base %s --active --wait %s --pivot";
-
/**
* Flag to take disk-only snapshots from VM.<br><br>
* Libvirt lib for java does not have the enum virDomainSnapshotCreateFlags.
@@ -1789,7 +1779,7 @@ public Answer createSnapshot(final CreateObjectCommand cmd) {
String diskLabel = takeVolumeSnapshot(resource.getDisks(conn, vmName), snapshotName, diskPath, vm);
String convertResult = convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, disk, snapshotPath, volume, cmd.getWait());
- mergeSnapshotIntoBaseFile(vm, diskLabel, diskPath, snapshotName, volume, conn);
+ resource.mergeSnapshotIntoBaseFile(vm, diskLabel, diskPath, null, true, snapshotName, volume, conn);
validateConvertResult(convertResult, snapshotPath);
} catch (LibvirtException e) {
@@ -1919,59 +1909,6 @@ protected void validateConvertResult(String convertResult, String snapshotPath)
throw new CloudRuntimeException(convertResult);
}
- /**
- * Merges the snapshot into base file to keep volume and VM behavior after stopping - starting.
- * @param vm Domain of the VM;
- * @param diskLabel Disk label to manage snapshot and base file;
- * @param baseFilePath Path of the base file;
- * @param snapshotName Name of the snapshot;
- * @throws LibvirtException
- */
- protected void mergeSnapshotIntoBaseFile(Domain vm, String diskLabel, String baseFilePath, String snapshotName, VolumeObjectTO volume,
- Connect conn) throws LibvirtException {
- boolean isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit = LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(conn);
- String vmName = vm.getName();
- String mergeCommand = String.format(COMMAND_MERGE_SNAPSHOT, vmName, diskLabel, baseFilePath, isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit ? "--delete" : "");
- String mergeResult = Script.runSimpleBashScript(mergeCommand);
-
- if (mergeResult == null) {
- logger.debug(String.format("Successfully merged snapshot [%s] into VM [%s] %s base file.", snapshotName, vmName, volume));
- manuallyDeleteUnusedSnapshotFile(isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit, getSnapshotTemporaryPath(baseFilePath, snapshotName));
- return;
- }
-
- String errorMsg = String.format("Failed to merge snapshot [%s] into VM [%s] %s base file. Command [%s] resulted in [%s]. If the VM is stopped and then started, it"
- + " will start to write in the base file again. All changes made between the snapshot and the VM stop will be in the snapshot. If the VM is stopped, the snapshot must be"
- + " merged into the base file manually.", snapshotName, vmName, volume, mergeCommand, mergeResult);
-
- logger.warn(String.format("%s VM XML: [%s].", errorMsg, vm.getXMLDesc(0)));
- throw new CloudRuntimeException(errorMsg);
- }
-
- /**
- * Manually deletes the unused snapshot file.<br/>
- * This method is necessary due to Libvirt created the tag '--delete' on command 'virsh blockcommit' on version <b>1.2.9</b>, however it was only implemented on version
- * <b>6.0.0</b>.
- * @param snapshotPath The unused snapshot file to manually delete.
- */
- protected void manuallyDeleteUnusedSnapshotFile(boolean isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit, String snapshotPath) {
- if (isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit) {
- logger.debug(String.format("The current Libvirt's version supports the flag '--delete' on command 'virsh blockcommit', we will skip the manually deletion of the"
- + " unused snapshot file [%s] as it already was automatically deleted.", snapshotPath));
- return;
- }
-
- logger.debug(String.format("The current Libvirt's version does not supports the flag '--delete' on command 'virsh blockcommit', therefore we will manually delete the"
- + " unused snapshot file [%s].", snapshotPath));
-
- try {
- Files.deleteIfExists(Paths.get(snapshotPath));
- logger.debug(String.format("Manually deleted unused snapshot file [%s].", snapshotPath));
- } catch (IOException ex) {
- throw new CloudRuntimeException(String.format("Unable to manually delete unused snapshot file [%s] due to [%s].", snapshotPath, ex.getMessage()));
- }
- }
-
/**
* Creates the snapshot directory in the primary storage, if it does not exist; then, converts the base file (VM's old writing file) to the snapshot directory.
* @param primaryPool Storage to create folder, if not exists;
@@ -2042,7 +1979,7 @@ protected String takeVolumeSnapshot(List<DiskDef> disks, String snapshotName, St
String diskLabelToSnapshot = diskToSnapshotAndDisksToAvoid.first();
String disksToAvoidsOnSnapshot = diskToSnapshotAndDisksToAvoid.second().stream().map(diskLabel -> String.format(TAG_AVOID_DISK_FROM_SNAPSHOT, diskLabel))
.collect(Collectors.joining());
- String snapshotTemporaryPath = getSnapshotTemporaryPath(diskPath, snapshotName);
+ String snapshotTemporaryPath = resource.getSnapshotTemporaryPath(diskPath, snapshotName);
String createSnapshotXmlFormated = String.format(XML_CREATE_DISK_SNAPSHOT, snapshotName, diskLabelToSnapshot, snapshotTemporaryPath, disksToAvoidsOnSnapshot);
@@ -2093,18 +2030,6 @@ protected Pair<String, Set<String>> getDiskToSnapshotAndDisksToAvoid(List<DiskDe
return new Pair<>(diskLabelToSnapshot, disksToAvoid);
}
- /**
- * Retrieves the temporary path of the snapshot.
- * @param diskPath Path of the disk to snapshot;
- * @param snapshotName Snapshot name;
- * @return the path of the disk replacing the disk with the snapshot.
- */
- protected String getSnapshotTemporaryPath(String diskPath, String snapshotName) {
- String[] diskPathSplitted = diskPath.split(File.separator);
- diskPathSplitted[diskPathSplitted.length - 1] = snapshotName;
- return String.join(File.separator, diskPathSplitted);
- }
-
/**
* Validate if the primary storage has enough capacity to take a disk snapshot, as the snapshot will duplicate the disk to backup.
* @param primaryPool Primary storage to verify capacity;
diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java
index bc123b294ce7..d4d750420ab6 100644
--- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java
+++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java
@@ -786,6 +786,46 @@ public void resize(final QemuImgFile file, final long size) throws QemuImgExcept
this.resize(file, size, false);
}
+ /**
+ * Commits an image.
+ *
+ * This method is a facade for 'qemu-img commit'.
+ *
+ * @param file
+ * The file to be commited.
+ * @param base
+ * If base is not specified, the immediate backing file of the top image (which is {@code file}) will be used.
+ * @param deleteFile
+ * If true, the commited file(s) will be deleted.
+ */
+ public void commit( QemuImgFile file, QemuImgFile base, boolean deleteFile) throws QemuImgException {
+ if (file == null) {
+ throw new QemuImgException("File should not be null");
+ }
+
+ final Script s = new Script(_qemuImgPath, timeout);
+ s.add("commit");
+ if (deleteFile) {
+ s.add("-d");
+ }
+
+ if (file.getFormat() != null) {
+ s.add("-f");
+ s.add(file.getFormat().format);
+ }
+
+ if (base != null) {
+ s.add("-b");
+ s.add(base.getFileName());
+ }
+
+ s.add(file.getFileName());
+ final String result = s.execute();
+ if (result != null) {
+ throw new QemuImgException(result);
+ }
+ }
+
/**
* Does qemu-img support --target-is-zero
* @return boolean
diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImgFile.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImgFile.java
index b461d967c893..1d00fd18e689 100644
--- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImgFile.java
+++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImgFile.java
@@ -17,6 +17,7 @@
package org.apache.cloudstack.utils.qemu;
import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
+import org.apache.commons.lang3.builder.ReflectionToStringBuilder;
public class QemuImgFile {
@@ -68,4 +69,9 @@ public PhysicalDiskFormat getFormat() {
return this.format;
}
+ @Override
+ public String toString() {
+ return ReflectionToStringBuilder.toString(this);
+ }
+
}
diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
index 920b3a439d0e..92f7c96dfe1b 100644
--- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
+++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
@@ -40,6 +40,8 @@
import java.net.NetworkInterface;
import java.net.URI;
import java.net.URISyntaxException;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
@@ -48,6 +50,7 @@
import java.util.Random;
import java.util.UUID;
import java.util.Vector;
+import java.util.concurrent.Semaphore;
import javax.naming.ConfigurationException;
import javax.xml.parsers.DocumentBuilderFactory;
@@ -63,6 +66,7 @@
import org.apache.cloudstack.storage.command.AttachAnswer;
import org.apache.cloudstack.storage.command.AttachCommand;
import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
+import org.apache.cloudstack.storage.to.SnapshotObjectTO;
import org.apache.cloudstack.storage.to.VolumeObjectTO;
import org.apache.cloudstack.utils.bytescale.ByteScaleUtils;
import org.apache.cloudstack.utils.linux.CPUStat;
@@ -72,6 +76,7 @@
import org.apache.commons.lang.SystemUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.ThreadContext;
import org.joda.time.Duration;
import org.junit.Assert;
import org.junit.Before;
@@ -253,6 +258,15 @@ public class LibvirtComputingResourceTest {
@Mock
DomainBlockStats domainBlockStatsMock;
+ @Mock
+ VolumeObjectTO volumeObjectToMock;
+
+ @Mock
+ SnapshotObjectTO snapshotObjectToMock;
+
+ @Mock
+ BlockCommitListener blockCommitListenerMock;
+
private final static long HYPERVISOR_LIBVIRT_VERSION_SUPPORTS_IOURING = 6003000;
private final static long HYPERVISOR_QEMU_VERSION_SUPPORTS_IOURING = 5000000;
@@ -6541,4 +6555,152 @@ public void testGetDiskModelFromVMDetailVirtioBlk() {
DiskDef.DiskBus diskBus = libvirtComputingResourceSpy.getDiskModelFromVMDetail(virtualMachineTO);
assertEquals(DiskDef.DiskBus.VIRTIOBLK, diskBus);
}
+
+ @Test
+ public void getSnapshotTemporaryPathTestReturnExpectedResult(){
+ String path = "/path/to/disk";
+ String snapshotName = "snapshot";
+ String expectedResult = "/path/to/snapshot";
+
+ String result = libvirtComputingResourceSpy.getSnapshotTemporaryPath(path, snapshotName);
+ Assert.assertEquals(expectedResult, result);
+ }
+
+ @Test
+ public void mergeSnapshotIntoBaseFileTestActiveAndDeleteFlags() throws Exception {
+ libvirtComputingResourceSpy.snapshotMergeTimeout = 10;
+
+ try (MockedStatic<LibvirtUtilitiesHelper> libvirtUtilitiesHelperMockedStatic = Mockito.mockStatic(LibvirtUtilitiesHelper.class);
+ MockedStatic<ThreadContext> threadContextMockedStatic = Mockito.mockStatic(ThreadContext.class)) {
+ libvirtUtilitiesHelperMockedStatic.when(() ->
+ LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(Mockito.any())).thenAnswer(invocation -> true);
+ Mockito.doReturn(new Semaphore(1)).when(libvirtComputingResourceSpy).getSemaphoreToWaitForMerge();
+
+ threadContextMockedStatic.when(() ->
+ ThreadContext.get(Mockito.anyString())).thenReturn("logid");
+ Mockito.doNothing().when(domainMock).addBlockJobListener(Mockito.any());
+ Mockito.doReturn(null).when(domainMock).getBlockJobInfo(Mockito.anyString(), Mockito.anyInt());
+ Mockito.doNothing().when(domainMock).removeBlockJobListener(Mockito.any());
+
+ String diskLabel = "vda";
+ String baseFilePath = "/file";
+ String snapshotName = "snap";
+
+ libvirtComputingResourceSpy.mergeSnapshotIntoBaseFileWithEventsAndConfigurableTimeout(domainMock, diskLabel, baseFilePath, null, true, snapshotName, volumeObjectToMock, connMock);
+
+ Mockito.verify(domainMock, Mockito.times(1)).blockCommit(diskLabel, baseFilePath, null, 0, Domain.BlockCommitFlags.ACTIVE | Domain.BlockCommitFlags.DELETE);
+ Mockito.verify(libvirtComputingResourceSpy, Mockito.times(1)).manuallyDeleteUnusedSnapshotFile(true, "/" + snapshotName);
+ }
+ }
+
+ @Test
+ public void mergeSnapshotIntoBaseFileTestActiveFlag() throws Exception {
+ try (MockedStatic<LibvirtUtilitiesHelper> libvirtUtilitiesHelperMockedStatic = Mockito.mockStatic(LibvirtUtilitiesHelper.class);
+ MockedStatic<ThreadContext> threadContextMockedStatic = Mockito.mockStatic(ThreadContext.class)) {
+ libvirtUtilitiesHelperMockedStatic.when(() ->
+ LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(Mockito.any())).thenAnswer(invocation -> false);
+ Mockito.doReturn(new Semaphore(1)).when(libvirtComputingResourceSpy).getSemaphoreToWaitForMerge();
+
+ threadContextMockedStatic.when(() ->
+ ThreadContext.get(Mockito.anyString())).thenReturn("logid");
+ Mockito.doNothing().when(domainMock).addBlockJobListener(Mockito.any());
+ Mockito.doNothing().when(domainMock).removeBlockJobListener(Mockito.any());
+ Mockito.doNothing().when(libvirtComputingResourceSpy).manuallyDeleteUnusedSnapshotFile(Mockito.anyBoolean(), Mockito.anyString());
+
+ String diskLabel = "vda";
+ String baseFilePath = "/file";
+ String snapshotName = "snap";
+
+ libvirtComputingResourceSpy.mergeSnapshotIntoBaseFileWithEventsAndConfigurableTimeout(domainMock, diskLabel, baseFilePath, null, true, snapshotName, volumeObjectToMock, connMock);
+
+ Mockito.verify(domainMock, Mockito.times(1)).blockCommit(diskLabel, baseFilePath, null, 0, Domain.BlockCommitFlags.ACTIVE);
+ Mockito.verify(libvirtComputingResourceSpy, Mockito.times(1)).manuallyDeleteUnusedSnapshotFile(false, "/" + snapshotName);
+ }
+ }
+
+ @Test
+ public void mergeSnapshotIntoBaseFileTestDeleteFlag() throws Exception {
+ try (MockedStatic<LibvirtUtilitiesHelper> libvirtUtilitiesHelperMockedStatic = Mockito.mockStatic(LibvirtUtilitiesHelper.class);
+ MockedStatic<ThreadContext> threadContextMockedStatic = Mockito.mockStatic(ThreadContext.class)) {
+ libvirtComputingResourceSpy.snapshotMergeTimeout = 10;
+ libvirtUtilitiesHelperMockedStatic.when(() -> LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(Mockito.any())).thenReturn(true);
+ Mockito.doReturn(new Semaphore(1)).when(libvirtComputingResourceSpy).getSemaphoreToWaitForMerge();
+ threadContextMockedStatic.when(() -> ThreadContext.get(Mockito.anyString())).thenReturn("logid");
+ Mockito.doNothing().when(domainMock).addBlockJobListener(Mockito.any());
+ Mockito.doReturn(null).when(domainMock).getBlockJobInfo(Mockito.anyString(), Mockito.anyInt());
+ Mockito.doNothing().when(domainMock).removeBlockJobListener(Mockito.any());
+ Mockito.doNothing().when(libvirtComputingResourceSpy).manuallyDeleteUnusedSnapshotFile(Mockito.anyBoolean(), Mockito.anyString());
+
+ String diskLabel = "vda";
+ String baseFilePath = "/file";
+ String snapshotName = "snap";
+
+ libvirtComputingResourceSpy.mergeSnapshotIntoBaseFileWithEventsAndConfigurableTimeout(domainMock, diskLabel, baseFilePath, null, false, snapshotName, volumeObjectToMock, connMock);
+
+ Mockito.verify(domainMock, Mockito.times(1)).blockCommit(diskLabel, baseFilePath, null, 0, Domain.BlockCommitFlags.DELETE);
+ Mockito.verify(libvirtComputingResourceSpy, Mockito.times(1)).manuallyDeleteUnusedSnapshotFile(true, "/" + snapshotName);
+ }
+ }
+
+ @Test
+ public void mergeSnapshotIntoBaseFileTestNoFlags() throws Exception {
+ try (MockedStatic<LibvirtUtilitiesHelper> libvirtUtilitiesHelperMockedStatic = Mockito.mockStatic(LibvirtUtilitiesHelper.class);
+ MockedStatic<ThreadContext> threadContextMockedStatic = Mockito.mockStatic(ThreadContext.class)) {
+ libvirtComputingResourceSpy.snapshotMergeTimeout = 10;
+ libvirtUtilitiesHelperMockedStatic.when(() -> LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(Mockito.any())).thenReturn(false);
+ Mockito.doReturn(new Semaphore(1)).when(libvirtComputingResourceSpy).getSemaphoreToWaitForMerge();
+ threadContextMockedStatic.when(() -> ThreadContext.get(Mockito.anyString())).thenReturn("logid");
+ Mockito.doNothing().when(domainMock).addBlockJobListener(Mockito.any());
+ Mockito.doReturn(null).when(domainMock).getBlockJobInfo(Mockito.anyString(), Mockito.anyInt());
+ Mockito.doNothing().when(domainMock).removeBlockJobListener(Mockito.any());
+ Mockito.doNothing().when(libvirtComputingResourceSpy).manuallyDeleteUnusedSnapshotFile(Mockito.anyBoolean(), Mockito.anyString());
+
+ String diskLabel = "vda";
+ String baseFilePath = "/file";
+ String snapshotName = "snap";
+
+ libvirtComputingResourceSpy.mergeSnapshotIntoBaseFileWithEventsAndConfigurableTimeout(domainMock, diskLabel, baseFilePath, null, false, snapshotName, volumeObjectToMock, connMock);
+
+ Mockito.verify(domainMock, Mockito.times(1)).blockCommit(diskLabel, baseFilePath, null, 0, 0);
+ Mockito.verify(libvirtComputingResourceSpy, Mockito.times(1)).manuallyDeleteUnusedSnapshotFile(false, "/" + snapshotName);
+ }
+ }
+
+ @Test (expected = CloudRuntimeException.class)
+ public void mergeSnapshotIntoBaseFileTestMergeFailsThrowException() throws Exception {
+ try (MockedStatic<LibvirtUtilitiesHelper> libvirtUtilitiesHelperMockedStatic = Mockito.mockStatic(LibvirtUtilitiesHelper.class);
+ MockedStatic<ThreadContext> threadContextMockedStatic = Mockito.mockStatic(ThreadContext.class)) {
+ libvirtComputingResourceSpy.snapshotMergeTimeout = 10;
+ libvirtUtilitiesHelperMockedStatic.when(() -> LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(Mockito.any())).thenReturn(false);
+ Mockito.doReturn(new Semaphore(1)).when(libvirtComputingResourceSpy).getSemaphoreToWaitForMerge();
+ threadContextMockedStatic.when(() -> ThreadContext.get(Mockito.anyString())).thenReturn("logid");
+ Mockito.doNothing().when(domainMock).addBlockJobListener(Mockito.any());
+ Mockito.doReturn(null).when(domainMock).getBlockJobInfo(Mockito.anyString(), Mockito.anyInt());
+ Mockito.doNothing().when(domainMock).removeBlockJobListener(Mockito.any());
+
+ Mockito.doReturn(blockCommitListenerMock).when(libvirtComputingResourceSpy).getBlockCommitListener(Mockito.any(), Mockito.any());
+ Mockito.doReturn("Failed").when(blockCommitListenerMock).getResult();
+
+ String diskLabel = "vda";
+ String baseFilePath = "/file";
+ String snapshotName = "snap";
+
+ libvirtComputingResourceSpy.mergeSnapshotIntoBaseFileWithEventsAndConfigurableTimeout(domainMock, diskLabel, baseFilePath, null, false, snapshotName, volumeObjectToMock, connMock);
+ }
+ }
+
+ @Test (expected = CloudRuntimeException.class)
+ public void manuallyDeleteUnusedSnapshotFileTestLibvirtDoesNotSupportsFlagDeleteExceptionOnFileDeletionThrowsException() throws IOException {
+ try (MockedStatic<Files> filesMockedStatic = Mockito.mockStatic(Files.class)) {
+ filesMockedStatic.when(() -> Files.deleteIfExists(Mockito.any(Path.class))).thenThrow(IOException.class);
+
+ libvirtComputingResourceSpy.manuallyDeleteUnusedSnapshotFile(false, "");
+ }
+ }
+
+ @Test
+ public void manuallyDeleteUnusedSnapshotFileTestLibvirtSupportingFlagDeleteOnCommandVirshBlockcommitIsTrueReturn() {
+ libvirtComputingResourceSpy.manuallyDeleteUnusedSnapshotFile(true, "");
+ Mockito.verify(libvirtComputingResourceSpy, Mockito.never()).deleteIfExists("");
+ }
}
diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java
index 0159deda3476..76598685dddd 100644
--- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java
+++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java
@@ -22,7 +22,6 @@
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
import com.cloud.hypervisor.kvm.resource.LibvirtDomainXMLParser;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef;
-import com.cloud.hypervisor.kvm.resource.wrapper.LibvirtUtilitiesHelper;
import com.cloud.storage.template.TemplateConstants;
import com.cloud.utils.Pair;
import com.cloud.utils.exception.CloudRuntimeException;
@@ -66,8 +65,8 @@ public class KVMStorageProcessorTest {
@Mock
KVMStoragePoolManager storagePoolManager;
- @Mock
- LibvirtComputingResource resource;
+
+ LibvirtComputingResource resource = Mockito.mock(LibvirtComputingResource.class);
@InjectMocks
private KVMStorageProcessor storageProcessor;
@@ -157,16 +156,6 @@ public void testIsEnoughSpaceForDownloadTemplateOnTemporaryLocationNotExistingLo
}
}
- @Test
- public void validateGetSnapshotTemporaryPath(){
- String path = "/path/to/disk";
- String snapshotName = "snapshot";
- String expectedResult = "/path/to/snapshot";
-
- String result = storageProcessor.getSnapshotTemporaryPath(path, snapshotName);
- Assert.assertEquals(expectedResult, result);
- }
-
@Test
public void validateGetSnapshotPathInPrimaryStorage(){
String path = "/path/to/disk";
@@ -241,6 +230,7 @@ public void validateGetDiskToSnapshotAndDisksToAvoidPathFoundReturnLabels() thro
public void validateTakeVolumeSnapshotFailToCreateSnapshotThrowLibvirtException() throws LibvirtException{
Mockito.doReturn(diskToSnapshotAndDisksToAvoidMock).when(storageProcessorSpy).getDiskToSnapshotAndDisksToAvoid(Mockito.any(), Mockito.anyString(), Mockito.any());
Mockito.doReturn(new HashSet<>()).when(diskToSnapshotAndDisksToAvoidMock).second();
+ Mockito.doReturn("").when(resource).getSnapshotTemporaryPath(Mockito.anyString(), Mockito.anyString());
Mockito.doThrow(LibvirtException.class).when(domainMock).snapshotCreateXML(Mockito.anyString(), Mockito.anyInt());
storageProcessorSpy.takeVolumeSnapshot(new ArrayList<>(), "", "", domainMock);
@@ -253,6 +243,7 @@ public void validateTakeVolumeSnapshotSuccessReturnDiskLabel() throws LibvirtExc
Mockito.doReturn(diskToSnapshotAndDisksToAvoidMock).when(storageProcessorSpy).getDiskToSnapshotAndDisksToAvoid(Mockito.any(), Mockito.anyString(), Mockito.any());
Mockito.doReturn(expectedResult).when(diskToSnapshotAndDisksToAvoidMock).first();
Mockito.doReturn(new HashSet<>()).when(diskToSnapshotAndDisksToAvoidMock).second();
+ Mockito.doReturn("").when(resource).getSnapshotTemporaryPath(Mockito.anyString(), Mockito.anyString());
Mockito.doReturn(null).when(domainMock).snapshotCreateXML(Mockito.anyString(), Mockito.anyInt());
String result = storageProcessorSpy.takeVolumeSnapshot(new ArrayList<>(), "", "", domainMock);
@@ -314,46 +305,6 @@ public void convertBaseFileToSnapshotFileInPrimaryStorageDirTestConvertSuccessRe
}
}
- @Test (expected = CloudRuntimeException.class)
- public void validateMergeSnapshotIntoBaseFileErrorOnMergeThrowCloudRuntimeException() throws Exception {
- try (MockedStatic<Script> ignored = Mockito.mockStatic(
- Script.class); MockedStatic<LibvirtUtilitiesHelper> ignored2 = Mockito.mockStatic(
- LibvirtUtilitiesHelper.class)) {
- Mockito.when(Script.runSimpleBashScript(Mockito.anyString())).thenReturn("");
- Mockito.when(LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(Mockito.any()))
- .thenReturn(true);
-
- storageProcessorSpy.mergeSnapshotIntoBaseFile(domainMock, "", "", "", volumeObjectToMock, connectMock);
- }
- }
-
- @Test
- public void validateMergeSnapshotIntoBaseFileMergeSuccessDoNothing() throws Exception {
- try (MockedStatic<Script> scriptMockedStatic = Mockito.mockStatic(
- Script.class); MockedStatic<LibvirtUtilitiesHelper> libvirtUtilitiesHelperMockedStatic = Mockito.mockStatic(
- LibvirtUtilitiesHelper.class)) {
- Mockito.when(Script.runSimpleBashScript(Mockito.anyString())).thenReturn(null);
- Mockito.when(LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(Mockito.any()))
- .thenReturn(true);
- Mockito.doNothing().when(storageProcessorSpy).manuallyDeleteUnusedSnapshotFile(Mockito.anyBoolean(),
- Mockito.any());
-
- storageProcessorSpy.mergeSnapshotIntoBaseFile(domainMock, "", "", "", volumeObjectToMock, connectMock);
- libvirtUtilitiesHelperMockedStatic.verify(() -> LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(Mockito.any()), Mockito.times(1));
- scriptMockedStatic.verify(() -> Script.runSimpleBashScript(Mockito.anyString()), Mockito.times(1));
-
- }
- }
-
- @Test (expected = CloudRuntimeException.class)
- public void validateManuallyDeleteUnusedSnapshotFileLibvirtDoesNotSupportsFlagDeleteExceptionOnFileDeletionThrowsException() throws IOException {
- try (MockedStatic<Files> ignored = Mockito.mockStatic(Files.class)) {
- Mockito.when(Files.deleteIfExists(Mockito.any(Path.class))).thenThrow(IOException.class);
-
- storageProcessorSpy.manuallyDeleteUnusedSnapshotFile(false, "");
- }
- }
-
@Test
public void validateIsAvailablePoolSizeDividedByDiskSizeLesserThanMinRate(){
Assert.assertTrue(storageProcessorSpy.isAvailablePoolSizeDividedByDiskSizeLesserThanMinRate(10499l, 10000l));
diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
index 1a25f36efe21..ed821f84b9e0 100644
--- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
@@ -35,11 +35,16 @@
import javax.inject.Inject;
+import com.cloud.projects.Project;
+import com.cloud.projects.ProjectManager;
+import com.cloud.vm.snapshot.VMSnapshot;
+import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
+import org.apache.cloudstack.api.command.user.volume.AssignVolumeCmd;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.ApiErrorCode;
import org.apache.cloudstack.api.InternalIdentity;
import org.apache.cloudstack.api.ServerApiException;
-import org.apache.cloudstack.api.command.user.volume.AssignVolumeCmd;
import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
import org.apache.cloudstack.api.command.user.volume.ChangeOfferingForVolumeCmd;
import org.apache.cloudstack.api.command.user.volume.CheckAndRepairVolumeCmd;
@@ -157,8 +162,6 @@
import com.cloud.offering.DiskOffering;
import com.cloud.org.Cluster;
import com.cloud.org.Grouping;
-import com.cloud.projects.Project;
-import com.cloud.projects.ProjectManager;
import com.cloud.resource.ResourceManager;
import com.cloud.resource.ResourceState;
import com.cloud.serializer.GsonHelper;
@@ -358,6 +361,11 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
@Inject
HostPodDao podDao;
+ @Inject
+ private VMSnapshotDetailsDao vmSnapshotDetailsDao;
+
+ protected static final String KVM_FILE_BASED_STORAGE_SNAPSHOT = "kvmFileBasedStorageSnapshot";
+
protected Gson _gson;
private static final List<HypervisorType> SupportedHypervisorsForVolResize = Arrays.asList(HypervisorType.KVM, HypervisorType.XenServer,
@@ -1398,6 +1406,29 @@ public VolumeVO resizeVolume(ResizeVolumeCmd cmd) throws ResourceAllocationExcep
shrinkOk);
}
+ protected void validateNoVmSnapshots(VolumeVO volume) {
+ if (volume.getInstanceId() != null) {
+ if (vmHasVmSnapshotsExceptKvmDiskOnlySnapshots(volume.getInstanceId())) {
+ throw new InvalidParameterValueException("The volume is attached to a VM with memory&disk VM snapshots; therefore, it cannot be resized.");
+ }
+ }
+ }
+
+ protected boolean vmHasVmSnapshotsExceptKvmDiskOnlySnapshots(long instanceId) {
+ for (VMSnapshotVO vmSnapshotVO : _vmSnapshotDao.findByVm(instanceId)) {
+ if (VMSnapshot.Type.DiskAndMemory.equals(vmSnapshotVO.getType())) {
+ return true;
+ }
+ List<VMSnapshotDetailsVO> vmSnapshotDetails = vmSnapshotDetailsDao.listDetails(vmSnapshotVO.getId());
+ if (vmSnapshotDetails.stream().
+ noneMatch(vmSnapshotDetailsVO -> vmSnapshotDetailsVO.getName().equals(KVM_FILE_BASED_STORAGE_SNAPSHOT))) {
+ return true;
+ }
+ }
+
+ return false;
+ }
+
/**
* A volume should not be resized if it covers ALL the following scenarios: <br>
* 1 - Root volume <br>
@@ -2374,12 +2405,7 @@ private void validateVolumeResizeWithSize(VolumeVO volume, long currentSize, Lon
// if the caller is looking to change the size of the volume
if (newSize != null && currentSize != newSize) {
- if (volume.getInstanceId() != null) {
- // Check that VM to which this volume is attached does not have VM snapshots
- if (_vmSnapshotDao.findByVm(volume.getInstanceId()).size() > 0) {
- throw new InvalidParameterValueException("A volume that is attached to a VM with any VM snapshots cannot be resized.");
- }
- }
+ validateNoVmSnapshots(volume);
if (!validateVolumeSizeInBytes(newSize)) {
throw new InvalidParameterValueException("Requested size out of range");
diff --git a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
index 907182edc2ab..1c1a28eb8c66 100644
--- a/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
@@ -27,7 +27,6 @@
import javax.inject.Inject;
import javax.naming.ConfigurationException;
-import com.cloud.storage.snapshot.SnapshotManager;
import org.apache.cloudstack.annotation.AnnotationService;
import org.apache.cloudstack.annotation.dao.AnnotationDao;
import org.apache.cloudstack.api.ApiConstants;
@@ -379,14 +378,8 @@ public VMSnapshot allocVMSnapshot(Long vmId, String vsDisplayName, String vsDesc
//Other Storage volume plugins could integrate this with their own functionality for group snapshots
VMSnapshotStrategy snapshotStrategy = storageStrategyFactory.getVmSnapshotStrategy(userVmVo.getId(), rootVolumePool.getId(), snapshotMemory);
if (snapshotStrategy == null) {
- String message;
- if (!SnapshotManager.VmStorageSnapshotKvm.value() && !snapshotMemory) {
- message = "Creating a snapshot of a running KVM instance without memory is not supported";
- } else {
- message = "KVM does not support the type of snapshot requested";
- }
-
- logger.debug(message);
+ String message = String.format("No strategy was able to handle requested snapshot for VM [%s].", userVmVo.getUuid());
+ logger.error(message);
throw new CloudRuntimeException(message);
}
diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java
index 7dcf30c55e40..f6070d7da802 100644
--- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java
+++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java
@@ -41,6 +41,14 @@
import java.util.UUID;
import java.util.concurrent.ExecutionException;
+import com.cloud.event.EventTypes;
+import com.cloud.event.UsageEventUtils;
+import com.cloud.host.HostVO;
+import com.cloud.service.ServiceOfferingVO;
+import com.cloud.service.dao.ServiceOfferingDao;
+import com.cloud.vm.snapshot.VMSnapshot;
+import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.SecurityChecker.AccessType;
import org.apache.cloudstack.api.command.user.volume.CheckAndRepairVolumeCmd;
@@ -99,12 +107,9 @@
import com.cloud.dc.dao.ClusterDao;
import com.cloud.dc.dao.DataCenterDao;
import com.cloud.dc.dao.HostPodDao;
-import com.cloud.event.EventTypes;
-import com.cloud.event.UsageEventUtils;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.exception.PermissionDeniedException;
import com.cloud.exception.ResourceAllocationException;
-import com.cloud.host.HostVO;
import com.cloud.host.dao.HostDao;
import com.cloud.hypervisor.Hypervisor.HypervisorType;
import com.cloud.org.Grouping;
@@ -113,8 +118,6 @@
import com.cloud.serializer.GsonHelper;
import com.cloud.server.ManagementService;
import com.cloud.server.TaggedResourceService;
-import com.cloud.service.ServiceOfferingVO;
-import com.cloud.service.dao.ServiceOfferingDao;
import com.cloud.storage.Storage.ProvisioningType;
import com.cloud.storage.Volume.Type;
import com.cloud.storage.dao.DiskOfferingDao;
@@ -268,6 +271,9 @@ public class VolumeApiServiceImplTest {
@Mock
private ManagementService managementService;
+ @Mock
+ private VMSnapshotDetailsDao vmSnapshotDetailsDaoMock;
+
private long accountMockId = 456l;
private long volumeMockId = 12313l;
private long vmInstanceMockId = 1123l;
@@ -2200,4 +2206,85 @@ public void testCreateVolumeOnSecondaryForAttachIfNeeded_NoSuitablePool_ReturnSa
Assert.fail();
}
}
+
+ @Test
+ public void validateNoVmSnapshotsTestNoInstanceId() {
+ Mockito.doReturn(null).when(volumeVoMock).getInstanceId();
+
+ volumeApiServiceImpl.validateNoVmSnapshots(volumeVoMock);
+
+ Mockito.verify(volumeApiServiceImpl, Mockito.never()).vmHasVmSnapshotsExceptKvmDiskOnlySnapshots(Mockito.anyLong());
+ }
+
+ @Test
+ public void validateNoVmSnapshotsTestVmHasVmSnapshotsExceptKvmDiskOnlySnapshotsIsFalse() {
+ Mockito.doReturn(1L).when(volumeVoMock).getInstanceId();
+ Mockito.doReturn(false).when(volumeApiServiceImpl).vmHasVmSnapshotsExceptKvmDiskOnlySnapshots(1L);
+
+ volumeApiServiceImpl.validateNoVmSnapshots(volumeVoMock);
+ }
+
+ @Test (expected = InvalidParameterValueException.class)
+ public void validateNoVmSnapshotsTestVmHasVmSnapshotsExceptKvmDiskOnlySnapshotsIsTrue() {
+ Mockito.doReturn(1L).when(volumeVoMock).getInstanceId();
+ Mockito.doReturn(true).when(volumeApiServiceImpl).vmHasVmSnapshotsExceptKvmDiskOnlySnapshots(1L);
+
+ volumeApiServiceImpl.validateNoVmSnapshots(volumeVoMock);
+ }
+
+ @Test
+ public void vmHasVmSnapshotsExceptKvmDiskOnlySnapshotsTestMultipleDiskAndMemory () {
+ List<VMSnapshotVO> snapList = generateVmSnapshotVoList(VMSnapshot.Type.DiskAndMemory, VMSnapshot.Type.DiskAndMemory);
+ Mockito.doReturn(snapList).when(_vmSnapshotDao).findByVm(0L);
+
+ Assert.assertTrue(volumeApiServiceImpl.vmHasVmSnapshotsExceptKvmDiskOnlySnapshots(0L));
+ }
+
+ @Test
+ public void vmHasVmSnapshotsExceptKvmDiskOnlySnapshotsTestOnlyOneDiskAndMemory () {
+ List<VMSnapshotVO> snapList = generateVmSnapshotVoList(VMSnapshot.Type.Disk, VMSnapshot.Type.DiskAndMemory);
+ Mockito.doReturn(snapList).when(_vmSnapshotDao).findByVm(0L);
+ VMSnapshotDetailsVO snapDetail = new VMSnapshotDetailsVO(0L, VolumeApiServiceImpl.KVM_FILE_BASED_STORAGE_SNAPSHOT, "0", true);
+ Mockito.doReturn(List.of(snapDetail)).when(vmSnapshotDetailsDaoMock).listDetails(Mockito.anyLong());
+
+ Assert.assertTrue(volumeApiServiceImpl.vmHasVmSnapshotsExceptKvmDiskOnlySnapshots(0L));
+ }
+ @Test
+ public void vmHasVmSnapshotsExceptKvmDiskOnlySnapshotsTestDiskSnapshotsButNotKvmFileBasedSnapshots () {
+ List<VMSnapshotVO> snapList = generateVmSnapshotVoList(VMSnapshot.Type.Disk, VMSnapshot.Type.Disk);
+ Mockito.doReturn(snapList).when(_vmSnapshotDao).findByVm(0L);
+
+ Assert.assertTrue(volumeApiServiceImpl.vmHasVmSnapshotsExceptKvmDiskOnlySnapshots(0L));
+ }
+
+ @Test
+ public void vmHasVmSnapshotsExceptKvmDiskOnlySnapshotsTestOnlyOneKvmFileBasedSnapshot () {
+ List<VMSnapshotVO> snapList = generateVmSnapshotVoList(VMSnapshot.Type.Disk, VMSnapshot.Type.Disk);
+ Mockito.doReturn(snapList).when(_vmSnapshotDao).findByVm(0L);
+ VMSnapshotDetailsVO snapDetail = new VMSnapshotDetailsVO(0L, VolumeApiServiceImpl.KVM_FILE_BASED_STORAGE_SNAPSHOT, "0", true);
+ Mockito.doReturn(List.of(snapDetail)).when(vmSnapshotDetailsDaoMock).listDetails(0);
+
+ Assert.assertTrue(volumeApiServiceImpl.vmHasVmSnapshotsExceptKvmDiskOnlySnapshots(0L));
+ }
+
+ @Test
+ public void vmHasVmSnapshotsExceptKvmDiskOnlySnapshotsTestAllKvmFileBasedSnapshots () {
+ List<VMSnapshotVO> snapList = generateVmSnapshotVoList(VMSnapshot.Type.Disk, VMSnapshot.Type.Disk);
+ Mockito.doReturn(snapList).when(_vmSnapshotDao).findByVm(0L);
+ VMSnapshotDetailsVO snapDetail = new VMSnapshotDetailsVO(0L, VolumeApiServiceImpl.KVM_FILE_BASED_STORAGE_SNAPSHOT, "0", true);
+ Mockito.doReturn(List.of(snapDetail)).when(vmSnapshotDetailsDaoMock).listDetails(0);
+ Mockito.doReturn(List.of(snapDetail)).when(vmSnapshotDetailsDaoMock).listDetails(1);
+
+ Assert.assertFalse(volumeApiServiceImpl.vmHasVmSnapshotsExceptKvmDiskOnlySnapshots(0L));
+ }
+
+ private List<VMSnapshotVO> generateVmSnapshotVoList(VMSnapshot.Type t1, VMSnapshot.Type t2) {
+ VMSnapshotVO mock1 = Mockito.mock(VMSnapshotVO.class);
+ Mockito.doReturn(t1).when(mock1).getType();
+ Mockito.doReturn(0L).when(mock1).getId();
+ VMSnapshotVO mock2 = Mockito.mock(VMSnapshotVO.class);
+ Mockito.doReturn(t2).when(mock2).getType();
+ Mockito.doReturn(1L).when(mock2).getId();
+ return List.of(mock1, mock2);
+ }
}
diff --git a/ui/src/config/section/compute.js b/ui/src/config/section/compute.js
index 6bbcf38e9b55..ba63f5d0da7d 100644
--- a/ui/src/config/section/compute.js
+++ b/ui/src/config/section/compute.js
@@ -192,8 +192,8 @@ export default {
args: ['virtualmachineid', 'name', 'description', 'snapshotmemory', 'quiescevm'],
show: (record) => {
return (((['Running'].includes(record.state) && record.hypervisor !== 'LXC') ||
- (['Stopped'].includes(record.state) && ((record.hypervisor !== 'KVM' && record.hypervisor !== 'LXC') ||
- (record.hypervisor === 'KVM' && record.pooltype === 'PowerFlex')))) && record.vmtype !== 'sharedfsvm')
+ (['Stopped'].includes(record.state) && (!['KVM', 'LXC'].includes(record.hypervisor) ||
+ (record.hypervisor === 'KVM' && ['PowerFlex', 'Filesystem', 'NetworkFilesystem', 'SharedMountPoint'].includes(record.pooltype))))) && record.vmtype !== 'sharedfsvm')
},
disabled: (record) => { return record.hostcontrolstate === 'Offline' && record.hypervisor === 'KVM' },
mapping: {
From 2bc8403d389b4888da1dd2347fd8237d607383dc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Jandre?=
<48719461+JoaoJandre@users.noreply.github.com>
Date: Mon, 12 May 2025 13:49:56 -0300
Subject: [PATCH 02/11] fix merge conflict issues
---
.../com/cloud/hypervisor/kvm/storage/KVMStoragePool.java | 6 ------
.../kvm/resource/LibvirtComputingResourceTest.java | 3 ---
2 files changed, 9 deletions(-)
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java
index 60be05e5d095..8dd2116e1235 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStoragePool.java
@@ -127,10 +127,4 @@ default LibvirtVMDef.DiskDef.BlockIOSize getSupportedPhysicalBlockSize() {
default void customizeLibvirtDiskDef(LibvirtVMDef.DiskDef disk) {
}
-
-
- default String getLocalPathFor(String relativePath) {
- return String.format("%s%s%s", getLocalPath(), File.separator, relativePath);
- }
-
}
diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
index 9c5b47e07dec..e2aa48cf8fbb 100644
--- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
+++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
@@ -266,9 +266,6 @@ public class LibvirtComputingResourceTest {
@Mock
DomainBlockStats domainBlockStatsMock;
- @Mock
- VolumeObjectTO volumeObjectToMock;
-
@Mock
SnapshotObjectTO snapshotObjectToMock;
From a14aae81fa56ddd7b0e7405f570b0a80934b52ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Jandre?=
<48719461+JoaoJandre@users.noreply.github.com>
Date: Tue, 27 May 2025 10:48:17 -0300
Subject: [PATCH 03/11] add restriction on snapshot from vm snapshot creation
---
api/src/main/java/com/cloud/storage/VolumeApiService.java | 2 +-
.../user/snapshot/CreateSnapshotFromVMSnapshotCmd.java | 2 +-
.../main/java/com/cloud/storage/VolumeApiServiceImpl.java | 6 +++++-
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/api/src/main/java/com/cloud/storage/VolumeApiService.java b/api/src/main/java/com/cloud/storage/VolumeApiService.java
index b7b5423244cf..98c9cf037224 100644
--- a/api/src/main/java/com/cloud/storage/VolumeApiService.java
+++ b/api/src/main/java/com/cloud/storage/VolumeApiService.java
@@ -137,7 +137,7 @@ Volume updateVolume(long volumeId, String path, String state, Long storageId,
void updateDisplay(Volume volume, Boolean displayVolume);
- Snapshot allocSnapshotForVm(Long vmId, Long volumeId, String snapshotName) throws ResourceAllocationException;
+ Snapshot allocSnapshotForVm(Long vmId, Long volumeId, String snapshotName, Long vmSnapshotId) throws ResourceAllocationException;
/**
* Checks if the storage pool supports the disk offering tags.
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotFromVMSnapshotCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotFromVMSnapshotCmd.java
index cdd908dfb87d..75089cece8e1 100644
--- a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotFromVMSnapshotCmd.java
+++ b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotFromVMSnapshotCmd.java
@@ -153,7 +153,7 @@ public ApiCommandResourceType getApiResourceType() {
@Override
public void create() throws ResourceAllocationException {
- Snapshot snapshot = this._volumeService.allocSnapshotForVm(getVmId(), getVolumeId(), getSnapshotName());
+ Snapshot snapshot = this._volumeService.allocSnapshotForVm(getVmId(), getVolumeId(), getSnapshotName(), getVMSnapshotId());
if (snapshot != null) {
this.setEntityId(snapshot.getId());
this.setEntityUuid(snapshot.getUuid());
diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
index 61324cd110cb..3cc8cb6d7e6d 100644
--- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
@@ -4017,7 +4017,7 @@ public Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName,
}
@Override
- public Snapshot allocSnapshotForVm(Long vmId, Long volumeId, String snapshotName) throws ResourceAllocationException {
+ public Snapshot allocSnapshotForVm(Long vmId, Long volumeId, String snapshotName, Long vmSnapshotId) throws ResourceAllocationException {
Account caller = CallContext.current().getCallingAccount();
VMInstanceVO vm = _vmInstanceDao.findById(vmId);
if (vm == null) {
@@ -4069,6 +4069,10 @@ public Snapshot allocSnapshotForVm(Long vmId, Long volumeId, String snapshotName
throw new InvalidParameterValueException("Cannot perform this operation, unsupported on storage pool type " + storagePool.getPoolType());
}
+ if (vmSnapshotDetailsDao.listDetails(vmSnapshotId).stream().anyMatch(vmSnapshotDetailsVO -> vmSnapshotDetailsVO.getName().equals(KVM_FILE_BASED_STORAGE_SNAPSHOT))) {
+ throw new InvalidParameterValueException("Cannot perform this operation, unsupported VM snapshot type.");
+ }
+
return snapshotMgr.allocSnapshot(volumeId, Snapshot.MANUAL_POLICY_ID, snapshotName, null, true, null);
}
From 6a08bf2573f96e72541639a57348da5302121cb0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Jandre?=
<48719461+JoaoJandre@users.noreply.github.com>
Date: Wed, 11 Jun 2025 09:03:02 -0300
Subject: [PATCH 04/11] Add limits to avoid data loss
---
.../com/cloud/storage/dao/SnapshotDao.java | 2 +
.../cloud/storage/dao/SnapshotDaoImpl.java | 23 ++++++++++
.../snapshot/DefaultSnapshotStrategy.java | 44 +++++++++++++++++++
...KvmFileBasedStorageVmSnapshotStrategy.java | 19 +++++++-
.../cloudstack/backup/NASBackupProvider.java | 21 +++++++++
.../cloud/storage/VolumeApiServiceImpl.java | 2 +-
6 files changed, 109 insertions(+), 2 deletions(-)
diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java
index 171634fb1044..3cda7d42760e 100755
--- a/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java
+++ b/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDao.java
@@ -58,4 +58,6 @@ public interface SnapshotDao extends GenericDao<SnapshotVO, Long>, StateDao<Snap
*/
List<SnapshotVO> listByIds(Object... ids);
List<SnapshotVO> searchByVolumes(List<Long> volumeIds);
+
+ List<SnapshotVO> listByVolumeIdAndTypeNotInAndStateNotRemoved(long volumeId, Type... type);
}
diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java
index f5fc9c47d036..c479a386d794 100755
--- a/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java
+++ b/engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java
@@ -19,6 +19,7 @@
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.List;
import javax.annotation.PostConstruct;
@@ -56,6 +57,10 @@ public class SnapshotDaoImpl extends GenericDaoBase<SnapshotVO, Long> implements
private static final String GET_LAST_SNAPSHOT =
"SELECT snapshots.id FROM snapshot_store_ref, snapshots where snapshots.id = snapshot_store_ref.snapshot_id AND snapshosts.volume_id = ? AND snapshot_store_ref.role = ? ORDER BY created DESC";
+ private static final String VOLUME_ID = "volumeId";
+ private static final String NOT_TYPE = "notType";
+ private static final String STATUS = "status";
+
private SearchBuilder<SnapshotVO> snapshotIdsSearch;
private SearchBuilder<SnapshotVO> VolumeIdSearch;
private SearchBuilder<SnapshotVO> VolumeIdTypeSearch;
@@ -66,6 +71,8 @@ public class SnapshotDaoImpl extends GenericDaoBase<SnapshotVO, Long> implements
private SearchBuilder<SnapshotVO> StatusSearch;
private SearchBuilder<SnapshotVO> notInStatusSearch;
private GenericSearchBuilder<SnapshotVO, Long> CountSnapshotsByAccount;
+
+ private SearchBuilder<SnapshotVO> volumeIdAndTypeNotInSearch;
@Inject
ResourceTagDao _tagsDao;
@Inject
@@ -181,6 +188,12 @@ protected void init() {
InstanceIdSearch.join("instanceSnapshots", volumeSearch, volumeSearch.entity().getId(), InstanceIdSearch.entity().getVolumeId(), JoinType.INNER);
InstanceIdSearch.done();
+
+ volumeIdAndTypeNotInSearch = createSearchBuilder();
+ volumeIdAndTypeNotInSearch.and(VOLUME_ID, volumeIdAndTypeNotInSearch.entity().getVolumeId(), SearchCriteria.Op.EQ);
+ volumeIdAndTypeNotInSearch.and(STATUS, volumeIdAndTypeNotInSearch.entity().getState(), SearchCriteria.Op.NEQ);
+ volumeIdAndTypeNotInSearch.and(NOT_TYPE, volumeIdAndTypeNotInSearch.entity().getTypeDescription(), SearchCriteria.Op.NOTIN);
+ volumeIdAndTypeNotInSearch.done();
}
@Override
@@ -299,4 +312,14 @@ public List<SnapshotVO> searchByVolumes(List<Long> volumeIds) {
sc.setParameters("volumeIds", volumeIds.toArray());
return search(sc, null);
}
+
+ @Override
+ public List<SnapshotVO> listByVolumeIdAndTypeNotInAndStateNotRemoved(long volumeId, Type... types) {
+ SearchCriteria<SnapshotVO> sc = volumeIdAndTypeNotInSearch.create();
+ sc.setParameters(VOLUME_ID, volumeId);
+ sc.setParameters(NOT_TYPE, Arrays.stream(types).map(Type::toString).toArray());
+ sc.setParameters(STATUS, State.Destroyed);
+
+ return listBy(sc);
+ }
}
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
index 0801731630b6..8db067131bf7 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
@@ -23,7 +23,15 @@
import javax.inject.Inject;
+import com.cloud.storage.VolumeApiServiceImpl;
import com.cloud.utils.db.TransactionCallback;
+import com.cloud.vm.VMInstanceVO;
+import com.cloud.vm.dao.VMInstanceDao;
+import com.cloud.vm.snapshot.VMSnapshot;
+import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
+import com.cloud.vm.snapshot.VMSnapshotVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event;
@@ -100,6 +108,15 @@ public class DefaultSnapshotStrategy extends SnapshotStrategyBase {
@Inject
SnapshotZoneDao snapshotZoneDao;
+ @Inject
+ private VMSnapshotDao vmSnapshotDao;
+
+ @Inject
+ private VMSnapshotDetailsDao vmSnapshotDetailsDao;
+
+ @Inject
+ private VMInstanceDao vmInstanceDao;
+
private final List<Snapshot.State> snapshotStatesAbleToDeleteSnapshot = Arrays.asList(Snapshot.State.Destroying, Snapshot.State.Destroyed, Snapshot.State.Error, Snapshot.State.Hidden);
public SnapshotDataStoreVO getSnapshotImageStoreRef(long snapshotId, long zoneId) {
@@ -610,6 +627,9 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
@Override
public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperation op) {
+ if (SnapshotOperation.TAKE.equals(op)) {
+ return validateVmSnapshot(snapshot);
+ }
if (SnapshotOperation.REVERT.equals(op)) {
long volumeId = snapshot.getVolumeId();
VolumeVO volumeVO = volumeDao.findById(volumeId);
@@ -626,6 +646,30 @@ public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperat
return StrategyPriority.DEFAULT;
}
+ private StrategyPriority validateVmSnapshot(Snapshot snapshot) {
+ VolumeVO volumeVO = volumeDao.findById(snapshot.getVolumeId());
+ Long instanceId = volumeVO.getInstanceId();
+ if (instanceId == null) {
+ return StrategyPriority.DEFAULT;
+ }
+
+ VMInstanceVO vm = vmInstanceDao.findById(instanceId);
+ if (vm == null) {
+ return StrategyPriority.DEFAULT;
+ }
+
+ for (VMSnapshotVO vmSnapshotVO : vmSnapshotDao.findByVmAndByType(vm.getId(), VMSnapshot.Type.Disk)) {
+ List<VMSnapshotDetailsVO> vmSnapshotDetails = vmSnapshotDetailsDao.listDetails(vmSnapshotVO.getId());
+ if (vmSnapshotDetails.stream().anyMatch(vmSnapshotDetailsVO -> vmSnapshotDetailsVO.getName().equals(VolumeApiServiceImpl.KVM_FILE_BASED_STORAGE_SNAPSHOT))) {
+ logger.warn("VM [{}] already has KVM File-Based storage VM snapshots. These VM snapshots and volume snapshots are not supported " +
+ "together for KVM. As restoring volume snapshots will erase the VM snapshots and cause data loss.", vm.getUuid());
+ return StrategyPriority.CANT_HANDLE;
+ }
+ }
+
+ return StrategyPriority.DEFAULT;
+ }
+
protected boolean isSnapshotStoredOnSameZoneStoreForQCOW2Volume(Snapshot snapshot, VolumeVO volumeVO) {
if (volumeVO == null || !ImageFormat.QCOW2.equals(volumeVO.getFormat())) {
return false;
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
index 6463bf738a41..eb6093312498 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
@@ -49,6 +49,8 @@
import com.cloud.vm.snapshot.VMSnapshot;
import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
import com.cloud.vm.snapshot.VMSnapshotVO;
+import org.apache.cloudstack.backup.BackupOfferingVO;
+import org.apache.cloudstack.backup.dao.BackupOfferingDao;
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
@@ -81,6 +83,9 @@ public class KvmFileBasedStorageVmSnapshotStrategy extends StorageVMSnapshotStra
@Inject
protected ResourceLimitService resourceLimitManager;
+ @Inject
+ protected BackupOfferingDao backupOfferingDao;
+
@Override
public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) {
Map<VolumeInfo, SnapshotObject> volumeInfoToSnapshotObjectMap = new HashMap<>();
@@ -316,11 +321,23 @@ public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMe
for (VolumeVO volume : volumes) {
StoragePoolVO storagePoolVO = storagePool.findById(volume.getPoolId());
if (!supportedStoragePoolTypes.contains(storagePoolVO.getPoolType())) {
- logger.debug("{} as the VM has a volume that is in a storage with unsupported type [{}].", cantHandleLog, storagePoolVO.getPoolType());
+ logger.debug(String.format("%s as the VM has a volume that is in a storage with unsupported type [%s].", cantHandleLog, storagePoolVO.getPoolType()));
+ return StrategyPriority.CANT_HANDLE;
+ }
+ List<SnapshotVO> snapshots = snapshotDao.listByVolumeIdAndTypeNotInAndStateNotRemoved(volume.getId(), Snapshot.Type.GROUP);
+ if (CollectionUtils.isNotEmpty(snapshots)) {
+ logger.debug("{} as VM has a volume with snapshots {}. Volume snapshots and KvmFileBasedStorageVmSnapshotStrategy are not compatible, as restoring volume snapshots will erase VM " +
+ "snapshots and cause data loss.", cantHandleLog, snapshots);
return StrategyPriority.CANT_HANDLE;
}
}
+ BackupOfferingVO backupOffering = backupOfferingDao.findById(vm.getBackupOfferingId());
+ if (backupOffering != null) {
+ logger.debug("{} as the VM has a backup offering. This strategy does not support snapshots on VMs with current backup providers.", cantHandleLog);
+ return StrategyPriority.CANT_HANDLE;
+ }
+
return StrategyPriority.HIGHEST;
}
diff --git a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java
index 1e3b5aaa8e5e..bc90753fe632 100644
--- a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java
+++ b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java
@@ -27,6 +27,7 @@
import com.cloud.storage.ScopeType;
import com.cloud.storage.StoragePoolHostVO;
import com.cloud.storage.Volume;
+import com.cloud.storage.VolumeApiServiceImpl;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.StoragePoolHostDao;
import com.cloud.storage.dao.VolumeDao;
@@ -35,6 +36,12 @@
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.vm.VirtualMachine;
import com.cloud.vm.dao.VMInstanceDao;
+import com.cloud.vm.snapshot.VMSnapshot;
+import com.cloud.vm.snapshot.VMSnapshotDetailsVO;
+import com.cloud.vm.snapshot.VMSnapshotVO;
+import com.cloud.vm.snapshot.dao.VMSnapshotDao;
+import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao;
+
import org.apache.cloudstack.backup.dao.BackupDao;
import org.apache.cloudstack.backup.dao.BackupRepositoryDao;
@@ -87,6 +94,12 @@ public class NASBackupProvider extends AdapterBase implements BackupProvider, Co
@Inject
private AgentManager agentManager;
+ @Inject
+ private VMSnapshotDao vmSnapshotDao;
+
+ @Inject
+ private VMSnapshotDetailsDao vmSnapshotDetailsDao;
+
protected Host getLastVMHypervisorHost(VirtualMachine vm) {
Long hostId = vm.getLastHostId();
if (hostId == null) {
@@ -400,6 +413,14 @@ public Backup createNewBackupEntryForRestorePoint(Backup.RestorePoint restorePoi
@Override
public boolean assignVMToBackupOffering(VirtualMachine vm, BackupOffering backupOffering) {
+ for (VMSnapshotVO vmSnapshotVO : vmSnapshotDao.findByVmAndByType(vm.getId(), VMSnapshot.Type.Disk)) {
+ List<VMSnapshotDetailsVO> vmSnapshotDetails = vmSnapshotDetailsDao.listDetails(vmSnapshotVO.getId());
+ if (vmSnapshotDetails.stream().anyMatch(vmSnapshotDetailsVO -> vmSnapshotDetailsVO.getName().equals(VolumeApiServiceImpl.KVM_FILE_BASED_STORAGE_SNAPSHOT))) {
+ logger.warn("VM [{}] has VM snapshots using the KvmFileBasedStorageVmSnapshot Strategy; this provider does not support backups on VMs with these snapshots!");
+ return false;
+ }
+ }
+
return Hypervisor.HypervisorType.KVM.equals(vm.getHypervisorType());
}
diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
index 3cc8cb6d7e6d..245125bc240b 100644
--- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
@@ -365,7 +365,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
@Inject
private VMSnapshotDetailsDao vmSnapshotDetailsDao;
- protected static final String KVM_FILE_BASED_STORAGE_SNAPSHOT = "kvmFileBasedStorageSnapshot";
+ public static final String KVM_FILE_BASED_STORAGE_SNAPSHOT = "kvmFileBasedStorageSnapshot";
protected Gson _gson;
From af9090e97a0ebfc80ba694dba1bed7af9344598e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Jandre?=
<48719461+JoaoJandre@users.noreply.github.com>
Date: Wed, 11 Jun 2025 17:10:41 -0300
Subject: [PATCH 05/11] fix validation
---
.../vmsnapshot/DefaultVMSnapshotStrategy.java | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
index c035700f64d2..8ec3d7843e7a 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
@@ -479,11 +479,17 @@ public boolean deleteVMSnapshotFromDB(VMSnapshot vmSnapshot, boolean unmanage) {
@Override
public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMemory) {
UserVmVO vm = userVmDao.findById(vmId);
- if (State.Running.equals(vm.getState()) && (!snapshotMemory || vmHasKvmDiskOnlySnapshot(vm))) {
+ if (State.Running.equals(vm.getState()) && !snapshotMemory) {
logger.debug("Default VM snapshot cannot handle VM snapshot for [{}] as it is running and its memory will not be affected.", vm);
return StrategyPriority.CANT_HANDLE;
}
+ if (vmHasKvmDiskOnlySnapshot(vm)) {
+ logger.debug("Default VM snapshot cannot handle VM snapshot for [{}] as it has a disk-only VM snapshot using kvmFileBasedStorageSnapshot strategy." +
+ "These two strategies are not compatible, as reverting a disk-only VM snapshot will erase newer disk-and-memory VM snapshots.", vm);
+ return StrategyPriority.CANT_HANDLE;
+ }
+
List<VolumeVO> volumes = volumeDao.findByInstance(vmId);
for (VolumeVO volume : volumes) {
if (volume.getFormat() != ImageFormat.QCOW2) {
@@ -501,9 +507,7 @@ private boolean vmHasKvmDiskOnlySnapshot(UserVm vm) {
for (VMSnapshotVO vmSnapshotVO : vmSnapshotDao.findByVmAndByType(vm.getId(), VMSnapshot.Type.Disk)) {
List<VMSnapshotDetailsVO> vmSnapshotDetails = vmSnapshotDetailsDao.listDetails(vmSnapshotVO.getId());
- if (vmSnapshotDetails.stream().
- anyMatch(vmSnapshotDetailsVO -> vmSnapshotDetailsVO.getName().equals(KVM_FILE_BASED_STORAGE_SNAPSHOT) ||
- vmSnapshotDetailsVO.getName().equals(STORAGE_SNAPSHOT))) {
+ if (vmSnapshotDetails.stream().anyMatch(vmSnapshotDetailsVO -> vmSnapshotDetailsVO.getName().equals(KVM_FILE_BASED_STORAGE_SNAPSHOT))) {
return true;
}
}
From 92c348698caf60ce8487fdef4572e1ca25ede9e0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Jandre?=
<48719461+JoaoJandre@users.noreply.github.com>
Date: Tue, 17 Jun 2025 13:18:02 -0300
Subject: [PATCH 06/11] fix merge witthout events and parent search
---
.../cloud/vm/snapshot/dao/VMSnapshotDao.java | 2 ++
.../vm/snapshot/dao/VMSnapshotDaoImpl.java | 19 ++++++++++++++
...KvmFileBasedStorageVmSnapshotStrategy.java | 8 +++---
.../resource/LibvirtComputingResource.java | 25 +++++++++++++++----
4 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDao.java b/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDao.java
index ace58167c0ed..4045af58d4bc 100644
--- a/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDao.java
+++ b/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDao.java
@@ -37,6 +37,8 @@ public interface VMSnapshotDao extends GenericDao<VMSnapshotVO, Long>, StateDao<
List<VMSnapshotVO> listByParent(Long vmSnapshotId);
+ List<VMSnapshotVO> listByParentAndStateIn(Long vmSnapshotId, VMSnapshot.State... states);
+
VMSnapshotVO findByName(Long vmId, String name);
List<VMSnapshotVO> listByAccountId(Long accountId);
diff --git a/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDaoImpl.java
index d7af7292d2e1..83411b3cf8f7 100644
--- a/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDaoImpl.java
+++ b/engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDaoImpl.java
@@ -42,6 +42,12 @@ public class VMSnapshotDaoImpl extends GenericDaoBase<VMSnapshotVO, Long> implem
private final SearchBuilder<VMSnapshotVO> SnapshotStatusSearch;
private final SearchBuilder<VMSnapshotVO> AllFieldsSearch;
+ private SearchBuilder<VMSnapshotVO> parentIdEqAndStateIn;
+
+ private static final String PARENT = "parent";
+
+ private static final String STATE = "state";
+
protected VMSnapshotDaoImpl() {
AllFieldsSearch = createSearchBuilder();
AllFieldsSearch.and("state", AllFieldsSearch.entity().getState(), Op.EQ);
@@ -71,6 +77,11 @@ protected VMSnapshotDaoImpl() {
SnapshotStatusSearch.and("vm_id", SnapshotStatusSearch.entity().getVmId(), SearchCriteria.Op.EQ);
SnapshotStatusSearch.and("state", SnapshotStatusSearch.entity().getState(), SearchCriteria.Op.IN);
SnapshotStatusSearch.done();
+
+ parentIdEqAndStateIn = createSearchBuilder();
+ parentIdEqAndStateIn.and(PARENT, parentIdEqAndStateIn.entity().getParent(), Op.EQ);
+ parentIdEqAndStateIn.and(STATE, parentIdEqAndStateIn.entity().getState(), Op.IN);
+ parentIdEqAndStateIn.done();
}
@Override
@@ -119,6 +130,14 @@ public List<VMSnapshotVO> listByParent(Long vmSnapshotId) {
return listBy(sc, null);
}
+ @Override
+ public List<VMSnapshotVO> listByParentAndStateIn(Long vmSnapshotId, State... states) {
+ SearchCriteria<VMSnapshotVO> sc = parentIdEqAndStateIn.create();
+ sc.setParameters(PARENT, vmSnapshotId);
+ sc.setParameters(STATE, (Object[])states);
+ return listBy(sc, null);
+ }
+
@Override
public VMSnapshotVO findByName(Long vmId, String name) {
SearchCriteria<VMSnapshotVO> sc = AllFieldsSearch.create();
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
index eb6093312498..21f73f69479e 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
@@ -123,7 +123,7 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) {
transitStateWithoutThrow(vmSnapshotBeingDeleted, VMSnapshot.Event.ExpungeRequested);
List<VolumeObjectTO> volumeTOs = vmSnapshotHelper.getVolumeTOList(vmSnapshotBeingDeleted.getVmId());
- List<VMSnapshotVO> snapshotChildren = vmSnapshotDao.listByParent(vmSnapshotBeingDeleted.getId());
+ List<VMSnapshotVO> snapshotChildren = vmSnapshotDao.listByParentAndStateIn(vmSnapshotBeingDeleted.getId(), VMSnapshot.State.Ready, VMSnapshot.State.Hidden);
long realSize = getVMSnapshotRealSize(vmSnapshotBeingDeleted);
int numberOfChildren = snapshotChildren.size();
@@ -248,10 +248,10 @@ private void mergeOldSiblingWithOldParentIfOldParentIsDead(VMSnapshotVO oldParen
if (oldParent.getCurrent()) {
snapshotVos = mergeCurrentDeltaOnSnapshot(oldParent, userVm, hostId, volumeTOs);
} else {
- List<VMSnapshotVO> oldSiblings = vmSnapshotDao.listByParent(oldParent.getId());
+ List<VMSnapshotVO> oldSiblings = vmSnapshotDao.listByParentAndStateIn(oldParent.getId(), VMSnapshot.State.Ready, VMSnapshot.State.Hidden);
if (oldSiblings.size() > 1) {
- logger.debug("The old snapshot [{}] is dead and still has more than one live snapshot. We will keep it on storage still.", oldParent.getUuid());
+ logger.debug("The old snapshot [{}] is dead and still has more than one live child snapshot. We will keep it on storage still.", oldParent.getUuid());
return;
}
@@ -366,7 +366,7 @@ private List<SnapshotVO> deleteSnapshot(VMSnapshotVO vmSnapshotVO, Long hostId)
private List<SnapshotVO> mergeSnapshots(VMSnapshotVO vmSnapshotVO, VMSnapshotVO childSnapshot, UserVmVO userVm, List<VolumeObjectTO> volumeObjectTOS, Long hostId) {
logger.debug("Merging VM snapshot [{}] with its child [{}].", vmSnapshotVO.getUuid(), childSnapshot.getUuid());
- List<VMSnapshotVO> snapshotGrandChildren = vmSnapshotDao.listByParent(childSnapshot.getId());
+ List<VMSnapshotVO> snapshotGrandChildren = vmSnapshotDao.listByParentAndStateIn(childSnapshot.getId(), VMSnapshot.State.Ready, VMSnapshot.State.Hidden);
if (userVm.getState().equals(VirtualMachine.State.Running) && !snapshotGrandChildren.isEmpty()) {
logger.debug("Removing VM snapshots that are part of the VM's [{}] current backing chain from the list of snapshots to be rebased.", userVm.getUuid());
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
index 8eee40fe21ae..0bc4276ab946 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
@@ -571,9 +571,8 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
* 1st parameter: VM's name;<br>
* 2nd parameter: disk's label (target.dev tag from VM's XML);<br>
* 3rd parameter: the absolute path of the base file;
- * 4th parameter: the flag '--delete', if Libvirt supports it. Libvirt started to support it on version <b>6.0.0</b>;
*/
- private static final String COMMAND_MERGE_SNAPSHOT = "virsh blockcommit %s %s --base %s --active --wait %s --pivot";
+ private static final String COMMAND_MERGE_SNAPSHOT = "virsh blockcommit %s %s --base %s";
public long getHypervisorLibvirtVersion() {
return hypervisorLibvirtVersion;
@@ -5878,7 +5877,7 @@ public void mergeSnapshotIntoBaseFile(Domain vm, String diskLabel, String baseFi
if (AgentPropertiesFileHandler.getPropertyValue(AgentProperties.LIBVIRT_EVENTS_ENABLED)) {
mergeSnapshotIntoBaseFileWithEventsAndConfigurableTimeout(vm, diskLabel, baseFilePath, topFilePath, active, snapshotName, volume, conn);
} else {
- mergeSnapshotIntoBaseFileWithoutEvents(vm, diskLabel, baseFilePath, snapshotName, volume, conn);
+ mergeSnapshotIntoBaseFileWithoutEvents(vm, diskLabel, baseFilePath, topFilePath, active, snapshotName, volume, conn);
}
}
@@ -5949,10 +5948,10 @@ protected void mergeSnapshotIntoBaseFileWithEventsAndConfigurableTimeout(Domain
* @param snapshotName Name of the snapshot;
* @throws LibvirtException
*/
- protected void mergeSnapshotIntoBaseFileWithoutEvents(Domain vm, String diskLabel, String baseFilePath, String snapshotName, VolumeObjectTO volume, Connect conn) throws LibvirtException {
+ protected void mergeSnapshotIntoBaseFileWithoutEvents(Domain vm, String diskLabel, String baseFilePath, String topFilePath, boolean active, String snapshotName, VolumeObjectTO volume, Connect conn) throws LibvirtException {
boolean isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit = LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(conn);
String vmName = vm.getName();
- String mergeCommand = String.format(COMMAND_MERGE_SNAPSHOT, vmName, diskLabel, baseFilePath, isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit ? "--delete" : "");
+ String mergeCommand = buildMergeCommand(vmName, diskLabel, baseFilePath, topFilePath, active, isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit);
String mergeResult = Script.runSimpleBashScript(mergeCommand);
if (mergeResult == null) {
@@ -5969,6 +5968,22 @@ protected void mergeSnapshotIntoBaseFileWithoutEvents(Domain vm, String diskLabe
throw new CloudRuntimeException(errorMsg);
}
+ protected String buildMergeCommand(String vmName, String diskLabel, String baseFilePath, String topFilePath, boolean active, boolean delete) {
+ StringBuilder cmd = new StringBuilder(COMMAND_MERGE_SNAPSHOT);
+ if (StringUtils.isNotEmpty(topFilePath)) {
+ cmd.append(" --top ");
+ cmd.append(topFilePath);
+ }
+ if (active) {
+ cmd.append(" --active --pivot");
+ }
+ if (delete) {
+ cmd.append(" --delete");
+ }
+ cmd.append(" --wait");
+ return String.format(cmd.toString(), vmName, diskLabel, baseFilePath);
+ }
+
/**
* This was created to facilitate testing.
* */
From 2433f7cb90c6d0cbaa70c2b6b236b9e62bafa536 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Jandre?=
<48719461+JoaoJandre@users.noreply.github.com>
Date: Wed, 2 Jul 2025 09:54:59 -0300
Subject: [PATCH 07/11] address reviews
---
.../api/storage/DeleteDiskOnlyVmSnapshotCommand.java | 2 +-
.../agent/api/storage/MergeDiskOnlyVmSnapshotCommand.java | 2 +-
.../api/storage/RevertDiskOnlyVmSnapshotCommand.java | 2 +-
.../java/org/apache/cloudstack/utils/qemu/QemuImg.java | 8 ++++----
4 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java b/core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java
index 368a506f2127..bf7bdd597360 100644
--- a/core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java
+++ b/core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java
@@ -39,6 +39,6 @@ public List<DataTO> getSnapshots() {
@Override
public boolean executeInSequence() {
- return true;
+ return false;
}
}
diff --git a/core/src/main/java/com/cloud/agent/api/storage/MergeDiskOnlyVmSnapshotCommand.java b/core/src/main/java/com/cloud/agent/api/storage/MergeDiskOnlyVmSnapshotCommand.java
index 463189b8b0b8..b6396c24d10a 100644
--- a/core/src/main/java/com/cloud/agent/api/storage/MergeDiskOnlyVmSnapshotCommand.java
+++ b/core/src/main/java/com/cloud/agent/api/storage/MergeDiskOnlyVmSnapshotCommand.java
@@ -49,7 +49,7 @@ public String getVmName() {
@Override
public boolean executeInSequence() {
- return true;
+ return false;
}
}
diff --git a/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java b/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java
index 6effca7dfa07..72bb92bcb10d 100644
--- a/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java
+++ b/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java
@@ -44,7 +44,7 @@ public String getVmName() {
@Override
public boolean executeInSequence() {
- return true;
+ return false;
}
}
diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java
index bdfba5769b92..61d40e0bb478 100644
--- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java
+++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java
@@ -835,17 +835,17 @@ public void resize(final QemuImgFile file, final long size) throws QemuImgExcept
* The file to be commited.
* @param base
* If base is not specified, the immediate backing file of the top image (which is {@code file}) will be used.
- * @param deleteFile
- * If true, the commited file(s) will be deleted.
+ * @param skipEmptyingFiles
+ * If true, the commited file(s) will not be emptied. If base is informed, skipEmptyingFiles is implied.
*/
- public void commit( QemuImgFile file, QemuImgFile base, boolean deleteFile) throws QemuImgException {
+ public void commit(QemuImgFile file, QemuImgFile base, boolean skipEmptyingFiles) throws QemuImgException {
if (file == null) {
throw new QemuImgException("File should not be null");
}
final Script s = new Script(_qemuImgPath, timeout);
s.add("commit");
- if (deleteFile) {
+ if (skipEmptyingFiles) {
s.add("-d");
}
From d5f8d055d6444005c25a695de3791e4570230597 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Jandre?=
<48719461+JoaoJandre@users.noreply.github.com>
Date: Mon, 7 Jul 2025 15:01:29 -0300
Subject: [PATCH 08/11] Address reviews
---
.../storage/vmsnapshot/DefaultVMSnapshotStrategy.java | 6 +++---
.../vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
index 8ec3d7843e7a..2d9b1e67e097 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
@@ -480,12 +480,12 @@ public boolean deleteVMSnapshotFromDB(VMSnapshot vmSnapshot, boolean unmanage) {
public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMemory) {
UserVmVO vm = userVmDao.findById(vmId);
if (State.Running.equals(vm.getState()) && !snapshotMemory) {
- logger.debug("Default VM snapshot cannot handle VM snapshot for [{}] as it is running and its memory will not be affected.", vm);
+ logger.debug("Default VM snapshot strategy cannot handle VM snapshot for [{}] as it is running and its memory will not be affected.", vm);
return StrategyPriority.CANT_HANDLE;
}
if (vmHasKvmDiskOnlySnapshot(vm)) {
- logger.debug("Default VM snapshot cannot handle VM snapshot for [{}] as it has a disk-only VM snapshot using kvmFileBasedStorageSnapshot strategy." +
+ logger.debug("Default VM snapshot strategy cannot handle VM snapshot for [{}] as it has a disk-only VM snapshot using kvmFileBasedStorageSnapshot strategy." +
"These two strategies are not compatible, as reverting a disk-only VM snapshot will erase newer disk-and-memory VM snapshots.", vm);
return StrategyPriority.CANT_HANDLE;
}
@@ -493,7 +493,7 @@ public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMe
List<VolumeVO> volumes = volumeDao.findByInstance(vmId);
for (VolumeVO volume : volumes) {
if (volume.getFormat() != ImageFormat.QCOW2) {
- logger.debug("Default VM snapshot cannot handle VM snapshot for [{}] as it has a volume [{}] that is not in the QCOW2 format.", vm, volume);
+ logger.debug("Default VM snapshot strategy cannot handle VM snapshot for [{}] as it has a volume [{}] that is not in the QCOW2 format.", vm, volume);
return StrategyPriority.CANT_HANDLE;
}
}
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
index 21f73f69479e..9ad564e32118 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
@@ -175,10 +175,10 @@ public boolean revertVMSnapshot(VMSnapshot vmSnapshot) {
}
VMSnapshotVO vmSnapshotBeingReverted = (VMSnapshotVO) vmSnapshot;
+ Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshotBeingReverted.getVmId());
transitStateWithoutThrow(vmSnapshotBeingReverted, VMSnapshot.Event.RevertRequested);
- Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshotBeingReverted.getVmId());
List<SnapshotDataStoreVO> volumeSnapshots = getVolumeSnapshotsAssociatedWithVmSnapshot(vmSnapshotBeingReverted);
List<SnapshotObjectTO> volumeSnapshotTos = volumeSnapshots.stream()
.map(snapshot -> (SnapshotObjectTO) snapshotDataFactory.getSnapshot(snapshot.getSnapshotId(), snapshot.getDataStoreId(), DataStoreRole.Primary).getTO())
From 588ca853bfe9eb425e3dd0ac74c27e364a62ec7a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Jandre?=
<48719461+JoaoJandre@users.noreply.github.com>
Date: Tue, 8 Jul 2025 08:40:25 -0300
Subject: [PATCH 09/11] address review and rename property
---
.../com/cloud/agent/properties/AgentProperties.java | 2 +-
.../KvmFileBasedStorageVmSnapshotStrategy.java | 7 ++++++-
.../kvm/resource/LibvirtComputingResource.java | 13 ++++++-------
.../kvm/resource/LibvirtComputingResourceTest.java | 8 ++++----
4 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java
index 75cb09fbb709..f1a2c57d5fc5 100644
--- a/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java
+++ b/agent/src/main/java/com/cloud/agent/properties/AgentProperties.java
@@ -161,7 +161,7 @@ public class AgentProperties{
* Data type: Integer.<br>
* Default value: <code>259200</code>
*/
- public static final Property<Integer> SNAPSHOT_MERGE_TIMEOUT = new Property<>("snapshot.merge.timeout", 60 * 60 * 72);
+ public static final Property<Integer> QCOW2_DELTA_MERGE_TIMEOUT = new Property<>("qcow2.delta.merge.timeout", 60 * 60 * 72);
/**
* This parameter sets the VM migration speed (in mbps). The default value is -1,<br>
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
index 9ad564e32118..18aa07ce05e8 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
@@ -230,7 +230,12 @@ private void updateSizeIfNeeded(List<SnapshotDataStoreVO> volumeSnapshots, Volum
return;
}
- resourceLimitManager.decrementResourceCount(volumeVO.getAccountId(), Resource.ResourceType.primary_storage, volumeVO.getSize() - snapshotRef.getSize());
+ long delta = volumeVO.getSize() - snapshotRef.getSize();
+ if (delta < 0){
+ resourceLimitManager.incrementResourceCount(volumeVO.getAccountId(), Resource.ResourceType.primary_storage, -delta);
+ } else {
+ resourceLimitManager.decrementResourceCount(volumeVO.getAccountId(), Resource.ResourceType.primary_storage, delta);
+ }
volumeVO.setSize(snapshotRef.getSize());
volumeObjectTO.setSize(snapshotRef.getSize());
volumeDao.update(volumeVO.getId(), volumeVO);
diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
index 0bc4276ab946..1351012b94cd 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
@@ -373,7 +373,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
public static final String CHECKPOINT_DELETE_COMMAND = "virsh checkpoint-delete --domain %s --checkpointname %s --metadata";
- protected int snapshotMergeTimeout;
+ protected int qcow2DeltaMergeTimeout;
private String modifyVlanPath;
private String versionStringPath;
@@ -1192,8 +1192,8 @@ public boolean configure(final String name, final Map<String, Object> params) th
cmdsTimeout = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.CMDS_TIMEOUT) * 1000;
noMemBalloon = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.VM_MEMBALLOON_DISABLE);
- snapshotMergeTimeout = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.SNAPSHOT_MERGE_TIMEOUT);
- snapshotMergeTimeout = snapshotMergeTimeout > 0 ? snapshotMergeTimeout : AgentProperties.SNAPSHOT_MERGE_TIMEOUT.getDefaultValue();
+ qcow2DeltaMergeTimeout = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.QCOW2_DELTA_MERGE_TIMEOUT);
+ qcow2DeltaMergeTimeout = qcow2DeltaMergeTimeout > 0 ? qcow2DeltaMergeTimeout : AgentProperties.QCOW2_DELTA_MERGE_TIMEOUT.getDefaultValue();
manualCpuSpeed = AgentPropertiesFileHandler.getPropertyValue(AgentProperties.HOST_CPU_MANUAL_SPEED_MHZ);
@@ -5913,7 +5913,7 @@ protected void mergeSnapshotIntoBaseFileWithEventsAndConfigurableTimeout(Domain
" The job will be left running to avoid data corruption, but ACS will return an error and volume [%s] will need to be normalized manually. If the commit" +
" involved the active image, the pivot will need to be manually done.", topFilePath, baseFilePath, snapshotName, vmName, volume);
try {
- if (!semaphore.tryAcquire(snapshotMergeTimeout, TimeUnit.SECONDS)) {
+ if (!semaphore.tryAcquire(qcow2DeltaMergeTimeout, TimeUnit.SECONDS)) {
throw new CloudRuntimeException("Timed out while waiting for " + errorMessage);
}
} catch (InterruptedException e) {
@@ -5999,7 +5999,7 @@ protected Semaphore getSemaphoreToWaitForMerge() {
}
protected void checkBlockCommitProgress(Domain vm, String diskLabel, String vmName, String snapshotName, String topFilePath, String baseFilePath) {
- int timeout = snapshotMergeTimeout;
+ int timeout = qcow2DeltaMergeTimeout;
DomainBlockJobInfo result;
long lastCommittedBytes = 0;
long endBytes = 0;
@@ -6034,8 +6034,7 @@ protected void checkBlockCommitProgress(Domain vm, String diskLabel, String vmNa
lastCommittedBytes = currentCommittedBytes;
endBytes = result.end;
}
- logger.warn("Block commit {} has timed out after waiting at least {} seconds. The progress of the operation was [{}] of [{}].", partialLog,
- snapshotMergeTimeout, lastCommittedBytes, endBytes);
+ logger.warn("Block commit {} has timed out after waiting at least {} seconds. The progress of the operation was [{}] of [{}].", partialLog, qcow2DeltaMergeTimeout, lastCommittedBytes, endBytes);
}
/**
diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
index e2aa48cf8fbb..ea380f1a8a74 100644
--- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
+++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
@@ -6644,7 +6644,7 @@ public void getSnapshotTemporaryPathTestReturnExpectedResult(){
@Test
public void mergeSnapshotIntoBaseFileTestActiveAndDeleteFlags() throws Exception {
- libvirtComputingResourceSpy.snapshotMergeTimeout = 10;
+ libvirtComputingResourceSpy.qcow2DeltaMergeTimeout = 10;
try (MockedStatic<LibvirtUtilitiesHelper> libvirtUtilitiesHelperMockedStatic = Mockito.mockStatic(LibvirtUtilitiesHelper.class);
MockedStatic<ThreadContext> threadContextMockedStatic = Mockito.mockStatic(ThreadContext.class)) {
@@ -6698,7 +6698,7 @@ public void mergeSnapshotIntoBaseFileTestActiveFlag() throws Exception {
public void mergeSnapshotIntoBaseFileTestDeleteFlag() throws Exception {
try (MockedStatic<LibvirtUtilitiesHelper> libvirtUtilitiesHelperMockedStatic = Mockito.mockStatic(LibvirtUtilitiesHelper.class);
MockedStatic<ThreadContext> threadContextMockedStatic = Mockito.mockStatic(ThreadContext.class)) {
- libvirtComputingResourceSpy.snapshotMergeTimeout = 10;
+ libvirtComputingResourceSpy.qcow2DeltaMergeTimeout = 10;
libvirtUtilitiesHelperMockedStatic.when(() -> LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(Mockito.any())).thenReturn(true);
Mockito.doReturn(new Semaphore(1)).when(libvirtComputingResourceSpy).getSemaphoreToWaitForMerge();
threadContextMockedStatic.when(() -> ThreadContext.get(Mockito.anyString())).thenReturn("logid");
@@ -6722,7 +6722,7 @@ public void mergeSnapshotIntoBaseFileTestDeleteFlag() throws Exception {
public void mergeSnapshotIntoBaseFileTestNoFlags() throws Exception {
try (MockedStatic<LibvirtUtilitiesHelper> libvirtUtilitiesHelperMockedStatic = Mockito.mockStatic(LibvirtUtilitiesHelper.class);
MockedStatic<ThreadContext> threadContextMockedStatic = Mockito.mockStatic(ThreadContext.class)) {
- libvirtComputingResourceSpy.snapshotMergeTimeout = 10;
+ libvirtComputingResourceSpy.qcow2DeltaMergeTimeout = 10;
libvirtUtilitiesHelperMockedStatic.when(() -> LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(Mockito.any())).thenReturn(false);
Mockito.doReturn(new Semaphore(1)).when(libvirtComputingResourceSpy).getSemaphoreToWaitForMerge();
threadContextMockedStatic.when(() -> ThreadContext.get(Mockito.anyString())).thenReturn("logid");
@@ -6746,7 +6746,7 @@ public void mergeSnapshotIntoBaseFileTestNoFlags() throws Exception {
public void mergeSnapshotIntoBaseFileTestMergeFailsThrowException() throws Exception {
try (MockedStatic<LibvirtUtilitiesHelper> libvirtUtilitiesHelperMockedStatic = Mockito.mockStatic(LibvirtUtilitiesHelper.class);
MockedStatic<ThreadContext> threadContextMockedStatic = Mockito.mockStatic(ThreadContext.class)) {
- libvirtComputingResourceSpy.snapshotMergeTimeout = 10;
+ libvirtComputingResourceSpy.qcow2DeltaMergeTimeout = 10;
libvirtUtilitiesHelperMockedStatic.when(() -> LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(Mockito.any())).thenReturn(false);
Mockito.doReturn(new Semaphore(1)).when(libvirtComputingResourceSpy).getSemaphoreToWaitForMerge();
threadContextMockedStatic.when(() -> ThreadContext.get(Mockito.anyString())).thenReturn("logid");
From ba3f0c8d3d7fed570ee4ac5ede9cf1110bfd4aae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Jandre?=
<48719461+JoaoJandre@users.noreply.github.com>
Date: Tue, 8 Jul 2025 13:29:48 -0300
Subject: [PATCH 10/11] Update
engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
Co-authored-by: Fabricio Duarte <fabricio.duarte.jr@gmail.com>
---
.../vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
index 18aa07ce05e8..c3eb9592dfe4 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
@@ -231,7 +231,7 @@ private void updateSizeIfNeeded(List<SnapshotDataStoreVO> volumeSnapshots, Volum
}
long delta = volumeVO.getSize() - snapshotRef.getSize();
- if (delta < 0){
+ if (delta < 0) {
resourceLimitManager.incrementResourceCount(volumeVO.getAccountId(), Resource.ResourceType.primary_storage, -delta);
} else {
resourceLimitManager.decrementResourceCount(volumeVO.getAccountId(), Resource.ResourceType.primary_storage, delta);
From 1c6db4f1ede49ab7af1c5d6cff82c5a191a809cf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20Jandre?=
<48719461+JoaoJandre@users.noreply.github.com>
Date: Mon, 14 Jul 2025 11:44:25 -0300
Subject: [PATCH 11/11] address review
---
.../storage/snapshot/DefaultSnapshotStrategy.java | 2 +-
.../vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java | 6 +++---
.../org/apache/cloudstack/backup/NASBackupProvider.java | 2 +-
.../main/java/com/cloud/storage/VolumeApiServiceImpl.java | 4 ++--
.../com/cloud/storage/snapshot/SnapshotManagerImpl.java | 2 ++
5 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
index 8db067131bf7..aedc2a12d0f0 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/DefaultSnapshotStrategy.java
@@ -660,7 +660,7 @@ private StrategyPriority validateVmSnapshot(Snapshot snapshot) {
for (VMSnapshotVO vmSnapshotVO : vmSnapshotDao.findByVmAndByType(vm.getId(), VMSnapshot.Type.Disk)) {
List<VMSnapshotDetailsVO> vmSnapshotDetails = vmSnapshotDetailsDao.listDetails(vmSnapshotVO.getId());
- if (vmSnapshotDetails.stream().anyMatch(vmSnapshotDetailsVO -> vmSnapshotDetailsVO.getName().equals(VolumeApiServiceImpl.KVM_FILE_BASED_STORAGE_SNAPSHOT))) {
+ if (vmSnapshotDetails.stream().anyMatch(vmSnapshotDetailsVO -> VolumeApiServiceImpl.KVM_FILE_BASED_STORAGE_SNAPSHOT.equals(vmSnapshotDetailsVO.getName()))) {
logger.warn("VM [{}] already has KVM File-Based storage VM snapshots. These VM snapshots and volume snapshots are not supported " +
"together for KVM. As restoring volume snapshots will erase the VM snapshots and cause data loss.", vm.getUuid());
return StrategyPriority.CANT_HANDLE;
diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
index 18aa07ce05e8..efa630eb5c7c 100644
--- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java
@@ -170,7 +170,7 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) {
@Override
public boolean revertVMSnapshot(VMSnapshot vmSnapshot) {
UserVmVO userVm = userVmDao.findById(vmSnapshot.getVmId());
- if (!userVm.getState().equals(VirtualMachine.State.Stopped)) {
+ if (!VirtualMachine.State.Stopped.equals(userVm.getState())) {
throw new CloudRuntimeException("VM must be stopped to revert disk-only VM snapshot.");
}
@@ -373,14 +373,14 @@ private List<SnapshotVO> mergeSnapshots(VMSnapshotVO vmSnapshotVO, VMSnapshotVO
List<VMSnapshotVO> snapshotGrandChildren = vmSnapshotDao.listByParentAndStateIn(childSnapshot.getId(), VMSnapshot.State.Ready, VMSnapshot.State.Hidden);
- if (userVm.getState().equals(VirtualMachine.State.Running) && !snapshotGrandChildren.isEmpty()) {
+ if (VirtualMachine.State.Running.equals(userVm.getState()) && !snapshotGrandChildren.isEmpty()) {
logger.debug("Removing VM snapshots that are part of the VM's [{}] current backing chain from the list of snapshots to be rebased.", userVm.getUuid());
removeCurrentBackingChainSnapshotFromVmSnapshotList(snapshotGrandChildren, userVm);
}
List<SnapshotMergeTreeTO> snapshotMergeTreeToList = generateSnapshotMergeTrees(vmSnapshotVO, childSnapshot, snapshotGrandChildren);
- if (childSnapshot.getCurrent() && !userVm.getState().equals(VirtualMachine.State.Running)) {
+ if (childSnapshot.getCurrent() && !VirtualMachine.State.Running.equals(userVm.getState())) {
for (VolumeObjectTO volumeObjectTO : volumeObjectTOS) {
snapshotMergeTreeToList.stream().filter(snapshotTree -> Objects.equals(((SnapshotObjectTO) snapshotTree.getParent()).getVolume().getId(), volumeObjectTO.getId()))
.findFirst()
diff --git a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java
index bc90753fe632..3f8fa9dd4286 100644
--- a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java
+++ b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java
@@ -415,7 +415,7 @@ public Backup createNewBackupEntryForRestorePoint(Backup.RestorePoint restorePoi
public boolean assignVMToBackupOffering(VirtualMachine vm, BackupOffering backupOffering) {
for (VMSnapshotVO vmSnapshotVO : vmSnapshotDao.findByVmAndByType(vm.getId(), VMSnapshot.Type.Disk)) {
List<VMSnapshotDetailsVO> vmSnapshotDetails = vmSnapshotDetailsDao.listDetails(vmSnapshotVO.getId());
- if (vmSnapshotDetails.stream().anyMatch(vmSnapshotDetailsVO -> vmSnapshotDetailsVO.getName().equals(VolumeApiServiceImpl.KVM_FILE_BASED_STORAGE_SNAPSHOT))) {
+ if (vmSnapshotDetails.stream().anyMatch(vmSnapshotDetailsVO -> VolumeApiServiceImpl.KVM_FILE_BASED_STORAGE_SNAPSHOT.equals(vmSnapshotDetailsVO.getName()))) {
logger.warn("VM [{}] has VM snapshots using the KvmFileBasedStorageVmSnapshot Strategy; this provider does not support backups on VMs with these snapshots!");
return false;
}
diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
index 245125bc240b..f2c653943ee2 100644
--- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
@@ -1422,7 +1422,7 @@ protected boolean vmHasVmSnapshotsExceptKvmDiskOnlySnapshots(long instanceId) {
}
List<VMSnapshotDetailsVO> vmSnapshotDetails = vmSnapshotDetailsDao.listDetails(vmSnapshotVO.getId());
if (vmSnapshotDetails.stream().
- noneMatch(vmSnapshotDetailsVO -> vmSnapshotDetailsVO.getName().equals(KVM_FILE_BASED_STORAGE_SNAPSHOT))) {
+ noneMatch(vmSnapshotDetailsVO -> KVM_FILE_BASED_STORAGE_SNAPSHOT.equals(vmSnapshotDetailsVO.getName()))) {
return true;
}
}
@@ -4069,7 +4069,7 @@ public Snapshot allocSnapshotForVm(Long vmId, Long volumeId, String snapshotName
throw new InvalidParameterValueException("Cannot perform this operation, unsupported on storage pool type " + storagePool.getPoolType());
}
- if (vmSnapshotDetailsDao.listDetails(vmSnapshotId).stream().anyMatch(vmSnapshotDetailsVO -> vmSnapshotDetailsVO.getName().equals(KVM_FILE_BASED_STORAGE_SNAPSHOT))) {
+ if (vmSnapshotDetailsDao.listDetails(vmSnapshotId).stream().anyMatch(vmSnapshotDetailsVO -> KVM_FILE_BASED_STORAGE_SNAPSHOT.equals(vmSnapshotDetailsVO.getName()))) {
throw new InvalidParameterValueException("Cannot perform this operation, unsupported VM snapshot type.");
}
diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
index 4dd074944938..66d474966b82 100755
--- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
+++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
@@ -1571,6 +1571,8 @@ public SnapshotInfo takeSnapshot(VolumeInfo volume) throws ResourceAllocationExc
SnapshotStrategy snapshotStrategy = _storageStrategyFactory.getSnapshotStrategy(snapshot, SnapshotOperation.TAKE);
if (snapshotStrategy == null) {
+ _snapshotDao.remove(snapshotId);
+ logger.debug("No strategy found for creation of snapshot [{}], removing its record from the database.", snapshot);
throw new CloudRuntimeException(String.format("Can't find snapshot strategy to deal with snapshot:%s", snapshot.getSnapshotVO()));
}