Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/src/main/java/com/cloud/vm/VmDetailConstants.java
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,13 @@ public interface VmDetailConstants {
String VMWARE_HOST_NAME = String.format("%s-host", VMWARE_TO_KVM_PREFIX);
String VMWARE_DISK = String.format("%s-disk", VMWARE_TO_KVM_PREFIX);
String VMWARE_MAC_ADDRESSES = String.format("%s-mac-addresses", VMWARE_TO_KVM_PREFIX);

// TPM
String VIRTUAL_TPM_ENABLED = "virtual.tpm.enabled";
String VIRTUAL_TPM_MODEL = "virtual.tpm.model";
String VIRTUAL_TPM_VERSION = "virtual.tpm.version";

// CPU mode and model, ADMIN only
String GUEST_CPU_MODE = "guest.cpu.mode";
String GUEST_CPU_MODEL = "guest.cpu.model";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can non-admin user deploy vTPM enabled instance without these settings? any other way for the normal user to provide these options, from service offering, etc?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently no.
I am thinking of adding global/domain/account settings for both.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, wait, these settings are also available for templates. But again, only available for admin. I will test it

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, these two settings are available for templates.
but only root admin can add

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
// under the License.
package org.apache.cloudstack.query;

import java.util.Arrays;
import java.util.List;

import org.apache.cloudstack.affinity.AffinityGroupResponse;
Expand Down Expand Up @@ -97,13 +98,16 @@
import org.apache.cloudstack.framework.config.ConfigKey;

import com.cloud.exception.PermissionDeniedException;
import com.cloud.vm.VmDetailConstants;

/**
* Service used for list api query.
*
*/
public interface QueryService {

List<String> RootAdminOnlyVmSettings = Arrays.asList(VmDetailConstants.GUEST_CPU_MODE, VmDetailConstants.GUEST_CPU_MODEL);

// Config keys
ConfigKey<Boolean> AllowUserViewDestroyedVM = new ConfigKey<>("Advanced", Boolean.class, "allow.user.view.destroyed.vm", "false",
"Determines whether users can view their destroyed or expunging vm ", true, ConfigKey.Scope.Account);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
-- Schema upgrade from 4.20.0.0 to 4.20.1.0
--;

-- Delete user vm details for guest CPU mode/model which are root admin only
DELETE FROM `cloud`.`user_vm_details` WHERE `name` IN ('guest.cpu.mode','guest.cpu.model');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@weizhouapache will this impact any VMs in existing deployments with these settings?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two settings have no impact in older versions, but are only available for root admin with this PR (because I think the host CPU is sensitive information).

I think it is better to remove them during upgrade. Otherwise user can add the settings before upgrade, and get the Host CPU after upgrade.


-- Add column api_key_access to user and account tables
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.user', 'api_key_access', 'boolean DEFAULT NULL COMMENT "is api key access allowed for the user" AFTER `secret_key`');
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.account', 'api_key_access', 'boolean DEFAULT NULL COMMENT "is api key access allowed for the account" ');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.SCSIDef;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.SerialDef;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.TermPolicy;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.TpmDef;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.VideoDef;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.WatchDogDef;
import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.WatchDogDef.WatchDogAction;
Expand Down Expand Up @@ -2660,6 +2661,11 @@ protected DevicesDef createDevicesDef(VirtualMachineTO vmTO, GuestDef guest, int
devices.addDevice(createGraphicDef(vmTO));
devices.addDevice(createTabletInputDef());

TpmDef tpmDef = createTpmDef(vmTO);
if (tpmDef != null) {
devices.addDevice(tpmDef);
}

if (isGuestAarch64()) {
createArm64UsbDef(devices);
}
Expand Down Expand Up @@ -2840,8 +2846,11 @@ public int calculateCpuShares(VirtualMachineTO vmTO) {

private CpuModeDef createCpuModeDef(VirtualMachineTO vmTO, int vcpus) {
final CpuModeDef cmd = new CpuModeDef();
cmd.setMode(guestCpuMode);
cmd.setModel(guestCpuModel);
Map<String, String> details = vmTO.getDetails();
String cpuMode = MapUtils.isNotEmpty(details) && details.get(VmDetailConstants.GUEST_CPU_MODE) != null ? details.get(VmDetailConstants.GUEST_CPU_MODE) : guestCpuMode;
String cpuModel = MapUtils.isNotEmpty(details) && details.get(VmDetailConstants.GUEST_CPU_MODEL) != null ? details.get(VmDetailConstants.GUEST_CPU_MODEL) : guestCpuModel;
cmd.setMode(cpuMode);
cmd.setModel(cpuModel);
if (VirtualMachine.Type.User.equals(vmTO.getType())) {
cmd.setFeatures(cpuFeatures);
}
Expand All @@ -2850,6 +2859,19 @@ private CpuModeDef createCpuModeDef(VirtualMachineTO vmTO, int vcpus) {
return cmd;
}

private TpmDef createTpmDef(VirtualMachineTO vmTO) {
Map<String, String> details = vmTO.getDetails();
if (MapUtils.isEmpty(details)) {
return null;
}
String tpmModel = details.get(VmDetailConstants.VIRTUAL_TPM_MODEL);
if (tpmModel == null) {
return null;
}
String tpmVersion = details.get(VmDetailConstants.VIRTUAL_TPM_VERSION);
return new TpmDef(tpmModel, tpmVersion);
}

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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

import java.io.File;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -2358,6 +2359,80 @@ public String toString() {
}
}

public static class TpmDef {
enum TpmModel {
TIS("tpm-tis"), // TPM Interface Specification (TIS)
CRB("tpm-crb"); // Command-Response Buffer (CRB)

final String model;

TpmModel(String model) {
this.model = model;
}

@Override
public String toString() {
return model;
}
}

enum TpmVersion {
V1_2("1.2"), // 1.2
V2_0("2.0"); // 2.0. Default version. The CRB model is only supported with version 2.0.

final String version;

TpmVersion(String version) {
this.version = version;
}

@Override
public String toString() {
return version;
}
}

private TpmModel model;
private TpmVersion version = TpmVersion.V2_0;

public TpmDef(TpmModel model, TpmVersion version) {
this.model = model;
if (version != null) {
this.version = version;
}
}

public TpmDef(String model, String version) {
this.model = Arrays.stream(TpmModel.values())
.filter(tpmModel -> tpmModel.toString().equals(model))
.findFirst()
.orElse(null);
if (version != null) {
this.version = Arrays.stream(TpmVersion.values())
.filter(tpmVersion -> tpmVersion.toString().equals(version))
.findFirst()
.orElse(null);;
}
}

public TpmModel getModel() {
return model;
}

public TpmVersion getVersion() {
return version;
}

@Override
public String toString() {
StringBuilder tpmBuidler = new StringBuilder();
tpmBuidler.append("<tpm model='").append(model).append("'>\n");
tpmBuidler.append("<backend type='emulator' version='").append(version).append("'/>\n");
tpmBuidler.append("</tpm>\n");
return tpmBuidler.toString();
}
}

public void setHvsType(String hvs) {
_hvsType = hvs;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -566,9 +566,20 @@ public void testTopology() {
assertEquals("<cpu><topology sockets='4' cores='2' threads='1' /></cpu>", cpuModeDef.toString());
}

@Test
public void testTopologyNoInfo() {
LibvirtVMDef.CpuModeDef cpuModeDef = new LibvirtVMDef.CpuModeDef();
cpuModeDef.setTopology(-1, -1, 4);
assertEquals("<cpu></cpu>", cpuModeDef.toString());
}

@Test
public void testTpmModel() {
LibvirtVMDef.TpmDef tpmDef = new LibvirtVMDef.TpmDef("tpm-tis", "2.0");
assertEquals(LibvirtVMDef.TpmDef.TpmModel.TIS, tpmDef.getModel());
assertEquals(LibvirtVMDef.TpmDef.TpmVersion.V2_0, tpmDef.getVersion());
assertEquals("<tpm model='tpm-tis'>\n" +
"<backend type='emulator' version='2.0'/>\n" +
"</tpm>\n", tpmDef.toString());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,15 @@

import com.cloud.capacity.CapacityManager;
import com.cloud.hypervisor.vmware.mo.HostDatastoreBrowserMO;
import com.vmware.vim25.Description;
import com.vmware.vim25.FileInfo;
import com.vmware.vim25.FileQueryFlags;
import com.vmware.vim25.FolderFileInfo;
import com.vmware.vim25.HostDatastoreBrowserSearchResults;
import com.vmware.vim25.HostDatastoreBrowserSearchSpec;
import com.vmware.vim25.VirtualCdromIsoBackingInfo;
import com.vmware.vim25.VirtualMachineConfigSummary;
import com.vmware.vim25.VirtualTPM;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.backup.PrepareForBackupRestorationCommand;
import org.apache.cloudstack.storage.command.CopyCommand;
Expand Down Expand Up @@ -2597,12 +2599,16 @@ protected StartAnswer execute(StartCommand cmd) {

setBootOptions(vmSpec, bootMode, vmConfigSpec);

// Config vTPM
configureVirtualTPM(vmMo, vmSpec, vmConfigSpec, bootMode);

if (StringUtils.isNotEmpty(vmStoragePolicyId)) {
vmConfigSpec.getVmProfile().add(vmProfileSpec);
if (logger.isTraceEnabled()) {
logger.trace(String.format("Configuring the VM %s with storage policy: %s", vmInternalCSName, vmStoragePolicyId));
}
}

//
// Configure VM
//
Expand Down Expand Up @@ -3203,6 +3209,50 @@ protected void configureSpecVideoCardNewVRamSize(VirtualMachineVideoCard videoCa
vmConfigSpec.getDeviceChange().add(arrayVideoCardConfigSpecs);
}

/**
* Add or Remove virtual TPM module
*
* @param vmMo virtual machine mo
* @param vmSpec virtual machine specs
* @param vmConfigSpec virtual machine config spec
* @throws Exception exception
*/
protected void configureVirtualTPM(VirtualMachineMO vmMo, VirtualMachineTO vmSpec, VirtualMachineConfigSpec vmConfigSpec, String bootMode) throws Exception {
String bootType = vmSpec.getDetails().getOrDefault(ApiConstants.BootType.UEFI.toString(), null);
String virtualTPMEnabled = vmSpec.getDetails().getOrDefault(VmDetailConstants.VIRTUAL_TPM_ENABLED, null);
if (StringUtils.isNotBlank(bootMode) && !bootMode.equalsIgnoreCase("bios")
&& "secure".equalsIgnoreCase(bootType)
&& Boolean.parseBoolean(virtualTPMEnabled)) {
logger.debug("Adding Virtual TPM device");
for (VirtualDevice device : vmMo.getAllDeviceList()) {
if (device instanceof VirtualTPM) {
return;
}
}
Description description = new Description();
description.setSummary("Trusted Platform Module");
description.setLabel("Trusted Platform Module");
VirtualTPM virtualTPM = new VirtualTPM();
virtualTPM.setDeviceInfo(description);
VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec();
deviceConfigSpec.setDevice(virtualTPM);
deviceConfigSpec.setOperation(VirtualDeviceConfigSpecOperation.ADD);
Comment thread
weizhouapache marked this conversation as resolved.
Outdated
vmConfigSpec.getDeviceChange().add(deviceConfigSpec);
} else {
logger.debug(String.format("Virtual TPM device is not enabled. It is only enabled when boot type is SECURE (actually %s) and vTPM is enabled (actually %s)", bootType, virtualTPMEnabled));
for (VirtualDevice device : vmMo.getAllDeviceList()) {
if (device instanceof VirtualTPM) {
VirtualTPM virtualTPM = (VirtualTPM) device;
VirtualDeviceConfigSpec virtualDeviceConfigSpec = new VirtualDeviceConfigSpec();
virtualDeviceConfigSpec.setDevice(virtualTPM);
virtualDeviceConfigSpec.setOperation(VirtualDeviceConfigSpecOperation.REMOVE);
vmConfigSpec.getDeviceChange().add(virtualDeviceConfigSpec);
}
}
Comment thread
weizhouapache marked this conversation as resolved.

}
}

private void tearDownVm(VirtualMachineMO vmMo) throws Exception {

if (vmMo == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5012,6 +5012,7 @@ public DetailOptionsResponse listDetailOptions(final ListDetailOptionsCmd cmd) {
final List<String> userDenyListedSettings = Stream.of(QueryService.UserVMDeniedDetails.value().split(","))
.map(item -> (item).trim())
.collect(Collectors.toList());
userDenyListedSettings.addAll(QueryService.RootAdminOnlyVmSettings);
for (final String detail : userDenyListedSettings) {
if (options.containsKey(detail)) {
options.remove(detail);
Expand Down Expand Up @@ -5062,6 +5063,10 @@ private void fillVMOrTemplateDetailOptions(final Map<String, List<String>> optio
options.put(VmDetailConstants.IOTHREADS, Arrays.asList("enabled"));
options.put(VmDetailConstants.NIC_MULTIQUEUE_NUMBER, Collections.emptyList());
options.put(VmDetailConstants.NIC_PACKED_VIRTQUEUES_ENABLED, Arrays.asList("true", "false"));
options.put(VmDetailConstants.VIRTUAL_TPM_MODEL, Arrays.asList("tpm-tis", "tpm-crb"));
options.put(VmDetailConstants.VIRTUAL_TPM_VERSION, Arrays.asList("1.2", "2.0"));
options.put(VmDetailConstants.GUEST_CPU_MODE, Arrays.asList("custom", "host-model", "host-passthrough"));
options.put(VmDetailConstants.GUEST_CPU_MODEL, Collections.emptyList());
}

if (HypervisorType.VMware.equals(hypervisorType)) {
Expand All @@ -5071,6 +5076,7 @@ private void fillVMOrTemplateDetailOptions(final Map<String, List<String>> optio
options.put(VmDetailConstants.NESTED_VIRTUALIZATION_FLAG, Arrays.asList("true", "false"));
options.put(VmDetailConstants.SVGA_VRAM_SIZE, Collections.emptyList());
options.put(VmDetailConstants.RAM_RESERVATION, Collections.emptyList());
options.put(VmDetailConstants.VIRTUAL_TPM_ENABLED, Arrays.asList("true", "false"));
}
}

Expand Down
1 change: 1 addition & 0 deletions server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -2840,6 +2840,7 @@ public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx
final List<String> userDenyListedSettings = Stream.of(QueryService.UserVMDeniedDetails.value().split(","))
.map(item -> (item).trim())
.collect(Collectors.toList());
userDenyListedSettings.addAll(QueryService.RootAdminOnlyVmSettings);
final List<String> userReadOnlySettings = Stream.of(QueryService.UserVMReadOnlyDetails.value().split(","))
.map(item -> (item).trim())
.collect(Collectors.toList());
Expand Down