Frank Kobzik has posted comments on this change. Change subject: backend: Control virtio rng device ......................................................................
Patch Set 24: (2 comments) http://gerrit.ovirt.org/#/c/18176/24/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java: Line 136: checkTrustedService(); Line 137: setSucceeded(true); Line 138: } Line 139: Line 140: private boolean updateRngDevice() { > is there a way to reuse this code with the same function in update template well, it could be trivial if template and vm commands had common ancestor. also there is the famous code duplication between VmManagementParametersBase and VmTemplateParametersBase which prevents updateRngDevice to be reusable. there could be way to reuse the code in this patch if i create some kind of wrapper class of VMPB and VTPB, but this is not nice too. the best way would be refactoring of those classes, but that's certainly not trivial and is out of scope of this patch. Line 141: // do not update if this flag is not set Line 142: if (getParameters().isUpdateRngDevice()) { Line 143: VdcQueryReturnValue query = Line 144: getBackend().runInternalQuery(VdcQueryType.GetRngDevice, new IdQueryParameters(getParameters().getVmId())); http://gerrit.ovirt.org/#/c/18176/24/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 67: public static final String BYTES_STRING = "bytes"; Line 68: public static final String PERIOD_STRING = "period"; Line 69: public static final String SOURCE_STRING = "source"; Line 70: Line 71: public static VmRngDevice create(Guid id, Guid vmId, Integer bytes, Integer period, Source source) { > why do you need a static method to return an instance? the original intention was to create couple of static methods with meaningful names for creating instance instead of using constructor. but gwt requires public constructor so i guess i'll remove this. Line 72: return new VmRngDevice(new VmDeviceId(id, vmId), createSpecPars(bytes, period, source)); Line 73: } Line 74: Line 75: public static VmRngDevice fromVmDevice(VmDevice dev) { -- 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: 24 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Frank Kobzik <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Frank Kobzik <[email protected]> Gerrit-Reviewer: Martin Betak <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: [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
