Maor Lipchuk has posted comments on this change.
Change subject: core: WIP: avoid having LOCKED domain on a failed reconstruct.
......................................................................
Patch Set 2: (3 inline comments)
logic seems fine, see minor comments inline
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ReconstructMasterDomainCommand.java
Line 59: return false;
Line 60: }
Line 61:
Line 62: @Override
Line 63: protected boolean canDoAction() {
+1
A comment here is crucial, please also add the reason why we check it here, the
memory variable that is being used in the compensation, and why that we know
that this storage also fetched again later
Line 64: if
(checkIsDomainLocked(getStorageDomain().getStoragePoolIsoMapData())) {
Line 65: return false;
Line 66: }
Line 67:
Line 61:
Line 62: @Override
Line 63: protected boolean canDoAction() {
Line 64: if
(checkIsDomainLocked(getStorageDomain().getStoragePoolIsoMapData())) {
Line 65: return false;
Please add a warning log that the master domain is locked.
I'm afraid that the user will get a validation message, while the storage is no
longer locked, and he will not understand why the operation was blocked
Line 66: }
Line 67:
Line 68: List<StoragePoolIsoMap> poolDomains = DbFacade.getInstance()
Line 69:
.getStoragePoolIsoMapDao().getAllForStoragePool(getStoragePool().getId());
Line 68: List<StoragePoolIsoMap> poolDomains = DbFacade.getInstance()
Line 69:
.getStoragePoolIsoMapDao().getAllForStoragePool(getStoragePool().getId());
Line 70: for (StoragePoolIsoMap poolDomain : poolDomains) {
Line 71: if (checkIsDomainLocked(poolDomain)) {
Line 72: return false;
I would also add a log here which storage has been locked
Line 73: }
Line 74: }
Line 75:
Line 76: return initializeVds();
--
To view, visit http://gerrit.ovirt.org/16043
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf13f36f30d4cf48e8517444a33bab6359416d37
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
