Federico Simoncelli has posted comments on this change. Change subject: core: add the domain detaching state ......................................................................
Patch Set 1: (4 comments) http://gerrit.ovirt.org/#/c/23556/1/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/StoragePoolDomainHelper.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/StoragePoolDomainHelper.java: Line 16: for (StoragePoolIsoMap domain : storagePoolIsoMaps) { Line 17: if (domain.getStatus() == StorageDomainStatus.Maintenance Line 18: || domain.getStatus() == StorageDomainStatus.MovingToMaintenance) { Line 19: storageDomains.put(domain.getstorage_id().toString(), "attached"); Line 20: } else if (domain.getStatus() != StorageDomainStatus.Detaching) { > shouldn't these be in 'attached' until detach succeeds? Yes, but it's not relevant to this class. Let's discuss it in DetachStorageDomainFromPoolCommand.java Line 21: storageDomains.put(domain.getstorage_id().toString(), Line 22: StorageDomainStatus.Active.toString().toLowerCase()); Line 23: } Line 24: } http://gerrit.ovirt.org/#/c/23556/1/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 1036: domainStatus = DomainVisibility.Status.MAINTENANCE; Line 1037: if (poolDomain.getStatus() == StorageDomainStatus.MovingToMaintenance) { Line 1038: moveDomainToMaintenance(poolDomain); Line 1039: } else if (poolDomain.getStatus() == StorageDomainStatus.Detaching) { Line 1040: detachDomainFromPool(poolDomain); > seems racy with the domain activation In what respect? If it's detaching you cannot activate it. Line 1041: } Line 1042: } Line 1043: } else { Line 1044: domainStatus = isDomainReportedAsProblematic(domainData, false) ? Line 1043: } else { Line 1044: domainStatus = isDomainReportedAsProblematic(domainData, false) ? Line 1045: DomainVisibility.Status.UNREACHABLE : DomainVisibility.Status.ACTIVE; Line 1046: if (poolDomain.getStatus() == StorageDomainStatus.MovingToMaintenance Line 1047: || poolDomain.getStatus() == StorageDomainStatus.Maintenance) { > shouldn't be Detaching? no sorry, this was part of another patch. Line 1048: vdsNeedsRefresh = true; Line 1049: } Line 1050: } Line 1051: Line 1125: Line 1126: log.infoFormat("Detaching domain {0} from pool", domain.getId(), _storagePoolId); Line 1127: StoragePoolIsoMap mapToRemove = domain.getStoragePoolIsoMapData(); Line 1128: DbFacade.getInstance().getStoragePoolIsoMapDao().remove( Line 1129: new StoragePoolIsoMapId(mapToRemove.getstorage_id(), mapToRemove.getstorage_pool_id())); > when it will be cleared from DomainsVisibllity? next round at line 1024 Line 1130: } Line 1131: Line 1132: private void moveDomainToMaintenance(StorageDomain domain) { Line 1133: // This is relevant only for domains that are in the process of being moved to maintenance -- To view, visit http://gerrit.ovirt.org/23556 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5732ef00a67ef1381ee0b6f29d08ab39cf63a1bf Gerrit-PatchSet: 1 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
