From d77fd742f5cea0216f34bb7f454b2f4b712c2b47 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 14 Jul 2025 13:47:36 +0200 Subject: [PATCH 1/4] make vlan check conditional on management network for xen --- .../wrapper/xenbase/CitrixSetupCommandWrapper.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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..2d961f700104 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,14 +104,14 @@ 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 (host.getAPIVersionMajor(conn) < 8 && rec.VLAN != null && rec.VLAN != -1) { final String msg = new StringBuilder("Unsupported configuration. Management network is on a VLAN. host=").append(citrixResourceBase.getHost().getUuid()) - .append("; pif=") - .append(rec.uuid) - .append("; vlan=") - .append(rec.VLAN) - .toString(); + .append("; pif=") + .append(rec.uuid) + .append("; vlan=") + .append(rec.VLAN) + .toString(); logger.warn(msg); return new SetupAnswer(command, msg); } From 2edd41deef2524437e2bbcbf5543981ee700b3e9 Mon Sep 17 00:00:00 2001 From: dahn Date: Fri, 30 Oct 2026 16:12:54 +0100 Subject: [PATCH 2/4] null check on api version Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../wrapper/xenbase/CitrixSetupCommandWrapper.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 2d961f700104..4f24ae35b412 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,13 @@ 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 (host.getAPIVersionMajor(conn) < 8 && rec.VLAN != null && rec.VLAN != -1) { + Integer apiVersionMajor = host.getAPIVersionMajor(conn); + if (apiVersionMajor == null) { + final String msg = "Unable to determine API version for host: " + citrixResourceBase.getHost().getUuid(); + logger.warn(msg); + return new SetupAnswer(command, msg); + } + if (apiVersionMajor < 8 && rec.VLAN != null && rec.VLAN != -1) { final String msg = new StringBuilder("Unsupported configuration. Management network is on a VLAN. host=").append(citrixResourceBase.getHost().getUuid()) .append("; pif=") From d5be1eafd50ab85a6400cbb5fcb031f56afc40d0 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Mon, 2 Nov 2026 11:45:42 +0100 Subject: [PATCH 3/4] xenserver: use product version, not XenAPI version, to allow management network on VLAN (8.0+) --- .../resource/CitrixResourceBase.java | 12 ++++++++++ .../xenbase/CitrixSetupCommandWrapper.java | 18 +++++--------- .../resource/CitrixResourceBaseTest.java | 24 +++++++++++++++++++ 3 files changed, 42 insertions(+), 12 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 cdb4d7434aef..1adbf0488277 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 @@ -5817,6 +5817,18 @@ 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; + } + 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 4f24ae35b412..b33ee726e228 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,20 +104,14 @@ public Answer execute(final SetupCommand command, final CitrixResourceBase citri for (final PIF pif : hostPifs) { final PIF.Record rec = pif.getRecord(conn); if (rec.management) { - Integer apiVersionMajor = host.getAPIVersionMajor(conn); - if (apiVersionMajor == null) { - final String msg = "Unable to determine API version for host: " + citrixResourceBase.getHost().getUuid(); - logger.warn(msg); - return new SetupAnswer(command, msg); - } - if (apiVersionMajor < 8 && rec.VLAN != null && rec.VLAN != -1) { + if (rec.VLAN != null && rec.VLAN != -1 && !citrixResourceBase.isManagementNetworkOnVlanSupported()) { final String msg = new StringBuilder("Unsupported configuration. Management network is on a VLAN. host=").append(citrixResourceBase.getHost().getUuid()) - .append("; pif=") - .append(rec.uuid) - .append("; vlan=") - .append(rec.VLAN) - .toString(); + .append("; pif=") + .append(rec.uuid) + .append("; vlan=") + .append(rec.VLAN) + .toString(); logger.warn(msg); return new SetupAnswer(command, msg); } 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..e3366421599c 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 @@ -616,4 +616,28 @@ 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()); + } + } } From 8c93891ca700bff19b5f44b73eaf399eef53e09b Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Tue, 3 Nov 2026 14:38:39 +0100 Subject: [PATCH 4/4] xenserver: allow tagged management and cloud-private PIFs on 8.0+ during host init and setup --- .../resource/CitrixResourceBase.java | 10 +++- .../xenbase/CitrixSetupCommandWrapper.java | 4 +- .../resource/CitrixResourceBaseTest.java | 56 +++++++++++++++++++ 3 files changed, 67 insertions(+), 3 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 1adbf0488277..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); @@ -5829,6 +5829,14 @@ public boolean isManagementNetworkOnVlanSupported() { 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 b33ee726e228..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 && !citrixResourceBase.isManagementNetworkOnVlanSupported()) { + 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 e3366421599c..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; @@ -640,4 +643,57 @@ public void isManagementNetworkOnVlanSupportedTest8AndLater() { 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); + } }