Sergey Gotliv has posted comments on this change. Change subject: core: prevent maintenance when domain is still in use ......................................................................
Patch Set 12: Code-Review+1 (4 comments) http://gerrit.ovirt.org/#/c/22045/12/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java: Line 263: if (getParameters().isInactive()) { Line 264: map.setStatus(StorageDomainStatus.InActive); Line 265: } else if (_isLastMaster) { Line 266: map.setStatus(StorageDomainStatus.Maintenance); Line 267: } I think that "else" with log message is mandatory here Line 268: getStoragePoolIsoMapDAO().updateStatus(map.getId(), map.getStatus()); Line 269: if (!Guid.Empty.equals(_newMasterStorageDomainId)) { Line 270: StoragePoolIsoMap mapOfNewMaster = getNewMaster(false).getStoragePoolIsoMapData(); Line 271: mapOfNewMaster.setStatus(StorageDomainStatus.Active); http://gerrit.ovirt.org/#/c/22045/12/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java: Line 209: for (VDS vds : DbFacade.getInstance().getVdsDao().getAllForStoragePool(storagePoolId)) { Line 210: if (vds.getStatus() == VDSStatus.Up Line 211: || vds.getStatus() == VDSStatus.NonResponsive Line 212: || vds.getStatus() == VDSStatus.PreparingForMaintenance Line 213: || vds.getStatus() == VDSStatus.NonOperational) { Can we add a method to the VDSStatus isConnected or something and just use it here. Line 214: vdsNotInMaintenance.add(vds.getId()); Line 215: } Line 216: } Line 217: Line 1115: Set<Guid> maintInPool = new HashSet<Guid>( Line 1116: DbFacade.getInstance().getStorageDomainStaticDao().getAllIds( Line 1117: _storagePoolId, StorageDomainStatus.Maintenance)); Line 1118: maintInPool.addAll(DbFacade.getInstance().getStorageDomainStaticDao().getAllIds( Line 1119: _storagePoolId, StorageDomainStatus.PreparingForMaintenance)); Why do you need a Set for maintInPool? Same domain can't be with both statuses at the same time, right? One stored procedure or API to get all storage domains matching all required statuses probably better... Line 1120: Line 1121: domainsInMaintenance = new HashSet<Guid>(); Line 1122: for (Guid tempDomainId : maintInPool) { Line 1123: if (!dataDomainIds.contains(tempDomainId)) { http://gerrit.ovirt.org/#/c/22045/12/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java: Line 1158: boolean isEditAvailable; Line 1159: boolean isActive = storageDomain.getStorageDomainSharedStatus() == StorageDomainSharedStatus.Active Line 1160: || storageDomain.getStorageDomainSharedStatus() == StorageDomainSharedStatus.Mixed; Line 1161: boolean isInMaintenance = (storageDomain.getStatus() == StorageDomainStatus.Maintenance Line 1162: || storageDomain.getStatus() == StorageDomainStatus.PreparingForMaintenance); Another, method in Enum? Line 1163: boolean isUnattached = (storageDomain.getStorageDomainSharedStatus() == StorageDomainSharedStatus.Unattached); Line 1164: boolean isDataDomain = storageDomain.getStorageDomainType().isDataDomain(); Line 1165: boolean isBlockStorage = storageDomain.getStorageType().isBlockDomain(); Line 1166: -- To view, visit http://gerrit.ovirt.org/22045 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I55cd5aa6a6dc32f374a4bb21b159d3cb30da65f5 Gerrit-PatchSet: 12 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
