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 2022/03/25 11:31:12 UTC
[GitHub] [cloudstack] ravening opened a new pull request #6164: Mount disabled storage pool on host reboot
ravening opened a new pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164
### Description
Add a global setting so that disabled pools will be mounted
again on host reboot
<!--- 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)
- [ ] Bug fix (non-breaking change which fixes an issue)
- [X] 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
- [X] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [ ] Major
- [ ] Minor
- [X] 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. -->
1. Disable the storage pool
2. Set the global setting `mount.disabled.storage.pool` to true
3. The pool will still be mounted in the host.
4. Reboot the host.
5. The pool will still be mounted
6. Change the setting to false
7. Reboot the host
8. The disabled pool will not be mounted
<!-- 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] ravening commented on a change in pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r838234557
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
Review comment:
@weizhouapache @nvazquez added cluster wide support. please review again
--
This is an automated message from the 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 pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#issuecomment-1085944422
@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] nvazquez commented on a change in pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r835471732
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
Review comment:
Cluster-wide storage pools are not considered?
##########
File path: engine/components-api/src/main/java/com/cloud/storage/StorageManager.java
##########
@@ -155,6 +155,15 @@
ConfigKey<String> PreferredStoragePool = new ConfigKey<String>(String.class, "preferred.storage.pool", "Advanced", "",
"The UUID of preferred storage pool for allocation.", true, ConfigKey.Scope.Account, null);
+ ConfigKey<Boolean> MountDisabledStoragePool = new ConfigKey<>(Boolean.class,
+ "mount.disabled.storage.pool",
+ "Storage",
+ "false",
+ "Mount disabled storage pool after node reboot",
+ true,
+ ConfigKey.Scope.Cluster,
Review comment:
Shouldn't this scope be set to StoragePool? If not, please update the configuration description mentioning that it impacts all the storage pools in the cluster
--
This is an automated message from the 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 pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#issuecomment-1080952590
@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 commented on a change in pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r837437698
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
Review comment:
> @weizhouapache you are saying that, I need to make an extra call like this `pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), host.getClusterId(), null, ScopeType.CLUSTER));` ?
@ravening
yes exactly. it would be good to have it.
please also consider change it to a cluster level 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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r836128599
##########
File path: engine/components-api/src/main/java/com/cloud/storage/StorageManager.java
##########
@@ -155,6 +155,15 @@
ConfigKey<String> PreferredStoragePool = new ConfigKey<String>(String.class, "preferred.storage.pool", "Advanced", "",
"The UUID of preferred storage pool for allocation.", true, ConfigKey.Scope.Account, null);
+ ConfigKey<Boolean> MountDisabledStoragePool = new ConfigKey<>(Boolean.class,
+ "mount.disabled.storage.pool",
+ "Storage",
+ "false",
+ "Mount disabled storage pool after node reboot",
+ true,
+ ConfigKey.Scope.Cluster,
Review comment:
@nvazquez I have updated the description
--
This is an automated message from the 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 #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#issuecomment-1084148941
@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] weizhouapache commented on a change in pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r837255989
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
Review comment:
@ravening
ok. I am not quite clear what you want to fix/achieve. can you explain a bit more ?
anyway, the support of cluster-wide pools is an improvement of this pr. it means not only zone-wide pools , but also cluster-wide pools are mounted. any issue ?
--
This is an automated message from the 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] ravening commented on a change in pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r837315264
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
Review comment:
@weizhouapache when we set the status of the storage pool as disabled, it will not be used for vm allocation but it will be still mounted/accessible for the vm's. When the node reboots, all the disabled pools will not be mounted and as a result the volume of the vm will not be accessible.
My understanding of the cluster wide pool is that it will be mounted on the hosts within the same cluster right? The same pool will not be mounted on the nodes of different cluster.
Lets say we have three clusters. A, B and C.
I want the disabled pool to be mounted on all three clusters rather than just on A or B or C. Thats why I am searching for zone wide pools
--
This is an automated message from the 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 #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r838469552
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
+ }
+
for (StoragePoolVO pool : pools) {
- if (pool.getStatus() != StoragePoolStatus.Up) {
Review comment:
@nvazquez
this might not be needed, because `findZoneWideStoragePoolsByHypervisor` has the condition `sc.and(sc.entity().getStatus(), Op.EQ, Status.Up);`
--
This is an automated message from the 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] ravening commented on a change in pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r835498633
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
Review comment:
no. only zone wide is considered as a single pool should be available for all clusters
--
This is an automated message from the 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] ravening commented on a change in pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r835498166
##########
File path: engine/components-api/src/main/java/com/cloud/storage/StorageManager.java
##########
@@ -155,6 +155,15 @@
ConfigKey<String> PreferredStoragePool = new ConfigKey<String>(String.class, "preferred.storage.pool", "Advanced", "",
"The UUID of preferred storage pool for allocation.", true, ConfigKey.Scope.Account, null);
+ ConfigKey<Boolean> MountDisabledStoragePool = new ConfigKey<>(Boolean.class,
+ "mount.disabled.storage.pool",
+ "Storage",
+ "false",
+ "Mount disabled storage pool after node reboot",
+ true,
+ ConfigKey.Scope.Cluster,
Review comment:
This should be zone wide as vm can be migrated across cluster. I will update the description
--
This is an automated message from the 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 #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r837011510
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
Review comment:
@weizhouapache same comment from me here :) https://github.com/apache/cloudstack/pull/6164/files#r835471732
--
This is an automated message from the 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 #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#issuecomment-1083067154
@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 commented on a change in pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r836702454
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
Review comment:
@ravening
how about cluster wide storage pools ?
--
This is an automated message from the 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 #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#issuecomment-1083355137
@nvazquez 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] blueorangutan commented on pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#issuecomment-1084149633
@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] nvazquez commented on a change in pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r837479377
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
+ }
+
for (StoragePoolVO pool : pools) {
- if (pool.getStatus() != StoragePoolStatus.Up) {
Review comment:
Actually not only Up state since you have added the Disabled pools as well, actually my suggestion above is not correct, should be:
````
if (pool.getStatus() != StoragePoolStatus.Up || (StorageManager.MountDisabledStoragePool.value() && pool.getStatus() != StoragePoolStatus.Disabled && pool.getStatus() != StoragePoolStatus.Up)) {
continue;
}
````
--
This is an automated message from the 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 pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#issuecomment-1079238130
@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] nvazquez commented on pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#issuecomment-1083354220
@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] weizhouapache commented on a change in pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r837167506
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
Review comment:
ah, ok thanks @nvazquez
@ravening any reason why cluster-wide pools are not supported ? can you extend this PR to support cluster-wide pools ?
--
This is an automated message from the 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 #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
weizhouapache commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r837326123
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
Review comment:
thanks @ravening
if node reboots, you cannot acess the disabled pools (and the volumes on the pools) because they are not mounted on node. is it the issue you want to fix ?
zone-wide pools will be mounted on all hosts in the zone. cluster-wide pools will be mounted on all hosts in specific cluster. I understand you use only zone-wide pools, but many other users use cluster-wide pools. why not mount cluster-wide pools 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] nvazquez merged pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
nvazquez merged pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164
--
This is an automated message from the 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 #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#issuecomment-1083899472
<b>Trillian test result (tid-3763)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 29588 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6164-t3763-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/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.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] ravening commented on a change in pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r837232978
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
Review comment:
@ravening since VM's can be migrated across clusters, I want to support only zone wide storage pools. If disabled pool is only mounted in cluster A but if we want to migrate vm to cluster B then its not possible.
--
This is an automated message from the 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] ravening commented on a change in pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r837234268
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
+ }
+
for (StoragePoolVO pool : pools) {
- if (pool.getStatus() != StoragePoolStatus.Up) {
Review comment:
all of the above functions are only returning pools in Up state. so this check is not needed
--
This is an automated message from the 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 #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#issuecomment-1085945049
@nvazquez 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] blueorangutan commented on pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#issuecomment-1079238723
@nvazquez a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. 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 #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#issuecomment-1086443379
<b>Trillian test result (tid-3792)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38073 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6164-t3792-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 #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#issuecomment-1079267310
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 2988
--
This is an automated message from the 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 #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r836686399
##########
File path: engine/components-api/src/main/java/com/cloud/storage/StorageManager.java
##########
@@ -155,6 +155,15 @@
ConfigKey<String> PreferredStoragePool = new ConfigKey<String>(String.class, "preferred.storage.pool", "Advanced", "",
"The UUID of preferred storage pool for allocation.", true, ConfigKey.Scope.Account, null);
+ ConfigKey<Boolean> MountDisabledStoragePool = new ConfigKey<>(Boolean.class,
+ "mount.disabled.storage.pool",
+ "Storage",
+ "false",
+ "Mount all disabled storage pools after node reboot",
Review comment:
```suggestion
"Mount all disabled zode-wide storage pools after node reboot",
```
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
+ }
+
for (StoragePoolVO pool : pools) {
- if (pool.getStatus() != StoragePoolStatus.Up) {
Review comment:
Maybe can revert this and extend the condition to `if (pool.getStatus() != StoragePoolStatus.Up && pool.getStatus() != StoragePoolStatus.Disabled)` ?
--
This is an automated message from the 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] ravening commented on a change in pull request #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
ravening commented on a change in pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#discussion_r837373733
##########
File path: server/src/main/java/com/cloud/storage/listener/StoragePoolMonitor.java
##########
@@ -108,10 +108,12 @@ public void processConnect(Host host, StartupCommand cmd, boolean forRebalance)
List<StoragePoolVO> zoneStoragePoolsByAnyHypervisor = _poolDao.findZoneWideStoragePoolsByHypervisor(host.getDataCenterId(), HypervisorType.Any);
pools.addAll(zoneStoragePoolsByAnyHypervisor);
+ // get the disabled pools list if global setting is true.
+ if (StorageManager.MountDisabledStoragePool.value()) {
+ pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), null, null, ScopeType.ZONE));
Review comment:
@weizhouapache you are saying that, I need to make an extra call like this `pools.addAll(_poolDao.findDisabledPoolsByScope(host.getDataCenterId(), host.getClusterId(), null, ScopeType.CLUSTER));` ?
--
This is an automated message from the 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 #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#issuecomment-1083067645
@weizhouapache a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. 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 #6164: Mount disabled storage pool on host reboot
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #6164:
URL: https://github.com/apache/cloudstack/pull/6164#issuecomment-1083104819
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 3023
--
This is an automated message from the 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