Skip to content

Commit d10d510

Browse files
shwstpprshwstppr
committed
kvm: allow skip forcing disk controller for Win VM with secure boot
A VM or template detail can be added with key `win.skip.force.disk.controller` and value `true` to allow skipping forcing DATA disk controller for the VM. Signed-off-by: Abhishek Kumar <[email protected]>
1 parent edd537f commit d10d510

File tree

3 files changed

+122
-13
lines changed

3 files changed

+122
-13
lines changed

‎api/src/main/java/com/cloud/vm/VmDetailConstants.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ public interface VmDetailConstants {
5353
String NIC_MULTIQUEUE_NUMBER = "nic.multiqueue.number";
5454
String NIC_PACKED_VIRTQUEUES_ENABLED = "nic.packed.virtqueues.enabled";
5555

56+
// KVM specific, disk controllers
57+
String KVM_WIN_SKIP_FORCE_DISK_CONTROLLER = "win.skip.force.disk.controller";
58+
5659
// Mac OSX guest specific (internal)
5760
String SMC_PRESENT = "smc.present";
5861
String FIRMWARE = "firmware";

‎plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java‎

Lines changed: 39 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@
100100
import org.xml.sax.InputSource;
101101
import org.xml.sax.SAXException;
102102

103-
104103
import com.cloud.agent.api.Answer;
105104
import com.cloud.agent.api.Command;
106105
import com.cloud.agent.api.HostVmStateReportEntry;
@@ -180,8 +179,8 @@
180179
import com.cloud.network.Networks.RouterPrivateIpStrategy;
181180
import com.cloud.network.Networks.TrafficType;
182181
import com.cloud.resource.AgentStatusUpdater;
183-
import com.cloud.resource.ResourceStatusUpdater;
184182
import com.cloud.resource.RequestWrapper;
183+
import com.cloud.resource.ResourceStatusUpdater;
185184
import com.cloud.resource.ServerResource;
186185
import com.cloud.resource.ServerResourceBase;
187186
import com.cloud.storage.JavaStorageLayer;
@@ -2995,6 +2994,42 @@ public static DiskDef.DiskType getDiskType(KVMPhysicalDisk physicalDisk) {
29952994
return useBLOCKDiskType(physicalDisk) ? DiskDef.DiskType.BLOCK : DiskDef.DiskType.FILE;
29962995
}
29972996

2997+
/**
2998+
* Defines the disk configuration for the default pool type based on the provided parameters.
2999+
* It determines the appropriate disk settings depending on whether the disk is a data disk, whether
3000+
* it's a Windows template, whether UEFI is enabled, and whether secure boot is active.
3001+
*
3002+
* @param disk The disk definition object that will be configured with the disk settings.
3003+
* @param volume The volume (disk) object, containing information about the type of disk.
3004+
* @param isWindowsTemplate Flag indicating whether the template is a Windows template.
3005+
* @param isUefiEnabled Flag indicating whether UEFI is enabled.
3006+
* @param isSecureBoot Flag indicating whether secure boot is enabled.
3007+
* @param physicalDisk The physical disk object that contains the path to the disk.
3008+
* @param devId The device ID for the disk.
3009+
* @param diskBusType The disk bus type to use if not skipping force disk controller.
3010+
* @param diskBusTypeData The disk bus type to use for data disks, if applicable.
3011+
* @param details A map of VM details containing additional configuration values, such as whether to skip force
3012+
* disk controller.
3013+
*/
3014+
protected void defineDiskForDefaultPoolType(DiskDef disk, DiskTO volume, boolean isWindowsTemplate,
3015+
boolean isUefiEnabled, boolean isSecureBoot, KVMPhysicalDisk physicalDisk, int devId,
3016+
DiskDef.DiskBus diskBusType, DiskDef.DiskBus diskBusTypeData, Map<String, String> details) {
3017+
boolean skipForceDiskController = BooleanUtils.toBoolean(details.get(
3018+
VmDetailConstants.KVM_WIN_SKIP_FORCE_DISK_CONTROLLER));
3019+
boolean isDataDiskWithoutForceController = volume.getType() == Volume.Type.DATADISK &&
3020+
(!(isWindowsTemplate && isUefiEnabled) || skipForceDiskController);
3021+
3022+
if (isDataDiskWithoutForceController) {
3023+
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusTypeData, DiskDef.DiskFmtType.QCOW2);
3024+
} else {
3025+
if (isSecureBoot) {
3026+
disk.defFileBasedDisk(physicalDisk.getPath(), devId, DiskDef.DiskFmtType.QCOW2, isWindowsTemplate);
3027+
} else {
3028+
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusType, DiskDef.DiskFmtType.QCOW2);
3029+
}
3030+
}
3031+
}
3032+
29983033
public void createVbd(final Connect conn, final VirtualMachineTO vmSpec, final String vmName, final LibvirtVMDef vm) throws InternalErrorException, LibvirtException, URISyntaxException {
29993034
final Map<String, String> details = vmSpec.getDetails();
30003035
final List<DiskTO> disks = Arrays.asList(vmSpec.getDisks());
@@ -3146,14 +3181,9 @@ public int compare(final DiskTO arg0, final DiskTO arg1) {
31463181
disk.setDiscard(DiscardType.UNMAP);
31473182
}
31483183
} else {
3149-
if (volume.getType() == Volume.Type.DATADISK && !(isWindowsTemplate && isUefiEnabled)) {
3150-
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusTypeData, DiskDef.DiskFmtType.QCOW2);
3151-
} else {
3152-
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusType, DiskDef.DiskFmtType.QCOW2);
3153-
}
3154-
3184+
defineDiskForDefaultPoolType(disk, volume, isWindowsTemplate, isUefiEnabled, isSecureBoot,
3185+
physicalDisk,devId, diskBusType, diskBusTypeData, details);
31553186
}
3156-
31573187
}
31583188

