Liron Ar has posted comments on this change.

Change subject: core: change iso prefix command to be vds broker command
......................................................................


Patch Set 8:

(2 comments)

....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/IsoPrefixVDSCommand.java
Line 23:     }
Line 24: 
Line 25:     private String getIsoPrefix() {
Line 26:         Guid storagePoolId = getParameters().getStoragePoolId();
Line 27:         if (!storagePoolIdToIsoPrefix.containsKey(storagePoolId)) {
If you are moving it from the irs broker command, i think that the proper place 
is an helper class, that way we aren't tied to the vds command and it makes it 
easier for debugging (as people aren't used for having that kind of logic in 
vds commands).
Line 28:             synchronized(getLockObjForStoragePool(storagePoolId)) {
Line 29:                 if 
(!storagePoolIdToIsoPrefix.containsKey(storagePoolId)) {
Line 30:                     try {
Line 31:                         StoragePoolInfoReturnForXmlRpc retVal =


Line 44:         }
Line 45:         return storagePoolIdToIsoPrefix.get(storagePoolId);
Line 46:     }
Line 47: 
Line 48:     static void clearCachedIsoPrefix(Guid storagePoolId) {
What i meant was is that getIsoPrefix() could return null if the remove is 
executed between the if clause check and the return statement, will the calling 
flows handle it properly?
Line 49:         storagePoolIdToIsoPrefix.remove(storagePoolId);
Line 50:     }
Line 51: 
Line 52:     private static Object getLockObjForStoragePool(Guid storagePoolId) 
{


-- 
To view, visit http://gerrit.ovirt.org/17815
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I272ce7b0407bf83bd47646941630362ecf0b18cc
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@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

Reply via email to