Ondřej Svoboda has posted comments on this change.
Change subject: net: ovs: better rollback ......................................................................
Patch Set 8: Code-Review-1
(18 comments)
I find modification of dictionaries while iterating them dangerous.
Some hook scripts don't have proper executable permissions. With assumed RPM magic not universally present, I would like to see +x permissions set explicitly :-)
https://gerrit.ovirt.org/#/c/46907/8//COMMIT_MSG Commit Message:
Line 9: Untill Until
Line 13: failtures failures
Line 19: witch with a
Line 20: OVS only an OVS-only (the hyphen helps, don't forget the next occurence as well)
https://gerrit.ovirt.org/#/c/46907/8/vdsm_hooks/ovs/README File vdsm_hooks/ovs/README:
Line 65: finishes finishes,
Line 118: happen happens
Line 136: trailing space
https://gerrit.ovirt.org/#/c/46907/8/vdsm_hooks/ovs/ovs_after_get_stats.py File vdsm_hooks/ovs/ovs_after_get_stats.py:
Line 1: #!/usr/bin/env python The file has a hashbang, but its mode changed to non-executable in this commit.
https://gerrit.ovirt.org/#/c/46907/8/vdsm_hooks/ovs/ovs_after_network_setup.... File vdsm_hooks/ovs/ovs_after_network_setup.py:
Line 1: #!/usr/bin/env python Also, it looks like it could be run standalone, but executable permissions are not set.
Line 41: successfull successful
https://gerrit.ovirt.org/#/c/46907/8/vdsm_hooks/ovs/ovs_after_network_setup_... File vdsm_hooks/ovs/ovs_after_network_setup_fail.py:
Line 1: #!/usr/bin/env python hashbang/non-executable mismatch, as with other files
Line 29: , : False) dict.get() would just return None.
Line 34: In At
https://gerrit.ovirt.org/#/c/46907/8/vdsm_hooks/ovs/ovs_before_network_setup... File vdsm_hooks/ovs/ovs_before_network_setup.py:
Line 82: running_config.networks.pop(net) Doesn't Python complain when you modify a dictionary while iterating it? You can use dict() to create a temporary copy on line 80.
Line 85: running_config.bonds.pop(bond) Likewise.
Line 94: Remove Removing (to keep uniformity)
Line 96: Reconfigure Reconfiguring
Line 135: inRollback No need to mimic camelCase :-)