Skip to content

[VMware] Make disk controller selection on volume attachment consistent with VM creation and start #9636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JoaoJandre merged 4 commits into from
Sep 19, 2024
Merged

[VMware] Make disk controller selection on volume attachment consistent with VM creation and start #9636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d12e9e0
Make volume attachment disk controller selection consistent with VM c…
winterhazel Sep 2, 2024
9a9c7a5
Update vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/Vmw…
winterhazel Sep 5, 2024
5cc1879
Choose disk controllers after converting osdefault
winterhazel Sep 9, 2024
e9d3682
Rename function
winterhazel Sep 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 9 additions & 44 deletions ...hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1975,16 +1975,8 @@ protected void ensureDiskControllers(VirtualMachineMO vmMo, Pair<String, String>
return;
}

String msg;
String rootDiskController = controllerInfo.first();
String dataDiskController = controllerInfo.second();
String scsiDiskController;
String recommendedDiskController = null;

if (VmwareHelper.isControllerOsRecommended(dataDiskController) || VmwareHelper.isControllerOsRecommended(rootDiskController)) {
recommendedDiskController = vmMo.getRecommendedDiskController(null);
}
scsiDiskController = HypervisorHostHelper.getScsiController(new Pair<String, String>(rootDiskController, dataDiskController), recommendedDiskController);
Pair<String, String> chosenDiskControllers = VmwareHelper.chooseRequiredDiskControllers(controllerInfo, vmMo, null, null);
String scsiDiskController = HypervisorHostHelper.getScsiController(chosenDiskControllers);
if (scsiDiskController == null) {
return;
}
Expand Down Expand Up @@ -2337,6 +2329,7 @@ protected StartAnswer execute(StartCommand cmd) {
}

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

//
// Setup ROOT/DATA disk devices
Expand All @@ -2361,10 +2354,7 @@ protected StartAnswer execute(StartCommand cmd) {
}

