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

Reply via email to