Change in vdsm[master]: tests: let VDSM consume a bond created by nmcli

danken at redhat.com danken at redhat.com
Mon Apr 25 15:49:08 UTC 2016


Dan Kenigsberg has posted comments on this change.

Change subject: tests: let VDSM consume a bond created by nmcli
......................................................................


Patch Set 8: Code-Review-1

(7 comments)

https://gerrit.ovirt.org/#/c/56059/8/tests/functional/networkTests.py
File tests/functional/networkTests.py:

PS8, Line 2996: # VERIFY: if NM is configured with [main] monitor-connection-files=
              :             # true, it should unmanage the bond and its slaves as soon as VDSM
              :             # writes the respective ifcfg files
please drop the comment now


https://gerrit.ovirt.org/#/c/56059/8/tests/network/nettestlib.py
File tests/network/nettestlib.py:

PS8, Line 27: from multiprocessing import Process
unrelated move


Line 445:         else:
Line 446:             raise
Line 447: 
Line 448: 
Line 449: def _nmcli_connection_add(device_name, conn_type, conn_name=None,
let's put these functions in a new nm module (still under tests/network)
Line 450:                           autoconnect=None, persistent=None, master=None,
Line 451:                           bond_mode=None, miimon=None, ipv4_address=None,
Line 452:                           ipv4_gateway=None):
Line 453:     nmcli_command = [_NMCLI.cmd, '--terse', 'connection', 'add',


Line 551: 
Line 552:         if dhcp_server_ipv4 and dhcp_server_ipv4_cidr:
Line 553:             for port in router_ports:
Line 554:                 addrAdd(port, dhcp_server_ipv4, dhcp_server_ipv4_cidr)
Line 555:                 # TODO? 'nmcli device connect' instead, just to be consistent?
I don't mind, you can drop the TODO.
Line 556:                 linkSet(port, ['up'])
Line 557: 
Line 558:         if dhcp_range_from and dhcp_range_to:
Line 559:             with dnsmasq_run(router_ports, dhcp_range_from, dhcp_range_to,


PS8, Line 561: mode!
please open an NM bugzilla. the comment here is not clear enough for me.


Line 558:         if dhcp_range_from and dhcp_range_to:
Line 559:             with dnsmasq_run(router_ports, dhcp_range_from, dhcp_range_to,
Line 560:                              router=ipv4_gateway):
Line 561:                 # TODO: NM tries to use incompatible options with this mode!
Line 562:                 # TODO: clear these from the log: "SELinux is preventing agetty
better open a BZ on the relevant component.

but you can leave

# TODO: understand why "SELinux is preventing agetty from using the net_admin capability" are reported to /var/log/messages
Line 563:                 # from using the net_admin capability"
Line 564:                 with _nm_connection(device_name=bond_name, conn_type='bond',
Line 565:                                     slaves=local_nics_to_enslave,
Line 566:                                     bond_mode=bond_mode,


PS8, Line 569: netlink.Monitor
this is relatively easy, please do that.


-- 
To view, visit https://gerrit.ovirt.org/56059
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7047ce59a515d0b8ed2c4c5307b4c0d47d4aa92b
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Edward Haas <edwardh at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda <osvoboda at redhat.com>
Gerrit-Reviewer: Petr Horáček <phoracek at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list