Fix: Convert volume to another directory instead of copying it while taking volume snapshots on KVM (#8041)

This commit is contained in:
Daniel Augusto Veronezi Salvador 2023-10-06 04:47:34 -03:00 committed by GitHub
parent 51add0a066
commit 9b8eaeea78
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 66 additions and 28 deletions

View File

@ -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());
}
}

View File

@ -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.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.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.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)