Antoni Segura Puimedon has posted comments on this change. Change subject: packaging: setup: bridge: support tagged VLANs ......................................................................
Patch Set 2: Code-Review-1 (3 comments) Some small things http://gerrit.ovirt.org/#/c/25894/2/src/plugins/ovirt-hosted-engine-setup/network/bridge.py File src/plugins/ovirt-hosted-engine-setup/network/bridge.py: Line 215: vcaps=vlan_caps[selected], Line 216: ) Line 217: ) Line 218: else: Line 219: caps = nics_caps[selected] If selected is a bond (for which there is code in lines 226-229, won't this result in a KeyError? I'd say that the bonding case would be better treated by having bonds_caps and an elif selected in bonds_caps. Line 220: self.logger.debug( Line 221: 'getVdsCaps for {iface}: {caps}'.format( Line 222: iface=iface, Line 223: caps=caps, Line 242: 'bridged=True', Line 243: 'ONBOOT=yes', Line 244: ] Line 245: boot_proto = None Line 246: if 'BOOTPROTO' in caps['cfg']: This is correct for now, but netinfo will provide a better point in the future (there is a patch for it) to get this information via netinfo.getBootProtocol. Note that for backwards compatibility we are adding the info in the 'cfg' field you check, so it it fine ;-) Line 247: boot_proto = caps['cfg']['BOOTPROTO'] Line 248: cmd += ['bootproto=%s' % boot_proto] Line 249: Line 250: if boot_proto in ('dhcp', 'bootp'): Line 264: self.execute( Line 265: cmd, Line 266: raiseOnError=True Line 267: ) Line 268: #TODO: try if implementing above with vdsmcli API still works: It does, but it is cleaner to use an adaptation of the commented code below. Line 269: # from vdsm import vdscli Line 270: # s = vdscli.connect() Line 271: # s.setupNetwork( Line 272: # {'netname': {'vlan': vlan, 'nic': iface, 'bootproto': boot_proto, -- To view, visit http://gerrit.ovirt.org/25894 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0abd18b3ae36265406afb80a1ee082d2d14d4a92 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-hosted-engine-setup Gerrit-Branch: master Gerrit-Owner: Sandro Bonazzola <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Doron Fediuck <[email protected]> Gerrit-Reviewer: Sandro Bonazzola <[email protected]> Gerrit-Reviewer: Simone Tiraboschi <[email protected]> Gerrit-Reviewer: Yedidyah Bar David <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
