Juan Hernandez has posted comments on this change. Change subject: restapi: Allow enabling/disabling SSO ......................................................................
Patch Set 7: (1 comment) http://gerrit.ovirt.org/#/c/21911/7/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/SsoMethod.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/SsoMethod.java: Line 2: Line 3: public enum SsoMethod { Line 4: Line 5: GUEST_AGENT, Line 6: NONE; > But how backend differentiate between empty <methods/> and no <methods/> at The point is that we need to differentiate three situations: * The user provides no <sso> element, or an <sso> element containing no <methods> element. When creating a new VM this means that the backend should select the default. When updating it means that no change should be made to the method. In this case the API should set the field of the parameter to null, and the backend should react accordingly. * The user provides an empty <methods> element. When creating or updating a VM this means that all the methods should be disabled. In this case the API should set the field of the parameter to NONE, and the backend should react accordingly. * The user provids a non empty <methods> element. When creating or updating a VM this means that all the selected methods should be enabled, and the rest disabled. As there is only one method currently the API should in this case check that the value is correct and set the field of the parameter to the corresponding backend enum, in this case GUEST_AGENT. This behaviour is needed in order to preserve backwards compatibility. Otherwise a user that is not aware of the new <sso> element may end up disabling SSO when creating or updating VMs. So, in my opinion, the backend needs to do something like this when creating a new VM: // If no SSO method has been given then select the default: if (vm.getSsoMethod() == null) { vm.setSsoMethod(SsoMethod.GUEST_AGENT); } And this when updating: // If no SSO method has been given then preserve the old value: if (vm.getSsoMethod() == null) { vm.setSsoMethod(oldvm.getSsoMethod()); } Line 7: Line 8: public String value() { Line 9: return name().toLowerCase(); Line 10: } -- To view, visit http://gerrit.ovirt.org/21911 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I25bf94646ea89684152e48f25ad604db6e59e86c Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Michael Pasternak <mishka8...@yahoo.com> Gerrit-Reviewer: Ori Liel <ol...@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