Roy Golan has posted comments on this change. Change subject: backend: Control virtio rng device ......................................................................
Patch Set 14: (7 comments) http://gerrit.ovirt.org/#/c/18176/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractRngDeviceCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractRngDeviceCommand.java: Line 25: protected AbstractRngDeviceCommand(T parameters) { Line 26: super(parameters); Line 27: Line 28: if (parameters.getRngDevice() == null) { Line 29: cachedEntity = null; cachedEntity and chachedRngDevices are anyway null I think I don't understand this block... Line 30: cachedRngDevices = null; Line 31: return; Line 32: } Line 33: http://gerrit.ovirt.org/#/c/18176/14/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 21: setActionReturnValue(rngDevice.getDeviceId()); Line 22: setSucceeded(true); Line 23: } Line 24: Line 25: protected boolean canDoAction() { please order the canDo() up before the execeute() Line 26: if (!super.canDoAction()) { Line 27: return false; Line 28: } Line 29: if (!getRngDevices().isEmpty()) { http://gerrit.ovirt.org/#/c/18176/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java: Line 746: throw IllegalArgumentException maybe? and with the long constructor also its worth to pass the result cando to this command result so the actual invoker knows what went wrong can you try? getCanDoActions().addAll(result.getCanDoActions()) http://gerrit.ovirt.org/#/c/18176/14/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 15: public class VmRngDevice extends VmDevice implements Serializable { Line 16: Line 17: public enum Source { Line 18: RANDOM("random"), Line 19: HWRNG("hwrng"); since the NAME and "val" are same do we really need them? Line 20: Line 21: private final String val; Line 22: Line 23: private Source(String val) { Line 23: private Source(String val) { Line 24: this.val = val; Line 25: } Line 26: Line 27: public static Source fromString(String str) { java Enum supplies valueOf(String name) so this should do the trick - just always capitalize the candidate name Line 28: for (Source val : values()) { Line 29: if (val.toString().equals(str)) { Line 30: return val; Line 31: } Line 146: } Line 147: Line 148: public void setSource(Source source) { Line 149: if (source == null) { Line 150: getSpecParams().put(SOURCE_STRING, Source.RANDOM.toString()); and here just use Enum's name() method source.name() or lowercase if needed Line 151: } else { Line 152: getSpecParams().put(SOURCE_STRING, source.toString()); Line 153: } Line 154: } http://gerrit.ovirt.org/#/c/18176/14/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/VmDeviceType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/VmDeviceType.java: Line 22: CHANNEL("channel"), Line 23: SMARTCARD("smartcard"), Line 24: BALLOON("balloon"), Line 25: CONSOLE("console"), Line 26: VIRTIO("virtio"), no rng in the name? or is this a kind of group name that RNG is under? how is this reported back from vdsm? Line 27: WATCHDOG("watchdog"), Line 28: VIRTIOSCSI("virtio-scsi"), Line 29: OTHER("other", "0"), Line 30: UNKNOWN("unknown", "-1"); -- 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: 14 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
