This is an automated email from the ASF dual-hosted git repository. dahn pushed a commit to branch ghi12316-storageManagerTechDebt in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit 5844dccb8cd6fe4c897deeb88251cac5e9ec503f Author: Daan Hoogland <[email protected]> AuthorDate: Mon Dec 22 19:56:33 2025 +0100 Deal with Storage Manager tech debt --- .../java/com/cloud/storage/StorageService.java | 16 +- .../command/admin/host/AddSecondaryStorageCmd.java | 24 +- .../command/admin/storage/AddImageStoreCmd.java | 32 +- .../command/admin/storage/AddImageStoreS3CMD.java | 44 +- .../admin/storage/CreateStoragePoolCmd.java | 74 ++- .../admin/storage/UpdateStoragePoolCmd.java | 2 +- .../api/command/admin/swift/AddSwiftCmd.java | 26 +- .../cloud/configuration/ConfigurationManager.java | 47 ++ .../java/com/cloud/storage/StorageManager.java | 60 ++- .../cache/manager/StorageCacheManagerImpl.java | 3 +- .../datastore/driver/S3ImageStoreDriverImpl.java | 12 +- .../driver/SwiftImageStoreDriverImpl.java | 19 +- .../main/java/com/cloud/configuration/Config.java | 65 --- .../configuration/ConfigurationManagerImpl.java | 3 +- .../java/com/cloud/storage/StorageManagerImpl.java | 592 ++++++--------------- 15 files changed, 375 insertions(+), 644 deletions(-) diff --git a/api/src/main/java/com/cloud/storage/StorageService.java b/api/src/main/java/com/cloud/storage/StorageService.java index a29c8f6aece..65f0904ab88 100644 --- a/api/src/main/java/com/cloud/storage/StorageService.java +++ b/api/src/main/java/com/cloud/storage/StorageService.java @@ -17,7 +17,6 @@ package com.cloud.storage; -import java.net.UnknownHostException; import java.util.Map; import org.apache.cloudstack.api.command.admin.storage.CancelPrimaryStorageMaintenanceCmd; @@ -35,10 +34,8 @@ import org.apache.cloudstack.api.command.admin.storage.UpdateImageStoreCmd; import org.apache.cloudstack.api.command.admin.storage.UpdateStoragePoolCmd; import com.cloud.exception.DiscoveryException; -import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.PermissionDeniedException; -import com.cloud.exception.ResourceInUseException; import com.cloud.exception.ResourceUnavailableException; import org.apache.cloudstack.api.command.admin.storage.heuristics.CreateSecondaryStorageSelectorCmd; import org.apache.cloudstack.api.command.admin.storage.heuristics.RemoveSecondaryStorageSelectorCmd; @@ -55,12 +52,9 @@ public interface StorageService { * storage pool. * @return * The StoragePool created. - * @throws ResourceInUseException * @throws IllegalArgumentException - * @throws UnknownHostException - * @throws ResourceUnavailableException */ - StoragePool createPool(CreateStoragePoolCmd cmd) throws ResourceInUseException, IllegalArgumentException, UnknownHostException, ResourceUnavailableException; + StoragePool createPool(CreateStoragePoolCmd cmd) throws IllegalArgumentException; ImageStore createSecondaryStagingStore(CreateSecondaryStagingStoreCmd cmd); @@ -79,10 +73,8 @@ public interface StorageService { * @param primaryStorageId * - the primaryStorageId * @return the primary storage pool - * @throws ResourceUnavailableException - * @throws InsufficientCapacityException */ - StoragePool preparePrimaryStorageForMaintenance(Long primaryStorageId) throws ResourceUnavailableException, InsufficientCapacityException; + StoragePool preparePrimaryStorageForMaintenance(Long primaryStorageId); /** * Complete maintenance for primary storage @@ -108,7 +100,7 @@ public interface StorageService { boolean deleteSecondaryStagingStore(DeleteSecondaryStagingStoreCmd cmd); - ImageStore discoverImageStore(String name, String url, String providerName, Long zoneId, Map details) throws IllegalArgumentException, DiscoveryException, InvalidParameterValueException; + ImageStore discoverImageStore(String name, String url, String providerName, Long zoneId, Map<String, String> details) throws IllegalArgumentException, InvalidParameterValueException; /** * Migrate existing NFS to use object store. @@ -134,7 +126,7 @@ public interface StorageService { void removeSecondaryStorageHeuristic(RemoveSecondaryStorageSelectorCmd cmd); - ObjectStore discoverObjectStore(String name, String url, Long size, String providerName, Map details) throws IllegalArgumentException, DiscoveryException, InvalidParameterValueException; + ObjectStore discoverObjectStore(String name, String url, Long size, String providerName, Map<String, String> details) throws IllegalArgumentException, DiscoveryException, InvalidParameterValueException; boolean deleteObjectStore(DeleteObjectStoragePoolCmd cmd); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddSecondaryStorageCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddSecondaryStorageCmd.java index 9a7eff7e2e5..1553d184a94 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddSecondaryStorageCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/host/AddSecondaryStorageCmd.java @@ -26,7 +26,6 @@ import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.ImageStoreResponse; import org.apache.cloudstack.api.response.ZoneResponse; -import com.cloud.exception.DiscoveryException; import com.cloud.storage.ImageStore; import com.cloud.user.Account; @@ -67,20 +66,15 @@ public class AddSecondaryStorageCmd extends BaseCmd { @Override public void execute(){ - try{ - ImageStore result = _storageService.discoverImageStore(null, getUrl(), "NFS", getZoneId(), null); - ImageStoreResponse storeResponse = null; - if (result != null ) { - storeResponse = _responseGenerator.createImageStoreResponse(result); - storeResponse.setResponseName(getCommandName()); - storeResponse.setObjectName("secondarystorage"); - setResponseObject(storeResponse); - } else { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add secondary storage"); - } - } catch (DiscoveryException ex) { - logger.warn("Exception: ", ex); - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex.getMessage()); + ImageStore result = _storageService.discoverImageStore(null, getUrl(), "NFS", getZoneId(), null); + ImageStoreResponse storeResponse = null; + if (result != null ) { + storeResponse = _responseGenerator.createImageStoreResponse(result); + storeResponse.setResponseName(getCommandName()); + storeResponse.setObjectName("secondarystorage"); + setResponseObject(storeResponse); + } else { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add secondary storage"); } } } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/AddImageStoreCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/AddImageStoreCmd.java index 72e2e96fe57..4f50b77d5f8 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/AddImageStoreCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/AddImageStoreCmd.java @@ -18,7 +18,6 @@ package org.apache.cloudstack.api.command.admin.storage; import java.util.Collection; import java.util.HashMap; -import java.util.Iterator; import java.util.Map; @@ -31,7 +30,6 @@ import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.ImageStoreResponse; import org.apache.cloudstack.api.response.ZoneResponse; -import com.cloud.exception.DiscoveryException; import com.cloud.storage.ImageStore; import com.cloud.user.Account; @@ -79,11 +77,10 @@ public class AddImageStoreCmd extends BaseCmd { public Map<String, String> getDetails() { Map<String, String> detailsMap = null; if (details != null && !details.isEmpty()) { - detailsMap = new HashMap<String, String>(); + detailsMap = new HashMap<>(); Collection<?> props = details.values(); - Iterator<?> iter = props.iterator(); - while (iter.hasNext()) { - HashMap<String, String> detail = (HashMap<String, String>)iter.next(); + for (Object prop : props) { + HashMap<String, String> detail = (HashMap<String, String>) prop; String key = detail.get("key"); String value = detail.get("value"); detailsMap.put(key, value); @@ -123,20 +120,15 @@ public class AddImageStoreCmd extends BaseCmd { @Override public void execute(){ - try{ - ImageStore result = _storageService.discoverImageStore(getName(), getUrl(), getProviderName(), getZoneId(), getDetails()); - ImageStoreResponse storeResponse = null; - if (result != null) { - storeResponse = _responseGenerator.createImageStoreResponse(result); - storeResponse.setResponseName(getCommandName()); - storeResponse.setObjectName("imagestore"); - setResponseObject(storeResponse); - } else { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add secondary storage"); - } - } catch (DiscoveryException ex) { - logger.warn("Exception: ", ex); - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex.getMessage()); + ImageStore result = _storageService.discoverImageStore(getName(), getUrl(), getProviderName(), getZoneId(), getDetails()); + ImageStoreResponse storeResponse; + if (result != null) { + storeResponse = _responseGenerator.createImageStoreResponse(result); + storeResponse.setResponseName(getCommandName()); + storeResponse.setObjectName("imagestore"); + setResponseObject(storeResponse); + } else { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add secondary storage"); } } } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/AddImageStoreS3CMD.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/AddImageStoreS3CMD.java index 2fe3c7cd106..6a8b10b8e85 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/AddImageStoreS3CMD.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/AddImageStoreS3CMD.java @@ -48,15 +48,14 @@ import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.ImageStoreResponse; import com.cloud.exception.ConcurrentOperationException; -import com.cloud.exception.DiscoveryException; import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.NetworkRuleConflictException; import com.cloud.exception.ResourceAllocationException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.storage.ImageStore; -@APICommand(name = "addImageStoreS3", description = "Adds S3 Image Store", responseObject = ImageStoreResponse.class, since = "4.7.0", - requestHasSensitiveInfo = true, responseHasSensitiveInfo = false) +@APICommand(name = "addImageStoreS3", description = "Adds S3 Image Store", responseObject = ImageStoreResponse.class, + since = "4.7.0", responseHasSensitiveInfo = false) public final class AddImageStoreS3CMD extends BaseCmd implements ClientOptions { private static final String s_name = "addImageStoreS3Response"; @@ -73,32 +72,32 @@ public final class AddImageStoreS3CMD extends BaseCmd implements ClientOptions { @Parameter(name = S3_BUCKET_NAME, type = STRING, required = true, description = "Name of the storage bucket") private String bucketName; - @Parameter(name = S3_SIGNER, type = STRING, required = false, description = "Signer Algorithm to use, either S3SignerType or AWSS3V4SignerType") + @Parameter(name = S3_SIGNER, type = STRING, description = "Signer Algorithm to use, either S3SignerType or AWSS3V4SignerType") private String signer; - @Parameter(name = S3_HTTPS_FLAG, type = BOOLEAN, required = false, description = "Use HTTPS instead of HTTP") + @Parameter(name = S3_HTTPS_FLAG, type = BOOLEAN, description = "Use HTTPS instead of HTTP") private Boolean httpsFlag; - @Parameter(name = S3_CONNECTION_TIMEOUT, type = INTEGER, required = false, description = "Connection timeout (milliseconds)") + @Parameter(name = S3_CONNECTION_TIMEOUT, type = INTEGER, description = "Connection timeout (milliseconds)") private Integer connectionTimeout; - @Parameter(name = S3_MAX_ERROR_RETRY, type = INTEGER, required = false, description = "Maximum number of times to retry on error") + @Parameter(name = S3_MAX_ERROR_RETRY, type = INTEGER, description = "Maximum number of times to retry on error") private Integer maxErrorRetry; - @Parameter(name = S3_SOCKET_TIMEOUT, type = INTEGER, required = false, description = "Socket timeout (milliseconds)") + @Parameter(name = S3_SOCKET_TIMEOUT, type = INTEGER, description = "Socket timeout (milliseconds)") private Integer socketTimeout; - @Parameter(name = S3_CONNECTION_TTL, type = INTEGER, required = false, description = "Connection TTL (milliseconds)") + @Parameter(name = S3_CONNECTION_TTL, type = INTEGER, description = "Connection TTL (milliseconds)") private Integer connectionTtl; - @Parameter(name = S3_USE_TCP_KEEPALIVE, type = BOOLEAN, required = false, description = "Whether TCP keep-alive is used") + @Parameter(name = S3_USE_TCP_KEEPALIVE, type = BOOLEAN, description = "Whether TCP keep-alive is used") private Boolean useTCPKeepAlive; @Override public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException { - Map<String, String> dm = new HashMap(); + Map<String, String> dm = new HashMap<>(); dm.put(ApiConstants.S3_ACCESS_KEY, getAccessKey()); dm.put(ApiConstants.S3_SECRET_KEY, getSecretKey()); @@ -127,20 +126,15 @@ public final class AddImageStoreS3CMD extends BaseCmd implements ClientOptions { dm.put(ApiConstants.S3_USE_TCP_KEEPALIVE, getUseTCPKeepAlive().toString()); } - try{ - ImageStore result = _storageService.discoverImageStore(null, null, "S3", null, dm); - ImageStoreResponse storeResponse; - if (result != null) { - storeResponse = _responseGenerator.createImageStoreResponse(result); - storeResponse.setResponseName(getCommandName()); - storeResponse.setObjectName("imagestore"); - setResponseObject(storeResponse); - } else { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add S3 Image Store."); - } - } catch (DiscoveryException ex) { - logger.warn("Exception: ", ex); - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex.getMessage()); + ImageStore result = _storageService.discoverImageStore(null, null, "S3", null, dm); + ImageStoreResponse storeResponse; + if (result != null) { + storeResponse = _responseGenerator.createImageStoreResponse(result); + storeResponse.setResponseName(getCommandName()); + storeResponse.setObjectName("imagestore"); + setResponseObject(storeResponse); + } else { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add S3 Image Store."); } } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/CreateStoragePoolCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/CreateStoragePoolCmd.java index 2aef856f58f..6b5bc633d53 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/CreateStoragePoolCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/CreateStoragePoolCmd.java @@ -16,7 +16,6 @@ // under the License. package org.apache.cloudstack.api.command.admin.storage; -import java.net.UnknownHostException; import java.util.Map; @@ -31,8 +30,6 @@ import org.apache.cloudstack.api.response.PodResponse; import org.apache.cloudstack.api.response.StoragePoolResponse; import org.apache.cloudstack.api.response.ZoneResponse; -import com.cloud.exception.ResourceInUseException; -import com.cloud.exception.ResourceUnavailableException; import com.cloud.storage.StoragePool; import com.cloud.user.Account; @@ -46,53 +43,85 @@ public class CreateStoragePoolCmd extends BaseCmd { //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// - @Parameter(name = ApiConstants.CLUSTER_ID, type = CommandType.UUID, entityType = ClusterResponse.class, description = "The cluster ID for the storage pool") + @Parameter(name = ApiConstants.CLUSTER_ID, + type = CommandType.UUID, + entityType = ClusterResponse.class, + description = "The cluster ID for the storage pool") private Long clusterId; - @Parameter(name = ApiConstants.DETAILS, type = CommandType.MAP, description = "The details for the storage pool") + @Parameter(name = ApiConstants.DETAILS, + type = CommandType.MAP, + description = "The details for the storage pool") private Map details; - @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, required = true, description = "The name for the storage pool") + @Parameter(name = ApiConstants.NAME, + type = CommandType.STRING, + required = true, + description = "The name for the storage pool") private String storagePoolName; - @Parameter(name = ApiConstants.POD_ID, type = CommandType.UUID, entityType = PodResponse.class, description = "The Pod ID for the storage pool") + @Parameter(name = ApiConstants.POD_ID, + type = CommandType.UUID, + entityType = PodResponse.class, + description = "The Pod ID for the storage pool") private Long podId; - @Parameter(name = ApiConstants.TAGS, type = CommandType.STRING, description = "The tags for the storage pool") + @Parameter(name = ApiConstants.TAGS, + type = CommandType.STRING, + description = "The tags for the storage pool") private String tags; - @Parameter(name = ApiConstants.STORAGE_ACCESS_GROUPS, type = CommandType.STRING, + @Parameter(name = ApiConstants.STORAGE_ACCESS_GROUPS, + type = CommandType.STRING, description = "comma separated list of storage access groups for connecting to hosts having those specific groups", since = "4.21.0") private String storageAccessGroups; - @Parameter(name = ApiConstants.URL, type = CommandType.STRING, required = true, description = "The URL of the storage pool") + @Parameter(name = ApiConstants.URL, + type = CommandType.STRING, + required = true, + description = "The URL of the storage pool") private String url; - @Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class, required = true, description = "The Zone ID for the storage pool") + @Parameter(name = ApiConstants.ZONE_ID, + type = CommandType.UUID, + entityType = ZoneResponse.class, + required = true, + description = "The Zone ID for the storage pool") private Long zoneId; - @Parameter(name = ApiConstants.PROVIDER, type = CommandType.STRING, required = false, description = "The storage provider name") + @Parameter(name = ApiConstants.PROVIDER, + type = CommandType.STRING, + description = "The storage provider name") private String storageProviderName; - @Parameter(name = ApiConstants.SCOPE, type = CommandType.STRING, required = false, description = "The scope of the storage: cluster or zone") + @Parameter(name = ApiConstants.SCOPE, + type = CommandType.STRING, + description = "The scope of the storage: cluster or zone") private String scope; - @Parameter(name = ApiConstants.MANAGED, type = CommandType.BOOLEAN, required = false, description = "Whether the storage should be managed by CloudStack") + @Parameter(name = ApiConstants.MANAGED, + type = CommandType.BOOLEAN, + description = "Whether the storage should be managed by CloudStack") private Boolean managed; - @Parameter(name = ApiConstants.CAPACITY_IOPS, type = CommandType.LONG, required = false, description = "IOPS CloudStack can provision from this storage pool") + @Parameter(name = ApiConstants.CAPACITY_IOPS, + type = CommandType.LONG, + description = "IOPS CloudStack can provision from this storage pool") private Long capacityIops; - @Parameter(name = ApiConstants.CAPACITY_BYTES, type = CommandType.LONG, required = false, description = "Bytes CloudStack can provision from this storage pool") + @Parameter(name = ApiConstants.CAPACITY_BYTES, + type = CommandType.LONG, + description = "Bytes CloudStack can provision from this storage pool") private Long capacityBytes; @Parameter(name = ApiConstants.HYPERVISOR, type = CommandType.STRING, - required = false, description = "Hypervisor type of the hosts in zone that will be attached to this storage pool. KVM, VMware supported as of now.") private String hypervisor; - @Parameter(name = ApiConstants.IS_TAG_A_RULE, type = CommandType.BOOLEAN, description = ApiConstants.PARAMETER_DESCRIPTION_IS_TAG_A_RULE) + @Parameter(name = ApiConstants.IS_TAG_A_RULE, + type = CommandType.BOOLEAN, + description = ApiConstants.PARAMETER_DESCRIPTION_IS_TAG_A_RULE) private Boolean isTagARule; ///////////////////////////////////////////////////// @@ -175,15 +204,6 @@ public class CreateStoragePoolCmd extends BaseCmd { } else { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add storage pool"); } - } catch (ResourceUnavailableException ex1) { - logger.warn("Exception: ", ex1); - throw new ServerApiException(ApiErrorCode.RESOURCE_UNAVAILABLE_ERROR, ex1.getMessage()); - } catch (ResourceInUseException ex2) { - logger.warn("Exception: ", ex2); - throw new ServerApiException(ApiErrorCode.RESOURCE_IN_USE_ERROR, ex2.getMessage()); - } catch (UnknownHostException ex3) { - logger.warn("Exception: ", ex3); - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex3.getMessage()); } catch (Exception ex4) { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex4.getMessage()); } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStoragePoolCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStoragePoolCmd.java index 4b0a6ba00b2..f37559cc404 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStoragePoolCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStoragePoolCmd.java @@ -131,7 +131,7 @@ public class UpdateStoragePoolCmd extends BaseCmd { return ApiCommandResourceType.StoragePool; } - public Map<String,String> getDetails() { + public Map<String,Object> getDetails() { return details; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/swift/AddSwiftCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/swift/AddSwiftCmd.java index cc0c77348a9..c322db8581a 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/swift/AddSwiftCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/swift/AddSwiftCmd.java @@ -28,7 +28,6 @@ import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.ImageStoreResponse; -import com.cloud.exception.DiscoveryException; import com.cloud.storage.ImageStore; import com.cloud.user.Account; @@ -83,25 +82,20 @@ public class AddSwiftCmd extends BaseCmd { @Override public void execute() { - Map<String, String> dm = new HashMap<String, String>(); + Map<String, String> dm = new HashMap<>(); dm.put(ApiConstants.ACCOUNT, getAccount()); dm.put(ApiConstants.USERNAME, getUsername()); dm.put(ApiConstants.KEY, getKey()); - try{ - ImageStore result = _storageService.discoverImageStore(null, getUrl(), "Swift", null, dm); - ImageStoreResponse storeResponse = null; - if (result != null) { - storeResponse = _responseGenerator.createImageStoreResponse(result); - storeResponse.setResponseName(getCommandName()); - storeResponse.setObjectName("secondarystorage"); - setResponseObject(storeResponse); - } else { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add Swift secondary storage"); - } - } catch (DiscoveryException ex) { - logger.warn("Exception: ", ex); - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, ex.getMessage()); + ImageStore result = _storageService.discoverImageStore(null, getUrl(), "Swift", null, dm); + ImageStoreResponse storeResponse; + if (result != null) { + storeResponse = _responseGenerator.createImageStoreResponse(result); + storeResponse.setResponseName(getCommandName()); + storeResponse.setObjectName("secondarystorage"); + setResponseObject(storeResponse); + } else { + throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to add Swift secondary storage"); } } } diff --git a/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java b/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java index 5909d098db8..fd87b094399 100644 --- a/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java +++ b/engine/components-api/src/main/java/com/cloud/configuration/ConfigurationManager.java @@ -71,6 +71,53 @@ public interface ConfigurationManager { "Weight for CPU (as a value between 0 and 1) applied to compute capacity for Pods, Clusters and Hosts for COMBINED capacityType for ordering. Weight for RAM will be (1 - weight of CPU)", true, ConfigKey.Scope.Global); + ConfigKey<Integer> ExpungeDelay = new ConfigKey<>( + ConfigKey.CATEGORY_ADVANCED, + Integer.class, + "expunge.delay", + "86400", + "Determines how long (in seconds) to wait before actually expunging destroyed vm. The default value = the default value of expunge.interval", + false, + ConfigKey.Scope.Global, + null); + ConfigKey<Integer> ExpungeInterval = new ConfigKey<>( + ConfigKey.CATEGORY_ADVANCED, + Integer.class, + "expunge.interval", + "86400", + "The interval (in seconds) to wait before running the expunge thread.", + false, + ConfigKey.Scope.Global, + null); + ConfigKey<Integer> ExpungeWorkers = new ConfigKey<>( + ConfigKey.CATEGORY_ADVANCED, + Integer.class, + "expunge.workers", + "10", + "Number of workers performing expunge", + false, + ConfigKey.Scope.Global, + null); + + ConfigKey<Integer> ExtractURLCleanUpInterval = new ConfigKey<>( + ConfigKey.CATEGORY_ADVANCED, + Integer.class, + "extract.url.cleanup.interval", + "7200", + "The interval (in seconds) to wait before cleaning up the extract URL's ", + false, + ConfigKey.Scope.Global, + null); + ConfigKey<Integer> ExtractURLExpirationInterval = new ConfigKey<>( + ConfigKey.CATEGORY_ADVANCED, + Integer.class, + "extract.url.expiration.interval", + "14400", + "The life of an extract URL after which it is deleted ", + false, + ConfigKey.Scope.Global, + null); + /** * Is this for a VPC * @param offering the offering to check diff --git a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java index 99504d4bc45..32a4bab3d02 100644 --- a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java +++ b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java @@ -232,6 +232,43 @@ public interface StorageManager extends StorageService { "Storage", "true", "Allow SSVMs to try copying public templates from one secondary storage to another instead of downloading them from the source.", true, ConfigKey.Scope.Zone, null); + ConfigKey<Integer> VmDiskThrottlingIopsReadRate = new ConfigKey<>( + Integer.class, + "vm.disk.throttling.iops_read_rate", + "Storage", + "0", + "Default disk I/O read rate in requests per second allowed in User vm's disk.", + true, + ConfigKey.Scope.Global, + null); + ConfigKey<Integer> VmDiskThrottlingIopsWriteRate = new ConfigKey<>( + Integer.class, + "vm.disk.throttling.iops_write_rate", + "Storage", + "0", + "Default disk I/O writerate in requests per second allowed in User vm's disk.", + true, + ConfigKey.Scope.Global, + null); + ConfigKey<Integer> VmDiskThrottlingBytesReadRate = new ConfigKey<>( + Integer.class, + "vm.disk.throttling.bytes_read_rate", + "Storage", + "0", + "Default disk I/O read rate in bytes per second allowed in User vm's disk.", + true, + ConfigKey.Scope.Global, + null); + ConfigKey<Integer> VmDiskThrottlingBytesWriteRate = new ConfigKey<>( + Integer.class, + "vm.disk.throttling.bytes_write_rate", + "Advanced", + "0", + "Default disk I/O writerate in bytes per second allowed in User vm's disk.", + true, + ConfigKey.Scope.Global, + null); + /** * should we execute in sequence not involving any storages? * @return true if commands should execute in sequence @@ -241,8 +278,8 @@ public interface StorageManager extends StorageService { } static boolean shouldExecuteInSequenceOnVmware(Long srcStoreId, Long dstStoreId) { - final Boolean fullClone = getFullCloneConfiguration(srcStoreId) || getFullCloneConfiguration(dstStoreId); - final Boolean allowParallel = getAllowParallelExecutionConfiguration(); + final boolean fullClone = getFullCloneConfiguration(srcStoreId) || getFullCloneConfiguration(dstStoreId); + final boolean allowParallel = getAllowParallelExecutionConfiguration(); return fullClone && !allowParallel; } @@ -284,10 +321,6 @@ public interface StorageManager extends StorageService { boolean canPoolProvideStorageStats(StoragePool pool); - boolean poolProvidesCustomStorageStats(StoragePool pool); - - Map<String, String> getCustomStorageStats(StoragePool pool); - /** * Checks if a host has running VMs that are using its local storage pool. * @return true if local storage is active on the host @@ -300,8 +333,6 @@ public interface StorageManager extends StorageService { */ void cleanupStorage(boolean recurring); - String getPrimaryStorageNameLabel(VolumeVO volume); - void createCapacityEntry(StoragePoolVO storagePool, short capacityType, long allocated); Answer sendToPool(StoragePool pool, long[] hostIdsToTryFirst, Command cmd) throws StorageUnavailableException; @@ -314,8 +345,6 @@ public interface StorageManager extends StorageService { CapacityVO getStoragePoolUsedStats(Long zoneId, Long podId, Long clusterId, List<Long> poolIds); - List<StoragePoolVO> ListByDataCenterHypervisor(long datacenterId, HypervisorType type); - List<VMInstanceVO> listByStoragePool(long storagePoolId); StoragePoolVO findLocalStorageOnHost(long hostId); @@ -328,11 +357,9 @@ public interface StorageManager extends StorageService { boolean canHostPrepareStoragePoolAccess(Host host, StoragePool pool); - boolean canDisconnectHostFromStoragePool(Host host, StoragePool pool); - Host getHost(long hostId); - Host updateSecondaryStorage(long secStorageId, String newUrl); + void updateSecondaryStorage(long secStorageId, String newUrl); void removeStoragePoolFromCluster(long hostId, String iScsiName, StoragePool storagePool); @@ -351,24 +378,19 @@ public interface StorageManager extends StorageService { /** * This comment is relevant to managed storage only. - * * Long clusterId = only used for managed storage - * * Some managed storage can be more efficient handling VM templates (via cloning) if it knows the capabilities of the compute cluster it is dealing with. * If the compute cluster supports UUID resigning and the storage system can clone a volume from a volume, then this determines how much more space a * new root volume (that makes use of a template) will take up on the storage system. - * * For example, if a storage system can clone a volume from a volume and the compute cluster supports UUID resigning (relevant for hypervisors like * XenServer and ESXi that put virtual disks in clustered file systems), then the storage system will need to determine if it already has a copy of * the template or if it will need to create one first before cloning the template to a new volume to be used for the new root disk (assuming the root - * disk is being deployed from a template). If the template doesn't already exists on the storage system, then you need to take into consideration space + * disk is being deployed from a template). If the template doesn't already exist on the storage system, then you need to take into consideration space * required for that template (stored in one volume) and space required for a new volume created from that template volume (for your new root volume). - * * If UUID resigning is not available in the compute cluster or the storage system doesn't support cloning a volume from a volume, then for each new * root disk that uses a template, CloudStack will have the template be copied down to a newly created volume on the storage system (i.e. no need * to take into consideration the possible need to first create a volume on the storage system for a template that will be used for the root disk * via cloning). - * * Cloning volumes on the back-end instead of copying down a new template for each new volume helps to alleviate load on the hypervisors. */ boolean storagePoolHasEnoughSpace(List<Pair<Volume, DiskProfile>> volume, StoragePool pool, Long clusterId); diff --git a/engine/storage/cache/src/main/java/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java b/engine/storage/cache/src/main/java/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java index 889d0ce14cc..e4a577646ac 100644 --- a/engine/storage/cache/src/main/java/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java +++ b/engine/storage/cache/src/main/java/org/apache/cloudstack/storage/cache/manager/StorageCacheManagerImpl.java @@ -32,6 +32,7 @@ import java.util.concurrent.TimeUnit; import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.configuration.ConfigurationManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; @@ -161,7 +162,7 @@ public class StorageCacheManagerImpl implements StorageCacheManager, Manager { public boolean configure(String name, Map<String, Object> params) throws ConfigurationException { cacheReplacementEnabled = Boolean.parseBoolean(configDao.getValue(Config.StorageCacheReplacementEnabled.key())); cacheReplaceMentInterval = NumbersUtil.parseInt(configDao.getValue(Config.StorageCacheReplacementInterval.key()), 86400); - workers = NumbersUtil.parseInt(configDao.getValue(Config.ExpungeWorkers.key()), 10); + workers = ConfigurationManager.ExpungeWorkers.value(); executors = Executors.newScheduledThreadPool(workers, new NamedThreadFactory("StorageCacheManager-cache-replacement")); return true; } diff --git a/plugins/storage/image/s3/src/main/java/org/apache/cloudstack/storage/datastore/driver/S3ImageStoreDriverImpl.java b/plugins/storage/image/s3/src/main/java/org/apache/cloudstack/storage/datastore/driver/S3ImageStoreDriverImpl.java index 9b2f3ddd100..e5fe2c4b923 100644 --- a/plugins/storage/image/s3/src/main/java/org/apache/cloudstack/storage/datastore/driver/S3ImageStoreDriverImpl.java +++ b/plugins/storage/image/s3/src/main/java/org/apache/cloudstack/storage/datastore/driver/S3ImageStoreDriverImpl.java @@ -25,6 +25,7 @@ import java.util.Map; import javax.inject.Inject; +import com.cloud.configuration.ConfigurationManager; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; @@ -37,7 +38,6 @@ import com.cloud.agent.api.to.DataStoreTO; import com.cloud.agent.api.to.S3TO; import com.cloud.configuration.Config; import com.cloud.storage.Storage.ImageFormat; -import com.cloud.utils.NumbersUtil; import com.cloud.utils.storage.S3.S3Utils; public class S3ImageStoreDriverImpl extends BaseImageStoreDriverImpl { @@ -86,22 +86,18 @@ public class S3ImageStoreDriverImpl extends BaseImageStoreDriverImpl { */ S3TO s3 = (S3TO)getStoreTO(store); - if(logger.isDebugEnabled()) { - logger.debug("Generating pre-signed s3 entity extraction URL for object: " + key); - } + logger.debug("Generating pre-signed s3 entity extraction URL for object: {}", key); Date expiration = new Date(); long milliSeconds = expiration.getTime(); // Get extract url expiration interval set in global configuration (in seconds) - String urlExpirationInterval = _configDao.getValue(Config.ExtractURLExpirationInterval.toString()); - // Expired after configured interval (in milliseconds), default 14400 seconds - milliSeconds += 1000 * NumbersUtil.parseInt(urlExpirationInterval, 14400); + milliSeconds += 1000L * ConfigurationManager.ExtractURLExpirationInterval.value(); expiration.setTime(milliSeconds); URL s3url = S3Utils.generatePresignedUrl(s3, s3.getBucketName(), key, expiration); - logger.info("Pre-Signed URL = " + s3url.toString()); + logger.info("Pre-Signed URL = {}", s3url.toString()); return s3url.toString(); } diff --git a/plugins/storage/image/swift/src/main/java/org/apache/cloudstack/storage/datastore/driver/SwiftImageStoreDriverImpl.java b/plugins/storage/image/swift/src/main/java/org/apache/cloudstack/storage/datastore/driver/SwiftImageStoreDriverImpl.java index 61ff57fd058..6670de1e469 100644 --- a/plugins/storage/image/swift/src/main/java/org/apache/cloudstack/storage/datastore/driver/SwiftImageStoreDriverImpl.java +++ b/plugins/storage/image/swift/src/main/java/org/apache/cloudstack/storage/datastore/driver/SwiftImageStoreDriverImpl.java @@ -24,6 +24,7 @@ import java.util.UUID; import javax.inject.Inject; +import com.cloud.configuration.ConfigurationManager; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataObject; @@ -33,7 +34,6 @@ import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; import org.apache.cloudstack.engine.subsystem.api.storage.StorageCacheManager; import org.apache.cloudstack.framework.async.AsyncCallbackDispatcher; import org.apache.cloudstack.framework.async.AsyncCompletionCallback; -import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.storage.command.DownloadCommand; import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao; import org.apache.cloudstack.storage.image.BaseImageStoreDriverImpl; @@ -44,7 +44,6 @@ import com.cloud.agent.api.storage.DownloadAnswer; import com.cloud.agent.api.to.DataObjectType; import com.cloud.agent.api.to.DataStoreTO; import com.cloud.agent.api.to.SwiftTO; -import com.cloud.configuration.Config; import com.cloud.storage.Storage.ImageFormat; import com.cloud.utils.SwiftUtil; import com.cloud.utils.exception.CloudRuntimeException; @@ -57,8 +56,6 @@ public class SwiftImageStoreDriverImpl extends BaseImageStoreDriverImpl { EndPointSelector _epSelector; @Inject StorageCacheManager cacheManager; - @Inject - ConfigurationDao _configDao; @Override public DataStoreTO getStoreTO(DataStore store) { @@ -83,15 +80,11 @@ public class SwiftImageStoreDriverImpl extends BaseImageStoreDriverImpl { String containerName = SwiftUtil.getContainerName(dataObject.getType().toString(), dataObject.getId()); String objectName = installPath.split("\\/")[1]; // Get extract url expiration interval set in global configuration (in seconds) - int urlExpirationInterval = Integer.parseInt(_configDao.getValue(Config.ExtractURLExpirationInterval.toString())); + int urlExpirationInterval = ConfigurationManager.ExtractURLExpirationInterval.value(); URL swiftUrl = SwiftUtil.generateTempUrl(swiftTO, containerName, objectName, tempKey, urlExpirationInterval); - if (swiftUrl != null) { - logger.debug("Swift temp-url: " + swiftUrl.toString()); - return swiftUrl.toString(); - } - - throw new CloudRuntimeException("Unable to create extraction URL"); + logger.debug("Swift temp-url: {}", swiftUrl); + return swiftUrl.toString(); } @Override @@ -115,7 +108,7 @@ public class SwiftImageStoreDriverImpl extends BaseImageStoreDriverImpl { throw new CloudRuntimeException(errMsg); } - CreateContext<CreateCmdResult> context = new CreateContext<CreateCmdResult>(callback, data); + CreateContext<CreateCmdResult> context = new CreateContext<>(callback, data); AsyncCallbackDispatcher<SwiftImageStoreDriverImpl, DownloadAnswer> caller = AsyncCallbackDispatcher.create(this); caller.setContext(context); @@ -125,7 +118,5 @@ public class SwiftImageStoreDriverImpl extends BaseImageStoreDriverImpl { caller.setCallback(caller.getTarget().createVolumeAsyncCallback(null, null)); } ep.sendMessageAsync(dcmd, caller); - } - } diff --git a/server/src/main/java/com/cloud/configuration/Config.java b/server/src/main/java/com/cloud/configuration/Config.java index abae4d3996c..cc79c426522 100644 --- a/server/src/main/java/com/cloud/configuration/Config.java +++ b/server/src/main/java/com/cloud/configuration/Config.java @@ -424,31 +424,6 @@ public enum Config { "The interval (in seconds) between cleanup for removed accounts", null), InstanceName("Advanced", AgentManager.class, String.class, "instance.name", "VM", "Name of the deployment instance.", "instanceName"), - ExpungeDelay( - "Advanced", - UserVmManager.class, - Integer.class, - "expunge.delay", - "86400", - "Determines how long (in seconds) to wait before actually expunging destroyed vm. The default value = the default value of expunge.interval", - null), - ExpungeInterval( - "Advanced", - UserVmManager.class, - Integer.class, - "expunge.interval", - "86400", - "The interval (in seconds) to wait before running the expunge thread.", - null), - ExpungeWorkers("Advanced", UserVmManager.class, Integer.class, "expunge.workers", "1", "Number of workers performing expunge ", null), - ExtractURLCleanUpInterval( - "Advanced", - ManagementServer.class, - Integer.class, - "extract.url.cleanup.interval", - "7200", - "The interval (in seconds) to wait before cleaning up the extract URL's ", - null), DisableExtraction( "Advanced", ManagementServer.class, @@ -457,14 +432,6 @@ public enum Config { "false", "Flag for disabling extraction of Templates, ISOs, Snapshots and volumes", null), - ExtractURLExpirationInterval( - "Advanced", - ManagementServer.class, - Integer.class, - "extract.url.expiration.interval", - "14400", - "The life of an extract URL after which it is deleted ", - null), HostStatsInterval( "Advanced", ManagementServer.class, @@ -750,38 +717,6 @@ public enum Config { "3600", "Time (in seconds) to wait before taking over a VM in transition state", null), - VmDiskThrottlingIopsReadRate( - "Advanced", - ManagementServer.class, - Integer.class, - "vm.disk.throttling.iops_read_rate", - "0", - "Default disk I/O read rate in requests per second allowed in User vm's disk.", - null), - VmDiskThrottlingIopsWriteRate( - "Advanced", - ManagementServer.class, - Integer.class, - "vm.disk.throttling.iops_write_rate", - "0", - "Default disk I/O writerate in requests per second allowed in User vm's disk.", - null), - VmDiskThrottlingBytesReadRate( - "Advanced", - ManagementServer.class, - Integer.class, - "vm.disk.throttling.bytes_read_rate", - "0", - "Default disk I/O read rate in bytes per second allowed in User vm's disk.", - null), - VmDiskThrottlingBytesWriteRate( - "Advanced", - ManagementServer.class, - Integer.class, - "vm.disk.throttling.bytes_write_rate", - "0", - "Default disk I/O writerate in bytes per second allowed in User vm's disk.", - null), ControlCidr( "Advanced", ManagementServer.class, diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 21995d5ae65..93ac9396aab 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -8489,7 +8489,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati BYTES_MAX_READ_LENGTH, BYTES_MAX_WRITE_LENGTH, ADD_HOST_ON_SERVICE_RESTART_KVM, SET_HOST_DOWN_TO_MAINTENANCE, VM_SERVICE_OFFERING_MAX_CPU_CORES, VM_SERVICE_OFFERING_MAX_RAM_SIZE, MIGRATE_VM_ACROSS_CLUSTERS, ENABLE_ACCOUNT_SETTINGS_FOR_DOMAIN, ENABLE_DOMAIN_SETTINGS_FOR_CHILD_DOMAIN, - ALLOW_DOMAIN_ADMINS_TO_CREATE_TAGGED_OFFERINGS, DELETE_QUERY_BATCH_SIZE, AllowNonRFC1918CompliantIPs, HostCapacityTypeCpuMemoryWeight + ALLOW_DOMAIN_ADMINS_TO_CREATE_TAGGED_OFFERINGS, DELETE_QUERY_BATCH_SIZE, AllowNonRFC1918CompliantIPs, HostCapacityTypeCpuMemoryWeight, + ExpungeDelay, ExpungeInterval, ExpungeWorkers, ExtractURLCleanUpInterval, ExtractURLExpirationInterval }; } diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index c1cee1385c6..dd181733552 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -18,16 +18,12 @@ package com.cloud.storage; import static com.cloud.utils.NumbersUtil.toHumanReadableSize; -import java.io.UnsupportedEncodingException; import java.math.BigDecimal; import java.math.BigInteger; import java.net.URI; import java.net.URISyntaxException; import java.net.URLDecoder; -import java.net.UnknownHostException; import java.nio.file.Files; -import java.sql.PreparedStatement; -import java.sql.ResultSet; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -35,7 +31,6 @@ import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; @@ -148,8 +143,8 @@ import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity; import org.apache.cloudstack.storage.object.ObjectStore; import org.apache.cloudstack.storage.object.ObjectStoreEntity; import org.apache.cloudstack.storage.to.VolumeObjectTO; -import org.apache.commons.collections.CollectionUtils; -import org.apache.commons.collections.MapUtils; +import org.apache.commons.collections4.CollectionUtils; +import org.apache.commons.collections4.MapUtils; import org.apache.commons.lang.time.DateUtils; import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.EnumUtils; @@ -182,7 +177,6 @@ import com.cloud.capacity.CapacityState; import com.cloud.capacity.CapacityVO; import com.cloud.capacity.dao.CapacityDao; import com.cloud.cluster.ClusterManagerListener; -import com.cloud.configuration.Config; import com.cloud.configuration.ConfigurationManager; import com.cloud.configuration.ConfigurationManagerImpl; import com.cloud.configuration.Resource.ResourceType; @@ -198,11 +192,9 @@ import com.cloud.event.EventTypes; import com.cloud.exception.AgentUnavailableException; import com.cloud.exception.ConnectionException; import com.cloud.exception.DiscoveryException; -import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.OperationTimedoutException; import com.cloud.exception.PermissionDeniedException; -import com.cloud.exception.ResourceInUseException; import com.cloud.exception.ResourceUnavailableException; import com.cloud.exception.StorageConflictException; import com.cloud.exception.StorageUnavailableException; @@ -219,11 +211,9 @@ import com.cloud.offering.ServiceOffering; import com.cloud.org.Grouping; import com.cloud.org.Grouping.AllocationState; import com.cloud.resource.ResourceState; -import com.cloud.server.ConfigurationServer; import com.cloud.server.ManagementServer; import com.cloud.server.ManagementService; import com.cloud.server.StatsCollector; -import com.cloud.service.dao.ServiceOfferingDetailsDao; import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.Volume.Type; @@ -273,7 +263,6 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.DiskProfile; import com.cloud.vm.UserVmManager; import com.cloud.vm.VMInstanceVO; -import com.cloud.vm.VirtualMachine; import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.dao.VMInstanceDao; import com.google.common.collect.Sets; @@ -356,8 +345,6 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Inject SnapshotDataFactory snapshotFactory; @Inject - ConfigurationServer _configServer; - @Inject DataStoreManager _dataStoreMgr; @Inject DataStoreProviderManager _dataStoreProviderMgr; @@ -374,8 +361,6 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Inject SnapshotService _snapshotService; @Inject - public StorageService storageService; - @Inject StoragePoolTagsDao _storagePoolTagsDao; @Inject StoragePoolAndAccessGroupMapDao _storagePoolAccessGroupMapDao; @@ -384,8 +369,6 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Inject DiskOfferingDetailsDao _diskOfferingDetailsDao; @Inject - ServiceOfferingDetailsDao _serviceOfferingDetailsDao; - @Inject VsphereStoragePolicyDao _vsphereStoragePolicyDao; @Inject private AnnotationDao annotationDao; @@ -414,20 +397,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Inject ResourceManager _resourceMgr; @Inject - StorageManager storageManager; - @Inject ManagementService managementService; - protected List<StoragePoolDiscoverer> _discoverers; - - public List<StoragePoolDiscoverer> getDiscoverers() { - return _discoverers; - } - - public void setDiscoverers(List<StoragePoolDiscoverer> discoverers) { - _discoverers = discoverers; - } - protected GenericSearchBuilder<StoragePoolHostVO, Long> UpHostsInPoolSearch; protected SearchBuilder<VMInstanceVO> StoragePoolSearch; protected SearchBuilder<StoragePoolVO> LocalStorageSearch; @@ -445,44 +416,6 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C private static final String NFS_MOUNT_OPTIONS_INCORRECT = "An incorrect mount option was specified"; - public boolean share(VMInstanceVO vm, List<VolumeVO> vols, HostVO host, boolean cancelPreviousShare) throws StorageUnavailableException { - - // if pool is in maintenance and it is the ONLY pool available; reject - List<VolumeVO> rootVolForGivenVm = volumeDao.findByInstanceAndType(vm.getId(), Type.ROOT); - if (rootVolForGivenVm != null && rootVolForGivenVm.size() > 0) { - boolean isPoolAvailable = isPoolAvailable(rootVolForGivenVm.get(0).getPoolId()); - if (!isPoolAvailable) { - throw new StorageUnavailableException("Can not share " + vm, rootVolForGivenVm.get(0).getPoolId()); - } - } - - // this check is done for maintenance mode for primary storage - // if any one of the volume is unusable, we return false - // if we return false, the allocator will try to switch to another PS if - // available - for (VolumeVO vol : vols) { - if (vol.getRemoved() != null) { - logger.warn("Volume: {} is removed, cannot share on this instance: {}", vol, vm); - // not ok to share - return false; - } - } - // ok to share - return true; - } - - private boolean isPoolAvailable(Long poolId) { - // get list of all pools - List<StoragePoolVO> pools = _storagePoolDao.listAll(); - - // if no pools or 1 pool which is in maintenance - if (pools == null || pools.size() == 0 || (pools.size() == 1 && pools.get(0).getStatus().equals(StoragePoolStatus.Maintenance))) { - return false; - } else { - return true; - } - } - protected void enableDefaultDatastoreDownloadRedirectionForExistingInstallations() { if (!configDepot.isNewConfig(DataStoreDownloadFollowRedirects)) { logger.trace("{} is not a new configuration, skipping updating its value", @@ -492,35 +425,12 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C List<DataCenterVO> zones = _dcDao.listAll(new Filter(1)); if (CollectionUtils.isNotEmpty(zones)) { - logger.debug(String.format("Updating value for configuration: %s to true", - DataStoreDownloadFollowRedirects.key())); + logger.debug("Updating value for configuration: {} to true", + DataStoreDownloadFollowRedirects.key()); configurationDao.update(DataStoreDownloadFollowRedirects.key(), "true"); } } - @Override - public List<StoragePoolVO> ListByDataCenterHypervisor(long datacenterId, HypervisorType type) { - List<StoragePoolVO> pools = _storagePoolDao.listByDataCenterId(datacenterId); - List<StoragePoolVO> retPools = new ArrayList<>(); - for (StoragePoolVO pool : pools) { - if (pool.getStatus() != StoragePoolStatus.Up) { - continue; - } - if (pool.getScope() == ScopeType.ZONE) { - if (pool.getHypervisor() != null && pool.getHypervisor() == type) { - retPools.add(pool); - } - } else { - ClusterVO cluster = _clusterDao.findById(pool.getClusterId()); - if (type == cluster.getHypervisorType()) { - retPools.add(pool); - } - } - } - Collections.shuffle(retPools); - return retPools; - } - @Override public boolean isLocalStorageActiveOnHost(Long hostId) { List<StoragePoolHostVO> storagePoolHostRefs = _storagePoolHostDao.listByHostId(hostId); @@ -542,7 +452,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C volumeSC.setJoinParameters("activeVmSB", "state", State.Starting, State.Running, State.Stopping, State.Migrating); List<VolumeVO> volumes = volumeDao.search(volumeSC, null); - if (volumes.size() > 0) { + if (!volumes.isEmpty()) { return true; } } @@ -611,31 +521,6 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C return storeDriver instanceof PrimaryDataStoreDriver && ((PrimaryDataStoreDriver)storeDriver).canProvideStorageStats(); } - @Override - public boolean poolProvidesCustomStorageStats(StoragePool pool) { - DataStoreProvider storeProvider = _dataStoreProviderMgr.getDataStoreProvider(pool.getStorageProviderName()); - DataStoreDriver storeDriver = storeProvider.getDataStoreDriver(); - return storeDriver instanceof PrimaryDataStoreDriver && ((PrimaryDataStoreDriver)storeDriver).poolProvidesCustomStorageStats(); - } - - @Override - public Map<String, String> getCustomStorageStats(StoragePool pool) { - if (pool == null) { - return null; - } - - if (!pool.isManaged()) { - return null; - } - - DataStoreProvider storeProvider = _dataStoreProviderMgr.getDataStoreProvider(pool.getStorageProviderName()); - DataStoreDriver storeDriver = storeProvider.getDataStoreDriver(); - if (storeDriver instanceof PrimaryDataStoreDriver) { - return ((PrimaryDataStoreDriver)storeDriver).getCustomStorageStats(pool); - } - return null; - } - @Override public Answer getVolumeStats(StoragePool pool, Command cmd) { DataStoreProvider storeProvider = _dataStoreProviderMgr.getDataStoreProvider(pool.getStorageProviderName()); @@ -657,56 +542,27 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C return new GetVolumeStatsAnswer(getVolumeStatsCommand, "", statEntry); } - public Long chooseHostForStoragePool(StoragePoolVO poolVO, List<Long> avoidHosts, boolean sendToVmResidesOn, Long vmId) { - if (sendToVmResidesOn) { - if (vmId != null) { - VMInstanceVO vmInstance = _vmInstanceDao.findById(vmId); - if (vmInstance != null) { - Long hostId = vmInstance.getHostId(); - if (hostId != null && !avoidHosts.contains(vmInstance.getHostId())) { - return hostId; - } - } - } - /* - * Can't find the vm where host resides on(vm is destroyed? or - * volume is detached from vm), randomly choose a host to send the - * cmd - */ - } - List<StoragePoolHostVO> poolHosts = _storagePoolHostDao.listByHostStatus(poolVO.getId(), Status.Up); - Collections.shuffle(poolHosts); - if (poolHosts != null && poolHosts.size() > 0) { - for (StoragePoolHostVO sphvo : poolHosts) { - if (!avoidHosts.contains(sphvo.getHostId())) { - return sphvo.getHostId(); - } - } - } - return null; - } - @Override public boolean configure(String name, Map<String, Object> params) { Map<String, String> configs = _configDao.getConfiguration("management-server", params); _storagePoolAcquisitionWaitSeconds = NumbersUtil.parseInt(configs.get("pool.acquisition.wait.seconds"), 1800); - logger.info("pool.acquisition.wait.seconds is configured as " + _storagePoolAcquisitionWaitSeconds + " seconds"); + logger.info("pool.acquisition.wait.seconds is configured as {} seconds", _storagePoolAcquisitionWaitSeconds); _agentMgr.registerForHostEvents(new StoragePoolMonitor(this, _storagePoolDao, _storagePoolHostDao, _dataStoreProviderMgr), true, false, true); - logger.info("Storage cleanup enabled: " + StorageCleanupEnabled.value() + ", interval: " + StorageCleanupInterval.value() + ", delay: " + StorageCleanupDelay.value() - + ", template cleanup enabled: " + TemplateCleanupEnabled.value()); + logger.info("Storage cleanup enabled: {}, interval: {}, delay: {}, template cleanup enabled: {}", + StorageCleanupEnabled.value(), + StorageCleanupInterval.value(), + StorageCleanupDelay.value(), + TemplateCleanupEnabled.value()); - String cleanupInterval = configs.get("extract.url.cleanup.interval"); - _downloadUrlCleanupInterval = NumbersUtil.parseInt(cleanupInterval, 7200); + _downloadUrlCleanupInterval = ConfigurationManager.ExtractURLCleanUpInterval.value(); - String urlExpirationInterval = configs.get("extract.url.expiration.interval"); - _downloadUrlExpirationInterval = NumbersUtil.parseInt(urlExpirationInterval, 14400); + _downloadUrlExpirationInterval = ConfigurationManager.ExtractURLExpirationInterval.value(); - String workers = configs.get("expunge.workers"); - int wrks = NumbersUtil.parseInt(workers, 10); - _executor = Executors.newScheduledThreadPool(wrks, new NamedThreadFactory("StorageManager-Scavenger")); + int workers = ConfigurationManager.ExpungeWorkers.value(); + _executor = Executors.newScheduledThreadPool(workers, new NamedThreadFactory("StorageManager-Scavenger")); _agentMgr.registerForHostEvents(ComponentContext.inject(LocalStoragePoolListener.class), true, false, false); @@ -829,7 +685,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C boolean useLocalStorageForSystemVM = false; Boolean isLocal = ConfigurationManagerImpl.SystemVMUseLocalStorage.valueIn(dc.getId()); if (isLocal != null) { - useLocalStorageForSystemVM = isLocal.booleanValue(); + useLocalStorageForSystemVM = isLocal; } if (!(dc.isLocalStorageEnabled() || useLocalStorageForSystemVM)) { return null; @@ -850,7 +706,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C // not able to distinguish multiple local datastores that may be // available on the host, to support smooth migration, we // need to perform runtime upgrade here - if (pInfo.getHostPath().length() > 0) { + if (!pInfo.getHostPath().isEmpty()) { pool = _storagePoolDao.findPoolByHostPath(host.getDataCenterId(), host.getPodId(), hostAddress, "", pInfo.getUuid()); } } @@ -858,7 +714,9 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C //the path can be different, but if they have the same uuid, assume they are the same storage pool = _storagePoolDao.findPoolByHostPath(host.getDataCenterId(), host.getPodId(), hostAddress, null, pInfo.getUuid()); if (pool != null) { - logger.debug("Found a storage pool: " + pInfo.getUuid() + ", but with different hostpath " + pInfo.getHostPath() + ", still treat it as the same pool"); + logger.debug("Found a storage pool: {}, but with different hostpath {}, still treat it as the same pool", + pInfo.getUuid(), + pInfo.getHostPath()); } } @@ -961,7 +819,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } @Override - public PrimaryDataStoreInfo createPool(CreateStoragePoolCmd cmd) throws ResourceInUseException, IllegalArgumentException, UnknownHostException, ResourceUnavailableException { + public PrimaryDataStoreInfo createPool(CreateStoragePoolCmd cmd) throws IllegalArgumentException { String providerName = cmd.getStorageProviderName(); Map<String,String> uriParams = extractUriParamsAsMap(cmd.getUrl()); boolean isFileScheme = "file".equals(uriParams.get("scheme")); @@ -997,7 +855,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C throw new InvalidParameterValueException("zone id can't be null, if scope is zone"); } - HypervisorType hypervisorType = HypervisorType.KVM; + HypervisorType hypervisorType; // defaults to HypervisorType.KVM if (scopeType == ScopeType.ZONE) { // ignore passed clusterId and podId clusterId = null; @@ -1072,7 +930,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C lifeCycle.attachZone(store, zoneScope, hypervisorType); } } catch (Exception e) { - logger.debug("Failed to add data store: " + e.getMessage(), e); + logger.debug("Failed to add data store: {}", e.getMessage(), e); try { // clean up the db, just absorb the exception thrown in deletion with error logged, so that user can get error for adding data store // not deleting data store. @@ -1080,7 +938,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C lifeCycle.deleteDataStore(store); } } catch (Exception ex) { - logger.debug("Failed to clean up storage pool: " + ex.getMessage()); + logger.debug("Failed to clean up storage pool: {}", ex.getMessage()); } throw new CloudRuntimeException("Failed to add data store: " + e.getMessage(), e); } @@ -1094,9 +952,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C try { uriInfo = UriUtils.getUriInfo(url); } catch (CloudRuntimeException cre) { - if (logger.isDebugEnabled()) { - logger.debug(String.format("URI validation for url: %s failed, returning empty uri params", url)); - } + logger.debug("URI validation for url: {} failed, returning empty uri params", url); return uriParams; } @@ -1104,9 +960,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C String storageHost = uriInfo.getStorageHost(); String storagePath = uriInfo.getStoragePath(); if (scheme == null) { - if (logger.isDebugEnabled()) { - logger.debug(String.format("Scheme for url: %s is not found, returning empty uri params", url)); - } + logger.debug("Scheme for url: {} is not found, returning empty uri params", url); return uriParams; } boolean isHostOrPathBlank = StringUtils.isAnyBlank(storagePath, storageHost); @@ -1139,12 +993,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } } - String hostPath = null; - try { - hostPath = URLDecoder.decode(storagePath, "UTF-8"); - } catch (UnsupportedEncodingException e) { - logger.error("[ignored] we are on a platform not supporting \"UTF-8\"!?!", e); - } + String hostPath = URLDecoder.decode(storagePath, StringUtils.getPreferredCharset()); if (hostPath == null) { // if decoding fails, use getPath() anyway hostPath = storagePath; } @@ -1159,17 +1008,15 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C return uriParams; } - private Map<String, String> extractApiParamAsMap(Map ds) { + private Map<String, String> extractApiParamAsMap(Map<String, Object> ds) { Map<String, String> details = new HashMap<>(); if (ds != null) { - Collection detailsCollection = ds.values(); - Iterator it = detailsCollection.iterator(); - while (it.hasNext()) { - HashMap d = (HashMap)it.next(); - Iterator it2 = d.entrySet().iterator(); - while (it2.hasNext()) { - Map.Entry entry = (Map.Entry)it2.next(); - details.put((String)entry.getKey(), (String)entry.getValue()); + Collection<Object> detailsCollection = ds.values(); + for (Object o : detailsCollection) { + HashMap d = (HashMap) o; + for (Object object : d.entrySet()) { + Map.Entry entry = (Map.Entry) object; + details.put((String) entry.getKey(), (String) entry.getValue()); } } } @@ -1233,17 +1080,14 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C String name = cmd.getName(); if(StringUtils.isNotBlank(name)) { - logger.debug("Updating Storage Pool name to: " + name); + logger.debug("Updating Storage Pool name to: {}", name); pool.setName(name); _storagePoolDao.update(pool.getId(), pool); } - final List<String> storagePoolTags = cmd.getTags(); if (storagePoolTags != null) { - if (logger.isDebugEnabled()) { - logger.debug("Updating Storage Pool Tags to :" + storagePoolTags); - } + logger.debug("Updating Storage Pool Tags to : {}", storagePoolTags); if (pool.getPoolType() == StoragePoolType.DatastoreCluster) { List<StoragePoolVO> childStoragePools = _storagePoolDao.listChildStoragePoolsInDatastoreCluster(pool.getId()); for (StoragePoolVO childPool : childStoragePools) { @@ -1274,9 +1118,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } // retrieve current details and merge/overlay input to capture changes - Map<String, String> details = null; - details = _storagePoolDetailsDao.listDetailsKeyPairs(id); - if (inputDetails != null) { + Map<String, String> details = _storagePoolDetailsDao.listDetailsKeyPairs(id); + if (!inputDetails.isEmpty()) { details.putAll(inputDetails); changes = true; } @@ -1288,11 +1131,11 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C if (dataStoreLifeCycle instanceof PrimaryDataStoreLifeCycle) { if (updatedCapacityBytes != null) { - details.put(PrimaryDataStoreLifeCycle.CAPACITY_BYTES, updatedCapacityBytes != null ? String.valueOf(updatedCapacityBytes) : null); + details.put(PrimaryDataStoreLifeCycle.CAPACITY_BYTES, String.valueOf(updatedCapacityBytes)); _storagePoolDao.updateCapacityBytes(id, updatedCapacityBytes); } if (updatedCapacityIops != null) { - details.put(PrimaryDataStoreLifeCycle.CAPACITY_IOPS, updatedCapacityIops != null ? String.valueOf(updatedCapacityIops) : null); + details.put(PrimaryDataStoreLifeCycle.CAPACITY_IOPS, String.valueOf(updatedCapacityIops)); _storagePoolDao.updateCapacityIops(id, updatedCapacityIops); } if (cmd.getUrl() != null) { @@ -1339,9 +1182,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C throw new PermissionDeniedException("Cannot perform this operation, Cluster is currently disabled: " + clusterId); } - List<VirtualMachine.State> states = Arrays.asList(State.Starting, State.Running, State.Stopping, State.Migrating, State.Restoring); - - Long id = primaryStorage.getId(); + long id = primaryStorage.getId(); Pair<List<VMInstanceVO>, Integer> vmsNotInClusterUsingPool = _vmInstanceDao.listByVmsNotInClusterUsingPool(clusterId, id); if (vmsNotInClusterUsingPool.second() != 0) { throw new CloudRuntimeException(String.format("Cannot change scope of the storage pool [%s] to cluster [%s] " + @@ -1528,9 +1369,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C throw new CloudRuntimeException("Storage Access Groups are not suitable for local storage"); } - if (logger.isDebugEnabled()) { - logger.debug("Updating Storage Pool Access Group Maps to :" + storageAccessGroups); - } + logger.debug("Updating Storage Pool Access Group Maps to : {}", storageAccessGroups); if (storagePool.getPoolType() == StoragePoolType.DatastoreCluster) { List<StoragePoolVO> childStoragePools = _storagePoolDao.listChildStoragePoolsInDatastoreCluster(storagePool.getId()); @@ -1553,9 +1392,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C String storageAccessGroupsOnZone = zoneVO.getStorageAccessGroups(); List<String> zoneTagsList = parseTags(storageAccessGroupsOnZone); - List<String> newTags = storageAccessGroups; - List<String> existingTagsOnZone = (List<String>) CollectionUtils.intersection(newTags, zoneTagsList); + List<String> existingTagsOnZone = new ArrayList<>(CollectionUtils.intersection(storageAccessGroups, zoneTagsList)); if (CollectionUtils.isNotEmpty(existingTagsOnZone)) { throw new CloudRuntimeException(String.format("access groups already exist on the zone: %s", existingTagsOnZone)); @@ -1571,10 +1409,9 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C List<String> podTagsList = parseTags(storageAccessGroupsOnPod); List<String> zoneTagsList = parseTags(storageAccessGroupsOnZone); - List<String> newTags = storageAccessGroups; - List<String> existingTagsOnPod = (List<String>) CollectionUtils.intersection(newTags, podTagsList); - List<String> existingTagsOnZone = (List<String>) CollectionUtils.intersection(newTags, zoneTagsList); + List<String> existingTagsOnPod = new ArrayList<>(CollectionUtils.intersection(storageAccessGroups, podTagsList)); + List<String> existingTagsOnZone = new ArrayList<>(CollectionUtils.intersection(storageAccessGroups, zoneTagsList)); if (CollectionUtils.isNotEmpty(existingTagsOnPod) || CollectionUtils.isNotEmpty(existingTagsOnZone)) { String message = "access groups already exist "; @@ -1605,11 +1442,10 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C List<String> podTagsList = parseTags(storageAccessGroupsOnPod); List<String> zoneTagsList = parseTags(storageAccessGroupsOnZone); List<String> clusterTagsList = parseTags(storageAccessGroupsOnCluster); - List<String> newTags = storageAccessGroups; - List<String> existingTagsOnCluster = (List<String>) CollectionUtils.intersection(newTags, clusterTagsList); - List<String> existingTagsOnPod = (List<String>) CollectionUtils.intersection(newTags, podTagsList); - List<String> existingTagsOnZone = (List<String>) CollectionUtils.intersection(newTags, zoneTagsList); + List<String> existingTagsOnCluster = new ArrayList<>(CollectionUtils.intersection(storageAccessGroups, clusterTagsList)); + List<String> existingTagsOnPod = new ArrayList<>(CollectionUtils.intersection(storageAccessGroups, podTagsList)); + List<String> existingTagsOnZone = new ArrayList<>(CollectionUtils.intersection(storageAccessGroups, zoneTagsList)); if (CollectionUtils.isNotEmpty(existingTagsOnCluster) || CollectionUtils.isNotEmpty(existingTagsOnPod) || CollectionUtils.isNotEmpty(existingTagsOnZone)) { String message = "access groups already exist "; @@ -1674,7 +1510,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C StoragePoolVO sPool = _storagePoolDao.findById(id); if (sPool == null) { - logger.warn("Unable to find pool:" + id); + logger.warn("Unable to find pool: {}", id); throw new InvalidParameterValueException("Unable to find pool by id " + id); } if (sPool.getStatus() != StoragePoolStatus.Maintenance) { @@ -1683,7 +1519,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } if (sPool.getPoolType() == StoragePoolType.DatastoreCluster) { - // FR41 yet to handle on failure of deletion of any of the child storage pool + // FR41 yet to handle on failure of deletion any child storage pool if (checkIfDataStoreClusterCanbeDeleted(sPool, forced)) { Transaction.execute(new TransactionCallbackNoReturn() { @Override @@ -1705,17 +1541,15 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Override public Pair<Map<String, String>, Boolean> getStoragePoolNFSMountOpts(StoragePool pool, Map<String, String> details) { boolean details_added = false; - if (!pool.getPoolType().equals(Storage.StoragePoolType.NetworkFilesystem)) { - return new Pair<>(details, details_added); - } - - StoragePoolDetailVO nfsMountOpts = _storagePoolDetailsDao.findDetail(pool.getId(), ApiConstants.NFS_MOUNT_OPTIONS); - if (nfsMountOpts != null) { - if (details == null) { - details = new HashMap<>(); + if (pool.getPoolType().equals(Storage.StoragePoolType.NetworkFilesystem)) { + StoragePoolDetailVO nfsMountOpts = _storagePoolDetailsDao.findDetail(pool.getId(), ApiConstants.NFS_MOUNT_OPTIONS); + if (nfsMountOpts != null) { + if (details == null) { + details = new HashMap<>(); + } + details.put(ApiConstants.NFS_MOUNT_OPTIONS, nfsMountOpts.getValue()); + details_added = true; } - details.put(ApiConstants.NFS_MOUNT_OPTIONS, nfsMountOpts.getValue()); - details_added = true; } return new Pair<>(details, details_added); } @@ -1931,7 +1765,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } } catch (Exception ex) { - logger.error("hostEnabled(long) failed for storage provider " + provider.getName(), ex); + logger.error("hostEnabled(long) failed for storage provider {}", provider.getName(), ex); } } } @@ -1940,7 +1774,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Override public BigDecimal getStorageOverProvisioningFactor(Long poolId) { - return new BigDecimal(CapacityManager.StorageOverprovisioningFactor.valueIn(poolId)); + return BigDecimal.valueOf(CapacityManager.StorageOverprovisioningFactor.valueIn(poolId)); } @Override @@ -1992,7 +1826,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } } - if (capacities.size() == 0) { + if (capacities.isEmpty()) { CapacityVO capacity = new CapacityVO(storagePool.getId(), storagePool.getDataCenterId(), storagePool.getPodId(), storagePool.getClusterId(), allocated, totalOverProvCapacity, capacityType); capacity.setCapacityState(capacityState); @@ -2034,7 +1868,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C if (hostIdsToAvoid != null) { hostIds.removeAll(hostIdsToAvoid); } - if (hostIds == null || hostIds.isEmpty()) { + if (hostIds.isEmpty()) { throw new StorageUnavailableException(String.format("Unable to send command to the pool %s due to there is no enabled hosts up in this cluster", pool), pool.getId()); } for (Long hostId : hostIds) { @@ -2045,7 +1879,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C long targetHostId = _hvGuruMgr.getGuruProcessedCommandTargetHost(hostId, cmd); answers.add(_agentMgr.send(targetHostId, cmd)); } - return new Pair<>(hostId, answers.toArray(new Answer[answers.size()])); + return new Pair<>(hostId, answers.toArray(new Answer[0])); } catch (AgentUnavailableException | OperationTimedoutException e) { logger.debug("Unable to send storage pool command to {} via {}", pool::toString, () -> _hostDao.findById(hostId), () -> e); } @@ -2104,8 +1938,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C _tmpltMgr.evictTemplateFromStoragePool(templatePoolVO); } } catch (Exception e) { - logger.error(String.format("Failed to clean up primary storage pool [%s] due to: [%s].", pool, e.getMessage())); - logger.debug(String.format("Failed to clean up primary storage pool [%s].", pool), e); + logger.error("Failed to clean up primary storage pool [{}] due to: [{}].", pool, e.getMessage()); + logger.debug("Failed to clean up primary storage pool [{}].", pool, e); } } } @@ -2119,7 +1953,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C if (logger.isDebugEnabled()) { snapshot = _snapshotDao.findById(snapshotDataStoreVO.getSnapshotId()); if (snapshot == null) { - logger.warn(String.format("Did not find snapshot [ID: %d] for which store reference is in destroying state; therefore, it cannot be destroyed.", snapshotDataStoreVO.getSnapshotId())); + logger.warn("Did not find snapshot [ID: {}] for which store reference is in destroying state; therefore, it cannot be destroyed.", + snapshotDataStoreVO.getSnapshotId()); continue; } snapshotUuid = snapshot.getUuid(); @@ -2127,22 +1962,21 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C try { if (logger.isDebugEnabled()) { - logger.debug(String.format("Verifying if snapshot [%s] is in destroying state in %s data store ID: %d.", snapshotUuid, storeRole, snapshotDataStoreVO.getDataStoreId())); + logger.debug("Verifying if snapshot [{}] is in destroying state in {} data store ID: {}.", + snapshotUuid, storeRole, snapshotDataStoreVO.getDataStoreId()); } SnapshotInfo snapshotInfo = snapshotFactory.getSnapshot(snapshotDataStoreVO.getSnapshotId(), snapshotDataStoreVO.getDataStoreId(), snapshotDataStoreVO.getRole()); if (snapshotInfo != null) { - if (logger.isDebugEnabled()) { - logger.debug(String.format("Snapshot [%s] in destroying state found in %s data store [%s]; therefore, it will be destroyed.", snapshotUuid, storeRole, snapshotInfo.getDataStore().getUuid())); - } + logger.debug("Snapshot [{}] in destroying state found in %s data store [{}]; therefore, it will be destroyed.", + snapshotUuid, storeRole, snapshotInfo.getDataStore().getUuid()); _snapshotService.deleteSnapshot(snapshotInfo); - } else if (logger.isDebugEnabled()) { - logger.debug(String.format("Did not find snapshot [%s] in destroying state in %s data store ID: %d.", snapshotUuid, storeRole, snapshotDataStoreVO.getDataStoreId())); + } else { + logger.debug("Did not find snapshot [{}] in destroying state in {} data store ID: {}.", + snapshotUuid, storeRole, snapshotDataStoreVO.getDataStoreId()); } } catch (Exception e) { logger.error("Failed to delete snapshot [{}] from storage due to: [{}].", snapshot, e.getMessage()); - if (logger.isDebugEnabled()) { - logger.debug("Failed to delete snapshot [{}] from storage.", snapshot, e); - } + logger.debug("Failed to delete snapshot [{}] from storage.", snapshot, e); } } cleanupSecondaryStorage(recurring); @@ -2158,7 +1992,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } } if (isVolumeSuspectedDestroyDuplicateOfVmVolume(vol)) { - logger.warn(String.format("Skipping cleaning up %s as it could be a duplicate for another volume on same pool", vol)); + logger.warn("Skipping cleaning up {} as it could be a duplicate for another volume on same pool", vol); continue; } try { @@ -2204,7 +2038,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C for (VolumeDataStoreVO volumeDataStore : volumeDataStores) { VolumeVO volume = volumeDao.findById(volumeDataStore.getVolumeId()); if (volume == null) { - logger.warn(String.format("Uploaded volume [%s] not found, so cannot be destroyed.", volumeDataStore.getVolumeId())); + logger.warn("Uploaded volume [{}] not found, so cannot be destroyed.", volumeDataStore.getVolumeId()); continue; } try { @@ -2216,7 +2050,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } Host host = _hostDao.findById(ep.getId()); if (host != null && host.getManagementServerId() != null) { - if (_serverId == host.getManagementServerId().longValue()) { + if (_serverId == host.getManagementServerId()) { volService.destroyVolume(volume.getId()); // decrement volume resource count _resourceLimitMgr.decrementVolumeResourceCount(volume.getAccountId(), volume.isDisplayVolume(), @@ -2244,7 +2078,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C for (TemplateDataStoreVO templateDataStore : templateDataStores) { VMTemplateVO template = _templateDao.findById(templateDataStore.getTemplateId()); if (template == null) { - logger.warn(String.format("Uploaded template [%s] not found, so cannot be destroyed.", templateDataStore.getTemplateId())); + logger.warn("Uploaded template [{}] not found, so cannot be destroyed.", templateDataStore.getTemplateId()); continue; } try { @@ -2256,7 +2090,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } Host host = _hostDao.findById(ep.getId()); if (host != null && host.getManagementServerId() != null) { - if (_serverId == host.getManagementServerId().longValue()) { + if (_serverId == host.getManagementServerId()) { AsyncCallFuture<TemplateApiResult> future = _imageSrv.deleteTemplateAsync(tmplFactory.getTemplate(template.getId(), dataStore)); TemplateApiResult result = future.get(); if (!result.isSuccess()) { @@ -2274,7 +2108,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C _templateStoreDao.removeByTemplateStore(template.getId(), dataStore.getId()); // find all eligible image stores for this template List<DataStore> imageStores = _tmpltMgr.getImageStoreByTemplate(template.getId(), null); - if (imageStores == null || imageStores.size() == 0) { + if (imageStores == null || imageStores.isEmpty()) { template.setState(VirtualMachineTemplate.State.Inactive); _templateDao.update(template.getId(), template); @@ -2316,7 +2150,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C List<VolumeVO> vmUsableVolumes = volumeDao.findUsableVolumesForInstance(vmId); for (VolumeVO vol : vmUsableVolumes) { if (gcVolume.getPoolId().equals(vol.getPoolId()) && gcVolume.getPath().equals(vol.getPath())) { - logger.debug(String.format("%s meant for garbage collection could a possible duplicate for %s", gcVolume, vol)); + logger.debug("{} meant for garbage collection could a possible duplicate for {}", gcVolume, vol); return true; } } @@ -2325,10 +2159,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C /** * This method only applies for managed storage. - * * For XenServer and vSphere, see if we need to remove an SR or a datastore, then remove the underlying volume * from any applicable access control list (before other code attempts to delete the volume that supports it). - * * For KVM, just tell the underlying storage plug-in to remove the volume from any applicable access control list * (before other code attempts to delete the volume that supports it). */ @@ -2379,52 +2211,10 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } } - @DB - List<Long> findAllVolumeIdInSnapshotTable(Long storeId) { - String sql = "SELECT volume_id from snapshots, snapshot_store_ref WHERE snapshots.id = snapshot_store_ref.snapshot_id and store_id=? GROUP BY volume_id"; - List<Long> list = new ArrayList<>(); - try { - TransactionLegacy txn = TransactionLegacy.currentTxn(); - ResultSet rs = null; - PreparedStatement pstmt = null; - pstmt = txn.prepareAutoCloseStatement(sql); - pstmt.setLong(1, storeId); - rs = pstmt.executeQuery(); - while (rs.next()) { - list.add(rs.getLong(1)); - } - return list; - } catch (Exception e) { - logger.debug("failed to get all volumes who has snapshots in secondary storage " + storeId + " due to " + e.getMessage()); - return null; - } - - } - - List<String> findAllSnapshotForVolume(Long volumeId) { - String sql = "SELECT backup_snap_id FROM snapshots WHERE volume_id=? and backup_snap_id is not NULL"; - try { - TransactionLegacy txn = TransactionLegacy.currentTxn(); - ResultSet rs = null; - PreparedStatement pstmt = null; - pstmt = txn.prepareAutoCloseStatement(sql); - pstmt.setLong(1, volumeId); - rs = pstmt.executeQuery(); - List<String> list = new ArrayList<>(); - while (rs.next()) { - list.add(rs.getString(1)); - } - return list; - } catch (Exception e) { - logger.debug("failed to get all snapshots for a volume " + volumeId + " due to " + e.getMessage()); - return null; - } - } - @Override @DB public void cleanupSecondaryStorage(boolean recurring) { - // NOTE that object_store refactor will immediately delete the object from secondary storage when deleteTemplate etc api is issued. + // NOTE that object_store refactor will immediately delete the object from secondary storage when deleteTemplate etc. api is issued. // so here we don't need to issue DeleteCommand to resource anymore, only need to remove db entry. try { // Cleanup templates in template_store_ref @@ -2436,7 +2226,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C logger.debug("Secondary storage garbage collector found {} Templates to cleanup on template_store_ref for store: {}", destroyedTemplateStoreVOs.size(), store); for (TemplateDataStoreVO destroyedTemplateStoreVO : destroyedTemplateStoreVOs) { if (logger.isDebugEnabled()) { - logger.debug("Deleting template store DB entry: " + destroyedTemplateStoreVO); + logger.debug("Deleting template store DB entry: {}", destroyedTemplateStoreVO); } _templateStoreDao.remove(destroyedTemplateStoreVO.getId()); } @@ -2454,13 +2244,11 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C // check if this snapshot has child SnapshotInfo snap = snapshotFactory.getSnapshot(destroyedSnapshotStoreVO.getSnapshotId(), store); if (snap.getChild() != null) { - logger.debug("Skip snapshot on store: " + destroyedSnapshotStoreVO + " , because it has child"); + logger.debug("Skip snapshot on store: {} , because it has child", destroyedSnapshotStoreVO); continue; } - if (logger.isDebugEnabled()) { - logger.debug("Deleting snapshot store DB entry: " + destroyedSnapshotStoreVO); - } + logger.debug("Deleting snapshot store DB entry: {}", destroyedSnapshotStoreVO); List<SnapshotDataStoreVO> imageStoreRefs = _snapshotStoreDao.listBySnapshotAndDataStoreRole(destroyedSnapshotStoreVO.getSnapshotId(), DataStoreRole.Image); if (imageStoreRefs.size() <= 1) { @@ -2468,9 +2256,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } SnapshotDataStoreVO snapshotOnPrimary = _snapshotStoreDao.findDestroyedReferenceBySnapshot(destroyedSnapshotStoreVO.getSnapshotId(), DataStoreRole.Primary); if (snapshotOnPrimary != null) { - if (logger.isDebugEnabled()) { - logger.debug("Deleting snapshot on primary store reference DB entry: " + snapshotOnPrimary); - } + logger.debug("Deleting snapshot on primary store reference DB entry: {}", snapshotOnPrimary); _snapshotStoreDao.remove(snapshotOnPrimary.getId()); } _snapshotStoreDao.remove(destroyedSnapshotStoreVO.getId()); @@ -2489,9 +2275,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C destroyedStoreVOs.addAll(_volumeDataStoreDao.listByVolumeState(Volume.State.Expunged)); logger.debug("Secondary storage garbage collector found {} volumes to cleanup on volume_store_ref for store: {}", destroyedStoreVOs.size(), store); for (VolumeDataStoreVO destroyedStoreVO : destroyedStoreVOs) { - if (logger.isDebugEnabled()) { - logger.debug("Deleting volume store DB entry: " + destroyedStoreVO); - } + logger.debug("Deleting volume store DB entry: {}", destroyedStoreVO); _volumeStoreDao.remove(destroyedStoreVO.getId()); } @@ -2504,24 +2288,12 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } } - @Override - public String getPrimaryStorageNameLabel(VolumeVO volume) { - Long poolId = volume.getPoolId(); - - // poolId is null only if volume is destroyed, which has been checked - // before. - assert poolId != null; - StoragePoolVO primaryDataStoreVO = _storagePoolDao.findById(poolId); - assert primaryDataStoreVO != null; - return primaryDataStoreVO.getUuid(); - } - @Override @DB @ActionEvent(eventType = EventTypes.EVENT_MAINTENANCE_PREPARE_PRIMARY_STORAGE, eventDescription = "preparing storage pool for maintenance", async = true) - public PrimaryDataStoreInfo preparePrimaryStorageForMaintenance(Long primaryStorageId) throws ResourceUnavailableException, InsufficientCapacityException { - StoragePoolVO primaryStorage = null; + public PrimaryDataStoreInfo preparePrimaryStorageForMaintenance(Long primaryStorageId) { + StoragePoolVO primaryStorage; primaryStorage = _storagePoolDao.findById(primaryStorageId); if (primaryStorage == null) { @@ -2566,14 +2338,12 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } } }); - for (Iterator<StoragePoolVO> iteratorChildDatastore = childDatastores.listIterator(); iteratorChildDatastore.hasNext(); ) { - DataStore childStore = _dataStoreMgr.getDataStore(iteratorChildDatastore.next().getId(), DataStoreRole.Primary); + for (StoragePoolVO datastore : childDatastores) { + DataStore childStore = _dataStoreMgr.getDataStore(datastore.getId(), DataStoreRole.Primary); try { lifeCycle.maintain(childStore); } catch (Exception e) { - if (logger.isDebugEnabled()) { - logger.debug("Exception on maintenance preparation of one of the child datastores in datastore cluster {} with error {}", datastoreCluster, e); - } + logger.debug("Exception on maintenance preparation of one of the child datastores in datastore cluster {}", datastoreCluster, e); // Set to ErrorInMaintenance state of all child storage pools and datastore cluster for (StoragePoolVO childDatastore : childDatastores) { childDatastore.setStatus(StoragePoolStatus.ErrorInMaintenance); @@ -2592,7 +2362,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C eventDescription = "canceling maintenance for primary storage pool", async = true) public PrimaryDataStoreInfo cancelPrimaryStorageForMaintenance(CancelPrimaryStorageMaintenanceCmd cmd) throws ResourceUnavailableException { Long primaryStorageId = cmd.getId(); - StoragePoolVO primaryStorage = null; + StoragePoolVO primaryStorage; primaryStorage = _storagePoolDao.findById(primaryStorageId); @@ -2649,7 +2419,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C List<Long> poolIds = new ArrayList<>(); poolIds.add(poolId); List<Long> hosts = _storagePoolHostDao.findHostsConnectedToPools(poolIds); - if (hosts.size() > 0) { + if (!hosts.isEmpty()) { Long hostId = hosts.get(0); ModifyStoragePoolCommand modifyStoragePoolCommand = new ModifyStoragePoolCommand(true, pool); final Answer answer = _agentMgr.easySend(hostId, modifyStoragePoolCommand); @@ -2766,9 +2536,9 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } if (dataStoreVO != null) { if (dataStoreVO.getParent() != datastoreClusterPoolId) { - logger.debug(String.format("Storage pool %s with uuid %s is found to be under datastore cluster %s at vCenter, " + + logger.debug("Storage pool {} with uuid {} is found to be under datastore cluster {} at vCenter, " + "so moving the storage pool to be a child storage pool under the datastore cluster in CloudStack management server", - childStoragePoolInfo.getName(), childStoragePoolInfo.getUuid(), datastoreClusterPool.getName())); + childStoragePoolInfo.getName(), childStoragePoolInfo.getUuid(), datastoreClusterPool.getName()); dataStoreVO.setParent(datastoreClusterPoolId); _storagePoolDao.update(dataStoreVO.getId(), dataStoreVO); if (CollectionUtils.isNotEmpty(storageTags)) { @@ -2780,9 +2550,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C Set<StoragePoolTagVO> set = new LinkedHashSet<>(storageTags); storageTags.clear(); storageTags.addAll(set); - if (logger.isDebugEnabled()) { - logger.debug("Updating Storage Pool Tags to :" + storageTags); - } + logger.debug("Updating Storage Pool Tags to : {}", storageTags); _storagePoolTagsDao.persist(storageTags); } } else { @@ -2887,11 +2655,11 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } if (ArrayUtils.isNotEmpty(hostStorageAccessGroups)) { - logger.debug(String.format("Storage access groups on the host %s are %s", host, hostStorageAccessGroups)); + logger.debug("Storage access groups on the host {} are {}", host, hostStorageAccessGroups); } if (CollectionUtils.isNotEmpty(storagePoolAccessGroups)) { - logger.debug(String.format("Storage access groups on the storage pool %s are %s", host, storagePoolAccessGroups)); + logger.debug("Storage access groups on the storage pool {} are {}", host, storagePoolAccessGroups); } List<String> hostTagList = Arrays.asList(hostStorageAccessGroups); @@ -2914,8 +2682,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C List<String> destStorageAccessGroups = _storagePoolAccessGroupMapDao.getStorageAccessGroups(destPool.getId()); if (CollectionUtils.isNotEmpty(srcStorageAccessGroups) && CollectionUtils.isNotEmpty(destStorageAccessGroups)) { - logger.debug(String.format("Storage access groups on source storage %s are %s and destination storage %s are %s", - srcPool, srcStorageAccessGroups, destPool, destStorageAccessGroups)); + logger.debug("Storage access groups on source storage {} are {} and destination storage {} are {}", + srcPool, srcStorageAccessGroups, destPool, destStorageAccessGroups); List<String> intersection = new ArrayList<>(srcStorageAccessGroups); intersection.retainAll(destStorageAccessGroups); if (CollectionUtils.isNotEmpty(intersection)) { @@ -2941,16 +2709,14 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C List<String> srcStorageAccessGroups = _storagePoolAccessGroupMapDao.getStorageAccessGroups(srcPoolId); List<String> destStorageAccessGroups = _storagePoolAccessGroupMapDao.getStorageAccessGroups(destPool.getId()); - logger.debug(String.format("Storage access groups on source storage %s are %s and destination storage %s are %s", - srcPool, srcStorageAccessGroups, destPool, destStorageAccessGroups)); + logger.debug("Storage access groups on source storage {} are {} and destination storage {} are {}", + srcPool, srcStorageAccessGroups, destPool, destStorageAccessGroups); if (CollectionUtils.isEmpty(srcStorageAccessGroups) && CollectionUtils.isEmpty(destStorageAccessGroups)) { return new Pair<>(true, "Success"); } if (CollectionUtils.isNotEmpty(srcStorageAccessGroups) && CollectionUtils.isNotEmpty(destStorageAccessGroups)) { - List<String> intersection = new ArrayList<>(srcStorageAccessGroups); - intersection.retainAll(destStorageAccessGroups); if (ArrayUtils.isNotEmpty(hostStorageAccessGroups)) { boolean hasSrcCommon = srcStorageAccessGroups.stream() @@ -3129,7 +2895,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C if (vo.getMsid() == _serverId) { logger.info("Cleaning up storage maintenance jobs associated with Management server: {}", vo); List<Long> poolIds = _storagePoolWorkDao.searchForPoolIdsForPendingWorkJobs(vo.getMsid()); - if (poolIds.size() > 0) { + if (!poolIds.isEmpty()) { for (Long poolId : poolIds) { StoragePoolVO pool = _storagePoolDao.findById(poolId); // check if pool is in an inconsistent state @@ -3326,17 +3092,6 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C return storeDriver instanceof PrimaryDataStoreDriver && ((PrimaryDataStoreDriver)storeDriver).canHostPrepareStoragePoolAccess(host, pool); } - @Override - public boolean canDisconnectHostFromStoragePool(Host host, StoragePool pool) { - if (pool == null || !pool.isManaged()) { - return true; - } - - DataStoreProvider storeProvider = _dataStoreProviderMgr.getDataStoreProvider(pool.getStorageProviderName()); - DataStoreDriver storeDriver = storeProvider.getDataStoreDriver(); - return storeDriver instanceof PrimaryDataStoreDriver && ((PrimaryDataStoreDriver)storeDriver).canDisconnectHostFromStoragePool(host, pool); - } - @Override @DB public Host getHost(long hostId) { @@ -3344,7 +3099,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } @Override - public Host updateSecondaryStorage(long secStorageId, String newUrl) { + public void updateSecondaryStorage(long secStorageId, String newUrl) { HostVO secHost = _hostDao.findById(secStorageId); if (secHost == null) { throw new InvalidParameterValueException("Can not find out the secondary storage id: " + secStorageId); @@ -3384,14 +3139,13 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C throw new InvalidParameterValueException("can not change old scheme:" + oldUri.getScheme() + " to " + uri.getScheme()); } } catch (URISyntaxException e) { - logger.debug("Failed to get uri from " + oldUrl); + logger.debug("Failed to get uri from {}", oldUrl); } secHost.setStorageUrl(newUrl); secHost.setGuid(newUrl); secHost.setName(newUrl); _hostDao.update(secHost.getId(), secHost); - return secHost; } @Override @@ -3477,14 +3231,15 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C long futureIops = currentIops + requestedIops; boolean hasEnoughIops = futureIops <= pool.getCapacityIops(); String hasCapacity = hasEnoughIops ? "has" : "does not have"; - logger.debug(String.format("Pool [%s] %s enough IOPS to allocate volumes [%s].", pool, hasCapacity, requestedVolumes)); + logger.debug("Pool [{}] {} enough IOPS to allocate volumes [{}].", pool, hasCapacity, requestedVolumes); return hasEnoughIops; } @Override public boolean storagePoolHasEnoughIops(List<Pair<Volume, DiskProfile>> requestedVolumes, StoragePool pool) { if (requestedVolumes == null || requestedVolumes.isEmpty() || pool == null) { - logger.debug(String.format("Cannot check if storage [%s] has enough IOPS to allocate volumes [%s].", pool, requestedVolumes)); + logger.debug("Cannot check if storage [{}] has enough IOPS to allocate volumes [{}].", + pool, requestedVolumes); return false; } if (checkIfPoolIopsCapacityNull(pool)) { @@ -3535,12 +3290,12 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Override public boolean storagePoolHasEnoughSpace(List<Pair<Volume, DiskProfile>> volumeDiskProfilesList, StoragePool pool, Long clusterId) { if (CollectionUtils.isEmpty(volumeDiskProfilesList)) { - logger.debug(String.format("Cannot check if pool [%s] has enough space to allocate volumes because the volumes list is empty.", pool)); + logger.debug("Cannot check if pool [{}] has enough space to allocate volumes because the volumes list is empty.", pool); return false; } if (!checkUsagedSpace(pool)) { - logger.debug(String.format("Cannot allocate pool [%s] because there is not enough space in this pool.", pool)); + logger.debug("Cannot allocate pool [{}] because there is not enough space in this pool.", pool); return false; } @@ -3569,7 +3324,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C volumeVO = volumeDao.findById(volume.getId()); } - // this if statement should resolve to true at most once per execution of the for loop its contained within (for a root disk that is + // this if statement should resolve to true at most once per execution of the for loop it's contained within (for a root disk that is // to leverage a template) if (volume.getTemplateId() != null) { VMTemplateVO tmpl = _templateDao.findByIdIncludingRemoved(volume.getTemplateId()); @@ -3631,10 +3386,10 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C long targetHostId = _hvGuruMgr.getGuruProcessedCommandTargetHost(hostIds.get(0), cmd); return _agentMgr.send(targetHostId, cmd); } catch (AgentUnavailableException e) { - logger.debug("Unable to send storage pool command to " + pool + " via " + hostIds.get(0), e); + logger.debug("Unable to send storage pool command to {} via {}", pool, hostIds.get(0), e); throw new StorageUnavailableException("Unable to send command to the pool ", pool.getId()); } catch (OperationTimedoutException e) { - logger.debug("Failed to process storage pool command to " + pool + " via " + hostIds.get(0), e); + logger.debug("Failed to process storage pool command to {} via {}", pool, hostIds.get(0), e); throw new StorageUnavailableException("Failed to process storage command to the pool ", pool.getId()); } } @@ -3718,14 +3473,14 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C return false; } if (!AllowVolumeReSizeBeyondAllocation.valueIn(pool.getId())) { - logger.debug(String.format("Skipping the pool %s as %s is false", pool, AllowVolumeReSizeBeyondAllocation.key())); + logger.debug("Skipping the pool {} as {} is false", pool, AllowVolumeReSizeBeyondAllocation.key()); return false; } double storageAllocatedThresholdForResize = CapacityManager.StorageAllocatedCapacityDisableThresholdForVolumeSize.valueIn(pool.getId()); if (usedPercentage > storageAllocatedThresholdForResize) { - logger.debug(String.format("Skipping the pool %s since its allocated percentage: %s has crossed the allocated %s: %s", - pool, usedPercentage, CapacityManager.StorageAllocatedCapacityDisableThresholdForVolumeSize.key(), storageAllocatedThresholdForResize)); + logger.debug("Skipping the pool {} since its allocated percentage: {} has crossed the allocated {}: {}", + pool, usedPercentage, CapacityManager.StorageAllocatedCapacityDisableThresholdForVolumeSize.key(), storageAllocatedThresholdForResize); return false; } } @@ -3747,7 +3502,6 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C /** * Storage plug-ins for managed storage can be designed in such a way as to store a template on the primary storage once and * make use of it via storage-side cloning. - * * This method determines how many more bytes it will need for the template (if the template is already stored on the primary storage, * then the answer is 0). */ @@ -3829,34 +3583,37 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Override public boolean storagePoolCompatibleWithVolumePool(StoragePool pool, Volume volume) { if (pool == null || volume == null) { - logger.debug(String.format("Cannot check if storage pool [%s] is compatible with volume [%s].", pool, volume)); + logger.debug("Cannot check if storage pool [{}] is compatible with volume [{}].", pool, volume); return false; } if (volume.getPoolId() == null) { - logger.debug(String.format("Volume [%s] is not allocated to any pool. Cannot check compatibility with pool [%s].", volume, pool)); + logger.debug("Volume [{}] is not allocated to any pool. Cannot check compatibility with pool [{}].", + volume, pool); return true; } StoragePool volumePool = _storagePoolDao.findById(volume.getPoolId()); if (volumePool == null) { - logger.debug(String.format("Pool [%s] used by volume [%s] does not exist. Cannot check compatibility.", pool, volume)); + logger.debug("Pool [{}] used by volume [{}] does not exist. Cannot check compatibility.", pool, volume); return true; } if (volume.getState() == Volume.State.Ready) { if (volumePool.getPoolType() == Storage.StoragePoolType.PowerFlex && pool.getPoolType() != Storage.StoragePoolType.PowerFlex) { - logger.debug(String.format("Pool [%s] with type [%s] does not match volume [%s] pool type [%s].", pool, pool.getPoolType(), volume, volumePool.getPoolType())); + logger.debug("Pool [{}] with type [{}] does not match volume [{}] pool type [{}].", + pool, pool.getPoolType(), volume, volumePool.getPoolType()); return false; } else if (volumePool.getPoolType() != Storage.StoragePoolType.PowerFlex && pool.getPoolType() == Storage.StoragePoolType.PowerFlex) { - logger.debug(String.format("Pool [%s] with type [%s] does not match volume [%s] pool type [%s].", pool, pool.getPoolType(), volume, volumePool.getPoolType())); + logger.debug("Pool [{}] with type [{}] does not match volume [{}] pool type [{}].", + pool, pool.getPoolType(), volume, volumePool.getPoolType()); return false; } } else { - logger.debug(String.format("Cannot check compatibility of pool [%s] because volume [%s] is not in [%s] state.", pool, volume, Volume.State.Ready)); + logger.debug("Cannot check compatibility of pool [{}] because volume [{}] is not in [{}] state.", pool, volume, Volume.State.Ready); return false; } - logger.debug(String.format("Pool [%s] is compatible with volume [%s].", pool, volume)); + logger.debug("Pool [{}] is compatible with volume [{}].", pool, volume); return true; } @@ -3903,7 +3660,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C return templateName; } @Override - public ImageStore discoverImageStore(String name, String url, String providerName, Long zoneId, Map details) throws IllegalArgumentException, DiscoveryException, InvalidParameterValueException { + public ImageStore discoverImageStore(String name, String url, String providerName, Long zoneId, Map<String, String> details) throws IllegalArgumentException, InvalidParameterValueException { DataStoreProvider storeProvider = _dataStoreProviderMgr.getDataStoreProvider(providerName); if (storeProvider == null) { @@ -3974,7 +3731,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C store = lifeCycle.initialize(params); } catch (Exception e) { if (logger.isDebugEnabled()) { - logger.debug("Failed to add data store: " + e.getMessage(), e); + logger.debug("Failed to add data store: {}", e.getMessage(), e); } throw new CloudRuntimeException("Failed to add data store: " + e.getMessage(), e); } @@ -4043,14 +3800,10 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C if (CollectionUtils.isEmpty(stores)) { List<Pair<HypervisorType, CPU.CPUArch>> hypervisorTypes = _clusterDao.listDistinctHypervisorsAndArchExcludingExternalType(zoneId); - TransactionLegacy txn = TransactionLegacy.open("AutomaticTemplateRegister"); - SystemVmTemplateRegistration systemVmTemplateRegistration = new SystemVmTemplateRegistration(); String filePath = null; - try { + try (TransactionLegacy txn = TransactionLegacy.open("AutomaticTemplateRegister")) { + SystemVmTemplateRegistration systemVmTemplateRegistration = new SystemVmTemplateRegistration(); filePath = Files.createTempDirectory(SystemVmTemplateRegistration.TEMPORARY_SECONDARY_STORE).toString(); - if (filePath == null) { - throw new CloudRuntimeException("Failed to create temporary file path to mount the store"); - } Pair<String, Long> storeUrlAndId = new Pair<>(url, store.getId()); String nfsVersion = imageStoreDetailsUtil.getNfsVersion(store.getId()); for (Pair<HypervisorType, CPU.CPUArch> hypervisorArchType : hypervisorTypes) { @@ -4068,7 +3821,6 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C logger.error("Failed to register systemVM template(s) due to: ", e); } finally { SystemVmTemplateRegistration.unmountStore(filePath); - txn.close(); } } } @@ -4080,7 +3832,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C // check if current cloud is ready to migrate, we only support cloud with only NFS secondary storages List<ImageStoreVO> imgStores = _imageStoreDao.listImageStores(); List<ImageStoreVO> nfsStores = new ArrayList<>(); - if (imgStores != null && imgStores.size() > 0) { + if (CollectionUtils.isNotEmpty(imgStores)) { for (ImageStoreVO store : imgStores) { if (!store.getProviderName().equals(DataStoreProvider.NFS_IMAGE)) { throw new InvalidParameterValueException("We only support migrate NFS secondary storage to use object store!"); @@ -4090,7 +3842,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } } // convert all NFS secondary storage to staging store - if (nfsStores != null && nfsStores.size() > 0) { + if (CollectionUtils.isNotEmpty(nfsStores)) { for (ImageStoreVO store : nfsStores) { long storeId = store.getId(); @@ -4188,11 +3940,17 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C List<Long> poolIds = new ArrayList<>(); poolIds.add(pool.getId()); List<Long> hosts = _storagePoolHostDao.findHostsConnectedToPools(poolIds); - if (hosts.size() > 0) { + if (!hosts.isEmpty()) { GetStoragePoolCapabilitiesCommand cmd = new GetStoragePoolCapabilitiesCommand(); cmd.setPool(new StorageFilerTO(pool)); GetStoragePoolCapabilitiesAnswer answer = (GetStoragePoolCapabilitiesAnswer) _agentMgr.easySend(hosts.get(0), cmd); - if (answer.getPoolDetails() != null && answer.getPoolDetails().containsKey(Storage.Capability.HARDWARE_ACCELERATION.toString())) { + if (answer == null) { + String msg = "Unable to get storage capabilities from storage pool: " + pool.getName(); + logger.error(msg); + if (failOnChecks) { + throw new CloudRuntimeException(msg); + } + } else if (answer.getPoolDetails() != null && answer.getPoolDetails().containsKey(Storage.Capability.HARDWARE_ACCELERATION.toString())) { StoragePoolDetailVO hardwareAccelerationSupported = _storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); if (hardwareAccelerationSupported == null) { StoragePoolDetailVO storagePoolDetailVO = new StoragePoolDetailVO(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString(), answer.getPoolDetails().get(Storage.Capability.HARDWARE_ACCELERATION.toString()), false); @@ -4201,12 +3959,10 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C hardwareAccelerationSupported.setValue(answer.getPoolDetails().get(Storage.Capability.HARDWARE_ACCELERATION.toString())); _storagePoolDetailsDao.update(hardwareAccelerationSupported.getId(), hardwareAccelerationSupported); } - } else { - if (answer != null && !answer.getResult()) { - logger.error("Failed to update storage pool capabilities: " + answer.getDetails()); - if (failOnChecks) { - throw new CloudRuntimeException(answer.getDetails()); - } + } else if (!answer.getResult()) { + logger.error("Failed to update storage pool capabilities: {}", answer.getDetails()); + if (failOnChecks) { + throw new CloudRuntimeException(answer.getDetails()); } } } @@ -4260,17 +4016,17 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C // Verify that there are no live snapshot, template, volume on the image // store to be deleted List<SnapshotDataStoreVO> snapshots = _snapshotStoreDao.listByStoreId(storeId, DataStoreRole.Image); - if (snapshots != null && snapshots.size() > 0) { + if (CollectionUtils.isNotEmpty(snapshots)) { throw new InvalidParameterValueException("Cannot delete image store with active snapshots backup!"); } List<VolumeDataStoreVO> volumes = _volumeStoreDao.listByStoreId(storeId); - if (volumes != null && volumes.size() > 0) { + if (CollectionUtils.isNotEmpty(volumes)) { throw new InvalidParameterValueException("Cannot delete image store with active volumes backup!"); } // search if there are user templates stored on this image store, excluding system, builtin templates List<TemplateJoinVO> templates = _templateViewDao.listActiveTemplates(storeId); - if (templates != null && templates.size() > 0) { + if (CollectionUtils.isNotEmpty(templates)) { throw new InvalidParameterValueException("Cannot delete image store with active Templates backup!"); } @@ -4349,11 +4105,11 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C params.put("role", DataStoreRole.ImageCache); DataStoreLifeCycle lifeCycle = storeProvider.getDataStoreLifeCycle(); - DataStore store = null; + DataStore store; try { store = lifeCycle.initialize(params); } catch (Exception e) { - logger.debug("Failed to add data store: " + e.getMessage(), e); + logger.debug("Failed to add data store: {}", e.getMessage(), e); throw new CloudRuntimeException("Failed to add data store: " + e.getMessage(), e); } @@ -4373,16 +4129,16 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C // Verify that there are no live snapshot, template, volume on the cache // store that is currently referenced List<SnapshotDataStoreVO> snapshots = _snapshotStoreDao.listActiveOnCache(storeId); - if (snapshots != null && snapshots.size() > 0) { + if (CollectionUtils.isNotEmpty(snapshots)) { throw new InvalidParameterValueException("Cannot delete cache store with staging snapshots currently in use!"); } List<VolumeDataStoreVO> volumes = _volumeStoreDao.listActiveOnCache(storeId); - if (volumes != null && volumes.size() > 0) { + if (CollectionUtils.isNotEmpty(volumes)) { throw new InvalidParameterValueException("Cannot delete cache store with staging Volumes currently in use!"); } List<TemplateDataStoreVO> templates = _templateStoreDao.listActiveOnCache(storeId); - if (templates != null && templates.size() > 0) { + if (CollectionUtils.isNotEmpty(templates)) { throw new InvalidParameterValueException("Cannot delete cache store with staging Templates currently in use!"); } @@ -4519,7 +4275,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C } else if ((diskOffering != null) && (diskOffering.getBytesReadRate() != null) && (diskOffering.getBytesReadRate() > 0)) { return diskOffering.getBytesReadRate(); } else { - Long bytesReadRate = Long.parseLong(_configDao.getValue(Config.VmDiskThrottlingBytesReadRate.key())); + long bytesReadRate = VmDiskThrottlingBytesReadRate.value(); if ((bytesReadRate > 0) && ((offering == null) || (!offering.isSystemUse()))) { return bytesReadRate; } @@ -4533,7 +4289,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C if ((diskOffering != null) && (diskOffering.getBytesWriteRate() != null) && (diskOffering.getBytesWriteRate() > 0)) { return diskOffering.getBytesWriteRate(); } else { - Long bytesWriteRate = Long.parseLong(_configDao.getValue(Config.VmDiskThrottlingBytesWriteRate.key())); + long bytesWriteRate = VmDiskThrottlingBytesWriteRate.value(); if ((bytesWriteRate > 0) && ((offering == null) || (!offering.isSystemUse()))) { return bytesWriteRate; } @@ -4547,7 +4303,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C if ((diskOffering != null) && (diskOffering.getIopsReadRate() != null) && (diskOffering.getIopsReadRate() > 0)) { return diskOffering.getIopsReadRate(); } else { - Long iopsReadRate = Long.parseLong(_configDao.getValue(Config.VmDiskThrottlingIopsReadRate.key())); + long iopsReadRate = VmDiskThrottlingIopsReadRate.value(); if ((iopsReadRate > 0) && ((offering == null) || (!offering.isSystemUse()))) { return iopsReadRate; } @@ -4561,7 +4317,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C if ((diskOffering != null) && (diskOffering.getIopsWriteRate() != null) && (diskOffering.getIopsWriteRate() > 0)) { return diskOffering.getIopsWriteRate(); } else { - Long iopsWriteRate = Long.parseLong(_configDao.getValue(Config.VmDiskThrottlingIopsWriteRate.key())); + long iopsWriteRate = VmDiskThrottlingIopsWriteRate.value(); if ((iopsWriteRate > 0) && ((offering == null) || (!offering.isSystemUse()))) { return iopsWriteRate; } @@ -4603,7 +4359,11 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C AllowVolumeReSizeBeyondAllocation, StoragePoolHostConnectWorkers, ObjectStorageCapacityThreshold, - COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES + COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES, + VmDiskThrottlingBytesReadRate, + VmDiskThrottlingBytesWriteRate, + VmDiskThrottlingIopsReadRate, + VmDiskThrottlingIopsWriteRate }; } @@ -4617,8 +4377,8 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Override public DiskTO getDiskWithThrottling(final DataTO volTO, final Volume.Type volumeType, final long deviceId, final String path, final long offeringId, final long diskOfferingId) { - DiskTO disk = null; - if (volTO != null && volTO instanceof VolumeObjectTO) { + DiskTO disk; + if (volTO instanceof VolumeObjectTO) { VolumeObjectTO volumeTO = (VolumeObjectTO)volTO; ServiceOffering offering = _entityMgr.findById(ServiceOffering.class, offeringId); DiskOffering diskOffering = _entityMgr.findById(DiskOffering.class, diskOfferingId); @@ -4637,10 +4397,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Override public boolean isStoragePoolDatastoreClusterParent(StoragePool pool) { List<StoragePoolVO> childStoragePools = _storagePoolDao.listChildStoragePoolsInDatastoreCluster(pool.getId()); - if (childStoragePools != null && !childStoragePools.isEmpty()) { - return true; - } - return false; + return CollectionUtils.isNotEmpty(childStoragePools); } private void setVolumeObjectTOThrottling(VolumeObjectTO volumeTO, final ServiceOffering offering, final DiskOffering diskOffering) { @@ -4652,7 +4409,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Override @ActionEvent(eventType = EventTypes.EVENT_OBJECT_STORE_CREATE, eventDescription = "creating object storage") - public ObjectStore discoverObjectStore(String name, String url, Long size, String providerName, Map details) + public ObjectStore discoverObjectStore(String name, String url, Long size, String providerName, Map<String, String> details) throws IllegalArgumentException, InvalidParameterValueException { DataStoreProvider storeProvider = _dataStoreProviderMgr.getDataStoreProvider(providerName); @@ -4682,11 +4439,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C Map<String, Object> params = new HashMap<>(); params.put("url", url); params.put("name", name); - if (size == null) { - params.put("size", 0L); - } else { - params.put("size", size); - } + params.put("size", size == null ? 0L : size); params.put("providerName", storeProvider.getName()); params.put("role", DataStoreRole.Object); params.put("details", details); @@ -4698,9 +4451,9 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C store = lifeCycle.initialize(params); } catch (Exception e) { if (logger.isDebugEnabled()) { - logger.debug("Failed to add object store: " + e.getMessage(), e); + logger.debug("Failed to add object store: {}", e.getMessage(), e); } - throw new CloudRuntimeException("Failed to add object store: " + e.getMessage(), e); + throw new CloudRuntimeException("Failed to add object store: {}" + e.getMessage(), e); } return (ObjectStore)_dataStoreMgr.getDataStore(store.getId(), DataStoreRole.Object); @@ -4718,7 +4471,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C // Verify that there are no buckets in the store List<BucketVO> buckets = _bucketDao.listByObjectStoreId(storeId); - if(buckets != null && buckets.size() > 0) { + if(CollectionUtils.isNotEmpty(buckets)) { throw new InvalidParameterValueException("Cannot delete object store with buckets"); } @@ -4791,7 +4544,6 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C total += objectStore.getTotalSize(); } } - CapacityVO capacity = new CapacityVO(null, zoneId, null, null, allocated, total, Capacity.CAPACITY_TYPE_OBJECT_STORAGE); - return capacity; + return new CapacityVO(null, zoneId, null, null, allocated, total, Capacity.CAPACITY_TYPE_OBJECT_STORAGE); } }
