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/08/27 10:39:54 UTC
[GitHub] [cloudstack] DK101010 opened a new pull request #5382: fix mismatching between db uuids and custom attributes uuids
DK101010 opened a new pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382
### Description
During the datastore cluster creation, cloudstack could not
recognize the existing primary storage and create a new one because
uuid format not equal.
<!--- 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. -->
Tested in my own Vmware env.
<!-- 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] nvazquez commented on pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1060774873
@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] rohityadavcloud commented on pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1007877810
@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] DaanHoogland commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r698425919
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
Review comment:
I agree with @sureshanaparti, a non conforming UUID is actually a corruption of data and should be fixed instead of worked around, @DK101010 .
--
This is an automated message from the 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] GutoVeronezi commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r714713643
##########
File path: engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41510to41600.java
##########
@@ -64,6 +67,30 @@ public boolean supportsRollingUpgrade() {
@Override
public void performDataMigration(Connection conn) {
+ fixWrongPoolUuid(conn);
+ }
+
+ public void fixWrongPoolUuid(Connection conn) {
+ LOG.debug("Replacement of faulty pool uuids");
+ try (PreparedStatement pstmt = conn.prepareStatement("SELECT id,uuid FROM storage_pool "
+ + "WHERE removed IS NULL;"); ResultSet rs = pstmt.executeQuery()) {
+ PreparedStatement updateStmt = conn.prepareStatement("update storage_pool set uuid = ? where id = ?");
+ while (rs.next()) {
+ if (!rs.getString(2).contains("-")) {
+ UUID poolUuid = new UUID(
+ new BigInteger(rs.getString(2).substring(0, 16), 16).longValue(),
+ new BigInteger(rs.getString(2).substring(16), 16).longValue()
+ );
+ updateStmt.setLong(2, rs.getLong(1));
+ updateStmt.setString(1, poolUuid.toString());
+ updateStmt.addBatch();
+ }
+ }
+ updateStmt.executeBatch();
+ } catch (SQLException ex) {
+ LOG.error("fixWrongPoolUuid:Exception while updating faulty pool uuids" + ex.getMessage());
Review comment:
Actually it is to log the stack trace and facilitate the troubleshooting (`LOG.error` has a `Throwable` parameter, which will print the stack trace into the log).
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-907200385
> > thought the code is not very bad, I don't think this is how the problem should be solved @DK101010 . Where the input does not adhere to the standard, it should be converted. not during the search. Don't you think?
>
> For sure, but I'm not sure why are uuids in custom attributes without and in db with separators. Also for this way we must update custom attributes of all storages and for this I can not estimate the impact of already existing implementation. Therefore this solution.
Ok, so how about removing the dashes unconditionally but just for the comparison and otherwise keep using the originals?
--
This is an automated message from the 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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r698317605
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
Review comment:
@sureshanaparti I just saw that the method is also called during the start of cloudstack. So as soon as cloustack starts, the primary storages in question are recreated.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1060776825
@nvazquez 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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r821394997
##########
File path: engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41520to41600.java
##########
@@ -75,6 +78,29 @@ public void performDataMigration(Connection conn) {
generateUuidForExistingSshKeyPairs(conn);
populateAnnotationPermissions(conn);
correctGuestOsIdsInHypervisorMapping(conn);
+ fixWrongPoolUuid(conn);
+ }
+
+ public void fixWrongPoolUuid(Connection conn) {
+ LOG.debug("Replacement of faulty pool uuids");
+ try (PreparedStatement pstmt = conn.prepareStatement("SELECT id,uuid FROM storage_pool "
+ + "WHERE uuid NOT LIKE \"%-%-%-%\" AND removed IS NULL;"); ResultSet rs = pstmt.executeQuery()) {
+ PreparedStatement updateStmt = conn.prepareStatement("update storage_pool set uuid = ? where id = ?");
+ while (rs.next()) {
+ UUID poolUuid = new UUID(
+ new BigInteger(rs.getString(2).substring(0, 16), 16).longValue(),
+ new BigInteger(rs.getString(2).substring(16), 16).longValue()
+ );
+ updateStmt.setLong(2, rs.getLong(1));
+ updateStmt.setString(1, poolUuid.toString());
+ updateStmt.addBatch();
+ }
+ updateStmt.executeBatch();
+ } catch (SQLException ex) {
+ String errorMsg = "fixWrongPoolUuid:Exception while updating faulty pool uuids";
+ LOG.error(errorMsg,ex);
+ throw new CloudRuntimeException(errorMsg, ex);
Review comment:
ahh sorry, I'll fix this
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1060797683
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 2786
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r718259812
##########
File path: engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41520to41600.java
##########
@@ -65,6 +68,31 @@ public boolean supportsRollingUpgrade() {
@Override
public void performDataMigration(Connection conn) {
generateUuidForExistingSshKeyPairs(conn);
+ fixWrongPoolUuid(conn);
+ }
+
+ public void fixWrongPoolUuid(Connection conn) {
+ LOG.debug("Replacement of faulty pool uuids");
+ try (PreparedStatement pstmt = conn.prepareStatement("SELECT id,uuid FROM storage_pool "
+ + "WHERE removed IS NULL;"); ResultSet rs = pstmt.executeQuery()) {
+ PreparedStatement updateStmt = conn.prepareStatement("update storage_pool set uuid = ? where id = ?");
+ while (rs.next()) {
+ if (!rs.getString(2).contains("-")) {
Review comment:
can we make this part of the where-clause of the select statement already? no reason to try and fix anything that isn't broken. (also is this condition good and enough?)
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1031038596
@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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1069168018
<b>Trillian test result (tid-3626)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33353 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5382-t3626-kvm-centos7.zip
Smoke tests completed. 92 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] sureshanaparti commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r698256812
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
Review comment:
@DK101010 I feel it is not correct to find the pool with the mismatched UUID (by removing the separators).
the UUIDs format in DB should be always with separators (`xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx`). It is better to fix if there is any mismatch / discrepancy in populating the UUIDs anywhere.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r697808315
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
+ if(uuid.contains("-")){
+ uuid = uuid.replaceAll("-", "");
+ storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+ }
+
+ //transform uuid in the valid format with separators
+ UUID poolUuid;
+ poolUuid = new UUID(
+ new BigInteger(uuid.substring(0, 16), 16).longValue(),
+ new BigInteger(uuid.substring(16), 16).longValue()
+ );
+
+ return _storagePoolDao.findByUuid(poolUuid.toString());
+ }
Review comment:
```suggestion
private StoragePoolVO getExistingPoolByUuid(String uuid){
String removedSepators = uuid.contains("-") ? uuid.replaceAll("-", "") : null;
String formatted = new UUID(
new BigInteger(uuid.substring(0, 16), 16).longValue(),
new BigInteger(uuid.substring(16), 16).longValue()).toString();
List<String> options = Arrays.asList(uuid, removedSepators, formatted);
return getFirstValidStoragePoolFromUuidOptions(options);
}
private StoragePoolVO getFirstValidStoragePoolFromUuidOptions(List<String> uuids) {
for (String uuid : uuids) {
if (StringUtils.isBlank(uuid)) {
continue;
}
StoragePoolVO pool = _storagePoolDao.findByUuid(uuid);
if (pool != null) {
return pool;
}
}
return null;
}
```
I think something like this could help readability. Can also add unit tests for the new method passing different options as a parameter
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r699050314
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
Review comment:
@sureshanaparti @DaanHoogland
I think I have expressed myself a little unclearly. Currently all custom attributes of the storage pools have a uuid without a separator (at least in the Vmware context). I think this is intentional even if I don't know why. The syncDatastoreClusterStoragePool method writes these uuids into the database without customising them.
I understand and am with you.
I can modify the method to only store uuids with separators in the database. But this means that we still need to upgrade the database to match existing uuids of primary storages, otherwise new primary storages will be created.
However, I don't know of any sql that can convert a uuid without separators into a uuid with separators.
--
This is an automated message from the 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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r698311272
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
Review comment:
@sureshanaparti ok, I already had the assumption that it should be like that.
We can set the correct uuid in the method here. However, this would mean that all primary storages that were created with a datastore cluster (4.15.1) are created again (with the correct uuid) when they are included 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] blueorangutan commented on pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1031051140
@sureshanaparti 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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r714499941
##########
File path: engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41510to41600.java
##########
@@ -64,6 +67,30 @@ public boolean supportsRollingUpgrade() {
@Override
public void performDataMigration(Connection conn) {
+ fixWrongPoolUuid(conn);
+ }
+
+ public void fixWrongPoolUuid(Connection conn) {
+ LOG.debug("Replacement of faulty pool uuids");
+ try (PreparedStatement pstmt = conn.prepareStatement("SELECT id,uuid FROM storage_pool "
+ + "WHERE removed IS NULL;"); ResultSet rs = pstmt.executeQuery()) {
+ PreparedStatement updateStmt = conn.prepareStatement("update storage_pool set uuid = ? where id = ?");
+ while (rs.next()) {
+ if (!rs.getString(2).contains("-")) {
+ UUID poolUuid = new UUID(
+ new BigInteger(rs.getString(2).substring(0, 16), 16).longValue(),
+ new BigInteger(rs.getString(2).substring(16), 16).longValue()
+ );
+ updateStmt.setLong(2, rs.getLong(1));
+ updateStmt.setString(1, poolUuid.toString());
+ updateStmt.addBatch();
+ }
+ }
+ updateStmt.executeBatch();
+ } catch (SQLException ex) {
+ LOG.error("fixWrongPoolUuid:Exception while updating faulty pool uuids" + ex.getMessage());
Review comment:
Hi @GutoVeronezi I think that is a matter of taste.
--
This is an automated message from the 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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r697419977
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,43 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
+ if(uuid.contains("-")){
+ uuid = uuid.replaceAll("-", "");
+ storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ storagePool.setUuid(uuid);
Review comment:
Hi @DaanHoogland, you are right. That was my bad. I will change this.
--
This is an automated message from the 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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r699860987
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
Review comment:
@DaanHoogland That's new for me. Where I can do that?
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r699388085
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
Review comment:
@DK101010 the upgrade includes a java step. it can be implemented in that.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r698256777
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
+ if(uuid.contains("-")){
+ uuid = uuid.replaceAll("-", "");
+ storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+ }
+
+ //transform uuid in the valid format with separators
+ UUID poolUuid;
+ poolUuid = new UUID(
+ new BigInteger(uuid.substring(0, 16), 16).longValue(),
+ new BigInteger(uuid.substring(16), 16).longValue()
+ );
+
+ return _storagePoolDao.findByUuid(poolUuid.toString());
+ }
Review comment:
what i mean is rigorously remove all '-'s and toLower() both strings and compare the results but not touch the originals.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1031050895
@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] DaanHoogland commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r718259812
##########
File path: engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41520to41600.java
##########
@@ -65,6 +68,31 @@ public boolean supportsRollingUpgrade() {
@Override
public void performDataMigration(Connection conn) {
generateUuidForExistingSshKeyPairs(conn);
+ fixWrongPoolUuid(conn);
+ }
+
+ public void fixWrongPoolUuid(Connection conn) {
+ LOG.debug("Replacement of faulty pool uuids");
+ try (PreparedStatement pstmt = conn.prepareStatement("SELECT id,uuid FROM storage_pool "
+ + "WHERE removed IS NULL;"); ResultSet rs = pstmt.executeQuery()) {
+ PreparedStatement updateStmt = conn.prepareStatement("update storage_pool set uuid = ? where id = ?");
+ while (rs.next()) {
+ if (!rs.getString(2).contains("-")) {
Review comment:
can we make this part of the where-clause of the select statement already? no reason to try and fix anything that isn't broken. (also is this condition good and enough?)
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r699388085
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
Review comment:
@DK101010 the upgrade includes a java step. it can be implemented in that.
--
This is an automated message from the 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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r699860987
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
Review comment:
@DaanHoogland That's new for me. Where I can do that?
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1079099780
@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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1068686346
@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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1007877834
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1007877834
@rohityadavcloud 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] rohityadavcloud commented on pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
rohityadavcloud commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1007877810
@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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r719267970
##########
File path: engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41520to41600.java
##########
@@ -65,6 +68,31 @@ public boolean supportsRollingUpgrade() {
@Override
public void performDataMigration(Connection conn) {
generateUuidForExistingSshKeyPairs(conn);
+ fixWrongPoolUuid(conn);
+ }
+
+ public void fixWrongPoolUuid(Connection conn) {
+ LOG.debug("Replacement of faulty pool uuids");
+ try (PreparedStatement pstmt = conn.prepareStatement("SELECT id,uuid FROM storage_pool "
+ + "WHERE removed IS NULL;"); ResultSet rs = pstmt.executeQuery()) {
+ PreparedStatement updateStmt = conn.prepareStatement("update storage_pool set uuid = ? where id = ?");
+ while (rs.next()) {
+ if (!rs.getString(2).contains("-")) {
Review comment:
Hi @DaanHoogland, hmm not quite sure what do you mean. I'm updating only uuids with the wrong format or do you want to reduce the result set of this query with a regular expression
`SELECT id,uuid FROM storage_pool WHERE removed IS NULL;`
If yes, I'm not really a fan of regular expressions in sql queries and I try to prevent this if it not absolutely necessary.
The result set should be not so big, therefore this solution. But I can change this if it requested.
I'm check out the storage pool table and in my opinion the conditions are right.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1031052412
Packaging result: :heavy_multiplication_x: el7 :heavy_multiplication_x: el8 :heavy_multiplication_x: debian :heavy_multiplication_x: suse15. SL-JID 2479
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1008606163
@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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1079565364
<b>Trillian test result (tid-3725)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39461 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5382-t3725-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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1068717522
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 2894
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1068718412
@blueorangutan test matrix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cloudstack] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r716386473
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,17 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ if(!uuid.contains("-")){
Review comment:
Hi @GutoVeronezi, thanks for your suggestion, but in this case I'll keep the current implementation.
--
This is an automated message from the 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] GutoVeronezi commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r714723688
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,17 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ if(!uuid.contains("-")){
Review comment:
My suggestion is to add two return statements. Inverting the `if` statement will reduce indentation and increase readability. Also, it will prevent this code to turn into a `hadouken code`.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1076561891
@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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r698221970
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
+ if(uuid.contains("-")){
+ uuid = uuid.replaceAll("-", "");
+ storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+ }
+
+ //transform uuid in the valid format with separators
+ UUID poolUuid;
+ poolUuid = new UUID(
+ new BigInteger(uuid.substring(0, 16), 16).longValue(),
+ new BigInteger(uuid.substring(16), 16).longValue()
+ );
+
+ return _storagePoolDao.findByUuid(poolUuid.toString());
+ }
Review comment:
@nvazquez I think that's a matter of taste :D. You make two actions in getExistingPoolByUuid that are not always necessary. But I also find your code quite nice and I will write a unit test.
@DaanHoogland I thought I had done that with the method :D.
Not quite sure what you mean.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1069201763
<b>Trillian test result (tid-3625)</b>
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35470 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5382-t3625-xenserver-71.zip
Smoke tests completed. 92 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1068718687
@nvazquez a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1069220074
<b>Trillian test result (tid-3627)</b>
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36646 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5382-t3627-vmware-65u2.zip
Smoke tests completed. 92 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1007899540
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 2145
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1083351717
PIng for review @weizhouapache @Pearl1594 @DaanHoogland
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-929846579
--
This is an automated message from the 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] GutoVeronezi commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r715761492
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,17 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ if(!uuid.contains("-")){
Review comment:
Inverting the `if` statement will reduce indentation and increase readability. Also, it will prevent this code to turn into a `hadouken code`.
It's not obligatory, it's just a suggestion.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1068686140
@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] DaanHoogland commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r698259107
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
+ if(uuid.contains("-")){
+ uuid = uuid.replaceAll("-", "");
+ storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+ }
+
+ //transform uuid in the valid format with separators
+ UUID poolUuid;
+ poolUuid = new UUID(
+ new BigInteger(uuid.substring(0, 16), 16).longValue(),
+ new BigInteger(uuid.substring(16), 16).longValue()
+ );
+
+ return _storagePoolDao.findByUuid(poolUuid.toString());
+ }
Review comment:
never mind my comment. I think @sureshanaparti 's comment at https://github.com/apache/cloudstack/pull/5382#discussion_r698256812 is more important.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-929823446
@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] blueorangutan commented on pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-929824444
--
This is an automated message from the 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] DK101010 commented on pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-907192932
> thought the code is not very bad, I don't think this is how the problem should be solved @DK101010 . Where the input does not adhere to the standard, it should be converted. not during the search. Don't you think?
For sure, but I'm not sure why are uuids in custom attributes without and in db with separators. Also for this way we must update custom attributes of all storages and for this I can not estimate the impact of already existing implementation. Therefore this solution.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-929823446
@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] blueorangutan commented on pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-929824444
@rhtyd 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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r714751905
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,17 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ if(!uuid.contains("-")){
Review comment:
With your suggestion keeps the if statement and the indentation. I'm not sure what do you exactly mean and we talk about one if statement :D.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r828686620
##########
File path: engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41520to41600.java
##########
@@ -33,6 +33,9 @@
import org.apache.log4j.Logger;
import com.cloud.utils.exception.CloudRuntimeException;
+import java.math.BigInteger;
+import java.util.UUID;
+
public class Upgrade41520to41600 implements DbUpgrade, DbUpgradeSystemVmTemplate {
Review comment:
This logic should be moved to `Upgrade41610to41700`
##########
File path: engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41520to41600.java
##########
@@ -75,6 +78,29 @@ public void performDataMigration(Connection conn) {
generateUuidForExistingSshKeyPairs(conn);
populateAnnotationPermissions(conn);
correctGuestOsIdsInHypervisorMapping(conn);
+ fixWrongPoolUuid(conn);
+ }
+
+ public void fixWrongPoolUuid(Connection conn) {
+ LOG.debug("Replacement of faulty pool uuids");
+ try (PreparedStatement pstmt = conn.prepareStatement("SELECT id,uuid FROM storage_pool "
+ + "WHERE uuid NOT LIKE \"%-%-%-%\" AND removed IS NULL;"); ResultSet rs = pstmt.executeQuery()) {
+ PreparedStatement updateStmt = conn.prepareStatement("update storage_pool set uuid = ? where id = ?");
+ while (rs.next()) {
+ UUID poolUuid = new UUID(
+ new BigInteger(rs.getString(2).substring(0, 16), 16).longValue(),
+ new BigInteger(rs.getString(2).substring(16), 16).longValue()
+ );
+ updateStmt.setLong(2, rs.getLong(1));
+ updateStmt.setString(1, poolUuid.toString());
+ updateStmt.addBatch();
+ }
+ updateStmt.executeBatch();
Review comment:
Sorry if not correct, but in case the result set is empty wouldn't this fail since the parameters are not set? Or does this method check the parameters before executing?
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1076602493
Packaging result: :heavy_check_mark: el7 :heavy_check_mark: el8 :heavy_check_mark: debian :heavy_check_mark: suse15. SL-JID 2971
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r697347340
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,43 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
+ if(uuid.contains("-")){
+ uuid = uuid.replaceAll("-", "");
+ storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ storagePool.setUuid(uuid);
Review comment:
You are setting a uuid on a VO but not persisting it. Then you return it for use in the system. I don't think this is the right way to go. You have the right object, why change it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r698414200
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
Review comment:
> @sureshanaparti ok, I already had the assumption that it should be like that.
> We can set the correct uuid in the method here. However, this would mean that all primary storages that were created with a datastore cluster (4.15.1) are created again (with the correct uuid) when they are included again.
@DK101010 If the UUID for any storage pool in datastore cluster is not inline with the actual UUID format used across, that UUID of these storage pool have to be fixed, instead looking for storage pool with other format.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r708325211
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
Review comment:
look for `Upgrade41510to41600.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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r699050314
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
Review comment:
@sureshanaparti @DaanHoogland
I think I have expressed myself a little unclearly. Currently all custom attributes of the storage pools have a uuid without a separator (at least in the Vmware context). I think this is intentional even if I don't know why. The syncDatastoreClusterStoragePool method writes these uuids into the database without customising them.
I understand and am with you.
I can modify the method to only store uuids with separators in the database. But this means that we still need to upgrade the database to match existing uuids of primary storages, otherwise new primary storages will be created.
However, I don't know of any sql that can convert a uuid without separators into a uuid with separators.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r699388085
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
Review comment:
@DK101010 the upgrade includes a java step. it can be implemented in that.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r820895751
##########
File path: engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41520to41600.java
##########
@@ -75,6 +78,29 @@ public void performDataMigration(Connection conn) {
generateUuidForExistingSshKeyPairs(conn);
populateAnnotationPermissions(conn);
correctGuestOsIdsInHypervisorMapping(conn);
+ fixWrongPoolUuid(conn);
+ }
+
+ public void fixWrongPoolUuid(Connection conn) {
+ LOG.debug("Replacement of faulty pool uuids");
+ try (PreparedStatement pstmt = conn.prepareStatement("SELECT id,uuid FROM storage_pool "
+ + "WHERE uuid NOT LIKE \"%-%-%-%\" AND removed IS NULL;"); ResultSet rs = pstmt.executeQuery()) {
+ PreparedStatement updateStmt = conn.prepareStatement("update storage_pool set uuid = ? where id = ?");
+ while (rs.next()) {
+ UUID poolUuid = new UUID(
+ new BigInteger(rs.getString(2).substring(0, 16), 16).longValue(),
+ new BigInteger(rs.getString(2).substring(16), 16).longValue()
+ );
+ updateStmt.setLong(2, rs.getLong(1));
+ updateStmt.setString(1, poolUuid.toString());
+ updateStmt.addBatch();
+ }
+ updateStmt.executeBatch();
+ } catch (SQLException ex) {
+ String errorMsg = "fixWrongPoolUuid:Exception while updating faulty pool uuids";
+ LOG.error(errorMsg,ex);
+ throw new CloudRuntimeException(errorMsg, ex);
Review comment:
```suggestion
throw new CloudRuntimeException(errorMsg, ex);
```
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1076562225
@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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r830819717
##########
File path: engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41520to41600.java
##########
@@ -75,6 +78,29 @@ public void performDataMigration(Connection conn) {
generateUuidForExistingSshKeyPairs(conn);
populateAnnotationPermissions(conn);
correctGuestOsIdsInHypervisorMapping(conn);
+ fixWrongPoolUuid(conn);
+ }
+
+ public void fixWrongPoolUuid(Connection conn) {
+ LOG.debug("Replacement of faulty pool uuids");
+ try (PreparedStatement pstmt = conn.prepareStatement("SELECT id,uuid FROM storage_pool "
+ + "WHERE uuid NOT LIKE \"%-%-%-%\" AND removed IS NULL;"); ResultSet rs = pstmt.executeQuery()) {
+ PreparedStatement updateStmt = conn.prepareStatement("update storage_pool set uuid = ? where id = ?");
+ while (rs.next()) {
+ UUID poolUuid = new UUID(
+ new BigInteger(rs.getString(2).substring(0, 16), 16).longValue(),
+ new BigInteger(rs.getString(2).substring(16), 16).longValue()
+ );
+ updateStmt.setLong(2, rs.getLong(1));
+ updateStmt.setString(1, poolUuid.toString());
+ updateStmt.addBatch();
+ }
+ updateStmt.executeBatch();
Review comment:
Hi @nvazquez, good point, no Idea how it works, I will check this.
--
This is an automated message from the 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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r831131793
##########
File path: engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41520to41600.java
##########
@@ -75,6 +78,29 @@ public void performDataMigration(Connection conn) {
generateUuidForExistingSshKeyPairs(conn);
populateAnnotationPermissions(conn);
correctGuestOsIdsInHypervisorMapping(conn);
+ fixWrongPoolUuid(conn);
+ }
+
+ public void fixWrongPoolUuid(Connection conn) {
+ LOG.debug("Replacement of faulty pool uuids");
+ try (PreparedStatement pstmt = conn.prepareStatement("SELECT id,uuid FROM storage_pool "
+ + "WHERE uuid NOT LIKE \"%-%-%-%\" AND removed IS NULL;"); ResultSet rs = pstmt.executeQuery()) {
+ PreparedStatement updateStmt = conn.prepareStatement("update storage_pool set uuid = ? where id = ?");
+ while (rs.next()) {
+ UUID poolUuid = new UUID(
+ new BigInteger(rs.getString(2).substring(0, 16), 16).longValue(),
+ new BigInteger(rs.getString(2).substring(16), 16).longValue()
+ );
+ updateStmt.setLong(2, rs.getLong(1));
+ updateStmt.setString(1, poolUuid.toString());
+ updateStmt.addBatch();
+ }
+ updateStmt.executeBatch();
Review comment:
I have checked it, executeBatch have no problems if no values are added.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-934127249
@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] DaanHoogland commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r720249450
##########
File path: engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41520to41600.java
##########
@@ -65,6 +68,31 @@ public boolean supportsRollingUpgrade() {
@Override
public void performDataMigration(Connection conn) {
generateUuidForExistingSshKeyPairs(conn);
+ fixWrongPoolUuid(conn);
+ }
+
+ public void fixWrongPoolUuid(Connection conn) {
+ LOG.debug("Replacement of faulty pool uuids");
+ try (PreparedStatement pstmt = conn.prepareStatement("SELECT id,uuid FROM storage_pool "
+ + "WHERE removed IS NULL;"); ResultSet rs = pstmt.executeQuery()) {
+ PreparedStatement updateStmt = conn.prepareStatement("update storage_pool set uuid = ? where id = ?");
+ while (rs.next()) {
+ if (!rs.getString(2).contains("-")) {
Review comment:
not vital, /me nitpicking. I meant to move the contains check on the string to the sql to reduce the result set . i.e. `SELECT id,uuid FROM storage_pool "
+ "WHERE removed IS NULL and uuid not LIKE '%-%-%' ` or some like it. please resolve if you don't feel like it, this should only be run once (on eache installation anyway.
--
This is an automated message from the 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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r714500109
##########
File path: engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41510to41600.java
##########
@@ -64,6 +67,30 @@ public boolean supportsRollingUpgrade() {
@Override
public void performDataMigration(Connection conn) {
+ fixWrongPoolUuid(conn);
+ }
+
+ public void fixWrongPoolUuid(Connection conn) {
+ LOG.debug("Replacement of faulty pool uuids");
+ try (PreparedStatement pstmt = conn.prepareStatement("SELECT id,uuid FROM storage_pool "
+ + "WHERE removed IS NULL;"); ResultSet rs = pstmt.executeQuery()) {
+ PreparedStatement updateStmt = conn.prepareStatement("update storage_pool set uuid = ? where id = ?");
+ while (rs.next()) {
+ if (!rs.getString(2).contains("-")) {
+ UUID poolUuid = new UUID(
+ new BigInteger(rs.getString(2).substring(0, 16), 16).longValue(),
+ new BigInteger(rs.getString(2).substring(16), 16).longValue()
+ );
+ updateStmt.setLong(2, rs.getLong(1));
+ updateStmt.setString(1, poolUuid.toString());
+ updateStmt.addBatch();
+ }
+ }
+ updateStmt.executeBatch();
+ } catch (SQLException ex) {
+ LOG.error("fixWrongPoolUuid:Exception while updating faulty pool uuids" + ex.getMessage());
+ throw new CloudRuntimeException("fixWrongPoolUuid:Exception while updating faulty pool uuids", ex);
Review comment:
Sure, I change that.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
sureshanaparti commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r698414200
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,42 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ /**
+ * fixed mismatching between db uuids and and custom
+ * attribute uuids
+ *
+ * To different formats of uuids exists
+ * 1. xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+ * 2. xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
+ * @param uuid of existing pool
+ * @return existing pull or null otherwise
+ */
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ StoragePoolVO storagePool = _storagePoolDao.findByUuid(uuid);
+ if(storagePool != null){
+ return storagePool;
+ }
+
+ //this case is unlikely (DB uuid without separators), but safety first
Review comment:
> @sureshanaparti ok, I already had the assumption that it should be like that.
> We can set the correct uuid in the method here. However, this would mean that all primary storages that were created with a datastore cluster (4.15.1) are created again (with the correct uuid) when they are included again.
@DK101010 If the UUID for any storage pool(s) in datastore cluster doesn't have separators, or not inline with the correct UUID format (i.e. `xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx`, which is used across CloudStack), then better to fix these UUIDs with correct format, instead looking for storage pool with other format (or without separators).
--
This is an automated message from the 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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r714501421
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,17 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ if(!uuid.contains("-")){
Review comment:
I don't see the benefit. When I change that, then I need additionally a else statement or two return statements.
--
This is an automated message from the 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] DK101010 commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
DK101010 commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r715336072
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,17 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ if(!uuid.contains("-")){
Review comment:
As I thought. But why should this code be better?
--
This is an automated message from the 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] GutoVeronezi commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r714766992
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,17 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ if(!uuid.contains("-")){
Review comment:
What I meant:
```java
private StoragePoolVO getExistingPoolByUuid(String uuid){
if (uuid.contains("-")) {
return _storagePoolDao.findByUuid(uuid);
}
UUID poolUuid = new UUID(
new BigInteger(uuid.substring(0, 16), 16).longValue(),
new BigInteger(uuid.substring(16), 16).longValue()
);
uuid = poolUuid.toString();
return _storagePoolDao.findByUuid(uuid);
}
```
or
```java
private StoragePoolVO getExistingPoolByUuid(String uuid){
if (uuid.contains("-")) {
return _storagePoolDao.findByUuid(uuid);
}
uuid = new UUID(
new BigInteger(uuid.substring(0, 16), 16).longValue(),
new BigInteger(uuid.substring(16), 16).longValue()
).toString();
return _storagePoolDao.findByUuid(uuid);
}
```
--
This is an automated message from the 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] GutoVeronezi commented on a change in pull request #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
GutoVeronezi commented on a change in pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#discussion_r714184186
##########
File path: engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41510to41600.java
##########
@@ -64,6 +67,30 @@ public boolean supportsRollingUpgrade() {
@Override
public void performDataMigration(Connection conn) {
+ fixWrongPoolUuid(conn);
+ }
+
+ public void fixWrongPoolUuid(Connection conn) {
+ LOG.debug("Replacement of faulty pool uuids");
+ try (PreparedStatement pstmt = conn.prepareStatement("SELECT id,uuid FROM storage_pool "
+ + "WHERE removed IS NULL;"); ResultSet rs = pstmt.executeQuery()) {
+ PreparedStatement updateStmt = conn.prepareStatement("update storage_pool set uuid = ? where id = ?");
+ while (rs.next()) {
+ if (!rs.getString(2).contains("-")) {
+ UUID poolUuid = new UUID(
+ new BigInteger(rs.getString(2).substring(0, 16), 16).longValue(),
+ new BigInteger(rs.getString(2).substring(16), 16).longValue()
+ );
+ updateStmt.setLong(2, rs.getLong(1));
+ updateStmt.setString(1, poolUuid.toString());
+ updateStmt.addBatch();
+ }
+ }
+ updateStmt.executeBatch();
+ } catch (SQLException ex) {
+ LOG.error("fixWrongPoolUuid:Exception while updating faulty pool uuids" + ex.getMessage());
Review comment:
We could pass the exception as parameter of `LOG.error(...)`.
##########
File path: server/src/main/java/com/cloud/storage/StorageManagerImpl.java
##########
@@ -1834,6 +1836,17 @@ public void syncDatastoreClusterStoragePool(long datastoreClusterPoolId, List<Mo
handleRemoveChildStoragePoolFromDatastoreCluster(childDatastoreUUIDs);
}
+ private StoragePoolVO getExistingPoolByUuid(String uuid){
+ if(!uuid.contains("-")){
Review comment:
We could invert this `if` to reduce indentation.
##########
File path: engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41510to41600.java
##########
@@ -64,6 +67,30 @@ public boolean supportsRollingUpgrade() {
@Override
public void performDataMigration(Connection conn) {
+ fixWrongPoolUuid(conn);
+ }
+
+ public void fixWrongPoolUuid(Connection conn) {
+ LOG.debug("Replacement of faulty pool uuids");
+ try (PreparedStatement pstmt = conn.prepareStatement("SELECT id,uuid FROM storage_pool "
+ + "WHERE removed IS NULL;"); ResultSet rs = pstmt.executeQuery()) {
+ PreparedStatement updateStmt = conn.prepareStatement("update storage_pool set uuid = ? where id = ?");
+ while (rs.next()) {
+ if (!rs.getString(2).contains("-")) {
+ UUID poolUuid = new UUID(
+ new BigInteger(rs.getString(2).substring(0, 16), 16).longValue(),
+ new BigInteger(rs.getString(2).substring(16), 16).longValue()
+ );
+ updateStmt.setLong(2, rs.getLong(1));
+ updateStmt.setString(1, poolUuid.toString());
+ updateStmt.addBatch();
+ }
+ }
+ updateStmt.executeBatch();
+ } catch (SQLException ex) {
+ LOG.error("fixWrongPoolUuid:Exception while updating faulty pool uuids" + ex.getMessage());
+ throw new CloudRuntimeException("fixWrongPoolUuid:Exception while updating faulty pool uuids", ex);
Review comment:
We could extract this message to a variable.
--
This is an automated message from the 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 #5382: fix mismatching between db uuids and custom attributes uuids
Posted by GitBox <gi...@apache.org>.
nvazquez commented on pull request #5382:
URL: https://github.com/apache/cloudstack/pull/5382#issuecomment-1056848121
Hi @DK101010 can you please fix the conflicts?
--
This is an automated message from the 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