Dan Kenigsberg has posted comments on this change. Change subject: teach addNetwork to add a netwrok to an existing bridge ......................................................................
Patch Set 11: Code-Review-1 (4 comments) nits http://gerrit.ovirt.org/#/c/35922/11/tests/functional/networkTests.py File tests/functional/networkTests.py: Line 1179: # the kernel bridge driver automatically updates the bridge to Line 1180: # the new minimum MTU of all of it's connected interfaces Line 1181: self.assertMtu(BIG, NETWORK_NAME, second) Line 1182: rc, out, err = execCmd([EXT_IFDOWN, NETWORK_NAME]) Line 1183: self.assertEquals(rc, 0, 'ifdown failed: rc=%s' % (rc,)) adding out,err to the error message is easy, and could save much frustration in the future. Line 1184: rc, out, err = execCmd([EXT_IFUP, NETWORK_NAME]) Line 1185: self.assertEquals(rc, 0, 'ifup failed: rc=%s' % (rc,)) Line 1186: self.vdsm_net.refreshNetinfo() Line 1187: self.assertMtu(BIG, NETWORK_NAME, second) http://gerrit.ovirt.org/#/c/35922/11/vdsm/network/api.py File vdsm/network/api.py: Line 317: if bridged and network in _netinfo.bridges: Line 318: net_ent_to_configure = net_ent.port Line 319: logging.info("Bridge %s already exists.", network) Line 320: # the bridge already exists and we attach a new underlying device to Line 321: # it. The we need to make sure that the bridge MTU configuration is English: The we -> We Line 322: # updated. Line 323: configurator.configApplier.setIfaceMtu(network, mtu) Line 324: # we must also update the vms` tap devices (the bridge ports in this Line 325: # case) so that their MTU is synced with the bridge Line 320: # the bridge already exists and we attach a new underlying device to Line 321: # it. The we need to make sure that the bridge MTU configuration is Line 322: # updated. Line 323: configurator.configApplier.setIfaceMtu(network, mtu) Line 324: # we must also update the vms` tap devices (the bridge ports in this ` -> ' Line 325: # case) so that their MTU is synced with the bridge Line 326: _update_bridge_ports_mtu(net_ent.name, mtu) Line 327: else: Line 328: net_ent_to_configure = net_ent Line 327: else: Line 328: net_ent_to_configure = net_ent Line 329: Line 330: if net_ent_to_configure is not None: Line 331: logging.info("Configuring device %s", net_ent_to_configure) we're trying to reduce our log verbosity. isn't the name of the net_ent_to_configure going to show up in the upcoming logs anyway? Line 332: net_ent_to_configure.configure(**options) Line 333: configurator.configureLibvirtNetwork(network, net_ent) Line 334: if hostQos is not None: Line 335: configurator.configureQoS(hostQos, net_ent) -- To view, visit http://gerrit.ovirt.org/35922 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0df074c6c9d5f846748c2cff2cc14ba24305123b Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ido Barkan <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Ido Barkan <[email protected]> Gerrit-Reviewer: Ondřej Svoboda <[email protected]> Gerrit-Reviewer: Petr Horáček <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
