Antoni Segura Puimedon has posted comments on this change. Change subject: bridge_opts: Add ifcfg configurator support ......................................................................
Patch Set 8: (3 comments) thanks for the review Dan ;-) http://gerrit.ovirt.org/#/c/26708/8//COMMIT_MSG Commit Message: Line 10: the VM networks bridge behavior by passing a string such as: Line 11: 'max_age=2100 forward_delay=300' Line 12: Line 13: This patch makes processing of such options part of vdsm proper by Line 14: including its processing in the ifcfg configurator. It also adds > Whenever I read "also" in a commit message, I'm alerted... Reporting bridgi I did it in this patch because I only added it for testability. You're right that this patch could be preceded by a standalone patch that introduced just the reporting. I'll do that ;-) Thanks for the suggestion. Line 15: reporting of the bridge_opts in netinfo and caps. Line 16: Line 17: A patch will follow up that adds support for other configurators. Line 18: http://gerrit.ovirt.org/#/c/26708/8/lib/vdsm/netinfo.py File lib/vdsm/netinfo.py: Line 531: info.update({'gateway': getgateway(gateways, link.name), Line 532: 'ipv6gateway': ipv6routes.get(link.name, '::'), Line 533: 'ports': ports(link.name), Line 534: 'stp': bridge_stp_state(link.name), Line 535: 'bridge_opts': bridgeOpts(link.name)}) > in this context, the "bridge_" prefix is redundant Right you are! I spent two weeks removing prefixes and here I go and add one :P Thanks for the catch! Line 536: return info Line 537: Line 538: Line 539: def _nicinfo(link, paddr, ipaddrs): http://gerrit.ovirt.org/#/c/26708/8/tests/functional/networkTests.py File tests/functional/networkTests.py: Line 676: @RequireDummyMod Line 677: @ValidateRunningAsRoot Line 678: def testSetupNetworksAddVlan(self, bridged): Line 679: BRIDGE_OPTS = {'multicast_router': '0', 'multicast_snooping': '0'} Line 680: frmtd_opts = ' '.join(['='.join(elem) for elem in BRIDGE_OPTS.items()]) > frmtd is short for "formatted" ? it is. Such a long word... But I shouldn't do this shortenings. Line 681: with dummyIf(1) as nics: Line 682: nic, = nics Line 683: attrs = {'vlan': VLAN_ID, 'nic': nic, 'bridged': bridged, Line 684: 'custom': {'bridge_opts': frmtd_opts}} -- To view, visit http://gerrit.ovirt.org/26708 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I506538384ce4273630cf5fd18c56b07b09bd4376 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Lior Vernia <[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
