Change in vdsm[master]: net: OVS netinfo
Dan Kenigsberg has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 34: Code-Review+2 raising score -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 34 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 35: * #1195208::Update tracker: OK * Set MODIFIED::bug 1195208#1195208IGNORE, not all related patches are closed, check 52336 -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 35 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Dan Kenigsberg has submitted this change and it was merged. Change subject: net: OVS netinfo .. net: OVS netinfo Map OvsInfo to netinfo format used by netinfo.cache:get() Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Bug-Url: https://bugzilla.redhat.com/1195208 Signed-off-by: Petr Horáček Reviewed-on: https://gerrit.ovirt.org/56972 Reviewed-by: Edward Haas Continuous-Integration: Jenkins CI Reviewed-by: Dan Kenigsberg --- M lib/vdsm/network/ovs/info.py M tests/network/ovs_info_test.py 2 files changed, 223 insertions(+), 1 deletion(-) Approvals: Jenkins CI: Passed CI tests Petr Horáček: Verified Dan Kenigsberg: Looks good to me, approved Edward Haas: Looks good to me, but someone else must approve -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 35 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Edward Haas has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 34: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 34 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Petr Horáček has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 34: Verified+1 Passed ovs_info_test.py -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 34 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Petr Horáček has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 33: -Verified -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 33 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Edward Haas has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 33: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 33 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Petr Horáček has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 33: Verified+1 Passed ovs_info_test.py -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 33 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Petr Horáček has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 32: Verified+1 (3 comments) Passed ovs_info_test.py https://gerrit.ovirt.org/#/c/56972/31/lib/vdsm/network/ovs/info.py File lib/vdsm/network/ovs/info.py: Line 137: Line 138: southbound = ovs_info.southbound_port(ports) Line 139: Line 140: # northbound ports represents networks Line 141: stp = bridge_attrs['stp'] > Interesting.. This means that STP can can be set only per OVS bridge? Can't Added note to trello. Line 142: for northbound_port in ovs_info.northbound_ports(ports): Line 143: _netinfo['networks'][northbound_port] = _get_network_info( Line 144: northbound_port, bridge, southbound, ports, stp, addresses, Line 145: routes) PS31, Line 174: ports(bridge, northbound, > This tag is hunting me now... it is already resolved at the caller. Done PS31, Line 188: bond_attrs) > Seems that we need only bond_attrs as an argument. Done -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 32 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Edward Haas has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 31: (4 comments) https://gerrit.ovirt.org/#/c/56972/30/lib/vdsm/network/ovs/info.py File lib/vdsm/network/ovs/info.py: PS30, Line 157: ound_bond_attrs['slaves' > Done for bridge_attrs - stp. For vlan I see no reason, we have to resolve i This is just a nit, it is fine as it is now. Just note that the caller can resolve the first lookup for (almost) free. I just find this func doing too much and taking too many arguments, but I have no good solution to offer. https://gerrit.ovirt.org/#/c/56972/31/lib/vdsm/network/ovs/info.py File lib/vdsm/network/ovs/info.py: Line 137: Line 138: southbound = ovs_info.southbound_port(ports) Line 139: Line 140: # northbound ports represents networks Line 141: stp = bridge_attrs['stp'] Interesting.. This means that STP can can be set only per OVS bridge? Can't we set it per VLAN/Network? If this is true, it is a gap from the legacy implementation. How do we handle it when two different networks request different STP settings? (both on the same OVS bridge) Line 142: for northbound_port in ovs_info.northbound_ports(ports): Line 143: _netinfo['networks'][northbound_port] = _get_network_info( Line 144: northbound_port, bridge, southbound, ports, stp, addresses, Line 145: routes) PS31, Line 174: ports[northbound]['tag'] This tag is hunting me now... it is already resolved at the caller. PS31, Line 188: port, ports Seems that we need only bond_attrs as an argument. That can be alreasy resolved for us by the OvsInfo.bonds generator. -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 31 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Petr Horáček has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 31: Verified+1 (5 comments) Passed ovs_info_test.py https://gerrit.ovirt.org/#/c/56972/30/lib/vdsm/network/ovs/info.py File lib/vdsm/network/ovs/info.py: PS30, Line 112: 'fak > Nice Thanks! PS30, Line 117: > What about a generator expressions? Done PS30, Line 122: > Same here, a generator expression may fit. Done PS30, Line 157: ound_bond_attrs['slaves' > I meant something else, which fits bridge_attrs as well: Done for bridge_attrs - stp. For vlan I see no reason, we have to resolve it per each network and both northbound and ports are needed here. Line 165: 'ports': _get_net_ports(bridge, northbound, southbound, ports), Line 166: 'stp': stp, Line 167: 'switch': 'ovs' Line 168: } Line 169: network_info.update(_get_iface_info(northbound, addresses, routes)) > Ah.. sorry for this: Done Line 170: return network_info Line 171: Line 172: Line 173: def _get_net_ports(bridge, northbound, southbound, ports): -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 31 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Edward Haas has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 30: (5 comments) Some small points to consider https://gerrit.ovirt.org/#/c/56972/30/lib/vdsm/network/ovs/info.py File lib/vdsm/network/ovs/info.py: PS30, Line 112: next( Nice PS30, Line 117: [ What about a generator expressions? PS30, Line 122: [ Same here, a generator expression may fit. PS30, Line 157: ports[northbound]['tag'] I meant something else, which fits bridge_attrs as well: Caller can call with the tag and stp resolved. (instead of the two attrs) Line 165: return network_info Line 166: Line 167: Line 168: def _get_net_ports(bridge, northbound, southbound, ports): Line 169: net_ports = [] Ah.. sorry for this: if net_tag: net_southbound_port = '{}.{}'.format(southbound, net_tag) else: net_southbound_port = southbound net_ports = [net_southbound_port] net_ports += [port for port, port_attrs in six.iteritems(ports) if (port_attrs['tag'] == net_tag and port_attrs['level'] not in (SOUTHBOUND, NORTHBOUND))] Line 170: Line 171: net_tag = ports[northbound]['tag'] Line 172: if net_tag: Line 173: expected_vlan_name = '%s.%s' % (southbound, net_tag) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 30 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Petr Horáček has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 27: (10 comments) https://gerrit.ovirt.org/#/c/56972/27/lib/vdsm/network/ovs/info.py File lib/vdsm/network/ovs/info.py: PS27, Line 120: for port, attrs in six.iteritems(self.bridges[bridge]['ports']): : if attrs['level'] == NORTHBOUND: : yield port, attrs > Will this work? Done PS27, Line 135: networking > nit: _netinfo? Done PS27, Line 138: bridge > You can pass 'ports' instead. Done PS27, Line 141: net > northbound_port Done PS27, Line 153: port_attrs > Only used once, better call it with port_attrs['tag'] Done. Cannot be done with bridge, we do not pass bridges to this function. PS27, Line 153: port > northbound Done PS27, Line 155: ports[southbound]['bond'] > southbound_bond = ports[southbound]['bond'] Done Line 172: def _get_net_ports(bridge, northbound, southbound, ports): Line 173: net_ports = [] Line 174: for port, port_attrs in six.iteritems(ports): Line 175: tag = port_attrs['tag'] Line 176: if tag == ports[northbound]['tag'] and port != bridge: > I do not fully understand this one. vdsClient getVdsCaps reports vnet tap devices as network ifaces. Line 177: if port != northbound: Line 178: net_ports.append(port.name) Line 179: elif tag is not None: Line 180: expected_vlan_name = '%s.%s' % (southbound, tag) PS27, Line 194: if bond_attrs['fake_iface']: : bond_info.update(_get_iface_info(port, addresses, routes)) > Why do we need to report the bond interface with the network IP info? I don't know what Engine expects, maybe it needs IP info to be there. Why should we report EMPTY_PORT_INFO when there is a real one? Line 200: Line 201: def _to_bond_opts(mode, lacp): Line 202: opts = {} Line 203: if mode or lacp: Line 204: custom_opts = [] > You can just update opts directly and there id no need for checking 'if mod Done Line 205: if mode: Line 206: custom_opts.append('ovs_mode:%s' % mode) Line 207: if lacp: Line 208: custom_opts.append('ovs_lacp:%s' % lacp) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 27 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Edward Haas has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 27: Code-Review-1 (11 comments) https://gerrit.ovirt.org/#/c/56972/26/lib/vdsm/network/ovs/info.py File lib/vdsm/network/ovs/info.py: Line 109: Line 110: return {'slaves': slaves, 'active_slave': active_slave, Line 111: 'fake_iface': fake_iface, 'mode': mode, 'lacp': lacp} Line 112: Line 113: def southbound_port(self, bridge): > I'd prefer to do that in another patch. There is also another problem: What If there are multiple southbound ports, we have a configuration corruption, and we need to recover (teardown the bridge and recreate it is an option). As the decision to have a single southbound port is in the core design, representing it in our data structure seems fair and can serve as a protection to a configuration corroption. I'm ok with patching this optimization later on. Line 114: for port, attrs in six.iteritems(self.bridges[bridge]['ports']): Line 115: if attrs['level'] == SOUTHBOUND: Line 116: return port Line 117: return None https://gerrit.ovirt.org/#/c/56972/27/lib/vdsm/network/ovs/info.py File lib/vdsm/network/ovs/info.py: PS27, Line 120: for port, attrs in six.iteritems(self.bridges[bridge]['ports']): : if attrs['level'] == NORTHBOUND: : yield port, attrs Will this work? return (port, attrs for port, attrs in six.iteritems(ports) if attrs['level'] == NORTHBOUND) Same for bonds PS27, Line 135: networking nit: _netinfo? PS27, Line 138: bridge You can pass 'ports' instead. Same for northbound_ports() and bonds(). PS27, Line 141: net northbound_port As there is no nicer way to do it (maybe you can come up with one), perhaps just add a comment: # The northbound port represents a net PS27, Line 153: port_attrs Only used once, better call it with port_attrs['tag'] Same for bridge_attrs['stp'] PS27, Line 153: port northbound PS27, Line 155: ports[southbound]['bond'] southbound_bond = ports[southbound]['bond'] and reuse it... Line 172: def _get_net_ports(bridge, northbound, southbound, ports): Line 173: net_ports = [] Line 174: for port, port_attrs in six.iteritems(ports): Line 175: tag = port_attrs['tag'] Line 176: if tag == ports[northbound]['tag'] and port != bridge: I do not fully understand this one. For the northbound(net) tag, except the bridge port itself: Ports that are with the same tag except the northbound port are reported. Other ports that have a tag are reported as vlan ifaces. But the 'other', includes not only southbound ports, but also possible taps etc... Feels like this one should have been done in _bridge_attr (OvsInfo), having there {'vlans': {vlan_id: []}} Line 177: if port != northbound: Line 178: net_ports.append(port.name) Line 179: elif tag is not None: Line 180: expected_vlan_name = '%s.%s' % (southbound, tag) PS27, Line 194: if bond_attrs['fake_iface']: : bond_info.update(_get_iface_info(port, addresses, routes)) Why do we need to report the bond interface with the network IP info? Isn't this needed for faking bridgeless only? Line 200: Line 201: def _to_bond_opts(mode, lacp): Line 202: opts = {} Line 203: if mode or lacp: Line 204: custom_opts = [] You can just update opts directly and there id no need for checking 'if mode or lacp', the individual checks are enough. Line 205: if mode: Line 206: custom_opts.append('ovs_mode:%s' % mode) Line 207: if lacp: Line 208: custom_opts.append('ovs_lacp:%s' % lacp) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 27 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Petr Horáček has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 27: Verified+1 (7 comments) Passed ovs_info_test.py https://gerrit.ovirt.org/#/c/56972/26/lib/vdsm/network/ovs/info.py File lib/vdsm/network/ovs/info.py: Line 109: Line 110: return {'slaves': slaves, 'active_slave': active_slave, Line 111: 'fake_iface': fake_iface, 'mode': mode, 'lacp': lacp} Line 112: Line 113: def southbound_port(self, bridge): > Perhaps we should cache this info at the bridge level with a 'southbond' ke I'd prefer to do that in another patch. There is also another problem: What should we do if there are multiple southbound ports set? It should never happen, but it sounds like bug danger. Line 114: for port, attrs in six.iteritems(self.bridges[bridge]['ports']): Line 115: if attrs['level'] == SOUTHBOUND: Line 116: return port Line 117: return None Line 184: Line 185: def _get_bond_info(port, bond_attrs, addresses, routes): Line 186: bond_info = { Line 187: 'slaves': bond_attrs['slaves'], Line 188: # TODO: what should we report when no slave is active? > Sorry for this nit, but could you please order the funcs from top to bottom Done Line 189: 'active_slave': (bond_attrs['active_slave'] or Line 190: bond_attrs['slaves'][0]), Line 191: 'opts': _to_bond_opts(bond_attrs['mode'], bond_attrs['lacp']), Line 192: 'switch': 'ovs' PS26, Line 190: bond_attrs['slaves'][0]), : 'opts': _to_bond_opt > Why would we need a default for this? Done PS26, Line 201: def _to_bond_opts(mode, lacp): : opts = {} > For southbound we use ovs_info.southbound_port and for northbound we use th Done PS26, Line 203: > Really confusing... this is the northbound port name that represents the ne Done https://gerrit.ovirt.org/#/c/56972/26/tests/network/ovs_info_test.py File tests/network/ovs_info_test.py: PS26, Line 208: else '::')) > This requires mocking the OvsInfo object, not its __init__. Done PS26, Line 209: getIpInfo', > I understand that it is easier to monkeypatch it like this, but it is a bad Done -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 27 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 27: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 27 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Edward Haas has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 26: Code-Review-1 (7 comments) https://gerrit.ovirt.org/#/c/56972/26/lib/vdsm/network/ovs/info.py File lib/vdsm/network/ovs/info.py: Line 109: Line 110: return {'slaves': slaves, 'active_slave': active_slave, Line 111: 'fake_iface': fake_iface, 'mode': mode, 'lacp': lacp} Line 112: Line 113: def southbound_port(self, bridge): Perhaps we should cache this info at the bridge level with a 'southbond' key? Does not need to be in this patch.. Hmm.. would it fit as a bridge property instead of a port level? Line 114: for port, attrs in six.iteritems(self.bridges[bridge]['ports']): Line 115: if attrs['level'] == SOUTHBOUND: Line 116: return port Line 117: return None Line 184: } Line 185: return bond_info Line 186: Line 187: Line 188: def get_netinfo(ovs_info=None): Sorry for this nit, but could you please order the funcs from top to bottom (public first and so on..) Line 189: Line 190: if ovs_info is None: Line 191: ovs_info = OvsInfo() Line 192: PS26, Line 190: if ovs_info is None: : ovs_info = OvsInfo() Why would we need a default for this? IMO instantiating an object like this better be controlled by the caller.. less surprises. PS26, Line 201: for port, port_attrs in six.iteritems(ports): : if port_attrs['level'] == NORTHBOUND: For southbound we use ovs_info.southbound_port and for northbound we use this? PS26, Line 203: port Really confusing... this is the northbound port name that represents the network name. That needs to be expressed somehow... https://gerrit.ovirt.org/#/c/56972/26/tests/network/ovs_info_test.py File tests/network/ovs_info_test.py: PS26, Line 208: @MonkeyPatch(info.OvsInfo, '__init__', mocked_OvsInfo_init) This requires mocking the OvsInfo object, not its __init__. But a proper test should have mocked the driver. (Because we should have unit tested OvsDB by mocking the driver, testing OvsInfo using the same mock and so on... PS26, Line 209: _get_iface_info I understand that it is easier to monkeypatch it like this, but it is a bad practice to patch/mock private funcs/methods. It makes the test strongly coupled to the CUD. You should mock/patch only public modules/classes/funcs, preferably not the ones from the same module you test. In this case, I guess you need to monkeypatch all the funcs called to process the info. This is also the reason why IP related stuff should have been done outside the info module. -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 26 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 26: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 26 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Petr Horáček has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 25: Verified+1 (4 comments) Passed ovs_info_test.py https://gerrit.ovirt.org/#/c/56972/24/lib/vdsm/network/ovs/info.py File lib/vdsm/network/ovs/info.py: Line 161: nics = ([southbound] if not ports[southbound]['bond'] Line 162: else ports[southbound]['bond']['slaves']) Line 163: network_info = { Line 164: 'iface': port, Line 165: 'bridged': True, > get info is expected to be called quite often. can we define these function Done Line 166: 'vlanid': port_attrs['tag'], Line 167: 'bond': bond, Line 168: 'nics': nics, Line 169: 'ports': _get_net_ports(bridge, port, southbound, ports), Line 182: bond_attrs['slaves'][0]), Line 183: 'opts': _to_bond_opts(bond_attrs['mode'], bond_attrs['lacp']), Line 184: 'switch': 'ovs' Line 185: } Line 186: if bond_attrs['fake_iface']: > Seems to me that you can move the ip tree here directly, without the need t Done Line 187: bond_info.update(_get_iface_info(port, addresses, routes)) Line 188: else: Line 189: bond_info.update(EMPTY_PORT_INFO) Line 190: return bond_info PS24, Line 199: es( > Do we need to support the 'cfg' portion? I think we deprecated it. Done PS24, Line 186: if bond_attrs['fake_iface']: : bond_info.update(_get_iface_info(port, addresses, routes)) : else: : bond_info.update(EMPTY_PORT_INFO) : return bond_info : : : def get_netinfo(ovs_info=None): : : if ovs_info is None: : ovs_info = OvsInfo() : : addresses = getIpAddrs() : routes = get_routes() : : networking = {'networks': {}, 'bondings': {}} : : for bridge, bridge_attrs in six.iterite > can you factor this (and other similar stanzas) out to a little helper func Done -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 25: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Edward Haas has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 24: (2 comments) partial review https://gerrit.ovirt.org/#/c/56972/24/lib/vdsm/network/ovs/info.py File lib/vdsm/network/ovs/info.py: Line 182: for bridge, bridge_attrs in six.iteritems(ovs_bridges): Line 183: ports = bridge_attrs['ports'] Line 184: southbound = _get_southbound_port(ports) Line 185: for port, port_attrs in six.iteritems(ports): Line 186: ip_attrs = port_attrs['ip'] Seems to me that you can move the ip tree here directly, without the need to keep it in OvsInfo. it also makes sense to name the ip keys as required to avoid the mapping. So filling port info here from _get_ip_info makes sense to me. Line 187: if ip_attrs: Line 188: port_info = { Line 189: 'mtu': port_attrs['mtu'], Line 190: 'addr': ip_attrs['ipv4addr'], PS24, Line 199: cfg Do we need to support the 'cfg' portion? I think we deprecated it. -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Dan Kenigsberg has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 24: (2 comments) https://gerrit.ovirt.org/#/c/56972/24/lib/vdsm/network/ovs/info.py File lib/vdsm/network/ovs/info.py: Line 161: custom_opts.append('ovs_lacp:%s' % lacp) Line 162: opts['custom'] = ','.join(custom_opts) Line 163: return opts Line 164: Line 165: def _get_fake_ports(bridge, northbound, southbound, ports): get info is expected to be called quite often. can we define these functions once, on module import? Line 166: fake_ports = [] Line 167: for port, port_attrs in six.iteritems(ports): Line 168: tag = port_attrs['tag'] Line 169: if tag == ports[northbound]['tag'] and port != bridge: PS24, Line 186: ip_attrs = port_attrs['ip'] : if ip_attrs: : port_info = { : 'mtu': port_attrs['mtu'], : 'addr': ip_attrs['ipv4addr'], : 'ipv4addrs': ip_attrs['ipv4addrs'], : 'gateway': ip_attrs['ipv4gateway'], : 'netmask': ip_attrs['ipv4netmask'], : 'dhcpv4': ip_attrs['dhcpv4'], : 'ipv6addrs': ip_attrs['ipv6addrs'], : 'ipv6autoconf': ip_attrs['ipv6autoconf'], : 'ipv6gateway': ip_attrs['ipv6gateway'], : 'dhcpv6': ip_attrs['dhcpv6'], : 'cfg': {'BOOTPROTO': ('dhcp' if ip_attrs['dhcpv4'] : else 'none')} : } : else: : port_info = EMPTY_PORT_INFO can you factor this (and other similar stanzas) out to a little helper function? I believe it would make this long function more readable. -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Petr Horáček has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 24: Verified+1 Passed ovs_info_test.py -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 24: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 23: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 23 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Petr Horáček has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 22: Verified+1 Passed ovs_info_test.py -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 22: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Petr Horáček has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 21: Verified+1 Passed ovs_info_test.py -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 21: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 20: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 19: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Petr Horáček has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 18: -Verified -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
Petr Horáček has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 18: Verified+1 Passed ovs_info_test.py -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 18: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS netinfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS netinfo .. Patch Set 17: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS NetInfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS NetInfo .. Patch Set 16: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS NetInfo
Petr Horáček has posted comments on this change. Change subject: net: OVS NetInfo .. Patch Set 15: Code-Review-1 Will be handled in a bit diffrent way, corresponding to legacy netinfo.cache, so they can be seamlessly switched. -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS NetInfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS NetInfo .. Patch Set 15: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS NetInfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS NetInfo .. Patch Set 14: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS NetInfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS NetInfo .. Patch Set 13: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS NetInfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS NetInfo .. Patch Set 12: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: OVS NetInfo
gerrit-hooks has posted comments on this change. Change subject: net: OVS NetInfo .. Patch Set 11: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56972 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia5dd351faf108fd04afa78208d9c34451a856d4a Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: net: ovs netinfo
gerrit-hooks has posted comments on this change. Change subject: net: ovs netinfo .. Patch Set 1: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6']) -- To view, visit https://gerrit.ovirt.org/56913 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iddd38dd8b75a1b4025f093c4c33ffcdd9f9c128b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches