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/01 15:15:16 UTC
[GitHub] [cloudstack] SadiJr commented on a change in pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
SadiJr commented on a change in pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#discussion_r662379950
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -2239,211 +2305,309 @@ 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();
+ /**
+ * Configure 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);
+ }
+
+ /**
+ * Add 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);
+ /**
+ * Add 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().
+ /*
+ * Add an explicit USB devices for ARM64
+ */
+ protected void createArm64UsbDef(DevicesDef devices) {
+ devices.addDevice(new InputDef(KEYBOARD, USB));
+ devices.addDevice(new InputDef(MOUSE, USB));
+ devices.addDevice(new LibvirtVMDef.USBDef((short)0, 0, 5, 0, 0));
+ }
- This way a >4.1 agent can work communicate a <=4.1 management server
+ protected InputDef createTabletInputDef() {
+ return new InputDef(TABLET, USB);
+ }
- This change is due to the overcommit feature in 4.2
- */
- if (vmTO.getMinSpeed() != null) {
- ctd.setShares(vmTO.getCpus() * vmTO.getMinSpeed());
- } else {
- ctd.setShares(vmTO.getCpus() * vmTO.getSpeed());
- }
+ /**
+ * Add the VNC port passwd here, get the passwd from the vmInstance.
+ */
+ protected GraphicDef createGraphicDef(VirtualMachineTO vmTO) {
+ return new GraphicDef(VNC, (short)0, true, vmTO.getVncAddr(), vmTO.getVncPassword(), null);
+ }
- setQuotaAndPeriod(vmTO, ctd);
+ /**
+ * Add a VirtIO channel for the Qemu Guest Agent tools.
+ */
+ protected ChannelDef createChannelDef(VirtualMachineTO vmTO) {
+ File virtIoChannel = Paths.get(_qemuSocketsPath.getPath(), vmTO.getName() + "." + _qemuGuestAgentSocketName).toFile();
+ return new ChannelDef(_qemuGuestAgentSocketName, ChannelDef.ChannelType.UNIX, virtIoChannel);
+ }
- vm.addComp(ctd);
- }
+ /**
+ * If we're using virtio scsi, then we need to add a virtual scsi controller.
+ */
+ protected SCSIDef createSCSIDef(int vcpus) {
+ return new SCSIDef((short)0, 0, 0, 9, 0, vcpus);
+ }
- final FeaturesDef features = new FeaturesDef();
- features.addFeatures("pae");
- features.addFeatures("apic");
- features.addFeatures("acpi");
- if (isUefiEnabled && isSecureMode(customParams.get(GuestDef.BootType.UEFI.toString()))) {
- features.addFeatures("smm");
- }
+ protected ConsoleDef createConsoleDef() {
+ return new ConsoleDef(PTY, null, null, (short)0);
+ }
- //KVM hyperv enlightenment features based on OS Type
- enlightenWindowsVm(vmTO, features);
+ protected VideoDef createVideoDef() {
+ return new VideoDef(_videoHw, _videoRam);
+ }
- vm.addComp(features);
+ protected RngDef createRngDef() {
+ return new RngDef(_rngPath, _rngBackendModel, _rngRateBytes, _rngRatePeriod);
+ }
- final TermPolicy term = new TermPolicy();
- term.setCrashPolicy("destroy");
- term.setPowerOffPolicy("destroy");
- term.setRebootPolicy("restart");
- vm.addComp(term);
+ protected SerialDef createSerialDef() {
+ return new SerialDef(PTY, null, (short)0);
+ }
- final ClockDef clock = new ClockDef();
- if (vmTO.getOs().startsWith("Windows")) {
+ protected ClockDef createClockDef(final VirtualMachineTO vmTO) {
+ ClockDef clock = new ClockDef();
+ if (org.apache.commons.lang.StringUtils.startsWith(vmTO.getOs(), WINDOWS)) {
clock.setClockOffset(ClockDef.ClockOffset.LOCALTIME);
- clock.setTimer("hypervclock", null, null);
- } else if (vmTO.getType() != VirtualMachine.Type.User || isGuestPVEnabled(vmTO.getOs())) {
- if (_hypervisorLibvirtVersion >= 9 * 1000 + 10) {
- clock.setTimer("kvmclock", null, null, _noKvmClock);
- }
+ clock.setTimer(HYPERVCLOCK, null, null);
+ } else if ((vmTO.getType() != VirtualMachine.Type.User || isGuestPVEnabled(vmTO.getOs())) && _hypervisorLibvirtVersion >= MIN_LIBVIRT_VERSION_FOR_GUEST_CPU_MODE) {
Review comment:
I can't understand what would be the use of extracting this if for a method, considering that it is not used anywhere else. Could you explain your point?
--
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