Moti Asayag has posted comments on this change. Change subject: engine: block migration if passthrough vnics exist ......................................................................
Patch Set 1: Code-Review-1 (4 comments) https://gerrit.ovirt.org/#/c/38891/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmValidator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmValidator.java: Line 227: Line 228: /** Line 229: * @return ValidationResult indicating whether the vm contains passthrough vnics Line 230: */ Line 231: public ValidationResult vmNotHavingPassthroughVnics() { seems that the name should be positive and plural by the way it is being used on the can-do-action. Please change to vmsHavingPassthroughVnic() Line 232: for (VM vm : vms) { Line 233: List<VmNetworkInterface> vnics = Line 234: DbFacade.getInstance().getVmNetworkInterfaceDao().getAllForVm(vm.getId()); Line 235: List<VmNetworkInterface> passthroughVnics = Line 230: */ Line 231: public ValidationResult vmNotHavingPassthroughVnics() { Line 232: for (VM vm : vms) { Line 233: List<VmNetworkInterface> vnics = Line 234: DbFacade.getInstance().getVmNetworkInterfaceDao().getAllForVm(vm.getId()); please extract the dao to getter so you'll be able to add a unit test for it ;-) Line 235: List<VmNetworkInterface> passthroughVnics = Line 236: LinqUtils.filter(vnics, new org.ovirt.engine.core.utils.linq.Predicate<VmNetworkInterface>() { Line 237: public boolean eval(VmNetworkInterface vnic) { Line 238: return vnic.isPassthrough(); Line 232: for (VM vm : vms) { Line 233: List<VmNetworkInterface> vnics = Line 234: DbFacade.getInstance().getVmNetworkInterfaceDao().getAllForVm(vm.getId()); Line 235: List<VmNetworkInterface> passthroughVnics = Line 236: LinqUtils.filter(vnics, new org.ovirt.engine.core.utils.linq.Predicate<VmNetworkInterface>() { why fqdn and not import ? Line 237: public boolean eval(VmNetworkInterface vnic) { Line 238: return vnic.isPassthrough(); Line 239: } Line 240: }); Line 239: } Line 240: }); Line 241: Line 242: Collection<String> replacements = ReplacementUtils.replaceWithNameable("interfaces", passthroughVnics); Line 243: replacements.add(String.format("$vmName %s", vm.getName())); this basically means that if the validator is set with list of vms where the first vm is ok and all the other violates the condition, it will returns success. it should look like: for (...) { ... if (!passthroughVnics.isEmpty()) { return new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_MIGRATION_OF_PASSTHROUGH_VNICS_IS_NOT_SUPPORTED, replacements); } } return ValidationResult.VALID; Line 244: return ValidationResult.failWith(VdcBllMessages.ACTION_TYPE_FAILED_MIGRATION_OF_PASSTHROUGH_VNICS_IS_NOT_SUPPORTED, Line 245: replacements.toArray(new String[] {})) Line 246: .unless(passthroughVnics.isEmpty()); Line 247: } -- To view, visit https://gerrit.ovirt.org/38891 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ieab61467f23f4e6684cf51b234faf8c595f279f8 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alona Kaplan <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
