Roy Golan has posted comments on this change.

Change subject: backend: Add cold VM reboot support
......................................................................


Patch Set 5:

(5 comments)

http://gerrit.ovirt.org/#/c/27012/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RebootVmCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RebootVmCommand.java:

Line 33:         VDSReturnValue returnValue;
Line 34:         final PowerDownVmVDSParameters parameters = 
createVdsParameters();
Line 35:         if (isEngineRebootRequired() && 
FeatureSupported.powerDownOptions(getVds().getVdsGroupCompatibilityVersion())) {
Line 36:             log.infoFormat("VM {0} configuration has changed. 
Destroying and recreating with new configuration.", getVm().getName());
Line 37:             setRebootWithReconfigurationFlag();
this command is non-tx. what do you expect to happen if the command fails with 
the flag?
Line 38:             parameters.setForceAsFallback(true);
Line 39:             returnValue = runVdsCommand(VDSCommandType.ShutdownVm, 
parameters);
Line 40:         } else {
Line 41:             log.infoFormat("Sending reboot command for VM {0}.", 
getVm().getName());


Line 44:         setActionReturnValue(returnValue.getReturnValue());
Line 45:         setSucceeded(returnValue.getSucceeded());
Line 46:     }
Line 47: 
Line 48:     private boolean isEngineRebootRequired() {
name is a bit confusing. this process is about cold boot so maybe it will give 
an idea.
Line 49:         return getVm().isRunOnce();
Line 50:     }
Line 51: 
Line 52:     @Override


Line 72:         return getSucceeded() ? AuditLogType.USER_REBOOT_VM : 
AuditLogType.USER_FAILED_REBOOT_VM;
Line 73:     }
Line 74: 
Line 75:     private void setRebootWithReconfigurationFlag() {
Line 76:         // this flag will be further handled in VdsUpdateRunTimeInfo
I'm all for comments but that's too specific details. may change, vary at time. 
I think the flag notion is enough to pick up that its handled by another party 
(btw I did the same evil myself)
Line 77:         getVm().getDynamicData().setEngineReboot(true);
Line 78:         getVmDynamicDao().update(getVm().getDynamicData());
Line 79:     }


http://gerrit.ovirt.org/#/c/27012/5/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java:

Line 1375:         
if you want this vm up again you have to go through regular handling == line 
1409

probably what got your vm up again was the rerun treatment and your runvm 
failed?


Line 1373:         VmExitStatus exitStatus = vmDynamic.getExitStatus();
Line 1374: 
Line 1375:         if (cacheVm != null && cacheVm.isEngineReboot()) {
Line 1376:             cacheVm.setEngineReboot(false);
Line 1377:             
getDbFacade().getVmDynamicDao().update(cacheVm.getDynamicData());
use addVmDynamicToList
Line 1378:             engineRebootingVms.add(cacheVm.getId());
Line 1379:             auditVmEngineReboot(cacheVm.getId());
Line 1380:             return;
Line 1381:         }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9ed08f6a6ce6dfc2257da1a7cd41141d05aff039
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Betak <[email protected]>
Gerrit-Reviewer: Arik Hadas <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[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

Reply via email to