[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

Author: nvazquez

plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java

@@ -226,6 +226,7 @@ public String toString() {
     protected static final HashMap<VmPowerState, PowerState> 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<VmPowerState, PowerState>();
@@ -1899,13 +1900,25 @@ protected void fillHostInfo(final Connection conn, final StartupRoutingCommand c
         }
     }
 
+    protected void syncPlatformAndCoresPerSocketSettings(String coresPerSocket, Map<String, String> 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<String, String> 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<String, String> platform = StringUtils.stringToMap(platformstring);
+                syncPlatformAndCoresPerSocketSettings(coresPerSocket, platform);
                 vm.setPlatform(conn, platform);
             } else {
                 final String timeoffset = details.get(VmDetailConstants.TIME_OFFSET);
@@ -1914,10 +1927,9 @@ protected void finalizeVmMetaData(final VM vm, final VM.Record vmr, final Connec
                     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<String, String> platform = vm.getPlatform(conn);
-                    platform.put("cores-per-socket", coresPerSocket);
+                    syncPlatformAndCoresPerSocketSettings(coresPerSocket, platform);
                     vm.setPlatform(conn, platform);
                 }
             }

plugins/hypervisors/xenserver/src/test/java/com/cloud/hypervisor/xenserver/resource/CitrixResourceBaseTest.java

@@ -23,6 +23,7 @@
 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.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 @@ protected String getPatchFilePath() {
 
     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 void initializeLocalSrTest() throws XenAPIException, XmlRpcException {
 
         Assert.assertEquals(1, startUpCommandsForLocalStorage.size());
     }
+
+    @Test
+    public void syncPlatformAndCoresPerSocketSettingsEmptyCoresPerSocket() {
+        String coresPerSocket = null;
+        Map<String, String> 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<String, String> 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<String, String> 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));
+    }
 }