make vlan check conditional on management network for xen

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

@@ -2785,7 +2785,7 @@ protected XsLocalNetwork getManagementNetwork(final Connection conn) throws XmlR
         for (final PIF pif : hostPifs) {
             final PIF.Record rec = pif.getRecord(conn);
             if (rec.management) {
-                if (rec.VLAN != null && rec.VLAN != -1) {
+                if (!isVlanAllowedForManagementPif(rec)) {
                     final String msg = "Unsupported configuration.  Management network is on a VLAN.  host=" + _host.getUuid() + "; pif=" + rec.uuid +
                             "; vlan=" + rec.VLAN;
                     logger.warn(msg);
@@ -5817,6 +5817,26 @@ public boolean isDestroyHaltedVms() {
         return version.compareTo(new ComparableVersion("8.0")) < 0;
     }
 
+    /**
+     * XenServer and XCP-ng support the management interface on a tagged VLAN from product version 8.0 onwards.
+     * The XenAPI protocol version (Host.getAPIVersionMajor) cannot be used for this; it does not follow the product version.
+     */
+    public boolean isManagementNetworkOnVlanSupported() {
+        String productVersion = getHost().getProductVersion();
+        if (StringUtils.isBlank(productVersion)) {
+            return false;
+        }
+        return new ComparableVersion(productVersion).compareTo(new ComparableVersion("8.0")) >= 0;
+    }
+
+    /**
+     * Whether a PIF may carry the management (or cloud-private) network: untagged PIFs always may, tagged ones only
+     * on hosts that support the management interface on a VLAN.
+     */
+    public boolean isVlanAllowedForManagementPif(PIF.Record rec) {
+        return rec.VLAN == null || rec.VLAN == -1 || isManagementNetworkOnVlanSupported();
+    }
+
     public void destroyVm(VM vm, Connection connection, boolean forced) throws XenAPIException, XmlRpcException {
         if (!isDestroyHaltedVms() && !forced) {
             return;

plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixSetupCommandWrapper.java

@@ -104,7 +104,7 @@ public Answer execute(final SetupCommand command, final CitrixResourceBase citri
                 for (final PIF pif : hostPifs) {
                     final PIF.Record rec = pif.getRecord(conn);
                     if (rec.management) {
-                        if (rec.VLAN != null && rec.VLAN != -1) {
+                        if (!citrixResourceBase.isVlanAllowedForManagementPif(rec)) {
                             final String msg =
                                     new StringBuilder("Unsupported configuration.  Management network is on a VLAN.  host=").append(citrixResourceBase.getHost().getUuid())
                                     .append("; pif=")
@@ -143,7 +143,7 @@ public Answer execute(final SetupCommand command, final CitrixResourceBase citri
                                 if (logger.isDebugEnabled()) {
                                     logger.debug("Found a network called cloud-private. host=" + citrixResourceBase.getHost().getUuid() + ";  Network=" + network.uuid + "; pif=" + pr.uuid);
                                 }
-                                if (pr.VLAN != null && pr.VLAN != -1) {
+                                if (!citrixResourceBase.isVlanAllowedForManagementPif(pr)) {
                                     final String msg =
                                             new StringBuilder("Unsupported configuration.  Network cloud-private is on a VLAN.  Network=").append(network.uuid)
                                             .append(" ; pif=")

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

@@ -56,11 +56,14 @@
 import com.cloud.template.TemplateManager;
 import com.cloud.utils.ExecutionResult;
 import com.cloud.utils.StringUtils;
+import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.script.Script;
 import com.xensource.xenapi.Connection;
 import com.xensource.xenapi.Host;
 import com.xensource.xenapi.Host.Record;
+import com.xensource.xenapi.Network;
 import com.xensource.xenapi.PBD;
+import com.xensource.xenapi.PIF;
 import com.xensource.xenapi.SR;
 import com.xensource.xenapi.Types.XenAPIException;
 import org.mockito.junit.MockitoJUnitRunner;
@@ -616,4 +619,81 @@ public void testListFilesAtPathWithFile() throws IOException, XmlRpcException {
         Assert.assertEquals(1024L, listAnswer.getSizes().get(0).longValue());
         Assert.assertEquals(123456789000L, listAnswer.getLastModified().get(0).longValue());
     }
+
+    @Test
+    public void isManagementNetworkOnVlanSupportedTestBlankVersion() {
+        citrixResourceBase._host.setProductVersion(null);
+        Assert.assertFalse(citrixResourceBase.isManagementNetworkOnVlanSupported());
+        citrixResourceBase._host.setProductVersion("");
+        Assert.assertFalse(citrixResourceBase.isManagementNetworkOnVlanSupported());
+    }
+
+    @Test
+    public void isManagementNetworkOnVlanSupportedTestBefore8() {
+        for (String version : new String[] {"6.5.0", "7.1.2", "7.6.0"}) {
+            citrixResourceBase._host.setProductVersion(version);
+            Assert.assertFalse(version, citrixResourceBase.isManagementNetworkOnVlanSupported());
+        }
+    }
+
+    @Test
+    public void isManagementNetworkOnVlanSupportedTest8AndLater() {
+        for (String version : new String[] {"8.0", "8.0.0", "8.2.1", "8.3.0", "8.4.0"}) {
+            citrixResourceBase._host.setProductVersion(version);
+            Assert.assertTrue(version, citrixResourceBase.isManagementNetworkOnVlanSupported());
+        }
+    }
+
+    private PIF.Record pifRecordWithVlan(Long vlan) {
+        PIF.Record rec = new PIF.Record();
+        rec.uuid = "pifUuid";
+        rec.VLAN = vlan;
+        return rec;
+    }
+
+    @Test
+    public void isVlanAllowedForManagementPifTestUntagged() {
+        citrixResourceBase._host.setProductVersion("7.1.2");
+        Assert.assertTrue(citrixResourceBase.isVlanAllowedForManagementPif(pifRecordWithVlan(null)));
+        Assert.assertTrue(citrixResourceBase.isVlanAllowedForManagementPif(pifRecordWithVlan(-1L)));
+    }
+
+    @Test
+    public void isVlanAllowedForManagementPifTestTagged() {
+        citrixResourceBase._host.setProductVersion("7.1.2");
+        Assert.assertFalse(citrixResourceBase.isVlanAllowedForManagementPif(pifRecordWithVlan(100L)));
+        citrixResourceBase._host.setProductVersion("8.2.1");
+        Assert.assertTrue(citrixResourceBase.isVlanAllowedForManagementPif(pifRecordWithVlan(100L)));
+    }
+
+    private PIF.Record mockManagementPifOnVlan(long vlan) throws XenAPIException, XmlRpcException {
+        PIF pif = Mockito.mock(PIF.class);
+        PIF.Record rec = pifRecordWithVlan(vlan);
+        rec.management = true;
+        rec.network = Mockito.mock(Network.class);
+        Mockito.when(pif.getRecord(connectionMock)).thenReturn(rec);
+        Mockito.when(hostMock.getPIFs(connectionMock)).thenReturn(Set.of(pif));
+        return rec;
+    }
+
+    @Test
+    public void getManagementNetworkTestVlanOnProductVersion8() throws XenAPIException, XmlRpcException {
+        citrixResourceBase._host.setProductVersion("8.2.1");
+        PIF.Record pifRecord = mockManagementPifOnVlan(100L);
+        Network.Record networkRecord = new Network.Record();
+        Mockito.when(pifRecord.network.getRecord(connectionMock)).thenReturn(networkRecord);
+
+        XsLocalNetwork network = citrixResourceBase.getManagementNetwork(connectionMock);
+
+        Assert.assertSame(networkRecord, network.getNetworkRecord(connectionMock));
+        Assert.assertSame(pifRecord, network.getPifRecord(connectionMock));
+    }
+
+    @Test(expected = CloudRuntimeException.class)
+    public void getManagementNetworkTestVlanBeforeProductVersion8() throws XenAPIException, XmlRpcException {
+        citrixResourceBase._host.setProductVersion("7.1.2");
+        mockManagementPifOnVlan(100L);
+
+        citrixResourceBase.getManagementNetwork(connectionMock);
+    }
 }