Skip to content

Commit f04f892

Browse files
sathvikaragisandeeplocharla
sathvikaragi
authored and
sandeeplocharla
committed
CSTACKEX-246: setting volume format based on protocol (#92)
### Description OntapPrimaryDatastoreDriver now returns RAW format for KVM+iSCSI (was always QCOW2), and VolumeServiceImpl.managedCopyBaseImageCallback adds a volume.getFormat() != null guard to prevent the template's format from overwriting it. <img width="836" height="1614" alt="image" src="https://github.com/user-attachments/assets/f74184e8-82d8-4382-894f-e7fa7ad6ea57" /> This PR... <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. --> <!-- For new features, provide link to FS, dev ML discussion etc. --> <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. --> <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged --> <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" --> <!-- Fixes: # --> <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [x] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) - [ ] Build/CI - [ ] Test (unit or integration test code) ### Feature/Enhancement Scale or Bug Severity #### Feature/Enhancement Scale - [ ] Major - [ ] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [x] Major - [ ] Minor - [ ] Trivial ### Screenshots (if appropriate): <img width="1704" height="200" alt="Screenshot 2026-08-13 at 11 48 46 AM" src="https://github.com/user-attachments/assets/21de01f3-8ed4-4485-9b23-65d310e69440" /> ### How Has This Been Tested? Ran Iscsi automation suite: Everything is green <img width="2100" height="1520" alt="image" src="https://github.com/user-attachments/assets/bcb44bad-ce86-480c-9274-4f128817affa" /> <!-- Please describe in detail how you tested your changes. --> <!-- Include details of your testing environment, and the tests you ran to --> #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
1 parent 09b74bd commit f04f892

3 files changed

Lines changed: 81 additions & 9 deletions

File tree

‎engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -765,7 +765,11 @@ protected Void managedCopyBaseImageCallback(AsyncCallbackDispatcher<VolumeServic
765765
volume.setPath(templateObjectTo.getPath());
766766

767767
if (templateObjectTo.getFormat() != null) {
768-
volume.setFormat(templateObjectTo.getFormat());
768+
PrimaryDataStore primaryDataStore = context.getPrimaryDataStore();
769+
boolean isOntap = primaryDataStore != null && DataStoreProvider.ONTAP_PLUGIN_NAME.equals(primaryDataStore.getStorageProviderName());
770+
if (!isOntap || volume.getFormat() == null) {
771+
volume.setFormat(templateObjectTo.getFormat());
772+
}
769773
}
770774

771775
volDao.update(volume.getId(), volume);

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriver.java‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,8 @@ public void createAsync(DataStore dataStore, DataObject dataObject, AsyncComplet
160160

161161
volumeVO.setPoolType(storagePool.getPoolType());
162162
volumeVO.setPoolId(storagePool.getId());
163-
volumeVO.setFormat(getImageFormatByHypervisor(storagePool.getHypervisor()));
164-
logger.info("createAsync: Volume format set to [{}] for hypervisor [{}]", volumeVO.getFormat(), storagePool.getHypervisor());
163+
volumeVO.setFormat(getImageFormatByHypervisorAndProtocol(storagePool.getHypervisor(), details.get(OntapStorageConstants.PROTOCOL)));
164+
logger.info("createAsync: Volume format set to [{}] for hypervisor [{}] and protocol [{}]", volumeVO.getFormat(), storagePool.getHypervisor(), details.get(OntapStorageConstants.PROTOCOL));
165165

166166
if (ProtocolType.ISCSI.name().equalsIgnoreCase(details.get(OntapStorageConstants.PROTOCOL))) {
167167
String lunName = created != null && created.getLun() != null ? created.getLun().getName() : null;
@@ -988,9 +988,17 @@ private String buildSnapshotName(String cloudStackSnapshotName, long snapshotId)
988988
}
989989

990990

991-
private Storage.ImageFormat getImageFormatByHypervisor(HypervisorType hypervisorType) {
991+
private Storage.ImageFormat getImageFormatByHypervisorAndProtocol(HypervisorType hypervisorType, String protocol) {
992992
if (HypervisorType.KVM.equals(hypervisorType)) {
993-
return Storage.ImageFormat.QCOW2;
993+
ProtocolType protocolType = ProtocolType.valueOf(protocol);
994+
switch (protocolType) {
995+
case NFS3:
996+
return Storage.ImageFormat.QCOW2;
997+
case ISCSI:
998+
return Storage.ImageFormat.RAW;
999+
default:
1000+
throw new CloudRuntimeException("Unsupported protocol [" + protocol + "] for ONTAP image format resolution");
1001+
}
9941002
}
9951003
throw new CloudRuntimeException("Unsupported hypervisor [" + hypervisorType + "] for ONTAP image format resolution");
9961004
}

‎plugins/storage/volume/ontap/src/test/java/org/apache/cloudstack/storage/driver/OntapPrimaryDatastoreDriverTest.java‎

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
4040
import org.apache.cloudstack.storage.feign.model.Igroup;
4141
import org.apache.cloudstack.storage.feign.model.Lun;
42+
import org.apache.cloudstack.storage.service.UnifiedNASStrategy;
4243
import org.apache.cloudstack.storage.service.UnifiedSANStrategy;
4344
import org.apache.cloudstack.storage.service.model.AccessGroup;
4445
import org.apache.cloudstack.storage.service.model.CloudStackVolume;
@@ -108,6 +109,9 @@ class OntapPrimaryDatastoreDriverTest {
108109
@Mock
109110
private UnifiedSANStrategy sanStrategy;
110111

112+
@Mock
113+
private UnifiedNASStrategy nasStrategy;
114+
111115
@Mock
112116
private AsyncCompletionCallback<CreateCmdResult> createCallback;
113117

@@ -167,7 +171,7 @@ void testCreateAsync_VolumeWithISCSI_Success() {
167171

168172
when(storagePoolDao.findById(1L)).thenReturn(storagePool);
169173
when(storagePool.getId()).thenReturn(1L);
170-
when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem);
174+
when(storagePool.getPoolType()).thenReturn(Storage.StoragePoolType.Iscsi);
171175
when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM);
172176

173177
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails);
@@ -204,7 +208,7 @@ void testCreateAsync_VolumeWithISCSI_Success() {
204208

205209
verify(volumeDetailsDao).addDetail(eq(100L), eq(OntapStorageConstants.LUN_DOT_UUID), eq("lun-uuid-123"), eq(false));
206210
verify(volumeDetailsDao).addDetail(eq(100L), eq(OntapStorageConstants.LUN_DOT_NAME), eq("/vol/vol1/lun1"), eq(false));
207-
verify(volumeVO).setFormat(Storage.ImageFormat.QCOW2);
211+
verify(volumeVO).setFormat(Storage.ImageFormat.RAW);
208212
verify(volumeDao).update(eq(100L), any(VolumeVO.class));
209213
}
210214
}
@@ -232,11 +236,11 @@ void testCreateAsync_VolumeWithNFS_Success() {
232236

233237
try (MockedStatic<OntapStorageUtils> utilityMock = mockStatic(OntapStorageUtils.class)) {
234238
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(storagePoolDetails))
235-
.thenReturn(sanStrategy);
239+
.thenReturn(nasStrategy);
236240
utilityMock.when(() -> OntapStorageUtils.createCloudStackVolumeRequestByProtocol(
237241
any(), any(), any())).thenReturn(mockCloudStackVolume);
238242

239-
when(sanStrategy.createCloudStackVolume(any())).thenReturn(mockCloudStackVolume);
243+
when(nasStrategy.createCloudStackVolume(any())).thenReturn(mockCloudStackVolume);
240244

241245
// Execute
242246
driver.createAsync(dataStore, volumeInfo, createCallback);
@@ -253,6 +257,62 @@ void testCreateAsync_VolumeWithNFS_Success() {
253257
}
254258
}
255259

260+
@Test
261+
void testCreateAsync_UnsupportedHypervisor_FailsWithError() {
262+
storagePoolDetails.put(OntapStorageConstants.PROTOCOL, ProtocolType.ISCSI.name());
263+
264+
when(dataStore.getId()).thenReturn(1L);
265+
when(dataStore.getName()).thenReturn("ontap-pool");
266+
when(volumeInfo.getType()).thenReturn(VOLUME);
267+
when(volumeInfo.getId()).thenReturn(100L);
268+
when(volumeInfo.getName()).thenReturn("test-volume");
269+
270+
when(storagePoolDao.findById(1L)).thenReturn(storagePool);
271+
when(storagePool.getId()).thenReturn(1L);
272+
when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.VMware);
273+
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails);
274+
when(volumeDao.findById(100L)).thenReturn(volumeVO);
275+
276+
try (MockedStatic<OntapStorageUtils> utilityMock = mockStatic(OntapStorageUtils.class)) {
277+
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy);
278+
279+
driver.createAsync(dataStore, volumeInfo, createCallback);
280+
281+
ArgumentCaptor<CreateCmdResult> resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class);
282+
verify(createCallback).complete(resultCaptor.capture());
283+
assertFalse(resultCaptor.getValue().isSuccess());
284+
assertTrue(resultCaptor.getValue().getResult().contains("Unsupported hypervisor [VMware]"));
285+
}
286+
}
287+
288+
@Test
289+
void testCreateAsync_KvmUnsupportedProtocol_FailsWithError() {
290+
storagePoolDetails.put(OntapStorageConstants.PROTOCOL, "FC");
291+
292+
when(dataStore.getId()).thenReturn(1L);
293+
when(dataStore.getName()).thenReturn("ontap-pool");
294+
when(volumeInfo.getType()).thenReturn(VOLUME);
295+
when(volumeInfo.getId()).thenReturn(100L);
296+
when(volumeInfo.getName()).thenReturn("test-volume");
297+
298+
when(storagePoolDao.findById(1L)).thenReturn(storagePool);
299+
when(storagePool.getId()).thenReturn(1L);
300+
when(storagePool.getHypervisor()).thenReturn(Hypervisor.HypervisorType.KVM);
301+
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails);
302+
when(volumeDao.findById(100L)).thenReturn(volumeVO);
303+
304+
try (MockedStatic<OntapStorageUtils> utilityMock = mockStatic(OntapStorageUtils.class)) {
305+
utilityMock.when(() -> OntapStorageUtils.getStrategyByStoragePoolDetails(any())).thenReturn(sanStrategy);
306+
307+
driver.createAsync(dataStore, volumeInfo, createCallback);
308+
309+
ArgumentCaptor<CreateCmdResult> resultCaptor = ArgumentCaptor.forClass(CreateCmdResult.class);
310+
verify(createCallback).complete(resultCaptor.capture());
311+
assertFalse(resultCaptor.getValue().isSuccess());
312+
assertTrue(resultCaptor.getValue().getResult().contains("No enum constant"));
313+
}
314+
}
315+
256316
@Test
257317
void testDeleteAsync_NullStore_ThrowsException() {
258318
ArgumentCaptor<CommandResult> resultCaptor = ArgumentCaptor.forClass(CommandResult.class);

0 commit comments

Comments
 (0)