Omer Frenkel has posted comments on this change.

Change subject: backend: Control virtio rng device
......................................................................


Patch Set 11:

(2 comments)

http://gerrit.ovirt.org/#/c/18176/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddRngDeviceCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddRngDeviceCommand.java:

Line 13: 
Line 14:     @Override
Line 15:     protected void executeCommand() {
Line 16:         VmRngDevice rngDevice = getParameters().getRngDevice();
Line 17:         if (rngDevice.getDeviceId() == null) {
> i saw this approach in other parts of the code (AddDiskCommand). I'd prefer
unless we actually need to allow users set the id, i would leave it to the 
command to set the id (db is also ok, but we usually do this in the code)
Line 18:             rngDevice.setDeviceId(Guid.newGuid());
Line 19:         }
Line 20:         getDbFacade().getVmDeviceDao().save(rngDevice);
Line 21:         setActionReturnValue(rngDevice.getDeviceId());


http://gerrit.ovirt.org/#/c/18176/11/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmRngDevice.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmRngDevice.java:

Line 23:         private Source(String val) {
Line 24:             this.val = val;
Line 25:         }
Line 26: 
Line 27:         public static Source fromString(String str) {
> yes, but it only works for 1:1 representation of enum names (e.g. "RANDOM",
no, now i understand so it sounds ok
Line 28:             for (Source val : values()) {
Line 29:                 if (val.toString().equals(str)) {
Line 30:                     return val;
Line 31:                 }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc26a484c84244074426f4b0c790e2cc4ebb8da6
Gerrit-PatchSet: 11
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Frank Kobzik <fkob...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com>
Gerrit-Reviewer: Itamar Heim <ih...@redhat.com>
Gerrit-Reviewer: Martin Betak <mbe...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
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