Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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,12 @@
-- 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.


-- Delete template details for guest CPU mode/model which are root admin only
DELETE FROM `cloud`.`vm_template_details` WHERE `name` IN ('guest.cpu.mode','guest.cpu.model');

-- 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;
}

protected 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,82 @@ 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(this.version);;
}
}

public TpmModel getModel() {
return model;
}

public TpmVersion getVersion() {
return version;
}

@Override
public String toString() {
StringBuilder tpmBuidler = new StringBuilder();
if (model != null) {
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 @@ -6541,4 +6541,28 @@ public void testGetDiskModelFromVMDetailVirtioBlk() {
DiskDef.DiskBus diskBus = libvirtComputingResourceSpy.getDiskModelFromVMDetail(virtualMachineTO);
assertEquals(DiskDef.DiskBus.VIRTIOBLK, diskBus);
}

@Test
public void testCreateTpmDef() {
VirtualMachineTO virtualMachineTO = Mockito.mock(VirtualMachineTO.class);
Map<String, String> details = new HashMap<>();
details.put(VmDetailConstants.VIRTUAL_TPM_MODEL, "tpm-tis");
details.put(VmDetailConstants.VIRTUAL_TPM_VERSION, "2.0");
Mockito.when(virtualMachineTO.getDetails()).thenReturn(details);
LibvirtVMDef.TpmDef tpmDef = libvirtComputingResourceSpy.createTpmDef(virtualMachineTO);
assertEquals(LibvirtVMDef.TpmDef.TpmModel.TIS, tpmDef.getModel());
assertEquals(LibvirtVMDef.TpmDef.TpmVersion.V2_0, tpmDef.getVersion());
}

@Test
public void testCreateTpmDefWithInvalidVersion() {
VirtualMachineTO virtualMachineTO = Mockito.mock(VirtualMachineTO.class);
Map<String, String> details = new HashMap<>();
details.put(VmDetailConstants.VIRTUAL_TPM_MODEL, "tpm-crb");
details.put(VmDetailConstants.VIRTUAL_TPM_VERSION, "3.0");
Mockito.when(virtualMachineTO.getDetails()).thenReturn(details);
LibvirtVMDef.TpmDef tpmDef = libvirtComputingResourceSpy.createTpmDef(virtualMachineTO);
assertEquals(LibvirtVMDef.TpmDef.TpmModel.CRB, tpmDef.getModel());
assertEquals(LibvirtVMDef.TpmDef.TpmVersion.V2_0, tpmDef.getVersion());
}
}
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);

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,57 @@ 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) throws Exception {
String virtualTPMEnabled = vmSpec.getDetails().getOrDefault(VmDetailConstants.VIRTUAL_TPM_ENABLED, null);
if (Boolean.parseBoolean(virtualTPMEnabled)) {
for (VirtualDevice device : vmMo.getAllDeviceList()) {
if (device instanceof VirtualTPM) {
logger.debug(String.format("Virtual TPM device has already been added to VM %s, returning", vmMo.getVmName()));
return;
}
}
logger.debug(String.format("Adding Virtual TPM device to the VM %s", vmMo.getVmName()));
addVirtualTPMDevice(vmConfigSpec);
} else if (virtualTPMEnabled == null) {
logger.debug(String.format("Virtual TPM device is neither enabled nor disabled for VM %s, skipping", vmMo.getVmName()));
} else {
logger.debug(String.format("Virtual TPM device is disabled for VM %s", vmMo.getVmName()));
for (VirtualDevice device : vmMo.getAllDeviceList()) {
if (device instanceof VirtualTPM) {
logger.debug(String.format("Removing Virtual TPM device from VM %s as it is disabled", vmMo.getVmName()));
removeVirtualTPMDevice(vmConfigSpec, (VirtualTPM) device);
}
}
}
}

protected void addVirtualTPMDevice(VirtualMachineConfigSpec vmConfigSpec) {
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);
vmConfigSpec.getDeviceChange().add(deviceConfigSpec);
}

