Allon Mureinik has posted comments on this change.
Change subject: core: retrieve correct domains status in updateConn
......................................................................
Patch Set 4: I would prefer that you didn't submit this
(2 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/UpdateStorageServerConnectionCommand.java
Line 157: StorageDomain domain = null;
Line 158: List<StoragePool> storagePools =
getStoragePoolDAO().getAllForStorageDomain(storageDomainId);
Line 159: if (!storagePools.isEmpty()) {
Line 160: pool = storagePools.get(0);
Line 161: domain =
getStorageDomainDao().getForStoragePool(storageDomainId,pool.getId());
formatting - missing space after the ","
Line 162: if
(!(domain.getStatus().equals(StorageDomainStatus.Maintenance))) {
Line 163: String domainName =
domain.getStorageName();
Line 164: problematicDomainNames.add(domainName);
Line 165: }
Line 232: }
Line 233:
getStorageConnDao().update(getParameters().getStorageServerConnection());
Line 234: if (isDomainUpdateRequired) {
Line 235: for (StorageDomain domain : domains) {
Line 236: for (StoragePoolIsoMap map :
getStoragePoolIsoMap(domain)) {
You have an asymmetry here - in the change at isConnectionEditable you assume
one SP, and here you iterate over all of them.
IMHO, the approach here is the correct one, and not the one above.
In any event, they should offer the same treatment.
Line 237: restoreStateAfterUpdate(map);
Line 238: }
Line 239: }
Line 240: if (hasConnectStorageSucceeded) {
--
To view, visit http://gerrit.ovirt.org/17070
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I19221ef1f32dd7117e020cf6750034d64b4b47db
Gerrit-PatchSet: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
