Alona Kaplan has posted comments on this change. Change subject: engine: refactoring NetworkUtils.getVlanDeviceName ......................................................................
Patch Set 5: (2 comments) http://gerrit.ovirt.org/#/c/27152/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/NetworkParametersBuilder.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/NetworkParametersBuilder.java: Line 124: protected VdsNetworkInterface getVlanDevice(List<VdsNetworkInterface> nics, Line 125: VdsNetworkInterface baseNic, Line 126: Network network) { Line 127: for (VdsNetworkInterface n : nics) { Line 128: if ((baseNic.getName().equals(n.getBaseInterface()) && ObjectUtils.objectsEqual(n.getVlanId(), > Can't you use NetworkUtils.interfaceBasedOn here? I can, it will work. But I don't think it is necessary here. NetworkUtils.interfaceBasedOn return true even if both of the params are the same nic. I know that the vlan check will fail in this case, but I think leaving the if as is, is more correct. Line 129: network.getVlanId())) Line 130: || StringUtils.equals(n.getNetworkName(), network.getName())) { Line 131: return n; Line 132: } http://gerrit.ovirt.org/#/c/27152/5/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/NetworkUtils.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/NetworkUtils.java: Line 304: * @param network Line 305: * the network which holds the vlan-id Line 306: * @return a name representing the vlan device Line 307: */ Line 308: public static String constructVlanDeviceName(VdsNetworkInterface underlyingNic, Network network) { > Not sure if this method still needed here since it's only used in one place You are right, but I prefer to leave it here. This is the default way to construct vlan device name. Line 309: return underlyingNic.getName() + "." + network.getVlanId(); Line 310: } Line 311: Line 312: /** -- To view, visit http://gerrit.ovirt.org/27152 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I34ed8b14fb8e22d9681bdbc34d863c52b6d23be5 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
