From 3ffbc4320abc9fd03ad3abd1f8077cc8d8f9ed4c Mon Sep 17 00:00:00 2001 From: Koushik Das Date: Mon, 7 Jan 2013 16:08:04 +0530 Subject: [PATCH] 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. --- .../vmware/resource/VmwareResource.java | 45 +++++++++---------- .../com/cloud/storage/StorageManagerImpl.java | 18 +++++--- 2 files changed, 34 insertions(+), 29 deletions(-) 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 0b0f285a960..b1b15bb30ea 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"); diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java index 3e3a67a96c2..54b6dc0c327 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 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;