Ido Barkan has posted comments on this change.

Change subject: Handle bridge reconfiguration
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.ovirt.org/#/c/37059/6/tests/functional/networkTests.py
File tests/functional/networkTests.py:

Line 1795:     @cleanupNet
Line 1796:     @RequireVethMod
Line 1797:     def testSetupNetworksReconfigureBridge(self):
Line 1798:         def setup_test_network(dhcp=True):
Line 1799:             network_params = {'nic': right, 'bridged': True}
> personally i don't like using variable which is not passed as parameter nor
I am not sure which variable you are referring to. perhaps you missed the inner 
function?
Line 1800:             if dhcp:
Line 1801:                 network_params.update(
Line 1802:                     {'bootproto': 'dhcp', 'blockingdhcp': True})
Line 1803:             else:


http://gerrit.ovirt.org/#/c/37059/6/vdsm/network/api.py
File vdsm/network/api.py:

Line 723: 
Line 724: def _should_keep_bridge(network_attrs, currently_bridged,
Line 725:                         network_running_config):
Line 726:     marked_for_removal = 'remove' in network_attrs
Line 727:     if marked_for_removal:
> when i look at it again, at least it is consistent with should_be_bridged
well, at least I am consistent. :)
Line 728:         return False
Line 729: 
Line 730:     should_be_bridged = network_attrs.get('bridged')
Line 731:     if currently_bridged and not should_be_bridged:


Line 731:     if currently_bridged and not should_be_bridged:
Line 732:         return False
Line 733: 
Line 734:     if currently_bridged and network_running_config != network_attrs:
Line 735:         # the bridge is being reconfigured
> why comment? log should be enough
Done
Line 736:         logging.debug("the bridge is being reconfigured")
Line 737:         return False
Line 738: 
Line 739:     return True


-- 
To view, visit http://gerrit.ovirt.org/37059
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia461004eb59d10283ac893dc8b5858388d5c4a8c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibar...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Ido Barkan <ibar...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Ondřej Svoboda <osvob...@redhat.com>
Gerrit-Reviewer: Petr Horáček <phora...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to