GutoVeronezi commented on code in PR #8041:
URL: https://github.com/apache/cloudstack/pull/8041#discussion_r1346726123
##########
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:
@DaanHoogland
Here we return a `String` instead of throwing an exception so we can handle
the merge of the delta before throwing the exception. We could refactor
`KVMStorageProcessor#createSnapshot` a little bit to work with a
`try-catch-finally`. Though, I think we could do it afterward, so we focus on
the issue fixing and reduce the scope of changes in this PR. What do you think?
--
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]
