Alona Kaplan has posted comments on this change. Change subject: engine,webadmin: Plugging of an unlinked vnic with an ext net is blocked ......................................................................
Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/32032/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java: Line 379: Line 380: public ValidationResult canExternalNetworkVnicBePlugged() { Line 381: if (RequiredAction.PLUG.equals(getRequiredAction()) && !nic.isLinked()) { Line 382: final boolean vnicAttachedToExternalNetwork = isVnicAttachedToExternalNetwork(); Line 383: if (validationResult != ValidationResult.VALID) { > I do not see how the error can be reported twice. The flow stops on the fir I agree, the error can't be reported twice. But it is not the issue. As I wrote- "this method should validate JUST if vnic with external network can be plugged". The validators are not independent, each validator can assume that all the relevant previous validation were done. In your validator you can assume that the profile is ok. You shouldn't do all the validations over and over again. The flow here- 1. calling 'isVnicAttachedToExternalNetwork()' which calls 2. 'VmNicValidator.findVnicNetwork(..)' which updates 3. validationResult (which is protected d.m of VmNicValidator and is used only to keep VdcBllMessages.ACTION_TYPE_FAILED_VNIC_PROFILE_NOT_EXISTS reported by the find method- which is by itself wrong desgin) doesn't make sense and it will be very hard to maintain it. Line 384: return validationResult; Line 385: } Line 386: Line 387: if (vnicAttachedToExternalNetwork) { -- To view, visit http://gerrit.ovirt.org/32032 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia944ca7e0430adc54a6cae6cb65b8a1df4e88d24 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yevgeny Zaspitsky <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Mike Kolesnik <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Yevgeny Zaspitsky <[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
