Skip to content

Allow KVM overcommit to work without reducing minimum VM memory when vm ballooning is disabled #7810

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaanHoogland merged 15 commits into from
Aug 6, 2023
Merged

Allow KVM overcommit to work without reducing minimum VM memory when vm ballooning is disabled #7810

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
091ed9a
Revert "KVM: fix SSVM starting when overprovisioning memory (#7663)"
yadvr Aug 3, 2023
2dcc441
server: KVM guru should pass max memory as per VM transfer object
yadvr Aug 3, 2023
a5cac4c
disable vm ballooning feature by default
yadvr Aug 3, 2023
4f82fa6
Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm…
yadvr Aug 3, 2023
757adfc
Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm…
yadvr Aug 3, 2023
b316059
Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm…
yadvr Aug 3, 2023
90f9f55
Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm…
yadvr Aug 3, 2023
6a14afd
Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm…
yadvr Aug 3, 2023
63f3f56
Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm…
yadvr Aug 3, 2023
d6d25e9
Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm…
yadvr Aug 3, 2023
56fe36a
Update plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm…
yadvr Aug 3, 2023
037bcbe
Revert "disable vm ballooning feature by default"
yadvr Aug 3, 2023
63cd85f
add logging around vm memory ballooning; don't use min memory for sys…
yadvr Aug 3, 2023
123b1e2
fix unit tests and xml
yadvr Aug 3, 2023
14c83ab
method cleanup
DaanHoogland Aug 4, 2023
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
18 changes: 9 additions & 9 deletions ...rvisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2780,15 +2780,10 @@ protected GuestResourceDef createGuestResourceDef(VirtualMachineTO vmTO){

grd.setMemBalloning(!_noMemBalloon);

long maxRam = ByteScaleUtils.bytesToKibibytes(vmTO.getMaxRam());
long currRam = vmTO.getType() == VirtualMachine.Type.User ? getCurrentMemAccordingToMemBallooning(vmTO, maxRam) : maxRam;

if (s_logger.isTraceEnabled()) {
s_logger.trace(String.format("memory values for VM %s are %d/%d",vmTO.getName(),maxRam, currRam));
}
Long maxRam = ByteScaleUtils.bytesToKibibytes(vmTO.getMaxRam());

grd.setMemorySize(maxRam);
grd.setCurrentMem(currRam);
grd.setCurrentMem(getCurrentMemAccordingToMemBallooning(vmTO, maxRam));
Copy link
Member Author

@yadvr yadvr Aug 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just realised the maxRam seetting is fixed as in the KVMGuru the correct maxRam is passed now.
cc @DaanHoogland @weizhouapache let's test


int vcpus = vmTO.getCpus();
Integer maxVcpus = vmTO.getVcpuMaxLimit();
Expand All @@ -2800,12 +2795,17 @@ protected GuestResourceDef createGuestResourceDef(VirtualMachineTO vmTO){
}

protected long getCurrentMemAccordingToMemBallooning(VirtualMachineTO vmTO, long maxRam) {
long retVal = maxRam;
if (_noMemBalloon) {
s_logger.warn(String.format("Setting VM's [%s] current memory as max memory [%s] due to memory ballooning is disabled. If you are using a custom service offering, verify if memory ballooning really should be disabled.", vmTO.toString(), maxRam));
return maxRam;
} else if (vmTO != null && vmTO.getType() != VirtualMachine.Type.User) {
s_logger.warn(String.format("Setting System VM's [%s] current memory as max memory [%s].", vmTO.toString(), maxRam));
} else {
return ByteScaleUtils.bytesToKibibytes(vmTO.getMinRam());
long minRam = ByteScaleUtils.bytesToKibibytes(vmTO.getMinRam());
s_logger.debug(String.format("Setting VM's [%s] current memory as min memory [%s] due to memory ballooning is enabled.", vmTO.toString(), minRam));
retVal = minRam;
}
return retVal;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ public void setMemBalloning(boolean memoryBalloning) {
@Override
public String toString() {
StringBuilder response = new StringBuilder();
response.append(String.format("<memory>%s</memory>\n", this.memory));
response.append(String.format("<memory>%s</memory>\n", this.currentMemory));
response.append(String.format("<currentMemory>%s</currentMemory>\n", this.currentMemory));

if (this.memory > this.currentMemory) {
Expand Down Expand Up @@ -1238,7 +1238,7 @@ public String getMemBalloonStatsPeriod() {
@Override
public String toString() {
StringBuilder memBalloonBuilder = new StringBuilder();
memBalloonBuilder.append("<memballoon model='" + memBalloonModel + "' autodeflate='on'>\n");
memBalloonBuilder.append("<memballoon model='" + memBalloonModel + "'>\n");
Copy link
Member

@harikrishna-patnala harikrishna-patnala Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rohityadavcloud does this mean, the blocker issue #7794 is fixed here?

Copy link
Member Author

@yadvr yadvr Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly - we need to check/test cc @DaanHoogland

Copy link
Contributor

@DaanHoogland DaanHoogland Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harikrishna-patnala , yes as shown by
#7810 (comment)

if (StringUtils.isNotBlank(memBalloonStatsPeriod)) {
memBalloonBuilder.append("<stats period='" + memBalloonStatsPeriod +"'/>\n");
}
Expand Down
4 changes: 3 additions & 1 deletion ...ors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ private void verifyVcpu(VirtualMachineTO to, Document domainDoc) {

private void verifyMemory(VirtualMachineTO to, Document domainDoc, String minRam) {
assertXpath(domainDoc, "/domain/maxMemory/text()", String.valueOf( to.getMaxRam() / 1024 ));
assertXpath(domainDoc, "/domain/currentMemory/text()",minRam);
assertXpath(domainDoc, "/domain/memory/text()",minRam);
assertXpath(domainDoc, "/domain/cpu/numa/cell/@memory", minRam);
assertXpath(domainDoc, "/domain/currentMemory/text()", minRam);
}
Expand Down Expand Up @@ -5762,6 +5762,7 @@ public void testAddExtraConfigComponentNotEmptyExtraConfig() {

public void validateGetCurrentMemAccordingToMemBallooningWithoutMemBalooning(){
VirtualMachineTO vmTo = Mockito.mock(VirtualMachineTO.class);
Mockito.when(vmTo.getType()).thenReturn(Type.User);
LibvirtComputingResource libvirtComputingResource = new LibvirtComputingResource();
libvirtComputingResource._noMemBalloon = true;
long maxMemory = 2048;
Expand All @@ -5780,6 +5781,7 @@ public void validateGetCurrentMemAccordingToMemBallooningWithtMemBalooning(){
long minMemory = ByteScaleUtils.mebibytesToBytes(64);

VirtualMachineTO vmTo = Mockito.mock(VirtualMachineTO.class);
Mockito.when(vmTo.getType()).thenReturn(Type.User);
Mockito.when(vmTo.getMinRam()).thenReturn(minMemory);

long currentMemory = libvirtComputingResource.getCurrentMemAccordingToMemBallooning(vmTo, maxMemory);
Expand Down
20 changes: 10 additions & 10 deletions ...ins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDefTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,11 @@ public void testDiskDef() {
assertEquals(bus, disk.getBusType());
assertEquals(DiskDef.DeviceType.DISK, disk.getDeviceType());

String resultingXml = disk.toString();
String xmlDef = disk.toString();
String expectedXml = "<disk device='disk' type='file'>\n<driver name='qemu' type='" + type.toString() + "' cache='" + cacheMode.toString() + "' />\n" +
"<source file='" + filePath + "'/>\n<target dev='" + diskLabel + "' bus='" + bus.toString() + "'/>\n</disk>\n";

assertEquals(expectedXml, resultingXml);
assertEquals(expectedXml, xmlDef);
}

@Test
Expand Down Expand Up @@ -346,7 +346,7 @@ public void testDiskDefWithBurst() {
LibvirtVMDef.setGlobalQemuVersion(2006000L);
LibvirtVMDef.setGlobalLibvirtVersion(9008L);

String resultingXml = disk.toString();
String xmlDef = disk.toString();
String expectedXml = "<disk device='disk' type='file'>\n<driver name='qemu' type='" + type.toString() + "' cache='none' />\n" +
"<source file='" + filePath + "'/>\n<target dev='" + diskLabel + "' bus='" + bus.toString() + "'/>\n" +
"<iotune>\n<read_bytes_sec>"+bytesReadRate+"</read_bytes_sec>\n<write_bytes_sec>"+bytesWriteRate+"</write_bytes_sec>\n" +
Expand All @@ -356,29 +356,29 @@ public void testDiskDefWithBurst() {
"<read_bytes_sec_max_length>"+bytesReadRateMaxLength+"</read_bytes_sec_max_length>\n<write_bytes_sec_max_length>"+bytesWriteRateMaxLength+"</write_bytes_sec_max_length>\n" +
"<read_iops_sec_max_length>"+iopsReadRateMaxLength+"</read_iops_sec_max_length>\n<write_iops_sec_max_length>"+iopsWriteRateMaxLength+"</write_iops_sec_max_length>\n</iotune>\n</disk>\n";

assertEquals(expectedXml, resultingXml);
assertEquals(expectedXml, xmlDef);
}

@Test
public void memBalloonDefTestNone() {
String expectedXml = "<memballoon model='none' autodeflate='on'>\n</memballoon>";
String expectedXml = "<memballoon model='none'>\n</memballoon>";
MemBalloonDef memBalloonDef = new MemBalloonDef();
memBalloonDef.defNoneMemBalloon();

String resultingXml = memBalloonDef.toString();
String xmlDef = memBalloonDef.toString();

assertEquals(expectedXml, resultingXml);
assertEquals(expectedXml, xmlDef);
}

@Test
public void memBalloonDefTestVirtio() {
String expectedXml = "<memballoon model='virtio' autodeflate='on'>\n<stats period='60'/>\n</memballoon>";
String expectedXml = "<memballoon model='virtio'>\n<stats period='60'/>\n</memballoon>";
MemBalloonDef memBalloonDef = new MemBalloonDef();
memBalloonDef.defVirtioMemBalloon("60");

String resultingXml = memBalloonDef.toString();
String xmlDef = memBalloonDef.toString();

assertEquals(expectedXml, resultingXml);
assertEquals(expectedXml, xmlDef);
}

@Test
Expand Down