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/09/23 07:29:16 UTC

[GitHub] [cloudstack] weizhouapache opened a new pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

weizhouapache opened a new pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501


   ### Description
   
   This PR fixes #4406 
   
   steps to reproduce the issue
   (1) update storage pool tags, (pool1 has tag 'tag1', pool2 has tag 'tag2')
   (2) create service offerings (offering-tag1 has storage tag 'tag1', offering-tag2 has storage tag 'tag2')
   (3) create a vm with service offering 'offering-tag2'. the disk will be allocated on pool2 (with tag 'tag2')
   (4) put pool2 to maintenance , vm will be stopped
   (5) start vm
   
   Expected result (With this PR: works as expected)
   vm cannot be restarted as there is no storage pool available for ROOT disk (because vm use service offering with storage tag 'tag2')
   
   Actual result (without this PR):
   vm is started and volume are reallocated to pool1.
   
   
   
   <!--- 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)
   - [ ] New feature (non-breaking change which adds functionality)
   - [x] 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
   
   - [ ] Major
   - [ ] Minor
   
   #### Bug Severity
   
   - [ ] BLOCKER
   - [ ] Critical
   - [x] Major
   - [ ] Minor
   - [ ] Trivial
   
   
   ### 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. -->
   
   
   <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/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.

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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-932636602


   <b>Trillian test result (tid-2278)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 36153 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5501-t2278-kvm-centos7.zip
   Smoke tests completed. 86 look OK, 4 have errors
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   test_01_add_primary_storage_disabled_host | `Error` | 0.62 | test_primary_storage.py
   test_01_primary_storage_nfs | `Error` | 0.12 | test_primary_storage.py
   ContextSuite context=TestStorageTags>:setup | `Error` | 0.22 | test_primary_storage.py
   test_01_secure_vm_migration | `Error` | 155.45 | test_vm_life_cycle.py
   test_02_unsecure_vm_migration | `Error` | 272.30 | test_vm_life_cycle.py
   test_03_secured_to_nonsecured_vm_migration | `Error` | 149.10 | test_vm_life_cycle.py
   test_08_migrate_vm | `Error` | 43.89 | test_vm_life_cycle.py
   test_02_list_snapshots_with_removed_data_store | `Error` | 9.56 | test_snapshots.py
   test_02_list_snapshots_with_removed_data_store | `Error` | 9.57 | test_snapshots.py
   test_hostha_enable_ha_when_host_in_maintenance | `Error` | 303.77 | test_hostha_kvm.py
   test_hostha_kvm_host_recovering | `Error` | 8.16 | test_hostha_kvm.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] sureshanaparti commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r718283317



