nvazquez commented on a change in pull request #4021:
URL: https://github.com/apache/cloudstack/pull/4021#discussion_r418220189
##########
File path: api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
##########
@@ -806,6 +806,7 @@
public static final String BOOT_TYPE ="boottype";
public static final String BOOT_MODE ="bootmode";
+ public static final String BOOT_INTO_SETUP ="bootintosetup";
Review comment:
Minor style fix: missing space after the '='
##########
File path:
api/src/main/java/org/apache/cloudstack/api/command/user/vm/DeployVMCmd.java
##########
@@ -281,15 +284,14 @@ public Long getDomainId() {
}
}
}
- if(getBootType() != null){ // export to get
- if(getBootType() == ApiConstants.BootType.UEFI) {
- customparameterMap.put(getBootType().toString(),
getBootMode().toString());
- }
+ if (ApiConstants.BootType.UEFI.equals(getBootType())) {
Review comment:
The null check was removed, can you include it inside this if clause?
##########
File path:
engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -1315,6 +1324,30 @@ public void orchestrateStart(final String vmUuid, final
Map<VirtualMachineProfil
}
}
+ private void logBootModeParameters(Map<VirtualMachineProfile.Param,
Object> params) {
+ StringBuffer msgBuf = new StringBuffer("Uefi params ");
+ boolean log = false;
+ if (params.get(VirtualMachineProfile.Param.UefiFlag) != null) {
+ msgBuf.append(String.format("UefiFlag: %s ",
params.get(VirtualMachineProfile.Param.UefiFlag)));
+ log = true;
+ }
+ if (params.get(VirtualMachineProfile.Param.BootType) != null) {
+ msgBuf.append(String.format("Boot Type: %s ",
params.get(VirtualMachineProfile.Param.BootType)));
+ log = true;
+ }
+ if (params.get(VirtualMachineProfile.Param.BootMode) != null) {
+ msgBuf.append(String.format("Boot Mode: %s ",
params.get(VirtualMachineProfile.Param.BootMode)));
+ log = true;
+ }
+ if (params.get(VirtualMachineProfile.Param.BootIntoSetup) != null) {
+ msgBuf.append(String.format("Boot into Setup: %s ",
params.get(VirtualMachineProfile.Param.BootIntoSetup)));
+ log = true;
+ }
+ if (log) {
+ s_logger.info(msgBuf.toString());
Review comment:
Minor improvement request: what about removing the boolean variable and
init the string buffer empty, so at this point instead of asking for the
boolean flag you could check if the generated string is not empty, in that case
log it
##########
File path:
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -3950,6 +3970,25 @@ protected Answer execute(RebootCommand cmd) {
}
}
+ private void checkAndSetEnableSetupConfig(VirtualMachineMO vmMo,
VirtualMachineTO virtualMachine) {
+ if (virtualMachine.isEnterHardwareSetup()) {
+ VirtualMachineBootOptions bootOptions = new
VirtualMachineBootOptions();
+ VirtualMachineConfigSpec vmConfigSpec = new
VirtualMachineConfigSpec();
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug(String.format("configuring VM '%s' to reboot
into hardware setup menu.",virtualMachine.getName()));
+ }
+
bootOptions.setEnterBIOSSetup(virtualMachine.isEnterHardwareSetup());
+ vmConfigSpec.setBootOptions(bootOptions);
+ try {
+ if (!vmMo.configureVm(vmConfigSpec)) {
+ throw new Exception("Failed to configure VM to boot into
hardware setup menu: " + vmMo.getName());
+ }
+ } catch (Exception e) {
+ s_logger.error(String.format("failed to reconfigure VM '%s' to
boot into hardware setup menu",virtualMachine.getName()),e);
Review comment:
We are swallowing the exception here, should we try to stop the process
instead?
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -969,8 +973,18 @@ private UserVm rebootVirtualMachine(long userId, long
vmId) throws InsufficientC
} catch (Exception ex){
throw new CloudRuntimeException("Router start failed due to" +
ex);
}finally {
- s_logger.info("Rebooting vm " + vm.getInstanceName());
- _itMgr.reboot(vm.getUuid(), null);
+ if (s_logger.isInfoEnabled()) {
+ s_logger.info(String.format("Rebooting vm %s%s.",
vm.getInstanceName(), enterSetup? " entering hardware setup menu" : " as is"));
+ }
+ Map<VirtualMachineProfile.Param,Object> params = null;
+ if(enterSetup) {
Review comment:
Space
##########
File path:
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -1723,10 +1723,13 @@ protected StartAnswer execute(StartCommand cmd) {
String dataDiskController =
vmSpec.getDetails().get(VmDetailConstants.DATA_DISK_CONTROLLER);
String rootDiskController =
vmSpec.getDetails().get(VmDetailConstants.ROOT_DISK_CONTROLLER);
DiskTO rootDiskTO = null;
- String bootMode = "bios";
+ String bootMode = null;
if (vmSpec.getDetails().containsKey(VmDetailConstants.BOOT_MODE)) {
bootMode = vmSpec.getDetails().get(VmDetailConstants.BOOT_MODE);
}
+ if(null == bootMode) {
Review comment:
Missing space
##########
File path:
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java
##########
@@ -686,8 +400,7 @@ public int compare(NicTO arg0, NicTO arg1) {
return new Pair<Boolean, Long>(Boolean.FALSE, new Long(hostId));
}
- @Override
- public boolean trackVmHostChange() {
+ @Override public boolean trackVmHostChange() {
Review comment:
Just format stuff: these annotations have been moved to the same line as
the method's firm all over the class
##########
File path: server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
##########
@@ -2866,7 +2892,11 @@ public UserVm rebootVirtualMachine(RebootVMCmd cmd)
throws InsufficientCapacityE
throw new InvalidParameterValueException("Unable to find service
offering: " + serviceOfferingId + " corresponding to the vm");
}
- UserVm userVm =
rebootVirtualMachine(CallContext.current().getCallingUserId(), vmId);
+ Boolean enterSetup = cmd.getBootIntoSetup();
+ if (! (enterSetup == null ||
HypervisorType.VMware.equals(vmInstance.getHypervisorType()))) {
Review comment:
For readability can you change it to: enterSetup != null && hypervisor
!= Vmware?
##########
File path:
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java
##########
@@ -172,326 +157,59 @@
public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru,
Configurable {
private static final Logger s_logger = Logger.getLogger(VMwareGuru.class);
- @Inject
- private NetworkDao _networkDao;
- @Inject
- private GuestOSDao _guestOsDao;
- @Inject
- private GuestOSHypervisorDao _guestOsHypervisorDao;
- @Inject
- private HostDao _hostDao;
- @Inject
- private HostDetailsDao _hostDetailsDao;
- @Inject
- private ClusterDetailsDao _clusterDetailsDao;
- @Inject
- private CommandExecLogDao _cmdExecLogDao;
- @Inject
- private VmwareManager _vmwareMgr;
- @Inject
- private SecondaryStorageVmManager _secStorageMgr;
- @Inject
- private NetworkModel _networkMgr;
- @Inject
- private NicDao _nicDao;
- @Inject
- private DomainRouterDao _domainRouterDao;
- @Inject
- private PhysicalNetworkTrafficTypeDao _physicalNetworkTrafficTypeDao;
- @Inject
- private VMInstanceDao _vmDao;
- @Inject
- private VirtualMachineManager vmManager;
- @Inject
- private ClusterManager _clusterMgr;
- @Inject
- VolumeDao _volumeDao;
- @Inject
- ResourceLimitService _resourceLimitService;
- @Inject
- PrimaryDataStoreDao _storagePoolDao;
- @Inject
- VolumeDataFactory _volFactory;
- @Inject
- private VmwareDatacenterDao vmwareDatacenterDao;
- @Inject
- private VmwareDatacenterZoneMapDao vmwareDatacenterZoneMapDao;
- @Inject
- private ServiceOfferingDao serviceOfferingDao;
- @Inject
- private VMTemplatePoolDao templateStoragePoolDao;
- @Inject
- private VMTemplateDao vmTemplateDao;
- @Inject
- private UserVmDao userVmDao;
- @Inject
- private DiskOfferingDao diskOfferingDao;
- @Inject
- private PhysicalNetworkDao physicalNetworkDao;
- @Inject
- private TemplateOVFPropertiesDao templateOVFPropertiesDao;
+
+ private VmwareVmImplementer vmwareVmImplementer;
+
+ @Inject NetworkDao _networkDao;
+ @Inject GuestOSDao _guestOsDao;
+ @Inject HostDao _hostDao;
+ @Inject HostDetailsDao _hostDetailsDao;
+ @Inject ClusterDetailsDao _clusterDetailsDao;
+ @Inject CommandExecLogDao _cmdExecLogDao;
+ @Inject VmwareManager _vmwareMgr;
+ @Inject SecondaryStorageVmManager _secStorageMgr;
+ @Inject NicDao _nicDao;
+ @Inject PhysicalNetworkTrafficTypeDao _physicalNetworkTrafficTypeDao;
+ @Inject VMInstanceDao _vmDao;
+ @Inject VirtualMachineManager vmManager;
+ @Inject ClusterManager _clusterMgr;
+ @Inject VolumeDao _volumeDao;
+ @Inject ResourceLimitService _resourceLimitService;
+ @Inject PrimaryDataStoreDao _storagePoolDao;
+ @Inject VolumeDataFactory _volFactory;
+ @Inject VmwareDatacenterDao vmwareDatacenterDao;
+ @Inject VmwareDatacenterZoneMapDao vmwareDatacenterZoneMapDao;
+ @Inject ServiceOfferingDao serviceOfferingDao;
+ @Inject VMTemplatePoolDao templateStoragePoolDao;
+ @Inject VMTemplateDao vmTemplateDao;
+ @Inject UserVmDao userVmDao;
+ @Inject DiskOfferingDao diskOfferingDao;
+ @Inject PhysicalNetworkDao physicalNetworkDao;
protected VMwareGuru() {
super();
}
public static final ConfigKey<Boolean> VmwareReserveCpu = new
ConfigKey<Boolean>(Boolean.class, "vmware.reserve.cpu", "Advanced", "false",
- "Specify whether or not to reserve CPU when deploying an instance.",
true, ConfigKey.Scope.Cluster,
- null);
+ "Specify whether or not to reserve CPU when deploying an
instance.", true, ConfigKey.Scope.Cluster, null);
public static final ConfigKey<Boolean> VmwareReserveMemory = new
ConfigKey<Boolean>(Boolean.class, "vmware.reserve.mem", "Advanced", "false",
- "Specify whether or not to reserve memory when deploying an
instance.", true,
- ConfigKey.Scope.Cluster, null);
+ "Specify whether or not to reserve memory when deploying an
instance.", true, ConfigKey.Scope.Cluster, null);
protected ConfigKey<Boolean> VmwareEnableNestedVirtualization = new
ConfigKey<Boolean>(Boolean.class, "vmware.nested.virtualization", "Advanced",
"false",
"When set to true this will enable nested virtualization when this
is supported by the hypervisor", true, ConfigKey.Scope.Global, null);
protected ConfigKey<Boolean> VmwareEnableNestedVirtualizationPerVM = new
ConfigKey<Boolean>(Boolean.class, "vmware.nested.virtualization.perVM",
"Advanced", "false",
"When set to true this will enable nested virtualization per vm",
true, ConfigKey.Scope.Global, null);
- @Override
- public HypervisorType getHypervisorType() {
+ @Override public HypervisorType getHypervisorType() {
return HypervisorType.VMware;
}
- @Override
- public VirtualMachineTO implement(VirtualMachineProfile vm) {
- VirtualMachineTO to = toVirtualMachineTO(vm);
- to.setBootloader(BootloaderType.HVM);
-
- Map<String, String> details = to.getDetails();
- if (details == null)
- details = new HashMap<String, String>();
-
- Type vmType = vm.getType();
- boolean userVm = !(vmType.equals(VirtualMachine.Type.DomainRouter) ||
vmType.equals(VirtualMachine.Type.ConsoleProxy)
- || vmType.equals(VirtualMachine.Type.SecondaryStorageVm));
-
- String nicDeviceType = details.get(VmDetailConstants.NIC_ADAPTER);
- if (!userVm) {
-
- if (nicDeviceType == null) {
- details.put(VmDetailConstants.NIC_ADAPTER,
_vmwareMgr.getSystemVMDefaultNicAdapterType());
- } else {
- try {
- VirtualEthernetCardType.valueOf(nicDeviceType);
- } catch (Exception e) {
- s_logger.warn("Invalid NIC device type " + nicDeviceType +
" is specified in VM details, switch to default E1000");
- details.put(VmDetailConstants.NIC_ADAPTER,
VirtualEthernetCardType.E1000.toString());
- }
- }
- } else {
- // for user-VM, use E1000 as default
- if (nicDeviceType == null) {
- details.put(VmDetailConstants.NIC_ADAPTER,
VirtualEthernetCardType.E1000.toString());
- } else {
- try {
- VirtualEthernetCardType.valueOf(nicDeviceType);
- } catch (Exception e) {
- s_logger.warn("Invalid NIC device type " + nicDeviceType +
" is specified in VM details, switch to default E1000");
- details.put(VmDetailConstants.NIC_ADAPTER,
VirtualEthernetCardType.E1000.toString());
- }
- }
- }
-
- details.put(VmDetailConstants.BOOT_MODE, to.getBootMode());
- String diskDeviceType =
details.get(VmDetailConstants.ROOT_DISK_CONTROLLER);
- if (userVm) {
- if (diskDeviceType == null) {
- details.put(VmDetailConstants.ROOT_DISK_CONTROLLER,
_vmwareMgr.getRootDiskController());
- }
- }
- String diskController =
details.get(VmDetailConstants.DATA_DISK_CONTROLLER);
- if (userVm) {
- if (diskController == null) {
- details.put(VmDetailConstants.DATA_DISK_CONTROLLER,
DiskControllerType.lsilogic.toString());
- }
- }
-
- if (vm.getType() == VirtualMachine.Type.NetScalerVm) {
- details.put(VmDetailConstants.ROOT_DISK_CONTROLLER, "scsi");
- }
-
- List<NicProfile> nicProfiles = vm.getNics();
-
- for (NicProfile nicProfile : nicProfiles) {
- if (nicProfile.getTrafficType() == TrafficType.Guest) {
- if
(_networkMgr.isProviderSupportServiceInNetwork(nicProfile.getNetworkId(),
Service.Firewall, Provider.CiscoVnmc)) {
- details.put("ConfigureVServiceInNexus",
Boolean.TRUE.toString());
- }
- break;
- }
- }
-
- long clusterId = getClusterId(vm.getId());
- details.put(VmwareReserveCpu.key(),
VmwareReserveCpu.valueIn(clusterId).toString());
- details.put(VmwareReserveMemory.key(),
VmwareReserveMemory.valueIn(clusterId).toString());
- to.setDetails(details);
-
- if (vmType.equals(VirtualMachine.Type.DomainRouter)) {
-
- NicProfile publicNicProfile = null;
- for (NicProfile nicProfile : nicProfiles) {
- if (nicProfile.getTrafficType() == TrafficType.Public) {
- publicNicProfile = nicProfile;
- break;
- }
- }
-
- if (publicNicProfile != null) {
- NicTO[] nics = to.getNics();
-
- // reserve extra NICs
- NicTO[] expandedNics = new NicTO[nics.length +
_vmwareMgr.getRouterExtraPublicNics()];
- int i = 0;
- int deviceId = -1;
- for (i = 0; i < nics.length; i++) {
- expandedNics[i] = nics[i];
- if (nics[i].getDeviceId() > deviceId)
- deviceId = nics[i].getDeviceId();
- }
- deviceId++;
-
- long networkId = publicNicProfile.getNetworkId();
- NetworkVO network = _networkDao.findById(networkId);
-
- for (; i < nics.length +
_vmwareMgr.getRouterExtraPublicNics(); i++) {
- NicTO nicTo = new NicTO();
-
- nicTo.setDeviceId(deviceId++);
-
nicTo.setBroadcastType(publicNicProfile.getBroadcastType());
- nicTo.setType(publicNicProfile.getTrafficType());
- nicTo.setIp("0.0.0.0");
- nicTo.setNetmask("255.255.255.255");
-
- try {
- String mac =
_networkMgr.getNextAvailableMacAddressInNetwork(networkId);
- nicTo.setMac(mac);
- } catch (InsufficientAddressCapacityException e) {
- throw new CloudRuntimeException("unable to allocate
mac address on network: " + networkId);
- }
- nicTo.setDns1(publicNicProfile.getIPv4Dns1());
- nicTo.setDns2(publicNicProfile.getIPv4Dns2());
- if (publicNicProfile.getIPv4Gateway() != null) {
- nicTo.setGateway(publicNicProfile.getIPv4Gateway());
- } else {
- nicTo.setGateway(network.getGateway());
- }
- nicTo.setDefaultNic(false);
- nicTo.setBroadcastUri(publicNicProfile.getBroadCastUri());
- nicTo.setIsolationuri(publicNicProfile.getIsolationUri());
-
- Integer networkRate =
_networkMgr.getNetworkRate(network.getId(), null);
- nicTo.setNetworkRateMbps(networkRate);
-
- expandedNics[i] = nicTo;
- }
-
- to.setNics(expandedNics);
-
- VirtualMachine router = vm.getVirtualMachine();
- DomainRouterVO routerVO =
_domainRouterDao.findById(router.getId());
- if (routerVO != null && routerVO.getIsRedundantRouter()) {
- Long peerRouterId =
_nicDao.getPeerRouterId(publicNicProfile.getMacAddress(), router.getId());
- DomainRouterVO peerRouterVO = null;
- if (peerRouterId != null) {
- peerRouterVO = _domainRouterDao.findById(peerRouterId);
- if (peerRouterVO != null) {
- details.put("PeerRouterInstanceName",
peerRouterVO.getInstanceName());
- }
- }
- }
- }
-
- StringBuffer sbMacSequence = new StringBuffer();
- for (NicTO nicTo : sortNicsByDeviceId(to.getNics())) {
- sbMacSequence.append(nicTo.getMac()).append("|");
- }
- if (!sbMacSequence.toString().isEmpty()) {
- sbMacSequence.deleteCharAt(sbMacSequence.length() - 1);
- String bootArgs = to.getBootArgs();
- to.setBootArgs(bootArgs + " nic_macs=" +
sbMacSequence.toString());
- }
-
- }
-
- // Don't do this if the virtual machine is one of the special types
- // Should only be done on user machines
- if (userVm) {
- configureNestedVirtualization(details, to);
- }
- // Determine the VM's OS description
- GuestOSVO guestOS =
_guestOsDao.findByIdIncludingRemoved(vm.getVirtualMachine().getGuestOSId());
- to.setOs(guestOS.getDisplayName());
- to.setHostName(vm.getHostName());
- HostVO host = _hostDao.findById(vm.getVirtualMachine().getHostId());
- GuestOSHypervisorVO guestOsMapping = null;
- if (host != null) {
- guestOsMapping =
_guestOsHypervisorDao.findByOsIdAndHypervisor(guestOS.getId(),
getHypervisorType().toString(), host.getHypervisorVersion());
- }
- if (guestOsMapping == null || host == null) {
- to.setPlatformEmulator(null);
- } else {
- to.setPlatformEmulator(guestOsMapping.getGuestOsName());
- }
-
- List<OVFPropertyTO> ovfProperties = new ArrayList<>();
- for (String detailKey : details.keySet()) {
- if (detailKey.startsWith(ApiConstants.OVF_PROPERTIES)) {
- String ovfPropKey =
detailKey.replace(ApiConstants.OVF_PROPERTIES + "-", "");
- TemplateOVFPropertyVO templateOVFPropertyVO =
templateOVFPropertiesDao.findByTemplateAndKey(vm.getTemplateId(), ovfPropKey);
- if (templateOVFPropertyVO == null) {
- s_logger.warn(String.format("OVF property %s not found on
template, discarding", ovfPropKey));
- continue;
- }
- String ovfValue = details.get(detailKey);
- boolean isPassword = templateOVFPropertyVO.isPassword();
- OVFPropertyTO propertyTO = new OVFPropertyTO(ovfPropKey,
ovfValue, isPassword);
- ovfProperties.add(propertyTO);
- }
- }
-
- if (CollectionUtils.isNotEmpty(ovfProperties)) {
- removeOvfPropertiesFromDetails(ovfProperties, details);
- String templateInstallPath = null;
- List<DiskTO> rootDiskList = vm.getDisks().stream().filter(x ->
x.getType() == Volume.Type.ROOT).collect(Collectors.toList());
- if (rootDiskList.size() != 1) {
- throw new CloudRuntimeException("Did not find only one root
disk for VM " + vm.getHostName());
- }
-
- DiskTO rootDiskTO = rootDiskList.get(0);
- DataStoreTO dataStore = rootDiskTO.getData().getDataStore();
- StoragePoolVO storagePoolVO =
_storagePoolDao.findByUuid(dataStore.getUuid());
- long dataCenterId = storagePoolVO.getDataCenterId();
- List<StoragePoolVO> pools =
_storagePoolDao.listByDataCenterId(dataCenterId);
- for (StoragePoolVO pool : pools) {
- VMTemplateStoragePoolVO ref =
templateStoragePoolDao.findByPoolTemplate(pool.getId(), vm.getTemplateId());
- if (ref != null && ref.getDownloadState() ==
VMTemplateStorageResourceAssoc.Status.DOWNLOADED) {
- templateInstallPath = ref.getInstallPath();
- break;
- }
- }
+ @Override public VirtualMachineTO implement(VirtualMachineProfile vm) {
+ vmwareVmImplementer = ComponentContext.inject(new
VmwareVmImplementer(this));
Review comment:
Why not using the @Inject annotation?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