31593189
if (data instanceof VolumeObjectTO) {

‎plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java‎

Lines changed: 80 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,6 @@
5656
import javax.xml.xpath.XPathExpressionException;
5757
import javax.xml.xpath.XPathFactory;
5858

59-
import com.cloud.utils.net.NetUtils;
60-
61-
import com.cloud.vm.VmDetailConstants;
6259
import org.apache.cloudstack.api.ApiConstants.IoDriverPolicy;
6360
import org.apache.cloudstack.storage.command.AttachAnswer;
6461
import org.apache.cloudstack.storage.command.AttachCommand;
@@ -217,13 +214,15 @@
217214
import com.cloud.template.VirtualMachineTemplate.BootloaderType;
218215
import com.cloud.utils.Pair;
219216
import com.cloud.utils.exception.CloudRuntimeException;
220-
import com.cloud.utils.script.Script;
217+
import com.cloud.utils.net.NetUtils;
221218
import com.cloud.utils.script.OutputInterpreter.OneLineParser;
219+
import com.cloud.utils.script.Script;
222220
import com.cloud.utils.ssh.SshHelper;
223221
import com.cloud.vm.DiskProfile;
224222
import com.cloud.vm.VirtualMachine;
225223
import com.cloud.vm.VirtualMachine.PowerState;
226224
import com.cloud.vm.VirtualMachine.Type;
225+
import com.cloud.vm.VmDetailConstants;
227226

228227
@RunWith(MockitoJUnitRunner.class)
229228
public class LibvirtComputingResourceTest {
@@ -240,6 +239,19 @@ public class LibvirtComputingResourceTest {
240239
Connect connMock;
241240
@Mock
242241
LibvirtDomainXMLParser parserMock;
242+
@Mock
243+
private DiskDef diskDef;
244+
@Mock
245+
private DiskTO volume;
246+
@Mock
247+
private KVMPhysicalDisk physicalDisk;
248+
@Mock
249+
private Map<String, String> details;
250+
251+
private static final String PHYSICAL_DISK_PATH = "/path/to/disk";
252+
private static final int DEV_ID = 1;
253+
private static final DiskDef.DiskBus DISK_BUS_TYPE = DiskDef.DiskBus.VIRTIO;
254+
private static final DiskDef.DiskBus DISK_BUS_TYPE_DATA = DiskDef.DiskBus.SCSI;
243255

244256
@Spy
245257
private LibvirtComputingResource libvirtComputingResourceSpy = Mockito.spy(new LibvirtComputingResource());
@@ -6408,4 +6420,68 @@ public void createLinstorVdb() throws LibvirtException, InternalErrorException,
64086420
assertEquals(DiskDef.DiscardType.UNMAP, rootDisk.getDiscard());
64096421
}
64106422
}
6423+
6424+
@Test
6425+
public void testDefineDiskForDefaultPoolType_DataDiskNonWinSkipForceController() {
6426+
when(volume.getType()).thenReturn(Volume.Type.DATADISK);
6427+
when(details.get(VmDetailConstants.KVM_WIN_SKIP_FORCE_DISK_CONTROLLER)).thenReturn("true");
6428+
when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH);
6429+
boolean isWindowsTemplate = false;
6430+
boolean isUefiEnabled = false;
6431+
boolean isSecureBoot = false;
6432+
libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, isWindowsTemplate, isUefiEnabled, isSecureBoot,
6433+
physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details);
6434+
verify(diskDef, times(1)).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE_DATA, DiskDef.DiskFmtType.QCOW2);
6435+
}
6436+
6437+
@Test
6438+
public void testDefineDiskForDefaultPoolType_DataDiskWinSecuredSkipForceController() {
6439+
when(volume.getType()).thenReturn(Volume.Type.DATADISK);
6440+
when(details.get(VmDetailConstants.KVM_WIN_SKIP_FORCE_DISK_CONTROLLER)).thenReturn("true");
6441+
when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH);
6442+
boolean isWindowsTemplate = true;
6443+
boolean isUefiEnabled = true;
6444+
boolean isSecureBoot = true;
6445+
libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, isWindowsTemplate, isUefiEnabled, isSecureBoot,
6446+
physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details);
6447+
verify(diskDef, times(1)).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE_DATA, DiskDef.DiskFmtType.QCOW2);
6448+
}
6449+
6450+
@Test
6451+
public void testDefineDiskForDefaultPoolType_WinSecureBootEnabledForced() {
6452+
when(volume.getType()).thenReturn(Volume.Type.DATADISK);
6453+
when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH);
6454+
boolean isWindowsTemplate = true;
6455+
boolean isUefiEnabled = true;
6456+
boolean isSecureBoot = true;
6457+
libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, isWindowsTemplate, isUefiEnabled, isSecureBoot,
6458+
physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details);
6459+
verify(diskDef, times(1)).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DiskDef.DiskFmtType.QCOW2, isWindowsTemplate);
6460+
}
6461+
6462+
@Test
6463+
public void testDefineDiskForDefaultPoolType_SecureBootDisabledForced() {
6464+
when(volume.getType()).thenReturn(Volume.Type.DATADISK);
6465+
when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH);
6466+
boolean isWindowsTemplate = true;
6467+
boolean isUefiEnabled = false;
6468+
boolean isSecureBoot = false;
6469+
libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, isWindowsTemplate, isUefiEnabled, isSecureBoot,
6470+
physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details);
6471+
verify(diskDef, times(1)).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE_DATA, DiskDef.DiskFmtType.QCOW2);
6472+
}
6473+
6474+
@Test
6475+
public void testDefineDiskForDefaultPoolType_NotDataDisk() {
6476+
when(volume.getType()).thenReturn(Volume.Type.ROOT);
6477+
when(details.get(VmDetailConstants.KVM_WIN_SKIP_FORCE_DISK_CONTROLLER)).thenReturn("false");
6478+
when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH);
6479+
boolean isWindowsTemplate = false;
6480+
boolean isUefiEnabled = false;
6481+
boolean isSecureBoot = false;
6482+
libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, isWindowsTemplate, isUefiEnabled, isSecureBoot,
6483+
physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details);
6484+
verify(diskDef, never()).defFileBasedDisk(PHYSICAL_DISK_PATH, PHYSICAL_DISK_PATH, DISK_BUS_TYPE_DATA, DiskDef.DiskFmtType.QCOW2);
6485+
verify(diskDef, times(1)).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE, DiskDef.DiskFmtType.QCOW2);
6486+
}
64116487
}

0 commit comments

Comments
 (0)