Tomas Jelinek has posted comments on this change.
Change subject: webadmin: Add Reboot VM action button
......................................................................
Patch Set 1:
(2 comments)
just small comment - otherwise seems nice
....................................................
File
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java
Line 3539: //
Line 3540: // also currently on VDSM side reboot is supported only
when the guest agent is present and responsive
Line 3541: // so we need to check for that
Line 3542: // TODO: better guest agent detection
Line 3543: // right now we check if the vmIp is set because for
some reason vm.getHasAgent() returns false even when GA is present
Could you than please provide an independent followup patch which will fix this?
Line 3544: if (!isCommandCompatible(VdcActionType.RebootVm,
version, version) || StringHelper.isNullOrEmpty(vm.getVmIp())) {
Line 3545: return false;
Line 3546: }
Line 3547: }
Line 3540: // also currently on VDSM side reboot is supported only
when the guest agent is present and responsive
Line 3541: // so we need to check for that
Line 3542: // TODO: better guest agent detection
Line 3543: // right now we check if the vmIp is set because for
some reason vm.getHasAgent() returns false even when GA is present
Line 3544: if (!isCommandCompatible(VdcActionType.RebootVm,
version, version) || StringHelper.isNullOrEmpty(vm.getVmIp())) {
Putting the same version even it is not correct and having a long comment
describing why does it does not matter is not exactly readable :)
I would prefer something like this:
Version clusterVersion = vm.getVdsGroupCompatibilityVersion();
Version anyDcVersion = new Version();
and than:
isCommandCompatible(VdcActionType.RebootVm, clusterVersion, anyDcVersion)
what do you think?
Line 3545: return false;
Line 3546: }
Line 3547: }
Line 3548: return true;
--
To view, visit http://gerrit.ovirt.org/22746
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a30dad6f2d3b909457e39ac15510f0468c2b1a1
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Betak <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[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
