Skip to content

Commit ef1aaa0

Browse files
shwstpprshwstppr
authored
kvm: allow skip forcing disk controller (#11750)
1 parent c91e84c commit ef1aaa0

File tree

4 files changed

+106
-15
lines changed

4 files changed

+106
-15
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
@@ -54,6 +54,9 @@ public interface VmDetailConstants {
5454
String NIC_MULTIQUEUE_NUMBER = "nic.multiqueue.number";
5555
String NIC_PACKED_VIRTQUEUES_ENABLED = "nic.packed.virtqueues.enabled";
5656

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

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

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@
107107
import org.xml.sax.InputSource;
108108
import org.xml.sax.SAXException;
109109

110-
111110
import com.cloud.agent.api.Answer;
112111
import com.cloud.agent.api.Command;
113112
import com.cloud.agent.api.HostVmStateReportEntry;
@@ -188,8 +187,8 @@
188187
import com.cloud.network.Networks.RouterPrivateIpStrategy;
189188
import com.cloud.network.Networks.TrafficType;
190189
import com.cloud.resource.AgentStatusUpdater;
191-
import com.cloud.resource.ResourceStatusUpdater;
192190
import com.cloud.resource.RequestWrapper;
191+
import com.cloud.resource.ResourceStatusUpdater;
193192
import com.cloud.resource.ServerResource;
194193
import com.cloud.resource.ServerResourceBase;
195194
import com.cloud.storage.JavaStorageLayer;
@@ -3083,6 +3082,44 @@ public static DiskDef.DiskType getDiskType(KVMPhysicalDisk physicalDisk) {
30833082
return useBLOCKDiskType(physicalDisk) ? DiskDef.DiskType.BLOCK : DiskDef.DiskType.FILE;
30843083
}
30853084

3085+
/**
3086+
* Defines the disk configuration for the default pool type based on the provided parameters.
3087+
* It determines the appropriate disk settings depending on whether the disk is a data disk, whether
3088+
* it's a Windows template, whether UEFI is enabled, and whether secure boot is active.
3089+
*
3090+
* @param disk The disk definition object that will be configured with the disk settings.
3091+
* @param volume The volume (disk) object, containing information about the type of disk.
3092+
* @param isWindowsTemplate Flag indicating whether the template is a Windows template.
3093+
* @param isUefiEnabled Flag indicating whether UEFI is enabled.
3094+
* @param isSecureBoot Flag indicating whether secure boot is enabled.
3095+
* @param physicalDisk The physical disk object that contains the path to the disk.
3096+
* @param devId The device ID for the disk.
3097+
* @param diskBusType The disk bus type to use if not skipping force disk controller.
3098+
* @param diskBusTypeData The disk bus type to use for data disks, if applicable.
3099+
* @param details A map of VM details containing additional configuration values, such as whether to skip force
3100+
* disk controller.
3101+
*/
3102+
protected void defineDiskForDefaultPoolType(DiskDef disk, DiskTO volume, boolean isWindowsTemplate,
3103+
boolean isUefiEnabled, boolean isSecureBoot, KVMPhysicalDisk physicalDisk, int devId,
3104+
DiskDef.DiskBus diskBusType, DiskDef.DiskBus diskBusTypeData, Map<String, String> details) {
3105+
boolean skipForceDiskController = MapUtils.getBoolean(details, VmDetailConstants.KVM_SKIP_FORCE_DISK_CONTROLLER,
3106+
false);
3107+
if (skipForceDiskController) {
3108+
disk.defFileBasedDisk(physicalDisk.getPath(), devId, Volume.Type.DATADISK.equals(volume.getType()) ?
3109+
diskBusTypeData : diskBusType, DiskDef.DiskFmtType.QCOW2);
3110+
return;
3111+
}
3112+
if (volume.getType() == Volume.Type.DATADISK && !(isWindowsTemplate && isUefiEnabled)) {
3113+
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusTypeData, DiskDef.DiskFmtType.QCOW2);
3114+
} else {
3115+
if (isSecureBoot) {
3116+
disk.defFileBasedDisk(physicalDisk.getPath(), devId, DiskDef.DiskFmtType.QCOW2, isWindowsTemplate);
3117+
} else {
3118+
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusType, DiskDef.DiskFmtType.QCOW2);
3119+
}
3120+
}
3121+
}
3122+
30863123
public void createVbd(final Connect conn, final VirtualMachineTO vmSpec, final String vmName, final LibvirtVMDef vm) throws InternalErrorException, LibvirtException, URISyntaxException {
30873124
final Map<String, String> details = vmSpec.getDetails();
30883125
final List<DiskTO> disks = Arrays.asList(vmSpec.getDisks());
@@ -3244,15 +3281,8 @@ public int compare(final DiskTO arg0, final DiskTO arg1) {
32443281
disk.setDiscard(DiscardType.UNMAP);
32453282
}
32463283
} else {
3247-
if (volume.getType() == Volume.Type.DATADISK && !(isWindowsTemplate && isUefiEnabled)) {
3248-
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusTypeData, DiskDef.DiskFmtType.QCOW2);
3249-
} else {
3250-
if (isSecureBoot) {
3251-
disk.defFileBasedDisk(physicalDisk.getPath(), devId, DiskDef.DiskFmtType.QCOW2, isWindowsTemplate);
3252-
} else {
3253-
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusType, DiskDef.DiskFmtType.QCOW2);
3254-
}
3255-
}
3284+
defineDiskForDefaultPoolType(disk, volume, isWindowsTemplate, isUefiEnabled, isSecureBoot,
3285+
physicalDisk, devId, diskBusType, diskBusTypeData, details);
32563286
}
32573287
pool.customizeLibvirtDiskDef(disk);
32583288
}

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

