Arik Hadas has posted comments on this change.
Change subject: core: clean-up RunVmCommand.CanDoAction (2/15)
......................................................................
Patch Set 2: (4 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java
Line 685: return true;
Line 686: }
Line 687:
Line 688: ArrayList<String> messages =
getReturnValue().getCanDoActionMessages();
Line 689: // custom properties:
this comment should be removed, it doesn't provide any additional information
Line 690: if
(VmHandler.handleCustomPropertiesError(getVmPropertiesUtils().validateVMProperties(
Line 691: vm.getVdsGroupCompatibilityVersion(),
Line 692: vm.getStaticData()), messages)) {
Line 693: return false;
Line 691: vm.getVdsGroupCompatibilityVersion(),
Line 692: vm.getStaticData()), messages)) {
Line 693: return false;
Line 694: }
Line 695: // boot sequence:
this comment should be removed, it doesn't provide any additional information
Line 696: BootSequence boot_sequence =
(getParameters().getBootSequence() != null) ?
Line 697: getParameters().getBootSequence() :
vm.getDefaultBootSequence();
Line 698: Guid storagePoolId = vm.getStoragePoolId();
Line 699: // Block from running a VM with no HDD when its first boot
device is
Line 790: * action
Line 791: */
Line 792: if (!VdcActionUtils.CanExecute(Arrays.asList(vm), VM.class,
Line 793: VdcActionType.RunVm)) {
Line 794:
messages.add(VdcBllMessages.ACTION_TYPE_FAILED_VM_STATUS_ILLEGAL.toString());
why not calling failCanDoAction method here?
Line 795: return false;
Line 796: }
Line 797:
Line 798: if (!validateNetworkInterfaces()) {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
Line 452: }
Line 453: return retVal;
Line 454: }
Line 455:
Line 456: protected static boolean
handleCustomPropertiesError(List<ValidationError> validationErrors,
this technique maybe makes the code shorter, but it's not good for readability
- this method now do too much work: returns whether the given list of errors
contains errors and convert the given errors. I would keep the previous way of
checking whether you got errors and if you do, convert them - it's more
intuitive to read
Line 457: ArrayList<String> message) {
Line 458: if (validationErrors == null || validationErrors.size() == 0)
{
Line 459: return false;
Line 460: }
--
To view, visit http://gerrit.ovirt.org/13112
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I87ce576fbf82ad5b1f677d3cef7113b4cb9fd1e1
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: ofri masad <oma...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
