Skip to content

Commit 3f8e618

Browse files
DaanHooglanddhslove
authored andcommitted
Revert "server: fix attach uploaded volume (apache#10267)" (apache#10323)
This reverts commit 1c84ce4.
1 parent cf061ca commit 3f8e618

File tree

2 files changed

+62
-26
lines changed

2 files changed

+62
-26
lines changed

server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java

Lines changed: 61 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,7 @@
135135
import com.cloud.dc.DataCenter;
136136
import com.cloud.dc.DataCenterVO;
137137
import com.cloud.dc.Pod;
138-
import com.cloud.dc.dao.ClusterDao;
139138
import com.cloud.dc.dao.DataCenterDao;
140-
import com.cloud.dc.dao.HostPodDao;
141139
import com.cloud.domain.Domain;
142140
import com.cloud.domain.dao.DomainDao;
143141
import com.cloud.event.ActionEvent;
@@ -157,7 +155,6 @@
157155
import com.cloud.hypervisor.HypervisorCapabilitiesVO;
158156
import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao;
159157
import com.cloud.offering.DiskOffering;
160-
import com.cloud.org.Cluster;
161158
import com.cloud.org.Grouping;
162159
import com.cloud.projects.Project;
163160
import com.cloud.projects.ProjectManager;
@@ -327,8 +324,6 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
327324
@Inject
328325
private VmWorkJobDao _workJobDao;
329326
@Inject
330-
ClusterDao clusterDao;
331-
@Inject
332327
private ClusterDetailsDao _clusterDetailsDao;
333328
@Inject
334329
private StorageManager storageMgr;
@@ -2589,8 +2584,63 @@ private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long device
25892584
}
25902585

25912586
UserVmVO vm = _userVmDao.findById(vmId);
2592-
VolumeVO existingVolumeOfVm = getVmExistingVolumeForVolumeAttach(vm, volumeToAttach);
2593-
VolumeInfo newVolumeOnPrimaryStorage = createVolumeOnPrimaryForAttachIfNeeded(volumeToAttach, vm, existingVolumeOfVm);
2587+
VolumeVO existingVolumeOfVm = null;
2588+
VMTemplateVO template = _templateDao.findById(vm.getTemplateId());
2589+
List<VolumeVO> rootVolumesOfVm = _volsDao.findByInstanceAndType(vmId, Volume.Type.ROOT);
2590+
if (rootVolumesOfVm.size() > 1 && template != null && !template.isDeployAsIs()) {
2591+
throw new CloudRuntimeException("The VM " + vm.getHostName() + " has more than one ROOT volume and is in an invalid state.");
2592+
} else {
2593+
if (!rootVolumesOfVm.isEmpty()) {
2594+
existingVolumeOfVm = rootVolumesOfVm.get(0);
2595+
} else {
2596+
// locate data volume of the vm
2597+
List<VolumeVO> diskVolumesOfVm = _volsDao.findByInstanceAndType(vmId, Volume.Type.DATADISK);
2598+
for (VolumeVO diskVolume : diskVolumesOfVm) {
2599+
if (diskVolume.getState() != Volume.State.Allocated) {
2600+
existingVolumeOfVm = diskVolume;
2601+
break;
2602+
}
2603+
}
2604+
}
2605+
}
2606+
if (s_logger.isTraceEnabled()) {
2607+
String msg = "attaching volume %s/%s to a VM (%s/%s) with an existing volume %s/%s on primary storage %s";
2608+
if (existingVolumeOfVm != null) {
2609+
s_logger.trace(String.format(msg,
2610+
volumeToAttach.getName(), volumeToAttach.getUuid(),
2611+
vm.getName(), vm.getUuid(),
2612+
existingVolumeOfVm.getName(), existingVolumeOfVm.getUuid(),
2613+
existingVolumeOfVm.getPoolId()));
2614+
}
2615+
}
2616+
2617+
HypervisorType rootDiskHyperType = vm.getHypervisorType();
2618+
HypervisorType volumeToAttachHyperType = _volsDao.getHypervisorType(volumeToAttach.getId());
2619+
2620+
VolumeInfo newVolumeOnPrimaryStorage = volumeToAttach;
2621+
2622+
//don't create volume on primary storage if its being attached to the vm which Root's volume hasn't been created yet
2623+
StoragePoolVO destPrimaryStorage = null;
2624+
if (existingVolumeOfVm != null && !existingVolumeOfVm.getState().equals(Volume.State.Allocated)) {
2625+
destPrimaryStorage = _storagePoolDao.findById(existingVolumeOfVm.getPoolId());
2626+
if (s_logger.isTraceEnabled() && destPrimaryStorage != null) {
2627+
s_logger.trace(String.format("decided on target storage: %s/%s", destPrimaryStorage.getName(), destPrimaryStorage.getUuid()));
2628+
}
2629+
}
2630+
2631+
boolean volumeOnSecondary = volumeToAttach.getState() == Volume.State.Uploaded;
2632+
2633+
if (destPrimaryStorage != null && (volumeToAttach.getState() == Volume.State.Allocated || volumeOnSecondary)) {
2634+
try {
2635+
if (volumeOnSecondary && destPrimaryStorage.getPoolType() == Storage.StoragePoolType.PowerFlex) {
2636+
throw new InvalidParameterValueException("Cannot attach uploaded volume, this operation is unsupported on storage pool type " + destPrimaryStorage.getPoolType());
2637+
}
2638+
newVolumeOnPrimaryStorage = _volumeMgr.createVolumeOnPrimaryStorage(vm, volumeToAttach, rootDiskHyperType, destPrimaryStorage);
2639+
} catch (NoTransitionException e) {
2640+
s_logger.debug("Failed to create volume on primary storage", e);
2641+
throw new CloudRuntimeException("Failed to create volume on primary storage", e);
2642+
}
2643+
}
25942644

