Arik Hadas has posted comments on this change.

Change subject: core: added is_run_once flag to vm_dynamic
......................................................................


Patch Set 1: (3 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmOnceCommand.java
Line 59:     @Override
Line 60:     protected CreateVmVDSCommandParameters initCreateVmParams() {
Line 61:         CreateVmVDSCommandParameters createVmParams = 
super.initCreateVmParams();
Line 62: 
Line 63:         getVm().setRunOnce(true);
shouldn't getVm() be replaced with createVmParams ?
Line 64:         RunVmOnceParams runOnceParams = getParameters();
Line 65: 
Line 66:         SysPrepParams sysPrepParams = new SysPrepParams();
Line 67:         
sysPrepParams.setSysPrepDomainName(runOnceParams.getSysPrepDomainName());


....................................................
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmDAO.java
Line 225:      *            The VM's ID
Line 226:      * @param isRunOnce
Line 227:      *            Whether or not the VM is running in run once.
Line 228:      */
Line 229:     void saveIsRunOnce(Guid vmid, boolean isRunOnce);
see comment on CreateVmVDSCommand, if there won't be any use case where 
is_run_once field is updated alone, then this method should be replaced with a 
different one that updates this field along with the rest of the fields which 
can be updated.
Line 230: 
Line 231:     /**
Line 232:      * Removes the VM with the specified id.
Line 233:      *


....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/CreateVmVDSCommand.java
Line 42:                 command.execute();
Line 43:                 if (command.getVDSReturnValue().getSucceeded()) {
Line 44:                     vm.setInitialized(true);
Line 45:                     saveSetInitializedToDb(vm.getId());
Line 46:                     
DbFacade.getInstance().getVmDao().saveIsRunOnce(vm.getId(), vm.isRunOnce());
this is adding another access to the DB which I think is redundant since we're 
updating the dynamic data anyway in line 56. can we set it on the VM instance 
as done in line 52 and change the call in line 56 to call a method that updates 
the dynamic fields including is_run_once?
Line 47: 
Line 48:                     
TransactionSupport.executeInScope(TransactionScopeOption.Required,
Line 49:                             new TransactionMethod<Object>() {
Line 50:                         @Override


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I25acdbcc84d326c694d7ae5ff1facaf13e5d683e
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to