Omer Frenkel has posted comments on this change. Change subject: core: move VM status handling to VdsManager. ......................................................................
Patch Set 4: (2 comments) looks good, i have only 2 comments for minor improvements http://gerrit.ovirt.org/#/c/28114/4/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java File backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java: Line 824: ThreadPoolUtil.execute(new Runnable() { Line 825: @Override Line 826: public void run() { Line 827: VDSReturnValue returnValue = null; Line 828: if (vm.getStatus() == VMStatus.MigratingFrom && vm.getMigratingToVds() != null) { please move this 'if' statement outside of the thread, it might be a minor improvement not to create thread for non-migrating vms. Line 829: returnValue = Line 830: ResourceManager.getInstance() Line 831: .runVdsCommand( Line 832: VDSCommandType.DestroyVm, Line 829: returnValue = Line 830: ResourceManager.getInstance() Line 831: .runVdsCommand( Line 832: VDSCommandType.DestroyVm, Line 833: new DestroyVmVDSCommandParameters(new Guid(vm.getMigratingToVds() this is old way of treating guids, you can safely just use vm.getMigratingToVds() (you check above its not null, no need to make string and new guid) Line 834: .toString()), vm Line 835: .getId(), true, false, 0) Line 836: ); Line 837: } -- To view, visit http://gerrit.ovirt.org/28114 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7304109b49a0fb8441f9fdc9aee6edb0c654ab0e Gerrit-PatchSet: 4 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eli Mesika <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[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
