You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2023/10/06 07:47:42 UTC
[cloudstack] branch 4.18 updated: Fix: Convert volume to another directory instead of copying it while taking volume snapshots on KVM (#8041)
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 <38...@users.noreply.github.com>
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)