From fc784f15309cac2e653b7fefc8a51256f83eab72 Mon Sep 17 00:00:00 2001 From: prachi Date: Tue, 26 Apr 2011 19:22:13 -0700 Subject: [PATCH] Bug 9585 - Existing Data Disk is being destroyed and recreated on Stop and Start of a User VM. Changes: - When the ROOT volume of a VM is found to be READY, changed planner to reuse the pool for every volume(root or data) that is READY and that has a pool not in maintenance and not in avoid state - If ROOT volume is not ready, we dont care about the DATA disk. Both would get re-allocated. - When a pool is reused for a ready volume, Planner does not call storagepool allocators. And such volumes are not assigned a pool in the deployment destination returned by the planner. Accordingly StorageManager :: prepare method wont recreate these volumes since they are not mentioned in the destination. --- .../src/com/cloud/deploy/FirstFitPlanner.java | 82 ++++++++++++++----- .../com/cloud/storage/StorageManagerImpl.java | 5 +- .../cloud/vm/VirtualMachineManagerImpl.java | 33 +++----- 3 files changed, 76 insertions(+), 44 deletions(-) diff --git a/server/src/com/cloud/deploy/FirstFitPlanner.java b/server/src/com/cloud/deploy/FirstFitPlanner.java index bbaf6ae9ccf..f10f08d5071 100644 --- a/server/src/com/cloud/deploy/FirstFitPlanner.java +++ b/server/src/com/cloud/deploy/FirstFitPlanner.java @@ -121,7 +121,11 @@ public class FirstFitPlanner extends PlannerBase implements DeploymentPlanner { //search for storage under the zone, pod, cluster of the host. DataCenterDeployment lastPlan = new DataCenterDeployment(host.getDataCenterId(), host.getPodId(), host.getClusterId(), hostIdSpecified, plan.getPoolId()); - Map> suitableVolumeStoragePools = findSuitablePoolsForVolumes(vmProfile, lastPlan, avoid, RETURN_UPTO_ALL); + + Pair>, List> result = findSuitablePoolsForVolumes(vmProfile, lastPlan, avoid, RETURN_UPTO_ALL); + Map> suitableVolumeStoragePools = result.first(); + List readyAndReusedVolumes = result.second(); + //choose the potential pool for this VM for this host if(!suitableVolumeStoragePools.isEmpty()){ List suitableHosts = new ArrayList(); @@ -131,7 +135,12 @@ public class FirstFitPlanner extends PlannerBase implements DeploymentPlanner { if(potentialResources != null){ Pod pod = _podDao.findById(vm.getPodId()); Cluster cluster = _clusterDao.findById(host.getClusterId()); - DeployDestination dest = new DeployDestination(dc, pod, cluster, host, potentialResources.second()); + Map storageVolMap = potentialResources.second(); + // remove the reused vol<->pool from destination, since we don't have to prepare this volume. + for(Volume vol : readyAndReusedVolumes){ + storageVolMap.remove(vol); + } + DeployDestination dest = new DeployDestination(dc, pod, cluster, host, storageVolMap); s_logger.debug("Returning Deployment Destination: "+ dest); return dest; } @@ -155,7 +164,9 @@ public class FirstFitPlanner extends PlannerBase implements DeploymentPlanner { s_logger.debug("Now checking for suitable pools under zone: "+vm.getDataCenterId() +", pod: "+ vm.getPodId()+", cluster: "+ host.getClusterId()); //search for storage under the zone, pod, cluster of the last host. DataCenterDeployment lastPlan = new DataCenterDeployment(vm.getDataCenterId(), vm.getPodId(), host.getClusterId(), host.getId(), plan.getPoolId()); - Map> suitableVolumeStoragePools = findSuitablePoolsForVolumes(vmProfile, lastPlan, avoid, RETURN_UPTO_ALL); + Pair>, List> result = findSuitablePoolsForVolumes(vmProfile, lastPlan, avoid, RETURN_UPTO_ALL); + Map> suitableVolumeStoragePools = result.first(); + List readyAndReusedVolumes = result.second(); //choose the potential pool for this VM for this host if(!suitableVolumeStoragePools.isEmpty()){ List suitableHosts = new ArrayList(); @@ -165,7 +176,12 @@ public class FirstFitPlanner extends PlannerBase implements DeploymentPlanner { if(potentialResources != null){ Pod pod = _podDao.findById(vm.getPodId()); Cluster cluster = _clusterDao.findById(host.getClusterId()); - DeployDestination dest = new DeployDestination(dc, pod, cluster, host, potentialResources.second()); + Map storageVolMap = potentialResources.second(); + // remove the reused vol<->pool from destination, since we don't have to prepare this volume. + for(Volume vol : readyAndReusedVolumes){ + storageVolMap.remove(vol); + } + DeployDestination dest = new DeployDestination(dc, pod, cluster, host, storageVolMap); s_logger.debug("Returning Deployment Destination: "+ dest); return dest; } @@ -351,7 +367,9 @@ public class FirstFitPlanner extends PlannerBase implements DeploymentPlanner { if (_allocationAlgorithm != null && _allocationAlgorithm.equalsIgnoreCase("random")) { Collections.shuffle(suitableHosts); } - Map> suitableVolumeStoragePools = findSuitablePoolsForVolumes(vmProfile, potentialPlan, avoid, RETURN_UPTO_ALL); + Pair>, List> result = findSuitablePoolsForVolumes(vmProfile, potentialPlan, avoid, RETURN_UPTO_ALL); + Map> suitableVolumeStoragePools = result.first(); + List readyAndReusedVolumes = result.second(); //choose the potential host and pool for the VM if(!suitableVolumeStoragePools.isEmpty()){ @@ -360,7 +378,12 @@ public class FirstFitPlanner extends PlannerBase implements DeploymentPlanner { if(potentialResources != null){ Pod pod = _podDao.findById(clusterVO.getPodId()); Host host = _hostDao.findById(potentialResources.first().getId()); - DeployDestination dest = new DeployDestination(dc, pod, clusterVO, host, potentialResources.second() ); + Map storageVolMap = potentialResources.second(); + // remove the reused vol<->pool from destination, since we don't have to prepare this volume. + for(Volume vol : readyAndReusedVolumes){ + storageVolMap.remove(vol); + } + DeployDestination dest = new DeployDestination(dc, pod, clusterVO, host, storageVolMap ); s_logger.debug("Returning Deployment Destination: "+ dest); return dest; } @@ -511,32 +534,47 @@ public class FirstFitPlanner extends PlannerBase implements DeploymentPlanner { return suitableHosts; } - protected Map> findSuitablePoolsForVolumes(VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo){ + protected Pair>, List> findSuitablePoolsForVolumes(VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo){ List volumesTobeCreated = _volsDao.findUsableVolumesForInstance(vmProfile.getId()); Map> suitableVolumeStoragePools = new HashMap>(); - - s_logger.debug("Calling StoragePoolAllocators to find suitable pools"); + List readyAndReusedVolumes = new ArrayList(); //for each volume find list of suitable storage pools by calling the allocators for (VolumeVO toBeCreated : volumesTobeCreated) { s_logger.debug("Checking suitable pools for volume (Id, Type): ("+toBeCreated.getId() +"," +toBeCreated.getVolumeType().name() + ")"); - //skip the volume if its already in READY state and has pool allocated + //If the plan specifies a poolId, it means that this VM's ROOT volume is ready and the pool should be reused. + //In this case, also check if rest of the volumes are ready and can be reused. if(plan.getPoolId() != null){ - if (toBeCreated.getVolumeType() == Volume.Type.ROOT && toBeCreated.getPoolId() != null && toBeCreated.getPoolId().longValue() == plan.getPoolId().longValue()) { - s_logger.debug("ROOT Volume is in READY state and has pool already allocated."); + if (toBeCreated.getState() == Volume.State.Ready && toBeCreated.getPoolId() != null) { + s_logger.debug("Volume is in READY state and has pool already allocated, checking if pool can be reused, poolId: "+toBeCreated.getPoolId()); List suitablePools = new ArrayList(); StoragePoolVO pool = _storagePoolDao.findById(toBeCreated.getPoolId()); - if(!avoid.shouldAvoid(pool)){ - s_logger.debug("Planner need not allocate a pool for this volume since its READY"); - suitablePools.add(pool); - suitableVolumeStoragePools.put(toBeCreated, suitablePools); - continue; + if(!pool.isInMaintenance()){ + if(!avoid.shouldAvoid(pool)){ + long exstPoolDcId = pool.getDataCenterId(); + Long exstPoolPodId = pool.getPodId(); + Long exstPoolClusterId = pool.getClusterId(); + if(plan.getDataCenterId() == exstPoolDcId && plan.getPodId() == exstPoolPodId && plan.getClusterId() == exstPoolClusterId){ + s_logger.debug("Planner need not allocate a pool for this volume since its READY"); + suitablePools.add(pool); + suitableVolumeStoragePools.put(toBeCreated, suitablePools); + readyAndReusedVolumes.add(toBeCreated); + continue; + }else{ + s_logger.debug("Pool of the volume does not fit the specified plan, need to reallocate a pool for this volume"); + } + }else{ + s_logger.debug("Pool of the volume is in avoid set, need to reallocate a pool for this volume"); + } }else{ - s_logger.debug("Pool of the ROOT volume is in avoid set, need to allocate a pool for this volume"); + s_logger.debug("Pool of the volume is in maintenance, need to reallocate a pool for this volume"); } } } + + s_logger.debug("Calling StoragePoolAllocators to find suitable pools"); + DiskOfferingVO diskOffering = _diskOfferingDao.findById(toBeCreated.getDiskOfferingId()); DiskProfile diskProfile = new DiskProfile(toBeCreated, diskOffering, vmProfile.getHypervisorType()); @@ -566,7 +604,10 @@ public class FirstFitPlanner extends PlannerBase implements DeploymentPlanner { } if(!foundPotentialPools){ - //No suitable storage pools found under this cluster for this volume. + s_logger.debug("No suitable pools found for volume: "+toBeCreated +" under cluster: "+plan.getClusterId()); + //No suitable storage pools found under this cluster for this volume. - remove any suitable pools found for other volumes. + //All volumes should get suitable pools under this cluster; else we cant use this cluster. + suitableVolumeStoragePools.clear(); break; } } @@ -574,7 +615,8 @@ public class FirstFitPlanner extends PlannerBase implements DeploymentPlanner { if(suitableVolumeStoragePools.isEmpty()){ s_logger.debug("No suitable pools found"); } - return suitableVolumeStoragePools; + + return new Pair>, List>(suitableVolumeStoragePools, readyAndReusedVolumes); } diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java index 51b84fba69e..59c83edda15 100755 --- a/server/src/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/com/cloud/storage/StorageManagerImpl.java @@ -2493,7 +2493,7 @@ public class StorageManagerImpl implements StorageManager, StorageService, Manag } List vols = _volsDao.findUsableVolumesForInstance(vm.getId()); if (s_logger.isDebugEnabled()) { - s_logger.debug("Preparing " + vols.size() + " volumes for " + vm); + s_logger.debug("Checking if we need to prepare " + vols.size() + " volumes for " + vm); } List recreateVols = new ArrayList(vols.size()); @@ -2525,6 +2525,9 @@ public class StorageManagerImpl implements StorageManager, StorageService, Manag if (vol.getPoolId() == null) { throw new StorageUnavailableException("Volume has no pool associate and also no storage pool assigned in DeployDestination, Unable to create " + vol, Volume.class, vol.getId()); } + if (s_logger.isDebugEnabled()) { + s_logger.debug("No need to recreate the volume: "+vol+ ", since it already has a pool assigned: "+vol.getPoolId()+", adding disk to VM"); + } StoragePoolVO pool = _storagePoolDao.findById(vol.getPoolId()); vm.addDisk(new VolumeTO(vol, pool)); } diff --git a/server/src/com/cloud/vm/VirtualMachineManagerImpl.java b/server/src/com/cloud/vm/VirtualMachineManagerImpl.java index 4b7e32df935..590e130835e 100755 --- a/server/src/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/server/src/com/cloud/vm/VirtualMachineManagerImpl.java @@ -583,34 +583,30 @@ public class VirtualMachineManagerImpl implements VirtualMachineManager, Listene Journal journal = start.second().getJournal(); // edit plan if this vm's ROOT volume is in READY state already - VolumeVO readyRootVolume = null; List vols = _volsDao.findReadyRootVolumesByInstance(vm.getId()); for (VolumeVO vol : vols) { Volume.State state = vol.getState(); if (state == Volume.State.Ready) { - // make sure if this is a System VM, templateId is unchanged. If it is changed, let planner - // reassign pool for the volume - if (VirtualMachine.Type.isSystemVM(vm.getType())) { - Long volTemplateId = vol.getTemplateId(); - if (volTemplateId != null && template != null) { - if (volTemplateId.longValue() != template.getId()) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Root Volume " + vol + " of " + vm.getType().toString() - + " System VM is ready, but volume's templateId does not match the System VM Template, let the planner reassign a new pool"); - } - continue; + // make sure if the templateId is unchanged. If it is changed, let planner + // reassign pool for the volume even if it ready. + Long volTemplateId = vol.getTemplateId(); + if (volTemplateId != null && template != null) { + if (volTemplateId.longValue() != template.getId()) { + if (s_logger.isDebugEnabled()) { + s_logger.debug("Root Volume " + vol + " of " + vm.getType().toString() + + " VM is READY, but volume's templateId does not match the VM's Template, let the planner reassign a new pool"); } + continue; } - } + StoragePoolVO pool = _storagePoolDao.findById(vol.getPoolId()); if (!pool.isInMaintenance()) { long rootVolDcId = pool.getDataCenterId(); Long rootVolPodId = pool.getPodId(); Long rootVolClusterId = pool.getClusterId(); plan = new DataCenterDeployment(rootVolDcId, rootVolPodId, rootVolClusterId, null, vol.getPoolId()); - readyRootVolume = vol; if (s_logger.isDebugEnabled()) { s_logger.debug("Root Volume " + vol + " is ready, changing deployment plan to use this pool's datacenterId: " + rootVolDcId + " , podId: " + rootVolPodId + " , and clusterId: " + rootVolClusterId); @@ -650,15 +646,6 @@ public class VirtualMachineManagerImpl implements VirtualMachineManager, Listene try { if (vm.getHypervisorType() != HypervisorType.BareMetal) { - if (readyRootVolume != null) { - // remove the vol<->pool from destination, since we don't have to prepare this volume. - if (dest.getStorageForDisks() != null) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("No need to prepare the READY Root Volume " + readyRootVolume + ", removing it from deploydestination"); - } - dest.getStorageForDisks().remove(readyRootVolume); - } - } _storageMgr.prepare(vmProfile, dest); } _networkMgr.prepare(vmProfile, dest, ctx);