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 cdb4d7434aef..8d33f7f873df 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 @@ -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; diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixSetupCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixSetupCommandWrapper.java index 57daba4da41d..d8947e3170f5 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixSetupCommandWrapper.java +++ b/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=") 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 aac865622196..adcb230f63fe 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 @@ -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); + } }