Return-Path: X-Original-To: apmail-incubator-cloudstack-commits-archive@minotaur.apache.org Delivered-To: apmail-incubator-cloudstack-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 16A87DB33 for ; Mon, 7 Jan 2013 10:44:07 +0000 (UTC) Received: (qmail 1334 invoked by uid 500); 7 Jan 2013 10:44:07 -0000 Delivered-To: apmail-incubator-cloudstack-commits-archive@incubator.apache.org Received: (qmail 1010 invoked by uid 500); 7 Jan 2013 10:44:06 -0000 Mailing-List: contact cloudstack-commits-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: cloudstack-dev@incubator.apache.org Delivered-To: mailing list cloudstack-commits@incubator.apache.org Received: (qmail 993 invoked by uid 99); 7 Jan 2013 10:44:06 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 07 Jan 2013 10:44:06 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id A8751823BD5; Mon, 7 Jan 2013 10:44:05 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: muralireddy@apache.org To: cloudstack-commits@incubator.apache.org X-Mailer: ASF-Git Admin Mailer Subject: [2/2] git commit: CLOUDSTACK-595 : Recreate root volume scenarios doesn't work in vmware Message-Id: <20130107104405.A8751823BD5@tyr.zones.apache.org> Date: Mon, 7 Jan 2013 10:44:05 +0000 (UTC) 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 Authored: Mon Jan 7 16:08:04 2013 +0530 Committer: Murali Reddy 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 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;