Moti Asayag has posted comments on this change. Change subject: core: add NetworkLinking support to RunVm command. ......................................................................
Patch Set 20: (5 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java Line 779: Set<String> clusterNetworksNames = Entities.objectNames(clusterNetworks); Line 780: Line 781: // Checking that the interface are all configured Line 782: // and they are attached to networks that are attached to the cluster Line 783: // and they are attached to VM networks the comment above should also be updated to state unlinked network might be supported (or better, these comments should be deleted from here and be added as javadoc for each of the isVmInterface* methods. Line 784: return isVmInterfacesConfigured() && Line 785: isVmInterfacesAttachedToClusterNetworks(clusterNetworksNames, interfaceNetworkNames) && Line 786: isVmInterfacesAttachedToVmNetworks(clusterNetworks, interfaceNetworkNames); Line 787: } Line 789: /** Line 790: * Checking that the interfaces are all configured If network linking is supported, interfaces with no network are Line 791: * allowed. Otherwise, canDoAction should be thrown. Line 792: * Line 793: * @param interfaceNetworkNames the parameter should be removed from the javadoc. Line 794: * VM interface network names Line 795: * @return true if all VM network interfaces are attached to existing cluster networks Line 796: */ Line 797: private boolean isVmInterfacesConfigured() { Line 791: * allowed. Otherwise, canDoAction should be thrown. Line 792: * Line 793: * @param interfaceNetworkNames Line 794: * VM interface network names Line 795: * @return true if all VM network interfaces are attached to existing cluster networks the description of the return isn't accurate: the vNic could be also not attached to any network. Line 796: */ Line 797: private boolean isVmInterfacesConfigured() { Line 798: for (VmNetworkInterface nic : getVm().getInterfaces()) { Line 799: if (nic.getNetworkName() == null) { .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/VmNicValidator.java Line 36: return !networkLinkingSupported(version) && nic.getNetworkName() == null Line 37: ? new ValidationResult(VdcBllMessages.NULL_NETWORK_IS_NOT_SUPPORTED, clusterVersion()) Line 38: : ValidationResult.VALID; Line 39: } Line 40: i was wondering when this should come :-) I think we can take it one step further into a class which contains all logic for detecting supported features. if agreed, you can land the foundation here and later patches could make use of it to simplify existing code and new one. The class could be located in common and could be used also by both clients and vdsbroker instead having same code repeated (same as repeated within this patch). Line 41: public static boolean networkLinkingSupported(String version) { Line 42: return Config.<Boolean> GetValue(ConfigValues.NetworkLinkingSupported, version); Line 43: } Line 44: .................................................... Commit Message Line 5: CommitDate: 2012-12-20 11:16:09 +0200 Line 6: Line 7: core: add NetworkLinking support to RunVm command. Line 8: Line 9: - If "linked" property on VmNetworkInterface is true commit message should be modify to reflect the changes: instead of linkState with up/down now linkAction with true/false is sent to VDSm. Line 10: 'linkState-> up' should be sent to the vdsm Line 11: Otherwise- Line 12: 'linkState-> down' should be sent to the vdsm Line 13: -- To view, visit http://gerrit.ovirt.org/9518 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib9805b18c364685a6533b89cfa31ae0575a00ea5 Gerrit-PatchSet: 20 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Muli Salem <msa...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches