Petr Horáček has posted comments on this change.

Change subject: net: introduce acquire module
......................................................................


Patch Set 16:

(2 comments)

https://gerrit.ovirt.org/#/c/60974/16/lib/vdsm/network/ifacquire.py
File lib/vdsm/network/ifacquire.py:

PS16, Line 59: netinfo_nets):
             :         used_ports = frozenset(itertools.chain.from_iterable(
             :             [attrs['ports'] for attrs in 
six.itervalues(netinfo_nets)]))
> Can be moved to init.
Done


https://gerrit.ovirt.org/#/c/60974/16/tests/network/ifacquire_test.py
File tests/network/ifacquire_test.py:

Line 69:     @mock.patch.object(ifacquire.ifcfg, 'ifdown')
Line 70:     @mock.patch.object(ifacquire, 'open', create=True)
Line 71:     def test_do_not_acquire_owned_nic(self, mock_open, mock_ifdown, 
mock_kill,
Line 72:                                       mock_flush):
Line 73:         with ifacquire.Transaction() as a:
> I find 'prepare' and 'turn_down' hard to understand, it does not describe w
disable_onboot is called after connectivity_check, so it should be all good. If 
we move it to __exit__ if will look like this:

if type is None:
    try:
        disable_onboot()
    except:
        rollback()
else:
    rollback()
Line 74:             a.prepare(ifaces=[NIC_NAME], netinfo_nets=NETINFO_NETS)
Line 75:             a.turn_down()
Line 76:             a.disable_onboot()
Line 77: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I180cfd7d69c0ae0a24188bc3d909b9d3d7c12145
Gerrit-PatchSet: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Edward Haas <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to