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

Change subject: net: native ovs: split to-be-removed and to-be-added
......................................................................


Patch Set 6:

(7 comments)

https://gerrit.ovirt.org/#/c/55313/6/lib/vdsm/network/ovs/switch.py
File lib/vdsm/network/ovs/switch.py:

PS6, Line 63: _filter_nets
> How about: _split_nets_action
Done


PS6, Line 67: . If it is not marked as {'remove': True},
> .. else ..
the comment only describes much more readable code below, i'll drop it.


Line 70:     nets_to_be_removed = set()
Line 71:     nets_to_be_added = {}
Line 72: 
Line 73:     for net, attrs in six.iteritems(nets):
Line 74:         if ('remove' not in attrs and
> What about this:
Much better, thanks.
Line 75:                 attrs != running_networks.get(net)):
Line 76:             nets_to_be_added[net] = attrs
Line 77:         if 'remove' in attrs or (
Line 78:                 (net in running_networks and


Line 87:     Only {'remove': True} bondings should be removed, the rest should 
be added
Line 88:     or edited if differs from the current configuration.
Line 89:     """
Line 90:     bonds_to_be_removed = set()
Line 91:     bonds_to_be_added = {}
> added also means edited in this case?
it does, maybe it would be better to split it into 3 iterables. let's see when 
we have bond setup implemented (?)
Line 92: 
Line 93:     for bond, attrs in six.iteritems(bonds):
Line 94:         if 'remove' in attrs:
Line 95:             bonds_to_be_removed.add(bond)


https://gerrit.ovirt.org/#/c/55313/6/tests/network/ovs_test.py
File tests/network/ovs_test.py:

PS6, Line 82: RemoveAddFilter
> SplitAction
Done


PS6, Line 86: net2
> It will help the reader if you call it: 
Done


PS6, Line 89: 'net4': {'remove': True}
> It will be appropriate to add it to the fakes.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3c8065561db3876ee28e007305d4578ab05b2971
Gerrit-PatchSet: 6
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/mailman/listinfo/vdsm-patches

Reply via email to