Skip to content

Commit 2811217

Browse files
author
Daan Hoogland
committed
Merge branch '4.22'
2 parents 1b861da + e25cf43 commit 2811217

File tree

7 files changed

+124
-19
lines changed

7 files changed

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

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

‎engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDaoImpl.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,8 @@ public class CapacityDaoImpl extends GenericDaoBase<CapacityVO, Long> implements
213213

214214
private static final String LEFT_JOIN_VM_TEMPLATE = "LEFT JOIN vm_template ON vm_template.id = vi.vm_template_id ";
215215

216+
private static final String STORAGE_POOLS_WITH_CHILDREN = "SELECT DISTINCT parent FROM storage_pool WHERE parent != 0 AND removed IS NULL";
217+
216218
public CapacityDaoImpl() {
217219
_hostIdTypeSearch = createSearchBuilder();
218220
_hostIdTypeSearch.and("hostId", _hostIdTypeSearch.entity().getHostOrPoolId(), SearchCriteria.Op.EQ);
@@ -379,6 +381,11 @@ public List<SummedCapacity> listCapacitiesGroupedByLevelAndType(Integer capacity
379381
finalQuery.append(" AND capacity_type = ?");
380382
resourceIdList.add(capacityType.longValue());
381383
}
384+
385+
// Exclude storage pools with children from capacity calculations to avoid double counting
386+
finalQuery.append(" AND NOT (capacity.capacity_type = ").append(Capacity.CAPACITY_TYPE_STORAGE_ALLOCATED)
387+
.append(" AND capacity.host_id IN (").append(STORAGE_POOLS_WITH_CHILDREN).append("))");
388+
382389
if (CollectionUtils.isNotEmpty(hostIds)) {
383390
finalQuery.append(String.format(" AND capacity.host_id IN (%s)", StringUtils.join(hostIds, ",")));
384391
if (capacityType == null) {
@@ -541,6 +548,10 @@ public List<SummedCapacity> findFilteredCapacityBy(Integer capacityType, Long zo
541548
StringBuilder sql = new StringBuilder(LIST_CAPACITY_GROUP_BY_CAPACITY_PART1);
542549
List<Long> resourceIdList = new ArrayList<Long>();
543550

551+
// Exclude storage pools with children from capacity calculations to avoid double counting
552+
sql.append(" AND NOT (capacity.capacity_type = ").append(Capacity.CAPACITY_TYPE_STORAGE_ALLOCATED)
553+
.append(" AND capacity.host_id IN (").append(STORAGE_POOLS_WITH_CHILDREN).append("))");
554+
544555
if (zoneId != null) {
545556
sql.append(" AND capacity.data_center_id = ?");
546557
resourceIdList.add(zoneId);

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

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3493,6 +3493,44 @@ public static DiskDef.DiskType getDiskType(KVMPhysicalDisk physicalDisk) {
34933493
return useBLOCKDiskType(physicalDisk) ? DiskDef.DiskType.BLOCK : DiskDef.DiskType.FILE;
34943494
}
34953495

3496+
/**
3497+
* Defines the disk configuration for the default pool type based on the provided parameters.
3498+
* It determines the appropriate disk settings depending on whether the disk is a data disk, whether
3499+
* it's a Windows template, whether UEFI is enabled, and whether secure boot is active.
3500+
*
3501+
* @param disk The disk definition object that will be configured with the disk settings.
3502+
* @param volume The volume (disk) object, containing information about the type of disk.
3503+
* @param isWindowsTemplate Flag indicating whether the template is a Windows template.
3504+
* @param isUefiEnabled Flag indicating whether UEFI is enabled.
3505+
* @param isSecureBoot Flag indicating whether secure boot is enabled.
3506+
* @param physicalDisk The physical disk object that contains the path to the disk.
3507+
* @param devId The device ID for the disk.
3508+
* @param diskBusType The disk bus type to use if not skipping force disk controller.
3509+
* @param diskBusTypeData The disk bus type to use for data disks, if applicable.
3510+
* @param details A map of VM details containing additional configuration values, such as whether to skip force
3511+
* disk controller.
3512+
*/
3513+
protected void defineDiskForDefaultPoolType(DiskDef disk, DiskTO volume, boolean isWindowsTemplate,
3514+
boolean isUefiEnabled, boolean isSecureBoot, KVMPhysicalDisk physicalDisk, int devId,
3515+
DiskDef.DiskBus diskBusType, DiskDef.DiskBus diskBusTypeData, Map<String, String> details) {
3516+
boolean skipForceDiskController = MapUtils.getBoolean(details, VmDetailConstants.KVM_SKIP_FORCE_DISK_CONTROLLER,
3517+
false);
3518+
if (skipForceDiskController) {
3519+
disk.defFileBasedDisk(physicalDisk.getPath(), devId, Volume.Type.DATADISK.equals(volume.getType()) ?
3520+
diskBusTypeData : diskBusType, DiskDef.DiskFmtType.QCOW2);
3521+
return;
3522+
}
3523+
if (volume.getType() == Volume.Type.DATADISK && !(isWindowsTemplate && isUefiEnabled)) {
3524+
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusTypeData, DiskDef.DiskFmtType.QCOW2);
3525+
} else {
3526+
if (isSecureBoot) {
3527+
disk.defFileBasedDisk(physicalDisk.getPath(), devId, DiskDef.DiskFmtType.QCOW2, isWindowsTemplate);
3528+
} else {
3529+
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusType, DiskDef.DiskFmtType.QCOW2);
3530+
}
3531+
}
3532+
}
3533+
34963534
public void createVbd(final Connect conn, final VirtualMachineTO vmSpec, final String vmName, final LibvirtVMDef vm) throws InternalErrorException, LibvirtException, URISyntaxException {
34973535
final Map<String, String> details = vmSpec.getDetails();
34983536
final List<DiskTO> disks = Arrays.asList(vmSpec.getDisks());
@@ -3654,15 +3692,8 @@ public int compare(final DiskTO arg0, final DiskTO arg1) {
36543692
disk.setDiscard(DiscardType.UNMAP);
36553693
}
36563694
} else {
3657-
if (volume.getType() == Volume.Type.DATADISK && !(isWindowsTemplate && isUefiEnabled)) {
3658-
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusTypeData, DiskDef.DiskFmtType.QCOW2);
3659-
} else {
3660-
if (isSecureBoot) {
3661-
disk.defFileBasedDisk(physicalDisk.getPath(), devId, DiskDef.DiskFmtType.QCOW2, isWindowsTemplate);
3662-
} else {
3663-
disk.defFileBasedDisk(physicalDisk.getPath(), devId, diskBusType, DiskDef.DiskFmtType.QCOW2);
3664-
}
3665-
}
3695+
defineDiskForDefaultPoolType(disk, volume, isWindowsTemplate, isUefiEnabled, isSecureBoot,
3696+
physicalDisk, devId, diskBusType, diskBusTypeData, details);
36663697
}
36673698
pool.customizeLibvirtDiskDef(disk);
36683699
}

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

Lines changed: 63 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,9 @@
6161
import javax.xml.xpath.XPathExpressionException;
6262
import javax.xml.xpath.XPathFactory;
6363

64-
import com.cloud.cpu.CPU;
65-
import com.cloud.utils.net.NetUtils;
66-
67-
import com.cloud.vm.VmDetailConstants;
6864
import com.google.gson.JsonObject;
6965
import com.google.gson.JsonParser;
66+
7067
import org.apache.cloudstack.api.ApiConstants.IoDriverPolicy;
7168
import org.apache.cloudstack.storage.command.AttachAnswer;
7269
import org.apache.cloudstack.storage.command.AttachCommand;
@@ -186,6 +183,7 @@
186183
import com.cloud.agent.properties.AgentProperties;
187184
import com.cloud.agent.properties.AgentPropertiesFileHandler;
188185
import com.cloud.agent.resource.virtualnetwork.VirtualRoutingResource;
186+
import com.cloud.cpu.CPU;
189187
import com.cloud.exception.InternalErrorException;
190188
import com.cloud.hypervisor.Hypervisor.HypervisorType;
191189
import com.cloud.hypervisor.kvm.resource.KVMHABase.HAStoragePool;
@@ -228,13 +226,15 @@
228226
import com.cloud.template.VirtualMachineTemplate.BootloaderType;
229227
import com.cloud.utils.Pair;
230228
import com.cloud.utils.exception.CloudRuntimeException;
231-
import com.cloud.utils.script.Script;
229+
import com.cloud.utils.net.NetUtils;
232230
import com.cloud.utils.script.OutputInterpreter.OneLineParser;
231+
import com.cloud.utils.script.Script;
233232
import com.cloud.utils.ssh.SshHelper;
234233
import com.cloud.vm.DiskProfile;
235234
import com.cloud.vm.VirtualMachine;
236235
import com.cloud.vm.VirtualMachine.PowerState;
237236
import com.cloud.vm.VirtualMachine.Type;
237+
import com.cloud.vm.VmDetailConstants;
238238

239239
@RunWith(MockitoJUnitRunner.class)
240240
public class LibvirtComputingResourceTest {
@@ -251,6 +251,19 @@ public class LibvirtComputingResourceTest {
251251
Connect connMock;
252252
@Mock
253253
LibvirtDomainXMLParser parserMock;
254+
@Mock
255+
private DiskDef diskDef;
256+
@Mock
257+
private DiskTO volume;
258+
@Mock
259+
private KVMPhysicalDisk physicalDisk;
260+
@Mock
261+
private Map<String, String> details;
262+
263+
private static final String PHYSICAL_DISK_PATH = "/path/to/disk";
264+
private static final int DEV_ID = 1;
265+
private static final DiskDef.DiskBus DISK_BUS_TYPE = DiskDef.DiskBus.VIRTIO;
266+
private static final DiskDef.DiskBus DISK_BUS_TYPE_DATA = DiskDef.DiskBus.SCSI;
254267

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

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5390,6 +5390,7 @@ private void fillVMOrTemplateDetailOptions(final Map<String, List<String>> optio
53905390
options.put(VmDetailConstants.VIRTUAL_TPM_VERSION, Arrays.asList("1.2", "2.0"));
53915391
options.put(VmDetailConstants.GUEST_CPU_MODE, Arrays.asList("custom", "host-model", "host-passthrough"));
53925392
options.put(VmDetailConstants.GUEST_CPU_MODEL, Collections.emptyList());
5393+
options.put(VmDetailConstants.KVM_SKIP_FORCE_DISK_CONTROLLER, Arrays.asList("true", "false"));
53935394
}
53945395

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

‎server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,6 +1044,10 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
10441044
category = config.getCategory();
10451045
}
10461046

1047+
if (value == null) {
1048+
throw new InvalidParameterValueException(String.format("The new value for the [%s] configuration must be given.", name));
1049+
}
1050+
10471051
validateIpAddressRelatedConfigValues(name, value);
10481052
validateConflictingConfigValue(name, value);
10491053

@@ -1052,10 +1056,6 @@ public Configuration updateConfiguration(final UpdateCfgCmd cmd) throws InvalidP
10521056
throw new CloudRuntimeException("Only Root Admin is allowed to edit this configuration.");
10531057
}
10541058

1055-
if (value == null) {
1056-
return _configDao.findByName(name);
1057-
}
1058-
10591059
ConfigKey.Scope scope = null;
10601060
Long id = null;
10611061
int paramCountCheck = 0;

‎server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2459,7 +2459,8 @@ private void validateVolumeResizeWithSize(VolumeVO volume, long currentSize, Lon
24592459
}
24602460
}
24612461

2462-
if (volume != null && ImageFormat.QCOW2.equals(volume.getFormat()) && !Volume.State.Allocated.equals(volume.getState()) && !StoragePoolType.StorPool.equals(volume.getPoolType())) {
2462+
if (volume != null && ImageFormat.QCOW2.equals(volume.getFormat()) && !Volume.State.Allocated.equals(volume.getState()) &&
2463+
!Arrays.asList(StoragePoolType.StorPool, StoragePoolType.Linstor).contains(volume.getPoolType())) {
24632464
String message = "Unable to shrink volumes of type QCOW2";
24642465
logger.warn(message);
24652466
throw new InvalidParameterValueException(message);

0 commit comments

Comments
 (0)