You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/02/02 10:35:21 UTC
[GitHub] [cloudstack] Spaceman1984 opened a new pull request #4640: Added disk provisioning type support for VMWare
Spaceman1984 opened a new pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640
### Description
This PR adds the ability to create thick and sparse disks using VMWare
A global variable (disk.provisioning.type.strictness) has been added to select if disk provisioning types will be strictly adhered to, meaning if a storage pool is not capable of creating the specific disk provisioning type, the pool will not be selected for the creation of the VM.
StoragePool capabilities are determined when the pool is added to Cloudstack and a new API is added ( updateStorageCapabilities(poolId) ) to update the hardware acceleration capability of storage pools if needed.
<!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
<!-- Fixes: # -->
<!--- ********************************************************************************* -->
<!--- NOTE: AUTOMATATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ********************************************************************************* -->
### Types of changes
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
### Feature/Enhancement Scale or Bug Severity
#### Feature/Enhancement Scale
- [x] Major
- [ ] Minor
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
<!-- see how your change affects other areas of the code, etc. -->
Marvin tests have been added and manual tests have been done using NFS without hardware acceleration and an ISCSI datastore.
Create a thin/sparse/thick disk/service offering, set the global var to true, depending on the offering selected, the disks should be created in the correct storage pool with the correct disk provisioning type.
If the global var is set to false, the disk will be created in any storage pool and if the pool supports the disk provisioning type it will be used, otherwise thin will be selected.
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/master/CONTRIBUTING.md) document -->
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-881323316
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-771540339
@Spaceman1984 can you kick pkging and testing, is this still in WIP - pl change it to draft. Thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r664236652
##########
File path: engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
##########
@@ -100,9 +110,48 @@ public boolean hostConnect(long hostId, long poolId) throws StorageConflictExcep
updateStoragePoolHostVOAndDetails(poolVO, hostId, mspAnswer);
if (pool.getPoolType() == Storage.StoragePoolType.DatastoreCluster) {
+ for (ModifyStoragePoolAnswer childDataStoreAnswer : ((ModifyStoragePoolAnswer) answer).getDatastoreClusterChildren()) {
+ StoragePoolInfo childStoragePoolInfo = childDataStoreAnswer.getPoolInfo();
+ StoragePoolVO dataStoreVO = primaryStoreDao.findPoolByUUID(childStoragePoolInfo.getUuid());
+ if (dataStoreVO != null) {
+ continue;
+ }
+ dataStoreVO = new StoragePoolVO();
+ dataStoreVO.setStorageProviderName(poolVO.getStorageProviderName());
+ dataStoreVO.setHostAddress(childStoragePoolInfo.getHost());
+ dataStoreVO.setPoolType(Storage.StoragePoolType.PreSetup);
+ dataStoreVO.setPath(childStoragePoolInfo.getHostPath());
+ dataStoreVO.setPort(poolVO.getPort());
+ dataStoreVO.setName(childStoragePoolInfo.getName());
+ dataStoreVO.setUuid(childStoragePoolInfo.getUuid());
+ dataStoreVO.setDataCenterId(poolVO.getDataCenterId());
+ dataStoreVO.setPodId(poolVO.getPodId());
+ dataStoreVO.setClusterId(poolVO.getClusterId());
+ dataStoreVO.setStatus(StoragePoolStatus.Up);
+ dataStoreVO.setUserInfo(poolVO.getUserInfo());
+ dataStoreVO.setManaged(poolVO.isManaged());
+ dataStoreVO.setCapacityIops(poolVO.getCapacityIops());
+ dataStoreVO.setCapacityBytes(childDataStoreAnswer.getPoolInfo().getCapacityBytes());
+ dataStoreVO.setUsedBytes(childDataStoreAnswer.getPoolInfo().getCapacityBytes() - childDataStoreAnswer.getPoolInfo().getAvailableBytes());
+ dataStoreVO.setHypervisor(poolVO.getHypervisor());
+ dataStoreVO.setScope(poolVO.getScope());
+ dataStoreVO.setParent(poolVO.getId());
Review comment:
yes @sureshanaparti these are not part of this PR. This might have happened while rebasing @Spaceman1984 please check with the latest main and rebase it appropriately.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569195343
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
##########
@@ -929,21 +929,21 @@ private String cloneVMforVvols(VmwareContext context, VmwareHypervisorHost hyper
return vmdkFileBaseName;
}
- private String createVMFolderWithVMName(VmwareContext context, VmwareHypervisorHost hyperHost, TemplateObjectTO template,
- VirtualMachineMO vmTemplate, VolumeObjectTO volume, DatacenterMO dcMo, DatastoreMO dsMo,
- String searchExcludedFolders) throws Exception {
+ private String createVMAndFolderWithVMName(VmwareContext context, VmwareHypervisorHost hyperHost, TemplateObjectTO template,
+ VirtualMachineMO vmTemplate, VolumeObjectTO volume, DatacenterMO dcMo, DatastoreMO dsMo,
+ String searchExcludedFolders) throws Exception {
String vmdkName = volume.getName();
try {
ManagedObjectReference morDatastore = dsMo.getMor();
ManagedObjectReference morPool = hyperHost.getHyperHostOwnerResourcePool();
ManagedObjectReference morCluster = hyperHost.getHyperHostCluster();
- if (template.getSize() != null){
+ if (template.getSize() != null) {
_fullCloneFlag = volume.getSize() > template.getSize() ? true : _fullCloneFlag;
}
if (!_fullCloneFlag) {
createVMLinkedClone(vmTemplate, dcMo, vmdkName, morDatastore, morPool);
Review comment:
Is the "disk provisioning type" not considered in case of linked clone ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-775830511
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-775091531
Packaging result: ✖centos7 ✖centos8 ✖debian. JID-2656
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-879046434
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r571963026
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java
##########
@@ -0,0 +1,80 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+
+@APICommand(name = UpdateStorageCapabilitiesCmd.APINAME, description = "Syncs hardware acceleration capabilities of storage pools",
+ responseObject = SuccessResponse.class,
+ requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class UpdateStorageCapabilitiesCmd extends BaseCmd {
+ public static final String APINAME = "updateStorageCapabilities";
+
+ /////////////////////////////////////////////////////
+ //////////////// API parameters /////////////////////
+ /////////////////////////////////////////////////////
+
+ @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = StoragePoolResponse.class, required = true, description = "Storage pool id")
+ private Long poolId;
+
+ /////////////////////////////////////////////////////
+ /////////////////// Accessors ///////////////////////
+ /////////////////////////////////////////////////////
+
+ public Long getPoolId() {
+ return poolId;
+ }
+
+ public void setPoolId(Long poolId) {
+ this.poolId = poolId;
+ }
+
+ /////////////////////////////////////////////////////
+ /////////////// API Implementation///////////////////
+ /////////////////////////////////////////////////////
+
+
+ @Override
+ public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException {
+ _storageService.syncHardwareCapability(poolId);
Review comment:
Done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-786576071
@blueorangutan test centos7 vmware-67u3
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-786502808
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-836582960
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r650803790
##########
File path: api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java
##########
@@ -94,7 +94,7 @@ public Long getPageSizeVal() {
if (pageSizeInt != null) {
defaultPageSize = pageSizeInt.longValue();
}
- if (defaultPageSize.longValue() == s_pageSizeUnlimited) {
+ if (defaultPageSize!= null && defaultPageSize.longValue() == s_pageSizeUnlimited) {
Review comment:
This is fine @DaanHoogland, this code came in as part of some unrelated merge. Not sure if we need something like:
if (s_pageSizeUnlimited.equals(defaultPageSize.longValue()) {
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-878949448
@blueorangutan test centos7 vmware-67u3
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r582809569
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
##########
@@ -179,9 +179,32 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
} else if(allocationAlgorithm.equals("firstfitleastconsumed")){
pools = reorderPoolsByCapacity(plan, pools);
}
+
+ // Hardware accelerated pools are preferred for thick disks
+ if (dskCh != null && !dskCh.getProvisioningType().equals(Storage.ProvisioningType.THIN) &&
+ !storageMgr.DiskProvisioningStrictness.valueIn(plan.getDataCenterId())) {
+ pools = reorderPoolsByDiskProvisioningType(pools, dskCh);
+ }
+
return pools;
}
+ private List<StoragePool> reorderPoolsByDiskProvisioningType(List<StoragePool> pools, DiskProfile dskCh) {
Review comment:
`DiskProfile dskCh` is unused here
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-786535007
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-856303876
<b>Trillian test result (tid-887)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36415 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t887-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Smoke tests completed. 89 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-878925933
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 538
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-792781581
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2886
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r572635381
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -5260,6 +5269,66 @@ protected Answer execute(ModifyStoragePoolCommand cmd) {
}
}
+ protected Answer execute(GetStoragePoolCapabilitiesCommand cmd) {
+
+ try {
+
+ VmwareHypervisorHost hyperHost = getHyperHost(getServiceContext());
+
+ HostMO host = (HostMO) hyperHost;
+
+ StorageFilerTO pool = cmd.getPool();
+
+ ManagedObjectReference morDatastore = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, pool.getUuid());
+
+ if (morDatastore == null) {
+ morDatastore = hyperHost.mountDatastore((pool.getType() == StoragePoolType.VMFS || pool.getType() == StoragePoolType.PreSetup || pool.getType() == StoragePoolType.DatastoreCluster), pool.getHost(), pool.getPort(), pool.getPath(), pool.getUuid().replace("-", ""), true);
+ }
+
+ assert (morDatastore != null);
+
+ DatastoreMO dsMo = new DatastoreMO(getServiceContext(), morDatastore);
+
+ GetStoragePoolCapabilitiesAnswer answer = new GetStoragePoolCapabilitiesAnswer(cmd);
+
+ if (pool.getType() == StoragePoolType.NetworkFilesystem) {
Review comment:
Not sure what you mean here, do you want me to remove the check for NFS?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-778168413
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-776489045
@blueorangutan test centos7 vmware-67u3
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569184108
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -729,6 +730,7 @@
static final ConfigKey<Integer> vmPasswordLength = new ConfigKey<Integer>("Advanced", Integer.class, "vm.password.length", "6", "Specifies the length of a randomly generated password", false);
static final ConfigKey<Integer> sshKeyLength = new ConfigKey<Integer>("Advanced", Integer.class, "ssh.key.length", "2048", "Specifies custom SSH key length (bit)", true, ConfigKey.Scope.Global);
static final ConfigKey<Boolean> humanReadableSizes = new ConfigKey<Boolean>("Advanced", Boolean.class, "display.human.readable.sizes", "true", "Enables outputting human readable byte sizes to logs and usage records.", false, ConfigKey.Scope.Global);
+ static final ConfigKey<Boolean> diskProvisioningStrictness = new ConfigKey<Boolean>("Storage", Boolean.class, "disk.provisioning.type.strictness", "false", "Use storage pools only with supported disk provisioning types for disk/service offerings.", true, ConfigKey.Scope.Zone);
Review comment:
can you please update the description here, it doesn't indicate anything related to disk provisioning strictness
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-878907826
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r582897918
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
##########
@@ -179,9 +179,32 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
} else if(allocationAlgorithm.equals("firstfitleastconsumed")){
pools = reorderPoolsByCapacity(plan, pools);
}
+
+ // Hardware accelerated pools are preferred for thick disks
+ if (dskCh != null && !dskCh.getProvisioningType().equals(Storage.ProvisioningType.THIN) &&
Review comment:
If I move that into the method I have to pass in the plan as well. I don't think it matters here. It will also impact the readability negatively as there is already alot of logic hapening inside of that method.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569178078
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
##########
@@ -207,6 +215,13 @@ protected boolean filter(ExcludeList avoid, StoragePool pool, DiskProfile dskCh,
return false;
}
+ if (diskProvisioningTypeStrictness){
+ StoragePoolDetailVO hardwareAcceleration = storagePoolDetailsDao.findDetail(pool.getId(), "hardwareAccelerationSupported");
Review comment:
it is better to define a string constant for "`hardwareAccelerationSupported`", in some common storage / util class and use it across.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-771616015
@blueorangutan test centos7 vmware-67u3
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 closed pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 closed pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-771542602
@blueorangutan test ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-878557393
<b>Trillian test result (tid-1231)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 53560 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t1231-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_network.py
Intermittent failure detected: /marvin/tests/smoke/test_nic.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
Smoke tests completed. 84 look OK, 5 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_nic | `Error` | 180.06 | test_nic.py
test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80 | `Failure` | 609.38 | test_internal_lb.py
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 411.17 | test_routers_network_ops.py
test_01_migrate_VM_and_root_volume | `Error` | 127.66 | test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks | `Error` | 86.35 | test_vm_life_cycle.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 653.43 | test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 742.29 | test_vpc_redundant.py
test_05_rvpc_multi_tiers | `Failure` | 597.30 | test_vpc_redundant.py
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r571966539
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
##########
@@ -929,21 +929,21 @@ private String cloneVMforVvols(VmwareContext context, VmwareHypervisorHost hyper
return vmdkFileBaseName;
}
- private String createVMFolderWithVMName(VmwareContext context, VmwareHypervisorHost hyperHost, TemplateObjectTO template,
- VirtualMachineMO vmTemplate, VolumeObjectTO volume, DatacenterMO dcMo, DatastoreMO dsMo,
- String searchExcludedFolders) throws Exception {
+ private String createVMAndFolderWithVMName(VmwareContext context, VmwareHypervisorHost hyperHost, TemplateObjectTO template,
+ VirtualMachineMO vmTemplate, VolumeObjectTO volume, DatacenterMO dcMo, DatastoreMO dsMo,
+ String searchExcludedFolders) throws Exception {
String vmdkName = volume.getName();
try {
ManagedObjectReference morDatastore = dsMo.getMor();
ManagedObjectReference morPool = hyperHost.getHyperHostOwnerResourcePool();
ManagedObjectReference morCluster = hyperHost.getHyperHostCluster();
- if (template.getSize() != null){
+ if (template.getSize() != null) {
_fullCloneFlag = volume.getSize() > template.getSize() ? true : _fullCloneFlag;
}
if (!_fullCloneFlag) {
createVMLinkedClone(vmTemplate, dcMo, vmdkName, morDatastore, morPool);
Review comment:
I have added checks
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
##########
@@ -3950,7 +3950,7 @@ public VirtualMachineMO cloneVMFromTemplate(VmwareHypervisorHost hyperHost, Stri
if (!_fullCloneFlag) {
createVMLinkedClone(templateMo, dcMo, cloneName, morDatastore, morPool);
Review comment:
I have added checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-851956901
@blueorangutan package
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-863980390
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r664236652
##########
File path: engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
##########
@@ -100,9 +110,48 @@ public boolean hostConnect(long hostId, long poolId) throws StorageConflictExcep
updateStoragePoolHostVOAndDetails(poolVO, hostId, mspAnswer);
if (pool.getPoolType() == Storage.StoragePoolType.DatastoreCluster) {
+ for (ModifyStoragePoolAnswer childDataStoreAnswer : ((ModifyStoragePoolAnswer) answer).getDatastoreClusterChildren()) {
+ StoragePoolInfo childStoragePoolInfo = childDataStoreAnswer.getPoolInfo();
+ StoragePoolVO dataStoreVO = primaryStoreDao.findPoolByUUID(childStoragePoolInfo.getUuid());
+ if (dataStoreVO != null) {
+ continue;
+ }
+ dataStoreVO = new StoragePoolVO();
+ dataStoreVO.setStorageProviderName(poolVO.getStorageProviderName());
+ dataStoreVO.setHostAddress(childStoragePoolInfo.getHost());
+ dataStoreVO.setPoolType(Storage.StoragePoolType.PreSetup);
+ dataStoreVO.setPath(childStoragePoolInfo.getHostPath());
+ dataStoreVO.setPort(poolVO.getPort());
+ dataStoreVO.setName(childStoragePoolInfo.getName());
+ dataStoreVO.setUuid(childStoragePoolInfo.getUuid());
+ dataStoreVO.setDataCenterId(poolVO.getDataCenterId());
+ dataStoreVO.setPodId(poolVO.getPodId());
+ dataStoreVO.setClusterId(poolVO.getClusterId());
+ dataStoreVO.setStatus(StoragePoolStatus.Up);
+ dataStoreVO.setUserInfo(poolVO.getUserInfo());
+ dataStoreVO.setManaged(poolVO.isManaged());
+ dataStoreVO.setCapacityIops(poolVO.getCapacityIops());
+ dataStoreVO.setCapacityBytes(childDataStoreAnswer.getPoolInfo().getCapacityBytes());
+ dataStoreVO.setUsedBytes(childDataStoreAnswer.getPoolInfo().getCapacityBytes() - childDataStoreAnswer.getPoolInfo().getAvailableBytes());
+ dataStoreVO.setHypervisor(poolVO.getHypervisor());
+ dataStoreVO.setScope(poolVO.getScope());
+ dataStoreVO.setParent(poolVO.getId());
Review comment:
yes @sureshanaparti these are not part of this PR. This might have happened while rebasing @Spaceman1984 please check with the latest main and rebase it appropriately.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r572608182
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -729,6 +730,7 @@
static final ConfigKey<Integer> vmPasswordLength = new ConfigKey<Integer>("Advanced", Integer.class, "vm.password.length", "6", "Specifies the length of a randomly generated password", false);
static final ConfigKey<Integer> sshKeyLength = new ConfigKey<Integer>("Advanced", Integer.class, "ssh.key.length", "2048", "Specifies custom SSH key length (bit)", true, ConfigKey.Scope.Global);
static final ConfigKey<Boolean> humanReadableSizes = new ConfigKey<Boolean>("Advanced", Boolean.class, "display.human.readable.sizes", "true", "Enables outputting human readable byte sizes to logs and usage records.", false, ConfigKey.Scope.Global);
+ static final ConfigKey<Boolean> diskProvisioningStrictness = new ConfigKey<Boolean>("Storage", Boolean.class, "disk.provisioning.type.strictness", "false", "Use storage pools only with supported disk provisioning types for disk/service offerings.", true, ConfigKey.Scope.Zone);
Review comment:
@Spaceman1984 may be some sort of clear description what it is for & what it does. Something like "_If set to true, the disk is created only when there is a suitable storage pool that supports the disk provisioning type. If set to false, the disk is created with other disk provisioning type when there are no suitable storage pool(s) that supports the original disk provisioning type. Default value is false, and this is currently supported for VMware only_."
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 closed pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 closed pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-777296490
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569176510
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
##########
@@ -82,6 +86,10 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
if (allocationAlgorithm != null) {
this.allocationAlgorithm = allocationAlgorithm;
}
+ String strictness = configs.get("disk.provisioning.type.strictness");
Review comment:
Agree with @harikrishna-patnala , please use Config framework to get the zone-wide setting.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-868288412
@Spaceman1984 can you check failed travis checks?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-828353449
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-827857903
<b>Trillian test result (tid-540)</b>
Environment: xcpng81 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41631 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t540-xcpng81.zip
Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
Smoke tests completed. 88 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_scale_vm | `Failure` | 5.17 | test_scale_vm.py
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r571964937
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java
##########
@@ -794,6 +798,33 @@ public boolean createFullClone(String cloneName, ManagedObjectReference morFolde
return false;
}
+ private void setDiskProvsioningType(VirtualMachineRelocateSpec relocSpec, ManagedObjectReference morDs,
+ Storage.ProvisioningType diskProvisioningType) throws Exception {
+ if (diskProvisioningType == null){
+ return;
+ }
+ List<VirtualMachineRelocateSpecDiskLocator> relocateDisks = relocSpec.getDisk();
+ List<VirtualDisk> disks = this.getVirtualDisks();
+ for (VirtualDisk disk: disks){
Review comment:
I haven't seen a case, but the getVirtualDisks() method returns a list, so just being safe here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-863101058
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-828294315
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 481
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-881713274
<b>Trillian test result (tid-1293)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36476 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t1293-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_deploy_vm_root_resize.py
Smoke tests completed. 89 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez merged pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
nvazquez merged pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r650803790
##########
File path: api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java
##########
@@ -94,7 +94,7 @@ public Long getPageSizeVal() {
if (pageSizeInt != null) {
defaultPageSize = pageSizeInt.longValue();
}
- if (defaultPageSize.longValue() == s_pageSizeUnlimited) {
+ if (defaultPageSize!= null && defaultPageSize.longValue() == s_pageSizeUnlimited) {
Review comment:
This is fine Daan, this code came in as part of some unrelated merge. Not sure if we need something like:
if(s_pageSizeUnlimited.equals(defaultPageSize.longValue()) {
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-878963417
<b>Trillian Build Failed (tid-1257)<b/>
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-772219969
<b>Trillian test result (tid-3473)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 54550 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t3473-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_snapshots.py
Smoke tests completed. 84 look OK, 3 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 818.42 | test_kubernetes_clusters.py
test_04_change_offering_small | `Error` | 5.33 | test_service_offerings.py
test_change_service_offering_for_vm_with_snapshots | `Error` | 140.77 | test_vm_snapshots.py
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r572617424
##########
File path: core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java
##########
@@ -0,0 +1,19 @@
+package com.cloud.agent.api;
+
+public class GetStoragePoolCapabilitiesAnswer extends Answer {
+ public GetStoragePoolCapabilitiesAnswer(GetStoragePoolCapabilitiesCommand cmd) {
+ super(cmd);
+
+ poolInfo = new StoragePoolInfo();
+ }
+
+ public StoragePoolInfo getPoolInfo() {
+ return poolInfo;
+ }
+
+ public void setPoolInfo(StoragePoolInfo poolInfo) {
+ this.poolInfo = poolInfo;
+ }
+
+ private StoragePoolInfo poolInfo;
Review comment:
I think capability details map should ok in the response.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-853600210
@DaanHoogland a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-814790616
Packaging result: :heavy_multiplication_x: centos7 :heavy_multiplication_x: centos8 :heavy_multiplication_x: debian. SL-JID 346
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569193640
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java
##########
@@ -781,6 +782,9 @@ public boolean createFullClone(String cloneName, ManagedObjectReference morFolde
relocSpec.setDatastore(morDs);
relocSpec.setPool(morResourcePool);
+
+ setDiskProvsioningType(relocSpec, morDs, diskProvisioningType);
+
Review comment:
when "_diskProvisioningType == null_" here, which provisioning type the disk will fall back to ? strictness check required here ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r572616290
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -5260,6 +5269,66 @@ protected Answer execute(ModifyStoragePoolCommand cmd) {
}
}
+ protected Answer execute(GetStoragePoolCapabilitiesCommand cmd) {
+
+ try {
+
+ VmwareHypervisorHost hyperHost = getHyperHost(getServiceContext());
+
+ HostMO host = (HostMO) hyperHost;
+
+ StorageFilerTO pool = cmd.getPool();
+
+ ManagedObjectReference morDatastore = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, pool.getUuid());
+
+ if (morDatastore == null) {
+ morDatastore = hyperHost.mountDatastore((pool.getType() == StoragePoolType.VMFS || pool.getType() == StoragePoolType.PreSetup || pool.getType() == StoragePoolType.DatastoreCluster), pool.getHost(), pool.getPort(), pool.getPath(), pool.getUuid().replace("-", ""), true);
+ }
+
+ assert (morDatastore != null);
+
+ DatastoreMO dsMo = new DatastoreMO(getServiceContext(), morDatastore);
+
+ GetStoragePoolCapabilitiesAnswer answer = new GetStoragePoolCapabilitiesAnswer(cmd);
+
+ if (pool.getType() == StoragePoolType.NetworkFilesystem) {
Review comment:
Can you define this method generic, irrespective of storage pool type.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-792762066
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r571967560
##########
File path: engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
##########
@@ -166,6 +166,13 @@ private void updateStoragePoolHostVOAndDetails(StoragePool pool, long hostId, Mo
storagePoolDetailsDao.persist(storagePoolDetailVO);
}
}
Review comment:
This path is used when the pool scope is zone level.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-771562576
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-879045673
@blueorangutan test centos7 vmware-67u3
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569195455
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
##########
@@ -3950,7 +3950,7 @@ public VirtualMachineMO cloneVMFromTemplate(VmwareHypervisorHost hyperHost, Stri
if (!_fullCloneFlag) {
createVMLinkedClone(templateMo, dcMo, cloneName, morDatastore, morPool);
Review comment:
"disk provisioning type" not considered in case of linked clone ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala closed pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala closed pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569185735
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -2331,6 +2336,17 @@ public ImageStore updateImageStoreStatus(Long id, Boolean readonly) {
return imageStoreVO;
}
+ @Override
+ public void syncHardwareCapability(long poolId) {
+ StoragePoolVO pool = _storagePoolDao.findById(poolId);
+ // find the host
+ List<StoragePoolHostVO> hosts = _storagePoolHostDao.listByPoolId(poolId);
+ if (hosts.size() > 0) {
+ StoragePoolHostVO host = hosts.get(0);
+ _agentMgr.easySend(host.getHostId(), new ModifyStoragePoolCommand(false, pool));
Review comment:
can use separate cmd for storage capability ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 removed a comment on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 removed a comment on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-778220630
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-771562576
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-878908470
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-836582240
@blueorangutan package
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-826597487
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r582896985
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
##########
@@ -179,9 +179,32 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
} else if(allocationAlgorithm.equals("firstfitleastconsumed")){
pools = reorderPoolsByCapacity(plan, pools);
}
+
+ // Hardware accelerated pools are preferred for thick disks
+ if (dskCh != null && !dskCh.getProvisioningType().equals(Storage.ProvisioningType.THIN) &&
+ !storageMgr.DiskProvisioningStrictness.valueIn(plan.getDataCenterId())) {
+ pools = reorderPoolsByDiskProvisioningType(pools, dskCh);
+ }
+
return pools;
}
+ private List<StoragePool> reorderPoolsByDiskProvisioningType(List<StoragePool> pools, DiskProfile dskCh) {
Review comment:
Removed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-880677286
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-881083439
<b>Trillian test result (tid-1279)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36928 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t1279-vmware-67u3.zip
Smoke tests completed. 89 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-771614505
@Spaceman1984 unsupported parameters provided. Supported mgmt server os are: `centos6, centos7, centos8, ubuntu`. Supported hypervisors are: `kvm-centos6, kvm-centos7, kvm-centos8, kvm-ubuntu, xenserver-71, xenserver-65sp1, vmware-67u3, vmware-65u2, vmware-60u2, vmware-55u3, xcpng76, xcpng80, xcpng81, xenserver-74, xcpng74`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r571962496
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
##########
@@ -82,6 +86,10 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
if (allocationAlgorithm != null) {
this.allocationAlgorithm = allocationAlgorithm;
}
+ String strictness = configs.get("disk.provisioning.type.strictness");
Review comment:
I have moved the config to StorageManager
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-778158347
Packaging result: ✖centos7 ✖centos8 ✖debian. JID-2690
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-775150741
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2658
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569174251
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java
##########
@@ -0,0 +1,80 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+
+@APICommand(name = UpdateStorageCapabilitiesCmd.APINAME, description = "Syncs hardware acceleration capabilities of storage pools",
Review comment:
In the future this API can be extended to fetch other storage capabilities, also, in the case of using this API, you won't have to wait for maintenance mode.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 closed pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 closed pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-826580106
@blueorangutan package
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-880676464
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r572761774
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -5260,6 +5269,66 @@ protected Answer execute(ModifyStoragePoolCommand cmd) {
}
}
+ protected Answer execute(GetStoragePoolCapabilitiesCommand cmd) {
+
+ try {
+
+ VmwareHypervisorHost hyperHost = getHyperHost(getServiceContext());
+
+ HostMO host = (HostMO) hyperHost;
+
+ StorageFilerTO pool = cmd.getPool();
+
+ ManagedObjectReference morDatastore = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, pool.getUuid());
+
+ if (morDatastore == null) {
+ morDatastore = hyperHost.mountDatastore((pool.getType() == StoragePoolType.VMFS || pool.getType() == StoragePoolType.PreSetup || pool.getType() == StoragePoolType.DatastoreCluster), pool.getHost(), pool.getPort(), pool.getPath(), pool.getUuid().replace("-", ""), true);
+ }
+
+ assert (morDatastore != null);
+
+ DatastoreMO dsMo = new DatastoreMO(getServiceContext(), morDatastore);
+
+ GetStoragePoolCapabilitiesAnswer answer = new GetStoragePoolCapabilitiesAnswer(cmd);
+
+ if (pool.getType() == StoragePoolType.NetworkFilesystem) {
+ boolean hardwareAccelerationSupportForDataStore = getHardwareAccelerationSupportForDataStore(host.getMor(), dsMo.getName());
+ StoragePoolInfo poolInfo = answer.getPoolInfo();
+ Map<String, String> poolDetails = poolInfo.getDetails();
+ if (poolDetails == null) {
+ poolDetails = new HashMap<>();
Review comment:
Done
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
##########
@@ -913,9 +914,12 @@ private String cloneVMforVvols(VmwareContext context, VmwareHypervisorHost hyper
if (volume.getVolumeType() == Volume.Type.DATADISK)
vmName = volume.getName();
if (!_fullCloneFlag) {
+ if (_diskProvisioningStrictness) {
+ throw new CloudRuntimeException("Unable to create linked clones with strict disk provisioning enabled");
Review comment:
Fixed
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
##########
@@ -929,21 +933,24 @@ private String cloneVMforVvols(VmwareContext context, VmwareHypervisorHost hyper
return vmdkFileBaseName;
}
- private String createVMFolderWithVMName(VmwareContext context, VmwareHypervisorHost hyperHost, TemplateObjectTO template,
- VirtualMachineMO vmTemplate, VolumeObjectTO volume, DatacenterMO dcMo, DatastoreMO dsMo,
- String searchExcludedFolders) throws Exception {
+ private String createVMAndFolderWithVMName(VmwareContext context, VmwareHypervisorHost hyperHost, TemplateObjectTO template,
+ VirtualMachineMO vmTemplate, VolumeObjectTO volume, DatacenterMO dcMo, DatastoreMO dsMo,
+ String searchExcludedFolders) throws Exception {
String vmdkName = volume.getName();
try {
ManagedObjectReference morDatastore = dsMo.getMor();
ManagedObjectReference morPool = hyperHost.getHyperHostOwnerResourcePool();
ManagedObjectReference morCluster = hyperHost.getHyperHostCluster();
- if (template.getSize() != null){
+ if (template.getSize() != null) {
_fullCloneFlag = volume.getSize() > template.getSize() ? true : _fullCloneFlag;
}
if (!_fullCloneFlag) {
+ if (_diskProvisioningStrictness) {
+ throw new CloudRuntimeException("Unable to create linked clones with strict disk provisioning enabled");
Review comment:
Fixed
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
##########
@@ -997,9 +1004,12 @@ private void createLinkedOrFullClone(TemplateObjectTO template, VolumeObjectTO v
_fullCloneFlag = volume.getSize() > template.getSize() || _fullCloneFlag;
}
if (!_fullCloneFlag) {
+ if (_diskProvisioningStrictness) {
+ throw new CloudRuntimeException("Unable to create linked clones with strict disk provisioning enabled");
Review comment:
fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r572762108
##########
File path: plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
##########
@@ -404,6 +406,12 @@ protected boolean createStoragePool(long hostId, StoragePool pool) {
CreateStoragePoolCommand cmd = new CreateStoragePoolCommand(true, pool);
final Answer answer = agentMgr.easySend(hostId, cmd);
if (answer != null && answer.getResult()) {
+ HypervisorType hyperVisorType = getHypervisorType(hostId);
+ if (pool.getPoolType() == StoragePoolType.NetworkFilesystem && hyperVisorType == HypervisorType.VMware) {
+ GetStoragePoolCapabilitiesCommand capsCmd = new GetStoragePoolCapabilitiesCommand();
+ capsCmd.setPool(new StorageFilerTO(pool));
+ agentMgr.easySend(hostId, capsCmd);
Review comment:
Fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 edited a comment on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 edited a comment on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-772390552
> @Spaceman1984 May I know what happens if strictness is set to false and disk is provisioned on unsupported storage pool ?
@harikrishna-patnala If strictness is set to false, a thin disk will be created on the unsupported pool.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-778123251
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-778027662
@Spaceman1984 there an empty file here test/__init__.py please delete it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r595924947
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java
##########
@@ -0,0 +1,84 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ListResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
+
+@APICommand(name = UpdateStorageCapabilitiesCmd.APINAME, description = "Syncs capabilities of storage pools",
+ responseObject = StoragePoolResponse.class,
+ requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.16.0")
+public class UpdateStorageCapabilitiesCmd extends BaseCmd {
+ public static final String APINAME = "updateStorageCapabilities";
+ private static final Logger LOG = Logger.getLogger(UpdateImageStoreCmd.class.getName());
Review comment:
@Spaceman1984 you need to change this to UpdateStorageCapabilitiesCmd.class.getName()
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-863979753
@blueorangutan test matrix
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r582814336
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
##########
@@ -179,9 +179,32 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
} else if(allocationAlgorithm.equals("firstfitleastconsumed")){
pools = reorderPoolsByCapacity(plan, pools);
}
+
+ // Hardware accelerated pools are preferred for thick disks
+ if (dskCh != null && !dskCh.getProvisioningType().equals(Storage.ProvisioningType.THIN) &&
Review comment:
i think it is better to move this disk profile checks to private method _reorderPoolsByDiskProvisioningType()_ below
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-864294980
<b>Trillian test result (tid-1001)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 34191 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t1001-kvm-centos7.zip
Smoke tests completed. 89 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r572762355
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -729,6 +730,7 @@
static final ConfigKey<Integer> vmPasswordLength = new ConfigKey<Integer>("Advanced", Integer.class, "vm.password.length", "6", "Specifies the length of a randomly generated password", false);
static final ConfigKey<Integer> sshKeyLength = new ConfigKey<Integer>("Advanced", Integer.class, "ssh.key.length", "2048", "Specifies custom SSH key length (bit)", true, ConfigKey.Scope.Global);
static final ConfigKey<Boolean> humanReadableSizes = new ConfigKey<Boolean>("Advanced", Boolean.class, "display.human.readable.sizes", "true", "Enables outputting human readable byte sizes to logs and usage records.", false, ConfigKey.Scope.Global);
+ static final ConfigKey<Boolean> diskProvisioningStrictness = new ConfigKey<Boolean>("Storage", Boolean.class, "disk.provisioning.type.strictness", "false", "Use storage pools only with supported disk provisioning types for disk/service offerings.", true, ConfigKey.Scope.Zone);
Review comment:
Done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r571964373
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -2331,6 +2336,17 @@ public ImageStore updateImageStoreStatus(Long id, Boolean readonly) {
return imageStoreVO;
}
+ @Override
+ public void syncHardwareCapability(long poolId) {
+ StoragePoolVO pool = _storagePoolDao.findById(poolId);
+ // find the host
+ List<StoragePoolHostVO> hosts = _storagePoolHostDao.listByPoolId(poolId);
+ if (hosts.size() > 0) {
+ StoragePoolHostVO host = hosts.get(0);
+ _agentMgr.easySend(host.getHostId(), new ModifyStoragePoolCommand(false, pool));
Review comment:
Done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r582897469
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
##########
@@ -179,9 +179,32 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
} else if(allocationAlgorithm.equals("firstfitleastconsumed")){
pools = reorderPoolsByCapacity(plan, pools);
}
+
+ // Hardware accelerated pools are preferred for thick disks
+ if (dskCh != null && !dskCh.getProvisioningType().equals(Storage.ProvisioningType.THIN) &&
+ !storageMgr.DiskProvisioningStrictness.valueIn(plan.getDataCenterId())) {
+ pools = reorderPoolsByDiskProvisioningType(pools, dskCh);
+ }
+
return pools;
}
+ private List<StoragePool> reorderPoolsByDiskProvisioningType(List<StoragePool> pools, DiskProfile dskCh) {
+ List<StoragePool> reorderedPools = new ArrayList<>();
+ for (StoragePool pool: pools) {
Review comment:
No, If thin disk provisioning is required, reordering is not required, all storage pools support thin disk provisioning.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 removed a comment on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 removed a comment on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-837927118
@blueorangutan test centos7 vmware-67u3
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r572625991
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -2331,6 +2338,30 @@ public ImageStore updateImageStoreStatus(Long id, Boolean readonly) {
return imageStoreVO;
}
+ @Override
+ public void updateStorageCapabilities(long poolId) {
+ StoragePoolVO pool = _storagePoolDao.findById(poolId);
+ // find the host
+ List<StoragePoolHostVO> hosts = _storagePoolHostDao.listByPoolId(poolId);
Review comment:
make sure the host is Up and Enabled.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r572625459
##########
File path: plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java
##########
@@ -404,6 +406,12 @@ protected boolean createStoragePool(long hostId, StoragePool pool) {
CreateStoragePoolCommand cmd = new CreateStoragePoolCommand(true, pool);
final Answer answer = agentMgr.easySend(hostId, cmd);
if (answer != null && answer.getResult()) {
+ HypervisorType hyperVisorType = getHypervisorType(hostId);
+ if (pool.getPoolType() == StoragePoolType.NetworkFilesystem && hyperVisorType == HypervisorType.VMware) {
+ GetStoragePoolCapabilitiesCommand capsCmd = new GetStoragePoolCapabilitiesCommand();
+ capsCmd.setPool(new StorageFilerTO(pool));
+ agentMgr.easySend(hostId, capsCmd);
Review comment:
not checking for the answer and updating the results from it ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-881257520
Should be good to go @nvazquez
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-878949905
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-776489362
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-881322691
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-792761707
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 closed pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 closed pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-881346250
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-826798151
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 466
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569181867
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -5260,6 +5272,17 @@ protected Answer execute(ModifyStoragePoolCommand cmd) {
}
}
+ private boolean getHardwareAcceleationSupportForDataStore(ManagedObjectReference host, String dataStoreName) throws Exception {
Review comment:
```suggestion
private boolean getHardwareAccelerationSupportForDataStore(ManagedObjectReference host, String dataStoreName) throws Exception {
```
typo correction - Acceleration
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-880693924
@blueorangutan test centos7 vmware-67u3
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r664228043
##########
File path: engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
##########
@@ -100,9 +110,48 @@ public boolean hostConnect(long hostId, long poolId) throws StorageConflictExcep
updateStoragePoolHostVOAndDetails(poolVO, hostId, mspAnswer);
if (pool.getPoolType() == Storage.StoragePoolType.DatastoreCluster) {
+ for (ModifyStoragePoolAnswer childDataStoreAnswer : ((ModifyStoragePoolAnswer) answer).getDatastoreClusterChildren()) {
+ StoragePoolInfo childStoragePoolInfo = childDataStoreAnswer.getPoolInfo();
+ StoragePoolVO dataStoreVO = primaryStoreDao.findPoolByUUID(childStoragePoolInfo.getUuid());
+ if (dataStoreVO != null) {
+ continue;
+ }
+ dataStoreVO = new StoragePoolVO();
+ dataStoreVO.setStorageProviderName(poolVO.getStorageProviderName());
+ dataStoreVO.setHostAddress(childStoragePoolInfo.getHost());
+ dataStoreVO.setPoolType(Storage.StoragePoolType.PreSetup);
+ dataStoreVO.setPath(childStoragePoolInfo.getHostPath());
+ dataStoreVO.setPort(poolVO.getPort());
+ dataStoreVO.setName(childStoragePoolInfo.getName());
+ dataStoreVO.setUuid(childStoragePoolInfo.getUuid());
+ dataStoreVO.setDataCenterId(poolVO.getDataCenterId());
+ dataStoreVO.setPodId(poolVO.getPodId());
+ dataStoreVO.setClusterId(poolVO.getClusterId());
+ dataStoreVO.setStatus(StoragePoolStatus.Up);
+ dataStoreVO.setUserInfo(poolVO.getUserInfo());
+ dataStoreVO.setManaged(poolVO.isManaged());
+ dataStoreVO.setCapacityIops(poolVO.getCapacityIops());
+ dataStoreVO.setCapacityBytes(childDataStoreAnswer.getPoolInfo().getCapacityBytes());
+ dataStoreVO.setUsedBytes(childDataStoreAnswer.getPoolInfo().getCapacityBytes() - childDataStoreAnswer.getPoolInfo().getAvailableBytes());
+ dataStoreVO.setHypervisor(poolVO.getHypervisor());
+ dataStoreVO.setScope(poolVO.getScope());
+ dataStoreVO.setParent(poolVO.getId());
Review comment:
@Spaceman1984 it seems the changes here are not part of this PR, these might be related to datastore cluster storage support. Can you please check and resolve ? cc @harikrishna-patnala
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-836530052
Packaging result: :heavy_multiplication_x: centos7 :heavy_multiplication_x: centos8 :heavy_multiplication_x: debian. SL-JID 552
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-827211957
<b>Trillian test result (tid-534)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39410 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t534-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_iso.py
Intermittent failure detected: /marvin/tests/smoke/test_primary_storage.py
Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
Intermittent failure detected: /marvin/tests/smoke/test_usage.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Smoke tests completed. 84 look OK, 5 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_deploy_vms_storage_tags | `Error` | 6.49 | test_primary_storage.py
test_02_list_snapshots_with_removed_data_store | `Error` | 6.65 | test_snapshots.py
test_01_volume_usage | `Error` | 83.17 | test_usage.py
test_02_migrate_VM_with_two_data_disks | `Error` | 50.94 | test_vm_life_cycle.py
test_03_migrate_detached_volume | `Error` | 54.89 | test_vm_life_cycle.py
test_11_destroy_vm_and_volumes | `Error` | 46.55 | test_vm_life_cycle.py
test_01_create_volume | `Error` | 4.26 | test_volumes.py
test_02_attach_volume | `Error` | 2.08 | test_volumes.py
test_03_download_attached_volume | `Error` | 2.08 | test_volumes.py
test_04_delete_attached_volume | `Error` | 2.08 | test_volumes.py
test_05_detach_volume | `Error` | 2.08 | test_volumes.py
test_06_download_detached_volume | `Error` | 2.08 | test_volumes.py
test_07_resize_fail | `Error` | 2.16 | test_volumes.py
test_08_resize_volume | `Error` | 2.08 | test_volumes.py
test_09_delete_detached_volume | `Error` | 2.13 | test_volumes.py
test_11_migrate_volume_and_change_offering | `Error` | 2.19 | test_volumes.py
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-778248285
@blueorangutan test centos7 vmware-67u3
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-836644178
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569174929
##########
File path: api/src/main/java/com/cloud/storage/StorageService.java
##########
@@ -104,4 +104,6 @@
ImageStore updateImageStoreStatus(Long id, Boolean readonly);
+ void syncHardwareCapability(long poolId);
Review comment:
use generic method here, instead hard coded to specific capability
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r571963872
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -5244,6 +5246,16 @@ protected Answer execute(ModifyStoragePoolCommand cmd) {
answer.setLocalDatastoreName(morDatastore.getValue());
}
+ HostMO host = (HostMO) hyperHost;
+ boolean hardwareAccelerationSupportForDataStore = getHardwareAcceleationSupportForDataStore(host.getMor(), dsMo.getName());
+ StoragePoolInfo poolInfo = answer.getPoolInfo();
+ Map<String, String> poolDetails = poolInfo.getDetails();
+ if (poolDetails == null) {
+ poolDetails = new HashMap<>();
+ }
+ poolDetails.put("hardwareAccelerationSupported", String.valueOf(hardwareAccelerationSupportForDataStore));
+ poolInfo.setDetails(poolDetails);
Review comment:
I have removed using the ModifyStoragePool command
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r575056709
##########
File path: engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
##########
@@ -210,18 +213,19 @@ protected Answer copyObject(DataObject srcData, DataObject destData, Host destHo
* @param dataTO Dest data store TO
* @return dataTO including fullCloneFlag, if provided
*/
- protected DataTO addFullCloneFlagOnVMwareDest(DataTO dataTO) {
+ protected DataTO addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(DataTO dataTO) {
if (dataTO != null && dataTO.getHypervisorType().equals(Hypervisor.HypervisorType.VMware)){
DataStoreTO dataStoreTO = dataTO.getDataStore();
if (dataStoreTO != null && dataStoreTO instanceof PrimaryDataStoreTO){
PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) dataStoreTO;
- Boolean value = CapacityManager.VmwareCreateCloneFull.valueIn(primaryDataStoreTO.getId());
- primaryDataStoreTO.setFullCloneFlag(value);
+ primaryDataStoreTO.setFullCloneFlag(CapacityManager.VmwareCreateCloneFull.valueIn(primaryDataStoreTO.getId()));
+ primaryDataStoreTO.setDiskProvisioningStrictnessFlag(storageManager.DiskProvisioningStrictness.valueIn(primaryDataStoreTO.getId()));
}
Review comment:
Done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-851957221
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-877969134
@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez closed pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
nvazquez closed pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-775066243
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-836644177
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-837927598
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan removed a comment on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan removed a comment on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-771562576
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-775127897
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-775065643
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-771542796
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-777328804
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2676
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-828353099
@blueorangutan test centos7 vmware-67u3
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-874677735
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-826596884
@blueorangutan test centos7 vmware-67u3
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569190531
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java
##########
@@ -794,6 +798,33 @@ public boolean createFullClone(String cloneName, ManagedObjectReference morFolde
return false;
}
+ private void setDiskProvsioningType(VirtualMachineRelocateSpec relocSpec, ManagedObjectReference morDs,
+ Storage.ProvisioningType diskProvisioningType) throws Exception {
+ if (diskProvisioningType == null){
+ return;
+ }
+ List<VirtualMachineRelocateSpecDiskLocator> relocateDisks = relocSpec.getDisk();
+ List<VirtualDisk> disks = this.getVirtualDisks();
+ for (VirtualDisk disk: disks){
Review comment:
in any case, does the VM have multiple disks here ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-827268943
<b>Trillian test result (tid-537)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43245 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t537-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_primary_storage.py
Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
Intermittent failure detected: /marvin/tests/smoke/test_templates.py
Intermittent failure detected: /marvin/tests/smoke/test_usage.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Smoke tests completed. 83 look OK, 6 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_deploy_vms_storage_tags | `Error` | 7.50 | test_primary_storage.py
test_02_list_snapshots_with_removed_data_store | `Error` | 9.79 | test_snapshots.py
test_01_create_template | `Error` | 49.18 | test_templates.py
test_CreateTemplateWithDuplicateName | `Error` | 87.97 | test_templates.py
test_02_create_template_with_checksum_sha1 | `Error` | 65.38 | test_templates.py
test_03_create_template_with_checksum_sha256 | `Error` | 65.39 | test_templates.py
test_04_create_template_with_checksum_md5 | `Error` | 65.40 | test_templates.py
test_05_create_template_with_no_checksum | `Error` | 65.41 | test_templates.py
test_09_list_templates_download_details | `Failure` | 0.05 | test_templates.py
test_01_volume_usage | `Error` | 85.40 | test_usage.py
test_02_migrate_VM_with_two_data_disks | `Error` | 56.06 | test_vm_life_cycle.py
test_03_migrate_detached_volume | `Error` | 54.01 | test_vm_life_cycle.py
test_11_destroy_vm_and_volumes | `Error` | 47.64 | test_vm_life_cycle.py
test_01_create_volume | `Error` | 4.28 | test_volumes.py
test_02_attach_volume | `Error` | 1.07 | test_volumes.py
test_03_download_attached_volume | `Error` | 2.08 | test_volumes.py
test_04_delete_attached_volume | `Error` | 2.08 | test_volumes.py
test_05_detach_volume | `Error` | 2.08 | test_volumes.py
test_06_download_detached_volume | `Error` | 2.08 | test_volumes.py
test_07_resize_fail | `Error` | 3.21 | test_volumes.py
test_08_resize_volume | `Error` | 2.09 | test_volumes.py
test_09_delete_detached_volume | `Error` | 2.15 | test_volumes.py
test_11_migrate_volume_and_change_offering | `Error` | 3.20 | test_volumes.py
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-775066769
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-775154550
@blueorangutan test centos7 vmware-67u3
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r571963127
##########
File path: api/src/main/java/com/cloud/storage/StorageService.java
##########
@@ -104,4 +104,6 @@
ImageStore updateImageStoreStatus(Long id, Boolean readonly);
+ void syncHardwareCapability(long poolId);
Review comment:
Done
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
##########
@@ -207,6 +215,13 @@ protected boolean filter(ExcludeList avoid, StoragePool pool, DiskProfile dskCh,
return false;
}
+ if (diskProvisioningTypeStrictness){
+ StoragePoolDetailVO hardwareAcceleration = storagePoolDetailsDao.findDetail(pool.getId(), "hardwareAccelerationSupported");
Review comment:
Done
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
##########
@@ -207,6 +215,13 @@ protected boolean filter(ExcludeList avoid, StoragePool pool, DiskProfile dskCh,
return false;
}
+ if (diskProvisioningTypeStrictness){
+ StoragePoolDetailVO hardwareAcceleration = storagePoolDetailsDao.findDetail(pool.getId(), "hardwareAccelerationSupported");
+ if (!dskCh.getProvisioningType().equals("thin") && hardwareAcceleration == null || !hardwareAcceleration.getValue().equals("true")){
Review comment:
Done
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -5260,6 +5272,17 @@ protected Answer execute(ModifyStoragePoolCommand cmd) {
}
}
+ private boolean getHardwareAcceleationSupportForDataStore(ManagedObjectReference host, String dataStoreName) throws Exception {
Review comment:
Done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-778543810
<b>Trillian test result (tid-3536)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37568 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t3536-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Smoke tests completed. 86 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 773.61 | test_kubernetes_clusters.py
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-778247431
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2695
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-788214742
<b>Trillian test result (tid-3626)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40336 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t3626-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_primary_storage.py
Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
Intermittent failure detected: /marvin/tests/smoke/test_usage.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Smoke tests completed. 81 look OK, 6 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_deploy_vms_storage_tags | `Error` | 6.44 | test_primary_storage.py
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 770.35 | test_kubernetes_clusters.py
test_02_list_snapshots_with_removed_data_store | `Error` | 6.63 | test_snapshots.py
test_01_volume_usage | `Error` | 81.08 | test_usage.py
test_02_migrate_VM_with_two_data_disks | `Error` | 49.88 | test_vm_life_cycle.py
test_03_migrate_detached_volume | `Error` | 49.75 | test_vm_life_cycle.py
ContextSuite context=TestVAppsVM>:setup | `Error` | 44.17 | test_vm_life_cycle.py
test_11_destroy_vm_and_volumes | `Error` | 46.54 | test_vm_life_cycle.py
test_01_create_volume | `Error` | 4.27 | test_volumes.py
test_02_attach_volume | `Error` | 2.07 | test_volumes.py
test_03_download_attached_volume | `Error` | 2.08 | test_volumes.py
test_04_delete_attached_volume | `Error` | 2.12 | test_volumes.py
test_05_detach_volume | `Error` | 1.05 | test_volumes.py
test_06_download_detached_volume | `Error` | 2.08 | test_volumes.py
test_07_resize_fail | `Error` | 2.14 | test_volumes.py
test_08_resize_volume | `Error` | 2.07 | test_volumes.py
test_09_delete_detached_volume | `Error` | 2.13 | test_volumes.py
test_11_migrate_volume_and_change_offering | `Error` | 2.16 | test_volumes.py
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-786576403
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569182576
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -5244,6 +5246,16 @@ protected Answer execute(ModifyStoragePoolCommand cmd) {
answer.setLocalDatastoreName(morDatastore.getValue());
}
+ HostMO host = (HostMO) hyperHost;
+ boolean hardwareAccelerationSupportForDataStore = getHardwareAcceleationSupportForDataStore(host.getMor(), dsMo.getName());
+ StoragePoolInfo poolInfo = answer.getPoolInfo();
+ Map<String, String> poolDetails = poolInfo.getDetails();
+ if (poolDetails == null) {
+ poolDetails = new HashMap<>();
+ }
+ poolDetails.put("hardwareAccelerationSupported", String.valueOf(hardwareAccelerationSupportForDataStore));
+ poolInfo.setDetails(poolDetails);
Review comment:
is it required to sent these details in ModifyStoragePoolAnswer ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569116418
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
##########
@@ -207,6 +215,13 @@ protected boolean filter(ExcludeList avoid, StoragePool pool, DiskProfile dskCh,
return false;
}
+ if (diskProvisioningTypeStrictness){
Review comment:
as mentioned above, please use direct configkey object with valueIn() method
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
##########
@@ -82,6 +86,10 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
if (allocationAlgorithm != null) {
this.allocationAlgorithm = allocationAlgorithm;
}
+ String strictness = configs.get("disk.provisioning.type.strictness");
Review comment:
"disk.provisioning.type.strictness" is defined as dynamic and zone level configuration parameter. Getting the value from 'configs' and using a variable in configure() method will not make it dynamic or read from specific zone. Please use ConfigKey object with valuein() method to get the value at the exact usage point.
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java
##########
@@ -0,0 +1,80 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+
+@APICommand(name = UpdateStorageCapabilitiesCmd.APINAME, description = "Syncs hardware acceleration capabilities of storage pools",
Review comment:
@Spaceman1984 Can you please help me in understanding why we need a new API to update hardware acceleration. In the implementation of this also we are finally using ModifyStoragePoolCommand.
I think keeping the storage pool in maintenance and mode and cancelling the maintenance mode will trigger ModifyStoragePool Command and that should be good enough to fetch hardware acceleration details if at all that is changed on datastore.
Please let me know if anything I'm missing here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-879030479
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-874757108
@blueorangutan test centos7 vmware-67u3
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-851957221
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r572761630
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -5260,6 +5269,66 @@ protected Answer execute(ModifyStoragePoolCommand cmd) {
}
}
+ protected Answer execute(GetStoragePoolCapabilitiesCommand cmd) {
+
+ try {
+
+ VmwareHypervisorHost hyperHost = getHyperHost(getServiceContext());
+
+ HostMO host = (HostMO) hyperHost;
+
+ StorageFilerTO pool = cmd.getPool();
+
+ ManagedObjectReference morDatastore = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, pool.getUuid());
+
+ if (morDatastore == null) {
+ morDatastore = hyperHost.mountDatastore((pool.getType() == StoragePoolType.VMFS || pool.getType() == StoragePoolType.PreSetup || pool.getType() == StoragePoolType.DatastoreCluster), pool.getHost(), pool.getPort(), pool.getPath(), pool.getUuid().replace("-", ""), true);
+ }
+
+ assert (morDatastore != null);
+
+ DatastoreMO dsMo = new DatastoreMO(getServiceContext(), morDatastore);
+
+ GetStoragePoolCapabilitiesAnswer answer = new GetStoragePoolCapabilitiesAnswer(cmd);
+
+ if (pool.getType() == StoragePoolType.NetworkFilesystem) {
Review comment:
Done
##########
File path: core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java
##########
@@ -0,0 +1,19 @@
+package com.cloud.agent.api;
+
+public class GetStoragePoolCapabilitiesAnswer extends Answer {
+ public GetStoragePoolCapabilitiesAnswer(GetStoragePoolCapabilitiesCommand cmd) {
+ super(cmd);
+
+ poolInfo = new StoragePoolInfo();
+ }
+
+ public StoragePoolInfo getPoolInfo() {
+ return poolInfo;
+ }
+
+ public void setPoolInfo(StoragePoolInfo poolInfo) {
+ this.poolInfo = poolInfo;
+ }
+
+ private StoragePoolInfo poolInfo;
Review comment:
Done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-864338490
<b>Trillian test result (tid-1000)</b>
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 48327 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t1000-xenserver-71.zip
Intermittent failure detected: /marvin/tests/smoke/test_nic.py
Intermittent failure detected: /marvin/tests/smoke/test_persistent_network.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_service_offerings.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermittent failure detected: /marvin/tests/smoke/test_host_maintenance.py
Smoke tests completed. 87 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true | `Failure` | 382.06 | test_routers_network_ops.py
test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false | `Error` | 82.72 | test_routers_network_ops.py
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL | `Failure` | 617.95 | test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers | `Failure` | 546.04 | test_vpc_redundant.py
test_05_rvpc_multi_tiers | `Failure` | 528.32 | test_vpc_redundant.py
test_05_rvpc_multi_tiers | `Error` | 528.33 | test_vpc_redundant.py
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-814807996
@blueorangutan package
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r572623170
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
##########
@@ -913,9 +914,12 @@ private String cloneVMforVvols(VmwareContext context, VmwareHypervisorHost hyper
if (volume.getVolumeType() == Volume.Type.DATADISK)
vmName = volume.getName();
if (!_fullCloneFlag) {
+ if (_diskProvisioningStrictness) {
+ throw new CloudRuntimeException("Unable to create linked clones with strict disk provisioning enabled");
Review comment:
is this applicable for thin and thick disk provisioning types?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-771609921
@blueorangutan test ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-771613912
@blueorangutan test centos7 ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r580140924
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -2331,6 +2338,80 @@ public ImageStore updateImageStoreStatus(Long id, Boolean readonly) {
return imageStoreVO;
}
+ @Override
+ public void updateStorageCapabilities(Long poolId, boolean failOnChecks) {
+ List<StoragePoolVO> pools = new ArrayList<>();
+ if (poolId == null) {
+ pools = _storagePoolDao.listByStatus(StoragePoolStatus.Up);
+ } else {
+ StoragePoolVO pool = _storagePoolDao.findById(poolId);
+
+ if (pool == null) {
+ throw new CloudRuntimeException("Primary storage not found for id: " + poolId);
+ }
+
+ pools.add(pool);
+ }
+
+ if (pools.size() == 0) {
+ throw new CloudRuntimeException("No storage pools found to update.");
+ }
+
+ for (StoragePoolVO pool: pools) {
+
+ // Only checking NFS for now - required for disk provisioning type support for vmware.
+ if (pool.getPoolType() != StoragePoolType.NetworkFilesystem) {
+ if (failOnChecks) {
Review comment:
@Spaceman1984 how about defining `failOnChecks`locally in this method and set to true when poolId is passed, instead method parameter.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-778248804
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 closed pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 closed pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] rhtyd commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-771540339
@Spaceman1984 can you kick pkging and testing, is this still in WIP - pl change it to draft. Thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-856310874
<b>Trillian test result (tid-885)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38196 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t885-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 89 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-878710802
@Spaceman1984 can you please investigate the test failures on VM migration, nics and LB (other failures are intermitent)
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-775155564
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r582897469
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
##########
@@ -179,9 +179,32 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
} else if(allocationAlgorithm.equals("firstfitleastconsumed")){
pools = reorderPoolsByCapacity(plan, pools);
}
+
+ // Hardware accelerated pools are preferred for thick disks
+ if (dskCh != null && !dskCh.getProvisioningType().equals(Storage.ProvisioningType.THIN) &&
+ !storageMgr.DiskProvisioningStrictness.valueIn(plan.getDataCenterId())) {
+ pools = reorderPoolsByDiskProvisioningType(pools, dskCh);
+ }
+
return pools;
}
+ private List<StoragePool> reorderPoolsByDiskProvisioningType(List<StoragePool> pools, DiskProfile dskCh) {
+ List<StoragePool> reorderedPools = new ArrayList<>();
+ for (StoragePool pool: pools) {
Review comment:
Yes, If thin disk provisioning is required, reordering is not required, all storage pools support thin disk provisioning.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r595946714
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java
##########
@@ -0,0 +1,84 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.ListResponse;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.log4j.Logger;
+
+@APICommand(name = UpdateStorageCapabilitiesCmd.APINAME, description = "Syncs capabilities of storage pools",
+ responseObject = StoragePoolResponse.class,
+ requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.16.0")
+public class UpdateStorageCapabilitiesCmd extends BaseCmd {
+ public static final String APINAME = "updateStorageCapabilities";
+ private static final Logger LOG = Logger.getLogger(UpdateImageStoreCmd.class.getName());
Review comment:
Thanks @harikrishna-patnala
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569166259
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java
##########
@@ -0,0 +1,80 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+
+@APICommand(name = UpdateStorageCapabilitiesCmd.APINAME, description = "Syncs hardware acceleration capabilities of storage pools",
Review comment:
keep the description generic, so that this can be extended for other capabilities later.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-880693697
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 556
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-826746029
Packaging result: :heavy_multiplication_x: centos7 :heavy_multiplication_x: centos8 :heavy_multiplication_x: debian. SL-JID 465
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r650788060
##########
File path: api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java
##########
@@ -94,7 +94,7 @@ public Long getPageSizeVal() {
if (pageSizeInt != null) {
defaultPageSize = pageSizeInt.longValue();
}
- if (defaultPageSize.longValue() == s_pageSizeUnlimited) {
+ if (defaultPageSize!= null && defaultPageSize.longValue() == s_pageSizeUnlimited) {
Review comment:
```suggestion
if (s_pageSizeUnlimited.equals(defaultPageSize) {
```
just being a pain here
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-828273053
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-778168034
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-874677576
@blueorangutan package
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r571967838
##########
File path: vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java
##########
@@ -781,6 +782,9 @@ public boolean createFullClone(String cloneName, ManagedObjectReference morFolde
relocSpec.setDatastore(morDs);
relocSpec.setPool(morResourcePool);
+
+ setDiskProvsioningType(relocSpec, morDs, diskProvisioningType);
+
Review comment:
Always thin
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-851982829
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 138
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-771616539
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-826580619
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r650803790
##########
File path: api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java
##########
@@ -94,7 +94,7 @@ public Long getPageSizeVal() {
if (pageSizeInt != null) {
defaultPageSize = pageSizeInt.longValue();
}
- if (defaultPageSize.longValue() == s_pageSizeUnlimited) {
+ if (defaultPageSize!= null && defaultPageSize.longValue() == s_pageSizeUnlimited) {
Review comment:
This is fine Daan, this code came in as part of some unrelated merge. Not sure if we need something like:
if (s_pageSizeUnlimited.equals(defaultPageSize.longValue()) {
##########
File path: api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java
##########
@@ -94,7 +94,7 @@ public Long getPageSizeVal() {
if (pageSizeInt != null) {
defaultPageSize = pageSizeInt.longValue();
}
- if (defaultPageSize.longValue() == s_pageSizeUnlimited) {
+ if (defaultPageSize!= null && defaultPageSize.longValue() == s_pageSizeUnlimited) {
Review comment:
This is fine @DaanHoogland, this code came in as part of some unrelated merge. Not sure if we need something like:
if (s_pageSizeUnlimited.equals(defaultPageSize.longValue()) {
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r571964256
##########
File path: server/src/main/java/com/cloud/server/ManagementServerImpl.java
##########
@@ -729,6 +730,7 @@
static final ConfigKey<Integer> vmPasswordLength = new ConfigKey<Integer>("Advanced", Integer.class, "vm.password.length", "6", "Specifies the length of a randomly generated password", false);
static final ConfigKey<Integer> sshKeyLength = new ConfigKey<Integer>("Advanced", Integer.class, "ssh.key.length", "2048", "Specifies custom SSH key length (bit)", true, ConfigKey.Scope.Global);
static final ConfigKey<Boolean> humanReadableSizes = new ConfigKey<Boolean>("Advanced", Boolean.class, "display.human.readable.sizes", "true", "Enables outputting human readable byte sizes to logs and usage records.", false, ConfigKey.Scope.Global);
+ static final ConfigKey<Boolean> diskProvisioningStrictness = new ConfigKey<Boolean>("Storage", Boolean.class, "disk.provisioning.type.strictness", "false", "Use storage pools only with supported disk provisioning types for disk/service offerings.", true, ConfigKey.Scope.Zone);
Review comment:
I tried to explain what provisioning strictness means. How can I make it clearer?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-786503049
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569179848
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
##########
@@ -207,6 +215,13 @@ protected boolean filter(ExcludeList avoid, StoragePool pool, DiskProfile dskCh,
return false;
}
+ if (diskProvisioningTypeStrictness){
+ StoragePoolDetailVO hardwareAcceleration = storagePoolDetailsDao.findDetail(pool.getId(), "hardwareAccelerationSupported");
+ if (!dskCh.getProvisioningType().equals("thin") && hardwareAcceleration == null || !hardwareAcceleration.getValue().equals("true")){
Review comment:
can you move this to a generic method, something like _supportsDiskProvisioingType_ , and better check for the hypervisor (as this is only for VMware) and other required checks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 closed pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 closed pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-856292793
<b>Trillian test result (tid-886)</b>
Environment: xenserver-72 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35711 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t886-xenserver-72.zip
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 89 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-853600010
@blueorangutan test matrix
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-771587296
Packaging result: ✔centos7 ✔centos8 ✖debian. JID-2634
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-878929777
@blueorangutan test centos7 vmware-67u3
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-881345155
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 568
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 closed pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 closed pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r572762204
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -2331,6 +2338,30 @@ public ImageStore updateImageStoreStatus(Long id, Boolean readonly) {
return imageStoreVO;
}
+ @Override
+ public void updateStorageCapabilities(long poolId) {
+ StoragePoolVO pool = _storagePoolDao.findById(poolId);
+ // find the host
+ List<StoragePoolHostVO> hosts = _storagePoolHostDao.listByPoolId(poolId);
Review comment:
Done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-828272514
@blueorangutan package
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez closed pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
nvazquez closed pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-878948886
<b>Trillian Build Failed (tid-1256)<b/>
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 removed a comment on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 removed a comment on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-775066769
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-878930337
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r571966803
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
##########
@@ -915,7 +915,7 @@ private String cloneVMforVvols(VmwareContext context, VmwareHypervisorHost hyper
if (!_fullCloneFlag) {
createVMLinkedClone(vmTemplate, dcMo, vmName, morDatastore, morPool);
Review comment:
I have added checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r572618295
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java
##########
@@ -5260,6 +5269,66 @@ protected Answer execute(ModifyStoragePoolCommand cmd) {
}
}
+ protected Answer execute(GetStoragePoolCapabilitiesCommand cmd) {
+
+ try {
+
+ VmwareHypervisorHost hyperHost = getHyperHost(getServiceContext());
+
+ HostMO host = (HostMO) hyperHost;
+
+ StorageFilerTO pool = cmd.getPool();
+
+ ManagedObjectReference morDatastore = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, pool.getUuid());
+
+ if (morDatastore == null) {
+ morDatastore = hyperHost.mountDatastore((pool.getType() == StoragePoolType.VMFS || pool.getType() == StoragePoolType.PreSetup || pool.getType() == StoragePoolType.DatastoreCluster), pool.getHost(), pool.getPort(), pool.getPath(), pool.getUuid().replace("-", ""), true);
+ }
+
+ assert (morDatastore != null);
+
+ DatastoreMO dsMo = new DatastoreMO(getServiceContext(), morDatastore);
+
+ GetStoragePoolCapabilitiesAnswer answer = new GetStoragePoolCapabilitiesAnswer(cmd);
+
+ if (pool.getType() == StoragePoolType.NetworkFilesystem) {
+ boolean hardwareAccelerationSupportForDataStore = getHardwareAccelerationSupportForDataStore(host.getMor(), dsMo.getName());
+ StoragePoolInfo poolInfo = answer.getPoolInfo();
+ Map<String, String> poolDetails = poolInfo.getDetails();
+ if (poolDetails == null) {
+ poolDetails = new HashMap<>();
Review comment:
this details should ok in response, as rest of the attributes in `poolInfo` are unused.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-881345671
@blueorangutan test centos7 vmware-67u3
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569167468
##########
File path: api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java
##########
@@ -0,0 +1,80 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.storage;
+
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.StoragePoolResponse;
+import org.apache.cloudstack.api.response.SuccessResponse;
+import org.apache.cloudstack.context.CallContext;
+
+@APICommand(name = UpdateStorageCapabilitiesCmd.APINAME, description = "Syncs hardware acceleration capabilities of storage pools",
+ responseObject = SuccessResponse.class,
+ requestHasSensitiveInfo = false, responseHasSensitiveInfo = false)
+public class UpdateStorageCapabilitiesCmd extends BaseCmd {
+ public static final String APINAME = "updateStorageCapabilities";
+
+ /////////////////////////////////////////////////////
+ //////////////// API parameters /////////////////////
+ /////////////////////////////////////////////////////
+
+ @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = StoragePoolResponse.class, required = true, description = "Storage pool id")
+ private Long poolId;
+
+ /////////////////////////////////////////////////////
+ /////////////////// Accessors ///////////////////////
+ /////////////////////////////////////////////////////
+
+ public Long getPoolId() {
+ return poolId;
+ }
+
+ public void setPoolId(Long poolId) {
+ this.poolId = poolId;
+ }
+
+ /////////////////////////////////////////////////////
+ /////////////// API Implementation///////////////////
+ /////////////////////////////////////////////////////
+
+
+ @Override
+ public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException {
+ _storageService.syncHardwareCapability(poolId);
Review comment:
also keep this method _syncHardwareCapability_ name generic, and can use a private method in service layer to update hardware acceleration capability ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-775849558
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 closed pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 closed pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] nvazquez commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-881147178
@Spaceman1984 is this PR ready for merge?
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r664228043
##########
File path: engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
##########
@@ -100,9 +110,48 @@ public boolean hostConnect(long hostId, long poolId) throws StorageConflictExcep
updateStoragePoolHostVOAndDetails(poolVO, hostId, mspAnswer);
if (pool.getPoolType() == Storage.StoragePoolType.DatastoreCluster) {
+ for (ModifyStoragePoolAnswer childDataStoreAnswer : ((ModifyStoragePoolAnswer) answer).getDatastoreClusterChildren()) {
+ StoragePoolInfo childStoragePoolInfo = childDataStoreAnswer.getPoolInfo();
+ StoragePoolVO dataStoreVO = primaryStoreDao.findPoolByUUID(childStoragePoolInfo.getUuid());
+ if (dataStoreVO != null) {
+ continue;
+ }
+ dataStoreVO = new StoragePoolVO();
+ dataStoreVO.setStorageProviderName(poolVO.getStorageProviderName());
+ dataStoreVO.setHostAddress(childStoragePoolInfo.getHost());
+ dataStoreVO.setPoolType(Storage.StoragePoolType.PreSetup);
+ dataStoreVO.setPath(childStoragePoolInfo.getHostPath());
+ dataStoreVO.setPort(poolVO.getPort());
+ dataStoreVO.setName(childStoragePoolInfo.getName());
+ dataStoreVO.setUuid(childStoragePoolInfo.getUuid());
+ dataStoreVO.setDataCenterId(poolVO.getDataCenterId());
+ dataStoreVO.setPodId(poolVO.getPodId());
+ dataStoreVO.setClusterId(poolVO.getClusterId());
+ dataStoreVO.setStatus(StoragePoolStatus.Up);
+ dataStoreVO.setUserInfo(poolVO.getUserInfo());
+ dataStoreVO.setManaged(poolVO.isManaged());
+ dataStoreVO.setCapacityIops(poolVO.getCapacityIops());
+ dataStoreVO.setCapacityBytes(childDataStoreAnswer.getPoolInfo().getCapacityBytes());
+ dataStoreVO.setUsedBytes(childDataStoreAnswer.getPoolInfo().getCapacityBytes() - childDataStoreAnswer.getPoolInfo().getAvailableBytes());
+ dataStoreVO.setHypervisor(poolVO.getHypervisor());
+ dataStoreVO.setScope(poolVO.getScope());
+ dataStoreVO.setParent(poolVO.getId());
Review comment:
@Spaceman1984 it seems the changes here are not part of this PR, these might be related to datastore cluster storage support. Can you please check and resolve ? cc @harikrishna-patnala
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r650803790
##########
File path: api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java
##########
@@ -94,7 +94,7 @@ public Long getPageSizeVal() {
if (pageSizeInt != null) {
defaultPageSize = pageSizeInt.longValue();
}
- if (defaultPageSize.longValue() == s_pageSizeUnlimited) {
+ if (defaultPageSize!= null && defaultPageSize.longValue() == s_pageSizeUnlimited) {
Review comment:
This is fine Daan, this code came in as part of some unrelated merge.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-775581471
<b>Trillian test result (tid-3511)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40717 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t3511-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Smoke tests completed. 86 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 779.78 | test_kubernetes_clusters.py
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-775876857
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2663
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-828802381
<b>Trillian test result (tid-551)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38734 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t551-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
Smoke tests completed. 88 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
ContextSuite context=TestScaleVm>:setup | `Error` | 0.00 | test_scale_vm.py
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-827209803
<b>Trillian test result (tid-536)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 32842 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t536-kvm-centos7.zip
Smoke tests completed. 89 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r572623700
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
##########
@@ -929,21 +933,24 @@ private String cloneVMforVvols(VmwareContext context, VmwareHypervisorHost hyper
return vmdkFileBaseName;
}
- private String createVMFolderWithVMName(VmwareContext context, VmwareHypervisorHost hyperHost, TemplateObjectTO template,
- VirtualMachineMO vmTemplate, VolumeObjectTO volume, DatacenterMO dcMo, DatastoreMO dsMo,
- String searchExcludedFolders) throws Exception {
+ private String createVMAndFolderWithVMName(VmwareContext context, VmwareHypervisorHost hyperHost, TemplateObjectTO template,
+ VirtualMachineMO vmTemplate, VolumeObjectTO volume, DatacenterMO dcMo, DatastoreMO dsMo,
+ String searchExcludedFolders) throws Exception {
String vmdkName = volume.getName();
try {
ManagedObjectReference morDatastore = dsMo.getMor();
ManagedObjectReference morPool = hyperHost.getHyperHostOwnerResourcePool();
ManagedObjectReference morCluster = hyperHost.getHyperHostCluster();
- if (template.getSize() != null){
+ if (template.getSize() != null) {
_fullCloneFlag = volume.getSize() > template.getSize() ? true : _fullCloneFlag;
}
if (!_fullCloneFlag) {
+ if (_diskProvisioningStrictness) {
+ throw new CloudRuntimeException("Unable to create linked clones with strict disk provisioning enabled");
Review comment:
will fail here for both thin and thick disk provisioning, please check.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r582812933
##########
File path: engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java
##########
@@ -179,9 +179,32 @@ public boolean configure(String name, Map<String, Object> params) throws Configu
} else if(allocationAlgorithm.equals("firstfitleastconsumed")){
pools = reorderPoolsByCapacity(plan, pools);
}
+
+ // Hardware accelerated pools are preferred for thick disks
+ if (dskCh != null && !dskCh.getProvisioningType().equals(Storage.ProvisioningType.THIN) &&
+ !storageMgr.DiskProvisioningStrictness.valueIn(plan.getDataCenterId())) {
+ pools = reorderPoolsByDiskProvisioningType(pools, dskCh);
+ }
+
return pools;
}
+ private List<StoragePool> reorderPoolsByDiskProvisioningType(List<StoragePool> pools, DiskProfile dskCh) {
+ List<StoragePool> reorderedPools = new ArrayList<>();
+ for (StoragePool pool: pools) {
Review comment:
re-ordering is strictly for thick disk provision type here, i think it should be based on the provisioning type in the disk profile?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-826595725
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 461
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569195743
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
##########
@@ -915,7 +915,7 @@ private String cloneVMforVvols(VmwareContext context, VmwareHypervisorHost hyper
if (!_fullCloneFlag) {
createVMLinkedClone(vmTemplate, dcMo, vmName, morDatastore, morPool);
Review comment:
"disk provisioning type" not considered in case of linked clone ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-775127477
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 closed pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 closed pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-814857073
Packaging result: :heavy_check_mark: centos7 :heavy_check_mark: centos8 :heavy_check_mark: debian. SL-JID 349
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-778177230
Packaging result: ✖centos7 ✖centos8 ✖debian. JID-2693
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-777295981
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-786535775
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-775850400
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-826799336
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DaanHoogland commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-853600010
@blueorangutan test matrix
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-778123001
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-859482260
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-827070602
<b>Trillian test result (tid-531)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38850 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t531-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_primary_storage.py
Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
Intermittent failure detected: /marvin/tests/smoke/test_usage.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Smoke tests completed. 84 look OK, 5 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_deploy_vms_storage_tags | `Error` | 7.52 | test_primary_storage.py
test_02_list_snapshots_with_removed_data_store | `Error` | 9.90 | test_snapshots.py
test_01_volume_usage | `Error` | 84.42 | test_usage.py
test_02_migrate_VM_with_two_data_disks | `Error` | 53.95 | test_vm_life_cycle.py
test_03_migrate_detached_volume | `Error` | 53.93 | test_vm_life_cycle.py
test_11_destroy_vm_and_volumes | `Error` | 46.59 | test_vm_life_cycle.py
test_01_create_volume | `Error` | 3.23 | test_volumes.py
test_02_attach_volume | `Error` | 2.08 | test_volumes.py
test_03_download_attached_volume | `Error` | 1.06 | test_volumes.py
test_04_delete_attached_volume | `Error` | 2.08 | test_volumes.py
test_05_detach_volume | `Error` | 2.08 | test_volumes.py
test_06_download_detached_volume | `Error` | 2.08 | test_volumes.py
test_07_resize_fail | `Error` | 2.16 | test_volumes.py
test_08_resize_volume | `Error` | 2.09 | test_volumes.py
test_09_delete_detached_volume | `Error` | 2.15 | test_volumes.py
test_10_list_volumes | `Failure` | 366.16 | test_volumes.py
test_11_migrate_volume_and_change_offering | `Error` | 3.21 | test_volumes.py
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r575128519
##########
File path: engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
##########
@@ -210,18 +213,19 @@ protected Answer copyObject(DataObject srcData, DataObject destData, Host destHo
* @param dataTO Dest data store TO
* @return dataTO including fullCloneFlag, if provided
*/
- protected DataTO addFullCloneFlagOnVMwareDest(DataTO dataTO) {
+ protected DataTO addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(DataTO dataTO) {
if (dataTO != null && dataTO.getHypervisorType().equals(Hypervisor.HypervisorType.VMware)){
DataStoreTO dataStoreTO = dataTO.getDataStore();
if (dataStoreTO != null && dataStoreTO instanceof PrimaryDataStoreTO){
PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) dataStoreTO;
- Boolean value = CapacityManager.VmwareCreateCloneFull.valueIn(primaryDataStoreTO.getId());
- primaryDataStoreTO.setFullCloneFlag(value);
+ primaryDataStoreTO.setFullCloneFlag(CapacityManager.VmwareCreateCloneFull.valueIn(primaryDataStoreTO.getId()));
+ primaryDataStoreTO.setDiskProvisioningStrictnessFlag(storageManager.DiskProvisioningStrictness.valueIn(primaryDataStoreTO.getId()));
}
Review comment:
Done
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-853600210
@DaanHoogland a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-772390552
> @Spaceman1984 May I know what happens if strictness is set to false and disk is provisioned on unsupported storage pool ?
If strictness is set to false, a thin disk will be created on the unsupported pool.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-777208135
<b>Trillian test result (tid-3529)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 44772 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t3529-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 85 look OK, 2 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 810.27 | test_kubernetes_clusters.py
test_05_rvpc_multi_tiers | `Failure` | 232.26 | test_vpc_redundant.py
test_05_rvpc_multi_tiers | `Error` | 263.19 | test_vpc_redundant.py
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r575018287
##########
File path: engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java
##########
@@ -210,18 +213,19 @@ protected Answer copyObject(DataObject srcData, DataObject destData, Host destHo
* @param dataTO Dest data store TO
* @return dataTO including fullCloneFlag, if provided
*/
- protected DataTO addFullCloneFlagOnVMwareDest(DataTO dataTO) {
+ protected DataTO addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(DataTO dataTO) {
if (dataTO != null && dataTO.getHypervisorType().equals(Hypervisor.HypervisorType.VMware)){
DataStoreTO dataStoreTO = dataTO.getDataStore();
if (dataStoreTO != null && dataStoreTO instanceof PrimaryDataStoreTO){
PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) dataStoreTO;
- Boolean value = CapacityManager.VmwareCreateCloneFull.valueIn(primaryDataStoreTO.getId());
- primaryDataStoreTO.setFullCloneFlag(value);
+ primaryDataStoreTO.setFullCloneFlag(CapacityManager.VmwareCreateCloneFull.valueIn(primaryDataStoreTO.getId()));
+ primaryDataStoreTO.setDiskProvisioningStrictnessFlag(storageManager.DiskProvisioningStrictness.valueIn(primaryDataStoreTO.getId()));
}
Review comment:
DiskProvisioningStrictness setting is of zone scope, please use zone id here, instead of primaryDataStoreTO.getId()
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-778226810
@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-771562103
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-778226040
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r580242683
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -2331,6 +2338,80 @@ public ImageStore updateImageStoreStatus(Long id, Boolean readonly) {
return imageStoreVO;
}
+ @Override
+ public void updateStorageCapabilities(Long poolId, boolean failOnChecks) {
+ List<StoragePoolVO> pools = new ArrayList<>();
+ if (poolId == null) {
+ pools = _storagePoolDao.listByStatus(StoragePoolStatus.Up);
+ } else {
+ StoragePoolVO pool = _storagePoolDao.findById(poolId);
+
+ if (pool == null) {
+ throw new CloudRuntimeException("Primary storage not found for id: " + poolId);
+ }
+
+ pools.add(pool);
+ }
+
+ if (pools.size() == 0) {
+ throw new CloudRuntimeException("No storage pools found to update.");
+ }
+
+ for (StoragePoolVO pool: pools) {
+
+ // Only checking NFS for now - required for disk provisioning type support for vmware.
+ if (pool.getPoolType() != StoragePoolType.NetworkFilesystem) {
+ if (failOnChecks) {
Review comment:
failOnChecks is not only dependent on the pool id being null or not, it is also dependent on where the method is being called from.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-826798744
@blueorangutan test centos7 vmware-67u3
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-778220630
@blueorangutan package
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] harikrishna-patnala closed pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
harikrishna-patnala closed pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-879045314
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 540
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-837927118
@blueorangutan test centos7 vmware-67u3
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-877968825
@blueorangutan test centos7 vmware-67u3
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-837256088
Packaging result: :heavy_check_mark: centos7 :heavy_multiplication_x: centos8 :heavy_check_mark: debian. SL-JID 562
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-786556632
Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2840
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r572624018
##########
File path: plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java
##########
@@ -997,9 +1004,12 @@ private void createLinkedOrFullClone(TemplateObjectTO template, VolumeObjectTO v
_fullCloneFlag = volume.getSize() > template.getSize() || _fullCloneFlag;
}
if (!_fullCloneFlag) {
+ if (_diskProvisioningStrictness) {
+ throw new CloudRuntimeException("Unable to create linked clones with strict disk provisioning enabled");
Review comment:
same as above.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] Spaceman1984 commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
Spaceman1984 commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-771542602
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-880694715
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] sureshanaparti commented on a change in pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#discussion_r569181159
##########
File path: engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java
##########
@@ -166,6 +166,13 @@ private void updateStoragePoolHostVOAndDetails(StoragePool pool, long hostId, Mo
storagePoolDetailsDao.persist(storagePoolDetailVO);
}
}
Review comment:
why on host connect again? as this is addressed / updated through API call.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-874757742
@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
--
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: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-788555413
<b>Trillian test result (tid-3630)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42485 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4640-t3630-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_primary_storage.py
Intermittent failure detected: /marvin/tests/smoke/test_snapshots.py
Intermittent failure detected: /marvin/tests/smoke/test_usage.py
Intermittent failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermittent failure detected: /marvin/tests/smoke/test_volumes.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 80 look OK, 7 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_deploy_vms_storage_tags | `Error` | 6.46 | test_primary_storage.py
test_03_deploy_and_upgrade_kubernetes_cluster | `Failure` | 777.56 | test_kubernetes_clusters.py
test_02_list_snapshots_with_removed_data_store | `Error` | 6.56 | test_snapshots.py
test_01_volume_usage | `Error` | 82.22 | test_usage.py
test_02_migrate_VM_with_two_data_disks | `Error` | 49.86 | test_vm_life_cycle.py
test_03_migrate_detached_volume | `Error` | 49.78 | test_vm_life_cycle.py
ContextSuite context=TestVAppsVM>:setup | `Error` | 42.10 | test_vm_life_cycle.py
test_11_destroy_vm_and_volumes | `Error` | 42.45 | test_vm_life_cycle.py
test_01_create_volume | `Error` | 3.22 | test_volumes.py
test_02_attach_volume | `Error` | 3.09 | test_volumes.py
test_03_download_attached_volume | `Error` | 1.06 | test_volumes.py
test_04_delete_attached_volume | `Error` | 2.08 | test_volumes.py
test_05_detach_volume | `Error` | 2.06 | test_volumes.py
test_06_download_detached_volume | `Error` | 2.08 | test_volumes.py
test_07_resize_fail | `Error` | 2.15 | test_volumes.py
test_08_resize_volume | `Error` | 2.07 | test_volumes.py
test_09_delete_detached_volume | `Error` | 2.13 | test_volumes.py
test_11_migrate_volume_and_change_offering | `Error` | 3.17 | test_volumes.py
test_01_redundant_vpc_site2site_vpn | `Failure` | 329.23 | test_vpc_vpn.py
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] blueorangutan commented on pull request #4640: Added disk provisioning type support for VMWare
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4640:
URL: https://github.com/apache/cloudstack/pull/4640#issuecomment-786530009
Packaging result: ✖centos7 ✔centos8 ✔debian. JID-2838
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org