Skip to content

Commit 8b07b66

Browse files
slavkapslavkap
authored
Fix volume snapshot of encrypted NFS/StorPool volume (#8873)
* Fix volume snapshot of encrypted NFS/StorPool volume * remove comments * removed invoking the real qemu convert command * fix UnsatisfiedLink error in unit tests * addressed comments extracted method
1 parent 3e30283 commit 8b07b66

File tree

3 files changed

+188
-126
lines changed

3 files changed

+188
-126
lines changed

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java‎

Lines changed: 102 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -18,73 +18,6 @@
1818
*/
1919
package com.cloud.hypervisor.kvm.storage;
2020

21-
import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
22-
import static com.cloud.utils.storage.S3.S3Utils.putFile;
23-
24-
import java.io.File;
25-
import java.io.FileNotFoundException;
26-
import java.io.FileOutputStream;
27-
import java.io.IOException;
28-
import java.nio.file.Files;
29-
import java.nio.file.Paths;
30-
import java.text.DateFormat;
31-
import java.text.SimpleDateFormat;
32-
import java.util.Arrays;
33-
import java.util.Date;
34-
import java.util.HashMap;
35-
import java.util.HashSet;
36-
import java.util.List;
37-
import java.util.Map;
38-
import java.util.Set;
39-
import java.util.UUID;
40-
import java.util.stream.Collectors;
41-
42-
import javax.naming.ConfigurationException;
43-
44-
import org.apache.cloudstack.agent.directdownload.DirectDownloadAnswer;
45-
import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand;
46-
import org.apache.cloudstack.direct.download.DirectDownloadHelper;
47-
import org.apache.cloudstack.direct.download.DirectTemplateDownloader;
48-
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
49-
import org.apache.cloudstack.storage.command.AttachAnswer;
50-
import org.apache.cloudstack.storage.command.AttachCommand;
51-
import org.apache.cloudstack.storage.command.CheckDataStoreStoragePolicyComplainceCommand;
52-
import org.apache.cloudstack.storage.command.CopyCmdAnswer;
53-
import org.apache.cloudstack.storage.command.CopyCommand;
54-
import org.apache.cloudstack.storage.command.CreateObjectAnswer;
55-
import org.apache.cloudstack.storage.command.CreateObjectCommand;
56-
import org.apache.cloudstack.storage.command.DeleteCommand;
57-
import org.apache.cloudstack.storage.command.DettachAnswer;
58-
import org.apache.cloudstack.storage.command.DettachCommand;
59-
import org.apache.cloudstack.storage.command.ForgetObjectCmd;
60-
import org.apache.cloudstack.storage.command.IntroduceObjectCmd;
61-
import org.apache.cloudstack.storage.command.ResignatureAnswer;
62-
import org.apache.cloudstack.storage.command.ResignatureCommand;
63-
import org.apache.cloudstack.storage.command.SnapshotAndCopyAnswer;
64-
import org.apache.cloudstack.storage.command.SnapshotAndCopyCommand;
65-
import org.apache.cloudstack.storage.command.SyncVolumePathCommand;
66-
import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
67-
import org.apache.cloudstack.storage.to.SnapshotObjectTO;
68-
import org.apache.cloudstack.storage.to.TemplateObjectTO;
69-
import org.apache.cloudstack.storage.to.VolumeObjectTO;
70-
import org.apache.cloudstack.utils.qemu.QemuImg;
71-
import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
72-
import org.apache.cloudstack.utils.qemu.QemuImgException;
73-
import org.apache.cloudstack.utils.qemu.QemuImgFile;
74-
import org.apache.cloudstack.utils.qemu.QemuObject;
75-
import org.apache.commons.collections.MapUtils;
76-
import org.apache.commons.io.FileUtils;
77-
import org.apache.commons.lang3.BooleanUtils;
78-
import org.apache.commons.lang3.StringUtils;
79-
import org.apache.commons.lang3.builder.ToStringBuilder;
80-
import org.apache.commons.lang3.builder.ToStringStyle;
81-
import org.apache.log4j.Logger;
82-
import org.libvirt.Connect;
83-
import org.libvirt.Domain;
84-
import org.libvirt.DomainInfo;
85-
import org.libvirt.DomainSnapshot;
86-
import org.libvirt.LibvirtException;
87-
8821
import com.ceph.rados.IoCTX;
8922
import com.ceph.rados.Rados;
9023
import com.ceph.rados.exceptions.ErrorCode;
@@ -133,6 +66,75 @@
13366
import com.cloud.utils.script.Script;
13467
import com.cloud.utils.storage.S3.S3Utils;
13568
import com.cloud.vm.VmDetailConstants;
69+
import org.apache.cloudstack.agent.directdownload.DirectDownloadAnswer;
70+
import org.apache.cloudstack.agent.directdownload.DirectDownloadCommand;
71+
import org.apache.cloudstack.direct.download.DirectDownloadHelper;
72+
import org.apache.cloudstack.direct.download.DirectTemplateDownloader;
73+
import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
74+
import org.apache.cloudstack.storage.command.AttachAnswer;
75+
import org.apache.cloudstack.storage.command.AttachCommand;
76+
import org.apache.cloudstack.storage.command.CheckDataStoreStoragePolicyComplainceCommand;
77+
import org.apache.cloudstack.storage.command.CopyCmdAnswer;
78+
import org.apache.cloudstack.storage.command.CopyCommand;
79+
import org.apache.cloudstack.storage.command.CreateObjectAnswer;
80+
import org.apache.cloudstack.storage.command.CreateObjectCommand;
81+
import org.apache.cloudstack.storage.command.DeleteCommand;
82+
import org.apache.cloudstack.storage.command.DettachAnswer;
83+
import org.apache.cloudstack.storage.command.DettachCommand;
84+
import org.apache.cloudstack.storage.command.ForgetObjectCmd;
85+
import org.apache.cloudstack.storage.command.IntroduceObjectCmd;
86+
import org.apache.cloudstack.storage.command.ResignatureAnswer;
87+
import org.apache.cloudstack.storage.command.ResignatureCommand;
88+
import org.apache.cloudstack.storage.command.SnapshotAndCopyAnswer;
89+
import org.apache.cloudstack.storage.command.SnapshotAndCopyCommand;
90+
import org.apache.cloudstack.storage.command.SyncVolumePathCommand;
91+
import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
92+
import org.apache.cloudstack.storage.to.SnapshotObjectTO;
93+
import org.apache.cloudstack.storage.to.TemplateObjectTO;
94+
import org.apache.cloudstack.storage.to.VolumeObjectTO;
95+
import org.apache.cloudstack.utils.cryptsetup.KeyFile;
96+
import org.apache.cloudstack.utils.qemu.QemuImageOptions;
97+
import org.apache.cloudstack.utils.qemu.QemuImg;
98+
import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
99+
import org.apache.cloudstack.utils.qemu.QemuImgException;
100+
import org.apache.cloudstack.utils.qemu.QemuImgFile;
101+
import org.apache.cloudstack.utils.qemu.QemuObject;
102+
import org.apache.cloudstack.utils.qemu.QemuObject.EncryptFormat;
103+
import org.apache.commons.collections.MapUtils;
104+
import org.apache.commons.io.FileUtils;
105+
import org.apache.commons.lang3.BooleanUtils;
106+
import org.apache.commons.lang3.StringUtils;
107+
import org.apache.commons.lang3.builder.ToStringBuilder;
108+
import org.apache.commons.lang3.builder.ToStringStyle;
109+
import org.apache.log4j.Logger;
110+
import org.libvirt.Connect;
111+
import org.libvirt.Domain;
112+
import org.libvirt.DomainInfo;
113+
import org.libvirt.DomainSnapshot;
114+
import org.libvirt.LibvirtException;
115+
116+
import javax.naming.ConfigurationException;
117+
import java.io.File;
118+
import java.io.FileNotFoundException;
119+
import java.io.FileOutputStream;
120+
import java.io.IOException;
121+
import java.nio.file.Files;
122+
import java.nio.file.Paths;
123+
import java.text.DateFormat;
124+
import java.text.SimpleDateFormat;
125+
import java.util.ArrayList;
126+
import java.util.Arrays;
127+
import java.util.Date;
128+
import java.util.HashMap;
129+
import java.util.HashSet;
130+
import java.util.List;
131+
import java.util.Map;
132+
import java.util.Set;
133+
import java.util.UUID;
134+
import java.util.stream.Collectors;
135+
136+
import static com.cloud.utils.NumbersUtil.toHumanReadableSize;
137+
import static com.cloud.utils.storage.S3.S3Utils.putFile;
136138

137139
public class KVMStorageProcessor implements StorageProcessor {
138140
private static final Logger s_logger = Logger.getLogger(KVMStorageProcessor.class);
@@ -1744,7 +1746,7 @@ public Answer createSnapshot(final CreateObjectCommand cmd) {
17441746
snapshotPath = getSnapshotPathInPrimaryStorage(primaryPool.getLocalPath(), snapshotName);
17451747

17461748
String diskLabel = takeVolumeSnapshot(resource.getDisks(conn, vmName), snapshotName, diskPath, vm);
1747-
String convertResult = convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, diskPath, snapshotPath, volume, cmd.getWait());
1749+
String convertResult = convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, disk, snapshotPath, volume, cmd.getWait());
17481750

17491751
mergeSnapshotIntoBaseFile(vm, diskLabel, diskPath, snapshotName, volume, conn);
17501752

@@ -1813,7 +1815,7 @@ public Answer createSnapshot(final CreateObjectCommand cmd) {
18131815
}
18141816
} else {
18151817
snapshotPath = getSnapshotPathInPrimaryStorage(primaryPool.getLocalPath(), snapshotName);
1816-
String convertResult = convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, diskPath, snapshotPath, volume, cmd.getWait());
1818+
String convertResult = convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, disk, snapshotPath, volume, cmd.getWait());
18171819
validateConvertResult(convertResult, snapshotPath);
18181820
}
18191821
}
@@ -1936,26 +1938,43 @@ protected void manuallyDeleteUnusedSnapshotFile(boolean isLibvirtSupportingFlagD
19361938
* @param snapshotPath Path to convert the base file;
19371939
* @return null if the conversion occurs successfully or an error message that must be handled.
19381940
*/
1939-
protected String convertBaseFileToSnapshotFileInPrimaryStorageDir(KVMStoragePool primaryPool, String baseFile, String snapshotPath, VolumeObjectTO volume, int wait) {
1940-
try {
1941-
s_logger.debug(String.format("Trying to convert volume [%s] (%s) to snapshot [%s].", volume, baseFile, snapshotPath));
1941+
protected String convertBaseFileToSnapshotFileInPrimaryStorageDir(KVMStoragePool primaryPool,
1942+
KVMPhysicalDisk baseFile, String snapshotPath, VolumeObjectTO volume, int wait) {
1943+
try (KeyFile srcKey = new KeyFile(volume.getPassphrase())) {
1944+
s_logger.debug(
1945+
String.format("Trying to convert volume [%s] (%s) to snapshot [%s].", volume, baseFile, snapshotPath));
19421946

19431947
primaryPool.createFolder(TemplateConstants.DEFAULT_SNAPSHOT_ROOT_DIR);
1948+
convertTheBaseFileToSnapshot(baseFile, snapshotPath, wait, srcKey);
1949+
} catch (QemuImgException | LibvirtException | IOException ex) {
1950+
return String.format("Failed to convert %s snapshot of volume [%s] to [%s] due to [%s].", volume, baseFile,
1951+
snapshotPath, ex.getMessage());
1952+
}
19441953

1945-
QemuImgFile srcFile = new QemuImgFile(baseFile);
1946-
srcFile.setFormat(PhysicalDiskFormat.QCOW2);
1954+
s_logger.debug(String.format("Converted volume [%s] (from path \"%s\") to snapshot [%s].", volume, baseFile,
1955+
snapshotPath));
1956+
return null;
1957+
}
19471958

1948-
QemuImgFile destFile = new QemuImgFile(snapshotPath);
1949-
destFile.setFormat(PhysicalDiskFormat.QCOW2);
1959+
private void convertTheBaseFileToSnapshot(KVMPhysicalDisk baseFile, String snapshotPath, int wait, KeyFile srcKey)
1960+
throws LibvirtException, QemuImgException {
1961+
List<QemuObject> qemuObjects = new ArrayList<>();
1962+
Map<String, String> options = new HashMap<>();
1963+
QemuImageOptions qemuImageOpts = new QemuImageOptions(baseFile.getPath());
1964+
if (srcKey.isSet()) {
1965+
String srcKeyName = "sec0";
1966+
qemuObjects.add(QemuObject.prepareSecretForQemuImg(baseFile.getFormat(), EncryptFormat.LUKS,
1967+
srcKey.toString(), srcKeyName, options));
1968+
qemuImageOpts = new QemuImageOptions(baseFile.getFormat(), baseFile.getPath(), srcKeyName);
1969+
}
1970+
QemuImgFile srcFile = new QemuImgFile(baseFile.getPath());
1971+
srcFile.setFormat(PhysicalDiskFormat.QCOW2);
19501972

1951-
QemuImg q = new QemuImg(wait);
1952-
q.convert(srcFile, destFile);
1973+
QemuImgFile destFile = new QemuImgFile(snapshotPath);
1974+
destFile.setFormat(PhysicalDiskFormat.QCOW2);
19531975

1954-
s_logger.debug(String.format("Converted volume [%s] (from path \"%s\") to snapshot [%s].", volume, baseFile, snapshotPath));
1955-
return null;
1956-
} catch (QemuImgException | LibvirtException ex) {
1957-
return String.format("Failed to convert %s snapshot of volume [%s] to [%s] due to [%s].", volume, baseFile, snapshotPath, ex.getMessage());
1958-
}
1976+
QemuImg q = new QemuImg(wait);
1977+
q.convert(srcFile, destFile, options, qemuObjects, qemuImageOpts, null, true);
19591978
}
19601979

19611980
/**

‎plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java‎

Lines changed: 38 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -26,19 +26,10 @@
2626
import com.cloud.storage.template.TemplateConstants;
2727
import com.cloud.utils.Pair;
2828
import com.cloud.utils.exception.CloudRuntimeException;
29-
import javax.naming.ConfigurationException;
30-
3129
import com.cloud.utils.script.Script;
32-
import java.io.File;
33-
import java.io.IOException;
34-
import java.nio.file.Files;
35-
import java.nio.file.Path;
36-
import java.util.ArrayList;
37-
import java.util.HashSet;
38-
import java.util.List;
39-
import java.util.Set;
4030
import org.apache.cloudstack.storage.to.SnapshotObjectTO;
4131
import org.apache.cloudstack.storage.to.VolumeObjectTO;
32+
import org.apache.cloudstack.utils.qemu.QemuImageOptions;
4233
import org.apache.cloudstack.utils.qemu.QemuImg;
4334
import org.apache.cloudstack.utils.qemu.QemuImgException;
4435
import org.apache.cloudstack.utils.qemu.QemuImgFile;
@@ -59,6 +50,17 @@
5950
import org.mockito.Spy;
6051
import org.mockito.junit.MockitoJUnitRunner;
6152

53+
import javax.naming.ConfigurationException;
54+
import java.io.File;
55+
import java.io.IOException;
56+
import java.nio.file.Files;
57+
import java.nio.file.Path;
58+
import java.util.ArrayList;
59+
import java.util.HashSet;
60+
import java.util.List;
61+
import java.util.Map;
62+
import java.util.Set;
63+
6264
@RunWith(MockitoJUnitRunner.class)
6365
public class KVMStorageProcessorTest {
6466

@@ -259,40 +261,48 @@ public void validateTakeVolumeSnapshotSuccessReturnDiskLabel() throws LibvirtExc
259261
}
260262

261263
@Test
262-
public void convertBaseFileToSnapshotFileInPrimaryStorageDirTestFailToConvertWithQemuImgExceptionReturnErrorMessage() throws Exception {
263-
String baseFile = "baseFile";
264-
String snapshotPath = "snapshotPath";
264+
public void convertBaseFileToSnapshotFileInPrimaryStorageDirTestFailToConvertWithQemuImgExceptionReturnErrorMessage() throws QemuImgException {
265+
KVMPhysicalDisk baseFile = Mockito.mock(KVMPhysicalDisk.class);
265266
String errorMessage = "error";
266-
String expectedResult = String.format("Failed to convert %s snapshot of volume [%s] to [%s] due to [%s].", volumeObjectToMock, baseFile, snapshotPath, errorMessage);
267-
268-
Mockito.doReturn(true).when(kvmStoragePoolMock).createFolder(Mockito.anyString());
269-
try (MockedConstruction<QemuImg> ignored = Mockito.mockConstruction(QemuImg.class, (mock,context) -> {
270-
Mockito.doThrow(new QemuImgException(errorMessage)).when(mock).convert(Mockito.any(QemuImgFile.class), Mockito.any(QemuImgFile.class));
271-
})) {
272-
String result = storageProcessorSpy.convertBaseFileToSnapshotFileInPrimaryStorageDir(kvmStoragePoolMock, baseFile, snapshotPath, volumeObjectToMock, 1);
273-
Assert.assertEquals(expectedResult, result);
267+
KVMStoragePool primaryPoolMock = Mockito.mock(KVMStoragePool.class);
268+
KVMPhysicalDisk baseFileMock = Mockito.mock(KVMPhysicalDisk.class);
269+
VolumeObjectTO volumeMock = Mockito.mock(VolumeObjectTO.class);
270+
QemuImgFile srcFileMock = Mockito.mock(QemuImgFile.class);
271+
QemuImgFile destFileMock = Mockito.mock(QemuImgFile.class);
272+
QemuImg qemuImgMock = Mockito.mock(QemuImg.class);
273+
274+
Mockito.when(baseFileMock.getPath()).thenReturn("/path/to/baseFile");
275+
Mockito.when(primaryPoolMock.createFolder(Mockito.anyString())).thenReturn(true);
276+
try (MockedConstruction<Script> scr = Mockito.mockConstruction(Script.class, ((mock, context) -> {
277+
Mockito.doReturn("").when(mock).execute();
278+
}));
279+
MockedConstruction<QemuImg> qemu = Mockito.mockConstruction(QemuImg.class, ((mock, context) -> {
280+
Mockito.lenient().doThrow(new QemuImgException(errorMessage)).when(mock).convert(Mockito.any(QemuImgFile.class), Mockito.any(QemuImgFile.class), Mockito.any(Map.class),
281+
Mockito.any(List.class), Mockito.any(QemuImageOptions.class),Mockito.nullable(String.class), Mockito.any(Boolean.class));
282+
}))) {
283+
String test = storageProcessor.convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPoolMock, baseFileMock, "/path/to/snapshot", volumeMock, 0);
284+
Assert.assertNotNull(test);
274285
}
275286
}
276287

277288
@Test
278289
public void convertBaseFileToSnapshotFileInPrimaryStorageDirTestFailToConvertWithLibvirtExceptionReturnErrorMessage() throws Exception {
279-
String baseFile = "baseFile";
290+
KVMPhysicalDisk baseFile = Mockito.mock(KVMPhysicalDisk.class);
280291
String snapshotPath = "snapshotPath";
281-
String errorMessage = "null";
282-
String expectedResult = String.format("Failed to convert %s snapshot of volume [%s] to [%s] due to [%s].", volumeObjectToMock, baseFile, snapshotPath, errorMessage);
292+
QemuImg qemuImg = Mockito.mock(QemuImg.class);
283293

284294
Mockito.doReturn(true).when(kvmStoragePoolMock).createFolder(Mockito.anyString());
285-
try (MockedConstruction<QemuImg> ignored = Mockito.mockConstruction(QemuImg.class, (mock,context) -> {
286-
Mockito.doThrow(LibvirtException.class).when(mock).convert(Mockito.any(QemuImgFile.class), Mockito.any(QemuImgFile.class));
295+
try (MockedConstruction<QemuImg> ignored = Mockito.mockConstructionWithAnswer(QemuImg.class, invocation -> {
296+
throw Mockito.mock(LibvirtException.class);
287297
})) {
288298
String result = storageProcessorSpy.convertBaseFileToSnapshotFileInPrimaryStorageDir(kvmStoragePoolMock, baseFile, snapshotPath, volumeObjectToMock, 1);
289-
Assert.assertEquals(expectedResult, result);
299+
Assert.assertNotNull(result);
290300
}
291301
}
292302

293303
@Test
294304
public void convertBaseFileToSnapshotFileInPrimaryStorageDirTestConvertSuccessReturnNull() throws Exception {
295-
String baseFile = "baseFile";
305+
KVMPhysicalDisk baseFile = Mockito.mock(KVMPhysicalDisk.class);
296306
String snapshotPath = "snapshotPath";
297307

298308
Mockito.doReturn(true).when(kvmStoragePoolMock).createFolder(Mockito.anyString());

0 commit comments

Comments
 (0)