Omer Frenkel has posted comments on this change.

Change subject: core, engine, webadmin: Initial support for alternative 
architectures
......................................................................


Patch Set 5:

(5 comments)

....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VM.java
Line 37:     private InitializationType initializationType;
Line 38: 
Line 39:     private Map<VmDevice, Map<String, String>> 
runtimeDeviceCustomProperties;
Line 40: 
Line 41:     private ArchitectureType architecture;
i think it should be called cluster arch since the vm arch is derived from the 
os,
i mean if user select rhel_6_x86 then the vm arch is x86, period. but the 
cluster arch is set apart
Line 42: 
Line 43:     public String getUserDefinedProperties() {
Line 44:         return vmStatic.getUserDefinedProperties();
Line 45:     }


....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmTemplate.java
Line 45:     private double bootDiskSizeGB;
Line 46: 
Line 47:     private double actualDiskSize;
Line 48: 
Line 49:     private ArchitectureType architecture;
same here, clusterArch
Line 50: 
Line 51:     public VmTemplate() {
Line 52:         setNiceLevel(0);
Line 53:         setCpuShares(0);


....................................................
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsGroupDAODbFacadeImpl.java
Line 210:             
entity.setClusterPolicyName(rs.getString("cluster_policy_name"));
Line 211:             
entity.setClusterPolicyProperties(SerializationFactory.getDeserializer()
Line 212:                     
.deserializeOrCreateNew(rs.getString("cluster_policy_custom_properties"), 
LinkedHashMap.class));
Line 213:             
entity.setEnableBallooning(rs.getBoolean("enable_balloon"));
Line 214:             
entity.setArchitecture(ArchitectureType.forValue(rs.getInt("architecture")));
personally i prefer saving in the db the enum name rather the ordinal what do 
you think?
Line 215:             return entity;
Line 216:         }
Line 217:     }
Line 218: 


....................................................
File 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfTemplateReader.java
Line 46:             
_vmTemplate.setOsId(osRepository.getOsIdByUniqueName(node.InnerText));
Line 47:         }
Line 48: 
Line 49:         // Workaround until the support for OVF on POWER is implemented
Line 50:         _vmTemplate.setArchitecture(ArchitectureType.x86_64);
if you will agree with me regarding this field is "cluster arch" and not 
template arch (look at comment on vmTemplate class) then setting it here will 
not be needed.
BUT i do think writing this value might be interesting even if just for 
"documenting" the cluster arch of the vm during the export.
Line 51:     }
Line 52: 
Line 53:     @Override
Line 54:     protected void readHardwareSection(XmlNode section) {


....................................................
File packaging/dbscripts/upgrade/03_03_0900_add_architecture_name_column.sql
Line 4: -- Existent clusters with cpu_name are x86_64, since alternative 
architectures are introduced after this upgrade
Line 5: UPDATE vds_groups SET architecture = 1 where cpu_name is not NULL and 
architecture = 0;
Line 6: 
Line 7: -- Update clusters
Line 8: create or replace FUNCTION __temp_update_vds_group()
not sure i understand this update clause and why its needed? the update above 
seems to do the work. no?
Line 9: returns void
Line 10: AS $procedure$
Line 11: declare
Line 12: r vds_groups%rowtype;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1ecd642e2bc05067d55884c948bdaeb6e7838c26
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vitor de Lima <vitor.l...@eldorado.org.br>
Gerrit-Reviewer: Gustavo Frederico Temple Pedrosa 
<gustavo.pedr...@eldorado.org.br>
Gerrit-Reviewer: Itamar Heim <ih...@redhat.com>
Gerrit-Reviewer: Leonardo Bianconi <leonardo.bianc...@eldorado.org.br>
Gerrit-Reviewer: Michael Pasternak <mpast...@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: Tomas Jelinek <tjeli...@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