Skip to content

Commit dd50ca5

Browse files
shwstpprdhslove
shwstppr
authored and
dhslove
committed
kvm: allow skip forcing disk controller (apache#11750)
1 parent 2a8c63b commit dd50ca5

4 files changed

Lines changed: 106 additions & 1 deletion

File tree

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

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

60+
// KVM specific, disk controllers
61+
String KVM_SKIP_FORCE_DISK_CONTROLLER = "skip.force.disk.controller";
62+
6063
// Mac OSX guest specific (internal)
6164
String SMC_PRESENT = "smc.present";
6265
String FIRMWARE = "firmware";

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3891,6 +3891,44 @@ public static DiskDef.DiskType getDiskType(KVMPhysicalDisk physicalDisk) {
38913891
return useBLOCKDiskType(physicalDisk) ? DiskDef.DiskType.BLOCK : DiskDef.DiskType.FILE;
38923892
}
38933893

3894+
/**
3895+
* Defines the disk configuration for the default pool type based on the provided parameters.
3896+
* It determines the appropriate disk settings depending on whether the disk is a data disk, whether
3897+
* it's a Windows template, whether UEFI is enabled, and whether secure boot is active.
3898+
*
3899+
* @param disk The disk definition object that will be configured with the disk settings.
3900+
* @param volume The volume (disk) object, containing information about the type of disk.
3901+
* @param isWindowsTemplate Flag indicating whether the template is a Windows template.
3902+
* @param isUefiEnabled Flag indicating whether UEFI is enabled.
3903+
* @param isSecureBoot Flag indicating whether secure boot is enabled.
3904+
* @param physicalDisk The physical disk object that contains the path to the disk.
3905+
* @param devId The device ID for the disk.
3906+
* @param diskBusType The disk bus type to use if not skipping force disk controller.
3907+
* @param diskBusTypeData The disk bus type to use for data disks, if applicable.
3908+
* @param details A map of VM details containing additional configuration values, such as whether to skip force
3909+
* disk controller.
3910+
*/
3911+
protected void defineDiskForDefaultPoolType(DiskDef disk, DiskTO volume, boolean isWindowsTemplate,
3912+
boolean isUefiEnabled, boolean isSecureBoot, KVMPhysicalDisk physicalDisk, int devId,
3913+
DiskDef.DiskBus diskBusType, DiskDef.DiskBus diskBusTypeData, Map<String, String> details) {
3914+
boolean skipForceDiskController = MapUtils.getBoolean(details, VmDetailConstants.KVM_SKIP_FORCE_DISK_CONTROLLER,
3915+
false);
3916+
if (skipForceDiskController) {
3917+
disk.defFileBasedDisk(physicalDisk.getPath(), devId, Volume.Type.DATADISK.equals(volume.getType()) ?
3918+
diskBusTypeData : diskBusType, DiskDef.DiskFmtType.QCOW2);
3919+
return;
3920+
}
3921+
if (volume.getType() == Volume.Type.DATADISK && !(isWindowsTemplate && isUefiEnabled)) {
3922+
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusTypeData, DiskDef.DiskFmtType.QCOW2);
3923+
} else {
3924+
if (isSecureBoot) {
3925+
disk.defFileBasedDisk(physicalDisk.getPath(), devId, DiskDef.DiskFmtType.QCOW2, isWindowsTemplate);
3926+
} else {
3927+
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusType, DiskDef.DiskFmtType.QCOW2);
3928+
}
3929+
}
3930+
}
3931+
38943932
public void createVbd(final Connect conn, final VirtualMachineTO vmSpec, final String vmName, final LibvirtVMDef vm) throws InternalErrorException, LibvirtException, URISyntaxException {
38953933
final Map<String, String> details = vmSpec.getDetails();
38963934
final List<DiskTO> disks = Arrays.asList(vmSpec.getDisks());
@@ -4102,6 +4140,9 @@ public int compare(final DiskTO arg0, final DiskTO arg1) {
41024140
// disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusType, DiskDef.DiskFmtType.QCOW2);
41034141
// }
41044142
}
4143+
4144+
defineDiskForDefaultPoolType(disk, volume, isWindowsTemplate, isUefiEnabled, isSecureBoot,
4145+
physicalDisk, devId, diskBusType, diskBusTypeData, details);
41054146
}
41064147
pool.customizeLibvirtDiskDef(disk);
41074148
}

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

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,13 +228,15 @@
228228
import com.cloud.template.VirtualMachineTemplate.BootloaderType;
229229
import com.cloud.utils.Pair;
230230
import com.cloud.utils.exception.CloudRuntimeException;
231-
import com.cloud.utils.script.Script;
231+
import com.cloud.utils.net.NetUtils;
232232
import com.cloud.utils.script.OutputInterpreter.OneLineParser;
233+
import com.cloud.utils.script.Script;
233234
import com.cloud.utils.ssh.SshHelper;
234235
import com.cloud.vm.DiskProfile;
235236
import com.cloud.vm.VirtualMachine;
236237
import com.cloud.vm.VirtualMachine.PowerState;
237238
import com.cloud.vm.VirtualMachine.Type;
239+
import com.cloud.vm.VmDetailConstants;
238240

