GutoVeronezi commented on code in PR #6413:
URL: https://github.com/apache/cloudstack/pull/6413#discussion_r902962633
##########
engine/components-api/src/main/java/com/cloud/storage/StorageManager.java:
##########
@@ -163,6 +163,47 @@ public interface StorageManager extends StorageService {
true,
ConfigKey.Scope.Cluster,
null);
+ ConfigKey<Boolean> VmwareCreateCloneFull = new ConfigKey<>(Boolean.class,
+ "vmware.create.full.clone",
+ "Storage",
+ "false",
+ "If set to true, creates VMs as full clones on ESX hypervisor",
+ true,
+ ConfigKey.Scope.StoragePool,
+ null);
+ ConfigKey<Boolean> VmwareAllowParallelExecution = new
ConfigKey<>(Boolean.class,
+ "vmware.allow.parallel.command.execution",
+ "Advanced",
+ "false",
+ "allow commands to be executed in parallel in spite of
'vmware.create.full.clone' being set to true.",
+ true,
+ ConfigKey.Scope.Global,
+ null);
+
+ /**
+ * should we execute in sequence not involving any storages?
+ * @return tru if commands should execute in sequence
+ */
+ static boolean shouldExecuteInSequenceOnVmware() {
+ return shouldExecuteInSequenceOnVmware(null, null);
+ }
+
+ static boolean shouldExecuteInSequenceOnVmware(Long srcStoreId, Long
dstStoreId) {
+ final Boolean fullClone = getFullCloneConfiguration(srcStoreId) ||
getFullCloneConfiguration(dstStoreId);
+ final Boolean allowParallel = getAllowParallelExecutionConfiguration();
+ return fullClone && !allowParallel;
+ }
+
+ static Boolean getAllowParallelExecutionConfiguration() {
+ return VmwareAllowParallelExecution.value();
+ }
+
+ static Boolean getFullCloneConfiguration(Long storeId) {
+ if (null == storeId){
+ return VmwareCreateCloneFull.value();
+ }
+ return VmwareCreateCloneFull.valueIn(storeId);
+ }
Review Comment:
`ConfigKey#valueIn` already handles `null` values:
```java
public T valueIn(Long id) {
if (id == null) {
return value();
}
String value = s_depot != null ?
s_depot.findScopedConfigStorage(this).getConfigValue(id, this) : null;
if (value == null) {
return value();
} else {
return valueOf(value);
}
}
```
Therefore, handling it is not necessary.
##########
engine/components-api/src/main/java/com/cloud/capacity/CapacityManager.java:
##########
@@ -40,57 +40,86 @@ public interface CapacityManager {
static final String StorageCapacityDisableThresholdCK =
"pool.storage.capacity.disablethreshold";
static final String StorageOverprovisioningFactorCK =
"storage.overprovisioning.factor";
static final String StorageAllocatedCapacityDisableThresholdCK =
"pool.storage.allocated.capacity.disablethreshold";
- static final String VmwareCreateCloneFullCK = "vmware.create.full.clone";
-
- static final ConfigKey<Float> CpuOverprovisioningFactor = new
ConfigKey<Float>(Float.class, CpuOverprovisioningFactorCK, "Advanced", "1.0",
- "Used for CPU overprovisioning calculation; available CPU will be
(actualCpuCapacity * cpu.overprovisioning.factor)", true,
ConfigKey.Scope.Cluster, null);
- static final ConfigKey<Float> MemOverprovisioningFactor = new
ConfigKey<Float>(Float.class, MemOverprovisioningFactorCK, "Advanced", "1.0",
- "Used for memory overprovisioning calculation", true,
ConfigKey.Scope.Cluster, null);
- static final ConfigKey<Double> StorageCapacityDisableThreshold = new
ConfigKey<Double>("Alert", Double.class, StorageCapacityDisableThresholdCK,
"0.85",
- "Percentage (as a value between 0 and 1) of storage utilization above
which allocators will disable using the pool for low storage available.", true,
- ConfigKey.Scope.Zone);
- static final ConfigKey<Double> StorageOverprovisioningFactor = new
ConfigKey<Double>("Storage", Double.class, StorageOverprovisioningFactorCK, "2",
- "Used for storage overprovisioning calculation; available storage will
be (actualStorageSize * storage.overprovisioning.factor)", true,
ConfigKey.Scope.StoragePool);
+
+ static final String CATEGORY_ADVANCED = "Advanced";
+ static final String CATEGORY_ALERT = "Alert";
+
+ static final ConfigKey<Float> CpuOverprovisioningFactor =
+ new ConfigKey<>(
+ Float.class,
+ CpuOverprovisioningFactorCK,
+ CATEGORY_ADVANCED,
+ "1.0",
+ "Used for CPU overprovisioning calculation; available CPU
will be (actualCpuCapacity * cpu.overprovisioning.factor)",
+ true,
+ ConfigKey.Scope.Cluster,
+ null);
Review Comment:
Is it necessary to separate the parameters per line?
##########
plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java:
##########
@@ -270,10 +262,15 @@ protected VMwareGuru() {
}
}
+ if (s_logger.isTraceEnabled()) {
+ s_logger.trace(String.format("Command of type %s is going to be
executed in sequence? %b", cmd.getClass(), cmd.executeInSequence()));
+ s_logger.trace(String.format("Command of type %s is going to need
delegation? %b", cmd.getClass(), needDelegation));
Review Comment:
We could join these logs.
##########
engine/components-api/src/main/java/com/cloud/storage/StorageManager.java:
##########
@@ -163,6 +163,47 @@ public interface StorageManager extends StorageService {
true,
ConfigKey.Scope.Cluster,
null);
+ ConfigKey<Boolean> VmwareCreateCloneFull = new ConfigKey<>(Boolean.class,
+ "vmware.create.full.clone",
+ "Storage",
+ "false",
+ "If set to true, creates VMs as full clones on ESX hypervisor",
+ true,
+ ConfigKey.Scope.StoragePool,
+ null);
+ ConfigKey<Boolean> VmwareAllowParallelExecution = new
ConfigKey<>(Boolean.class,
+ "vmware.allow.parallel.command.execution",
+ "Advanced",
+ "false",
+ "allow commands to be executed in parallel in spite of
'vmware.create.full.clone' being set to true.",
+ true,
+ ConfigKey.Scope.Global,
+ null);
+
+ /**
+ * should we execute in sequence not involving any storages?
+ * @return tru if commands should execute in sequence
+ */
+ static boolean shouldExecuteInSequenceOnVmware() {
+ return shouldExecuteInSequenceOnVmware(null, null);
+ }
+
+ static boolean shouldExecuteInSequenceOnVmware(Long srcStoreId, Long
dstStoreId) {
+ final Boolean fullClone = getFullCloneConfiguration(srcStoreId) ||
getFullCloneConfiguration(dstStoreId);
+ final Boolean allowParallel = getAllowParallelExecutionConfiguration();
+ return fullClone && !allowParallel;
+ }
+
+ static Boolean getAllowParallelExecutionConfiguration() {
+ return VmwareAllowParallelExecution.value();
+ }
Review Comment:
Why this method is necessary?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
