From f5b9a96d24cb0c64c57eb8f1802813ce0f8b3671 Mon Sep 17 00:00:00 2001 From: Koushik Das Date: Wed, 9 Sep 2015 13:00:17 +0530 Subject: [PATCH] CLOUDSTACK-8826: XenServer - Use device id passed as part of attach volume API properly If device id passed as part of API and available then use it otherwise fallback on XS to automatically assign one. For ISO device id used is 3 and it is processed before any other entry to avoid conflict. --- .../resource/CitrixResourceBase.java | 45 +++++++++---------- .../resource/XenServerStorageProcessor.java | 32 ++++--------- .../xenbase/CitrixStartCommandWrapper.java | 21 ++++++--- 3 files changed, 44 insertions(+), 54 deletions(-) diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java index dd44ad81534..9a34cd2c08b 100644 --- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java @@ -1129,18 +1129,19 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe vbdr.bootable = true; } - vbdr.userdevice = Long.toString(volume.getDiskSeq()); if (volume.getType() == Volume.Type.ISO) { vbdr.mode = Types.VbdMode.RO; vbdr.type = Types.VbdType.CD; - } else if (volume.getType() == Volume.Type.ROOT) { - vbdr.mode = Types.VbdMode.RW; - vbdr.type = Types.VbdType.DISK; - vbdr.unpluggable = false; + vbdr.userdevice = "3"; } else { vbdr.mode = Types.VbdMode.RW; vbdr.type = Types.VbdType.DISK; - vbdr.unpluggable = true; + vbdr.unpluggable = (volume.getType() == Volume.Type.ROOT) ? false : true; + vbdr.userdevice = "autodetect"; + final Long deviceId = volume.getDiskSeq(); + if (deviceId != null && !isDeviceUsed(conn, vm, deviceId)) { + vbdr.userdevice = deviceId.toString(); + } } final VBD vbd = VBD.create(conn, vbdr); @@ -1400,7 +1401,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe vbdr.bootable = false; vbdr.unpluggable = true; } - vbdr.userdevice = Long.toString(volumeTO.getDeviceId()); + vbdr.userdevice = "autodetect"; vbdr.mode = Types.VbdMode.RW; vbdr.type = Types.VbdType.DISK; VBD.create(conn, vbdr); @@ -3072,24 +3073,6 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe return com.cloud.host.Host.Type.Routing; } - public String getUnusedDeviceNum(final Connection conn, final VM vm) { - // Figure out the disk number to attach the VM to - try { - final Set allowedVBDDevices = vm.getAllowedVBDDevices(conn); - if (allowedVBDDevices.size() == 0) { - throw new CloudRuntimeException("Could not find an available slot in VM with name: " + vm.getNameLabel(conn) + " to attach a new disk."); - } - return allowedVBDDevices.iterator().next(); - } catch (final XmlRpcException e) { - final String msg = "Catch XmlRpcException due to: " + e.getMessage(); - s_logger.warn(msg, e); - } catch (final XenAPIException e) { - final String msg = "Catch XenAPIException due to: " + e.toString(); - s_logger.warn(msg, e); - } - throw new CloudRuntimeException("Could not find an available slot in VM with name to attach a new disk."); - } - protected VDI getVDIbyLocationandSR(final Connection conn, final String loc, final SR sr) { try { final Set vdis = sr.getVDIs(conn); @@ -3407,6 +3390,18 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe removeSR(conn, sr); } + protected void destroyUnattachedVBD(Connection conn, VM vm) { + try { + for (VBD vbd : vm.getVBDs(conn)) { + if (Types.VbdType.DISK.equals(vbd.getType(conn)) && !vbd.getCurrentlyAttached(conn)) { + vbd.destroy(conn); + } + } + } catch (final Exception e) { + s_logger.debug("Failed to destroy unattached VBD due to ", e); + } + } + public String handleVmStartFailure(final Connection conn, final String vmName, final VM vm, final String message, final Throwable th) { final String msg = "Unable to start " + vmName + " due to " + message; s_logger.warn(msg, th); diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java index f489e5c949e..94cf5df7ee9 100644 --- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java @@ -269,34 +269,18 @@ public class XenServerStorageProcessor implements StorageProcessor { vdi = hypervisorResource.mount(conn, null, null, data.getPath()); } - // Figure out the disk number to attach the VM to - String diskNumber = null; - final Long deviceId = disk.getDiskSeq(); - - if (deviceId != null) { - if (deviceId.longValue() == 3) { - final String msg = "Device 3 is reserved for CD-ROM, choose other device"; - - return new AttachAnswer(msg); - } - - if (hypervisorResource.isDeviceUsed(conn, vm, deviceId)) { - final String msg = "Device " + deviceId + " is used in VM " + vmName; - - return new AttachAnswer(msg); - } - - diskNumber = deviceId.toString(); - } else { - diskNumber = hypervisorResource.getUnusedDeviceNum(conn, vm); - } + hypervisorResource.destroyUnattachedVBD(conn, vm); final VBD.Record vbdr = new VBD.Record(); vbdr.VM = vm; vbdr.VDI = vdi; vbdr.bootable = false; - vbdr.userdevice = diskNumber; + vbdr.userdevice = "autodetect"; + final Long deviceId = disk.getDiskSeq(); + if (deviceId != null && !hypervisorResource.isDeviceUsed(conn, vm, deviceId)) { + vbdr.userdevice = deviceId.toString(); + } vbdr.mode = Types.VbdMode.RW; vbdr.type = Types.VbdType.DISK; vbdr.unpluggable = true; @@ -313,11 +297,11 @@ public class XenServerStorageProcessor implements StorageProcessor { // Update the VDI's label to include the VM name vdi.setNameLabel(conn, vdiNameLabel); - final DiskTO newDisk = new DiskTO(disk.getData(), Long.parseLong(diskNumber), vdi.getUuid(conn), disk.getType()); + final DiskTO newDisk = new DiskTO(disk.getData(), Long.parseLong(vbd.getUserdevice(conn)), vdi.getUuid(conn), disk.getType()); return new AttachAnswer(newDisk); } catch (final Exception e) { - final String msg = "Failed to attach volume" + " for uuid: " + data.getPath() + " due to " + e.toString(); + final String msg = "Failed to attach volume for uuid: " + data.getPath() + " due to " + e.toString(); s_logger.warn(msg, e); return new AttachAnswer(msg); } diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStartCommandWrapper.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStartCommandWrapper.java index c0e4d0a6410..241110253c3 100644 --- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStartCommandWrapper.java +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/wrapper/xenbase/CitrixStartCommandWrapper.java @@ -19,6 +19,7 @@ package com.cloud.hypervisor.xenserver.resource.wrapper.xenbase; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -41,6 +42,7 @@ import com.cloud.network.Networks.BroadcastDomainType; import com.cloud.network.Networks.IsolationType; import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; +import com.cloud.storage.Volume; import com.cloud.vm.VirtualMachine; import com.xensource.xenapi.Connection; import com.xensource.xenapi.Host; @@ -92,22 +94,31 @@ public final class CitrixStartCommandWrapper extends CommandWrapper disks = new ArrayList(vmSpec.getDisks().length); + int index = 0; for (final DiskTO disk : vmSpec.getDisks()) { + if (Volume.Type.ISO.equals(disk.getType())) { + disks.add(0, disk); + } else { + disks.add(index, disk); + } + index++; + } + for (DiskTO disk : disks) { final VDI newVdi = citrixResourceBase.prepareManagedDisk(conn, disk, vmName); if (newVdi != null) { final String path = newVdi.getUuid(conn); - iqnToPath.put(disk.getDetails().get(DiskTO.IQN), path); } citrixResourceBase.createVbd(conn, disk, vmName, vm, vmSpec.getBootloader(), newVdi); } - if (vmSpec.getType() != VirtualMachine.Type.User) { - citrixResourceBase.createPatchVbd(conn, vmName, vm); - } - for (final NicTO nic : vmSpec.getNics()) { citrixResourceBase.createVif(conn, vmName, vm, vmSpec, nic); }