Skip to content

Commit 0583966

Browse files
Srivastava, PiyushSrivastava, Piyush
Srivastava, Piyush
authored and
Srivastava, Piyush
committed
feature/CSTACKEX-122: fix for boot disk
1 parent f25cd63 commit 0583966

File tree

4 files changed

+28
-52
lines changed

4 files changed

+28
-52
lines changed

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

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,6 @@ public class VolumeServiceImpl implements VolumeService {
222222
@Inject
223223
protected DiskOfferingDao diskOfferingDao;
224224

225-
private static final String ONTAP_PROVIDER_NAME = "NetApp ONTAP";
226-
227225
public VolumeServiceImpl() {
228226
}
229227

@@ -732,13 +730,8 @@ protected Void managedCopyBaseImageCallback(AsyncCallbackDispatcher<VolumeServic
732730
CopyCmdAnswer answer = (CopyCmdAnswer)result.getAnswer();
733731
TemplateObjectTO templateObjectTo = (TemplateObjectTO)answer.getNewData();
734732

735-
// For NFS managed storage, preserve the volume UUID path to avoid file collision
736-
PrimaryDataStore primaryDataStore = (PrimaryDataStore) volumeInfo.getDataStore();
737-
if (primaryDataStore.isManaged() && ONTAP_PROVIDER_NAME.equals(primaryDataStore.getStorageProviderName()) && primaryDataStore.getPoolType() == StoragePoolType.NetworkFilesystem) {
738-
logger.debug("NFS managed storage - preserving volume path: " + volume.getPath() + " (not overwriting with template path: " + templateObjectTo.getPath() + ")");
739-
}else {
740-
volume.setPath(templateObjectTo.getPath());
741-
}
733+
volume.setPath(templateObjectTo.getPath());
734+
742735
if (templateObjectTo.getFormat() != null) {
743736
volume.setFormat(templateObjectTo.getFormat());
744737
}
@@ -1327,7 +1320,8 @@ private void createManagedVolumeCopyTemplateAsync(VolumeInfo volumeInfo, Primary
13271320
details.put(PrimaryDataStore.STORAGE_HOST, primaryDataStore.getHostAddress());
13281321
details.put(PrimaryDataStore.STORAGE_PORT, String.valueOf(primaryDataStore.getPort()));
13291322
// for managed storage, the storage repository (XenServer) or datastore (ESX) name is based off of the iScsiName property of a volume
1330-
details.put(PrimaryDataStore.MANAGED_STORE_TARGET, volumeInfo.get_iScsiName());
1323+
String managedStoreTarget = volumeInfo.get_iScsiName() != null ? volumeInfo.get_iScsiName() : volumeInfo.getUuid();
1324+
details.put(PrimaryDataStore.MANAGED_STORE_TARGET, managedStoreTarget);
13311325
details.put(PrimaryDataStore.MANAGED_STORE_TARGET_ROOT_VOLUME, volumeInfo.getName());
13321326
details.put(PrimaryDataStore.VOLUME_SIZE, String.valueOf(volumeInfo.getSize()));
13331327
details.put(StorageManager.STORAGE_POOL_DISK_WAIT.toString(), String.valueOf(StorageManager.STORAGE_POOL_DISK_WAIT.valueIn(primaryDataStore.getId())));
@@ -1345,7 +1339,8 @@ private void createManagedVolumeCopyTemplateAsync(VolumeInfo volumeInfo, Primary
13451339

13461340
grantAccess(volumeInfo, destHost, primaryDataStore);
13471341
volumeInfo = volFactory.getVolume(volumeInfo.getId(), primaryDataStore);
1348-
details.put(PrimaryDataStore.MANAGED_STORE_TARGET, volumeInfo.get_iScsiName());
1342+
managedStoreTarget = volumeInfo.get_iScsiName() != null ? volumeInfo.get_iScsiName() : volumeInfo.getUuid();
1343+
details.put(PrimaryDataStore.MANAGED_STORE_TARGET, managedStoreTarget);
13491344
primaryDataStore.setDetails(details);
13501345

13511346
// Update destTemplateInfo with the iSCSI path from volumeInfo

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

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@
3636
import com.cloud.storage.dao.VolumeDetailsDao;
3737
import com.cloud.utils.Pair;
3838
import com.cloud.utils.exception.CloudRuntimeException;
39-
import com.cloud.vm.VirtualMachine;
40-
import com.cloud.vm.dao.VMInstanceDao;
4139
import org.apache.cloudstack.engine.subsystem.api.storage.ChapInfo;
4240
import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
4341
import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
@@ -75,7 +73,6 @@
7573

7674
import javax.inject.Inject;
7775
import java.util.ArrayList;
78-
import java.util.Arrays;
7976
import java.util.HashMap;
8077
import java.util.List;
8178
import java.util.Map;
@@ -90,7 +87,6 @@ public class OntapPrimaryDatastoreDriver implements PrimaryDataStoreDriver {
9087

9188
@Inject private StoragePoolDetailsDao storagePoolDetailsDao;
9289
@Inject private PrimaryDataStoreDao storagePoolDao;
93-
@Inject private VMInstanceDao vmDao;
9490
@Inject private VolumeDao volumeDao;
9591
@Inject private VolumeDetailsDao volumeDetailsDao;
9692
@Inject private SnapshotDetailsDao snapshotDetailsDao;
@@ -485,22 +481,6 @@ public void revokeAccess(DataObject dataObject, Host host, DataStore dataStore)
485481
throw new InvalidParameterValueException("host should not be null");
486482
}
487483

488-
// Safety check: don't revoke access if volume is still attached to an active VM
489-
if (dataObject.getType() == DataObjectType.VOLUME) {
490-
Volume volume = volumeDao.findById(dataObject.getId());
491-
if (volume.getInstanceId() != null) {
492-
VirtualMachine vm = vmDao.findById(volume.getInstanceId());
493-
if (vm != null && !Arrays.asList(
494-
VirtualMachine.State.Destroyed,
495-
VirtualMachine.State.Expunging,
496-
VirtualMachine.State.Error).contains(vm.getState())) {
497-
s_logger.warn("revokeAccess: Volume [{}] is still attached to VM [{}] in state [{}], skipping revokeAccess",
498-
dataObject.getId(), vm.getInstanceName(), vm.getState());
499-
return;
500-
}
501-
}
502-
}
503-
504484
StoragePoolVO storagePool = storagePoolDao.findById(dataStore.getId());
505485
if (storagePool == null) {
506486
s_logger.error("revokeAccess: Storage Pool not found for id: " + dataStore.getId());

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/UnifiedSANStrategy.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ public AccessGroup createAccessGroup(AccessGroup accessGroup) {
221221
initiators.add(initiator);
222222
igroupRequest.setInitiators(initiators);
223223
igroupRequest.setDeleteOnUnmap(true);
224+
igroupRequest.setDeleteOnUnmap(true);
224225
}
225226
igroupRequest.setProtocol(Igroup.ProtocolEnum.valueOf(Constants.ISCSI));
226227
// Create Igroup

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

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,6 @@
2828
import com.cloud.storage.dao.VolumeDao;
2929
import com.cloud.storage.dao.VolumeDetailsDao;
3030
import com.cloud.utils.exception.CloudRuntimeException;
31-
import com.cloud.vm.VirtualMachine;
32-
import com.cloud.vm.VMInstanceVO;
33-
import com.cloud.vm.dao.VMInstanceDao;
3431
import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult;
3532
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
3633
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
@@ -67,7 +64,6 @@
6764
import static org.junit.jupiter.api.Assertions.assertThrows;
6865
import static org.junit.jupiter.api.Assertions.assertTrue;
6966
import static org.mockito.ArgumentMatchers.any;
70-
import static org.mockito.ArgumentMatchers.anyLong;
7167
import static org.mockito.ArgumentMatchers.anyString;
7268
import static org.mockito.ArgumentMatchers.argThat;
7369
import static org.mockito.ArgumentMatchers.eq;
@@ -87,9 +83,6 @@ class OntapPrimaryDatastoreDriverTest {
8783
@Mock
8884
private PrimaryDataStoreDao storagePoolDao;
8985

90-
@Mock
91-
private VMInstanceDao vmDao;
92-
9386
@Mock
9487
private VolumeDao volumeDao;
9588

@@ -442,25 +435,33 @@ void testGrantAccess_IgroupNotFound_CreatesNewIgroup() {
442435
}
443436

444437
@Test
445-
void testRevokeAccess_VolumeAttachedToRunningVM_SkipsRevoke() {
446-
// Setup
438+
void testRevokeAccess_NFSVolume_SkipsRevoke() {
439+
// Setup - NFS volumes have no LUN mapping, so revokeAccess is a no-op
440+
when(dataStore.getId()).thenReturn(1L);
447441
when(volumeInfo.getType()).thenReturn(VOLUME);
448442
when(volumeInfo.getId()).thenReturn(100L);
449443

450-
VolumeVO mockVolume = mock(VolumeVO.class);
451-
when(mockVolume.getInstanceId()).thenReturn(200L);
452-
when(volumeDao.findById(100L)).thenReturn(mockVolume);
444+
when(volumeDao.findById(100L)).thenReturn(volumeVO);
445+
when(volumeVO.getId()).thenReturn(100L);
446+
when(volumeVO.getName()).thenReturn("test-volume");
447+
448+
when(storagePoolDao.findById(1L)).thenReturn(storagePool);
449+
when(storagePool.getId()).thenReturn(1L);
450+
when(storagePool.getScope()).thenReturn(ScopeType.CLUSTER);
451+
when(storagePool.getUuid()).thenReturn("pool-uuid-123");
452+
when(storagePoolDetailsDao.listDetailsKeyPairs(1L)).thenReturn(storagePoolDetails);
453+
when(host.getName()).thenReturn("host1");
453454

454-
VMInstanceVO vm = mock(VMInstanceVO.class);
455-
when(vm.getState()).thenReturn(VirtualMachine.State.Running);
456-
when(vm.getInstanceName()).thenReturn("i-2-100-VM");
457-
when(vmDao.findById(200L)).thenReturn(vm);
455+
try (MockedStatic<Utility> utilityMock = mockStatic(Utility.class)) {
456+
utilityMock.when(() -> Utility.getStrategyByStoragePoolDetails(storagePoolDetails))
457+
.thenReturn(sanStrategy);
458458

459-
// Execute
460-
driver.revokeAccess(volumeInfo, host, dataStore);
459+
// Execute - NFS has no iSCSI protocol, so revokeAccessForVolume does nothing
460+
driver.revokeAccess(volumeInfo, host, dataStore);
461461

462-
// Verify - should skip revoke for running VM
463-
verify(storagePoolDao, never()).findById(anyLong());
462+
// Verify - no LUN unmap operations for NFS
463+
verify(sanStrategy, never()).disableLogicalAccess(any());
464+
}
464465
}
465466

466467
@Test
@@ -472,7 +473,6 @@ void testRevokeAccess_ISCSIVolume_Success() {
472473

473474
when(volumeDao.findById(100L)).thenReturn(volumeVO);
474475
when(volumeVO.getId()).thenReturn(100L);
475-
when(volumeVO.getInstanceId()).thenReturn(null);
476476
when(volumeVO.getName()).thenReturn("test-volume");
477477

478478
when(storagePoolDao.findById(1L)).thenReturn(storagePool);

0 commit comments

Comments
 (0)