239241
@RunWith(MockitoJUnitRunner.class)
240242
public class LibvirtComputingResourceTest {
@@ -251,6 +253,19 @@ public class LibvirtComputingResourceTest {
251253
Connect connMock;
252254
@Mock
253255
LibvirtDomainXMLParser parserMock;
256+
@Mock
257+
private DiskDef diskDef;
258+
@Mock
259+
private DiskTO volume;
260+
@Mock
261+
private KVMPhysicalDisk physicalDisk;
262+
@Mock
263+
private Map<String, String> details;
264+
265+
private static final String PHYSICAL_DISK_PATH = "/path/to/disk";
266+
private static final int DEV_ID = 1;
267+
private static final DiskDef.DiskBus DISK_BUS_TYPE = DiskDef.DiskBus.VIRTIO;
268+
private static final DiskDef.DiskBus DISK_BUS_TYPE_DATA = DiskDef.DiskBus.SCSI;
254269

255270
@Mock
256271
DiskTO diskToMock;
@@ -7145,4 +7160,49 @@ public void parseCpuFeaturesTestReturnListOfCpuFeaturesAndIgnoreMultipleWhitespa
71457160
Assert.assertEquals("-mmx", cpuFeatures.get(2));
71467161
Assert.assertEquals("hle", cpuFeatures.get(3));
71477162
}
7163+
7164+
@Test
7165+
public void defineDiskForDefaultPoolTypeSkipsForceDiskController() {
7166+
Map<String, String> details = new HashMap<>();
7167+
details.put(VmDetailConstants.KVM_SKIP_FORCE_DISK_CONTROLLER, "true");
7168+
Mockito.when(volume.getType()).thenReturn(Volume.Type.DATADISK);
7169+
Mockito.when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH);
7170+
libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, false, false, false, physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details);
7171+
Mockito.verify(diskDef).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE_DATA, DiskDef.DiskFmtType.QCOW2);
7172+
}
7173+
7174+
@Test
7175+
public void defineDiskForDefaultPoolTypeUsesDiskBusTypeDataForDataDiskWithoutWindowsAndUefi() {
7176+
Map<String, String> details = new HashMap<>();
7177+
Mockito.when(volume.getType()).thenReturn(Volume.Type.DATADISK);
7178+
Mockito.when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH);
7179+
libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, false, false, false, physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details);
7180+
Mockito.verify(diskDef).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE_DATA, DiskDef.DiskFmtType.QCOW2);
7181+
}
7182+
7183+
@Test
7184+
public void defineDiskForDefaultPoolTypeUsesDiskBusTypeForRootDisk() {
7185+
Map<String, String> details = new HashMap<>();
7186+
Mockito.when(volume.getType()).thenReturn(Volume.Type.ROOT);
7187+
Mockito.when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH);
7188+
libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, false, false, false, physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details);
7189+
Mockito.verify(diskDef).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE, DiskDef.DiskFmtType.QCOW2);
7190+
}
7191+
7192+
@Test
7193+
public void defineDiskForDefaultPoolTypeUsesSecureBootConfiguration() {
7194+
Map<String, String> details = new HashMap<>();
7195+
Mockito.when(volume.getType()).thenReturn(Volume.Type.ROOT);
7196+
Mockito.when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH);
7197+
libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, true, true, true, physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details);
7198+
Mockito.verify(diskDef).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DiskDef.DiskFmtType.QCOW2, true);
7199+
}
7200+
7201+
@Test
7202+
public void defineDiskForDefaultPoolTypeHandlesNullDetails() {
7203+
Mockito.when(volume.getType()).thenReturn(Volume.Type.DATADISK);
7204+
Mockito.when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH);
7205+
libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, false, false, false, physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, null);
7206+
Mockito.verify(diskDef).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE_DATA, DiskDef.DiskFmtType.QCOW2);
7207+
}
71487208
}

‎server/src/main/java/com/cloud/api/query/QueryManagerImpl.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5486,6 +5486,7 @@ private void fillVMOrTemplateDetailOptions(final Map<String, List<String>> optio
54865486
options.put(VmDetailConstants.GUEST_CPU_MODE, Arrays.asList("custom", "host-model", "host-passthrough"));
54875487
options.put(VmDetailConstants.GUEST_CPU_MODEL, Collections.emptyList());
54885488
options.put(VmDetailConstants.BOOT_ORDER, Arrays.asList("cdrom", "hd"));
5489+
options.put(VmDetailConstants.KVM_SKIP_FORCE_DISK_CONTROLLER, Arrays.asList("true", "false"));
54895490
}
54905491

54915492
if (HypervisorType.VMware.equals(hypervisorType)) {

0 commit comments

Comments
 (0)