From 4d8df029d3762f49002eddbebc8eb81bf2b7bfd9 Mon Sep 17 00:00:00 2001 From: alena Date: Tue, 12 Apr 2011 14:43:02 -0700 Subject: [PATCH] bug 8245: mark storage pool status as Removed before performing actual cleanup status 8245: resolved fixed --- .../com/cloud/api/commands/DeletePoolCmd.java | 41 ++--- api/src/com/cloud/storage/StorageService.java | 45 ++++-- core/src/com/cloud/alert/AlertManager.java | 57 +++---- .../com/cloud/storage/StorageManagerImpl.java | 141 +++++++++++------- 4 files changed, 168 insertions(+), 116 deletions(-) diff --git a/api/src/com/cloud/api/commands/DeletePoolCmd.java b/api/src/com/cloud/api/commands/DeletePoolCmd.java index 0e7521eee19..117c24768cf 100644 --- a/api/src/com/cloud/api/commands/DeletePoolCmd.java +++ b/api/src/com/cloud/api/commands/DeletePoolCmd.java @@ -8,52 +8,57 @@ import com.cloud.api.Implementation; import com.cloud.api.Parameter; import com.cloud.api.ServerApiException; import com.cloud.api.response.SuccessResponse; +import com.cloud.storage.StoragePool; +import com.cloud.storage.StoragePoolStatus; import com.cloud.user.Account; -@Implementation(description="Deletes a storage pool.", responseObject=SuccessResponse.class) +@Implementation(description = "Deletes a storage pool.", responseObject = SuccessResponse.class) public class DeletePoolCmd extends BaseCmd { public static final Logger s_logger = Logger.getLogger(DeletePoolCmd.class.getName()); private static final String s_name = "deletestoragepoolresponse"; - - ///////////////////////////////////////////////////// - //////////////// API parameters ///////////////////// - ///////////////////////////////////////////////////// - @Parameter(name=ApiConstants.ID, type=CommandType.LONG, required=true, description="Storage pool id") + // /////////////////////////////////////////////////// + // ////////////// API parameters ///////////////////// + // /////////////////////////////////////////////////// + + @Parameter(name = ApiConstants.ID, type = CommandType.LONG, required = true, description = "Storage pool id") private Long id; - - ///////////////////////////////////////////////////// - /////////////////// Accessors /////////////////////// - ///////////////////////////////////////////////////// + // /////////////////////////////////////////////////// + // ///////////////// Accessors /////////////////////// + // /////////////////////////////////////////////////// public Long getId() { return id; } - - ///////////////////////////////////////////////////// - /////////////// API Implementation/////////////////// - ///////////////////////////////////////////////////// + // /////////////////////////////////////////////////// + // ///////////// API Implementation/////////////////// + // /////////////////////////////////////////////////// @Override public String getCommandName() { return s_name; } - + @Override public long getEntityOwnerId() { return Account.ACCOUNT_ID_SYSTEM; } - + @Override - public void execute(){ + public void execute() { boolean result = _storageService.deletePool(this); if (result) { SuccessResponse response = new SuccessResponse(getCommandName()); this.setResponseObject(response); } else { - throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to delete storage pool"); + StoragePool pool = _storageService.getStoragePool(id); + if (pool != null && pool.getStatus() == StoragePoolStatus.Removed) { + throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to finish storage pool removal. The storage pool will not be used but cleanup is needed"); + } else { + throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to delete storage pool"); + } } } } diff --git a/api/src/com/cloud/storage/StorageService.java b/api/src/com/cloud/storage/StorageService.java index 5e83d1a6636..72ecd4a2415 100644 --- a/api/src/com/cloud/storage/StorageService.java +++ b/api/src/com/cloud/storage/StorageService.java @@ -37,18 +37,23 @@ import com.cloud.exception.ResourceUnavailableException; public interface StorageService { /** * Create StoragePool based on uri - * @param cmd the command object that specifies the zone, cluster/pod, URI, details, etc. to use to create the storage pool. + * + * @param cmd + * the command object that specifies the zone, cluster/pod, URI, details, etc. to use to create the storage pool. * @return * @throws ResourceInUseException * @throws IllegalArgumentException * @throws UnknownHostException - * @throws ResourceUnavailableException TODO + * @throws ResourceUnavailableException + * TODO */ StoragePool createPool(CreateStoragePoolCmd cmd) throws ResourceInUseException, IllegalArgumentException, UnknownHostException, ResourceUnavailableException; - + /** * Creates the database object for a volume based on the given criteria - * @param cmd the API command wrapping the criteria (account/domainId [admin only], zone, diskOffering, snapshot, name) + * + * @param cmd + * the API command wrapping the criteria (account/domainId [admin only], zone, diskOffering, snapshot, name) * @return the volume object * @throws InvalidParameterValueException * @throws PermissionDeniedException @@ -57,39 +62,51 @@ public interface StorageService { /** * Creates the volume based on the given criteria - * @param cmd the API command wrapping the criteria (account/domainId [admin only], zone, diskOffering, snapshot, name) + * + * @param cmd + * the API command wrapping the criteria (account/domainId [admin only], zone, diskOffering, snapshot, name) * @return the volume object */ Volume createVolume(CreateVolumeCmd cmd); boolean deleteVolume(DeleteVolumeCmd cmd) throws ConcurrentOperationException; + /** * Delete the storage pool - * @param cmd - the command specifying poolId + * + * @param cmd + * - the command specifying poolId * @return success or failure * @throws InvalidParameterValueException */ boolean deletePool(DeletePoolCmd cmd) throws InvalidParameterValueException; + /** * Enable maintenance for primary storage - * @param cmd - the command specifying primaryStorageId + * + * @param cmd + * - the command specifying primaryStorageId * @return the primary storage pool - * @throws ResourceUnavailableException TODO - * @throws InsufficientCapacityException TODO + * @throws ResourceUnavailableException + * TODO + * @throws InsufficientCapacityException + * TODO */ public StoragePool preparePrimaryStorageForMaintenance(PreparePrimaryStorageForMaintenanceCmd cmd) throws ResourceUnavailableException, InsufficientCapacityException; - + /** * Complete maintenance for primary storage - * @param cmd - the command specifying primaryStorageId + * + * @param cmd + * - the command specifying primaryStorageId * @return the primary storage pool - * @throws ResourceUnavailableException TODO + * @throws ResourceUnavailableException + * TODO */ public StoragePool cancelPrimaryStorageForMaintenance(CancelPrimaryStorageMaintenanceCmd cmd) throws ResourceUnavailableException; public StoragePool updateStoragePool(UpdateStoragePoolCmd cmd) throws IllegalArgumentException; - - + public StoragePool getStoragePool(long id); } diff --git a/core/src/com/cloud/alert/AlertManager.java b/core/src/com/cloud/alert/AlertManager.java index 6378bf5b3eb..67fdb17f35c 100644 --- a/core/src/com/cloud/alert/AlertManager.java +++ b/core/src/com/cloud/alert/AlertManager.java @@ -16,34 +16,37 @@ * */ -package com.cloud.alert; - +package com.cloud.alert; + import com.cloud.capacity.CapacityVO; import com.cloud.utils.component.Manager; - -public interface AlertManager extends Manager { - public static final short ALERT_TYPE_MEMORY = CapacityVO.CAPACITY_TYPE_MEMORY; - public static final short ALERT_TYPE_CPU = CapacityVO.CAPACITY_TYPE_CPU; - public static final short ALERT_TYPE_STORAGE = CapacityVO.CAPACITY_TYPE_STORAGE; - public static final short ALERT_TYPE_STORAGE_ALLOCATED = CapacityVO.CAPACITY_TYPE_STORAGE_ALLOCATED; - public static final short ALERT_TYPE_PUBLIC_IP = CapacityVO.CAPACITY_TYPE_PUBLIC_IP; - public static final short ALERT_TYPE_PRIVATE_IP = CapacityVO.CAPACITY_TYPE_PRIVATE_IP; - public static final short ALERT_TYPE_HOST = 6; - public static final short ALERT_TYPE_USERVM = 7; - public static final short ALERT_TYPE_DOMAIN_ROUTER = 8; - public static final short ALERT_TYPE_CONSOLE_PROXY = 9; - public static final short ALERT_TYPE_ROUTING = 10; // lost connection to default route (to the gateway) - public static final short ALERT_TYPE_STORAGE_MISC = 11; // lost connection to default route (to the gateway) - public static final short ALERT_TYPE_USAGE_SERVER = 12; // lost connection to default route (to the gateway) - public static final short ALERT_TYPE_MANAGMENT_NODE = 13; // lost connection to default route (to the gateway) - public static final short ALERT_TYPE_DOMAIN_ROUTER_MIGRATE = 14; - public static final short ALERT_TYPE_CONSOLE_PROXY_MIGRATE = 15; - public static final short ALERT_TYPE_USERVM_MIGRATE = 16; + +public interface AlertManager extends Manager { + public static final short ALERT_TYPE_MEMORY = CapacityVO.CAPACITY_TYPE_MEMORY; + public static final short ALERT_TYPE_CPU = CapacityVO.CAPACITY_TYPE_CPU; + public static final short ALERT_TYPE_STORAGE = CapacityVO.CAPACITY_TYPE_STORAGE; + public static final short ALERT_TYPE_STORAGE_ALLOCATED = CapacityVO.CAPACITY_TYPE_STORAGE_ALLOCATED; + public static final short ALERT_TYPE_PUBLIC_IP = CapacityVO.CAPACITY_TYPE_PUBLIC_IP; + public static final short ALERT_TYPE_PRIVATE_IP = CapacityVO.CAPACITY_TYPE_PRIVATE_IP; + public static final short ALERT_TYPE_HOST = 6; + public static final short ALERT_TYPE_USERVM = 7; + public static final short ALERT_TYPE_DOMAIN_ROUTER = 8; + public static final short ALERT_TYPE_CONSOLE_PROXY = 9; + public static final short ALERT_TYPE_ROUTING = 10; // lost connection to default route (to the gateway) + public static final short ALERT_TYPE_STORAGE_MISC = 11; // lost connection to default route (to the gateway) + public static final short ALERT_TYPE_USAGE_SERVER = 12; // lost connection to default route (to the gateway) + public static final short ALERT_TYPE_MANAGMENT_NODE = 13; // lost connection to default route (to the gateway) + public static final short ALERT_TYPE_DOMAIN_ROUTER_MIGRATE = 14; + public static final short ALERT_TYPE_CONSOLE_PROXY_MIGRATE = 15; + public static final short ALERT_TYPE_USERVM_MIGRATE = 16; public static final short ALERT_TYPE_VLAN = 17; public static final short ALERT_TYPE_SSVM = 18; - public static final short ALERT_TYPE_USAGE_SERVER_RESULT = 19; // Usage job result - - void clearAlert(short alertType, long dataCenterId, long podId); - void sendAlert(short alertType, long dataCenterId, Long podId, String subject, String body); - void recalculateCapacity(); -} + public static final short ALERT_TYPE_USAGE_SERVER_RESULT = 19; // Usage job result + public static final short ALERT_TYPE_STORAGE_DELETE = 20; + + void clearAlert(short alertType, long dataCenterId, long podId); + + void sendAlert(short alertType, long dataCenterId, Long podId, String subject, String body); + + void recalculateCapacity(); +} diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java index 19ff3bc76aa..522ad33921d 100755 --- a/server/src/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/com/cloud/storage/StorageManagerImpl.java @@ -1230,69 +1230,87 @@ public class StorageManagerImpl implements StorageManager, StorageService, Manag throw new InvalidParameterValueException("Unable to delete local storage id: " + id); } + // Check if the pool has associated volumes in the volumes table + // If it does, then you cannot delete the pool + Pair volumeRecords = _volsDao.getCountAndTotalByPool(id); + + if (volumeRecords.first() > 0) { + s_logger.warn("Cannot delete pool " + sPool.getName() + " as there are associated vols for this pool"); + return false; // cannot delete as there are associated vols + } + + // First get the host_id from storage_pool_host_ref for given pool id + StoragePoolVO lock = _storagePoolDao.acquireInLockTable(sPool.getId()); + + if (lock == null) { + if (s_logger.isDebugEnabled()) { + s_logger.debug("Failed to acquire lock when deleting StoragePool with ID: " + sPool.getId()); + } + return false; + } + + // mark storage pool as removed (so it can't be used for new volumes creation), release the lock + boolean isLockReleased = false; + sPool.setStatus(StoragePoolStatus.Removed); + _storagePoolDao.update(id, sPool); + isLockReleased = _storagePoolDao.releaseFromLockTable(lock.getId()); + s_logger.trace("Released lock for storage pool " + id); + // for the given pool id, find all records in the storage_pool_host_ref List hostPoolRecords = _storagePoolHostDao.listByPoolId(id); + Transaction txn = Transaction.currentTxn(); + try { + // if not records exist, delete the given pool (base case) + if (hostPoolRecords.size() == 0) { - // if not records exist, delete the given pool (base case) - if (hostPoolRecords.size() == 0) { - sPool.setUuid(null); - _storagePoolDao.update(id, sPool); - _storagePoolDao.remove(id); - deletePoolStats(id); - return true; - } else { - // 1. Check if the pool has associated volumes in the volumes table - // 2. If it does, then you cannot delete the pool - Pair volumeRecords = _volsDao.getCountAndTotalByPool(id); + txn.start(); + sPool.setUuid(null); + _storagePoolDao.update(id, sPool); + _storagePoolDao.remove(id); + deletePoolStats(id); + txn.commit(); - if (volumeRecords.first() > 0) { - s_logger.warn("Cannot delete pool " + sPool.getName() + " as there are associated vols for this pool"); - return false; // cannot delete as there are associated vols + deleteFlag = true; + return true; + } else { + // Remove the SR associated with the Xenserver + for (StoragePoolHostVO host : hostPoolRecords) { + DeleteStoragePoolCommand cmd = new DeleteStoragePoolCommand(sPool); + final Answer answer = _agentMgr.easySend(host.getHostId(), cmd); + + if (answer != null && answer.getResult()) { + deleteFlag = true; + break; + } + } } - // 3. Else part, remove the SR associated with the Xenserver - else { - // First get the host_id from storage_pool_host_ref for given - // pool id - StoragePoolVO lock = _storagePoolDao.acquireInLockTable(sPool.getId()); - try { - if (lock == null) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Failed to acquire lock when deleting StoragePool with ID: " + sPool.getId()); - } - return false; - } - - for (StoragePoolHostVO host : hostPoolRecords) { - DeleteStoragePoolCommand cmd = new DeleteStoragePoolCommand(sPool); - final Answer answer = _agentMgr.easySend(host.getHostId(), cmd); - - if (answer != null && answer.getResult()) { - deleteFlag = true; - break; - } - } - - } finally { - if (lock != null) { - _storagePoolDao.releaseFromLockTable(lock.getId()); - } + } finally { + if (deleteFlag) { + // now delete the storage_pool_host_ref and storage_pool records + txn.start(); + for (StoragePoolHostVO host : hostPoolRecords) { + _storagePoolHostDao.deleteStoragePoolHostDetails(host.getHostId(), host.getPoolId()); } + sPool.setUuid(null); + _storagePoolDao.update(id, sPool); + _storagePoolDao.remove(id); + deletePoolStats(id); + txn.commit(); - if (deleteFlag) { - // now delete the storage_pool_host_ref and storage_pool - // records - for (StoragePoolHostVO host : hostPoolRecords) { - _storagePoolHostDao.deleteStoragePoolHostDetails(host.getHostId(), host.getPoolId()); - } - sPool.setUuid(null); - sPool.setStatus(StoragePoolStatus.Removed); - _storagePoolDao.update(id, sPool); - _storagePoolDao.remove(id); - deletePoolStats(id); - return true; - } + s_logger.debug("Storage pool id=" + id + " is removed successfully"); + return true; + } else { + // alert that the storage cleanup is required + s_logger.warn("Failed to Delete storage pool id: " + id); + _alertMgr.sendAlert(AlertManager.ALERT_TYPE_STORAGE_DELETE, sPool.getDataCenterId(), sPool.getPodId(), "Unable to delete storage pool id= " + id, + "Delete storage pool command failed. Please check logs."); + } + + if (lock != null && !isLockReleased) { + _storagePoolDao.releaseFromLockTable(lock.getId()); } } + return false; } @@ -1304,8 +1322,14 @@ public class StorageManagerImpl implements StorageManager, StorageService, Manag CapacityVO capacity2 = _capacityDao.findByHostIdType(poolId, CapacityVO.CAPACITY_TYPE_STORAGE_ALLOCATED); Transaction txn = Transaction.currentTxn(); txn.start(); - _capacityDao.remove(capacity1.getId()); - _capacityDao.remove(capacity2.getId()); + if (capacity1 != null) { + _capacityDao.remove(capacity1.getId()); + } + + if (capacity2 != null) { + _capacityDao.remove(capacity2.getId()); + } + txn.commit(); return true; @@ -2059,7 +2083,6 @@ public class StorageManagerImpl implements StorageManager, StorageService, Manag } } } - } // 5. Update the status @@ -2826,4 +2849,8 @@ public class StorageManagerImpl implements StorageManager, StorageService, Manag return capacities; } + @Override + public StoragePool getStoragePool(long id) { + return _storagePoolDao.findById(id); + } }