From da5c6cd9b0e29fa41198203633b883b559ecaacd Mon Sep 17 00:00:00 2001 From: DK101010 Date: Mon, 21 Feb 2022 08:24:02 +0000 Subject: [PATCH 01/24] add volume group handling add db object, dao's, vo's for VolumeGroups add VolumeGroup during volume attach remove VolumeGroup durning volume detach read and apply VolumeGroups during start vm process --- .../java/com/cloud/agent/api/to/DiskTO.java | 12 ++++- .../apache/cloudstack/api/ApiConstants.java | 1 + .../command/user/volume/AttachVolumeCmd.java | 7 +++ .../java/com/cloud/vm/VmWorkAttachVolume.java | 10 +++- ...spring-engine-schema-core-daos-context.xml | 1 + .../vmware/resource/VmwareResource.java | 41 ++++++++++---- .../resource/VmwareStorageProcessor.java | 6 +-- .../cloud/hypervisor/HypervisorGuruBase.java | 20 +++++-- .../cloud/storage/VolumeApiServiceImpl.java | 38 +++++++------ .../vmware/mo/VirtualMachineMO.java | 54 +++++++++---------- 10 files changed, 128 insertions(+), 62 deletions(-) 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 7b3d10bc4dbe..245706e91d9b 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 @@ -46,9 +46,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) { @@ -56,6 +57,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() { @@ -97,4 +99,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/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index 55002f70b1b2..9a4046d29e4f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -449,6 +449,7 @@ 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 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/user/volume/AttachVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java index 687d683309c6..a7b34b055797 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") + 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/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/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 fcd3be6c92eb..746a15f48d73 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 @@ -251,6 +251,7 @@ + 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..7bc025035cb7 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]; @@ -2293,17 +2294,17 @@ protected StartAnswer execute(StartCommand cmd) { } } } else { - if (VmwareHelper.isReservedScsiDeviceNumber(scsiUnitNumber)) { - scsiUnitNumber++; + if (VmwareHelper.isReservedScsiDeviceNumber(scsiUnitNumberMap.getOrDefault(vol.getGroupNumber(),0))) { + scsiUnitNumberMap.merge(-1, 0, (a,b) -> a + 1); } - controllerKey = vmMo.getScsiDiskControllerKeyNoException(diskController, scsiUnitNumber); + controllerKey = vmMo.getScsiDiskControllerKeyNoException(diskController, scsiUnitNumberMap.getOrDefault(vol.getGroupNumber(), 0), vol.getGroupNumber()); 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(vol.getGroupNumber(), 0), vol.getGroupNumber()); } } if (!hasSnapshot) { @@ -2361,8 +2362,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(vol.getGroupNumber(), 0) % VmwareHelper.MAX_ALLOWED_DEVICES_SCSI_CONTROLLER; + scsiUnitNumberMap.merge(vol.getGroupNumber(), 0, (a,b) -> a + 1); } VirtualDevice device = VmwareHelper.prepareDiskDevice(vmMo, null, controllerKey, diskChain, volumeDsDetails.first(), deviceNumber, i + 1); @@ -2392,7 +2393,7 @@ protected StartAnswer execute(StartCommand cmd) { if (controllerKey == vmMo.getIDEControllerKey(ideUnitNumber)) ideUnitNumber++; else - scsiUnitNumber++; + scsiUnitNumberMap.merge(vol.getGroupNumber(), 0, (a,b) -> a + 1); } } @@ -3661,7 +3662,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 +3670,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 fd541cca80a4..68293bc75741 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..fdf98247aace 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 9ce294d2332f..833c28ff5447 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -154,6 +154,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; @@ -240,6 +241,8 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic @Inject private VolumeDetailsDao _volsDetailsDao; @Inject + private VolumeGroupDao _volsGroupDao; + @Inject private HostDao _hostDao; @Inject private SnapshotDao _snapshotDao; @@ -958,7 +961,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()); @@ -1690,6 +1693,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); @@ -2065,10 +2069,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()) { @@ -2169,11 +2173,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); @@ -2217,14 +2221,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 { @@ -2250,13 +2254,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()); } @@ -2730,6 +2734,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"); @@ -3843,7 +3848,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; @@ -3897,6 +3902,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()); @@ -3952,6 +3958,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()); @@ -3976,6 +3983,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()); @@ -4182,7 +4190,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(); @@ -4203,7 +4211,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()); @@ -4382,7 +4390,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/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 a25480f4241e..4de3fbeffafb 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 @@ -1396,10 +1396,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: " @@ -1422,9 +1422,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; } @@ -2376,23 +2376,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(); } } @@ -2402,7 +2402,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) { @@ -2411,35 +2411,31 @@ 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)) { + if (scsiControllerDeviceCount == requiredScsiController || groupNumber > -1) { + if (isValidScsiDiskController((VirtualLsiLogicController)device,groupNumber)) { return ((VirtualLsiLogicController)device).getKey(); } - break; } scsiControllerDeviceCount++; } else if ((diskControllerType == DiskControllerType.lsisas1068 || diskControllerType == DiskControllerType.scsi) && device instanceof VirtualLsiLogicSASController) { - if (scsiControllerDeviceCount == requiredScsiController) { - if (isValidScsiDiskController((VirtualLsiLogicSASController)device)) { + if (scsiControllerDeviceCount == requiredScsiController || groupNumber > -1) { + if (isValidScsiDiskController((VirtualLsiLogicSASController)device,groupNumber)) { return ((VirtualLsiLogicSASController)device).getKey(); } - break; } scsiControllerDeviceCount++; } else if ((diskControllerType == DiskControllerType.pvscsi || diskControllerType == DiskControllerType.scsi) && device instanceof ParaVirtualSCSIController) { - if (scsiControllerDeviceCount == requiredScsiController) { - if (isValidScsiDiskController((ParaVirtualSCSIController)device)) { + if (scsiControllerDeviceCount == requiredScsiController || groupNumber > -1) { + if (isValidScsiDiskController((ParaVirtualSCSIController)device,groupNumber)) { return ((ParaVirtualSCSIController)device).getKey(); } - break; } scsiControllerDeviceCount++; } else if ((diskControllerType == DiskControllerType.buslogic || diskControllerType == DiskControllerType.scsi) && device instanceof VirtualBusLogicController) { - if (scsiControllerDeviceCount == requiredScsiController) { - if (isValidScsiDiskController((VirtualBusLogicController)device)) { + if (scsiControllerDeviceCount == requiredScsiController || groupNumber > -1) { + if (isValidScsiDiskController((VirtualBusLogicController)device,groupNumber)) { return ((VirtualBusLogicController)device).getKey(); } - break; } scsiControllerDeviceCount++; } @@ -2449,18 +2445,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(); } } @@ -2475,7 +2471,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(); } } @@ -2572,7 +2568,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; } @@ -2586,6 +2582,10 @@ private boolean isValidScsiDiskController(VirtualSCSIController scsiDiskControll return false; } + if(groupNumber > -1 && scsiDiskController.getBusNumber() != groupNumber){ + return false; + } + return true; } From 7ab053049e184bc6a02a56f89fcd98a54c09d87b Mon Sep 17 00:00:00 2001 From: DK101010 Date: Mon, 21 Feb 2022 11:02:48 +0000 Subject: [PATCH 02/24] add VolumeGroup --- .../java/com/cloud/storage/VolumeGroupVO.java | 83 +++++++++++++++++++ .../com/cloud/storage/dao/VolumeGroupDao.java | 26 ++++++ .../cloud/storage/dao/VolumeGroupDaoImpl.java | 63 ++++++++++++++ 3 files changed, 172 insertions(+) create mode 100644 engine/schema/src/main/java/com/cloud/storage/VolumeGroupVO.java create mode 100644 engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDao.java create mode 100644 engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDaoImpl.java 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..3833e3cb6d1f --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/storage/VolumeGroupVO.java @@ -0,0 +1,83 @@ +/* + * Copyright 2022 The Apache Software Foundation. + * + * Licensed 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..a0d08341e601 --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDao.java @@ -0,0 +1,26 @@ +/* + * Copyright 2022 The Apache Software Foundation. + * + * Licensed 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..d0943d28f263 --- /dev/null +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDaoImpl.java @@ -0,0 +1,63 @@ +/* + * Copyright 2022 The Apache Software Foundation. + * + * Licensed 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; + + public VolumeGroupDaoImpl(){ + allFieldsSearch = createSearchBuilder(); + allFieldsSearch.and("vmId", allFieldsSearch.entity().getVmId(), SearchCriteria.Op.EQ); + allFieldsSearch.and("volumeId", allFieldsSearch.entity().getVolumeId(), SearchCriteria.Op.EQ); + allFieldsSearch.and("groupNumber", 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 = 1; + } + 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("volumeId", volumeId); + expunge(sc); + } + + @Override + public VolumeGroupVO findByVmAndVolume(long vmId, long volumeId){ + SearchCriteria sc = allFieldsSearch.create(); + sc.setParameters("vmId", vmId); + sc.setParameters("volumeId", volumeId); + return findOneBy(sc); + } +} \ No newline at end of file From 8c8df111aa8d47d222a73aaab841c6b89e83d2a3 Mon Sep 17 00:00:00 2001 From: DK101010 Date: Mon, 21 Feb 2022 11:04:10 +0000 Subject: [PATCH 03/24] add since parameter to AttachVolumeCmd --- .../cloudstack/api/command/user/volume/AttachVolumeCmd.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a7b34b055797..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,7 +62,7 @@ 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") + @Parameter(name = ApiConstants.VOLUME_GROUP, type = CommandType.INTEGER, required = false, description = "group of volume", since = "4.17.0") private int volumeGroup=-1; ///////////////////////////////////////////////////// From 9fa59ecbe5c39568b45c9f31f0558ed7c47bcf28 Mon Sep 17 00:00:00 2001 From: DK101010 Date: Thu, 3 Mar 2022 11:27:24 +0000 Subject: [PATCH 04/24] adapt marvin for volumegroups --- tools/marvin/marvin/lib/base.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index c6a595810b01..c9f860764bf6 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -883,7 +883,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 @@ -892,6 +892,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): From 7541179e50cdd8934644fe735a6a61d3f2e4fcfb Mon Sep 17 00:00:00 2001 From: DK101010 Date: Thu, 3 Mar 2022 11:28:19 +0000 Subject: [PATCH 05/24] adapt/add test volumegroup handling --- .../storage/VolumeApiServiceImplTest.java | 16 +- .../component/test_volume_groups.py | 174 ++++++++++++++++++ 2 files changed, 182 insertions(+), 8 deletions(-) create mode 100644 test/integration/component/test_volume_groups.py diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index 5b9875bc61e0..00faaf3fa1a3 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -376,44 +376,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); } // volume not Ready @@ -523,7 +523,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/test/integration/component/test_volume_groups.py b/test/integration/component/test_volume_groups.py new file mode 100644 index 000000000000..29fdd084cdbb --- /dev/null +++ b/test/integration/component/test_volume_groups.py @@ -0,0 +1,174 @@ +# 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.cloudstackAPI import (listHypervisorCapabilities, + attachIso, + deleteVolume + ) +from marvin.lib.utils import cleanup_resources, validateList +from marvin.lib.base import (Account, + ServiceOffering, + VirtualMachine, + Volume, + Host, + Iso, + Configurations, + DiskOffering, + Domain, + StoragePool) +from marvin.lib.common import (get_domain, + get_zone, + get_template, + get_pod, + find_storage_pool_type, + update_resource_limit) +import re +import json +from types import SimpleNamespace +from marvin.codes import PASS +# Import System modules +import time + + +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.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) + + cls.virtual_machine = VirtualMachine.create( + cls.api_client, + cls.test_data["virtual_machine"], + accountid=cls.account.name, + domainid=cls.account.domainid, + serviceofferingid=cls.service_offering.id, + templateid=cls.template.id, + zoneid=cls.zone.id + ) + cls._cleanup.append(cls.virtual_machine) + + @classmethod + def tearDownClass(cls): + super(TestAttachVolumeWithGroup, cls).tearDownClass() + + def setUp(self): + self.api_client = self.test_client.getApiClient() + self.cleanup = [] + + def tearDown(self): + super(TestAttachVolumeWithGroup, self).tearDown() + + @attr(tags=["advanced", "advancedns", "needle"]) + def test_attach_mixed_volumes(self): + volume_list = [] + volume_count = 9 + 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]) + + for i in range(0, volume_count): + if i > 5: + self.virtual_machine.attach_volume( + self.api_client, + volume_list[i][0], + volumegroup=2 + ) + volume_list[i][1] = 2 + volume_list[i][2] = i % 3 + continue + if i > 2: + self.virtual_machine.attach_volume( + self.api_client, + volume_list[i][0], + volumegroup=1 + ) + volume_list[i][1] = 1 + volume_list[i][2] = i % 3 + continue + + self.virtual_machine.attach_volume( + self.api_client, + volume_list[i][0] + ) + volume_list[i][1] = 0 + volume_list[i][2] = (i % 3) + 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)) + + for volume in volume_list: + self.virtual_machine.detach_volume( + self.api_client, + volume[0] + ) + self.cleanup.append(volume) + + @classmethod + def tearDownClass(cls): + super(TestAttachVolumeWithGroup, cls).tearDownClass() \ No newline at end of file From 77232e9ef9d7c38d5b61052254c1e436a7f76df0 Mon Sep 17 00:00:00 2001 From: DK101010 Date: Mon, 21 Mar 2022 07:46:05 +0000 Subject: [PATCH 06/24] add condition for volumes in group null in case of device count during start of an vm --- .../cloud/hypervisor/vmware/resource/VmwareResource.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 7bc025035cb7..c63370f9d2a2 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 @@ -2362,8 +2362,10 @@ protected StartAnswer execute(StartCommand cmd) { deviceNumber = ideUnitNumber % VmwareHelper.MAX_ALLOWED_DEVICES_IDE_CONTROLLER; ideUnitNumber++; } else { - deviceNumber = scsiUnitNumberMap.getOrDefault(vol.getGroupNumber(), 0) % VmwareHelper.MAX_ALLOWED_DEVICES_SCSI_CONTROLLER; - scsiUnitNumberMap.merge(vol.getGroupNumber(), 0, (a,b) -> a + 1); + // 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(); + deviceNumber = scsiUnitNumberMap.getOrDefault(groupNumber, 0) % VmwareHelper.MAX_ALLOWED_DEVICES_SCSI_CONTROLLER; + scsiUnitNumberMap.merge(groupNumber, 0, (a,b) -> a + 1); } VirtualDevice device = VmwareHelper.prepareDiskDevice(vmMo, null, controllerKey, diskChain, volumeDsDetails.first(), deviceNumber, i + 1); From ee39c930a492bf34654012300c1d9ef0578b4c0e Mon Sep 17 00:00:00 2001 From: DK101010 Date: Mon, 21 Mar 2022 08:09:37 +0000 Subject: [PATCH 07/24] add groups for import/unmanage vm's --- .../java/com/cloud/vm/VmDetailConstants.java | 1 + .../admin/vm/ImportUnmanagedInstanceCmd.java | 12 ++++++++++++ .../service/VolumeOrchestrationService.java | 3 ++- .../orchestration/VolumeOrchestrator.java | 12 +++++++++++- .../cloud/storage/dao/VolumeGroupDaoImpl.java | 2 +- .../cloudstack/vm/UnmanagedVMsManagerImpl.java | 17 +++++++++++------ 6 files changed, 38 insertions(+), 9 deletions(-) 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/command/admin/vm/ImportUnmanagedInstanceCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java index dca683f787f0..907621e174ed 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 @@ -259,6 +259,18 @@ 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); + 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(); 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/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 bf21e62c6e59..669acf80782d 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 @@ -124,6 +124,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.storage.snapshot.SnapshotManager; import com.cloud.template.TemplateManager; import com.cloud.template.VirtualMachineTemplate; @@ -232,6 +233,8 @@ public enum UserVmCloneType { private SecondaryStorageVmDao secondaryStorageVmDao; @Inject VolumeApiService _volumeApiService; + @Inject + private VolumeGroupDao _volumeGroupDao; @Inject protected SnapshotHelper snapshotHelper; @@ -2022,7 +2025,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 { @@ -2069,6 +2072,12 @@ 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){ + int volGroup = volumeGroup; + _volumeGroupDao.addVolumeToGroup(vm.getId(), vol.getId(), vol.getDeviceId(), volGroup); + } + return toDiskProfile(vol, offering); } @@ -2091,6 +2100,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/dao/VolumeGroupDaoImpl.java b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDaoImpl.java index d0943d28f263..24434d4c09fa 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDaoImpl.java @@ -36,7 +36,7 @@ public VolumeGroupDaoImpl(){ public void addVolumeToGroup(long vmId, long volumeId, long deviceId, int groupNumber) { if (groupNumber != -1) { if (deviceId == 0L) { - groupNumber = 1; + groupNumber = 0; } VolumeGroupVO group = new VolumeGroupVO(); group.setVmId(vmId); 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..a205d2e722d6 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) { final DataCenter zone = dataCenterDao.findById(vm.getDataCenterId()); final String path = StringUtils.isEmpty(disk.getFileBaseName()) ? disk.getImagePath() : disk.getFileBaseName(); String chainInfo = disk.getChainInfo(); @@ -693,7 +693,7 @@ private Pair importDisk(UnmanagedInstanceTO.Disk disk, } StoragePool storagePool = getStoragePool(disk, zone, cluster); 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,7 +908,7 @@ 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) { UserVm userVm = null; @@ -998,16 +998,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)); 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)); deviceId++; } } catch (Exception e) { @@ -1180,6 +1184,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,7 +1238,7 @@ 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); break; From 2ff2fac6a449a64c0a0ceb53a5144f7933f7ff04 Mon Sep 17 00:00:00 2001 From: DK101010 Date: Mon, 21 Mar 2022 08:10:30 +0000 Subject: [PATCH 08/24] adapt java test case --- .../org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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..c47e6393c643 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(), null)).thenReturn(Mockito.mock(DiskProfile.class)); when(volumeDao.findByInstance(Mockito.anyLong())).thenReturn(volumes); List userVmResponses = new ArrayList<>(); UserVmResponse userVmResponse = new UserVmResponse(); From d55e26cd6343e19edef394383320078d049e8f2e Mon Sep 17 00:00:00 2001 From: DK101010 Date: Mon, 21 Mar 2022 08:11:39 +0000 Subject: [PATCH 09/24] adapt marvin to import vms --- tools/marvin/marvin/lib/base.py | 50 +++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index c9f860764bf6..1b4d9480f2e8 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -1027,6 +1027,56 @@ 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 + ): + cmd = importUnmanagedInstance.importUnmanagedInstanceCmd() + cmd.clusterid = clusterid + cmd.name = name + cmd.serviceofferingid = serviceofferingid + + 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() From f98e08d54fb6b7187b4310e0efd008bbe40ab03c Mon Sep 17 00:00:00 2001 From: DK101010 Date: Mon, 21 Mar 2022 08:13:07 +0000 Subject: [PATCH 10/24] add test group handling for import/unmanage vm's --- .../component/test_volume_groups.py | 134 ++++++++++++++---- 1 file changed, 105 insertions(+), 29 deletions(-) diff --git a/test/integration/component/test_volume_groups.py b/test/integration/component/test_volume_groups.py index 29fdd084cdbb..3ae3c920711e 100644 --- a/test/integration/component/test_volume_groups.py +++ b/test/integration/component/test_volume_groups.py @@ -19,33 +19,19 @@ # Import Local Modules from nose.plugins.attrib import attr from marvin.cloudstackTestCase import cloudstackTestCase -from marvin.cloudstackAPI import (listHypervisorCapabilities, - attachIso, - deleteVolume - ) -from marvin.lib.utils import cleanup_resources, validateList from marvin.lib.base import (Account, ServiceOffering, VirtualMachine, Volume, - Host, - Iso, - Configurations, - DiskOffering, - Domain, - StoragePool) + Cluster, + DiskOffering) from marvin.lib.common import (get_domain, get_zone, get_template, - get_pod, - find_storage_pool_type, - update_resource_limit) + get_pod) + import re -import json -from types import SimpleNamespace -from marvin.codes import PASS -# Import System modules -import time + class TestAttachVolumeWithGroup(cloudstackTestCase): @@ -58,6 +44,7 @@ def setUpClass(cls): 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, @@ -84,16 +71,7 @@ def setUpClass(cls): ) cls._cleanup.append(cls.service_offering) - cls.virtual_machine = VirtualMachine.create( - cls.api_client, - cls.test_data["virtual_machine"], - accountid=cls.account.name, - domainid=cls.account.domainid, - serviceofferingid=cls.service_offering.id, - templateid=cls.template.id, - zoneid=cls.zone.id - ) - cls._cleanup.append(cls.virtual_machine) + @classmethod def tearDownClass(cls): @@ -101,6 +79,7 @@ def tearDownClass(cls): def setUp(self): self.api_client = self.test_client.getApiClient() + self.dbclient = self.testClient.getDbConnection() self.cleanup = [] def tearDown(self): @@ -108,6 +87,16 @@ def tearDown(self): @attr(tags=["advanced", "advancedns", "needle"]) def test_attach_mixed_volumes(self): + 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(self.virtual_machine) volume_list = [] volume_count = 9 for i in range(0, volume_count): @@ -169,6 +158,93 @@ def test_attach_mixed_volumes(self): ) self.cleanup.append(volume) + + @attr(tags=["advanced", "advancedns", "needle"]) + def test_unmanage_ingest_vm(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 + ) + 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): + data_disk_offering_list.append({ + 'disk': volume.id, + 'diskOffering': self.disk_offering.id, + 'volumeGroup': volume.controllerunit + }) + 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']} + 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.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 + + @classmethod def tearDownClass(cls): super(TestAttachVolumeWithGroup, cls).tearDownClass() \ No newline at end of file From fe51bd571f736d06ef23e14d7ffc72aa4e513185 Mon Sep 17 00:00:00 2001 From: DK101010 Date: Tue, 5 Apr 2022 13:17:25 +0100 Subject: [PATCH 11/24] adapt code for special controller unit 7 --- .../vmware/resource/VmwareResource.java | 16 ++++++++-------- .../hypervisor/vmware/mo/VirtualMachineMO.java | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) 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 c63370f9d2a2..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 @@ -2264,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); @@ -2294,17 +2296,17 @@ protected StartAnswer execute(StartCommand cmd) { } } } else { - if (VmwareHelper.isReservedScsiDeviceNumber(scsiUnitNumberMap.getOrDefault(vol.getGroupNumber(),0))) { - scsiUnitNumberMap.merge(-1, 0, (a,b) -> a + 1); + if (VmwareHelper.isReservedScsiDeviceNumber(scsiUnitNumberMap.getOrDefault(groupNumber,0))) { + scsiUnitNumberMap.merge(groupNumber, 1, (a,b) -> a + b); } - controllerKey = vmMo.getScsiDiskControllerKeyNoException(diskController, scsiUnitNumberMap.getOrDefault(vol.getGroupNumber(), 0), vol.getGroupNumber()); + 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(), scsiUnitNumberMap.getOrDefault(vol.getGroupNumber(), 0), vol.getGroupNumber()); + controllerKey = vmMo.getScsiDiskControllerKeyNoException(existingControllerType.toString(), scsiUnitNumberMap.getOrDefault(groupNumber, 0), groupNumber); } } if (!hasSnapshot) { @@ -2362,10 +2364,8 @@ protected StartAnswer execute(StartCommand cmd) { deviceNumber = ideUnitNumber % VmwareHelper.MAX_ALLOWED_DEVICES_IDE_CONTROLLER; ideUnitNumber++; } else { - // 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(); deviceNumber = scsiUnitNumberMap.getOrDefault(groupNumber, 0) % VmwareHelper.MAX_ALLOWED_DEVICES_SCSI_CONTROLLER; - scsiUnitNumberMap.merge(groupNumber, 0, (a,b) -> a + 1); + scsiUnitNumberMap.merge(groupNumber, 1, (a,b) -> a + b); } VirtualDevice device = VmwareHelper.prepareDiskDevice(vmMo, null, controllerKey, diskChain, volumeDsDetails.first(), deviceNumber, i + 1); @@ -2395,7 +2395,7 @@ protected StartAnswer execute(StartCommand cmd) { if (controllerKey == vmMo.getIDEControllerKey(ideUnitNumber)) ideUnitNumber++; else - scsiUnitNumberMap.merge(vol.getGroupNumber(), 0, (a,b) -> a + 1); + scsiUnitNumberMap.merge(groupNumber, 1, (a,b) -> a + b); } } 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 4de3fbeffafb..225e3208427c 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 @@ -3185,7 +3185,7 @@ 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)) + if (!VmwareHelper.isReservedScsiDeviceNumber(deviceNumber)) break; } ++deviceNumber; From 40d0d1e4fd05aabce34b04d053c8bb0b37d7cdcc Mon Sep 17 00:00:00 2001 From: DK101010 Date: Tue, 5 Apr 2022 13:21:38 +0100 Subject: [PATCH 12/24] add test for special controller unit 7(attach volume and start vm) --- .../component/test_volume_groups.py | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/integration/component/test_volume_groups.py b/test/integration/component/test_volume_groups.py index 3ae3c920711e..6185fea59ad0 100644 --- a/test/integration/component/test_volume_groups.py +++ b/test/integration/component/test_volume_groups.py @@ -244,6 +244,46 @@ def test_unmanage_ingest_vm(self): 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) + + for j in range(0, volume_count): + virtual_machine.detach_volume( + self.api_client, + volume_list[j]) + @classmethod def tearDownClass(cls): From 29172ace2a00cfe5e806957356678c458537bbda Mon Sep 17 00:00:00 2001 From: DK101010 Date: Tue, 12 Apr 2022 09:43:19 +0100 Subject: [PATCH 13/24] Add useConntrollerConfiguration to allow user to use the current controller conf of an unmanaged vm --- .../org/apache/cloudstack/api/ApiConstants.java | 1 + .../admin/vm/ImportUnmanagedInstanceCmd.java | 15 +++++++++++++-- .../cloudstack/vm/UnmanagedVMsManagerImpl.java | 15 ++++++++++----- 3 files changed, 24 insertions(+), 7 deletions(-) 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 9a4046d29e4f..1002539ea8a7 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -450,6 +450,7 @@ public class ApiConstants { 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 907621e174ed..7b4ff48c4b48 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,11 @@ 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") + private Boolean useControllerConfiguration; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -264,8 +269,10 @@ public Map getVolumeGroups() { if (MapUtils.isNotEmpty(dataDiskToDiskOfferingList)) { for (Map entry : (Collection>)dataDiskToDiskOfferingList.values()) { String disk = entry.get(VmDetailConstants.DISK); - Integer volumeGroup = Integer.parseInt(entry.get(VmDetailConstants.VOLUME_GROUP)); - dataDiskVolumeGroups.put(disk, volumeGroup); + if(entry.get(VmDetailConstants.VOLUME_GROUP) != null){ + Integer volumeGroup = Integer.parseInt(entry.get(VmDetailConstants.VOLUME_GROUP)); + dataDiskVolumeGroups.put(disk, volumeGroup); + } } } return dataDiskVolumeGroups; @@ -328,4 +335,8 @@ public long getEntityOwnerId() { } return accountId; } + + public Boolean isUseControllerConfiguration() { + return BooleanUtils.isTrue(this.useControllerConfiguration); + } } 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 a205d2e722d6..af19845d9610 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, Integer volumeGroup) { + 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,6 +692,11 @@ private Pair importDisk(UnmanagedInstanceTO.Disk disk, chainInfo = gson.toJson(diskInfo); } StoragePool storagePool = getStoragePool(disk, zone, cluster); + + if ( useControllerConfiguration ){ + volumeGroup = disk.getControllerUnit(); + } + DiskProfile profile = volumeManager.importVolume(type, name, diskOffering, diskSize, minIops, maxIops, vm, template, owner, deviceId, storagePool.getId(), path, chainInfo, volumeGroup); @@ -910,7 +915,7 @@ private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedI 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 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,7 +1003,7 @@ 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, null)); + template, owner, null, null, false)); long deviceId = 1L; for (UnmanagedInstanceTO.Disk disk : dataDisks) { Integer volumeGroup = -1; @@ -1011,7 +1016,7 @@ private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedI 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, volumeGroup)); + template, owner, deviceId, volumeGroup, useControllerConfiguration)); deviceId++; } } catch (Exception e) { @@ -1240,7 +1245,7 @@ public UserVmResponse importUnmanagedInstance(ImportUnmanagedInstanceCmd cmd) { template, displayName, hostName, caller, owner, userId, serviceOffering, dataDiskOfferingMap, diskVolumeGroupMap, nicNetworkMap, nicIpAddressMap, - details, cmd.getMigrateAllowed(), forced); + details, cmd.getMigrateAllowed(), forced, cmd.isUseControllerConfiguration()); break; } } From 3d7382326fcc05c8428a2c1dcbb1c9b2185a8e5b Mon Sep 17 00:00:00 2001 From: DK101010 Date: Tue, 12 Apr 2022 09:44:26 +0100 Subject: [PATCH 14/24] extend test call importUnmanagedVM --- tools/marvin/marvin/lib/base.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index 1b4d9480f2e8..2ba544e42c4b 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -1031,12 +1031,13 @@ def scale(self, apiclient, serviceOfferingId, 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 - ): + 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 From adb7462a62318e52213897e91122a3685c054466 Mon Sep 17 00:00:00 2001 From: DK101010 Date: Tue, 12 Apr 2022 09:55:41 +0100 Subject: [PATCH 15/24] Adapt test for useControllerConfiguration --- .../component/test_volume_groups.py | 113 ++++++++++-------- 1 file changed, 61 insertions(+), 52 deletions(-) diff --git a/test/integration/component/test_volume_groups.py b/test/integration/component/test_volume_groups.py index 6185fea59ad0..3a2b5c2595b1 100644 --- a/test/integration/component/test_volume_groups.py +++ b/test/integration/component/test_volume_groups.py @@ -87,7 +87,7 @@ def tearDown(self): @attr(tags=["advanced", "advancedns", "needle"]) def test_attach_mixed_volumes(self): - self.virtual_machine = VirtualMachine.create( + virtual_machine = VirtualMachine.create( self.api_client, self.test_data["virtual_machine"], accountid=self.account.name, @@ -96,9 +96,9 @@ def test_attach_mixed_volumes(self): templateid=self.template.id, zoneid=self.zone.id ) - self.cleanup.append(self.virtual_machine) + self.cleanup.append(virtual_machine) volume_list = [] - volume_count = 9 + volume_count = 6 for i in range(0, volume_count): volume_list.append([Volume.create( self.api_client, @@ -108,33 +108,34 @@ def test_attach_mixed_volumes(self): 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 > 5: - self.virtual_machine.attach_volume( + 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 % 3 + volume_list[i][2] = i % 2 continue - if i > 2: - self.virtual_machine.attach_volume( + 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 % 3 + volume_list[i][2] = i % 2 continue - self.virtual_machine.attach_volume( + virtual_machine.attach_volume( self.api_client, volume_list[i][0] ) volume_list[i][1] = 0 - volume_list[i][2] = (i % 3) + 1 + volume_list[i][2] = (i % 2) + 1 for i in range(0, volume_count): vol_res = Volume.list( @@ -151,16 +152,15 @@ def test_attach_mixed_volumes(self): else: self.assertTrue(False, "Volume not found with id: {}".format(volume_list[i][0].id)) - for volume in volume_list: - self.virtual_machine.detach_volume( - self.api_client, - volume[0] - ) - self.cleanup.append(volume) - + @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(self): + 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'], @@ -193,37 +193,51 @@ def test_unmanage_ingest_vm(self): 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) - + 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) + 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): - data_disk_offering_list.append({ - 'disk': volume.id, - 'diskOffering': self.disk_offering.id, - 'volumeGroup': volume.controllerunit - }) + 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']} - 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) + 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) @@ -236,6 +250,7 @@ def test_unmanage_ingest_vm(self): 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.") @@ -261,30 +276,24 @@ def test_controller_pos_seven_with_groups(self): 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.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) + self.api_client, + volume_list[j], + volumegroup=i) - virtual_machine.stop(self.api_client) - virtual_machine.start(self.api_client) + virtual_machine.stop(self.api_client) + virtual_machine.start(self.api_client) - for j in range(0, volume_count): virtual_machine.detach_volume( self.api_client, volume_list[j]) - - - @classmethod - def tearDownClass(cls): - super(TestAttachVolumeWithGroup, cls).tearDownClass() \ No newline at end of file From 541f4acf74c7e3b145dcfbaf71a173ba15251687 Mon Sep 17 00:00:00 2001 From: DK101010 <57522802+DK101010@users.noreply.github.com> Date: Wed, 13 Apr 2022 10:20:21 +0200 Subject: [PATCH 16/24] Update api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java Co-authored-by: dahn --- .../api/command/admin/vm/ImportUnmanagedInstanceCmd.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 7b4ff48c4b48..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 @@ -160,7 +160,8 @@ public class ImportUnmanagedInstanceCmd extends BaseAsyncCmd { @Parameter(name = ApiConstants.USE_CONTROLLER_CONFIGURATION, type = CommandType.BOOLEAN, - description = "volumes automatically get volume groups based on their current controller connections") + description = "volumes automatically get volume groups based on their current controller connections", + since="4.17.0") private Boolean useControllerConfiguration; ///////////////////////////////////////////////////// From 5d9e10f672160ab3be76b9ffeb0ea6ab3d924197 Mon Sep 17 00:00:00 2001 From: DK101010 Date: Thu, 14 Apr 2022 09:19:46 +0100 Subject: [PATCH 17/24] Update vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java Update server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java Co-authored-by: dahn --- .../java/com/cloud/storage/VolumeApiServiceImpl.java | 4 ++-- .../cloud/hypervisor/vmware/mo/VirtualMachineMO.java | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 833c28ff5447..35b0d55939d5 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -3958,7 +3958,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); + _volsGroupDao.addVolumeToGroup(vm.getId(), volumeToAttach.getId(), deviceId,groupNumber); volumeToAttach = _volsDao.findById(volumeToAttach.getId()); @@ -3983,7 +3983,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); + _volsGroupDao.addVolumeToGroup(vm.getId(), volumeToAttach.getId(), deviceId,groupNumber); volumeToAttach = _volsDao.findById(volumeToAttach.getId()); 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 225e3208427c..a08a3670cede 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 @@ -2419,21 +2419,21 @@ public int getScsiDiskControllerKeyNoException(String diskController, int scsiUn scsiControllerDeviceCount++; } else if ((diskControllerType == DiskControllerType.lsisas1068 || diskControllerType == DiskControllerType.scsi) && device instanceof VirtualLsiLogicSASController) { if (scsiControllerDeviceCount == requiredScsiController || groupNumber > -1) { - if (isValidScsiDiskController((VirtualLsiLogicSASController)device,groupNumber)) { + if (isValidScsiDiskController((VirtualLsiLogicSASController)device, groupNumber)) { return ((VirtualLsiLogicSASController)device).getKey(); } } scsiControllerDeviceCount++; } else if ((diskControllerType == DiskControllerType.pvscsi || diskControllerType == DiskControllerType.scsi) && device instanceof ParaVirtualSCSIController) { if (scsiControllerDeviceCount == requiredScsiController || groupNumber > -1) { - if (isValidScsiDiskController((ParaVirtualSCSIController)device,groupNumber)) { + if (isValidScsiDiskController((ParaVirtualSCSIController)device, groupNumber)) { return ((ParaVirtualSCSIController)device).getKey(); } } scsiControllerDeviceCount++; } else if ((diskControllerType == DiskControllerType.buslogic || diskControllerType == DiskControllerType.scsi) && device instanceof VirtualBusLogicController) { if (scsiControllerDeviceCount == requiredScsiController || groupNumber > -1) { - if (isValidScsiDiskController((VirtualBusLogicController)device,groupNumber)) { + if (isValidScsiDiskController((VirtualBusLogicController)device, groupNumber)) { return ((VirtualBusLogicController)device).getKey(); } } @@ -2456,7 +2456,7 @@ public int getScsiDeviceControllerKey(int groupNumber) throws Exception { if (devices != null && devices.size() > 0) { for (VirtualDevice device : devices) { - if (device instanceof VirtualSCSIController && isValidScsiDiskController((VirtualSCSIController)device,groupNumber)) { + if (device instanceof VirtualSCSIController && isValidScsiDiskController((VirtualSCSIController)device, groupNumber)) { return device.getKey(); } } @@ -2471,7 +2471,7 @@ public int getScsiDeviceControllerKeyNoException() throws Exception { if (devices != null && devices.size() > 0) { for (VirtualDevice device : devices) { - if (device instanceof VirtualSCSIController && isValidScsiDiskController((VirtualSCSIController)device,-1)) { + if (device instanceof VirtualSCSIController && isValidScsiDiskController((VirtualSCSIController)device, -1)) { return device.getKey(); } } From c7f16058c221937a0765beb10eab2d922228f4d3 Mon Sep 17 00:00:00 2001 From: DK101010 Date: Mon, 25 Apr 2022 09:17:27 +0100 Subject: [PATCH 18/24] change license header to ASF --- .../java/com/cloud/storage/VolumeGroupVO.java | 25 +++++++++++-------- .../com/cloud/storage/dao/VolumeGroupDao.java | 25 +++++++++++-------- .../cloud/storage/dao/VolumeGroupDaoImpl.java | 25 +++++++++++-------- 3 files changed, 42 insertions(+), 33 deletions(-) diff --git a/engine/schema/src/main/java/com/cloud/storage/VolumeGroupVO.java b/engine/schema/src/main/java/com/cloud/storage/VolumeGroupVO.java index 3833e3cb6d1f..3153db001312 100644 --- a/engine/schema/src/main/java/com/cloud/storage/VolumeGroupVO.java +++ b/engine/schema/src/main/java/com/cloud/storage/VolumeGroupVO.java @@ -1,17 +1,20 @@ /* - * Copyright 2022 The Apache Software Foundation. + * 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 * - * Licensed 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 * - * 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. + * 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; 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 index a0d08341e601..d483013ffab3 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDao.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDao.java @@ -1,17 +1,20 @@ /* - * Copyright 2022 The Apache Software Foundation. + * 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 * - * Licensed 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 * - * 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. + * 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; 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 index 24434d4c09fa..a0796b8a7556 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDaoImpl.java @@ -1,17 +1,20 @@ /* - * Copyright 2022 The Apache Software Foundation. + * 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 * - * Licensed 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 * - * 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. + * 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; From 677e34bedfa865b2f00c247dcad945e9f398d760 Mon Sep 17 00:00:00 2001 From: DK101010 Date: Fri, 14 Oct 2022 09:34:05 +0100 Subject: [PATCH 19/24] adapt test --- .../test/java/com/cloud/storage/VolumeApiServiceImplTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index 995c6c8769c2..748f645dda50 100644 --- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java @@ -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 From 3a84ea167d4a4fa0e0f0bd3d0da4f07cd5d60afc Mon Sep 17 00:00:00 2001 From: DK101010 Date: Fri, 14 Oct 2022 10:28:17 +0100 Subject: [PATCH 20/24] adapt test --- .../org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c47e6393c643..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(), null)).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(); From 7f0de61e7e839019d7bd220bd68cf901dc6560e1 Mon Sep 17 00:00:00 2001 From: DK101010 Date: Mon, 17 Oct 2022 14:02:40 +0100 Subject: [PATCH 21/24] refactoring --- .../orchestration/VolumeOrchestrator.java | 6 ++-- .../cloud/storage/dao/VolumeGroupDaoImpl.java | 15 ++++---- .../cloud/hypervisor/HypervisorGuruBase.java | 4 +-- .../cloud/storage/VolumeApiServiceImpl.java | 10 +++--- .../vmware/mo/VirtualMachineMO.java | 36 ++++++------------- 5 files changed, 30 insertions(+), 41 deletions(-) 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 693be4eea92e..28140525fbd3 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 @@ -235,7 +235,7 @@ public enum UserVmCloneType { @Inject VolumeApiService _volumeApiService; @Inject - private VolumeGroupDao _volumeGroupDao; + private VolumeGroupDao volumeGroupDao; PassphraseDao passphraseDao; @Inject @@ -2125,7 +2125,7 @@ public DiskProfile importVolume(Type type, String name, DiskOffering offering, L if(volumeGroup != null){ int volGroup = volumeGroup; - _volumeGroupDao.addVolumeToGroup(vm.getId(), vol.getId(), vol.getDeviceId(), volGroup); + volumeGroupDao.addVolumeToGroup(vm.getId(), vol.getId(), vol.getDeviceId(), volGroup); } return toDiskProfile(vol, offering); @@ -2150,7 +2150,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()); + volumeGroupDao.deleteVolumeFromGroup(vol.getId()); } } } 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 index a0796b8a7556..9fff2778e57b 100644 --- a/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/storage/dao/VolumeGroupDaoImpl.java @@ -26,12 +26,15 @@ 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("vmId", allFieldsSearch.entity().getVmId(), SearchCriteria.Op.EQ); - allFieldsSearch.and("volumeId", allFieldsSearch.entity().getVolumeId(), SearchCriteria.Op.EQ); - allFieldsSearch.and("groupNumber", allFieldsSearch.entity().getGroupNumber(), SearchCriteria.Op.EQ); + 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(); } @@ -52,15 +55,15 @@ public void addVolumeToGroup(long vmId, long volumeId, long deviceId, int groupN @Override public void deleteVolumeFromGroup(long volumeId) { SearchCriteria sc = allFieldsSearch.create(); - sc.setParameters("volumeId", volumeId); + sc.setParameters(VOLUME_ID, volumeId); expunge(sc); } @Override public VolumeGroupVO findByVmAndVolume(long vmId, long volumeId){ SearchCriteria sc = allFieldsSearch.create(); - sc.setParameters("vmId", vmId); - sc.setParameters("volumeId", volumeId); + sc.setParameters(VM_ID, vmId); + sc.setParameters(VOLUME_ID, volumeId); return findOneBy(sc); } } \ No newline at end of file diff --git a/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java b/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java index fdf98247aace..1182c2d856fb 100644 --- a/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java +++ b/server/src/main/java/com/cloud/hypervisor/HypervisorGuruBase.java @@ -98,7 +98,7 @@ public abstract class HypervisorGuruBase extends AdapterBase implements Hypervis @Inject protected HostDao hostDao; @Inject - private VolumeGroupDao _volumeGroupDao; + 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); @@ -310,7 +310,7 @@ protected Long findClusterOfVm(VirtualMachine vm) { private List addGroupsToDisks(long vmId, List disks){ disks.forEach((DiskTO disk) -> { - VolumeGroupVO volumeGroup = _volumeGroupDao.findByVmAndVolume(vmId, disk.getData().getId()); + VolumeGroupVO volumeGroup = volumeGroupDao.findByVmAndVolume(vmId, disk.getData().getId()); if(volumeGroup != null){ disk.setGroupNumber(volumeGroup.getGroupNumber()); } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index da118c829f36..96e5bb1aabbe 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -242,7 +242,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic @Inject private VolumeDetailsDao _volsDetailsDao; @Inject - private VolumeGroupDao _volsGroupDao; + private VolumeGroupDao volsGroupDao; @Inject private HostDao _hostDao; @Inject @@ -1704,7 +1704,7 @@ public Volume recoverVolume(long volumeId) { try { _volsDao.detachVolume(volume.getId()); - _volsGroupDao.deleteVolumeFromGroup(volume.getId()); + volsGroupDao.deleteVolumeFromGroup(volume.getId()); stateTransitTo(volume, Volume.Event.RecoverRequested); } catch (NoTransitionException e) { s_logger.debug("Failed to recover volume" + volume.getId(), e); @@ -2776,7 +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()); + volsGroupDao.deleteVolumeFromGroup(volume.getId()); if (answer != null) { String datastoreName = answer.getContextParam("datastoreName"); @@ -4021,7 +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); + volsGroupDao.addVolumeToGroup(vm.getId(), volumeToAttach.getId(), deviceId, groupNumber); volumeToAttach = _volsDao.findById(volumeToAttach.getId()); @@ -4046,7 +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); + volsGroupDao.addVolumeToGroup(vm.getId(), volumeToAttach.getId(), deviceId, groupNumber); volumeToAttach = _volsDao.findById(volumeToAttach.getId()); 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 6d65aaa9c6a8..6a7114f744f3 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 @@ -2398,31 +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 || groupNumber > -1) { - if (isValidScsiDiskController((VirtualLsiLogicController)device,groupNumber)) { - return ((VirtualLsiLogicController)device).getKey(); - } + 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 || groupNumber > -1) { - if (isValidScsiDiskController((VirtualLsiLogicSASController)device, groupNumber)) { - return ((VirtualLsiLogicSASController)device).getKey(); - } + 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 || groupNumber > -1) { - if (isValidScsiDiskController((ParaVirtualSCSIController)device, groupNumber)) { - return ((ParaVirtualSCSIController)device).getKey(); - } + 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 || groupNumber > -1) { - if (isValidScsiDiskController((VirtualBusLogicController)device, groupNumber)) { - return ((VirtualBusLogicController)device).getKey(); - } + if ((scsiControllerDeviceCount == requiredScsiController || groupNumber > -1) && isValidScsiDiskController((VirtualBusLogicController)device, groupNumber)) { + return ((VirtualBusLogicController)device).getKey(); } scsiControllerDeviceCount++; } @@ -2569,11 +2561,7 @@ private boolean isValidScsiDiskController(VirtualSCSIController scsiDiskControll return false; } - if(groupNumber > -1 && scsiDiskController.getBusNumber() != groupNumber){ - 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 (!VmwareHelper.isReservedScsiDeviceNumber(deviceNumber)) - break; + if (!existingUnitNumbers.contains(Integer.valueOf(deviceNumber)) && !VmwareHelper.isReservedScsiDeviceNumber(deviceNumber)) { + break; } ++deviceNumber; } From 323b69e8ce3a4ac00f6dbeba05fc8cbd04a6f65e Mon Sep 17 00:00:00 2001 From: DK101010 Date: Tue, 18 Oct 2022 07:44:20 +0100 Subject: [PATCH 22/24] adapt importVolume --- .../cloudstack/engine/orchestration/VolumeOrchestrator.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 28140525fbd3..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 @@ -2123,9 +2123,8 @@ public DiskProfile importVolume(Type type, String name, DiskOffering offering, L vol.setAttached(new Date()); vol = _volsDao.persist(vol); - if(volumeGroup != null){ - int volGroup = volumeGroup; - volumeGroupDao.addVolumeToGroup(vm.getId(), vol.getId(), vol.getDeviceId(), volGroup); + if (volumeGroup != null) { + volumeGroupDao.addVolumeToGroup(vm.getId(), vol.getId(), vol.getDeviceId(), volumeGroup); } return toDiskProfile(vol, offering); From 0785a99bccd4792604e8d7c794a3e6bce6b30ee2 Mon Sep 17 00:00:00 2001 From: DK101010 <57522802+DK101010@users.noreply.github.com> Date: Wed, 19 Oct 2022 09:25:37 +0200 Subject: [PATCH 23/24] Update vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java Co-authored-by: dahn --- .../java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6a7114f744f3..f3ebfd3163f2 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 @@ -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,int groupNumber) 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) { From 699adbd75ed98aa6223f2fa18bf5bc39fad32710 Mon Sep 17 00:00:00 2001 From: DK101010 <57522802+DK101010@users.noreply.github.com> Date: Wed, 19 Oct 2022 11:55:39 +0200 Subject: [PATCH 24/24] Apply suggestions from code review Co-authored-by: dahn --- .../apache/cloudstack/vm/UnmanagedVMsManagerImpl.java | 6 +++--- .../cloud/hypervisor/vmware/mo/VirtualMachineMO.java | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) 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 af19845d9610..238ac18345c5 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -693,7 +693,7 @@ private Pair importDisk(UnmanagedInstanceTO.Disk disk, } StoragePool storagePool = getStoragePool(disk, zone, cluster); - if ( useControllerConfiguration ){ + if (Boolean.TRUE.equals(useControllerConfiguration)) { volumeGroup = disk.getControllerUnit(); } @@ -913,7 +913,7 @@ 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 Map diskVolumeGroupMap, + final ServiceOfferingVO serviceOffering, final Map dataDiskOfferingMap, final Map diskVolumeGroupMap, final Map nicNetworkMap, final Map callerNicIpAddressMap, final Map details, final boolean migrateAllowed, final boolean forced, boolean useControllerConfiguration) { UserVm userVm = null; @@ -1007,7 +1007,7 @@ private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedI long deviceId = 1L; for (UnmanagedInstanceTO.Disk disk : dataDisks) { Integer volumeGroup = -1; - if(diskVolumeGroupMap != null && !diskVolumeGroupMap.isEmpty()){ + if (diskVolumeGroupMap != null && !diskVolumeGroupMap.isEmpty()) { volumeGroup = diskVolumeGroupMap.get(disk.getDiskId()); } if (disk.getCapacity() == null || disk.getCapacity() == 0) { 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 f3ebfd3163f2..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 @@ -1417,7 +1417,7 @@ public void attachDisk(String[] vmdkDatastorePathChain, ManagedObjectReference m unitNumber = getFreeUnitNumberOnIDEController(controllerKey); } else { if (StringUtils.isNotBlank(diskController)) { - controllerKey = getScsiDiskControllerKey(diskController,groupNumber); + controllerKey = getScsiDiskControllerKey(diskController, groupNumber); } else { controllerKey = getScsiDeviceControllerKey(groupNumber); } @@ -2370,16 +2370,16 @@ public int getScsiDiskControllerKey(String diskController, int groupNumber) thro DiskControllerType diskControllerType = DiskControllerType.getType(diskController); for (VirtualDevice device : devices) { if ((diskControllerType == DiskControllerType.lsilogic || diskControllerType == DiskControllerType.scsi) - && device instanceof VirtualLsiLogicController && isValidScsiDiskController((VirtualLsiLogicController)device,groupNumber)) { + && 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,groupNumber)) { + && 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,groupNumber)) { + && 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,groupNumber)) { + && device instanceof VirtualBusLogicController && isValidScsiDiskController((VirtualBusLogicController)device, groupNumber)) { return ((VirtualBusLogicController)device).getKey(); } }