You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by mu...@apache.org on 2013/01/07 11:44:05 UTC

[2/2] git commit: CLOUDSTACK-595 : Recreate root volume scenarios doesn't work in vmware

Updated Branches:
  refs/heads/4.0 97f6fe1e5 -> c69da4c1b
  refs/heads/events-framework 7d9381edd -> abd768592


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/c69da4c1
Tree: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/c69da4c1
Diff: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/c69da4c1

Branch: refs/heads/4.0
Commit: c69da4c1b30ac1658b2ea5ca5ec7f2a175e40954
Parents: 97f6fe1
Author: Koushik Das <ko...@citrix.com>
Authored: Mon Jan 7 16:08:04 2013 +0530
Committer: Murali Reddy <mu...@citrix.com>
Committed: Mon Jan 7 16:08:04 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/c69da4c1/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 0cdc3f3..66c421f 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
@@ -3708,10 +3708,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);
@@ -3723,16 +3725,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();
@@ -3756,41 +3757,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 {
@@ -3809,12 +3809,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/c69da4c1/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 fc6fb5b..ed8ef20 100755
--- a/server/src/com/cloud/storage/StorageManagerImpl.java
+++ b/server/src/com/cloud/storage/StorageManagerImpl.java
@@ -3382,12 +3382,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();
@@ -3398,7 +3392,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;