CLOUDSTACK-7077. Quickly attaching multiple data disks to a VM fails. [VMware] Synchronize the tasks of disk preparation and reconfiguration of a VM with the disk. This is to avoid attaching a disk with a wrong device number on the controller key.
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/a9b3ab08 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/a9b3ab08 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/a9b3ab08 Branch: refs/heads/vpc-toolkit-hugo Commit: a9b3ab089d6cb0bc1494e10b2be90292f4c79289 Parents: 62c18ca Author: Likitha Shetty <likitha.she...@citrix.com> Authored: Fri May 30 11:09:43 2014 +0530 Committer: Likitha Shetty <likitha.she...@citrix.com> Committed: Tue Jul 8 13:24:13 2014 +0530 ---------------------------------------------------------------------- .../hypervisor/vmware/mo/VirtualMachineMO.java | 108 ++++++++++++------- 1 file changed, 69 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/a9b3ab08/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java ---------------------------------------------------------------------- diff --git a/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java b/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java index b5b9a5e..df05f8a 100644 --- a/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java +++ b/vmware-base/src/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java @@ -107,11 +107,13 @@ import com.cloud.utils.ActionDelegate; import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.utils.concurrency.NamedThreadFactory; +import com.cloud.utils.db.GlobalLock; import com.cloud.utils.script.Script; public class VirtualMachineMO extends BaseMO { private static final Logger s_logger = Logger.getLogger(VirtualMachineMO.class); private static final ExecutorService MonitorServiceExecutor = Executors.newCachedThreadPool(new NamedThreadFactory("VM-Question-Monitor")); + private static final int ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_DISK_ATTACH = 5 * 60; // Wait for a maximum of 5 minutes to prepare a disk while VM is being re-configured with another disk private ManagedObjectReference _vmEnvironmentBrowser = null; public VirtualMachineMO(VmwareContext context, ManagedObjectReference morVm) { @@ -1050,62 +1052,90 @@ public class VirtualMachineMO extends BaseMO { } public void attachDisk(String[] vmdkDatastorePathChain, ManagedObjectReference morDs) throws Exception { + // Add lock to ensure that only one disk is being prepared and attached to the VM at a time + GlobalLock lock = GlobalLock.getInternLock("disk.attach"); + try { + s_logger.trace("Grabbing lock to ensure that only one disk is being prepared and attached to the VM at a time."); + if (lock.lock(ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_DISK_ATTACH)) { + try { + if (s_logger.isTraceEnabled()) + s_logger.trace("vCenter API trace - attachDisk(). target MOR: " + _mor.getValue() + ", vmdkDatastorePath: " + new Gson().toJson(vmdkDatastorePathChain) + + ", datastore: " + morDs.getValue()); - if (s_logger.isTraceEnabled()) - s_logger.trace("vCenter API trace - attachDisk(). target MOR: " + _mor.getValue() + ", vmdkDatastorePath: " + new Gson().toJson(vmdkDatastorePathChain) + - ", datastore: " + morDs.getValue()); - - VirtualDevice newDisk = VmwareHelper.prepareDiskDevice(this, null, getScsiDeviceControllerKey(), vmdkDatastorePathChain, morDs, -1, 1); - VirtualMachineConfigSpec reConfigSpec = new VirtualMachineConfigSpec(); - VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec(); + VirtualDevice newDisk = VmwareHelper.prepareDiskDevice(this, null, getScsiDeviceControllerKey(), vmdkDatastorePathChain, morDs, -1, 1); + VirtualMachineConfigSpec reConfigSpec = new VirtualMachineConfigSpec(); + VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec(); - deviceConfigSpec.setDevice(newDisk); - deviceConfigSpec.setOperation(VirtualDeviceConfigSpecOperation.ADD); + deviceConfigSpec.setDevice(newDisk); + deviceConfigSpec.setOperation(VirtualDeviceConfigSpecOperation.ADD); - reConfigSpec.getDeviceChange().add(deviceConfigSpec); + reConfigSpec.getDeviceChange().add(deviceConfigSpec); - ManagedObjectReference morTask = _context.getService().reconfigVMTask(_mor, reConfigSpec); - boolean result = _context.getVimClient().waitForTask(morTask); + ManagedObjectReference morTask = _context.getService().reconfigVMTask(_mor, reConfigSpec); + boolean result = _context.getVimClient().waitForTask(morTask); - if (!result) { - if (s_logger.isTraceEnabled()) - s_logger.trace("vCenter API trace - attachDisk() done(failed)"); - throw new Exception("Failed to attach disk due to " + TaskMO.getTaskFailureInfo(_context, morTask)); - } + if (!result) { + if (s_logger.isTraceEnabled()) + s_logger.trace("vCenter API trace - attachDisk() done(failed)"); + throw new Exception("Failed to attach disk due to " + TaskMO.getTaskFailureInfo(_context, morTask)); + } - _context.waitForTaskProgressDone(morTask); + _context.waitForTaskProgressDone(morTask); - if (s_logger.isTraceEnabled()) - s_logger.trace("vCenter API trace - attachDisk() done(successfully)"); + if (s_logger.isTraceEnabled()) + s_logger.trace("vCenter API trace - attachDisk() done(successfully)"); + } finally { + lock.unlock(); + } + } else { + s_logger.warn("Couldn't get lock on VM: " + _mor.getValue() + " to attach disk: " + vmdkDatastorePathChain + " ,maybe another disk is being attached to the VM."); + } + } finally { + lock.releaseRef(); + } } public void attachDisk(Pair<String, ManagedObjectReference>[] vmdkDatastorePathChain, int controllerKey) throws Exception { + // Add lock to ensure that only one disk is being prepared and attached to the VM at a time + GlobalLock lock = GlobalLock.getInternLock("disk.attach"); + try { + s_logger.trace("Grabbing lock to ensure that only one disk is being prepared and attached to the VM at a time."); + if (lock.lock(ACQUIRE_GLOBAL_LOCK_TIMEOUT_FOR_DISK_ATTACH)) { + try { + if (s_logger.isTraceEnabled()) + s_logger.trace("vCenter API trace - attachDisk(). target MOR: " + _mor.getValue() + ", vmdkDatastorePath: " + new Gson().toJson(vmdkDatastorePathChain)); - if (s_logger.isTraceEnabled()) - s_logger.trace("vCenter API trace - attachDisk(). target MOR: " + _mor.getValue() + ", vmdkDatastorePath: " + new Gson().toJson(vmdkDatastorePathChain)); + VirtualDevice newDisk = VmwareHelper.prepareDiskDevice(this, controllerKey, vmdkDatastorePathChain, -1, 1); + VirtualMachineConfigSpec reConfigSpec = new VirtualMachineConfigSpec(); + VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec(); - VirtualDevice newDisk = VmwareHelper.prepareDiskDevice(this, controllerKey, vmdkDatastorePathChain, -1, 1); - VirtualMachineConfigSpec reConfigSpec = new VirtualMachineConfigSpec(); - VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec(); + deviceConfigSpec.setDevice(newDisk); + deviceConfigSpec.setOperation(VirtualDeviceConfigSpecOperation.ADD); - deviceConfigSpec.setDevice(newDisk); - deviceConfigSpec.setOperation(VirtualDeviceConfigSpecOperation.ADD); + reConfigSpec.getDeviceChange().add(deviceConfigSpec); - reConfigSpec.getDeviceChange().add(deviceConfigSpec); + ManagedObjectReference morTask = _context.getService().reconfigVMTask(_mor, reConfigSpec); + boolean result = _context.getVimClient().waitForTask(morTask); - ManagedObjectReference morTask = _context.getService().reconfigVMTask(_mor, reConfigSpec); - boolean result = _context.getVimClient().waitForTask(morTask); + if (!result) { + if (s_logger.isTraceEnabled()) + s_logger.trace("vCenter API trace - attachDisk() done(failed)"); + throw new Exception("Failed to attach disk due to " + TaskMO.getTaskFailureInfo(_context, morTask)); + } - if (!result) { - if (s_logger.isTraceEnabled()) - s_logger.trace("vCenter API trace - attachDisk() done(failed)"); - throw new Exception("Failed to attach disk due to " + TaskMO.getTaskFailureInfo(_context, morTask)); - } - - _context.waitForTaskProgressDone(morTask); + _context.waitForTaskProgressDone(morTask); - if (s_logger.isTraceEnabled()) - s_logger.trace("vCenter API trace - attachDisk() done(successfully)"); + if (s_logger.isTraceEnabled()) + s_logger.trace("vCenter API trace - attachDisk() done(successfully)"); + } finally { + lock.unlock(); + } + } else { + s_logger.warn("Couldn't get lock on VM: " + _mor.getValue() + " to attach disk: " + vmdkDatastorePathChain + " ,maybe another disk is being attached to the VM."); + } + } finally { + lock.releaseRef(); + } } // vmdkDatastorePath: [datastore name] vmdkFilePath