25952645
// reload the volume from db
25962646
newVolumeOnPrimaryStorage = volFactory.getVolume(newVolumeOnPrimaryStorage.getId());
@@ -2609,7 +2659,6 @@ private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long device
26092659
StoragePoolVO vmRootVolumePool = _storagePoolDao.findById(existingVolumeOfVm.getPoolId());
26102660

26112661
try {
2612-
HypervisorType volumeToAttachHyperType = _volsDao.getHypervisorType(volumeToAttach.getId());
26132662
newVolumeOnPrimaryStorage = _volumeMgr.moveVolume(newVolumeOnPrimaryStorage, vmRootVolumePool.getDataCenterId(), vmRootVolumePool.getPodId(), vmRootVolumePool.getClusterId(),
26142663
volumeToAttachHyperType);
26152664
} catch (ConcurrentOperationException | StorageUnavailableException e) {
@@ -2619,7 +2668,7 @@ private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long device
26192668
}
26202669
VolumeVO newVol = _volsDao.findById(newVolumeOnPrimaryStorage.getId());
26212670
// Getting the fresh vm object in case of volume migration to check the current state of VM
2622-
if (moveVolumeNeeded) {
2671+
if (moveVolumeNeeded || volumeOnSecondary) {
26232672
vm = _userVmDao.findById(vmId);
26242673
if (vm == null) {
26252674
throw new InvalidParameterValueException("VM not found.");
@@ -2804,6 +2853,9 @@ private void checkDeviceId(Long deviceId, VolumeInfo volumeToAttach, UserVmVO vm
28042853
if (!_volsDao.findByInstanceAndDeviceId(vm.getId(), 0).isEmpty()) {
28052854
throw new InvalidParameterValueException("Vm already has root volume attached to it");
28062855
}
2856+
if (volumeToAttach.getState() == Volume.State.Uploaded) {
2857+
throw new InvalidParameterValueException("No support for Root volume attach in state " + Volume.State.Uploaded);
2858+
}
28072859
}
28082860
}
28092861

server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
import org.apache.cloudstack.api.command.user.volume.ResizeVolumeCmd;
5151
import org.apache.cloudstack.backup.dao.BackupDao;
5252
import org.apache.cloudstack.context.CallContext;
53-
import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
5453
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
5554
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
5655
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
@@ -93,12 +92,8 @@
9392
import com.cloud.configuration.ConfigurationManager;
9493
import com.cloud.configuration.Resource;
9594
import com.cloud.configuration.Resource.ResourceType;
96-
import com.cloud.dc.ClusterVO;
9795
import com.cloud.dc.DataCenterVO;
98-
import com.cloud.dc.HostPodVO;
99-
import com.cloud.dc.dao.ClusterDao;
10096
import com.cloud.dc.dao.DataCenterDao;
101-
import com.cloud.dc.dao.HostPodDao;
10297
import com.cloud.event.EventTypes;
10398
import com.cloud.event.UsageEventUtils;
10499
import com.cloud.exception.InvalidParameterValueException;
@@ -134,12 +129,10 @@
134129
import com.cloud.utils.db.TransactionLegacy;
135130
import com.cloud.utils.exception.CloudRuntimeException;
136131
import com.cloud.utils.fsm.NoTransitionException;
137-
import com.cloud.vm.DiskProfile;
138132
import com.cloud.vm.UserVmManager;
139133
import com.cloud.vm.UserVmVO;
140134
import com.cloud.vm.VirtualMachine;
141135
import com.cloud.vm.VirtualMachine.State;
142-
import com.cloud.vm.VirtualMachineManager;
143136
import com.cloud.vm.dao.UserVmDao;
144137
import com.cloud.vm.dao.VMInstanceDao;
145138
import com.cloud.vm.snapshot.VMSnapshotVO;
@@ -215,15 +208,6 @@ public class VolumeApiServiceImplTest {
215208
private DataStoreManager dataStoreMgr;
216209
@Mock
217210
private SnapshotHelper snapshotHelper;
218-
@Mock
219-
VirtualMachineManager virtualMachineManager;
220-
@Mock
221-
HostPodDao podDao;
222-
@Mock
223-
ClusterDao clusterDao;
224-
@Mock
225-
VolumeOrchestrationService volumeOrchestrationService;
226-
227211

228212
private DetachVolumeCmd detachCmd = new DetachVolumeCmd();
229213
private Class<?> _detachCmdClass = detachCmd.getClass();
@@ -2200,4 +2184,4 @@ public void testCreateVolumeOnSecondaryForAttachIfNeeded_NoSuitablePool_ReturnSa
22002184
Assert.fail();
22012185
}
22022186
}
2203-
}
2187+
}

0 commit comments

Comments
 (0)