Yair Zaslavsky has posted comments on this change.
Change subject: core, engine, webadmin: Cluster and architecture related changes
......................................................................
Patch Set 2:
(4 comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ChangeVDSClusterCommand.java
Line 92: if (getTargetCluster().supportsGlusterService() &&
!hasUpServerInTarget(getTargetCluster())) {
Line 93: return false;
Line 94: }
Line 95:
Line 96: if (vds.getCpuName() == null) {
I see some inconsitency in the check - in two cases you check for null,
and then u check "isNotEmpty" - why is that?
Line 97:
vds.setCpuName(CpuFlagsManagerHandler.FindMaxServerCpuByFlags(vds.getCpuFlags(),
Line 98: getTargetCluster().getcompatibility_version()));
Line 99: }
Line 100:
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HandleVdsCpuFlagsOrClusterChangedCommand.java
Line 45: @Override
Line 46: protected void executeCommand() {
Line 47: VDS vds = getVds();
Line 48: String vdsGroupCpuName = vds.getVdsGroupCpuName();
Line 49: VDSGroup cluster =
DbFacade.getInstance().getVdsGroupDao().get(vds.getVdsGroupId());
I would appreciate here if instead of DbFacade.getInstance().getVdsGroupDao()
you will extract a method of getVdsGroupDao(). This can help us in future plans
we have (mocking, CDI...)
Line 50:
Line 51: _foundCPU = true;
Line 52:
Line 53: ServerCpu sc =
CpuFlagsManagerHandler.FindMaxServerCpuByFlags(vds.getCpuFlags(), vds
Line 95:
Line 96: _architectureMismatch = true;
Line 97:
Line 98: addCustomValue("VdsArchitecture",
vds.getCpuName().getArchitecture().name());
Line 99: addCustomValue("VdsGroupArchitecture",
cluster.getArchitecture().name());
Maybe I'm missing here something - you perform addCustomValue twice, but where
do you perform the audit log?
Do you rely on the log that happens at the end of CommandBase.execute?
Was this your intention?
Line 100:
Line 101: SetNonOperationalVdsParameters tempVar = new
SetNonOperationalVdsParameters(getVdsId(),
Line 102: ARCHITECTURE_INCOMPATIBLE_WITH_CLUSTER);
Line 103:
....................................................
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
Line 928: ACTION_TYPE_FAILED_GLUSTER_HOOK_DOES_NOT_EXIST=Cannot ${action}
${type}. Gluster hook does not exist.
Line 929: ERROR_GET_STORAGE_DOMAIN_LIST=Cannot get Storage Domains list.
Line 930: VDS_CANNOT_REMOVE_HOST_HAVING_GLUSTER_VOLUME=Cannot remove gluster
server. Server having Gluster volume(s).
Line 931: CPU_TYPE_UNSUPPORTED_IN_THIS_CLUSTER_VERSION=Host CPU type is not
supported in this cluster compatibility version or is not supported at all.
Line 932: ACTION_TYPE_FAILED_VDS_CLUSTER_DIFFERENT_ARCHITECTURES=Cannot
${action} ${type}. The host and the destination cluster architectures do not
match.
Trailing whitespace, please remove
Line 933:
ACTION_TYPE_FAILED_VM_CANNOT_BE_PINNED_TO_CPU_AND_MIGRATABLE=Migratable VM's
cannot be pinned to CPU.
Line 934:
ACTION_TYPE_FAILED_VM_CANNOT_BE_PINNED_TO_CPU_WITH_UNDEFINED_HOST=Cannot set
host CPU pinning when host is not selected
Line 935: ACTION_TYPE_FAILED_NETWORK_INTERFACE_MAC_INVALID=Cannot ${action}
${type}. The Network Interface ${IfaceName} has an invalid MAC address
${MacAddress}. MAC address must be in format "HH:HH:HH:HH:HH:HH" where H is a
hexadecimal character (either a digit or A-F, case is insignificant).
Line 936: MIGRATE_PAUSED_VM_IS_UNSUPPORTED=Migrating a VM in paused status is
unsupported.
--
To view, visit http://gerrit.ovirt.org/18226
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If19c3ee99f5ef17721bb4111ddfb48977d1b578b
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Leonardo Bianconi <[email protected]>
Gerrit-Reviewer: Gustavo Frederico Temple Pedrosa
<[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Leonardo Bianconi <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Tomáš Došek <[email protected]>
Gerrit-Reviewer: Vitor de Lima <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[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