VirtualMachineDiskInfo matchingExistingDisk = getMatchingExistingDisk(diskInfoBuilder, vol, hyperHost, context);
String diskController = getDiskController(vmMo, matchingExistingDisk, vol, controllerInfo, deployAsIs);
if (DiskControllerType.getType(diskController) == DiskControllerType.osdefault) {
diskController = vmMo.getRecommendedDiskController(null);
}
String diskController = getDiskController(vmMo, matchingExistingDisk, vol, chosenDiskControllers, deployAsIs);
if (DiskControllerType.getType(diskController) == DiskControllerType.ide) {
controllerKey = vmMo.getIDEControllerKey(ideUnitNumber);
if (vol.getType() == Volume.Type.DATADISK) {
Expand Down Expand Up @@ -2847,27 +2837,10 @@ private String getGuestOsIdFromVmSpec(VirtualMachineTO vmSpec, boolean deployAsI
}

private Pair<String, String> getControllerInfoFromVmSpec(VirtualMachineTO vmSpec) throws CloudRuntimeException {
String dataDiskController = vmSpec.getDetails().get(VmDetailConstants.DATA_DISK_CONTROLLER);
String rootDiskController = vmSpec.getDetails().get(VmDetailConstants.ROOT_DISK_CONTROLLER);

// If root disk controller is scsi, then data disk controller would also be scsi instead of using 'osdefault'
// This helps avoid mix of different scsi subtype controllers in instance.
if (DiskControllerType.osdefault == DiskControllerType.getType(dataDiskController) && DiskControllerType.lsilogic == DiskControllerType.getType(rootDiskController)) {
dataDiskController = DiskControllerType.scsi.toString();
}

// Validate the controller types
dataDiskController = DiskControllerType.getType(dataDiskController).toString();
rootDiskController = DiskControllerType.getType(rootDiskController).toString();

if (DiskControllerType.getType(rootDiskController) == DiskControllerType.none) {
throw new CloudRuntimeException("Invalid root disk controller detected : " + rootDiskController);
}
if (DiskControllerType.getType(dataDiskController) == DiskControllerType.none) {
throw new CloudRuntimeException("Invalid data disk controller detected : " + dataDiskController);
}

return new Pair<>(rootDiskController, dataDiskController);
String rootDiskControllerDetail = vmSpec.getDetails().get(VmDetailConstants.ROOT_DISK_CONTROLLER);
String dataDiskControllerDetail = vmSpec.getDetails().get(VmDetailConstants.DATA_DISK_CONTROLLER);
VmwareHelper.validateDiskControllerDetails(rootDiskControllerDetail, dataDiskControllerDetail);
return new Pair<>(rootDiskControllerDetail, dataDiskControllerDetail);
}

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

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

private void postDiskConfigBeforeStart(VirtualMachineMO vmMo, VirtualMachineTO vmSpec, DiskTO[] sortedDisks, int ideControllerKey,
Expand Down
15 changes: 9 additions & 6 deletions ...s/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2100,15 +2100,18 @@ private Answer attachVolume(Command cmd, DiskTO disk, boolean isAttach, boolean
AttachAnswer answer = new AttachAnswer(disk);

if (isAttach) {
String diskController = getLegacyVmDataDiskController();

String rootDiskControllerDetail = DiskControllerType.ide.toString();
if (controllerInfo != null && StringUtils.isNotEmpty(controllerInfo.get(VmDetailConstants.ROOT_DISK_CONTROLLER))) {
rootDiskControllerDetail = controllerInfo.get(VmDetailConstants.ROOT_DISK_CONTROLLER);
}
String dataDiskControllerDetail = getLegacyVmDataDiskController();
if (controllerInfo != null && StringUtils.isNotEmpty(controllerInfo.get(VmDetailConstants.DATA_DISK_CONTROLLER))) {
diskController = controllerInfo.get(VmDetailConstants.DATA_DISK_CONTROLLER);
dataDiskControllerDetail = controllerInfo.get(VmDetailConstants.DATA_DISK_CONTROLLER);
}

if (DiskControllerType.getType(diskController) == DiskControllerType.osdefault) {
diskController = vmMo.getRecommendedDiskController(null);
}
VmwareHelper.validateDiskControllerDetails(rootDiskControllerDetail, dataDiskControllerDetail);
Pair<String, String> chosenDiskControllers = VmwareHelper.chooseRequiredDiskControllers(new Pair<>(rootDiskControllerDetail, dataDiskControllerDetail), vmMo, null, null);
String diskController = VmwareHelper.getControllerBasedOnDiskType(chosenDiskControllers, disk);

vmMo.attachDisk(new String[] { datastoreVolumePath }, morDs, diskController, storagePolicyId, volumeTO.getIopsReadRate() + volumeTO.getIopsWriteRate());
VirtualMachineDiskInfoBuilder diskInfoBuilder = vmMo.getDiskInfoBuilder();
Expand Down
23 changes: 4 additions & 19 deletions vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1567,15 +1567,8 @@ public static boolean createBlankVm(VmwareHypervisorHost host, String vmName, St

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

String newRootDiskController = controllerInfo.first();
String newDataDiskController = controllerInfo.second();
String recommendedController = null;
if (VmwareHelper.isControllerOsRecommended(newRootDiskController) || VmwareHelper.isControllerOsRecommended(newDataDiskController)) {
recommendedController = host.getRecommendedDiskController(guestOsIdentifier);
}

Pair<String, String> updatedControllerInfo = new Pair<String, String>(newRootDiskController, newDataDiskController);
String scsiDiskController = HypervisorHostHelper.getScsiController(updatedControllerInfo, recommendedController);
Pair<String, String> chosenDiskControllers = VmwareHelper.chooseRequiredDiskControllers(controllerInfo, null, host, guestOsIdentifier);
String scsiDiskController = HypervisorHostHelper.getScsiController(chosenDiskControllers);
// If there is requirement for a SCSI controller, ensure to create those.
if (scsiDiskController != null) {
int busNum = 0;
Expand Down Expand Up @@ -2249,19 +2242,11 @@ public static ManagedObjectReference getHypervisorHostMorFromGuid(String guid) {
return morHyperHost;
}

public static String getScsiController(Pair<String, String> controllerInfo, String recommendedController) {
public static String getScsiController(Pair<String, String> controllerInfo) {
String rootDiskController = controllerInfo.first();
String dataDiskController = controllerInfo.second();

// If "osdefault" is specified as controller type, then translate to actual recommended controller.
if (VmwareHelper.isControllerOsRecommended(rootDiskController)) {
rootDiskController = recommendedController;
}
if (VmwareHelper.isControllerOsRecommended(dataDiskController)) {
dataDiskController = recommendedController;
}

String scsiDiskController = null; //If any of the controller provided is SCSI then return it's sub-type.
String scsiDiskController; //If any of the controller provided is SCSI then return it's sub-type.
if (isIdeController(rootDiskController) && isIdeController(dataDiskController)) {
//Default controllers would exist
return null;
Expand Down
75 changes: 75 additions & 0 deletions vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,14 @@
import javax.xml.datatype.DatatypeFactory;
import javax.xml.datatype.XMLGregorianCalendar;

import com.cloud.agent.api.to.DiskTO;
import com.cloud.hypervisor.vmware.mo.ClusterMO;
import com.cloud.hypervisor.vmware.mo.DatastoreFile;
import com.cloud.hypervisor.vmware.mo.DistributedVirtualSwitchMO;
import com.cloud.hypervisor.vmware.mo.HypervisorHostHelper;
import com.cloud.serializer.GsonHelper;
import com.cloud.storage.Volume;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.net.NetUtils;
import com.vmware.vim25.DatastoreInfo;
import com.vmware.vim25.DistributedVirtualPort;
Expand Down Expand Up @@ -1063,4 +1066,76 @@ public static String getAbsoluteVmdkFile(VirtualDisk disk) {
}
return vmdkAbsFile;
}

/**
* Validates an instance's <code>rootDiskController</code> and <code>dataDiskController</code> details. Throws a
* <code>CloudRuntimeException</code> if they are invalid.
*/
public static void validateDiskControllerDetails(String rootDiskControllerDetail, String dataDiskControllerDetail) {
rootDiskControllerDetail = DiskControllerType.getType(rootDiskControllerDetail).toString();
if (DiskControllerType.getType(rootDiskControllerDetail) == DiskControllerType.none) {
throw new CloudRuntimeException(String.format("[%s] is not a valid root disk controller", rootDiskControllerDetail));
}
dataDiskControllerDetail = DiskControllerType.getType(dataDiskControllerDetail).toString();
if (DiskControllerType.getType(dataDiskControllerDetail) == DiskControllerType.none) {
throw new CloudRuntimeException(String.format("[%s] is not a valid data disk controller", dataDiskControllerDetail));
}
}

/**
* Based on an instance's <code>rootDiskController</code> and <code>dataDiskController</code> details, returns a pair
* containing the disk controllers that should be used for root disk and the data disks, respectively.
*
* @param controllerInfo pair containing the root disk and data disk controllers, respectively.
* @param vmMo virtual machine to derive the recommended disk controllers from. If not null, <code>host</code> and <code>guestOsIdentifier</code> will be ignored.
* @param host host to derive the recommended disk controllers from. Must be provided with <code>guestOsIdentifier</code>.
* @param guestOsIdentifier used to derive the recommended disk controllers from the host.
*/
public static Pair<String, String> chooseRequiredDiskControllers(Pair<String, String> controllerInfo, VirtualMachineMO vmMo,
VmwareHypervisorHost host, String guestOsIdentifier) throws Exception {
String recommendedDiskControllerClassName = vmMo != null ? vmMo.getRecommendedDiskController(null) : host.getRecommendedDiskController(guestOsIdentifier);
String recommendedDiskController = DiskControllerType.getType(recommendedDiskControllerClassName).toString();

String convertedRootDiskController = controllerInfo.first();
if (isControllerOsRecommended(convertedRootDiskController)) {
convertedRootDiskController = recommendedDiskController;
}

String convertedDataDiskController = controllerInfo.second();
if (isControllerOsRecommended(convertedDataDiskController)) {
convertedDataDiskController = recommendedDiskController;
}

if (diskControllersShareTheSameBusType(convertedRootDiskController, convertedDataDiskController)) {
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.");
return new Pair<>(convertedRootDiskController, convertedRootDiskController);
}

return new Pair<>(convertedRootDiskController, convertedDataDiskController);
}

protected static boolean diskControllersShareTheSameBusType(String rootDiskController, String dataDiskController) {
DiskControllerType rootDiskControllerType = DiskControllerType.getType(rootDiskController);
DiskControllerType dataDiskControllerType = DiskControllerType.getType(dataDiskController);
if (rootDiskControllerType.equals(dataDiskControllerType)) {
return true;
}
List<DiskControllerType> scsiDiskControllers = List.of(DiskControllerType.scsi, DiskControllerType.lsilogic, DiskControllerType.lsisas1068,
DiskControllerType.buslogic ,DiskControllerType.pvscsi);
return scsiDiskControllers.contains(rootDiskControllerType) && scsiDiskControllers.contains(dataDiskControllerType);
}

/**
* Identifies whether the disk is a root or data disk, and returns the controller from the provided pair that should
* be used for the disk.
* @param controllerInfo pair containing the root disk and data disk controllers, respectively.
*/
public static String getControllerBasedOnDiskType(Pair<String, String> controllerInfo, DiskTO disk) {
if (disk.getType() == Volume.Type.ROOT || disk.getDiskSeq() == 0) {
s_logger.debug(String.format("Choosing disk controller [%s] for the root disk.", controllerInfo.first()));
return controllerInfo.first();
}
s_logger.debug(String.format("Choosing disk controller [%s] for the data disks.", controllerInfo.second()));
return controllerInfo.second();
}
}