From 8a3e3a3f6c099edf9b8d532827f8fdafde160439 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 11 Jun 2024 15:36:03 +0530 Subject: [PATCH 1/2] xenserver: attach regular iso with configdrive Fixes #7902 This PR allows attaching a regular ISO to a VM when it already has the config drive ISO attached. Config-drive ISO is now attached with the SR name-label -CONFIGDRIVE-ISO. While regular ISOs continue to attach with SR name-label -ISO. VM which already have the configdrive ISO attached before this fix will return an appropriate error and will need to be stopped-start. Signed-off-by: Abhishek Kumar --- .../resource/CitrixResourceBase.java | 26 +++++++++++++++---- .../xenbase/CitrixStopCommandWrapper.java | 4 ++- 2 files changed, 24 insertions(+), 6 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 90473705a53e..024630f36aca 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 @@ -62,6 +62,7 @@ import org.apache.cloudstack.hypervisor.xenserver.ExtraConfigurationUtility; import org.apache.cloudstack.storage.command.browser.ListDataStoreObjectsAnswer; import org.apache.cloudstack.storage.command.browser.ListDataStoreObjectsCommand; +import org.apache.cloudstack.storage.configdrive.ConfigDrive; import org.apache.cloudstack.storage.to.TemplateObjectTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.cloudstack.utils.security.ParserUtils; @@ -222,6 +223,8 @@ public String toString() { private final static String VM_NAME_ISO_SUFFIX = "-ISO"; + private final static String VM_NAME_CONFIGDRIVE_ISO_SUFFIX = "-CONFIGDRIVE-ISO"; + private final static String VM_FILE_ISO_SUFFIX = ".iso"; public final static int DEFAULTDOMRSSHPORT = 3922; @@ -1020,12 +1023,13 @@ public ExecutionResult copyPatchFilesToVR(final String routerIp, final String pa protected SR createIsoSRbyURI(final Connection conn, final URI uri, final String vmName, final boolean shared) { try { final Map deviceConfig = new HashMap(); + final boolean isConfigDrive = uri.toString().endsWith(ConfigDrive.CONFIGDRIVEDIR); String path = uri.getPath(); path = path.replace("//", "/"); deviceConfig.put("location", uri.getHost() + ":" + path); final Host host = Host.getByUuid(conn, _host.getUuid()); final SR sr = SR.create(conn, host, deviceConfig, new Long(0), uri.getHost() + path, "iso", "iso", "iso", shared, new HashMap()); - sr.setNameLabel(conn, vmName + "-ISO"); + sr.setNameLabel(conn, vmName + (isConfigDrive ? VM_NAME_CONFIGDRIVE_ISO_SUFFIX: VM_NAME_ISO_SUFFIX)); sr.setNameDescription(conn, deviceConfig.get("location")); sr.scan(conn); @@ -2648,9 +2652,10 @@ private String probeScisiId(Connection conn, Host host, Map devi return scsiid; } - public SR getISOSRbyVmName(final Connection conn, final String vmName) { + public SR getISOSRbyVmName(final Connection conn, final String vmName, boolean isConfigDrive) { try { - final Set srs = SR.getByNameLabel(conn, vmName + "-ISO"); + final Set srs = SR.getByNameLabel(conn, vmName + + (isConfigDrive ? VM_NAME_CONFIGDRIVE_ISO_SUFFIX : VM_NAME_ISO_SUFFIX)); if (srs.size() == 0) { return null; } else if (srs.size() == 1) { @@ -2697,9 +2702,20 @@ public VDI getIsoVDIByURL(final Connection conn, final String vmName, final Stri } catch (final URISyntaxException e) { throw new CloudRuntimeException("isoURL is wrong: " + isoURL); } - isoSR = getISOSRbyVmName(conn, vmName); + isoSR = getISOSRbyVmName(conn, vmName, false); if (isoSR == null) { isoSR = createIsoSRbyURI(conn, uri, vmName, false); + } else { + try { + String description = isoSR.getNameDescription(conn); + if (description.endsWith(ConfigDrive.CONFIGDRIVEDIR)) { + throw new CloudRuntimeException(String.format("VM %s already has %s ISO attached. Please " + + "stop-start VM for allowing attaching both ISOs", vmName, ConfigDrive.CONFIGDRIVEDIR)); + } + } catch (XenAPIException | XmlRpcException e) { + throw new CloudRuntimeException(String.format("Unable to retrieve name description for the already " + + "attached ISO on VM %s", vmName)); + } } final String isoName = isoURL.substring(index + 1); @@ -5667,7 +5683,7 @@ public boolean attachConfigDriveToMigratedVm(Connection conn, String vmName, Str s_logger.debug("Attaching config drive iso device for the VM " + vmName + " In host " + ipAddr); Set vms = VM.getByNameLabel(conn, vmName); - SR sr = getSRByNameLabel(conn, vmName + VM_NAME_ISO_SUFFIX); + SR sr = getSRByNameLabel(conn, vmName + VM_NAME_CONFIGDRIVE_ISO_SUFFIX); //Here you will find only two vdis with the .iso. //one is from source host and second from dest host Set vdis = VDI.getByNameLabel(conn, vmName + VM_FILE_ISO_SUFFIX); diff --git a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStopCommandWrapper.java b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStopCommandWrapper.java index 45171a49f9ff..8e7eb4caec29 100644 --- a/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStopCommandWrapper.java +++ b/plugins/hypervisors/xenserver/src/main/java/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStopCommandWrapper.java @@ -144,8 +144,10 @@ public Answer execute(final StopCommand command, final CitrixResourceBase citrix networks.add(vif.getNetwork(conn)); } vm.destroy(conn); - final SR sr = citrixResourceBase.getISOSRbyVmName(conn, command.getVmName()); + final SR sr = citrixResourceBase.getISOSRbyVmName(conn, command.getVmName(), false); citrixResourceBase.removeSR(conn, sr); + final SR configDriveSR = citrixResourceBase.getISOSRbyVmName(conn, command.getVmName(), true); + citrixResourceBase.removeSR(conn, configDriveSR); // Disable any VLAN networks that aren't used // anymore for (final Network network : networks) { From 5db9811559cd711cbc39f8c5e325f5f5914296b8 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 12 Jun 2024 13:29:08 +0530 Subject: [PATCH 2/2] refactor error message --- .../cloud/hypervisor/xenserver/resource/CitrixResourceBase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 024630f36aca..f53a70de7228 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 @@ -2710,7 +2710,7 @@ public VDI getIsoVDIByURL(final Connection conn, final String vmName, final Stri String description = isoSR.getNameDescription(conn); if (description.endsWith(ConfigDrive.CONFIGDRIVEDIR)) { throw new CloudRuntimeException(String.format("VM %s already has %s ISO attached. Please " + - "stop-start VM for allowing attaching both ISOs", vmName, ConfigDrive.CONFIGDRIVEDIR)); + "stop-start VM to allow attaching-detaching both ISOs", vmName, ConfigDrive.CONFIGDRIVEDIR)); } } catch (XenAPIException | XmlRpcException e) { throw new CloudRuntimeException(String.format("Unable to retrieve name description for the already " +