DaanHoogland commented on code in PR #8041:
URL: https://github.com/apache/cloudstack/pull/8041#discussion_r1345803580
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java:
##########
@@ -1784,8 +1784,8 @@ public Answer createSnapshot(final CreateObjectCommand
cmd) {
}
} else {
snapshotPath =
getSnapshotPathInPrimaryStorage(primaryPool.getLocalPath(), snapshotName);
- String copyResult =
copySnapshotToPrimaryStorageDir(primaryPool, diskPath, snapshotPath, volume);
- validateCopyResult(copyResult, snapshotPath);
+ String copyResult =
convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, diskPath,
snapshotPath, volume, cmd.getWait());
+ validateConvertResult(copyResult, snapshotPath);
Review Comment:
rename these as well?
```suggestion
String convertResult =
convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, diskPath,
snapshotPath, volume, cmd.getWait());
validateConvertResult(convertResult, snapshotPath);
```
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java:
##########
@@ -1901,20 +1901,31 @@ protected void manuallyDeleteUnusedSnapshotFile(boolean
isLibvirtSupportingFlagD
}
/**
- * Creates the snapshot directory in the primary storage, if it does not
exist; then copies the base file (VM's old writing file) to the snapshot dir..
+ * 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;
- * @param baseFile Base file of VM, which will be copied;
- * @param snapshotPath Path to copy the base file;
- * @return null if copies successfully or a error message.
+ * @param baseFile Base file of VM, which will be converted;
+ * @param snapshotPath Path to convert the base file;
+ * @return null if the conversion occurs successfully or an error message
that must be handled.
*/
- protected String copySnapshotToPrimaryStorageDir(KVMStoragePool
primaryPool, String baseFile, String snapshotPath, VolumeObjectTO volume) {
+ protected String
convertBaseFileToSnapshotFileInPrimaryStorageDir(KVMStoragePool primaryPool,
String baseFile, String snapshotPath, VolumeObjectTO volume, int wait) {
try {
+ s_logger.debug(String.format("Trying to convert volume [%s] (%s)
to snapshot [%s].", volume, baseFile, snapshotPath));
+
primaryPool.createFolder(TemplateConstants.DEFAULT_SNAPSHOT_ROOT_DIR);
- Files.copy(Paths.get(baseFile), Paths.get(snapshotPath));
- s_logger.debug(String.format("Copied %s snapshot from [%s] to
[%s].", volume, baseFile, snapshotPath));
+
+ QemuImgFile srcFile = new QemuImgFile(baseFile);
+ srcFile.setFormat(PhysicalDiskFormat.QCOW2);
+
+ QemuImgFile destFile = new QemuImgFile(snapshotPath);
+ destFile.setFormat(PhysicalDiskFormat.QCOW2);
+
+ QemuImg q = new QemuImg(wait);
+ q.convert(srcFile, destFile);
+
+ s_logger.debug(String.format("Converted volume [%s] (from path
\"%s\") to snapshot [%s].", volume, baseFile, snapshotPath));
return null;
- } catch (IOException ex) {
- return String.format("Unable to copy %s snapshot [%s] to [%s] due
to [%s].", volume, baseFile, snapshotPath, ex.getMessage());
+ } catch (QemuImgException | LibvirtException ex) {
+ return String.format("Failed to convert %s snapshot of volume [%s]
to [%s] due to [%s].", volume, baseFile, snapshotPath, ex.getMessage());
Review Comment:
wouldn't it be convenient to throw the
`CloudRuntimeException(convertResult)` instead of in the validate method?
--
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]
