From ddd311c695d1c77b0cca159ab2fd04bd40affa2d Mon Sep 17 00:00:00 2001 From: Nicolas Vazquez Date: Wed, 2 Feb 2022 03:51:47 -0300 Subject: [PATCH] [XenServer/XCP-ng] Sync the 'platform' setting according to the 'cpu.corespersocket' setting (#5892) * Update platform setting if cpu.corespersocket is set * Refactor conditions * Allow empty map but not null --- .../resource/CitrixResourceBase.java | 16 +++++++-- .../resource/CitrixResourceBaseTest.java | 33 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java index 9348bcaab9f..19625a2f09a 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java @@ -226,6 +226,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe protected static final HashMap s_powerStatesTable; public static final String XS_TOOLS_ISO_AFTER_70 = "guest-tools.iso"; + protected static final String PLATFORM_CORES_PER_SOCKET_KEY = "cores-per-socket"; static { s_powerStatesTable = new HashMap(); @@ -1899,13 +1900,25 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe } } + protected void syncPlatformAndCoresPerSocketSettings(String coresPerSocket, Map platform) { + if (org.apache.commons.lang3.StringUtils.isBlank(coresPerSocket) || platform == null) { + return; + } + if (platform.containsKey(PLATFORM_CORES_PER_SOCKET_KEY)) { + s_logger.debug("Updating the cores per socket value from: " + platform.get(PLATFORM_CORES_PER_SOCKET_KEY) + " to " + coresPerSocket); + } + platform.put(PLATFORM_CORES_PER_SOCKET_KEY, coresPerSocket); + } + protected void finalizeVmMetaData(final VM vm, final VM.Record vmr, final Connection conn, final VirtualMachineTO vmSpec) throws Exception { final Map details = vmSpec.getDetails(); if (details != null) { final String platformstring = details.get(VmDetailConstants.PLATFORM); + final String coresPerSocket = details.get(VmDetailConstants.CPU_CORE_PER_SOCKET); if (platformstring != null && !platformstring.isEmpty()) { final Map platform = StringUtils.stringToMap(platformstring); + syncPlatformAndCoresPerSocketSettings(coresPerSocket, platform); vm.setPlatform(conn, platform); } else { final String timeoffset = details.get(VmDetailConstants.TIME_OFFSET); @@ -1914,10 +1927,9 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe platform.put(VmDetailConstants.TIME_OFFSET, timeoffset); vm.setPlatform(conn, platform); } - final String coresPerSocket = details.get(VmDetailConstants.CPU_CORE_PER_SOCKET); if (coresPerSocket != null) { final Map platform = vm.getPlatform(conn); - platform.put("cores-per-socket", coresPerSocket); + syncPlatformAndCoresPerSocketSettings(coresPerSocket, platform); vm.setPlatform(conn, platform); } } diff --git a/plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBaseTest.java b/plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBaseTest.java index b1a89c9da82..e40c7e89cee 100644 --- a/plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBaseTest.java +++ b/plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBaseTest.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import com.cloud.utils.StringUtils; import org.apache.xmlrpc.XmlRpcException; import org.junit.Assert; import org.junit.Before; @@ -51,6 +52,8 @@ import com.xensource.xenapi.PBD; import com.xensource.xenapi.SR; import com.xensource.xenapi.Types.XenAPIException; +import static com.cloud.hypervisor.xenserver.resource.CitrixResourceBase.PLATFORM_CORES_PER_SOCKET_KEY; + @RunWith(PowerMockRunner.class) @PrepareForTest({Host.class, Script.class, SR.class}) public class CitrixResourceBaseTest { @@ -72,6 +75,8 @@ public class CitrixResourceBaseTest { private String hostUuidMock = "hostUuidMock"; + private static final String platformString = "device-model:qemu-upstream-compat;vga:std;videoram:8;apic:true;viridian:false;timeoffset:0;pae:true;acpi:1;hpet:true;secureboot:false;nx:true"; + @Before public void beforeTest() throws XenAPIException, XmlRpcException { citrixResourceBase._host.setUuid(hostUuidMock); @@ -369,4 +374,32 @@ public class CitrixResourceBaseTest { Assert.assertEquals(1, startUpCommandsForLocalStorage.size()); } + + @Test + public void syncPlatformAndCoresPerSocketSettingsEmptyCoresPerSocket() { + String coresPerSocket = null; + Map platform = Mockito.mock(Map.class); + citrixResourceBase.syncPlatformAndCoresPerSocketSettings(coresPerSocket, platform); + Mockito.verify(platform, Mockito.never()).put(Mockito.any(), Mockito.any()); + Mockito.verify(platform, Mockito.never()).remove(Mockito.any()); + } + + @Test + public void syncPlatformAndCoresPerSocketSettingsEmptyCoresPerSocketOnPlatform() { + String coresPerSocket = "2"; + Map platform = StringUtils.stringToMap(platformString); + citrixResourceBase.syncPlatformAndCoresPerSocketSettings(coresPerSocket, platform); + Assert.assertTrue(platform.containsKey(PLATFORM_CORES_PER_SOCKET_KEY)); + Assert.assertEquals(coresPerSocket, platform.get(PLATFORM_CORES_PER_SOCKET_KEY)); + } + + @Test + public void syncPlatformAndCoresPerSocketSettingsUpdateCoresPerSocketOnPlatform() { + String coresPerSocket = "2"; + String platformStr = platformString + "," + PLATFORM_CORES_PER_SOCKET_KEY + ":3"; + Map platform = StringUtils.stringToMap(platformStr); + citrixResourceBase.syncPlatformAndCoresPerSocketSettings(coresPerSocket, platform); + Assert.assertTrue(platform.containsKey(PLATFORM_CORES_PER_SOCKET_KEY)); + Assert.assertEquals(coresPerSocket, platform.get(PLATFORM_CORES_PER_SOCKET_KEY)); + } }