You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/07/19 07:57:59 UTC
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
DaanHoogland commented on a change in pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#discussion_r672068109
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -2239,215 +2309,314 @@ public LibvirtVMDef createVMFromSpec(final VirtualMachineTO vmTO) {
Map<String, String> customParams = vmTO.getDetails();
boolean isUefiEnabled = false;
boolean isSecureBoot = false;
- String bootMode =null;
+ String bootMode = null;
+
if (MapUtils.isNotEmpty(customParams) && customParams.containsKey(GuestDef.BootType.UEFI.toString())) {
isUefiEnabled = true;
- bootMode = customParams.get(GuestDef.BootType.UEFI.toString());
- if (StringUtils.isNotBlank(bootMode) && "secure".equalsIgnoreCase(bootMode)) {
+ s_logger.debug(String.format("Enabled UEFI for VM UUID [%s].", uuid));
+
+ if (isSecureMode(customParams.get(GuestDef.BootType.UEFI.toString()))) {
+ s_logger.debug(String.format("Enabled Secure Boot for VM UUID [%s].", uuid));
isSecureBoot = true;
}
}
Map<String, String> extraConfig = vmTO.getExtraConfig();
if (dpdkSupport && (!extraConfig.containsKey(DpdkHelper.DPDK_NUMA) || !extraConfig.containsKey(DpdkHelper.DPDK_HUGE_PAGES))) {
- s_logger.info("DPDK is enabled but it needs extra configurations for CPU NUMA and Huge Pages for VM deployment");
+ s_logger.info(String.format("DPDK is enabled for VM [%s], but it needs extra configurations for CPU NUMA and Huge Pages for VM deployment.", vmTO.toString()));
}
+ configureVM(vmTO, vm, customParams, isUefiEnabled, isSecureBoot, bootMode, extraConfig, uuid);
+ return vm;
+ }
- final GuestDef guest = new GuestDef();
+ /**
+ * Configures created VM from specification, adding the necessary components to VM.
+ */
+ private void configureVM(VirtualMachineTO vmTO, LibvirtVMDef vm, Map<String, String> customParams, boolean isUefiEnabled, boolean isSecureBoot, String bootMode,
+ Map<String, String> extraConfig, String uuid) {
+ s_logger.debug(String.format("Configuring VM with UUID [%s].", uuid));
- if (HypervisorType.LXC == _hypervisorType && VirtualMachine.Type.User == vmTO.getType()) {
- // LXC domain is only valid for user VMs. Use KVM for system VMs.
- guest.setGuestType(GuestDef.GuestType.LXC);
- vm.setHvsType(HypervisorType.LXC.toString().toLowerCase());
- } else {
- guest.setGuestType(GuestDef.GuestType.KVM);
- vm.setHvsType(HypervisorType.KVM.toString().toLowerCase());
- vm.setLibvirtVersion(_hypervisorLibvirtVersion);
- vm.setQemuVersion(_hypervisorQemuVersion);
+ GuestDef guest = createGuestFromSpec(vmTO, vm, uuid, customParams);
+ if (isUefiEnabled) {
+ configureGuestIfUefiEnabled(isSecureBoot, bootMode, guest);
}
- guest.setGuestArch(_guestCpuArch != null ? _guestCpuArch : vmTO.getArch());
- guest.setMachineType(_guestCpuArch != null && _guestCpuArch.equals("aarch64") ? "virt" : "pc");
- guest.setBootType(GuestDef.BootType.BIOS);
- if (MapUtils.isNotEmpty(customParams) && customParams.containsKey(GuestDef.BootType.UEFI.toString())) {
- guest.setBootType(GuestDef.BootType.UEFI);
- guest.setBootMode(GuestDef.BootMode.LEGACY);
- guest.setMachineType("q35");
- if (StringUtils.isNotBlank(customParams.get(GuestDef.BootType.UEFI.toString())) && "secure".equalsIgnoreCase(customParams.get(GuestDef.BootType.UEFI.toString()))) {
- guest.setBootMode(GuestDef.BootMode.SECURE); // setting to secure mode
- }
+
+ vm.addComp(guest);
+ vm.addComp(createGuestResourceDef(vmTO));
+
+ int vcpus = vmTO.getCpus();
+ if (!extraConfig.containsKey(DpdkHelper.DPDK_NUMA)) {
+ vm.addComp(createCpuModeDef(vmTO, vcpus));
}
- guest.setUuid(uuid);
- guest.setBootOrder(GuestDef.BootOrder.CDROM);
- guest.setBootOrder(GuestDef.BootOrder.HARDISK);
- if (isUefiEnabled) {
- if (_uefiProperties.getProperty(GuestDef.GUEST_LOADER_SECURE) != null && "secure".equalsIgnoreCase(bootMode)) {
- guest.setLoader(_uefiProperties.getProperty(GuestDef.GUEST_LOADER_SECURE));
- }
+ if (_hypervisorLibvirtVersion >= MIN_LIBVIRT_VERSION_FOR_GUEST_CPU_TUNE) {
+ vm.addComp(createCpuTuneDef(vmTO));
+ }
- if (_uefiProperties.getProperty(GuestDef.GUEST_LOADER_LEGACY) != null && "legacy".equalsIgnoreCase(bootMode)) {
- guest.setLoader(_uefiProperties.getProperty(GuestDef.GUEST_LOADER_LEGACY));
- }
+ FeaturesDef features = createFeaturesDef(customParams, isUefiEnabled, isSecureBoot);
+ enlightenWindowsVm(vmTO, features);
+ vm.addComp(features);
- if (_uefiProperties.getProperty(GuestDef.GUEST_NVRAM_PATH) != null) {
- guest.setNvram(_uefiProperties.getProperty(GuestDef.GUEST_NVRAM_PATH));
- }
+ vm.addComp(createTermPolicy());
+ vm.addComp(createClockDef(vmTO));
+ vm.addComp(createDevicesDef(vmTO, guest, vcpus, isUefiEnabled));
- if (isSecureBoot) {
- if (_uefiProperties.getProperty(GuestDef.GUEST_NVRAM_TEMPLATE_SECURE) != null && "secure".equalsIgnoreCase(bootMode)) {
- guest.setNvramTemplate(_uefiProperties.getProperty(GuestDef.GUEST_NVRAM_TEMPLATE_SECURE));
- }
- } else {
- if (_uefiProperties.getProperty(GuestDef.GUEST_NVRAM_TEMPLATE_LEGACY) != null) {
- guest.setNvramTemplate(_uefiProperties.getProperty(GuestDef.GUEST_NVRAM_TEMPLATE_LEGACY));
- }
- }
+ addExtraConfigsToVM(vmTO, vm, extraConfig);
+ }
+
+ /**
+ * Adds extra configuration to User VM Domain XML before starting.
+ */
+ private void addExtraConfigsToVM(VirtualMachineTO vmTO, LibvirtVMDef vm, Map<String, String> extraConfig) {
+ if (MapUtils.isNotEmpty(extraConfig) && VirtualMachine.Type.User.equals(vmTO.getType())) {
+ s_logger.debug(String.format("Appending extra configuration data [%s] to guest VM [%s] domain XML.", extraConfig, vmTO.toString()));
+ addExtraConfigComponent(extraConfig, vm);
}
+ }
- vm.addComp(guest);
+ /**
+ * Adds devices components to VM.
+ */
+ protected DevicesDef createDevicesDef(VirtualMachineTO vmTO, GuestDef guest, int vcpus, boolean isUefiEnabled) {
+ DevicesDef devices = new DevicesDef();
+ devices.setEmulatorPath(_hypervisorPath);
+ devices.setGuestType(guest.getGuestType());
+ devices.addDevice(createSerialDef());
- final GuestResourceDef grd = new GuestResourceDef();
+ if (_rngEnable) {
+ devices.addDevice(createRngDef());
+ }
- if (vmTO.getMinRam() != vmTO.getMaxRam() && !_noMemBalloon) {
- grd.setMemBalloning(true);
- grd.setCurrentMem(vmTO.getMinRam() / 1024);
- grd.setMemorySize(vmTO.getMaxRam() / 1024);
- } else {
- grd.setMemorySize(vmTO.getMaxRam() / 1024);
+ devices.addDevice(createChannelDef(vmTO));
+ devices.addDevice(createWatchDogDef());
+ devices.addDevice(createVideoDef());
+ devices.addDevice(createConsoleDef());
+ devices.addDevice(createGraphicDef(vmTO));
+ devices.addDevice(createTabletInputDef());
+
+ if (isGuestAarch64()) {
+ createArm64UsbDef(devices);
}
- final int vcpus = vmTO.getCpus();
- grd.setVcpuNum(vcpus);
- vm.addComp(grd);
- if (!extraConfig.containsKey(DpdkHelper.DPDK_NUMA)) {
- final CpuModeDef cmd = new CpuModeDef();
- cmd.setMode(_guestCpuMode);
- cmd.setModel(_guestCpuModel);
- if (vmTO.getType() == VirtualMachine.Type.User) {
- cmd.setFeatures(_cpuFeatures);
- }
- setCpuTopology(cmd, vcpus, vmTO.getDetails());
- vm.addComp(cmd);
+ DiskDef.DiskBus busT = getDiskModelFromVMDetail(vmTO);
+ if (busT == null) {
+ busT = getGuestDiskModel(vmTO.getPlatformEmulator(), isUefiEnabled);
+ }
+
+ if (busT == DiskDef.DiskBus.SCSI) {
+ devices.addDevice(createSCSIDef(vcpus));
}
+ return devices;
+ }
- if (_hypervisorLibvirtVersion >= 9000) {
- final CpuTuneDef ctd = new CpuTuneDef();
- /**
- A 4.0.X/4.1.X management server doesn't send the correct JSON
- command for getMinSpeed, it only sends a 'speed' field.
+ protected WatchDogDef createWatchDogDef() {
+ return new WatchDogDef(_watchDogAction, _watchDogModel);
+ }
- So if getMinSpeed() returns null we fall back to getSpeed().
+ /*
+ * Adds an explicit USB devices for ARM64
+ */
Review comment:
this comment doesn't add much to the contents of the method. Is it really needed?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org