slavkap commented on a change in pull request #5297:
URL: https://github.com/apache/cloudstack/pull/5297#discussion_r782155384
##########
File path:
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
##########
@@ -1585,17 +1607,214 @@ public Answer createSnapshot(final CreateObjectCommand
cmd) {
s_logger.debug("Failed to manage snapshot: " + result);
return new CreateObjectAnswer("Failed to manage
snapshot: " + result);
}
+ } else {
+ snapshotPath =
getSnapshotPathInPrimaryStorage(primaryPool.getLocalPath(), snapshotName);
+ String copyResult =
copySnapshotToPrimaryStorageDir(primaryPool, diskPath, snapshotPath, volume);
+ validateCopyResult(copyResult, snapshotPath);
}
}
final SnapshotObjectTO newSnapshot = new SnapshotObjectTO();
- // NOTE: sort of hack, we'd better just put snapshtoName
- newSnapshot.setPath(disk.getPath() + File.separator +
snapshotName);
+
+ newSnapshot.setPath(snapshotPath);
return new CreateObjectAnswer(newSnapshot);
- } catch (final LibvirtException e) {
- s_logger.debug("Failed to manage snapshot: ", e);
- return new CreateObjectAnswer("Failed to manage snapshot: " +
e.toString());
+ } catch (CloudRuntimeException | LibvirtException | IOException ex) {
+ String errorMsg = String.format("Failed take snapshot for volume
[%s], in VM [%s], due to [%s].", volume, vmName, ex.getMessage());
+ s_logger.error(errorMsg, ex);
+ return new CreateObjectAnswer(errorMsg);
+ }
+ }
+
+ protected void validateCopyResult(String copyResult, String snapshotPath)
throws CloudRuntimeException, IOException {
+ if (copyResult == null) {
+ return;
+ }
+
+ Files.deleteIfExists(Paths.get(snapshotPath));
+ throw new CloudRuntimeException(copyResult);
+ }
+
+ /**
+ * 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) {
+ s_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);
+
+ s_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 boolean manuallyDeleteUnusedSnapshotFile(boolean
isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit, String snapshotPath) {
Review comment:
maybe it doesn't have to be `boolean`
```suggestion
protected void manuallyDeleteUnusedSnapshotFile(boolean
isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit, String snapshotPath) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]