Tomas Jelinek has posted comments on this change.
Change subject: engine: Run Once dialog ingores emtpy boot options (#857848)
......................................................................
Patch Set 1: (2 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
Line 136: getVm().setLastStartTime(new Date());
Line 137: }
Line 138:
Line 139: /**
Line 140: * By default use default boot sequence. This method can be
overridden to it up according to the RunVmParams.
:-D
sure
Line 141: */
Line 142: protected void refreshBootSequenceParameter(RunVmParams
runVmParameters) {
Line 143: getVm().setboot_sequence(getVm().getdefault_boot_sequence());
Line 144: }
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java
Line 65: * is used when VM is reloaded from the DB while its parameters
hasn't been persisted (e.g. when running 'as once')
Line 66: */
Line 67: @Override
Line 68: protected void refreshBootParameters(RunVmParams runVmParameters) {
Line 69: super.refreshBootParameters(runVmParameters);
The problem with inlining the refreshBootSequenceParameter is, that it is
called also from attachCd() where only the boot sequence should be set up - I
think the separation of this two methods was OK and should not be changed.
Also, I don't see the problem calling the setboot_seq twice. If I will not
call the super(), imagine you need to set up some common stuff in
refreshBootParameters - what would you do?
I understand that calling super, setting up something and than in the child
override it is also not too nice. A really correct solution would be to have a
final refreshBootParameters() method which calls an overridable
doRefreshBootParameters() method which would than do the actual logic. The
common (currently none) stuff would be in the refreshBootParameters, the
specific in doRefreshBootParameters().
This would be correct and clean solution, but kind of an overhead if there is
no common logic. But on the other hand, I would be afraid to completely
override the common parent without calling super...
Line 70:
Line 71: getVm().setinitrd_url(runVmParameters.getinitrd_url());
Line 72: getVm().setkernel_url(runVmParameters.getkernel_url());
Line 73: getVm().setkernel_params(runVmParameters.getkernel_params());
--
To view, visit http://gerrit.ovirt.org/8083
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I32e59783d9c4d8e8b61ec96c6bae5577fc59756c
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
