Skip to content

Commit 075f981

Browse files
winterhazelDaanHoogland
winterhazel
and
DaanHoogland
authored
[VMware] Make disk controller selection on volume attachment consistent with VM creation and start (#9636)
* Make volume attachment disk controller selection consistent with VM creation and start * Update vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java Co-authored-by: dahn <daan.hoogland@gmail.com> * Choose disk controllers after converting osdefault * Rename function --------- Co-authored-by: dahn <daan.hoogland@gmail.com>
1 parent 7f2ab63 commit 075f981

File tree

4 files changed

+97
-69
lines changed

4 files changed

+97
-69
lines changed

‎plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java‎

Lines changed: 9 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1975,16 +1975,8 @@ protected void ensureDiskControllers(VirtualMachineMO vmMo, Pair<String, String>
19751975
return;
19761976
}
19771977

1978-
String msg;
1979-
String rootDiskController = controllerInfo.first();
1980-
String dataDiskController = controllerInfo.second();
1981-
String scsiDiskController;
1982-
String recommendedDiskController = null;
1983-
1984-
if (VmwareHelper.isControllerOsRecommended(dataDiskController) || VmwareHelper.isControllerOsRecommended(rootDiskController)) {
1985-
recommendedDiskController = vmMo.getRecommendedDiskController(null);
1986-
}
1987-
scsiDiskController = HypervisorHostHelper.getScsiController(new Pair<String, String>(rootDiskController, dataDiskController), recommendedDiskController);
1978+
Pair<String, String> chosenDiskControllers = VmwareHelper.chooseRequiredDiskControllers(controllerInfo, vmMo, null, null);
1979+
String scsiDiskController = HypervisorHostHelper.getScsiController(chosenDiskControllers);
19881980
if (scsiDiskController == null) {
19891981
return;
19901982
}
@@ -2337,6 +2329,7 @@ protected StartAnswer execute(StartCommand cmd) {
23372329
}
23382330

23392331
int controllerKey;
2332+
Pair<String, String> chosenDiskControllers = VmwareHelper.chooseRequiredDiskControllers(controllerInfo,vmMo, null, null);
23402333

23412334
//
23422335
// Setup ROOT/DATA disk devices
@@ -2361,10 +2354,7 @@ protected StartAnswer execute(StartCommand cmd) {
23612354
}
23622355

23632356
VirtualMachineDiskInfo matchingExistingDisk = getMatchingExistingDisk(diskInfoBuilder, vol, hyperHost, context);
2364-
String diskController = getDiskController(vmMo, matchingExistingDisk, vol, controllerInfo, deployAsIs);
2365-
if (DiskControllerType.getType(diskController) == DiskControllerType.osdefault) {
2366-
diskController = vmMo.getRecommendedDiskController(null);
2367-
}
2357+
String diskController = getDiskController(vmMo, matchingExistingDisk, vol, chosenDiskControllers, deployAsIs);
23682358
if (DiskControllerType.getType(diskController) == DiskControllerType.ide) {
23692359
controllerKey = vmMo.getIDEControllerKey(ideUnitNumber);
23702360
if (vol.getType() == Volume.Type.DATADISK) {
@@ -2847,27 +2837,10 @@ private String getGuestOsIdFromVmSpec(VirtualMachineTO vmSpec, boolean deployAsI
28472837
}
28482838

28492839
private Pair<String, String> getControllerInfoFromVmSpec(VirtualMachineTO vmSpec) throws CloudRuntimeException {
2850-
String dataDiskController = vmSpec.getDetails().get(VmDetailConstants.DATA_DISK_CONTROLLER);
2851-
String rootDiskController = vmSpec.getDetails().get(VmDetailConstants.ROOT_DISK_CONTROLLER);
2852-
2853-
// If root disk controller is scsi, then data disk controller would also be scsi instead of using 'osdefault'
2854-
// This helps avoid mix of different scsi subtype controllers in instance.
2855-
if (DiskControllerType.osdefault == DiskControllerType.getType(dataDiskController) && DiskControllerType.lsilogic == DiskControllerType.getType(rootDiskController)) {
2856-
dataDiskController = DiskControllerType.scsi.toString();
2857-
}
2858-
2859-
// Validate the controller types
2860-
dataDiskController = DiskControllerType.getType(dataDiskController).toString();
2861-
rootDiskController = DiskControllerType.getType(rootDiskController).toString();
2862-
2863-
if (DiskControllerType.getType(rootDiskController) == DiskControllerType.none) {
2864-
throw new CloudRuntimeException("Invalid root disk controller detected : " + rootDiskController);
2865-
}
2866-
if (DiskControllerType.getType(dataDiskController) == DiskControllerType.none) {
2867-
throw new CloudRuntimeException("Invalid data disk controller detected : " + dataDiskController);
2868-
}
2869-
2870-
return new Pair<>(rootDiskController, dataDiskController);
2840+
String rootDiskControllerDetail = vmSpec.getDetails().get(VmDetailConstants.ROOT_DISK_CONTROLLER);
2841+
String dataDiskControllerDetail = vmSpec.getDetails().get(VmDetailConstants.DATA_DISK_CONTROLLER);
2842+
VmwareHelper.validateDiskControllerDetails(rootDiskControllerDetail, dataDiskControllerDetail);
2843+
return new Pair<>(rootDiskControllerDetail, dataDiskControllerDetail);
28712844
}
28722845

28732846
private String getBootModeFromVmSpec(VirtualMachineTO vmSpec, boolean deployAsIs) {
@@ -3615,15 +3588,7 @@ private String getDiskController(VirtualMachineMO vmMo, VirtualMachineDiskInfo m
36153588
return controllerType.toString();
36163589
}
36173590

3618-
if (vol.getType() == Volume.Type.ROOT) {
3619-
s_logger.info("Chose disk controller for vol " + vol.getType() + " -> " + controllerInfo.first()
3620-
+ ", based on root disk controller settings at global configuration setting.");
3621-
return controllerInfo.first();
3622-
} else {
3623-
s_logger.info("Chose disk controller for vol " + vol.getType() + " -> " + controllerInfo.second()
3624-
+ ", based on default data disk controller setting i.e. Operating system recommended."); // Need to bring in global configuration setting & template level setting.
3625-
return controllerInfo.second();
3626-
}
3591+
return VmwareHelper.getControllerBasedOnDiskType(controllerInfo, vol);
36273592
}
36283593

36293594
private void postDiskConfigBeforeStart(VirtualMachineMO vmMo, VirtualMachineTO vmSpec, DiskTO[] sortedDisks, int ideControllerKey,

‎plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2100,15 +2100,18 @@ private Answer attachVolume(Command cmd, DiskTO disk, boolean isAttach, boolean
21002100
AttachAnswer answer = new AttachAnswer(disk);
21012101

21022102
if (isAttach) {
2103-
String diskController = getLegacyVmDataDiskController();
2104-
2103+
String rootDiskControllerDetail = DiskControllerType.ide.toString();
2104+
if (controllerInfo != null && StringUtils.isNotEmpty(controllerInfo.get(VmDetailConstants.ROOT_DISK_CONTROLLER))) {
2105+
rootDiskControllerDetail = controllerInfo.get(VmDetailConstants.ROOT_DISK_CONTROLLER);
2106+
}
2107+
String dataDiskControllerDetail = getLegacyVmDataDiskController();
21052108
if (controllerInfo != null && StringUtils.isNotEmpty(controllerInfo.get(VmDetailConstants.DATA_DISK_CONTROLLER))) {
2106-
diskController = controllerInfo.get(VmDetailConstants.DATA_DISK_CONTROLLER);
2109+
dataDiskControllerDetail = controllerInfo.get(VmDetailConstants.DATA_DISK_CONTROLLER);
21072110
}
21082111

2109-
if (DiskControllerType.getType(diskController) == DiskControllerType.osdefault) {
2110-
diskController = vmMo.getRecommendedDiskController(null);
2111-
}
2112+
VmwareHelper.validateDiskControllerDetails(rootDiskControllerDetail, dataDiskControllerDetail);
2113+
Pair<String, String> chosenDiskControllers = VmwareHelper.chooseRequiredDiskControllers(new Pair<>(rootDiskControllerDetail, dataDiskControllerDetail), vmMo, null, null);
2114+
String diskController = VmwareHelper.getControllerBasedOnDiskType(chosenDiskControllers, disk);
21122115

21132116
vmMo.attachDisk(new String[] { datastoreVolumePath }, morDs, diskController, storagePolicyId, volumeTO.getIopsReadRate() + volumeTO.getIopsWriteRate());
21142117
VirtualMachineDiskInfoBuilder diskInfoBuilder = vmMo.getDiskInfoBuilder();

‎vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java‎

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,15 +1567,8 @@ public static boolean createBlankVm(VmwareHypervisorHost host, String vmName, St
15671567

15681568
VmwareHelper.setBasicVmConfig(vmConfig, cpuCount, cpuSpeedMHz, cpuReservedMHz, memoryMB, memoryReserveMB, guestOsIdentifier, limitCpuUse, false);
15691569

1570-
String newRootDiskController = controllerInfo.first();
1571-
String newDataDiskController = controllerInfo.second();
1572-
String recommendedController = null;
1573-
if (VmwareHelper.isControllerOsRecommended(newRootDiskController) || VmwareHelper.isControllerOsRecommended(newDataDiskController)) {
1574-
recommendedController = host.getRecommendedDiskController(guestOsIdentifier);
1575-
}
1576-
1577-
Pair<String, String> updatedControllerInfo = new Pair<String, String>(newRootDiskController, newDataDiskController);
1578-
String scsiDiskController = HypervisorHostHelper.getScsiController(updatedControllerInfo, recommendedController);
1570+
Pair<String, String> chosenDiskControllers = VmwareHelper.chooseRequiredDiskControllers(controllerInfo, null, host, guestOsIdentifier);
1571+
String scsiDiskController = HypervisorHostHelper.getScsiController(chosenDiskControllers);
15791572
// If there is requirement for a SCSI controller, ensure to create those.
15801573
if (scsiDiskController != null) {
15811574
int busNum = 0;
@@ -2249,19 +2242,11 @@ public static ManagedObjectReference getHypervisorHostMorFromGuid(String guid) {
22492242
return morHyperHost;
22502243
}
22512244

2252-
public static String getScsiController(Pair<String, String> controllerInfo, String recommendedController) {
2245+
public static String getScsiController(Pair<String, String> controllerInfo) {
22532246
String rootDiskController = controllerInfo.first();
22542247
String dataDiskController = controllerInfo.second();
22552248

2256-
// If "osdefault" is specified as controller type, then translate to actual recommended controller.
2257-
if (VmwareHelper.isControllerOsRecommended(rootDiskController)) {
2258-
rootDiskController = recommendedController;
2259-
}
2260-
if (VmwareHelper.isControllerOsRecommended(dataDiskController)) {
2261-
dataDiskController = recommendedController;
2262-
}
2263-
2264-
String scsiDiskController = null; //If any of the controller provided is SCSI then return it's sub-type.
2249+
String scsiDiskController; //If any of the controller provided is SCSI then return it's sub-type.
22652250
if (isIdeController(rootDiskController) && isIdeController(dataDiskController)) {
22662251
//Default controllers would exist
22672252
return null;

‎vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,14 @@
4040
import javax.xml.datatype.DatatypeFactory;
4141
import javax.xml.datatype.XMLGregorianCalendar;
4242

43+
import com.cloud.agent.api.to.DiskTO;
4344
import com.cloud.hypervisor.vmware.mo.ClusterMO;
4445
import com.cloud.hypervisor.vmware.mo.DatastoreFile;
4546
import com.cloud.hypervisor.vmware.mo.DistributedVirtualSwitchMO;
4647
import com.cloud.hypervisor.vmware.mo.HypervisorHostHelper;
4748
import com.cloud.serializer.GsonHelper;
49+
import com.cloud.storage.Volume;
50+
import com.cloud.utils.exception.CloudRuntimeException;
4851
import com.cloud.utils.net.NetUtils;
4952
import com.vmware.vim25.DatastoreInfo;
5053
import com.vmware.vim25.DistributedVirtualPort;
@@ -1063,4 +1066,76 @@ public static String getAbsoluteVmdkFile(VirtualDisk disk) {
10631066
}
10641067
return vmdkAbsFile;
10651068
}
1069+
1070+
/**
1071+
* Validates an instance's <code>rootDiskController</code> and <code>dataDiskController</code> details. Throws a
1072+
* <code>CloudRuntimeException</code> if they are invalid.
1073+
*/
1074+
public static void validateDiskControllerDetails(String rootDiskControllerDetail, String dataDiskControllerDetail) {
1075+
rootDiskControllerDetail = DiskControllerType.getType(rootDiskControllerDetail).toString();
1076+
if (DiskControllerType.getType(rootDiskControllerDetail) == DiskControllerType.none) {
1077+
throw new CloudRuntimeException(String.format("[%s] is not a valid root disk controller", rootDiskControllerDetail));
1078+
}
1079+
dataDiskControllerDetail = DiskControllerType.getType(dataDiskControllerDetail).toString();
1080+
if (DiskControllerType.getType(dataDiskControllerDetail) == DiskControllerType.none) {
1081+
throw new CloudRuntimeException(String.format("[%s] is not a valid data disk controller", dataDiskControllerDetail));
1082+
}
1083+
}
1084+
1085+
/**
1086+
* Based on an instance's <code>rootDiskController</code> and <code>dataDiskController</code> details, returns a pair
1087+
* containing the disk controllers that should be used for root disk and the data disks, respectively.
1088+
*
1089+
* @param controllerInfo pair containing the root disk and data disk controllers, respectively.
1090+
* @param vmMo virtual machine to derive the recommended disk controllers from. If not null, <code>host</code> and <code>guestOsIdentifier</code> will be ignored.
1091+
* @param host host to derive the recommended disk controllers from. Must be provided with <code>guestOsIdentifier</code>.
1092+
* @param guestOsIdentifier used to derive the recommended disk controllers from the host.
1093+
*/
1094+
public static Pair<String, String> chooseRequiredDiskControllers(Pair<String, String> controllerInfo, VirtualMachineMO vmMo,
1095+
VmwareHypervisorHost host, String guestOsIdentifier) throws Exception {
1096+
String recommendedDiskControllerClassName = vmMo != null ? vmMo.getRecommendedDiskController(null) : host.getRecommendedDiskController(guestOsIdentifier);
1097+
String recommendedDiskController = DiskControllerType.getType(recommendedDiskControllerClassName).toString();
1098+
1099+
String convertedRootDiskController = controllerInfo.first();
1100+
if (isControllerOsRecommended(convertedRootDiskController)) {
1101+
convertedRootDiskController = recommendedDiskController;
1102+
}
1103+
1104+
String convertedDataDiskController = controllerInfo.second();
1105+
if (isControllerOsRecommended(convertedDataDiskController)) {
1106+
convertedDataDiskController = recommendedDiskController;
1107+
}
1108+
1109+
if (diskControllersShareTheSameBusType(convertedRootDiskController, convertedDataDiskController)) {
1110+
s_logger.debug("Root and data disk controllers share the same bus type; therefore, we will only use the controllers specified for the root disk.");
1111+
return new Pair<>(convertedRootDiskController, convertedRootDiskController);
1112+
}
1113+
1114+
return new Pair<>(convertedRootDiskController, convertedDataDiskController);
1115+
}
1116+
1117+
protected static boolean diskControllersShareTheSameBusType(String rootDiskController, String dataDiskController) {
1118+
DiskControllerType rootDiskControllerType = DiskControllerType.getType(rootDiskController);
1119+
DiskControllerType dataDiskControllerType = DiskControllerType.getType(dataDiskController);
1120+
if (rootDiskControllerType.equals(dataDiskControllerType)) {
1121+
return true;
1122+
}
1123+
List<DiskControllerType> scsiDiskControllers = List.of(DiskControllerType.scsi, DiskControllerType.lsilogic, DiskControllerType.lsisas1068,
1124+
DiskControllerType.buslogic ,DiskControllerType.pvscsi);
1125+
return scsiDiskControllers.contains(rootDiskControllerType) && scsiDiskControllers.contains(dataDiskControllerType);
1126+
}
1127+
1128+
/**
1129+
* Identifies whether the disk is a root or data disk, and returns the controller from the provided pair that should
1130+
* be used for the disk.
1131+
* @param controllerInfo pair containing the root disk and data disk controllers, respectively.
1132+
*/
1133+
public static String getControllerBasedOnDiskType(Pair<String, String> controllerInfo, DiskTO disk) {
1134+
if (disk.getType() == Volume.Type.ROOT || disk.getDiskSeq() == 0) {
1135+
s_logger.debug(String.format("Choosing disk controller [%s] for the root disk.", controllerInfo.first()));
1136+
return controllerInfo.first();
1137+
}
1138+
s_logger.debug(String.format("Choosing disk controller [%s] for the data disks.", controllerInfo.second()));
1139+
return controllerInfo.second();
1140+
}
10661141
}

0 commit comments

Comments
 (0)