This is an automated email from the ASF dual-hosted git repository.
dahn pushed a commit to branch 4.18
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.18 by this push:
new 9b8eaeea78a Fix: Convert volume to another directory instead of
copying it while taking volume snapshots on KVM (#8041)
9b8eaeea78a is described below
commit 9b8eaeea78a203926f0fc00e948ce30cfb2eb519
Author: Daniel Augusto Veronezi Salvador
<[email protected]>
AuthorDate: Fri Oct 6 04:47:34 2023 -0300
Fix: Convert volume to another directory instead of copying it while taking
volume snapshots on KVM (#8041)
---
.../kvm/storage/KVMStorageProcessor.java | 43 +++++++++++-------
.../kvm/storage/KVMStorageProcessorTest.java | 51 +++++++++++++++++-----
2 files changed, 66 insertions(+), 28 deletions(-)
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 69c28d4350a..ae0fa637bf0 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
@@ -1715,11 +1715,11 @@ public class KVMStorageProcessor implements
StorageProcessor {
snapshotPath =
getSnapshotPathInPrimaryStorage(primaryPool.getLocalPath(), snapshotName);
String diskLabel =
takeVolumeSnapshot(resource.getDisks(conn, vmName), snapshotName, diskPath, vm);
- String copyResult =
copySnapshotToPrimaryStorageDir(primaryPool, diskPath, snapshotPath, volume);
+ String convertResult =
convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, diskPath,
snapshotPath, volume, cmd.getWait());
mergeSnapshotIntoBaseFile(vm, diskLabel, diskPath,
snapshotName, volume, conn);
- validateCopyResult(copyResult, snapshotPath);
+ validateConvertResult(convertResult, snapshotPath);
} catch (LibvirtException e) {
if
(!e.getMessage().contains(LIBVIRT_OPERATION_NOT_SUPPORTED_MESSAGE)) {
throw e;
@@ -1784,8 +1784,8 @@ public class KVMStorageProcessor implements
StorageProcessor {
}
} else {
snapshotPath =
getSnapshotPathInPrimaryStorage(primaryPool.getLocalPath(), snapshotName);
- String copyResult =
copySnapshotToPrimaryStorageDir(primaryPool, diskPath, snapshotPath, volume);
- validateCopyResult(copyResult, snapshotPath);
+ String convertResult =
convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, diskPath,
snapshotPath, volume, cmd.getWait());
+ validateConvertResult(convertResult, snapshotPath);
}
}
@@ -1838,13 +1838,13 @@ public class KVMStorageProcessor implements
StorageProcessor {
s_logger.debug(String.format("Full VM Snapshot [%s] of VM [%s] took
[%s] seconds to finish.", snapshotName, vmName, (System.currentTimeMillis() -
start)/1000));
}
- protected void validateCopyResult(String copyResult, String snapshotPath)
throws CloudRuntimeException, IOException {
- if (copyResult == null) {
+ protected void validateConvertResult(String convertResult, String
snapshotPath) throws CloudRuntimeException, IOException {
+ if (convertResult == null) {
return;
}
Files.deleteIfExists(Paths.get(snapshotPath));
- throw new CloudRuntimeException(copyResult);
+ throw new CloudRuntimeException(convertResult);
}
/**
@@ -1901,20 +1901,31 @@ public class KVMStorageProcessor implements
StorageProcessor {
}
/**
- * 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());
}
}
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 98a5b130bae..aab0ce19559 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
@@ -39,6 +39,9 @@ import java.util.List;
import java.util.Set;
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.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -91,6 +94,9 @@ public class KVMStorageProcessorTest {
@Mock
Connect connectMock;
+ @Mock
+ QemuImg qemuImgMock;
+
@Mock
LibvirtDomainXMLParser libvirtDomainXMLParserMock;
@Mock
@@ -251,32 +257,53 @@ public class KVMStorageProcessorTest {
@Test
@PrepareForTest(KVMStorageProcessor.class)
- public void
validateCopySnapshotToPrimaryStorageDirFailToCopyReturnErrorMessage() throws
Exception {
+ public void
convertBaseFileToSnapshotFileInPrimaryStorageDirTestFailToConvertWithQemuImgExceptionReturnErrorMessage()
throws Exception {
String baseFile = "baseFile";
String snapshotPath = "snapshotPath";
String errorMessage = "error";
- String expectedResult = String.format("Unable to copy %s snapshot [%s]
to [%s] due to [%s].", volumeObjectToMock, baseFile, snapshotPath,
errorMessage);
+ String expectedResult = String.format("Failed to convert %s snapshot
of volume [%s] to [%s] due to [%s].", volumeObjectToMock, baseFile,
snapshotPath, errorMessage);
Mockito.doReturn(true).when(kvmStoragePoolMock).createFolder(Mockito.anyString());
- PowerMockito.mockStatic(Files.class);
- PowerMockito.when(Files.copy(Mockito.any(Path.class),
Mockito.any(Path.class), Mockito.any())).thenThrow(new
IOException(errorMessage));
- String result =
storageProcessorSpy.copySnapshotToPrimaryStorageDir(kvmStoragePoolMock,
baseFile, snapshotPath, volumeObjectToMock);
+
PowerMockito.whenNew(QemuImg.class).withArguments(Mockito.anyInt()).thenReturn(qemuImgMock);
+ Mockito.doThrow(new
QemuImgException(errorMessage)).when(qemuImgMock).convert(Mockito.any(QemuImgFile.class),
Mockito.any(QemuImgFile.class));
+
+ String result =
storageProcessorSpy.convertBaseFileToSnapshotFileInPrimaryStorageDir(kvmStoragePoolMock,
baseFile, snapshotPath, volumeObjectToMock, 1);
Assert.assertEquals(expectedResult, result);
}
@Test
@PrepareForTest(KVMStorageProcessor.class)
- public void validateCopySnapshotToPrimaryStorageDirCopySuccessReturnNull()
throws Exception {
+ public void
convertBaseFileToSnapshotFileInPrimaryStorageDirTestFailToConvertWithLibvirtExceptionReturnErrorMessage()
throws Exception {
String baseFile = "baseFile";
String snapshotPath = "snapshotPath";
+ String errorMessage = "null";
+ String expectedResult = String.format("Failed to convert %s snapshot
of volume [%s] to [%s] due to [%s].", volumeObjectToMock, baseFile,
snapshotPath, errorMessage);
Mockito.doReturn(true).when(kvmStoragePoolMock).createFolder(Mockito.anyString());
- PowerMockito.mockStatic(Files.class);
- PowerMockito.when(Files.copy(Mockito.any(Path.class),
Mockito.any(Path.class), Mockito.any())).thenReturn(null);
- String result =
storageProcessorSpy.copySnapshotToPrimaryStorageDir(kvmStoragePoolMock,
baseFile, snapshotPath, volumeObjectToMock);
+
PowerMockito.whenNew(QemuImg.class).withArguments(Mockito.anyInt()).thenReturn(qemuImgMock);
+
Mockito.doThrow(LibvirtException.class).when(qemuImgMock).convert(Mockito.any(QemuImgFile.class),
Mockito.any(QemuImgFile.class));
+
+ String result =
storageProcessorSpy.convertBaseFileToSnapshotFileInPrimaryStorageDir(kvmStoragePoolMock,
baseFile, snapshotPath, volumeObjectToMock, 1);
+
+ Assert.assertEquals(expectedResult, result);
+ }
+
+
+ @Test
+ @PrepareForTest(KVMStorageProcessor.class)
+ public void
convertBaseFileToSnapshotFileInPrimaryStorageDirTestConvertSuccessReturnNull()
throws Exception {
+ String baseFile = "baseFile";
+ String snapshotPath = "snapshotPath";
+
+
Mockito.doReturn(true).when(kvmStoragePoolMock).createFolder(Mockito.anyString());
+
+
PowerMockito.whenNew(QemuImg.class).withArguments(Mockito.anyInt()).thenReturn(qemuImgMock);
+
Mockito.doNothing().when(qemuImgMock).convert(Mockito.any(QemuImgFile.class),
Mockito.any(QemuImgFile.class));
+
+ String result =
storageProcessorSpy.convertBaseFileToSnapshotFileInPrimaryStorageDir(kvmStoragePoolMock,
baseFile, snapshotPath, volumeObjectToMock, 1);
Assert.assertNull(result);
}
@@ -321,14 +348,14 @@ public class KVMStorageProcessorTest {
@Test
public void validateValidateCopyResultResultIsNullReturn() throws
CloudRuntimeException, IOException{
- storageProcessorSpy.validateCopyResult(null, "");
+ storageProcessorSpy.validateConvertResult(null, "");
}
@Test (expected = IOException.class)
public void validateValidateCopyResultFailToDeleteThrowIOException()
throws CloudRuntimeException, IOException{
PowerMockito.mockStatic(Files.class);
PowerMockito.when(Files.deleteIfExists(Mockito.any())).thenThrow(new
IOException(""));
- storageProcessorSpy.validateCopyResult("", "");
+ storageProcessorSpy.validateConvertResult("", "");
}
@Test (expected = CloudRuntimeException.class)
@@ -336,7 +363,7 @@ public class KVMStorageProcessorTest {
public void
validateValidateCopyResulResultNotNullThrowCloudRuntimeException() throws
CloudRuntimeException, IOException{
PowerMockito.mockStatic(Files.class);
PowerMockito.when(Files.deleteIfExists(Mockito.any())).thenReturn(true);
- storageProcessorSpy.validateCopyResult("", "");
+ storageProcessorSpy.validateConvertResult("", "");
}
@Test (expected = CloudRuntimeException.class)