protected void removeVirtualTPMDevice(VirtualMachineConfigSpec vmConfigSpec, VirtualTPM virtualTPM) {
VirtualDeviceConfigSpec virtualDeviceConfigSpec = new VirtualDeviceConfigSpec();
virtualDeviceConfigSpec.setDevice(virtualTPM);
virtualDeviceConfigSpec.setOperation(VirtualDeviceConfigSpecOperation.REMOVE);
vmConfigSpec.getDeviceChange().add(virtualDeviceConfigSpec);
}

private void tearDownVm(VirtualMachineMO vmMo) throws Exception {

if (vmMo == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@
import com.vmware.vim25.FileInfo;
import com.vmware.vim25.HostDatastoreBrowserSearchResults;
import com.vmware.vim25.HostDatastoreBrowserSearchSpec;
import com.vmware.vim25.VirtualTPM;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.storage.command.CopyCommand;
import org.apache.cloudstack.storage.command.browser.ListDataStoreObjectsAnswer;
import org.apache.cloudstack.storage.command.browser.ListDataStoreObjectsCommand;
Expand Down Expand Up @@ -842,4 +844,41 @@ public void testExecuteWithEmptyPath() throws Exception {
assertEquals(Collections.singletonList(1L), answer.getSizes());
assertEquals(Collections.singletonList(date.getTime()), answer.getLastModified());
}

@Test
public void testAddVirtualTPMDevice() throws Exception {
VirtualMachineMO vmMo = Mockito.mock(VirtualMachineMO.class);
VirtualMachineTO vmSpec = Mockito.mock(VirtualMachineTO.class);
VirtualMachineConfigSpec vmConfigSpec = Mockito.mock(VirtualMachineConfigSpec.class);
Map<String, String> details = new HashMap<>();
details.put(ApiConstants.BootType.UEFI.toString(), "SECURE");
details.put(VmDetailConstants.VIRTUAL_TPM_ENABLED, "true");
when(vmSpec.getDetails()).thenReturn(details);
when(vmMo.getAllDeviceList()).thenReturn(new ArrayList<>());
List<VirtualDeviceConfigSpec> deviceChanges = Mockito.mock(List.class);
when(vmConfigSpec.getDeviceChange()).thenReturn(deviceChanges);

vmwareResource.configureVirtualTPM(vmMo, vmSpec, vmConfigSpec);
Mockito.verify(vmwareResource, Mockito.times(1)).addVirtualTPMDevice(vmConfigSpec);
Mockito.verify(deviceChanges, Mockito.times(1)).add(any(VirtualDeviceConfigSpec.class));
}

@Test
public void testRemoveVirtualTPMDevice() throws Exception {
VirtualMachineMO vmMo = Mockito.mock(VirtualMachineMO.class);
VirtualMachineTO vmSpec = Mockito.mock(VirtualMachineTO.class);
VirtualMachineConfigSpec vmConfigSpec = Mockito.mock(VirtualMachineConfigSpec.class);
Map<String, String> details = new HashMap<>();
details.put(ApiConstants.BootType.UEFI.toString(), "SECURE");
details.put(VmDetailConstants.VIRTUAL_TPM_ENABLED, "false");
when(vmSpec.getDetails()).thenReturn(details);
VirtualTPM tpm = new VirtualTPM();
when(vmMo.getAllDeviceList()).thenReturn(List.of(tpm));
List<VirtualDeviceConfigSpec> deviceChanges = Mockito.mock(List.class);
when(vmConfigSpec.getDeviceChange()).thenReturn(deviceChanges);

vmwareResource.configureVirtualTPM(vmMo, vmSpec, vmConfigSpec);
Mockito.verify(vmwareResource, Mockito.times(1)).removeVirtualTPMDevice(vmConfigSpec, tpm);
Mockito.verify(deviceChanges, Mockito.times(1)).add(any(VirtualDeviceConfigSpec.class));
}
}
Loading