diff --git a/api/src/main/java/com/cloud/agent/api/to/DiskTO.java b/api/src/main/java/com/cloud/agent/api/to/DiskTO.java index d22df2df172e..316204dc36ea 100644 --- a/api/src/main/java/com/cloud/agent/api/to/DiskTO.java +++ b/api/src/main/java/com/cloud/agent/api/to/DiskTO.java @@ -47,9 +47,10 @@ public class DiskTO { private String path; private Volume.Type type; private Map _details; + private int groupNumber; public DiskTO() { - + this.groupNumber = -1; } public DiskTO(DataTO data, Long diskSeq, String path, Volume.Type type) { @@ -57,6 +58,7 @@ public DiskTO(DataTO data, Long diskSeq, String path, Volume.Type type) { this.diskSeq = diskSeq; this.path = path; this.type = type; + this.groupNumber = -1; } public DataTO getData() { @@ -98,4 +100,12 @@ public void setDetails(Map details) { public Map getDetails() { return _details; } + + public int getGroupNumber() { + return groupNumber; + } + + public void setGroupNumber(int groupNumber) { + this.groupNumber = groupNumber; + } } diff --git a/api/src/main/java/com/cloud/vm/VmDetailConstants.java b/api/src/main/java/com/cloud/vm/VmDetailConstants.java index 45dc7a2946f4..0e32bed5e405 100644 --- a/api/src/main/java/com/cloud/vm/VmDetailConstants.java +++ b/api/src/main/java/com/cloud/vm/VmDetailConstants.java @@ -72,6 +72,7 @@ public interface VmDetailConstants { String IP6_ADDRESS = "ip6Address"; String DISK = "disk"; String DISK_OFFERING = "diskOffering"; + String VOLUME_GROUP = "volumeGroup"; String DEPLOY_AS_IS_CONFIGURATION = "configurationId"; String KEY_PAIR_NAMES = "keypairnames"; diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index 21088592d8eb..b637f157c68d 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -459,6 +459,8 @@ public class ApiConstants { public static final String IS_VOLATILE = "isvolatile"; public static final String VOLUME_ID = "volumeid"; public static final String VOLUMES = "volumes"; + public static final String VOLUME_GROUP = "volumegroup"; + public static final String USE_CONTROLLER_CONFIGURATION = "usecontrollerconfiguration"; public static final String ZONE = "zone"; public static final String ZONE_ID = "zoneid"; public static final String ZONE_NAME = "zonename"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java index dca683f787f0..63f7072d014e 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java @@ -158,6 +158,12 @@ public class ImportUnmanagedInstanceCmd extends BaseAsyncCmd { description = "VM is imported despite some of its NIC's MAC addresses are already present") private Boolean forced; + @Parameter(name = ApiConstants.USE_CONTROLLER_CONFIGURATION, + type = CommandType.BOOLEAN, + description = "volumes automatically get volume groups based on their current controller connections", + since="4.17.0") + private Boolean useControllerConfiguration; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -259,6 +265,20 @@ public Map getDataDiskToDiskOfferingList() { return dataDiskToDiskOfferingMap; } + public Map getVolumeGroups() { + Map dataDiskVolumeGroups = new HashMap<>(); + if (MapUtils.isNotEmpty(dataDiskToDiskOfferingList)) { + for (Map entry : (Collection>)dataDiskToDiskOfferingList.values()) { + String disk = entry.get(VmDetailConstants.DISK); + if(entry.get(VmDetailConstants.VOLUME_GROUP) != null){ + Integer volumeGroup = Integer.parseInt(entry.get(VmDetailConstants.VOLUME_GROUP)); + dataDiskVolumeGroups.put(disk, volumeGroup); + } + } + } + return dataDiskVolumeGroups; + } + public Map getDetails() { if (MapUtils.isEmpty(details)) { return new HashMap(); @@ -316,4 +336,8 @@ public long getEntityOwnerId() { } return accountId; } + + public Boolean isUseControllerConfiguration() { + return BooleanUtils.isTrue(this.useControllerConfiguration); + } } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java index 687d683309c6..8ebd0ef5ac45 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java @@ -62,6 +62,9 @@ public class AttachVolumeCmd extends BaseAsyncCmd implements UserCmd { required=true, description=" the ID of the virtual machine") private Long virtualMachineId; + @Parameter(name = ApiConstants.VOLUME_GROUP, type = CommandType.INTEGER, required = false, description = "group of volume", since = "4.17.0") + private int volumeGroup=-1; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -116,6 +119,10 @@ public String getEventDescription() { return "attaching volume: " + this._uuidMgr.getUuid(Volume.class, getId()) + " to vm: " + this._uuidMgr.getUuid(VirtualMachine.class, getVirtualMachineId()); } + public int getVolumeGroup() { + return volumeGroup; + } + @Override public void execute() { CallContext.current().setEventDetails("Volume Id: " + this._uuidMgr.getUuid(Volume.class, getId()) + " VmId: " + this._uuidMgr.getUuid(VirtualMachine.class, getVirtualMachineId())); diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java index 2666cfadc70a..3402f5673aae 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java @@ -163,10 +163,11 @@ List allocateTemplatedVolumes(Type type, String name, DiskOffering * @param poolId ID of pool in which volume is stored * @param path image path of the volume * @param chainInfo chain info for the volume. Hypervisor specific. + * @param volumeGroup group of volume * @return DiskProfile of imported volume */ DiskProfile importVolume(Type type, String name, DiskOffering offering, Long size, Long minIops, Long maxIops, VirtualMachine vm, VirtualMachineTemplate template, - Account owner, Long deviceId, Long poolId, String path, String chainInfo); + Account owner, Long deviceId, Long poolId, String path, String chainInfo, Integer volumeGroup); /** * Unmanage VM volumes diff --git a/engine/components-api/src/main/java/com/cloud/vm/VmWorkAttachVolume.java b/engine/components-api/src/main/java/com/cloud/vm/VmWorkAttachVolume.java index 2d2fefbc9aa6..1b9759f2d3a3 100644 --- a/engine/components-api/src/main/java/com/cloud/vm/VmWorkAttachVolume.java +++ b/engine/components-api/src/main/java/com/cloud/vm/VmWorkAttachVolume.java @@ -21,11 +21,13 @@ public class VmWorkAttachVolume extends VmWork { private Long volumeId; private Long deviceId; + private int groupNumber; - public VmWorkAttachVolume(long userId, long accountId, long vmId, String handlerName, Long volumeId, Long deviceId) { + public VmWorkAttachVolume(long userId, long accountId, long vmId, String handlerName, Long volumeId, Long deviceId, int groupNumber) { super(userId, accountId, vmId, handlerName); this.volumeId = volumeId; this.deviceId = deviceId; + this.groupNumber = groupNumber; } public Long getVolumeId() { @@ -35,4 +37,8 @@ public Long getVolumeId() { public Long getDeviceId() { return deviceId; } -} + + public int getGroupNumber() { + return groupNumber; + } +} \ No newline at end of file diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index 07b475af67ac..1ded1d924d33 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java @@ -126,6 +126,7 @@ import com.cloud.storage.dao.VMTemplateDetailsDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.storage.dao.VolumeDetailsDao; +import com.cloud.storage.dao.VolumeGroupDao; import com.cloud.template.TemplateManager; import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; @@ -234,6 +235,7 @@ public enum UserVmCloneType { @Inject VolumeApiService _volumeApiService; @Inject + private VolumeGroupDao volumeGroupDao; PassphraseDao passphraseDao; @Inject @@ -2073,7 +2075,7 @@ public void updateVolumeDiskChain(long volumeId, String path, String chainInfo, @Override public DiskProfile importVolume(Type type, String name, DiskOffering offering, Long size, Long minIops, Long maxIops, VirtualMachine vm, VirtualMachineTemplate template, Account owner, - Long deviceId, Long poolId, String path, String chainInfo) { + Long deviceId, Long poolId, String path, String chainInfo, Integer volumeGroup) { if (size == null) { size = offering.getDiskSize(); } else { @@ -2120,6 +2122,11 @@ public DiskProfile importVolume(Type type, String name, DiskOffering offering, L vol.setState(Volume.State.Ready); vol.setAttached(new Date()); vol = _volsDao.persist(vol); + + if (volumeGroup != null) { + volumeGroupDao.addVolumeToGroup(vm.getId(), vol.getId(), vol.getDeviceId(), volumeGroup); + } + return toDiskProfile(vol, offering); } @@ -2142,6 +2149,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) { s_logger.debug(String.format("Skipping Destroy for the volume [%s] as it is in [%s] state.", volToString, vol.getState().toString())); } else { volService.unmanageVolume(vol.getId()); + volumeGroupDao.deleteVolumeFromGroup(vol.getId()); } } } diff --git a/engine/schema/src/main/java/com/cloud/storage/VolumeGroupVO.java b/engine/schema/src/main/java/com/cloud/storage/VolumeGroupVO.java new file mode 100644 index 000000000000..3153db001312 --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/storage/VolumeGroupVO.java @@ -0,0 +1,86 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package com.cloud.storage; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.GenerationType; +import javax.persistence.Id; +import javax.persistence.Table; +import org.apache.cloudstack.api.InternalIdentity; + +@Entity +@Table(name = "volume_group") +public class VolumeGroupVO implements InternalIdentity{ + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + @Column(name = "id") + private long id; + + @Column(name = "vm_id") + private long vmId; + + @Column(name = "volume_id") + private long volumeId; + + @Column(name = "group_number") + private int groupNumber; + + + public VolumeGroupVO() {} + + public VolumeGroupVO(long vmId, long volumeId, int groupNumber) { + this.vmId = vmId; + this.volumeId = volumeId; + this.groupNumber = groupNumber; + } + + public long getId() { + return id; + } + + public void setId(long id) { + this.id = id; + } + + public long getVmId() { + return vmId; + } + + public void setVmId(long vmId) { + this.vmId = vmId; + } + + public int getGroupNumber() { + return groupNumber; + } + + public void setGroupNumber(int groupNumber) { + this.groupNumber = groupNumber; + } + + public long getVolumeId() { + return volumeId; + } + + public void setVolumeId(long volumeId) { + this.volumeId = volumeId; + } +} diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDao.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDao.java new file mode 100644 index 000000000000..d483013ffab3 --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDao.java @@ -0,0 +1,29 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package com.cloud.storage.dao; + +import com.cloud.utils.db.GenericDao; +import com.cloud.storage.VolumeGroupVO; + + +public interface VolumeGroupDao extends GenericDao { + public void addVolumeToGroup(long vmId, long volumeId, long deviceId, int groupNumber); + public void deleteVolumeFromGroup(long volumeId); + public VolumeGroupVO findByVmAndVolume(long vmId, long volumeId); +} diff --git a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDaoImpl.java new file mode 100644 index 000000000000..9fff2778e57b --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDaoImpl.java @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package com.cloud.storage.dao; + +import com.cloud.utils.db.GenericDaoBase; +import com.cloud.storage.VolumeGroupVO; +import com.cloud.utils.db.SearchBuilder; +import com.cloud.utils.db.SearchCriteria; + + +public class VolumeGroupDaoImpl extends GenericDaoBase implements VolumeGroupDao { + protected final SearchBuilder allFieldsSearch; + private static final String VOLUME_ID = "volumeId"; + private static final String VM_ID = "vmId"; + private static final String GROUP_NUMBER = "groupNumber"; + + public VolumeGroupDaoImpl(){ + allFieldsSearch = createSearchBuilder(); + allFieldsSearch.and(VM_ID, allFieldsSearch.entity().getVmId(), SearchCriteria.Op.EQ); + allFieldsSearch.and(VOLUME_ID, allFieldsSearch.entity().getVolumeId(), SearchCriteria.Op.EQ); + allFieldsSearch.and(GROUP_NUMBER, allFieldsSearch.entity().getGroupNumber(), SearchCriteria.Op.EQ); + allFieldsSearch.done(); + } + + @Override + public void addVolumeToGroup(long vmId, long volumeId, long deviceId, int groupNumber) { + if (groupNumber != -1) { + if (deviceId == 0L) { + groupNumber = 0; + } + VolumeGroupVO group = new VolumeGroupVO(); + group.setVmId(vmId); + group.setVolumeId(volumeId); + group.setGroupNumber(groupNumber); + persist(group); + } + } + + @Override + public void deleteVolumeFromGroup(long volumeId) { + SearchCriteria sc = allFieldsSearch.create(); + sc.setParameters(VOLUME_ID, volumeId); + expunge(sc); + } + + @Override + public VolumeGroupVO findByVmAndVolume(long vmId, long volumeId){ + SearchCriteria sc = allFieldsSearch.create(); + sc.setParameters(VM_ID, vmId); + sc.setParameters(VOLUME_ID, volumeId); + return findOneBy(sc); + } +} \ No newline at end of file diff --git a/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml b/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml index b35054ecd8ac..60aeb3cd316f 100644 --- a/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml +++ b/engine/schema/src/main/resources/META-INF/cloudstack/core/spring-engine-schema-core-daos-context.xml @@ -221,6 +221,8 @@ + + diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 04e9dd4cc969..3053ba30ffc6 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -2148,7 +2148,8 @@ protected StartAnswer execute(StartCommand cmd) { int i = 0; int ideUnitNumber = !deployAsIs ? 0 : vmMo.getNextIDEDeviceNumber(); - int scsiUnitNumber = !deployAsIs ? 0 : vmMo.getNextScsiDiskDeviceNumber(); + Map scsiUnitNumberMap = new HashMap<>(); + scsiUnitNumberMap.put(-1,!deployAsIs ? 0 : vmMo.getNextScsiDiskDeviceNumber()); int ideControllerKey = vmMo.getIDEDeviceControllerKey(); int scsiControllerKey = vmMo.getScsiDeviceControllerKeyNoException(); VirtualDeviceConfigSpec[] deviceConfigSpecArray = new VirtualDeviceConfigSpec[totalChangeDevices]; @@ -2263,6 +2264,8 @@ protected StartAnswer execute(StartCommand cmd) { } for (DiskTO vol : sortedDisks) { + // group 0 is root controller, therefore we replace it with -1 (volume with no group) for right device number + int groupNumber = vol.getGroupNumber() == 0 ? -1 : vol.getGroupNumber(); if (vol.getType() == Volume.Type.ISO) { if (deployAsIs) { configureIso(hyperHost, vmMo, vol, deviceConfigSpecArray, ideUnitNumber++, i); @@ -2293,17 +2296,17 @@ protected StartAnswer execute(StartCommand cmd) { } } } else { - if (VmwareHelper.isReservedScsiDeviceNumber(scsiUnitNumber)) { - scsiUnitNumber++; + if (VmwareHelper.isReservedScsiDeviceNumber(scsiUnitNumberMap.getOrDefault(groupNumber,0))) { + scsiUnitNumberMap.merge(groupNumber, 1, (a,b) -> a + b); } - controllerKey = vmMo.getScsiDiskControllerKeyNoException(diskController, scsiUnitNumber); + controllerKey = vmMo.getScsiDiskControllerKeyNoException(diskController, scsiUnitNumberMap.getOrDefault(groupNumber, 0), groupNumber); if (controllerKey == -1) { // This may happen for ROOT legacy VMs which doesn't have recommended disk controller when global configuration parameter 'vmware.root.disk.controller' is set to "osdefault" // Retrieve existing controller and use. Ternary vmScsiControllerInfo = vmMo.getScsiControllerInfo(); DiskControllerType existingControllerType = vmScsiControllerInfo.third(); - controllerKey = vmMo.getScsiDiskControllerKeyNoException(existingControllerType.toString(), scsiUnitNumber); + controllerKey = vmMo.getScsiDiskControllerKeyNoException(existingControllerType.toString(), scsiUnitNumberMap.getOrDefault(groupNumber, 0), groupNumber); } } if (!hasSnapshot) { @@ -2361,8 +2364,8 @@ protected StartAnswer execute(StartCommand cmd) { deviceNumber = ideUnitNumber % VmwareHelper.MAX_ALLOWED_DEVICES_IDE_CONTROLLER; ideUnitNumber++; } else { - deviceNumber = scsiUnitNumber % VmwareHelper.MAX_ALLOWED_DEVICES_SCSI_CONTROLLER; - scsiUnitNumber++; + deviceNumber = scsiUnitNumberMap.getOrDefault(groupNumber, 0) % VmwareHelper.MAX_ALLOWED_DEVICES_SCSI_CONTROLLER; + scsiUnitNumberMap.merge(groupNumber, 1, (a,b) -> a + b); } VirtualDevice device = VmwareHelper.prepareDiskDevice(vmMo, null, controllerKey, diskChain, volumeDsDetails.first(), deviceNumber, i + 1); @@ -2392,7 +2395,7 @@ protected StartAnswer execute(StartCommand cmd) { if (controllerKey == vmMo.getIDEControllerKey(ideUnitNumber)) ideUnitNumber++; else - scsiUnitNumber++; + scsiUnitNumberMap.merge(groupNumber, 1, (a,b) -> a + b); } } @@ -3661,7 +3664,6 @@ public int compare(NicTO arg0, NicTO arg1) { } private static DiskTO[] sortVolumesByDeviceId(DiskTO[] volumes) { - List listForSort = new ArrayList(); for (DiskTO vol : volumes) { listForSort.add(vol); @@ -3670,12 +3672,31 @@ private static DiskTO[] sortVolumesByDeviceId(DiskTO[] volumes) { @Override public int compare(DiskTO arg0, DiskTO arg1) { + if (arg0.getType().equals(Volume.Type.ROOT)){ + return -1; + } + if(arg1.getType().equals(Volume.Type.ROOT)){ + return 1; + } + if(arg0.getGroupNumber() > -1 && arg1.getGroupNumber() > -1){ + return checkDiskSeq(arg0, arg1); + } + if(arg0.getGroupNumber() > -1){ + return -1; + } + if(arg1.getGroupNumber() > -1){ + return 1; + } + return checkDiskSeq(arg0, arg1); + } + + private int checkDiskSeq(DiskTO arg0, DiskTO arg1){ if (arg0.getDiskSeq() < arg1.getDiskSeq()) { return -1; - } else if (arg0.getDiskSeq().equals(arg1.getDiskSeq())) { + } + if (arg0.getDiskSeq().equals(arg1.getDiskSeq())) { return 0; } - return 1; } }); diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java index 403daf3d4b03..3457be5f4dd8 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java @@ -2128,7 +2128,7 @@ private Answer attachVolume(Command cmd, DiskTO disk, boolean isAttach, boolean diskController = vmMo.getRecommendedDiskController(null); } - vmMo.attachDisk(new String[] { datastoreVolumePath }, morDs, diskController, storagePolicyId); + vmMo.attachDisk(new String[] { datastoreVolumePath }, morDs, diskController, storagePolicyId, disk.getGroupNumber()); VirtualMachineDiskInfoBuilder diskInfoBuilder = vmMo.getDiskInfoBuilder(); VirtualMachineDiskInfo diskInfo = diskInfoBuilder.getDiskInfoByBackingFileBaseName(volumePath, dsMo.getName()); chainInfo = _gson.toJson(diskInfo); @@ -2450,7 +2450,7 @@ public Answer createVolume(CreateObjectCommand cmd) { synchronized (this) { try { - vmMo.createDisk(volumeDatastorePath, (int)(volume.getSize() / (1024L * 1024L)), morDatastore, vmMo.getScsiDeviceControllerKey(), vSphereStoragePolicyId); + vmMo.createDisk(volumeDatastorePath, (int)(volume.getSize() / (1024L * 1024L)), morDatastore, vmMo.getScsiDeviceControllerKey(-1), vSphereStoragePolicyId); vmMo.detachDisk(volumeDatastorePath, false); } catch (Exception e1) { @@ -3139,7 +3139,7 @@ private void createVmdk(Command cmd, DatastoreMO dsMo, String vmdkDatastorePath, Long volumeSizeToUse = volumeSize < dsMo.getDatastoreSummary().getFreeSpace() ? volumeSize : dsMo.getDatastoreSummary().getFreeSpace(); - vmMo.createDisk(vmdkDatastorePath, getMBsFromBytes(volumeSizeToUse), dsMo.getMor(), vmMo.getScsiDeviceControllerKey(), null); + vmMo.createDisk(vmdkDatastorePath, getMBsFromBytes(volumeSizeToUse), dsMo.getMor(), vmMo.getScsiDeviceControllerKey(-1), null); vmMo.detachDisk(vmdkDatastorePath, false); vmMo.destroy(); } diff --git a/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java b/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java index 3c8b6c58d9bf..1182c2d856fb 100644 --- a/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java +++ b/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java @@ -55,6 +55,8 @@ import com.cloud.service.dao.ServiceOfferingDetailsDao; import com.cloud.storage.StoragePool; import com.cloud.storage.Volume; +import com.cloud.storage.VolumeGroupVO; +import com.cloud.storage.dao.VolumeGroupDao; import com.cloud.utils.Pair; import com.cloud.utils.component.AdapterBase; import com.cloud.vm.NicProfile; @@ -94,8 +96,9 @@ public abstract class HypervisorGuruBase extends AdapterBase implements Hypervis @Inject private NetworkDetailsDao networkDetailsDao; @Inject - protected - HostDao hostDao; + protected HostDao hostDao; + @Inject + private VolumeGroupDao volumeGroupDao; public static ConfigKey VmMinMemoryEqualsMemoryDividedByMemOverprovisioningFactor = new ConfigKey("Advanced", Boolean.class, "vm.min.memory.equals.memory.divided.by.mem.overprovisioning.factor", "true", "If we set this to 'true', a minimum memory (memory/ mem.overprovisioning.factor) will be set to the VM, independent of using a scalable service offering or not.", true, ConfigKey.Scope.Cluster); @@ -250,7 +253,8 @@ protected VirtualMachineTO toVirtualMachineTO(VirtualMachineProfile vmProfile) { } to.setNics(nics); - to.setDisks(vmProfile.getDisks().toArray(new DiskTO[vmProfile.getDisks().size()])); + List disks = addGroupsToDisks(vm.getId(), vmProfile.getDisks()); + to.setDisks(disks.toArray(new DiskTO[vmProfile.getDisks().size()])); if (vmProfile.getTemplate().getBits() == 32) { to.setArch("i686"); @@ -304,6 +308,16 @@ protected Long findClusterOfVm(VirtualMachine vm) { return null; } + private List addGroupsToDisks(long vmId, List disks){ + disks.forEach((DiskTO disk) -> { + VolumeGroupVO volumeGroup = volumeGroupDao.findByVmAndVolume(vmId, disk.getData().getId()); + if(volumeGroup != null){ + disk.setGroupNumber(volumeGroup.getGroupNumber()); + } + }); + return disks; + } + @Override /** * The basic implementation assumes that the initial "host" defined to execute the command is the host that is in fact going to execute it. diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index bb4da36dd988..96e5bb1aabbe 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -155,6 +155,7 @@ import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.storage.dao.VolumeDetailsDao; +import com.cloud.storage.dao.VolumeGroupDao; import com.cloud.storage.snapshot.SnapshotApiService; import com.cloud.storage.snapshot.SnapshotManager; import com.cloud.template.TemplateManager; @@ -241,6 +242,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic @Inject private VolumeDetailsDao _volsDetailsDao; @Inject + private VolumeGroupDao volsGroupDao; + @Inject private HostDao _hostDao; @Inject private SnapshotDao _snapshotDao; @@ -964,7 +967,7 @@ public VolumeVO createVolume(CreateVolumeCmd cmd) { // if VM Id is provided, attach the volume to the VM if (cmd.getVirtualMachineId() != null) { try { - attachVolumeToVM(cmd.getVirtualMachineId(), volume.getId(), volume.getDeviceId()); + attachVolumeToVM(cmd.getVirtualMachineId(), volume.getId(), volume.getDeviceId(),-1); } catch (Exception ex) { StringBuilder message = new StringBuilder("Volume: "); message.append(volume.getUuid()); @@ -1701,6 +1704,7 @@ public Volume recoverVolume(long volumeId) { try { _volsDao.detachVolume(volume.getId()); + volsGroupDao.deleteVolumeFromGroup(volume.getId()); stateTransitTo(volume, Volume.Event.RecoverRequested); } catch (NoTransitionException e) { s_logger.debug("Failed to recover volume" + volume.getId(), e); @@ -2102,10 +2106,10 @@ private void validateVolumeResizeWithSize(VolumeVO volume, long currentSize, Lon @Override @ActionEvent(eventType = EventTypes.EVENT_VOLUME_ATTACH, eventDescription = "attaching volume", async = true) public Volume attachVolumeToVM(AttachVolumeCmd command) { - return attachVolumeToVM(command.getVirtualMachineId(), command.getId(), command.getDeviceId()); + return attachVolumeToVM(command.getVirtualMachineId(), command.getId(), command.getDeviceId(), command.getVolumeGroup()); } - private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long deviceId) { + private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long deviceId, int groubNumber) { VolumeInfo volumeToAttach = volFactory.getVolume(volumeId); if (volumeToAttach.isAttachedVM()) { @@ -2206,11 +2210,11 @@ private Volume orchestrateAttachVolumeToVM(Long vmId, Long volumeId, Long device throw new InvalidParameterValueException("VM not found."); } } - newVol = sendAttachVolumeCommand(vm, newVol, deviceId); + newVol = sendAttachVolumeCommand(vm, newVol, deviceId, groubNumber); return newVol; } - public Volume attachVolumeToVM(Long vmId, Long volumeId, Long deviceId) { + public Volume attachVolumeToVM(Long vmId, Long volumeId, Long deviceId, int groupNumber) { Account caller = CallContext.current().getCallingAccount(); VolumeInfo volumeToAttach = getAndCheckVolumeInfo(volumeId); @@ -2259,14 +2263,14 @@ public Volume attachVolumeToVM(Long vmId, Long volumeId, Long deviceId) { _jobMgr.updateAsyncJobAttachment(job.getId(), "Volume", volumeId); if (asyncExecutionContext.isJobDispatchedBy(VmWorkConstants.VM_WORK_JOB_DISPATCHER)) { - return safelyOrchestrateAttachVolume(vmId, volumeId, deviceId); + return safelyOrchestrateAttachVolume(vmId, volumeId, deviceId, groupNumber); } else { - return getVolumeAttachJobResult(vmId, volumeId, deviceId); + return getVolumeAttachJobResult(vmId, volumeId, deviceId, groupNumber); } } - @Nullable private Volume getVolumeAttachJobResult(Long vmId, Long volumeId, Long deviceId) { - Outcome outcome = attachVolumeToVmThroughJobQueue(vmId, volumeId, deviceId); + @Nullable private Volume getVolumeAttachJobResult(Long vmId, Long volumeId, Long deviceId, int groupNumber) { + Outcome outcome = attachVolumeToVmThroughJobQueue(vmId, volumeId, deviceId, groupNumber); Volume vol = null; try { @@ -2292,13 +2296,13 @@ public Volume attachVolumeToVM(Long vmId, Long volumeId, Long deviceId) { return vol; } - private Volume safelyOrchestrateAttachVolume(Long vmId, Long volumeId, Long deviceId) { + private Volume safelyOrchestrateAttachVolume(Long vmId, Long volumeId, Long deviceId, int groupNumber) { // avoid re-entrance VmWorkJobVO placeHolder = null; placeHolder = createPlaceHolderWork(vmId); try { - return orchestrateAttachVolumeToVM(vmId, volumeId, deviceId); + return orchestrateAttachVolumeToVM(vmId, volumeId, deviceId, groupNumber); } finally { _workJobDao.expunge(placeHolder.getId()); } @@ -2772,6 +2776,7 @@ private Volume orchestrateDetachVolumeFromVM(long vmId, long volumeId) { if (!sendCommand || (answer != null && answer.getResult())) { // Mark the volume as detached _volsDao.detachVolume(volume.getId()); + volsGroupDao.deleteVolumeFromGroup(volume.getId()); if (answer != null) { String datastoreName = answer.getContextParam("datastoreName"); @@ -3898,7 +3903,7 @@ private HostVO getHostForVmVolumeAttach(UserVmVO vm, StoragePoolVO volumeToAttac return host; } - private VolumeVO sendAttachVolumeCommand(UserVmVO vm, VolumeVO volumeToAttach, Long deviceId) { + private VolumeVO sendAttachVolumeCommand(UserVmVO vm, VolumeVO volumeToAttach, Long deviceId, int groupNumber) { String errorMsg = "Failed to attach volume " + volumeToAttach.getName() + " to VM " + vm.getHostName(); boolean sendCommand = vm.getState() == State.Running; AttachAnswer answer = null; @@ -3960,6 +3965,7 @@ private VolumeVO sendAttachVolumeCommand(UserVmVO vm, VolumeVO volumeToAttach, L DiskTO disk = storageMgr.getDiskWithThrottling(volTO, volumeToAttach.getVolumeType(), deviceId, volumeToAttach.getPath(), vm.getServiceOfferingId(), volumeToAttach.getDiskOfferingId()); + disk.setGroupNumber(groupNumber); AttachCommand cmd = new AttachCommand(disk, vm.getInstanceName()); @@ -4015,6 +4021,7 @@ private VolumeVO sendAttachVolumeCommand(UserVmVO vm, VolumeVO volumeToAttach, L DiskTO disk = answer.getDisk(); _volsDao.attachVolume(volumeToAttach.getId(), vm.getId(), disk.getDiskSeq()); + volsGroupDao.addVolumeToGroup(vm.getId(), volumeToAttach.getId(), deviceId, groupNumber); volumeToAttach = _volsDao.findById(volumeToAttach.getId()); @@ -4039,6 +4046,7 @@ private VolumeVO sendAttachVolumeCommand(UserVmVO vm, VolumeVO volumeToAttach, L deviceId = getDeviceId(vm, deviceId); _volsDao.attachVolume(volumeToAttach.getId(), vm.getId(), deviceId); + volsGroupDao.addVolumeToGroup(vm.getId(), volumeToAttach.getId(), deviceId, groupNumber); volumeToAttach = _volsDao.findById(volumeToAttach.getId()); @@ -4245,7 +4253,7 @@ protected Snapshot retrieve() { } } - public Outcome attachVolumeToVmThroughJobQueue(final Long vmId, final Long volumeId, final Long deviceId) { + public Outcome attachVolumeToVmThroughJobQueue(final Long vmId, final Long volumeId, final Long deviceId, final int groubNumber) { final CallContext context = CallContext.current(); final User callingUser = context.getCallingUser(); @@ -4266,7 +4274,7 @@ public Outcome attachVolumeToVmThroughJobQueue(final Long vmId, final Lo workJob.setRelated(AsyncJobExecutionContext.getOriginJobId()); // save work context info (there are some duplications) - VmWorkAttachVolume workInfo = new VmWorkAttachVolume(callingUser.getId(), callingAccount.getId(), vm.getId(), VolumeApiServiceImpl.VM_WORK_JOB_HANDLER, volumeId, deviceId); + VmWorkAttachVolume workInfo = new VmWorkAttachVolume(callingUser.getId(), callingAccount.getId(), vm.getId(), VolumeApiServiceImpl.VM_WORK_JOB_HANDLER, volumeId, deviceId, groubNumber); workJob.setCmdInfo(VmWorkSerializer.serialize(workInfo)); _jobMgr.submitAsyncJob(workJob, VmWorkConstants.VM_WORK_QUEUE, vm.getId()); @@ -4445,7 +4453,7 @@ private Pair orchestrateExtractVolume(VmWorkExtractVolum @ReflectionUse private Pair orchestrateAttachVolumeToVM(VmWorkAttachVolume work) throws Exception { - Volume vol = orchestrateAttachVolumeToVM(work.getVmId(), work.getVolumeId(), work.getDeviceId()); + Volume vol = orchestrateAttachVolumeToVM(work.getVmId(), work.getVolumeId(), work.getDeviceId(), work.getGroupNumber()); return new Pair(JobInfo.Status.SUCCEEDED, _jobMgr.marshallResultObject(new Long(vol.getId()))); } diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 0a4599e1560c..238ac18345c5 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -681,7 +681,7 @@ private Map getUnmanagedNicNetworkMap(List importDisk(UnmanagedInstanceTO.Disk disk, VirtualMachine vm, Cluster cluster, DiskOffering diskOffering, Volume.Type type, String name, Long diskSize, Long minIops, Long maxIops, VirtualMachineTemplate template, - Account owner, Long deviceId) { + Account owner, Long deviceId, Integer volumeGroup, Boolean useControllerConfiguration) { final DataCenter zone = dataCenterDao.findById(vm.getDataCenterId()); final String path = StringUtils.isEmpty(disk.getFileBaseName()) ? disk.getImagePath() : disk.getFileBaseName(); String chainInfo = disk.getChainInfo(); @@ -692,8 +692,13 @@ private Pair importDisk(UnmanagedInstanceTO.Disk disk, chainInfo = gson.toJson(diskInfo); } StoragePool storagePool = getStoragePool(disk, zone, cluster); + + if (Boolean.TRUE.equals(useControllerConfiguration)) { + volumeGroup = disk.getControllerUnit(); + } + DiskProfile profile = volumeManager.importVolume(type, name, diskOffering, diskSize, - minIops, maxIops, vm, template, owner, deviceId, storagePool.getId(), path, chainInfo); + minIops, maxIops, vm, template, owner, deviceId, storagePool.getId(), path, chainInfo, volumeGroup); return new Pair(profile, storagePool); } @@ -908,9 +913,9 @@ private void publishVMUsageUpdateResourceCount(final UserVm userVm, ServiceOffer private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedInstance, final String instanceName, final DataCenter zone, final Cluster cluster, final HostVO host, final VirtualMachineTemplate template, final String displayName, final String hostName, final Account caller, final Account owner, final Long userId, - final ServiceOfferingVO serviceOffering, final Map dataDiskOfferingMap, + final ServiceOfferingVO serviceOffering, final Map dataDiskOfferingMap, final Map diskVolumeGroupMap, final Map nicNetworkMap, final Map callerNicIpAddressMap, - final Map details, final boolean migrateAllowed, final boolean forced) { + final Map details, final boolean migrateAllowed, final boolean forced, boolean useControllerConfiguration) { UserVm userVm = null; ServiceOfferingVO validatedServiceOffering = null; @@ -998,16 +1003,20 @@ private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedI DiskOfferingVO diskOffering = diskOfferingDao.findById(serviceOffering.getDiskOfferingId()); diskProfileStoragePoolList.add(importDisk(rootDisk, userVm, cluster, diskOffering, Volume.Type.ROOT, String.format("ROOT-%d", userVm.getId()), (rootDisk.getCapacity() / Resource.ResourceType.bytesToGiB), minIops, maxIops, - template, owner, null)); + template, owner, null, null, false)); long deviceId = 1L; for (UnmanagedInstanceTO.Disk disk : dataDisks) { + Integer volumeGroup = -1; + if (diskVolumeGroupMap != null && !diskVolumeGroupMap.isEmpty()) { + volumeGroup = diskVolumeGroupMap.get(disk.getDiskId()); + } if (disk.getCapacity() == null || disk.getCapacity() == 0) { throw new InvalidParameterValueException(String.format("Disk ID: %s size is invalid", rootDisk.getDiskId())); } DiskOffering offering = diskOfferingDao.findById(dataDiskOfferingMap.get(disk.getDiskId())); diskProfileStoragePoolList.add(importDisk(disk, userVm, cluster, offering, Volume.Type.DATADISK, String.format("DATA-%d-%s", userVm.getId(), disk.getDiskId()), (disk.getCapacity() / Resource.ResourceType.bytesToGiB), offering.getMinIops(), offering.getMaxIops(), - template, owner, deviceId)); + template, owner, deviceId, volumeGroup, useControllerConfiguration)); deviceId++; } } catch (Exception e) { @@ -1180,6 +1189,7 @@ public UserVmResponse importUnmanagedInstance(ImportUnmanagedInstanceCmd cmd) { final Map nicNetworkMap = cmd.getNicNetworkList(); final Map nicIpAddressMap = cmd.getNicIpAddressList(); final Map dataDiskOfferingMap = cmd.getDataDiskToDiskOfferingList(); + final Map diskVolumeGroupMap = cmd.getVolumeGroups(); final Map details = cmd.getDetails(); final boolean forced = cmd.isForced(); List hosts = resourceManager.listHostsInClusterByStatus(clusterId, Status.Up); @@ -1233,9 +1243,9 @@ public UserVmResponse importUnmanagedInstance(ImportUnmanagedInstanceCmd cmd) { } userVm = importVirtualMachineInternal(unmanagedInstance, instanceName, zone, cluster, host, template, displayName, hostName, caller, owner, userId, - serviceOffering, dataDiskOfferingMap, + serviceOffering, dataDiskOfferingMap, diskVolumeGroupMap, nicNetworkMap, nicIpAddressMap, - details, cmd.getMigrateAllowed(), forced); + details, cmd.getMigrateAllowed(), forced, cmd.isUseControllerConfiguration()); break; } } diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index 4c4bbf789b06..748f645dda50 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -415,44 +415,44 @@ public void testDetachVolumeFromStoppedXenVm() throws NoSuchFieldException, Ille // Negative test - try to attach non-root non-datadisk volume @Test(expected = InvalidParameterValueException.class) public void attachIncorrectDiskType() throws NoSuchFieldException, IllegalAccessException { - volumeApiServiceImpl.attachVolumeToVM(1L, 5L, 0L); + volumeApiServiceImpl.attachVolumeToVM(1L, 5L, 0L, -1); } // Negative test - attach root volume to running vm @Test(expected = InvalidParameterValueException.class) public void attachRootDiskToRunningVm() throws NoSuchFieldException, IllegalAccessException { - volumeApiServiceImpl.attachVolumeToVM(1L, 6L, 0L); + volumeApiServiceImpl.attachVolumeToVM(1L, 6L, 0L, -1); } // Negative test - attach root volume to non-xen vm @Test(expected = InvalidParameterValueException.class) public void attachRootDiskToHyperVm() throws NoSuchFieldException, IllegalAccessException { - volumeApiServiceImpl.attachVolumeToVM(3L, 6L, 0L); + volumeApiServiceImpl.attachVolumeToVM(3L, 6L, 0L, -1); } // Negative test - attach root volume from the managed data store @Test(expected = InvalidParameterValueException.class) public void attachRootDiskOfManagedDataStore() throws NoSuchFieldException, IllegalAccessException { - volumeApiServiceImpl.attachVolumeToVM(2L, 7L, 0L); + volumeApiServiceImpl.attachVolumeToVM(2L, 7L, 0L, -1); } // Negative test - root volume can't be attached to the vm already having a root volume attached @Test(expected = InvalidParameterValueException.class) public void attachRootDiskToVmHavingRootDisk() throws NoSuchFieldException, IllegalAccessException { - volumeApiServiceImpl.attachVolumeToVM(4L, 6L, 0L); + volumeApiServiceImpl.attachVolumeToVM(4L, 6L, 0L, -1); } // Negative test - root volume in uploaded state can't be attached @Test(expected = InvalidParameterValueException.class) public void attachRootInUploadedState() throws NoSuchFieldException, IllegalAccessException { - volumeApiServiceImpl.attachVolumeToVM(2L, 8L, 0L); + volumeApiServiceImpl.attachVolumeToVM(2L, 8L, 0L, -1); } // Positive test - attach ROOT volume in correct state, to the vm not having root volume attached @Test public void attachRootVolumePositive() throws NoSuchFieldException, IllegalAccessException { thrown.expect(NullPointerException.class); - volumeApiServiceImpl.attachVolumeToVM(2L, 6L, 0L); + volumeApiServiceImpl.attachVolumeToVM(2L, 6L, 0L, -1); } // Negative test - attach data volume, to the vm on non-kvm hypervisor @@ -461,7 +461,7 @@ public void attachDiskWithEncryptEnabledOfferingonNonKVM() throws NoSuchFieldExc DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class); when(diskOffering.getEncrypt()).thenReturn(true); when(_diskOfferingDao.findById(anyLong())).thenReturn(diskOffering); - volumeApiServiceImpl.attachVolumeToVM(2L, 10L, 1L); + volumeApiServiceImpl.attachVolumeToVM(2L, 10L, 1L,-1); } // Positive test - attach data volume, to the vm on kvm hypervisor @@ -471,7 +471,7 @@ public void attachDiskWithEncryptEnabledOfferingOnKVM() throws NoSuchFieldExcept DiskOfferingVO diskOffering = Mockito.mock(DiskOfferingVO.class); when(diskOffering.getEncrypt()).thenReturn(true); when(_diskOfferingDao.findById(anyLong())).thenReturn(diskOffering); - volumeApiServiceImpl.attachVolumeToVM(4L, 10L, 1L); + volumeApiServiceImpl.attachVolumeToVM(4L, 10L, 1L,-1); } // volume not Ready @@ -581,7 +581,7 @@ public void testResourceLimitCheckForUploadedVolume() throws NoSuchFieldExceptio when(_dcDao.findById(anyLong())).thenReturn(zoneWithDisabledLocalStorage); when(zoneWithDisabledLocalStorage.isLocalStorageEnabled()).thenReturn(true); try { - volumeApiServiceImpl.attachVolumeToVM(2L, 9L, null); + volumeApiServiceImpl.attachVolumeToVM(2L, 9L, null, 1); } catch (InvalidParameterValueException e) { Assert.assertEquals(e.getMessage(), ("primary storage resource limit check failed")); } diff --git a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java index bf392916fd32..62a0d538c0f8 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -315,7 +315,7 @@ public void setUp() throws Exception { when(networkOrchestrationService.importNic(nullable(String.class), nullable(Integer.class), nullable(Network.class), nullable(Boolean.class), nullable(VirtualMachine.class), nullable(Network.IpAddresses.class), anyBoolean())).thenReturn(pair); when(volumeManager.importVolume(Mockito.any(Volume.Type.class), Mockito.anyString(), Mockito.any(DiskOffering.class), Mockito.anyLong(), Mockito.anyLong(), Mockito.anyLong(), Mockito.any(VirtualMachine.class), Mockito.any(VirtualMachineTemplate.class), - Mockito.any(Account.class), Mockito.anyLong(), Mockito.anyLong(), Mockito.anyString(), Mockito.anyString())).thenReturn(Mockito.mock(DiskProfile.class)); + Mockito.any(Account.class), Mockito.anyLong(), Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.eq(-1))).thenReturn(Mockito.mock(DiskProfile.class)); when(volumeDao.findByInstance(Mockito.anyLong())).thenReturn(volumes); List userVmResponses = new ArrayList<>(); UserVmResponse userVmResponse = new UserVmResponse(); diff --git a/test/integration/component/test_volume_groups.py b/test/integration/component/test_volume_groups.py new file mode 100644 index 000000000000..3a2b5c2595b1 --- /dev/null +++ b/test/integration/component/test_volume_groups.py @@ -0,0 +1,299 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" P1 tests for Volumes +""" +# Import Local Modules +from nose.plugins.attrib import attr +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.base import (Account, + ServiceOffering, + VirtualMachine, + Volume, + Cluster, + DiskOffering) +from marvin.lib.common import (get_domain, + get_zone, + get_template, + get_pod) + +import re + + + +class TestAttachVolumeWithGroup(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + cls.test_client = super(TestAttachVolumeWithGroup, cls).getClsTestClient() + cls.api_client = cls.test_client.getApiClient() + cls.test_data = cls.test_client.getParsedTestDataConfig() + cls.domain = get_domain(cls.api_client) + cls.zone = get_zone(cls.api_client, cls.test_client.getZoneForTests()) + cls.pod = get_pod(cls.api_client, cls.zone.id) + cls.check_volume_group_tbl = "SELECT volume_group.id FROM volume_group,vm_instance WHERE volume_group.vm_id = vm_instance.id AND vm_instance.uuid='{}'" + cls.template = get_template( + cls.api_client, + cls.zone.id, + cls.test_data["ostype"] + ) + cls._cleanup = [] + + cls.disk_offering = DiskOffering.create( + cls.api_client, + cls.test_data["disk_offering"] + ) + cls._cleanup.append(cls.disk_offering) + + cls.account = Account.create( + cls.api_client, + cls.test_data["account"], + domainid=cls.domain.id + ) + cls._cleanup.append(cls.account) + + cls.service_offering = ServiceOffering.create( + cls.api_client, + cls.test_data["service_offering"] + ) + cls._cleanup.append(cls.service_offering) + + + + @classmethod + def tearDownClass(cls): + super(TestAttachVolumeWithGroup, cls).tearDownClass() + + def setUp(self): + self.api_client = self.test_client.getApiClient() + self.dbclient = self.testClient.getDbConnection() + self.cleanup = [] + + def tearDown(self): + super(TestAttachVolumeWithGroup, self).tearDown() + + @attr(tags=["advanced", "advancedns", "needle"]) + def test_attach_mixed_volumes(self): + virtual_machine = VirtualMachine.create( + self.api_client, + self.test_data["virtual_machine"], + accountid=self.account.name, + domainid=self.account.domainid, + serviceofferingid=self.service_offering.id, + templateid=self.template.id, + zoneid=self.zone.id + ) + self.cleanup.append(virtual_machine) + volume_list = [] + volume_count = 6 + for i in range(0, volume_count): + volume_list.append([Volume.create( + self.api_client, + self.test_data["volume"], + zoneid=self.zone.id, + account=self.account.name, + domainid=self.account.domainid, + diskofferingid=self.disk_offering.id + ), 0, 0]) + self.cleanup.append(volume_list[i]) + + for i in range(0, volume_count): + if i > 3: + virtual_machine.attach_volume( + self.api_client, + volume_list[i][0], + volumegroup=2 + ) + volume_list[i][1] = 2 + volume_list[i][2] = i % 2 + continue + if i > 1: + virtual_machine.attach_volume( + self.api_client, + volume_list[i][0], + volumegroup=1 + ) + volume_list[i][1] = 1 + volume_list[i][2] = i % 2 + continue + + virtual_machine.attach_volume( + self.api_client, + volume_list[i][0] + ) + volume_list[i][1] = 0 + volume_list[i][2] = (i % 2) + 1 + + for i in range(0, volume_count): + vol_res = Volume.list( + self.api_client, + id=volume_list[i][0].id + ) + controller = "scsi{}:{}".format(volume_list[i][1], volume_list[i][2]) + if vol_res is not None: + m = re.search(controller, vol_res[0]['chaininfo']) + if m is not None: + self.assertTrue(True) + else: + self.assertTrue(False, "Volume is not on the right controller") + else: + self.assertTrue(False, "Volume not found with id: {}".format(volume_list[i][0].id)) + + @attr(tags=["advanced", "advancedns", "needle"]) + def test_unmanage_ingest_vm_with_vol_groups(self): + self.helper_unmanageInstance() + + @attr(tags=["advanced", "advancedns", "needle"]) + def test_unmanage_ingest_vm_with_controller_conf_param(self): + self.helper_unmanageInstance(False, True) + + def helper_unmanageInstance(self, withVolGroups=True, withUseControllerConf=False): + virtual_machine = VirtualMachine.create( + self.api_client, + self.test_data['virtual_machine'], + accountid=self.account.name, + domainid=self.account.domainid, + serviceofferingid=self.service_offering.id, + templateid=self.template.id, + zoneid=self.zone.id + ) + volume_count = 3 + volume_list = [] + + for i in range(0, volume_count): + volume_list.append(Volume.create( + self.api_client, + self.test_data["volume"], + zoneid=self.zone.id, + account=self.account.name, + domainid=self.account.domainid, + diskofferingid=self.disk_offering.id)) + + for i in range(0, volume_count): + virtual_machine.attach_volume( + self.api_client, + volume_list[i], + volumegroup=i) + + virtual_machine.unmanage(self.api_client) + result = self.dbclient.execute(self.check_volume_group_tbl.format(virtual_machine.id)) + self.assertEqual(len(result), 0, msg="After unmanage vm, at least one volume group was not removed") + + cluster_list = Cluster.list(self.api_client, + hypervisor=virtual_machine.hypervisor, + zonename=virtual_machine.zonename) + + for cluster in cluster_list: + unmanage_vm_list = VirtualMachine.listUnmanagedInstances(self.api_client, + clusterid=cluster.id, + name=virtual_machine.instancename) + + if unmanage_vm_list and len(unmanage_vm_list) == 1: + data_disk_offering_list = [] + controller_units = [0, 1, 2] + for volume in unmanage_vm_list[0].disk: + if not (volume.controllerunit == 0 and volume.position == 0): + if(withVolGroups): + data_disk_offering_list.append({ + 'disk': volume.id, + 'diskOffering': self.disk_offering.id, + 'volumeGroup': volume.controllerunit + }) + else: + data_disk_offering_list.append({ + 'disk': volume.id, + 'diskOffering': self.disk_offering.id + }) + self.assertIn(volume.controllerunit, controller_units, + msg="After unmanage vm, at least one volume is connected on wrong controller") + controller_units.remove(volume.controllerunit) + + details = {'dataDiskController': virtual_machine.details['dataDiskController'], + 'rootDiskController': virtual_machine.details['rootDiskController']} + if(withUseControllerConf): + imported_vm = VirtualMachine.importUnmanagedVM(apiclient=self.api_client, + clusterid=unmanage_vm_list[0].clusterid, + name=unmanage_vm_list[0].name, + details=[details], + datadiskofferinglist=data_disk_offering_list, + serviceofferingid=virtual_machine.serviceofferingid, + useControllerConfiguration='true') + else: + imported_vm = VirtualMachine.importUnmanagedVM(apiclient=self.api_client, + clusterid=unmanage_vm_list[0].clusterid, + name=unmanage_vm_list[0].name, + details=[details], + datadiskofferinglist=data_disk_offering_list, + serviceofferingid=virtual_machine.serviceofferingid) + vm = VirtualMachine(imported_vm.__dict__, {}) + vm.stop(apiclient=self.api_client) + vm.start(apiclient=self.api_client) + + vols = Volume.list(apiclient=self.api_client, virtualmachineid=vm.id) + allowed_controller_cfg = ["scsi0:0", "scsi0:1", "scsi1:0", "scsi2:0"] + for vol in vols: + for cfg in allowed_controller_cfg: + m = re.search(cfg, vol['chaininfo']) + if m is not None: + allowed_controller_cfg.remove(cfg) + break + self.cleanup.append(vol) + self.assertEqual(len(allowed_controller_cfg), 0, + msg="After import vm, at least one volume is connected on the wrong controller.") + + vm.delete(apiclient=self.api_client, expunge=True) + result = self.dbclient.execute(self.check_volume_group_tbl.format(vm.id)) + self.assertEqual(len(result), 0, msg="After delete vm, at least one volume group was not removed") + break + + @attr(tags=["advanced", "advancedns", "needle"]) + def test_controller_pos_seven_with_groups(self): + virtual_machine = VirtualMachine.create( + self.api_client, + self.test_data['virtual_machine'], + accountid=self.account.name, + domainid=self.account.domainid, + serviceofferingid=self.service_offering.id, + templateid=self.template.id, + zoneid=self.zone.id + ) + self.cleanup.append(virtual_machine) + volume_count = 8 + volume_list = [] + + for i in range(0, volume_count): + volume_list.append(Volume.create( + self.api_client, + self.test_data["volume"], + zoneid=self.zone.id, + account=self.account.name, + domainid=self.account.domainid, + diskofferingid=self.disk_offering.id)) + self.cleanup.append(volume_list[i]) + + for i in range(0, 2): + for j in range(0, volume_count): + virtual_machine.attach_volume( + self.api_client, + volume_list[j], + volumegroup=i) + + virtual_machine.stop(self.api_client) + virtual_machine.start(self.api_client) + + virtual_machine.detach_volume( + self.api_client, + volume_list[j]) diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index 00e834ffc4ff..ae6f6b45365a 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -889,7 +889,7 @@ def migrate_vm_with_volume(self, apiclient, hostid=None, migrateto=None): }) apiclient.migrateVirtualMachineWithVolume(cmd) - def attach_volume(self, apiclient, volume, deviceid=None): + def attach_volume(self, apiclient, volume, deviceid=None, volumegroup=None): """Attach volume to instance""" cmd = attachVolume.attachVolumeCmd() cmd.id = volume.id @@ -898,6 +898,9 @@ def attach_volume(self, apiclient, volume, deviceid=None): if deviceid is not None: cmd.deviceid = deviceid + if volumegroup is not None: + cmd.volumegroup = volumegroup + return apiclient.attachVolume(cmd) def detach_volume(self, apiclient, volume): @@ -1030,6 +1033,57 @@ def scale(self, apiclient, serviceOfferingId, cmd.details[0]["memory"] = custommemory return apiclient.scaleVirtualMachine(cmd) + @classmethod + def importUnmanagedVM(cls, apiclient, clusterid, name, serviceofferingid, displayname=None, + diskofferinglist=None, nicnetworklist=None, nicipaddresslist=None, + datadiskofferinglist=None, details=None, projectid=None, hostname=None, + migrateallowed=None, domainid=None, account=None, templateid=None, + useControllerConfiguration=None): + cmd = importUnmanagedInstance.importUnmanagedInstanceCmd() + cmd.clusterid = clusterid + cmd.name = name + cmd.serviceofferingid = serviceofferingid + cmd.useControllerConfiguration = useControllerConfiguration + + if displayname: + cmd.displayname = displayname + + if diskofferinglist: + cmd.diskofferinglist = diskofferinglist + + if nicnetworklist: + cmd.nicnetworklist = nicnetworklist + + if nicipaddresslist: + cmd.nicipaddresslist = nicipaddresslist + + if datadiskofferinglist: + cmd.datadiskofferinglist = datadiskofferinglist + + if details: + cmd.details = details + + if projectid: + cmd.projectid = projectid + + if hostname: + cmd.hostname = hostname + + if migrateallowed: + cmd.migrateallowed = migrateallowed + + if domainid: + cmd.domainid = domainid + + if account: + cmd.account = account + + if templateid: + cmd.templateid = templateid + + return apiclient.importUnmanagedInstance(cmd) + + def unmanage(self, apiclient): """Unmanage a VM from CloudStack (currently VMware only)""" cmd = unmanageVirtualMachine.unmanageVirtualMachineCmd() diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java index d26607d82097..b571e16ba1fc 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java @@ -1391,10 +1391,10 @@ public void updateAdapterTypeIfRequired(String vmdkFileName) throws Exception { } public void attachDisk(String[] vmdkDatastorePathChain, ManagedObjectReference morDs) throws Exception { - attachDisk(vmdkDatastorePathChain, morDs, null, null); + attachDisk(vmdkDatastorePathChain, morDs, null, null, -1); } - public void attachDisk(String[] vmdkDatastorePathChain, ManagedObjectReference morDs, String diskController, String vSphereStoragePolicyId) throws Exception { + public void attachDisk(String[] vmdkDatastorePathChain, ManagedObjectReference morDs, String diskController, String vSphereStoragePolicyId, int groupNumber) throws Exception { if(s_logger.isTraceEnabled()) s_logger.trace("vCenter API trace - attachDisk(). target MOR: " + _mor.getValue() + ", vmdkDatastorePath: " @@ -1417,9 +1417,9 @@ public void attachDisk(String[] vmdkDatastorePathChain, ManagedObjectReference m unitNumber = getFreeUnitNumberOnIDEController(controllerKey); } else { if (StringUtils.isNotBlank(diskController)) { - controllerKey = getScsiDiskControllerKey(diskController); + controllerKey = getScsiDiskControllerKey(diskController, groupNumber); } else { - controllerKey = getScsiDeviceControllerKey(); + controllerKey = getScsiDeviceControllerKey(groupNumber); } unitNumber = -1; } @@ -2363,23 +2363,23 @@ public boolean isPvScsiSupported() throws Exception { } // Would be useful if there exists multiple sub types of SCSI controllers per VM are supported in CloudStack f - public int getScsiDiskControllerKey(String diskController) throws Exception { + public int getScsiDiskControllerKey(String diskController, int groupNumber) throws Exception { List devices = (List)_context.getVimClient().getDynamicProperty(_mor, "config.hardware.device"); if (CollectionUtils.isNotEmpty(devices)) { DiskControllerType diskControllerType = DiskControllerType.getType(diskController); for (VirtualDevice device : devices) { if ((diskControllerType == DiskControllerType.lsilogic || diskControllerType == DiskControllerType.scsi) - && device instanceof VirtualLsiLogicController && isValidScsiDiskController((VirtualLsiLogicController)device)) { + && device instanceof VirtualLsiLogicController && isValidScsiDiskController((VirtualLsiLogicController)device, groupNumber)) { return ((VirtualLsiLogicController)device).getKey(); } else if ((diskControllerType == DiskControllerType.lsisas1068 || diskControllerType == DiskControllerType.scsi) - && device instanceof VirtualLsiLogicSASController && isValidScsiDiskController((VirtualLsiLogicSASController)device)) { + && device instanceof VirtualLsiLogicSASController && isValidScsiDiskController((VirtualLsiLogicSASController)device, groupNumber)) { return ((VirtualLsiLogicSASController)device).getKey(); } else if ((diskControllerType == DiskControllerType.pvscsi || diskControllerType == DiskControllerType.scsi) - && device instanceof ParaVirtualSCSIController && isValidScsiDiskController((ParaVirtualSCSIController)device)) { + && device instanceof ParaVirtualSCSIController && isValidScsiDiskController((ParaVirtualSCSIController)device, groupNumber)) { return ((ParaVirtualSCSIController)device).getKey(); } else if ((diskControllerType == DiskControllerType.buslogic || diskControllerType == DiskControllerType.scsi) - && device instanceof VirtualBusLogicController && isValidScsiDiskController((VirtualBusLogicController)device)) { + && device instanceof VirtualBusLogicController && isValidScsiDiskController((VirtualBusLogicController)device, groupNumber)) { return ((VirtualBusLogicController)device).getKey(); } } @@ -2389,7 +2389,7 @@ && device instanceof VirtualBusLogicController && isValidScsiDiskController((Vir throw new IllegalStateException("Scsi disk controller of type " + diskController + " not found among configured devices."); } - public int getScsiDiskControllerKeyNoException(String diskController, int scsiUnitNumber) throws Exception { + public int getScsiDiskControllerKeyNoException(String diskController, int scsiUnitNumber, int groupNumber) throws Exception { List devices = (List)_context.getVimClient().getDynamicProperty(_mor, "config.hardware.device"); if (CollectionUtils.isNotEmpty(devices) && scsiUnitNumber >= 0) { @@ -2398,35 +2398,23 @@ public int getScsiDiskControllerKeyNoException(String diskController, int scsiUn DiskControllerType diskControllerType = DiskControllerType.getType(diskController); for (VirtualDevice device : devices) { if ((diskControllerType == DiskControllerType.lsilogic || diskControllerType == DiskControllerType.scsi) && device instanceof VirtualLsiLogicController) { - if (scsiControllerDeviceCount == requiredScsiController) { - if (isValidScsiDiskController((VirtualLsiLogicController)device)) { - return ((VirtualLsiLogicController)device).getKey(); - } - break; + if ((scsiControllerDeviceCount == requiredScsiController || groupNumber > -1) && isValidScsiDiskController((VirtualLsiLogicController)device, groupNumber)) { + return ((VirtualLsiLogicController)device).getKey(); } scsiControllerDeviceCount++; } else if ((diskControllerType == DiskControllerType.lsisas1068 || diskControllerType == DiskControllerType.scsi) && device instanceof VirtualLsiLogicSASController) { - if (scsiControllerDeviceCount == requiredScsiController) { - if (isValidScsiDiskController((VirtualLsiLogicSASController)device)) { - return ((VirtualLsiLogicSASController)device).getKey(); - } - break; + if ((scsiControllerDeviceCount == requiredScsiController || groupNumber > -1) && isValidScsiDiskController((VirtualLsiLogicSASController)device, groupNumber)) { + return ((VirtualLsiLogicSASController)device).getKey(); } scsiControllerDeviceCount++; } else if ((diskControllerType == DiskControllerType.pvscsi || diskControllerType == DiskControllerType.scsi) && device instanceof ParaVirtualSCSIController) { - if (scsiControllerDeviceCount == requiredScsiController) { - if (isValidScsiDiskController((ParaVirtualSCSIController)device)) { - return ((ParaVirtualSCSIController)device).getKey(); - } - break; + if ((scsiControllerDeviceCount == requiredScsiController || groupNumber > -1) && isValidScsiDiskController((ParaVirtualSCSIController)device, groupNumber)) { + return ((ParaVirtualSCSIController)device).getKey(); } scsiControllerDeviceCount++; } else if ((diskControllerType == DiskControllerType.buslogic || diskControllerType == DiskControllerType.scsi) && device instanceof VirtualBusLogicController) { - if (scsiControllerDeviceCount == requiredScsiController) { - if (isValidScsiDiskController((VirtualBusLogicController)device)) { - return ((VirtualBusLogicController)device).getKey(); - } - break; + if ((scsiControllerDeviceCount == requiredScsiController || groupNumber > -1) && isValidScsiDiskController((VirtualBusLogicController)device, groupNumber)) { + return ((VirtualBusLogicController)device).getKey(); } scsiControllerDeviceCount++; } @@ -2436,18 +2424,18 @@ public int getScsiDiskControllerKeyNoException(String diskController, int scsiUn } public int getNextScsiDiskDeviceNumber() throws Exception { - int scsiControllerKey = getScsiDeviceControllerKey(); + int scsiControllerKey = getScsiDeviceControllerKey(-1); int deviceNumber = getNextDeviceNumber(scsiControllerKey); return deviceNumber; } - public int getScsiDeviceControllerKey() throws Exception { + public int getScsiDeviceControllerKey(int groupNumber) throws Exception { List devices = _context.getVimClient().getDynamicProperty(_mor, "config.hardware.device"); if (devices != null && devices.size() > 0) { for (VirtualDevice device : devices) { - if (device instanceof VirtualSCSIController && isValidScsiDiskController((VirtualSCSIController)device)) { + if (device instanceof VirtualSCSIController && isValidScsiDiskController((VirtualSCSIController)device, groupNumber)) { return device.getKey(); } } @@ -2462,7 +2450,7 @@ public int getScsiDeviceControllerKeyNoException() throws Exception { if (devices != null && devices.size() > 0) { for (VirtualDevice device : devices) { - if (device instanceof VirtualSCSIController && isValidScsiDiskController((VirtualSCSIController)device)) { + if (device instanceof VirtualSCSIController && isValidScsiDiskController((VirtualSCSIController)device, -1)) { return device.getKey(); } } @@ -2559,7 +2547,7 @@ public void ensureScsiDeviceControllers(int count, int availableBusNum) throws E } } - private boolean isValidScsiDiskController(VirtualSCSIController scsiDiskController) { + private boolean isValidScsiDiskController(VirtualSCSIController scsiDiskController, int groupNumber) { if (scsiDiskController == null) { return false; } @@ -2573,7 +2561,7 @@ private boolean isValidScsiDiskController(VirtualSCSIController scsiDiskControll return false; } - return true; + return !(groupNumber > -1 && scsiDiskController.getBusNumber() != groupNumber); } // return pair of VirtualDisk and disk device bus name(ide0:0, etc) @@ -3163,7 +3151,6 @@ public int getNextDeviceNumber(int controllerKey) throws Exception { List existingUnitNumbers = new ArrayList(); int deviceNumber = 0; - int scsiControllerKey = getScsiDeviceControllerKeyNoException(); if (devices != null && devices.size() > 0) { for (VirtualDevice device : devices) { if (device.getControllerKey() != null && device.getControllerKey().intValue() == controllerKey) { @@ -3173,9 +3160,8 @@ public int getNextDeviceNumber(int controllerKey) throws Exception { } while (true) { // Next device number should be the lowest device number on the key that is not in use and is not reserved. - if (!existingUnitNumbers.contains(Integer.valueOf(deviceNumber))) { - if (controllerKey != scsiControllerKey || !VmwareHelper.isReservedScsiDeviceNumber(deviceNumber)) - break; + if (!existingUnitNumbers.contains(Integer.valueOf(deviceNumber)) && !VmwareHelper.isReservedScsiDeviceNumber(deviceNumber)) { + break; } ++deviceNumber; }