Lines changed: 61 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());
@@ -6565,4 +6577,49 @@ public void testCreateTpmDefWithInvalidVersion() {
65656577
assertEquals(LibvirtVMDef.TpmDef.TpmModel.CRB, tpmDef.getModel());
65666578
assertEquals(LibvirtVMDef.TpmDef.TpmVersion.V2_0, tpmDef.getVersion());
65676579
}
6580+
6581+
@Test
6582+
public void defineDiskForDefaultPoolTypeSkipsForceDiskController() {
6583+
Map<String, String> details = new HashMap<>();
6584+
details.put(VmDetailConstants.KVM_SKIP_FORCE_DISK_CONTROLLER, "true");
6585+
Mockito.when(volume.getType()).thenReturn(Volume.Type.DATADISK);
6586+
Mockito.when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH);
6587+
libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, false, false, false, physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details);
6588+
Mockito.verify(diskDef).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE_DATA, DiskDef.DiskFmtType.QCOW2);
6589+
}
6590+
6591+
@Test
6592+
public void defineDiskForDefaultPoolTypeUsesDiskBusTypeDataForDataDiskWithoutWindowsAndUefi() {
6593+
Map<String, String> details = new HashMap<>();
6594+
Mockito.when(volume.getType()).thenReturn(Volume.Type.DATADISK);
6595+
Mockito.when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH);
6596+
libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, false, false, false, physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details);
6597+
Mockito.verify(diskDef).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE_DATA, DiskDef.DiskFmtType.QCOW2);
6598+
}
6599+
6600+
@Test
6601+
public void defineDiskForDefaultPoolTypeUsesDiskBusTypeForRootDisk() {
6602+
Map<String, String> details = new HashMap<>();
6603+
Mockito.when(volume.getType()).thenReturn(Volume.Type.ROOT);
6604+
Mockito.when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH);
6605+
libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, false, false, false, physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details);
6606+
Mockito.verify(diskDef).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE, DiskDef.DiskFmtType.QCOW2);
6607+
}
6608+
6609+
@Test
6610+
public void defineDiskForDefaultPoolTypeUsesSecureBootConfiguration() {
6611+
Map<String, String> details = new HashMap<>();
6612+
Mockito.when(volume.getType()).thenReturn(Volume.Type.ROOT);
6613+
Mockito.when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH);
6614+
libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, true, true, true, physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, details);
6615+
Mockito.verify(diskDef).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DiskDef.DiskFmtType.QCOW2, true);
6616+
}
6617+
6618+
@Test
6619+
public void defineDiskForDefaultPoolTypeHandlesNullDetails() {
6620+
Mockito.when(volume.getType()).thenReturn(Volume.Type.DATADISK);
6621+
Mockito.when(physicalDisk.getPath()).thenReturn(PHYSICAL_DISK_PATH);
6622+
libvirtComputingResourceSpy.defineDiskForDefaultPoolType(diskDef, volume, false, false, false, physicalDisk, DEV_ID, DISK_BUS_TYPE, DISK_BUS_TYPE_DATA, null);
6623+
Mockito.verify(diskDef).defFileBasedDisk(PHYSICAL_DISK_PATH, DEV_ID, DISK_BUS_TYPE_DATA, DiskDef.DiskFmtType.QCOW2);
6624+
}
65686625
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5072,6 +5072,7 @@ private void fillVMOrTemplateDetailOptions(final Map<String, List<String>> optio
50725072
options.put(VmDetailConstants.VIRTUAL_TPM_VERSION, Arrays.asList("1.2", "2.0"));
50735073
options.put(VmDetailConstants.GUEST_CPU_MODE, Arrays.asList("custom", "host-model", "host-passthrough"));
50745074
options.put(VmDetailConstants.GUEST_CPU_MODEL, Collections.emptyList());
5075+
options.put(VmDetailConstants.KVM_SKIP_FORCE_DISK_CONTROLLER, Arrays.asList("true", "false"));
50755076
}
50765077

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

0 commit comments

Comments
 (0)