##########
File path: server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
##########
@@ -1558,7 +1558,8 @@ protected boolean hostCanAccessSPool(Host host, StoragePool pool) {
 
             DiskOfferingVO diskOffering = _diskOfferingDao.findById(toBeCreated.getDiskOfferingId());
 
-            if (vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO && vmProfile.getServiceOffering().getTagsArray().length != 0) {
+            if ((vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO || toBeCreated.getVolumeType() == Volume.Type.ROOT)
+                    && vmProfile.getServiceOffering().getTagsArray().length != 0) {

Review comment:
       @weizhouapache first scenario - when VM is deployed from ISO, what is the other scenario ?
   
   Does attach data disk as root disk supported?




-- 
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] shwstppr commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r717370098



##########
File path: engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
##########
@@ -217,6 +218,15 @@ public VolumeVO findByPoolIdName(long poolId, String name) {
         return listBy(sc);
     }
 
+    @Override
+    public List<VolumeVO> findReadyAndAllocatedRootVolumesByInstance(long instanceId) {

Review comment:
       @DaanHoogland not sure but don't we have multi-disk OVA in VMware? Is only a single volume considered ROOT in that case?




-- 
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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r718283317



##########
File path: server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
##########
@@ -1558,7 +1558,8 @@ protected boolean hostCanAccessSPool(Host host, StoragePool pool) {
 
             DiskOfferingVO diskOffering = _diskOfferingDao.findById(toBeCreated.getDiskOfferingId());
 
-            if (vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO && vmProfile.getServiceOffering().getTagsArray().length != 0) {
+            if ((vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO || toBeCreated.getVolumeType() == Volume.Type.ROOT)
+                    && vmProfile.getServiceOffering().getTagsArray().length != 0) {

Review comment:
       @weizhouapache first scenario - when VM is deployed from ISO, what is the other scenario ?
   
   Does attach data disk as root disk supported?

##########
File path: engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
##########
@@ -365,6 +375,12 @@ public VolumeDaoImpl() {
         AllFieldsSearch.and("name", AllFieldsSearch.entity().getName(), Op.EQ);
         AllFieldsSearch.done();
 
+        RootDiskStateSearch = createSearchBuilder();

Review comment:
       May be you can re-use _AllFieldsSearch_, by changing Op.EQ to Op.IN here: https://github.com/apache/cloudstack/blob/67bc74de56357eeefdec9cccb6314d73dc757f5d/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java#L362




-- 
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] weizhouapache closed pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501


   


-- 
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 commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r718003828



##########
File path: engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
##########
@@ -217,6 +218,15 @@ public VolumeVO findByPoolIdName(long poolId, String name) {
         return listBy(sc);
     }
 
+    @Override
+    public List<VolumeVO> findReadyAndAllocatedRootVolumesByInstance(long instanceId) {

Review comment:
       Yes, it is possible to have multiple ROOT disks for deploy-as-is templates on Vmware. However, I'm not seeing a problem as the method is returning a list of volumes




-- 
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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-932896080


   @rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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] weizhouapache commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r718308595



##########
File path: server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
##########
@@ -1558,7 +1558,8 @@ protected boolean hostCanAccessSPool(Host host, StoragePool pool) {
 
             DiskOfferingVO diskOffering = _diskOfferingDao.findById(toBeCreated.getDiskOfferingId());
 
-            if (vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO && vmProfile.getServiceOffering().getTagsArray().length != 0) {
+            if ((vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO || toBeCreated.getVolumeType() == Volume.Type.ROOT)
+                    && vmProfile.getServiceOffering().getTagsArray().length != 0) {

Review comment:
       @sureshanaparti 
   as I said in the description
   
   ```
   In some cases, the disk offering of ROOT disk is not same as service offering of VM.
   for example
   (1) create a vm but not start it, then scale vm to another service offering
   (2) reinstall vm, then scale vm to another service offering
   
   These two issues are fixed by this PR.
   The issue might exist in some other scenarios, so it would be better to always update ROOT disk offering tags to the tags of vm service offering in DeploymentPlanningManagerImpl.java.
   ```




-- 
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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-932191776


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 1475


-- 
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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-929029446


   @weizhouapache a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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] weizhouapache commented on pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-929029120


   @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.

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 pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-932716356


   @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.

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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r718291297



##########
File path: engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
##########
@@ -365,6 +375,12 @@ public VolumeDaoImpl() {
         AllFieldsSearch.and("name", AllFieldsSearch.entity().getName(), Op.EQ);
         AllFieldsSearch.done();
 
+        RootDiskStateSearch = createSearchBuilder();

Review comment:
       May be you can re-use _AllFieldsSearch_, by changing Op.EQ to Op.IN here: https://github.com/apache/cloudstack/blob/67bc74de56357eeefdec9cccb6314d73dc757f5d/engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java#L362




-- 
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] DaanHoogland commented on pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-928991500


   @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] sureshanaparti commented on pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-932220806


   @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.

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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-932177215


   @weizhouapache 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] weizhouapache commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r718307096



##########
File path: engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
##########
@@ -365,6 +375,12 @@ public VolumeDaoImpl() {
         AllFieldsSearch.and("name", AllFieldsSearch.entity().getName(), Op.EQ);
         AllFieldsSearch.done();
 
+        RootDiskStateSearch = createSearchBuilder();

Review comment:
       @sureshanaparti I was thinking about it. but at the end I decided to add a new searchbuilder.
   no idea which way is suggested.




-- 
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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-933008281


   <b>Trillian test result (tid-2302)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 33829 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5501-t2302-kvm-centos7.zip
   Smoke tests completed. 90 look OK, 0 have errors
   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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-926369319


   @davidjumani 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] sureshanaparti commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r719135445



##########
File path: engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
##########
@@ -365,6 +375,12 @@ public VolumeDaoImpl() {
         AllFieldsSearch.and("name", AllFieldsSearch.entity().getName(), Op.EQ);
         AllFieldsSearch.done();
 
+        RootDiskStateSearch = createSearchBuilder();

Review comment:
       @weizhouapache suggested to re-use _AllFieldsSearch_ , but it is ok with new searchbuilder as well.




-- 
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] weizhouapache commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r716708470



##########
File path: server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
##########
@@ -1558,7 +1558,8 @@ protected boolean hostCanAccessSPool(Host host, StoragePool pool) {
 
             DiskOfferingVO diskOffering = _diskOfferingDao.findById(toBeCreated.getDiskOfferingId());
 
-            if (vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO && vmProfile.getServiceOffering().getTagsArray().length != 0) {
+            if ((vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO || toBeCreated.getVolumeType() == Volume.Type.ROOT)
+                    && vmProfile.getServiceOffering().getTagsArray().length != 0) {

Review comment:
       @sureshanaparti @davidjumani 
   found two scenarios that root disk offering is not same as vm service offering.
   there might be more scenarios (for example, attach data disk as root disk ?).
   
   please review




-- 
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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-926379957


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 1386


-- 
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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
nvazquez merged pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501


   


-- 
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] DaanHoogland commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r717364990



##########
File path: engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
##########
@@ -217,6 +218,15 @@ public VolumeVO findByPoolIdName(long poolId, String name) {
         return listBy(sc);
     }
 
+    @Override
+    public List<VolumeVO> findReadyAndAllocatedRootVolumesByInstance(long instanceId) {

Review comment:
       nitpick: `findReadyAndAllocatedRootVolumeByInstance` (singular) we are only looking for one volume, can we encounter more?




-- 
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] weizhouapache closed pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501


   


-- 
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] weizhouapache closed pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501


   


-- 
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] weizhouapache commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r717371907



##########
File path: engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
##########
@@ -217,6 +218,15 @@ public VolumeVO findByPoolIdName(long poolId, String name) {
         return listBy(sc);
     }
 
+    @Override
+    public List<VolumeVO> findReadyAndAllocatedRootVolumesByInstance(long instanceId) {

Review comment:
       @DaanHoogland for kvm, there is only 1 root disk. 
   for other hypervisors eg vmware, I have no idea. 
   so I'd like to use similar code as findReadyRootVolumesByInstance which returns a list of root volumes and processed in other java files.




-- 
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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-933006459


   <b>Trillian test result (tid-2300)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 33338 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5501-t2300-kvm-centos7.zip
   Smoke tests completed. 90 look OK, 0 have errors
   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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-928992080


   @DaanHoogland 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] sureshanaparti commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r715564711



##########
File path: server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
##########
@@ -1558,7 +1558,8 @@ protected boolean hostCanAccessSPool(Host host, StoragePool pool) {
 
             DiskOfferingVO diskOffering = _diskOfferingDao.findById(toBeCreated.getDiskOfferingId());
 
-            if (vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO && vmProfile.getServiceOffering().getTagsArray().length != 0) {
+            if ((vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO || toBeCreated.getVolumeType() == Volume.Type.ROOT)
+                    && vmProfile.getServiceOffering().getTagsArray().length != 0) {

Review comment:
       For ROOT volume from template, the disk / service offering storage tags are the same. Is it required to explicitly set the disk offering tags? 




-- 
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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-932716521


   @sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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] weizhouapache commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r715570210



##########
File path: server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
##########
@@ -1558,7 +1558,8 @@ protected boolean hostCanAccessSPool(Host host, StoragePool pool) {
 
             DiskOfferingVO diskOffering = _diskOfferingDao.findById(toBeCreated.getDiskOfferingId());
 
-            if (vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO && vmProfile.getServiceOffering().getTagsArray().length != 0) {
+            if ((vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO || toBeCreated.getVolumeType() == Volume.Type.ROOT)
+                    && vmProfile.getServiceOffering().getTagsArray().length != 0) {

Review comment:
       @sureshanaparti 
   yes, you are right. it is not required. this is why I converted this PR to draft today.
   
   I thought I have reproduced the issue two days before. but today I am not able to reproduce.
   I am still investigating the issue #4406 
   When I found some clue I will update this PR




-- 
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] shwstppr commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
shwstppr commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r717370098



##########
File path: engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
##########
@@ -217,6 +218,15 @@ public VolumeVO findByPoolIdName(long poolId, String name) {
         return listBy(sc);
     }
 
+    @Override
+    public List<VolumeVO> findReadyAndAllocatedRootVolumesByInstance(long instanceId) {

Review comment:
       @DaanHoogland not sure but don't we have multi-disk OVA in VMware? Is only a single volume considered ROOT in that case?




-- 
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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-928992080






-- 
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] weizhouapache commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r719147375



##########
File path: server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
##########
@@ -1558,7 +1558,8 @@ protected boolean hostCanAccessSPool(Host host, StoragePool pool) {
 
             DiskOfferingVO diskOffering = _diskOfferingDao.findById(toBeCreated.getDiskOfferingId());
 
-            if (vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO && vmProfile.getServiceOffering().getTagsArray().length != 0) {
+            if ((vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO || toBeCreated.getVolumeType() == Volume.Type.ROOT)
+                    && vmProfile.getServiceOffering().getTagsArray().length != 0) {

Review comment:
       @sureshanaparti 
   only the root volumes in Ready state are updated (root volumes are Allocated in these two scenarios).
   the issues are fixed in this PR.




-- 
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] DaanHoogland commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r717364990



##########
File path: engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
##########
@@ -217,6 +218,15 @@ public VolumeVO findByPoolIdName(long poolId, String name) {
         return listBy(sc);
     }
 
+    @Override
+    public List<VolumeVO> findReadyAndAllocatedRootVolumesByInstance(long instanceId) {

Review comment:
       nitpick: `findReadyAndAllocatedRootVolumeByInstance` (singular) we are only looking for one volume, can we encounter more?




-- 
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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r719138427



##########
File path: server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
##########
@@ -1558,7 +1558,8 @@ protected boolean hostCanAccessSPool(Host host, StoragePool pool) {
 
             DiskOfferingVO diskOffering = _diskOfferingDao.findById(toBeCreated.getDiskOfferingId());
 
-            if (vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO && vmProfile.getServiceOffering().getTagsArray().length != 0) {
+            if ((vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO || toBeCreated.getVolumeType() == Volume.Type.ROOT)
+                    && vmProfile.getServiceOffering().getTagsArray().length != 0) {

Review comment:
       @weizhouapache in both these scenarios, does the ROOT disk offering in volumes table updated with the new service offering?




-- 
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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-932222332


   @sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) 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] weizhouapache commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r715570210



##########
File path: server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
##########
@@ -1558,7 +1558,8 @@ protected boolean hostCanAccessSPool(Host host, StoragePool pool) {
 
             DiskOfferingVO diskOffering = _diskOfferingDao.findById(toBeCreated.getDiskOfferingId());
 
-            if (vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO && vmProfile.getServiceOffering().getTagsArray().length != 0) {
+            if ((vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO || toBeCreated.getVolumeType() == Volume.Type.ROOT)
+                    && vmProfile.getServiceOffering().getTagsArray().length != 0) {

Review comment:
       @sureshanaparti 
   yes, you are right. it is not required. this is why I converted this PR to draft today.
   
   I thought I have reproduced the issue two days before. but today I am not able to reproduce.
   I am still investigating the issue #4406 
   When I find some clue I will update and reopen this PR




-- 
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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-932816808


   <b>Trillian test result (tid-2283)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 40643 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5501-t2283-kvm-centos7.zip
   Smoke tests completed. 88 look OK, 2 have errors
   Only failed tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   ContextSuite context=TestKubernetesCluster>:teardown | `Error` | 84.12 | test_kubernetes_clusters.py
   test_disable_oobm_ha_state_ineligible | `Error` | 1512.68 | test_hostha_kvm.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] weizhouapache commented on pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-932176880


   @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] weizhouapache closed pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501


   


-- 
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] weizhouapache commented on pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-929029120


   @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.

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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-929520895


   <b>Trillian test result (tid-2228)</b>
   Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
   Total time taken: 30502 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5501-t2228-kvm-centos7.zip
   Smoke tests completed. 87 look OK, 0 have errors
   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] weizhouapache commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r717371907



##########
File path: engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
##########
@@ -217,6 +218,15 @@ public VolumeVO findByPoolIdName(long poolId, String name) {
         return listBy(sc);
     }
 
+    @Override
+    public List<VolumeVO> findReadyAndAllocatedRootVolumesByInstance(long instanceId) {

Review comment:
       @DaanHoogland for kvm, there is only 1 root disk. 
   for other hypervisors eg vmware, I have no idea. 
   so I'd like to use similar code as findReadyRootVolumesByInstance which returns a list of root volumes and processed in other java files.




-- 
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 commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r718003828



##########
File path: engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
##########
@@ -217,6 +218,15 @@ public VolumeVO findByPoolIdName(long poolId, String name) {
         return listBy(sc);
     }
 
+    @Override
+    public List<VolumeVO> findReadyAndAllocatedRootVolumesByInstance(long instanceId) {

Review comment:
       Yes, it is possible to have multiple ROOT disks for deploy-as-is templates on Vmware. However, I'm not seeing a problem as the method is returning a list of volumes




-- 
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] DaanHoogland commented on pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-928991500


   @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] weizhouapache closed pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache closed pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501


   


-- 
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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-929027697


   Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian. SL-JID 1417


-- 
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] weizhouapache commented on a change in pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#discussion_r718307096



##########
File path: engine/schema/src/main/java/com/cloud/storage/dao/VolumeDaoImpl.java
##########
@@ -365,6 +375,12 @@ public VolumeDaoImpl() {
         AllFieldsSearch.and("name", AllFieldsSearch.entity().getName(), Op.EQ);
         AllFieldsSearch.done();
 
+        RootDiskStateSearch = createSearchBuilder();

Review comment:
       @sureshanaparti I was thinking about it. but at the end I decided to add a new searchbuilder.
   no idea which way is suggested.

##########
File path: server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java
##########
@@ -1558,7 +1558,8 @@ protected boolean hostCanAccessSPool(Host host, StoragePool pool) {
 
             DiskOfferingVO diskOffering = _diskOfferingDao.findById(toBeCreated.getDiskOfferingId());
 
-            if (vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO && vmProfile.getServiceOffering().getTagsArray().length != 0) {
+            if ((vmProfile.getTemplate().getFormat() == Storage.ImageFormat.ISO || toBeCreated.getVolumeType() == Volume.Type.ROOT)
+                    && vmProfile.getServiceOffering().getTagsArray().length != 0) {

Review comment:
       @sureshanaparti 
   as I said in the description
   
   ```
   In some cases, the disk offering of ROOT disk is not same as service offering of VM.
   for example
   (1) create a vm but not start it, then scale vm to another service offering
   (2) reinstall vm, then scale vm to another service offering
   
   These two issues are fixed by this PR.
   The issue might exist in some other scenarios, so it would be better to always update ROOT disk offering tags to the tags of vm service offering in DeploymentPlanningManagerImpl.java.
   ```




-- 
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] davidjumani commented on pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
davidjumani commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-926368704


   @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] sureshanaparti commented on pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-932176706


   @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] sureshanaparti commented on pull request #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-932175488


   @weizhouapache can you re-target to 'main' please.


-- 
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 #5501: server: check service offering (storage) tags when reallocate a ROOT disk

Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5501:
URL: https://github.com/apache/cloudstack/pull/5501#issuecomment-932895570


   @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.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org