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/06/23 16:20:02 UTC
[GitHub] [cloudstack] SadiJr opened a new pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
SadiJr opened a new pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149
### Description
This PR goal is to refactor the method createVMFromSpec(final VirtualMachineTO vmTO) in class com.cloud.hypervisor.kvm.resource.LibvirtComputingResource and improve its logs, javadocs, while adding unit tests for all of the code that is extracted and modified.
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [x] Cleanup (Code refactoring and cleanup, that may add test cases)
### How Has This Been Tested?
All unit tests have been tested, and I've checked that the results on the main branch are the same of this refactor.
--
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:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
SadiJr commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-882732411
@DaanHoogland Thanks for the review.
I agree with your opinion about the LXC support, but I think is better to discuss this in an issue or an PR specific for it.
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-883896285
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 611
--
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
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [cloudstack] nvazquez commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-883017007
Looks really nice @SadiJr.
@davidjumani this PR will require some effort to test on KVM ensuring there are no regressions, let's try to find some time for testing it
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-867459457
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 351
--
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:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on a change in pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
SadiJr commented on a change in pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#discussion_r662556881
##########
File path: api/src/main/java/com/cloud/agent/api/to/VirtualMachineTO.java
##########
@@ -413,4 +413,9 @@ public DeployAsIsInfoTO getDeployAsIsInfo() {
public void setDeployAsIsInfo(DeployAsIsInfoTO deployAsIsInfo) {
this.deployAsIsInfo = deployAsIsInfo;
}
+
+ @Override
+ public String toString() {
+ return String.format("VM {\"id\": %s, \"name\": \"%s\", \"uuid\": \"%s\", \"type\": \"%s\"}", id, name, uuid, type);
Review comment:
Good observation, done.
--
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
[GitHub] [cloudstack] GutoVeronezi commented on a change in pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#discussion_r658003465
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -2239,211 +2305,303 @@ 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));
- }
- }
+ // Add extra configuration to User VM Domain XML before starting
+ 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);
+ }
Review comment:
We can extract this code to a method and turn the comment into a javadoc.
--
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:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
SadiJr commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-867698291
@DaanHoogland I literally debug the existing tests in main and checked the VM XML settings for each test. And I repeated this process on the branch where I made the changes to compare the results.
--
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:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
SadiJr commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-882732411
@DaanHoogland Thanks for the review.
I agree with your opinion about the LXC support, but I think is better to discuss this in an issue or an PR specific for it.
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-881685542
<b>Trillian test result (tid-1291)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37598 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5149-t1291-kvm-centos7.zip
Smoke tests completed. 88 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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
[GitHub] [cloudstack] DaanHoogland commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-868402660
> @DaanHoogland I literally debug the existing tests in main and checked the VM XML settings for each test. And I repeated this process on the branch where I made the changes to compare the results.
tnx, code looks good, but need testing.
--
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:
users@infra.apache.org
[GitHub] [cloudstack] SadiJr commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
SadiJr commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-872331355
@GabrielBrascher Thanks for the suggestions. Most have been implemented, although there is a suggestion that I don't quite understand how useful it is.
--
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
[GitHub] [cloudstack] nvazquez commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-883017007
Looks really nice @SadiJr.
@davidjumani this PR will require some effort to test on KVM ensuring there are no regressions, let's try to find some time for testing it
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-883883847
@davidjumani a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
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
[GitHub] [cloudstack] nvazquez commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-883017007
Looks really nice @SadiJr.
@davidjumani this PR will require some effort to test on KVM ensuring there are no regressions, let's try to find some time for testing it
--
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
[GitHub] [cloudstack] SadiJr commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
SadiJr commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-882732411
@DaanHoogland Thanks for the review.
I agree with your opinion about the LXC support, but I think is better to discuss this in an issue or an PR specific for it.
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-883899046
@davidjumani a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
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
[GitHub] [cloudstack] SadiJr commented on a change in pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
SadiJr commented on a change in pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#discussion_r658027142
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -2239,211 +2305,303 @@ 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));
- }
- }
+ // Add extra configuration to User VM Domain XML before starting
+ 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);
+ }
Review comment:
Done. Thanks for the tip.
--
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:
users@infra.apache.org
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#discussion_r661963262
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -209,6 +209,67 @@
public class LibvirtComputingResource extends ServerResourceBase implements ServerResource, VirtualRouterDeployer {
private static final Logger s_logger = Logger.getLogger(LibvirtComputingResource.class);
+ private static final String LEGACY = "legacy";
+ private static final String SECURE = "secure";
+
+ /**
+ * Machine type.
+ */
+ private static final String PC = "pc";
+ private static final String VIRT = "virt";
+
+ /**
+ * Possible devices to add to VM.
+ */
+ private static final String TABLET = "tablet";
+ private static final String USB = "usb";
+ private static final String MOUSE = "mouse";
+ private static final String KEYBOARD = "keyboard";
+
+ /**
+ * Policies used by VM.
+ */
+ private static final String RESTART = "restart";
+ private static final String DESTROY = "destroy";
+
+ private static final String KVMCLOCK = "kvmclock";
+ private static final String HYPERVCLOCK = "hypervclock";
+ private static final String WINDOWS = "Windows";
+ private static final String Q35 = "q35";
+ private static final String PTY = "pty";
+ private static final String VNC = "vnc";
+
+ /**
+ * Acronym of System Management Mode. Perform low-level system management operations while an OS is running.
+ */
+ private static final String SMM = "smm";
+ /**
+ * Acronym of Advanced Configuration and Power Interface.<br>
+ * Provides an open standard that operating systems can use to discover and configure
+ * computer hardware components, to perform power management.
+ */
+ private static final String ACPI = "acpi";
+ /**
+ * Acronym of Advanced Programmable Interrupt Controllers.<br>
+ * With an I/O APIC, operating systems can use more than 16 interrupt requests (IRQs)
+ * and therefore avoid IRQ sharing for improved reliability.
+ */
+ private static final String APIC = "apic";
+ /**
+ * Acronym of Physical Address Extension. Feature implemented in modern x86 processors.<br>
+ * PAE extends memory addressing capabilities, allowing more than 4 GB of random access memory (RAM) to be used.
+ */
+ private static final String PAE = "pae";
+ /**
+ * Libvirt only suport guest cpu mode in versions from 0.9.10.
+ */
+ private static final int MIN_LIBVIRT_VERSION_FOR_GUEST_CPU_MODE = 9010;
+ private static final int MIN_LIBVIRT_VERSION_FOR_GUEST_CPU_TUNE = 9000;
+ /**
+ * Verify if guest is ARM64
Review comment:
Verify if guest is ARM64
:arrow_down:
1. Verifies if the guest is ARM64
OR
2. Constant that defines ARM64 (aarch64) guest architectures.
##########
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) {
+ clock.setTimer(KVMCLOCK, null, null, _noKvmClock);
}
+ return clock;
+ }
- vm.addComp(clock);
+ protected TermPolicy createTermPolicy() {
+ TermPolicy term = new TermPolicy();
+ term.setCrashPolicy(DESTROY);
+ term.setPowerOffPolicy(DESTROY);
+ term.setRebootPolicy(RESTART);
+ return term;
+ }
- final DevicesDef devices = new DevicesDef();
- devices.setEmulatorPath(_hypervisorPath);
- devices.setGuestType(guest.getGuestType());
+ protected FeaturesDef createFeaturesDef(Map<String, String> customParams, boolean isUefiEnabled, boolean isSecureBoot) {
+ FeaturesDef features = new FeaturesDef();
+ features.addFeatures(PAE);
+ features.addFeatures(APIC);
+ features.addFeatures(ACPI);
+ if (isUefiEnabled && isSecureBoot) {
+ features.addFeatures(SMM);
+ }
+ return features;
+ }
- final SerialDef serial = new SerialDef("pty", null, (short)0);
- devices.addDevice(serial);
+ /**
+ * A 4.0.X/4.1.X management server doesn't send the correct JSON
+ * command for getMinSpeed, it only sends a 'speed' field.<br>
+ * So, to create a cpu tune, if getMinSpeed() returns null we fall back to getSpeed().<br>
+ * This way a >4.1 agent can work communicate a <=4.1 management server.<br>
+ * This change is due to the overcommit feature in 4.2.
+ */
+ protected CpuTuneDef createCpuTuneDef(VirtualMachineTO vmTO) {
+ CpuTuneDef ctd = new CpuTuneDef();
+ int shares = vmTO.getCpus() * (vmTO.getMinSpeed() != null ? vmTO.getMinSpeed() : vmTO.getSpeed());
+ ctd.setShares(shares);
+ setQuotaAndPeriod(vmTO, ctd);
+ return ctd;
+ }
- if (_rngEnable) {
- final RngDef rngDevice = new RngDef(_rngPath, _rngBackendModel, _rngRateBytes, _rngRatePeriod);
- devices.addDevice(rngDevice);
+ private CpuModeDef createCpuModeDef(VirtualMachineTO vmTO, int vcpus) {
+ final CpuModeDef cmd = new CpuModeDef();
+ cmd.setMode(_guestCpuMode);
+ cmd.setModel(_guestCpuModel);
+ if (VirtualMachine.Type.User.equals(vmTO.getType())) {
+ cmd.setFeatures(_cpuFeatures);
}
+ setCpuTopology(cmd, vcpus, vmTO.getDetails());
+ return cmd;
+ }
- /* Add a VirtIO channel for the Qemu Guest Agent tools */
- File virtIoChannel = Paths.get(_qemuSocketsPath.getPath(), vmTO.getName() + "." + _qemuGuestAgentSocketName).toFile();
- devices.addDevice(new ChannelDef(_qemuGuestAgentSocketName, ChannelDef.ChannelType.UNIX, virtIoChannel));
-
- devices.addDevice(new WatchDogDef(_watchDogAction, _watchDogModel));
+ /**
+ * Creates guest resources based in VM specification.
+ */
+ protected GuestResourceDef createGuestResourceDef(VirtualMachineTO vmTO) {
+ GuestResourceDef grd = new GuestResourceDef();
- final VideoDef videoCard = new VideoDef(_videoHw, _videoRam);
- devices.addDevice(videoCard);
+ grd.setMemorySize(vmTO.getMaxRam() / 1024);
+ if (vmTO.getMinRam() != vmTO.getMaxRam() && !_noMemBalloon) {
+ grd.setMemBalloning(true);
+ grd.setCurrentMem(vmTO.getMinRam() / 1024);
+ }
+ grd.setVcpuNum(vmTO.getCpus());
+ return grd;
+ }
- final ConsoleDef console = new ConsoleDef("pty", null, null, (short)0);
- devices.addDevice(console);
+ private void configureGuestIfUefiEnabled(boolean isSecureBoot, String bootMode, GuestDef guest) {
+ setGuestLoader(bootMode, SECURE, guest, GuestDef.GUEST_LOADER_SECURE);
+ setGuestLoader(bootMode, LEGACY, guest, GuestDef.GUEST_LOADER_LEGACY);
- //add the VNC port passwd here, get the passwd from the vmInstance.
- final String passwd = vmTO.getVncPassword();
- final GraphicDef grap = new GraphicDef("vnc", (short)0, true, vmTO.getVncAddr(), passwd, null);
- devices.addDevice(grap);
+ if (isUefiPropertieNotNull(GuestDef.GUEST_NVRAM_PATH)) {
+ guest.setNvram(_uefiProperties.getProperty(GuestDef.GUEST_NVRAM_PATH));
+ }
- final InputDef input = new InputDef("tablet", "usb");
- devices.addDevice(input);
+ if (isSecureBoot && isUefiPropertieNotNull(GuestDef.GUEST_NVRAM_TEMPLATE_SECURE) && SECURE.equalsIgnoreCase(bootMode)) {
+ guest.setNvramTemplate(_uefiProperties.getProperty(GuestDef.GUEST_NVRAM_TEMPLATE_SECURE));
+ } else if (isUefiPropertieNotNull(GuestDef.GUEST_NVRAM_TEMPLATE_LEGACY)) {
+ guest.setNvramTemplate(_uefiProperties.getProperty(GuestDef.GUEST_NVRAM_TEMPLATE_LEGACY));
+ }
+ }
- // Add an explicit USB devices for ARM64
- if (_guestCpuArch != null && _guestCpuArch.equals("aarch64")) {
- devices.addDevice(new InputDef("keyboard", "usb"));
- devices.addDevice(new InputDef("mouse", "usb"));
- devices.addDevice(new LibvirtVMDef.USBDef((short)0, 0, 5, 0, 0));
+ private void setGuestLoader(String bootMode, String mode, GuestDef guest, String propertie) {
+ if (isUefiPropertieNotNull(propertie) && mode.equalsIgnoreCase(bootMode)) {
+ guest.setLoader(_uefiProperties.getProperty(propertie));
}
+ }
- DiskDef.DiskBus busT = getDiskModelFromVMDetail(vmTO);
+ private boolean isUefiPropertieNotNull(String propertie) {
+ return _uefiProperties.getProperty(propertie) != null;
+ }
- if (busT == null) {
- busT = getGuestDiskModel(vmTO.getPlatformEmulator(), isUefiEnabled);
- }
+ private boolean isGuestAarch64() {
+ return AARCH64.equals(_guestCpuArch);
+ }
- // If we're using virtio scsi, then we need to add a virtual scsi controller
- if (busT == DiskDef.DiskBus.SCSI) {
- final SCSIDef sd = new SCSIDef((short)0, 0, 0, 9, 0, vcpus);
- devices.addDevice(sd);
- }
+ /**
+ * Create a guest definition from a VM specification.
Review comment:
Some documentations have the verb as Imperative (e.g. **create**), some Indicative (e.g. it **creates**).
This is a small observation, but maybe you can normalize them. (I would go to the indicative form; but feel free to choose)
##########
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:
This if is quite big, maybe extract it to a method?
##########
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()));
Review comment:
I think that `VirtualMachineTO` does not implement `toString`, in this case, the log would be something such as:
`DPDK is enabled for VM [com.cloud.agent.api.to.VirtualMachineTO@38588dea], but it ...`
Either:
1. this log needs to be changed, or
2. `VirtualMachineTO` needs a toString() implementation to override the `Object.toString()`
##########
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.
Review comment:
I think that this Javadoc does not entirely match with what is done (please correct me if I am wrong)
`Add the VNC port passwd here, get the passwd from the vmInstance.`
Suggestion :arrow_down:
`Given a VM (VirtualMachineTO), it creates a Libvirt Graphic Definition with the VM's password and VNC address.`
_OR simply:_
`Creates a Libvirt Graphic Definition with the VM's password and VNC address.`
##########
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.
Review comment:
`VirtIO` -> `Virtio`
See:
- https://wiki.libvirt.org/page/Virtio
- https://libvirt.org/formatdomain.html#virtio-transitional-devices
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -209,6 +209,67 @@
public class LibvirtComputingResource extends ServerResourceBase implements ServerResource, VirtualRouterDeployer {
private static final Logger s_logger = Logger.getLogger(LibvirtComputingResource.class);
+ private static final String LEGACY = "legacy";
+ private static final String SECURE = "secure";
+
+ /**
+ * Machine type.
+ */
+ private static final String PC = "pc";
+ private static final String VIRT = "virt";
+
+ /**
+ * Possible devices to add to VM.
+ */
+ private static final String TABLET = "tablet";
+ private static final String USB = "usb";
+ private static final String MOUSE = "mouse";
+ private static final String KEYBOARD = "keyboard";
+
+ /**
+ * Policies used by VM.
+ */
+ private static final String RESTART = "restart";
+ private static final String DESTROY = "destroy";
+
+ private static final String KVMCLOCK = "kvmclock";
+ private static final String HYPERVCLOCK = "hypervclock";
+ private static final String WINDOWS = "Windows";
+ private static final String Q35 = "q35";
+ private static final String PTY = "pty";
+ private static final String VNC = "vnc";
+
+ /**
+ * Acronym of System Management Mode. Perform low-level system management operations while an OS is running.
+ */
+ private static final String SMM = "smm";
+ /**
+ * Acronym of Advanced Configuration and Power Interface.<br>
+ * Provides an open standard that operating systems can use to discover and configure
+ * computer hardware components, to perform power management.
+ */
+ private static final String ACPI = "acpi";
+ /**
+ * Acronym of Advanced Programmable Interrupt Controllers.<br>
+ * With an I/O APIC, operating systems can use more than 16 interrupt requests (IRQs)
+ * and therefore avoid IRQ sharing for improved reliability.
+ */
+ private static final String APIC = "apic";
+ /**
+ * Acronym of Physical Address Extension. Feature implemented in modern x86 processors.<br>
+ * PAE extends memory addressing capabilities, allowing more than 4 GB of random access memory (RAM) to be used.
+ */
+ private static final String PAE = "pae";
+ /**
+ * Libvirt only suport guest cpu mode in versions from 0.9.10.
+ */
+ private static final int MIN_LIBVIRT_VERSION_FOR_GUEST_CPU_MODE = 9010;
+ private static final int MIN_LIBVIRT_VERSION_FOR_GUEST_CPU_TUNE = 9000;
Review comment:
/**
* The CPU tune element provides details of the CPU tunable parameters for the domain.<br>
* It is supported since Libvirt 0.9.0
*/
##########
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.
+ */
Review comment:
I would suggest changing it to:
```
/**
* Creates Virtio SCSI controller.
*/
```
I am not sure if it is relevant to mention `If we're using virtio scsi, then we need to add a virtual scsi controller`. The method logic is restricted to creating a SCSI XML definition.
The logic that defines if it does create (or not) such definition is:
~~~
if (busT == DiskDef.DiskBus.SCSI) {
devices.addDevice(createSCSIDef(vcpus));
}
~~~
But if you think it still relevant, I would then suggest adding the respective SCSI validation to this method.
In such case, here follows a suggestion (if you think it is better) :-)
```
/**
* Creates Virtio SCSI controller. <br>
* The respective Virtio SCSI XML definition is generated only if the VM's Disk Bus is of ISCSI.
*/
```
##########
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()));
Review comment:
Another occurrence of `vmTO.toString()`, it needs to be checked.
##########
File path: plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
##########
@@ -209,6 +209,67 @@
public class LibvirtComputingResource extends ServerResourceBase implements ServerResource, VirtualRouterDeployer {
private static final Logger s_logger = Logger.getLogger(LibvirtComputingResource.class);
+ private static final String LEGACY = "legacy";
+ private static final String SECURE = "secure";
+
+ /**
+ * Machine type.
+ */
+ private static final String PC = "pc";
+ private static final String VIRT = "virt";
+
+ /**
+ * Possible devices to add to VM.
+ */
+ private static final String TABLET = "tablet";
+ private static final String USB = "usb";
+ private static final String MOUSE = "mouse";
+ private static final String KEYBOARD = "keyboard";
+
+ /**
+ * Policies used by VM.
+ */
+ private static final String RESTART = "restart";
+ private static final String DESTROY = "destroy";
+
+ private static final String KVMCLOCK = "kvmclock";
+ private static final String HYPERVCLOCK = "hypervclock";
+ private static final String WINDOWS = "Windows";
+ private static final String Q35 = "q35";
+ private static final String PTY = "pty";
+ private static final String VNC = "vnc";
+
+ /**
+ * Acronym of System Management Mode. Perform low-level system management operations while an OS is running.
+ */
+ private static final String SMM = "smm";
+ /**
+ * Acronym of Advanced Configuration and Power Interface.<br>
+ * Provides an open standard that operating systems can use to discover and configure
+ * computer hardware components, to perform power management.
+ */
+ private static final String ACPI = "acpi";
+ /**
+ * Acronym of Advanced Programmable Interrupt Controllers.<br>
+ * With an I/O APIC, operating systems can use more than 16 interrupt requests (IRQs)
+ * and therefore avoid IRQ sharing for improved reliability.
+ */
+ private static final String APIC = "apic";
+ /**
+ * Acronym of Physical Address Extension. Feature implemented in modern x86 processors.<br>
+ * PAE extends memory addressing capabilities, allowing more than 4 GB of random access memory (RAM) to be used.
+ */
+ private static final String PAE = "pae";
+ /**
+ * Libvirt only suport guest cpu mode in versions from 0.9.10.
Review comment:
Libvirt only suport guest cpu mode in versions from 0.9.10.
:arrow_down:
Libvirt **supports** guest **CPU** mode since 0.9.10.
--
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
[GitHub] [cloudstack] SadiJr commented on a change in pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [cloudstack] davidjumani commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-883898813
@blueorangutan test keepEnv
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-881288091
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 564
--
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
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#discussion_r662495524
##########
File path: api/src/main/java/com/cloud/agent/api/to/VirtualMachineTO.java
##########
@@ -413,4 +413,9 @@ public DeployAsIsInfoTO getDeployAsIsInfo() {
public void setDeployAsIsInfo(DeployAsIsInfoTO deployAsIsInfo) {
this.deployAsIsInfo = deployAsIsInfo;
}
+
+ @Override
+ public String toString() {
+ return String.format("VM {\"id\": %s, \"name\": \"%s\", \"uuid\": \"%s\", \"type\": \"%s\"}", id, name, uuid, type);
Review comment:
A "cosmetic" observation/question.
This way it will print: `VM {"id": 123, "name": my VM, "uuid": 73ec..uuid, "type": User}`
Wouldn't the idea be printing the following?
`VM {id: "123", name: "my VM", uuid: "73ec..uuid", type: "User"}`
If the second option is desired then it would need to change to:
`return String.format("VM {id: \"%s\", name: \"%s\", uuid: \"%s\", type: \"%s\"}", id, name, uuid, type);`
--
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
[GitHub] [cloudstack] GabrielBrascher commented on a change in pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
GabrielBrascher commented on a change in pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#discussion_r662457986
##########
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:
That's a good question @SadiJr. In fact, some code extractions have little to do with duplicated code but with making cleaner code and also helping with unit test coverage.
For instance, there are cases where conditionals can be replaced by polymorphism. In some cases, it is also possible to replace a conditional with map/set. As an example:
```
if (var == value1 || var == value2 || ... var == valueN) {
<body>
}
```
:arrow_down:
```
/**
* This JavaDoc might help. But if the variable name is self-explanatory this Javadoc might be unnecessary.
*/
final static Set<Type> GOOD_VAR_NAME = new HashSet<>(Arrays.asList(value1, value2, ..., valueN));
....
protected void method() {
....
if (GOOD_VAR_NAME.contains(var)) {
<body>
}
}
```
Therefore, the goal of such extraction would be to:
1. keep code clean
2. add documentation (or not)
3. allow easy JUnit test that covers such if-conditional
You can proceed with the code as it is or look for some alternatives that might improve this IF. I would not block or judge if you keep it as it is. What matters the most is to always try to keep such care when developing/reviewing.
--
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
[GitHub] [cloudstack] nvazquez merged pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
nvazquez merged pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149
--
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
[GitHub] [cloudstack] SadiJr commented on a change in pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
SadiJr commented on a change in pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#discussion_r662468353
##########
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:
Now I get your point. However, while I don't see any problem making this change, I also don't see the need to make it, so I'll keep the code as is. But thanks for the clarification, it was a great explanation.
--
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
[GitHub] [cloudstack] blueorangutan commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-884376204
<b>Trillian test result (tid-1332)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43232 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5149-t1332-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 88 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 571.39 | test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 422.31 | test_vpc_redundant.py
--
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
[GitHub] [cloudstack] davidjumani commented on pull request #5149: Refactor and improvements for method com.cloud.hypervisor.kvm.resource.LibvirtComputingResource.createVMFromSpec()
Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #5149:
URL: https://github.com/apache/cloudstack/pull/5149#issuecomment-883883602
@blueorangutan package
--
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