Juan Hernandez has posted comments on this change. Change subject: core: Add selinux host info to VdsDynamic ......................................................................
Patch Set 2: (6 comments) Find some comments about the RESTAPI inside. For the backend, I would suggest to define a enum with the possible values, instead of using numeric values. http://gerrit.ovirt.org/#/c/26955/2/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/SELinuxEnforceMode.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/SELinuxEnforceMode.java: Line 2: Line 3: /** Line 4: * Created by dkuznets on 4/23/14. Line 5: */ Line 6: public enum SELinuxEnforceMode { The name should be "SELinuxMode". Line 7: ENFORCING("enforcing", 1), PERMISSIVE("permissive", 0), DISABLED("disabled", -1); Line 8: Line 9: private String stringValue; Line 10: private int intValue; Line 3: /** Line 4: * Created by dkuznets on 4/23/14. Line 5: */ Line 6: public enum SELinuxEnforceMode { Line 7: ENFORCING("enforcing", 1), PERMISSIVE("permissive", 0), DISABLED("disabled", -1); Please try to avoid the stringValue and the intValue, look at DiskType for an example. Line 8: Line 9: private String stringValue; Line 10: private int intValue; Line 11: http://gerrit.ovirt.org/#/c/26955/2/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd File backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd: Line 1122: </xs:sequence> Line 1123: </xs:complexType> Line 1124: Line 1125: Line 1126: <xs:element name="selinux_enforce_modes" type="SELinuxEnforceModes" /> I think that this should be named "selinux_modes", as it may be disabled as well. Line 1127: Line 1128: <xs:complexType name="SELinuxEnforceModes"> Line 1129: <xs:sequence> Line 1130: <xs:element name="selinux_enforce_mode" type="xs:string" minOccurs="0" maxOccurs="unbounded"> Line 1126: <xs:element name="selinux_enforce_modes" type="SELinuxEnforceModes" /> Line 1127: Line 1128: <xs:complexType name="SELinuxEnforceModes"> Line 1129: <xs:sequence> Line 1130: <xs:element name="selinux_enforce_mode" type="xs:string" minOccurs="0" maxOccurs="unbounded"> And this should be "selinux_mode". Line 1131: <xs:annotation> Line 1132: <xs:appinfo> Line 1133: <jaxb:property name="SELinuxEnforceModes" /> Line 1134: </xs:appinfo> Line 1512: Line 1513: <xs:element name="selinux" type="SELinux" /> Line 1514: <xs:complexType name="SELinux"> Line 1515: <xs:sequence> Line 1516: <xs:element name="enforce_mode" type="xs:string" /> And this should be just "mode". Also try to be explicit with the cardinality: minOccurs="0" maxOccurs="1" Line 1517: </xs:sequence> Line 1518: </xs:complexType> Line 1519: Line 1520: <xs:element name="host" type="Host"/> http://gerrit.ovirt.org/#/c/26955/2/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/HostMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/HostMapper.java: Line 687: return model; Line 688: } Line 689: Line 690: if (entity.getSELinuxEnforceMode() == -1) { Line 691: model.setEnforceMode("disabled"); Instead of the string literal, try to use this: model.setEnforceMode(SELinuxMode.DISABLED.toString()) Line 692: } else if (entity.getSELinuxEnforceMode() == 0) { Line 693: model.setEnforceMode("permissive"); Line 694: } else if (entity.getSELinuxEnforceMode() == 1) { Line 695: model.setEnforceMode("enforcing"); -- To view, visit http://gerrit.ovirt.org/26955 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If472f68702b59280c721450d4db50dc27dc19a30 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Dima Kuznetsov <[email protected]> Gerrit-Reviewer: Dima Kuznetsov <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Liran Zelkha <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: mooli tayer <[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
