Liron Ar has posted comments on this change.
Change subject: core: change iso prefix command to be vds broker command
......................................................................
Patch Set 8:
(3 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)) {
I'm against having this logic reside here, it can be in a helper class..VDS
command should just execute the vds verb as we do in all the rest of the
commands. It can be extracted to an helper class which will fit better IMO.
Line 28: synchronized(getLockObjForStoragePool(storagePoolId)) {
Line 29: if
(!storagePoolIdToIsoPrefix.containsKey(storagePoolId)) {
Line 30: try {
Line 31: StoragePoolInfoReturnForXmlRpc retVal =
Line 34: storagePoolIdToIsoPrefix.put(storagePoolId,
Line 35:
retVal.mStoragePoolInfo.containsKey(IrsProperties.isoPrefix) ?
Line 36:
retVal.mStoragePoolInfo.get(IrsProperties.isoPrefix).toString()
Line 37: : StringUtils.EMPTY);
Line 38: } catch (Exception ex) {
will we have have an exception here?
Line 39: log.errorFormat("IsoPrefix Failed to get
storage pool info (vds {0}).", getParameters().getVdsId());
Line 40: return StringUtils.EMPTY;
Line 41: }
Line 42: }
Line 44: }
Line 45: return storagePoolIdToIsoPrefix.get(storagePoolId);
Line 46: }
Line 47:
Line 48: static void clearCachedIsoPrefix(Guid storagePoolId) {
possible race with getIsoPrefix(), are you fine with that?
we already have the eventqueue interface/infra for serializing events on a
pool, perhaps you could use it as well,.
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
