Arik Hadas has posted comments on this change.
Change subject: core: change iso prefix command to be vds broker command
......................................................................
Patch Set 6:
(4 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
Line 788: getVm().setVmPayload(getParameters().getVmPayload());
Line 789: }
Line 790:
Line 791: if (!vm.isAutoStartup() &&
!StringUtils.isEmpty(getVm().getIsoPath())
Line 792: &&
getIsoDomainListSyncronizer().findActiveISODomain(getVm().getStoragePoolId())
== null) {
I added a comment in the previous patch for this 'if' statement
Line 793: return
failCanDoAction(VdcBllMessages.VM_CANNOT_RUN_FROM_CD_WITHOUT_ACTIVE_STORAGE_DOMAIN_ISO);
Line 794: }
Line 795:
Line 796: return true;
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/IsoPrefixVDSCommand.java
Line 10: import
org.ovirt.engine.core.vdsbroker.irsbroker.StoragePoolInfoReturnForXmlRpc;
Line 11:
Line 12: public class IsoPrefixVDSCommand<T extends
VdsAndPoolIDVDSParametersBase> extends VdsBrokerCommand<T> {
Line 13:
Line 14: private static Map<Guid, String> storagePoolId2IsoPrefix = new
ConcurrentHashMap<Guid, String>();
Done
Line 15: private static ConcurrentHashMap<Guid, Object>
storagePoolId2LockObj = new ConcurrentHashMap<Guid, Object>();
Line 16:
Line 17: public IsoPrefixVDSCommand(T parameters) {
Line 18: super(parameters);
Line 33:
Line 34: storagePoolId2IsoPrefix.put(storagePoolId,
Line 35:
returnValue.mStoragePoolInfo.containsKey(IrsProperties.isoPrefix) ?
Line 36:
returnValue.mStoragePoolInfo.get(IrsProperties.isoPrefix).toString()
Line 37: : StringUtils.EMPTY);
I think it's cleaner that way but that's a matter of taste I guess.. I renamed
'returnValue' to 'retVal' - is it better now?
Line 38: } catch (Exception ex) {
Line 39: log.errorFormat("IsoPrefix Failed to get storage
pool info (vds {0}).", getParameters().getVdsId());
Line 40: return StringUtils.EMPTY;
Line 41: }
....................................................
File
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/ResetISOPathVDSCommand.java
Line 10: }
Line 11:
Line 12: @Override
Line 13: protected void executeVDSCommand() {
Line 14:
IsoPrefixVDSCommand.clearCachedIsoPrefix(getParameters().getStoragePoolId());
me neither, but it was like that before.. I didn't want to make too many
changes in this patch. I think we should think about two options:
1. not to ask the iso-prefix from vdsm. we already know the answer as we know
the storage pool id and domain id - we can calculate the iso prefix on our own.
2. pass only the iso name to vdsm and let him create the full path. the iso
prefix is not really being used by the engine.
I prefer the second option, but it will be more complex in terms of backward
compatibility. anyway, it's not for now..
Line 15: }
--
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: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Arik Hadas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[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
