Lior Vernia has posted comments on this change.
Change subject: engine: Support 3.1 network topologies by AttachNetwork
......................................................................
Patch Set 1: (2 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/AttachNetworkToVdsInterfaceCommand.java
Line 114: } else {
Line 115: logicalNetwork =
networksByName.get(params.getNetwork().getName());
Line 116: }
Line 117:
Line 118: if (!networkConfigurationSupported(iface, networksByName)) {
I'm not sure how the logic works when the new network is VLAN, but it seems to
me like the original check here was fine.
The new check looks too lenient; it would allow to attach a VM network when a
non-VM network is attached, or a non-VM network when a VM network is attached
(even if both are non-VLAN).
Line 119:
addCanDoActionMessage(VdcBllMessages.NETWORK_INTERFACE_ALREADY_HAVE_NETWORK);
Line 120: return false;
Line 121: }
Line 122:
Line 141: }
Line 142: }
Line 143:
Line 144: // check that nic have no vlans
Line 145: if (vmNetworkNonVlan(params.getNetwork())) {
I think this is where the feature supported check should come in.
Line 146: VdcQueryReturnValue ret =
Backend.getInstance().runInternalQuery(
Line 147: VdcQueryType.GetAllChildVlanInterfaces,
Line 148: new
InterfaceAndIdQueryParameters(params.getVdsId(), params
Line 149: .getInterface()));
--
To view, visit http://gerrit.ovirt.org/13722
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e1abe84e2877171be5af41c9bb3ab590fed3653
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Moti Asayag <[email protected]>
Gerrit-Reviewer: Lior Vernia <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
