Martin Sivák has posted comments on this change. Change subject: engine: Numa feature ......................................................................
Patch Set 5: Code-Review-1 (3 comments) I reviewed only data structure issues for now. I would prefer to have the parsed NUMA topology representation available through the Vm and Vds objects and only use the serialized form in the Db and VDSM communication. Also please document any new field you add. I know we used to be sloppy about that, but it causes issues when we do not clearly see the meaning and formats of data fields. http://gerrit.ovirt.org/#/c/23702/5/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VdsDynamic.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VdsDynamic.java: Line 84: private String kernel_version; Line 85: Line 86: private String iScsiInitiatorName; Line 87: Line 88: private String numa_nodes; Please describe the format of the data structure here. I would also prefer having access to the parsed representation as well so the other logic does not have to do the parsing again and again. Line 89: Line 90: private VdsTransparentHugePagesState transparentHugePagesState; Line 91: Line 92: @Size(max = BusinessEntitiesDefinitions.GENERAL_NAME_SIZE) http://gerrit.ovirt.org/#/c/23702/5/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmStatic.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmStatic.java: Line 45: @Size(max = BusinessEntitiesDefinitions.GENERAL_MAX_SIZE) Line 46: private String cpuPinning; Line 47: Line 48: private NumaType memNumaType; Line 49: Please describe the formats and expected values for the added fields. Line 50: @OvfExportOnlyField(exportOption = ExportOption.EXPORT_NON_IGNORED_VALUES) Line 51: @Size(max = BusinessEntitiesDefinitions.MEMORY_NUMA_NODE_SIZE) Line 52: private String memNumaNode; Line 53: http://gerrit.ovirt.org/#/c/23702/5/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDynamicDAODbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsDynamicDAODbFacadeImpl.java: Line 72: entity.setlibvirt_version(new RpmVersion(rs.getString("libvirt_version"))); Line 73: entity.setspice_version(rs.getString("spice_version")); Line 74: entity.setGlusterVersion(new RpmVersion(rs.getString("gluster_version"))); Line 75: entity.setkernel_version(rs.getString("kernel_version")); Line 76: entity.setnuma_nodes(rs.getString("numa_nodes")); As I mentioned in the VdsDynamic class, I would actually like to have more high level structure in memory and use the string representation only in the serialization classes (like here) and in vdsm communication (VdsBrokerObjectBuilder and others). Line 77: entity.setIScsiInitiatorName(rs Line 78: .getString("iscsi_initiator_name")); Line 79: entity.setTransparentHugePagesState(VdsTransparentHugePagesState Line 80: .forValue(rs.getInt("transparent_hugepages_state"))); -- To view, visit http://gerrit.ovirt.org/23702 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifefade432e7955503980bdc6fc5d73ea32818a95 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Xiaolei Shi <[email protected]> Gerrit-Reviewer: Liran Zelkha <[email protected]> Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Xiaolei Shi <[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
