Updated Branches: refs/heads/master 5502f4b20 -> 3ffbc4320
CLOUDSTACK-595 : Recreate root volume scenarios doesn't work in vmware Issue seen during system vm template upgrade and restoreVM command scenarios for vmware. In these cases CS tries to recreate root disk with same name as the existing one, in case of vmware this results in creation of vmdk file with same name for both existing and new root volume. This results in undesired behavior when storage cleanup thread tries to cleanup old volume. Made the vmdk file name unique by adding the volume id to it. This will ensure that during volume recreation in the scenarios mentioned vmdk will get created with a new name and there will be no undesired side effects of running the storage cleanup thread. Project: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/commit/3ffbc432 Tree: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/3ffbc432 Diff: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/3ffbc432 Branch: refs/heads/master Commit: 3ffbc4320abc9fd03ad3abd1f8077cc8d8f9ed4c Parents: 5502f4b Author: Koushik Das <[email protected]> Authored: Mon Jan 7 16:08:04 2013 +0530 Committer: Murali Reddy <[email protected]> Committed: Mon Jan 7 16:18:05 2013 +0530 ---------------------------------------------------------------------- .../hypervisor/vmware/resource/VmwareResource.java | 45 +++++++-------- .../src/com/cloud/storage/StorageManagerImpl.java | 18 ++++-- 2 files changed, 34 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/3ffbc432/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 0b0f285..b1b15bb 100755 --- a/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -3705,10 +3705,12 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa ManagedObjectReference morDatastore = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, pool.getUuid()); if (morDatastore == null) throw new Exception("Unable to find datastore in vSphere"); - + DatastoreMO dsMo = new DatastoreMO(context, morDatastore); - if (cmd.getDiskCharacteristics().getType() == Volume.Type.ROOT) { + if (dskch.getType() == Volume.Type.ROOT) { + // attach volume id to make the name unique + String vmdkName = dskch.getName() + "-" + dskch.getVolumeId(); if (cmd.getTemplateUrl() == null) { // create a root volume for blank VM String dummyVmName = getWorkerName(context, cmd, 0); @@ -3720,16 +3722,15 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa throw new Exception("Unable to create a dummy VM for volume creation"); } - String volumeDatastorePath = String.format("[%s] %s.vmdk", dsMo.getName(), cmd.getDiskCharacteristics().getName()); + String volumeDatastorePath = String.format("[%s] %s.vmdk", dsMo.getName(), vmdkName); synchronized (this) { s_logger.info("Delete file if exists in datastore to clear the way for creating the volume. file: " + volumeDatastorePath); - VmwareHelper.deleteVolumeVmdkFiles(dsMo, cmd.getDiskCharacteristics().getName(), dcMo); - vmMo.createDisk(volumeDatastorePath, (int) (cmd.getDiskCharacteristics().getSize() / (1024L * 1024L)), morDatastore, -1); + VmwareHelper.deleteVolumeVmdkFiles(dsMo, vmdkName, dcMo); + vmMo.createDisk(volumeDatastorePath, (int) (dskch.getSize() / (1024L * 1024L)), morDatastore, -1); vmMo.detachDisk(volumeDatastorePath, false); } - VolumeTO vol = new VolumeTO(cmd.getVolumeId(), dskch.getType(), pool.getType(), pool.getUuid(), cmd.getDiskCharacteristics().getName(), pool.getPath(), cmd - .getDiskCharacteristics().getName(), cmd.getDiskCharacteristics().getSize(), null); + VolumeTO vol = new VolumeTO(cmd.getVolumeId(), dskch.getType(), pool.getType(), pool.getUuid(), dskch.getName(), pool.getPath(), vmdkName, dskch.getSize(), null); return new CreateAnswer(cmd, vol); } finally { vmMo.detachAllDisks(); @@ -3753,41 +3754,40 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa throw new Exception(msg); } - String name = cmd.getDiskCharacteristics().getName(); - if(dsMo.folderExists(String.format("[%s]", dsMo.getName()), name)) - dsMo.deleteFile(String.format("[%s] %s/", dsMo.getName(), name), dcMo.getMor(), false); + if(dsMo.folderExists(String.format("[%s]", dsMo.getName()), vmdkName)) + dsMo.deleteFile(String.format("[%s] %s/", dsMo.getName(), vmdkName), dcMo.getMor(), false); s_logger.info("create linked clone from template"); - if (!vmTemplate.createLinkedClone(name, morBaseSnapshot, dcMo.getVmFolder(), morPool, morDatastore)) { + if (!vmTemplate.createLinkedClone(vmdkName, morBaseSnapshot, dcMo.getVmFolder(), morPool, morDatastore)) { String msg = "Unable to clone from the template"; s_logger.error(msg); throw new Exception(msg); } - VirtualMachineMO vmMo = new ClusterMO(context, morCluster).findVmOnHyperHost(name); + VirtualMachineMO vmMo = new ClusterMO(context, morCluster).findVmOnHyperHost(vmdkName); assert (vmMo != null); // we can't rely on un-offical API (VirtualMachineMO.moveAllVmDiskFiles() any more, use hard-coded disk names that we know // to move files s_logger.info("Move volume out of volume-wrapper VM "); - dsMo.moveDatastoreFile(String.format("[%s] %s/%s.vmdk", dsMo.getName(), name, name), - dcMo.getMor(), dsMo.getMor(), - String.format("[%s] %s.vmdk", dsMo.getName(), name), dcMo.getMor(), true); + dsMo.moveDatastoreFile(String.format("[%s] %s/%s.vmdk", dsMo.getName(), vmdkName, vmdkName), + dcMo.getMor(), dsMo.getMor(), + String.format("[%s] %s.vmdk", dsMo.getName(), vmdkName), dcMo.getMor(), true); - dsMo.moveDatastoreFile(String.format("[%s] %s/%s-delta.vmdk", dsMo.getName(), name, name), + dsMo.moveDatastoreFile(String.format("[%s] %s/%s-delta.vmdk", dsMo.getName(), vmdkName, vmdkName), dcMo.getMor(), dsMo.getMor(), String.format("[%s] %s-delta.vmdk", dsMo.getName(), name), dcMo.getMor(), true); - s_logger.info("detach disks from volume-wrapper VM " + name); + s_logger.info("detach disks from volume-wrapper VM " + vmdkName); vmMo.detachAllDisks(); - s_logger.info("destroy volume-wrapper VM " + name); + s_logger.info("destroy volume-wrapper VM " + vmdkName); vmMo.destroy(); - String srcFile = String.format("[%s] %s/", dsMo.getName(), name); + String srcFile = String.format("[%s] %s/", dsMo.getName(), vmdkName); dsMo.deleteFile(srcFile, dcMo.getMor(), true); - VolumeTO vol = new VolumeTO(cmd.getVolumeId(), dskch.getType(), pool.getType(), pool.getUuid(), name, pool.getPath(), name, cmd.getDiskCharacteristics().getSize(), null); + VolumeTO vol = new VolumeTO(cmd.getVolumeId(), dskch.getType(), pool.getType(), pool.getUuid(), dskch.getName(), pool.getPath(), name, cmd.getDiskCharacteristics().getSize(), null); return new CreateAnswer(cmd, vol); } } else { @@ -3806,12 +3806,11 @@ public class VmwareResource implements StoragePoolResource, ServerResource, Vmwa // s_logger.info("Delete file if exists in datastore to clear the way for creating the volume. file: " + volumeDatastorePath); VmwareHelper.deleteVolumeVmdkFiles(dsMo, volumeUuid.toString(), dcMo); - vmMo.createDisk(volumeDatastorePath, (int) (cmd.getDiskCharacteristics().getSize() / (1024L * 1024L)), morDatastore, vmMo.getScsiDeviceControllerKey()); + vmMo.createDisk(volumeDatastorePath, (int) (dskch.getSize() / (1024L * 1024L)), morDatastore, vmMo.getScsiDeviceControllerKey()); vmMo.detachDisk(volumeDatastorePath, false); } - VolumeTO vol = new VolumeTO(cmd.getVolumeId(), dskch.getType(), pool.getType(), pool.getUuid(), cmd.getDiskCharacteristics().getName(), pool.getPath(), volumeUuid, cmd - .getDiskCharacteristics().getSize(), null); + VolumeTO vol = new VolumeTO(cmd.getVolumeId(), dskch.getType(), pool.getType(), pool.getUuid(), dskch.getName(), pool.getPath(), volumeUuid, dskch.getSize(), null); return new CreateAnswer(cmd, vol); } finally { s_logger.info("Destroy dummy VM after volume creation"); http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/3ffbc432/server/src/com/cloud/storage/StorageManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java index 3e3a67a..54b6dc0 100755 --- a/server/src/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/com/cloud/storage/StorageManagerImpl.java @@ -3404,12 +3404,6 @@ public class StorageManagerImpl implements StorageManager, Manager, ClusterManag @DB protected VolumeVO switchVolume(VolumeVO existingVolume, VirtualMachineProfile<? extends VirtualMachine> vm) throws StorageUnavailableException { Transaction txn = Transaction.currentTxn(); - txn.start(); - try { - stateTransitTo(existingVolume, Volume.Event.DestroyRequested); - } catch (NoTransitionException e) { - s_logger.debug("Unable to destroy existing volume: " + e.toString()); - } Long templateIdToUse = null; Long volTemplateId = existingVolume.getTemplateId(); @@ -3420,7 +3414,19 @@ public class StorageManagerImpl implements StorageManager, Manager, ClusterManag } templateIdToUse = vmTemplateId; } + + txn.start(); VolumeVO newVolume = allocateDuplicateVolume(existingVolume, templateIdToUse); + // In case of Vmware if vm reference is not removed then during root disk cleanup + // the vm also gets deleted, so remove the reference + if (vm.getHypervisorType() == HypervisorType.VMware) { + _volsDao.detachVolume(existingVolume.getId()); + } + try { + stateTransitTo(existingVolume, Volume.Event.DestroyRequested); + } catch (NoTransitionException e) { + s_logger.debug("Unable to destroy existing volume: " + e.toString()); + } txn.commit(); return newVolume;
