Liron Ar has posted comments on this change.

Change subject: core: prevent maintenance when domain is still in use
......................................................................


Patch Set 11:

(1 comment)

reviewed irsbrokercommand change

http://gerrit.ovirt.org/#/c/22045/11/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 319:                         // domain not attached to pool anymore
Line 320:                         DbFacade.getInstance()
Line 321:                                 .getStoragePoolIsoMapDao()
Line 322:                                 .remove(new 
StoragePoolIsoMapId(domainInDb.getId(),
Line 323:                                         _storagePoolId));
few concerns - 
there are synchronization issues that we'll encounter if this code is present 
here (examples):

*hosts might be activated during that operation so the domain will move wrongly 
to maintenance

*synchronization issues while accessing the map (updates to the map during the 
time that we execute here)

*race with the domain failover process

I think that the approach should be to have this code exactly where we have the 
domain failover code, that way hosts won't be activated meanwhile and we will 
reduce massivly the amount of possible races/issues in that flow as they are 
already handled today by the domain failover process which is very similar to 
what we try to do here.
Line 324:                     }
Line 325:                     if (domainInDb.getStatus() == 
StorageDomainStatus.PreparingForMaintenance) {
Line 326:                         if (activeVdsInPool == null) {
Line 327:                             activeVdsInPool = 
getVdsConnectedToPool(_storagePoolId);


-- 
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: 11
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to