Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 30: * #1364081::Update tracker: OK * Set MODIFIED::bug 1364081#1364081IGNORE, not all related patches are closed, check 60404 -- 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: 30 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Dan Kenigsberg has submitted this change and it was merged. Change subject: net: introduce acquire module .. net: introduce acquire module This module will be used to acquire ifaces, both persisted by ifcfg and not persisted. It is needed for OVS switch (and eventually also for iproute2 and pyroute2). Change-Id: I180cfd7d69c0ae0a24188bc3d909b9d3d7c12145 Bug-Url: https://bugzilla.redhat.com/1364081 Signed-off-by: Petr Horáček Reviewed-on: https://gerrit.ovirt.org/60974 Reviewed-by: Dan Kenigsberg Continuous-Integration: Jenkins CI --- M lib/vdsm/network/Makefile.am M lib/vdsm/network/configurators/ifcfg.py A lib/vdsm/network/ifacquire.py A tests/network/ifacquire_test.py M vdsm.spec.in 5 files changed, 302 insertions(+), 0 deletions(-) Approvals: Jenkins CI: Passed CI tests Petr Horáček: Verified Dan Kenigsberg: Looks good to me, approved -- To view, visit https://gerrit.ovirt.org/60974 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I180cfd7d69c0ae0a24188bc3d909b9d3d7c12145 Gerrit-PatchSet: 30 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Petr Horáček has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 29: Verified+1 Passed network/*_test.py -- 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: 29 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Dan Kenigsberg has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 29: Code-Review+2 (1 comment) copy and raise score https://gerrit.ovirt.org/#/c/60974/28/lib/vdsm/network/ifacquire.py File lib/vdsm/network/ifacquire.py: PS28, Line 140: unmanage this iface. unmanage this iface -- 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: 29 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 29: * #1364081::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1364081::OK, public bug * Check Product::#1364081::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 29 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Edward Haas has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 28: Code-Review+1 -- 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: 28 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 28: * #1364081::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1364081::OK, public bug * Check Product::#1364081::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 28 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Petr Horáček has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 27: Verified+1 Passed network/*_test.py -- 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: 27 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 27: * #1364081::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1364081::OK, public bug * Check Product::#1364081::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 27 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 26: * #1364081::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1364081::OK, public bug * Check Product::#1364081::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 26 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 25: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 25 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 24: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 24 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 23: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 23 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Edward Haas has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 22: Code-Review+1 -- 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: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 22: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Petr Horáček has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 22: Verified+1 (1 comment) Passed network/*test.py https://gerrit.ovirt.org/#/c/60974/21/tests/network/ifacquire_test.py File tests/network/ifacquire_test.py: PS21, Line 106: to open : @mock.patch.object(ifacquire.utils, 'atomic_fi > Ha... this works just because these atomic_file_write has an api similar to Done -- 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: 22 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Edward Haas has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 21: Code-Review+1 (1 comment) Very nice. Only one recommendation for comment. https://gerrit.ovirt.org/#/c/60974/21/tests/network/ifacquire_test.py File tests/network/ifacquire_test.py: PS21, Line 106: 'atomic_file_write', :new_callable=mock.mock_open Ha... this works just because these atomic_file_write has an api similar to open. Seems like a unique thing to do... better comment it here. -- 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: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Petr Horáček has posted comments on this change.
Change subject: net: introduce acquire module
..
Patch Set 19:
(11 comments)
https://gerrit.ovirt.org/#/c/60974/19/lib/vdsm/network/ifacquire.py
File lib/vdsm/network/ifacquire.py:
Line 51:
Line 52: def __enter__(self):
Line 53: return self
Line 54:
Line 55: def __exit__(self, type, value, traceback):
> We better log the exception and the transaction failure.
IIUIC it will be raised up to rollback and rollback will log it, or am i wrong?
Line 56: if type is None:
Line 57: try:
Line 58: self._disable_onboot()
Line 59: except:
Line 65: for iface, ifcfg_lines in six.iteritems(self._ifaces):
Line 66: if ifcfg_lines:
Line 67: _rollback_ifcfg_iface(iface, ifcfg_lines)
Line 68:
Line 69: def acquire(self, ifaces):
> Move after __init__ please or right after __exit__.
Done
Line 70: self._backup(ifaces)
Line 71: self._turn_down()
Line 72:
Line 73: def _backup(self, ifaces):
PS19, Line 80: _turn_down
> How about _release_iface ?
Done
PS19, Line 83: turn_down
> And here 'release' as well... (if you agree with the previous one)
Done
https://gerrit.ovirt.org/#/c/60974/19/tests/network/ifacquire_test.py
File tests/network/ifacquire_test.py:
PS19, Line 82: mock_isfile
> not needed, no need to generate a MagicMock, just replace 'return_value' wi
Done
PS19, Line 109: mock_isfile
> not needed.
Done
PS19, Line 113: handle
> How about 'file' instead of 'handle'?
Done
Line 116: atomic_write_handle.readlines.return_value = original_ifcfg
Line 117:
Line 118: with ifacquire.Transaction(netinfo_nets={}) as a:
Line 119: a.acquire(ifaces=[NIC_NAME])
Line 120: atomic_write_handle.writelines.assert_called_with(
> New line before this please.
Done
Line 121: ifcfg_after_turn_down)
Line 122: atomic_write_handle.readlines.return_value =
ifcfg_after_turn_down
Line 123: mock_ifdown.assert_called_with(NIC_NAME)
Line 124: atomic_write_handle.writelines.assert_called_with(
Line 120: atomic_write_handle.writelines.assert_called_with(
Line 121: ifcfg_after_turn_down)
Line 122: atomic_write_handle.readlines.return_value =
ifcfg_after_turn_down
Line 123: mock_ifdown.assert_called_with(NIC_NAME)
Line 124: atomic_write_handle.writelines.assert_called_with(
> New line before this please
Done
Line 125: ifcfg_after_disable_onboot)
Line 126:
Line 127: @mock.patch.object(ifacquire.ifcfg, 'ifdown', return_value=None)
Line 128: @mock.patch.object(ifacquire.os.path, 'isfile', return_value=True)
PS19, Line 134: mock_isfile,
: mock_ifdown
> No need for these two.
Done
Line 140:
Line 141: with self.assertRaises(TestException):
Line 142: with ifacquire.Transaction(netinfo_nets={}) as a:
Line 143: a.acquire(ifaces=[NIC_NAME])
Line 144: raise TestException()
> new line before this one.
Done
--
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: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Edward Haas
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 21: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 20: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 20 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Edward Haas has posted comments on this change.
Change subject: net: introduce acquire module
..
Patch Set 19: Code-Review-1
(11 comments)
https://gerrit.ovirt.org/#/c/60974/19/lib/vdsm/network/ifacquire.py
File lib/vdsm/network/ifacquire.py:
Line 51:
Line 52: def __enter__(self):
Line 53: return self
Line 54:
Line 55: def __exit__(self, type, value, traceback):
We better log the exception and the transaction failure.
Line 56: if type is None:
Line 57: try:
Line 58: self._disable_onboot()
Line 59: except:
Line 65: for iface, ifcfg_lines in six.iteritems(self._ifaces):
Line 66: if ifcfg_lines:
Line 67: _rollback_ifcfg_iface(iface, ifcfg_lines)
Line 68:
Line 69: def acquire(self, ifaces):
Move after __init__ please or right after __exit__.
Line 70: self._backup(ifaces)
Line 71: self._turn_down()
Line 72:
Line 73: def _backup(self, ifaces):
PS19, Line 80: _turn_down
How about _release_iface ?
PS19, Line 83: turn_down
And here 'release' as well... (if you agree with the previous one)
For non_ifcfg as well
https://gerrit.ovirt.org/#/c/60974/19/tests/network/ifacquire_test.py
File tests/network/ifacquire_test.py:
PS19, Line 82: mock_isfile
not needed, no need to generate a MagicMock, just replace 'return_value' with a
lambda.
@mock.patch.object(ifacquire.os.path, 'isfile', lambda x: False)
PS19, Line 109: mock_isfile
not needed.
PS19, Line 113: handle
How about 'file' instead of 'handle'?
True for all 'handle' usages...
Line 116: atomic_write_handle.readlines.return_value = original_ifcfg
Line 117:
Line 118: with ifacquire.Transaction(netinfo_nets={}) as a:
Line 119: a.acquire(ifaces=[NIC_NAME])
Line 120: atomic_write_handle.writelines.assert_called_with(
New line before this please.
Please separate between the action and the assertion parts.. just for
readability.
Line 121: ifcfg_after_turn_down)
Line 122: atomic_write_handle.readlines.return_value =
ifcfg_after_turn_down
Line 123: mock_ifdown.assert_called_with(NIC_NAME)
Line 124: atomic_write_handle.writelines.assert_called_with(
Line 120: atomic_write_handle.writelines.assert_called_with(
Line 121: ifcfg_after_turn_down)
Line 122: atomic_write_handle.readlines.return_value =
ifcfg_after_turn_down
Line 123: mock_ifdown.assert_called_with(NIC_NAME)
Line 124: atomic_write_handle.writelines.assert_called_with(
New line before this please
Line 125: ifcfg_after_disable_onboot)
Line 126:
Line 127: @mock.patch.object(ifacquire.ifcfg, 'ifdown', return_value=None)
Line 128: @mock.patch.object(ifacquire.os.path, 'isfile', return_value=True)
PS19, Line 134: mock_isfile,
: mock_ifdown
No need for these two.
@mock.patch.object(ifacquire.ifcfg, 'ifdown', lambda x: None)
@mock.patch.object(ifacquire.os.path, 'isfile', lambda x: True)
Line 140:
Line 141: with self.assertRaises(TestException):
Line 142: with ifacquire.Transaction(netinfo_nets={}) as a:
Line 143: a.acquire(ifaces=[NIC_NAME])
Line 144: raise TestException()
new line before this one.
--
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: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Edward Haas
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Petr Horáček has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 19: Verified+1 Passed ifacquire_test.py -- 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: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 19: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 18: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 17: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 17 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Edward Haas has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 16: (2 comments) https://gerrit.ovirt.org/#/c/60974/16/tests/network/ifacquire_test.py File tests/network/ifacquire_test.py: Line 112: def _test_acquire_ifcfg_nic(self, mock_isfile, mock_ifdown, Line 113: original_ifcfg, Line 114: ifcfg_after_turn_down, Line 115: ifcfg_after_disable_onboot): Line 116: with mock_ifacquire_opens() as (open_handle, atomic_write_handle): > Mock themselves or mock_ifacquire_opens context manager? The mocks, there will be no need for the helper context manager. I'm not clear what is not working exactly and why you had to use a helper to create the two mocks. Line 117: open_handle.readlines.return_value = original_ifcfg Line 118: atomic_write_handle.readlines.return_value = original_ifcfg Line 119: Line 120: with ifacquire.Transaction() as a: Line 133: Line 134: @mock.patch.object(ifacquire.ifcfg, 'ifdown', return_value=None) Line 135: @mock.patch.object(ifacquire.os.path, 'isfile', return_value=True) Line 136: def test_rollback_acquired_ifcfg_nic(self, mock_isfile, mock_ifdown): Line 137: with mock_ifacquire_opens() as (open_handle, atomic_write_handle): > How? Just because there is no helper fun in between like in the previous case. Line 138: open_handle.readlines.return_value = NIC_IFCFG Line 139: atomic_write_handle.readlines.return_value = NIC_IFCFG Line 140: Line 141: with self.assertRaises(TestException): -- 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 Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Petr Horáček has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 16: (3 comments) 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: > Hmm... yea even this is a bit ugly. Done Line 74: a.prepare(ifaces=[NIC_NAME], netinfo_nets=NETINFO_NETS) Line 75: a.turn_down() Line 76: a.disable_onboot() Line 77: Line 112: def _test_acquire_ifcfg_nic(self, mock_isfile, mock_ifdown, Line 113: original_ifcfg, Line 114: ifcfg_after_turn_down, Line 115: ifcfg_after_disable_onboot): Line 116: with mock_ifacquire_opens() as (open_handle, atomic_write_handle): > OK, I think I understand now... this with with made me dizzy. Mock themselves or mock_ifacquire_opens context manager? Line 117: open_handle.readlines.return_value = original_ifcfg Line 118: atomic_write_handle.readlines.return_value = original_ifcfg Line 119: Line 120: with ifacquire.Transaction() as a: Line 133: Line 134: @mock.patch.object(ifacquire.ifcfg, 'ifdown', return_value=None) Line 135: @mock.patch.object(ifacquire.os.path, 'isfile', return_value=True) Line 136: def test_rollback_acquired_ifcfg_nic(self, mock_isfile, mock_ifdown): Line 137: with mock_ifacquire_opens() as (open_handle, atomic_write_handle): > I guess here it is even easier to get rid of the context. How? Line 138: open_handle.readlines.return_value = NIC_IFCFG Line 139: atomic_write_handle.readlines.return_value = NIC_IFCFG Line 140: Line 141: with self.assertRaises(TestException): -- 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 Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Edward Haas has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 16: (1 comment) 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: > disable_onboot is called after connectivity_check, so it should be all good Hmm... yea even this is a bit ugly. But it somehow feels more correct to give as much as little control to the transaction user on things that are expected. I mean, the context gives the ability to define an init action, arbitrary user actions and an exit point action. If we require the exit point to always occur, seems that it fits the exit point. 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 Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
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 Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Edward Haas has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 16: (4 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. used_ports can be called owned_ports. 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: > it cannot be part of the init, take a look at https://gerrit.ovirt.org/#/c/ I find 'prepare' and 'turn_down' hard to understand, it does not describe well what it does in relation to the acquire. How about something like this: with ifacquire.Transaction(netinfo) as a: do_somthing() a.acquire(iface) do_more() Regarding the 'disable_onboot': What will happen if setSafeNetworkConfig command will not arrive? I guess this means that we need to restore the ifcfg file to a 'release' state. Ok so how about applying the disable onboot and the context exit point (if no exceptions are detected)? Line 74: a.prepare(ifaces=[NIC_NAME], netinfo_nets=NETINFO_NETS) Line 75: a.turn_down() Line 76: a.disable_onboot() Line 77: Line 112: def _test_acquire_ifcfg_nic(self, mock_isfile, mock_ifdown, Line 113: original_ifcfg, Line 114: ifcfg_after_turn_down, Line 115: ifcfg_after_disable_onboot): Line 116: with mock_ifacquire_opens() as (open_handle, atomic_write_handle): > "So I would recommend mocking only utils.atomic_write, checking that it is OK, I think I understand now... this with with made me dizzy. How about moving the mocks to the caller func, it will make it cleaner without the need of the double 'with' blocks. Line 117: open_handle.readlines.return_value = original_ifcfg Line 118: atomic_write_handle.readlines.return_value = original_ifcfg Line 119: Line 120: with ifacquire.Transaction() as a: Line 133: Line 134: @mock.patch.object(ifacquire.ifcfg, 'ifdown', return_value=None) Line 135: @mock.patch.object(ifacquire.os.path, 'isfile', return_value=True) Line 136: def test_rollback_acquired_ifcfg_nic(self, mock_isfile, mock_ifdown): Line 137: with mock_ifacquire_opens() as (open_handle, atomic_write_handle): I guess here it is even easier to get rid of the context. Line 138: open_handle.readlines.return_value = NIC_IFCFG Line 139: atomic_write_handle.readlines.return_value = NIC_IFCFG Line 140: Line 141: with self.assertRaises(TestException): -- 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 Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
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/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'm not clear of why it is split between prepare and turn_down. Couldn't th it cannot be part of the init, take a look at https://gerrit.ovirt.org/#/c/60404/28/lib/vdsm/network/netswitch.py. we must open Setup inside ifacquire.Transaction (if we want to call disable_onboot from that), but setup.acquired_ifaces are not available back then. prepare can be part of turn_down, but then we must say, that turn_down must be called before disable_onboot. i will change it if you prefer it. i like separate prepare as 'postponed __init__'. i think it fits here well. this is the last point of network setup (except post_setup_hook which must not fail) before we finish setupNetworks. if it fail now, we are still able to rollback preexisting networking configuration. if we move it outside it it won't save our networking after reboot -- nic would be attached to OVS and even with IP on top it it wouldn't be able to reach outside network. Line 74: a.prepare(ifaces=[NIC_NAME], netinfo_nets=NETINFO_NETS) Line 75: a.turn_down() Line 76: a.disable_onboot() Line 77: Line 112: def _test_acquire_ifcfg_nic(self, mock_isfile, mock_ifdown, Line 113: original_ifcfg, Line 114: ifcfg_after_turn_down, Line 115: ifcfg_after_disable_onboot): Line 116: with mock_ifacquire_opens() as (open_handle, atomic_write_handle): > I do not understand why you are testing the atomic write. "So I would recommend mocking only utils.atomic_write, checking that it is called with." i think that is what i do now, but i have to mock open as well because of it is used to backup original ifcfg. i mock open and atomic_write (in contextmanager, since i wanted to extract double with statement and handles) and assert if we called expected methods. Line 117: open_handle.readlines.return_value = original_ifcfg Line 118: atomic_write_handle.readlines.return_value = original_ifcfg Line 119: Line 120: with ifacquire.Transaction() as a: -- 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 Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Edward Haas has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 16: Code-Review-1 (2 comments) 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'm not clear of why it is split between prepare and turn_down. Couldn't these be part of the Transaction init? The disable_onboot should not be part of this transaction, it should be set when Engine instructs to persist the config. Line 74: a.prepare(ifaces=[NIC_NAME], netinfo_nets=NETINFO_NETS) Line 75: a.turn_down() Line 76: a.disable_onboot() Line 77: Line 112: def _test_acquire_ifcfg_nic(self, mock_isfile, mock_ifdown, Line 113: original_ifcfg, Line 114: ifcfg_after_turn_down, Line 115: ifcfg_after_disable_onboot): Line 116: with mock_ifacquire_opens() as (open_handle, atomic_write_handle): I do not understand why you are testing the atomic write. You just need to test ifacquire, assuming the atomic write works as expected (you should have tests for that already). So I would recommend mocking only utils.atomic_write, checking that it is called with. And use it as a decorator and drop the contextmanager.. Line 117: open_handle.readlines.return_value = original_ifcfg Line 118: atomic_write_handle.readlines.return_value = original_ifcfg Line 119: Line 120: with ifacquire.Transaction() as a: -- 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 Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 16: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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 Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Petr Horáček has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 15: Passed network/*_test.py. -- 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: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Petr Horáček has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 15: Verified+1 Passed network/_test.py. -- 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: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 15: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 15 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 14: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 14 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 13: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 12: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Edward Haas has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 11: Code-Review-1 (3 comments) Partial review https://gerrit.ovirt.org/#/c/60974/11/lib/vdsm/network/netswitch.py File lib/vdsm/network/netswitch.py: PS11, Line 164: disable NM This needs to happen before we start the setup transaction as a pre requirement. Or do we make sure no changes are applied before exiting the context? Line 167: _setup_ipv6autoconf(networks) Line 168: _set_ovs_links_up(nets2add, bonds2add, bonds2edit) Line 169: _setup_ovs_ip_config(nets2add, nets2remove) Line 170: connectivity.check(options) Line 171: # TODO: set onboot=no This needs to be done on 'safe' verb, not here. Line 172: Line 173: Line 174: def _update_running_config(networks, bondings, running_config): Line 175: """Update running_config with expected configuration. https://gerrit.ovirt.org/#/c/60974/11/tests/testlib.py File tests/testlib.py: Line 578: with open(path) as src: Line 579: return src.read() Line 580: Line 581: Line 582: class PersistingStringIO(io.StringIO): Not needed anymore, right? Line 583: """Keeps its contents even after it's closed. Can be used to mock files.""" Line 584: Line 585: def __init__(self, *args, **kwargs): Line 586: self._pre_close_content = None -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 11: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 10: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 9: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 8: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 7: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Petr Horáček has posted comments on this change.
Change subject: net: introduce acquire module
..
Patch Set 5:
(1 comment)
https://gerrit.ovirt.org/#/c/60974/5/tests/network/acquire_ifaces_test.py
File tests/network/acquire_ifaces_test.py:
Line 90: def test_acquire_ifcfg_persisted_nic(self, mock_os, mock_ifcfg):
Line 91: mock_os.path.isfile.return_value = True
Line 92: mock_ifcfg.ifdown.return_value = None
Line 93:
Line 94: mock_file = PersistingStringIO(NIC_IFCFG)
> Have you specified what readlines() returns?
I set only mock_open(read_data=...) I will try to mock readlines as well.
Line 95: with mock.patch('vdsm.network.acquire.open',
return_value=mock_file,
Line 96: create=True):
Line 97:
Line 98: acquire.acquire_unowned_ifaces(
--
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: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Edward Haas
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Edward Haas has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 6: (1 comment) https://gerrit.ovirt.org/#/c/60974/5/tests/network/acquire_ifaces_test.py File tests/network/acquire_ifaces_test.py: Line 90 Line 91 Line 92 Line 93 Line 94 > I'm not sure, i tried that but got this: Have you specified what readlines() returns? m_open.readlines.return_value = NIC_IFCFG.format(NIC_NAME).split() -- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Petr Horáček Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 6: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Petr Horáček has posted comments on this change.
Change subject: net: introduce acquire module
..
Patch Set 6:
(18 comments)
https://gerrit.ovirt.org/#/c/60974/5//COMMIT_MSG
Commit Message:
PS5, Line 9: faces, both pers
> 'external' seems redundant and I would use ifaces, seems more focused to wh
Done
https://gerrit.ovirt.org/#/c/60974/5/lib/vdsm/network/acquire.py
File lib/vdsm/network/acquire.py:
> The name is a bit too generic... acquire what?
Done
PS5, Line 40:
> can be removed
Done
PS5, Line 41:
> can be removed
Done
PS5, Line 49:
> can be removed
Done
PS5, Line 54:
> Did you mean non_ifcfg? There can be other types of persistence.
Done
PS5, Line 57:
> can be removed
Done
Line 61
Line 62
Line 63
Line 64
Line 65
> Add TODOL For NM, the file needs to be reloaded explicitly.
Done
PS5, Line 74:
> can be removed
Done
https://gerrit.ovirt.org/#/c/60974/5/tests/network/acquire_ifaces_test.py
File tests/network/acquire_ifaces_test.py:
PS5, Line 26:
> not needed
Done
Line 52
Line 53
Line 54
Line 55
Line 56
> @mock.patch.object(acquire, 'address') is nicer in this case.
Done
PS5, Line 60:
> Can be moved to the class level as it is common to all.
Each of them use different return_value. I think it is better to have each test
with all needed mocking near it.
PS5, Line 61:
> owned
Done
Line 62
Line 63
Line 64
Line 65
Line 66
> There is no need to check this, it is an internal intermediate usage and no
Done
Line 73
Line 74
Line 75
Line 76
Line 77
> If it is not costing a line, try to add the return_value in the patch defin
Done
Line 87
Line 88
Line 89
Line 90
Line 91
> If it is not costing a line, try to add the return_value in the patch defin
Done
Line 90
Line 91
Line 92
Line 93
Line 94
> Will this work?
I'm not sure, i tried that but got this:
[call('/etc/sysconfig/network-scripts/ifcfg-eth3', 'r+'),
call().__enter__(),
call().readlines(),
call().readlines().__iter__(),
call().seek(0),
call().writelines(),
call().write(u'\n# This device is now owned by
VDSM\nONBOOT=no\nNM_CONTROLLED=no\n# end of VDSM suffix.'),
call().__exit__(None, None, None)]
I cannot assert writelines with original string/list.
Line 93
Line 94
Line 95
Line 96
Line 97
> Why new line?
Done
--
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Petr Horáček
Gerrit-Reviewer: Dan Kenigsberg
Gerrit-Reviewer: Edward Haas
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček
Gerrit-Reviewer: gerrit-hooks
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Edward Haas has posted comments on this change.
Change subject: net: introduce acquire module
..
Patch Set 5: Code-Review-1
(18 comments)
https://gerrit.ovirt.org/#/c/60974/5//COMMIT_MSG
Commit Message:
PS5, Line 9: external devices
'external' seems redundant and I would use ifaces, seems more focused to what
we do.
https://gerrit.ovirt.org/#/c/60974/5/lib/vdsm/network/acquire.py
File lib/vdsm/network/acquire.py:
The name is a bit too generic... acquire what?
Here some options for you too choose from:
netacquire
ifacquire
acquire_iface
Line 1: # Copyright 2016 Red Hat, Inc.
Line 2: #
Line 3: # This program is free software; you can redistribute it and/or modify
Line 4: # it under the terms of the GNU General Public License as published by
PS5, Line 40: external_
can be removed
PS5, Line 41: external
can be removed
PS5, Line 49: external
can be removed
PS5, Line 54: non_persistent
Did you mean non_ifcfg? There can be other types of persistence.
PS5, Line 57: external
can be removed
Line 61: _comment_out_parameters(('ONBOOT', 'NM_CONTROLLED'), lines)
Line 62: f.seek(0)
Line 63: f.writelines(lines)
Line 64: f.write(ACQUIRED_IFCFG_SUFFIX)
Line 65: ifcfg.ifdown(iface)
Add TODOL For NM, the file needs to be reloaded explicitly.
Line 66:
Line 67:
Line 68: def _comment_out_parameters(parameters, lines):
Line 69: for i, line in enumerate(lines):
PS5, Line 74: external
can be removed
https://gerrit.ovirt.org/#/c/60974/5/tests/network/acquire_ifaces_test.py
File tests/network/acquire_ifaces_test.py:
PS5, Line 26: as TestCaseBase
not needed
Line 52:
Line 53: @attr(type='unit')
Line 54: class AcquireNicTest(TestCaseBase):
Line 55:
Line 56: @mock.patch('vdsm.network.acquire.address')
@mock.patch.object(acquire, 'address') is nicer in this case.
Same for the rest.
Line 57: @mock.patch('vdsm.network.acquire.dhclient')
Line 58: @mock.patch('vdsm.network.acquire.ifcfg')
Line 59: @mock.patch('vdsm.network.acquire.open', create=True)
Line 60: @mock.patch('vdsm.network.acquire.os')
PS5, Line 60: @mock.patch('vdsm.network.acquire.os')
Can be moved to the class level as it is common to all.
PS5, Line 61: acquired
owned
Line 62: mock_dhclient, mock_address):
Line 63: acquire.acquire_unowned_ifaces(
Line 64: netinfo_nets=NETINFO_NETS, external_ifaces=[NIC_NAME])
Line 65:
Line 66: mock_os.path.isfile.assert_not_called()
There is no need to check this, it is an internal intermediate usage and not a
result.
A test cares about a result only.
You use a mock if the result is not returned but passed to a 3rd party (like
creating a file).
Line 67: mock_open.assert_not_called()
Line 68: mock_ifcfg.ifdown.assert_not_called()
Line 69: mock_dhclient.kill.assert_not_called()
Line 70: mock_address.flush.assert_not_called()
Line 73: @mock.patch('vdsm.network.acquire.dhclient')
Line 74: @mock.patch('vdsm.network.acquire.os')
Line 75: def test_acquire_non_persisted_nic(self, mock_os, mock_dhclient,
Line 76:mock_address):
Line 77: mock_os.path.isfile.return_value = False
If it is not costing a line, try to add the return_value in the patch
definition.
Line 78: mock_dhclient.kill.return_value = None
Line 79: mock_address.flush.return_value = None
Line 80:
Line 81: acquire.acquire_unowned_ifaces(
Line 87:
Line 88: @mock.patch('vdsm.network.acquire.ifcfg')
Line 89: @mock.patch('vdsm.network.acquire.os')
Line 90: def test_acquire_ifcfg_persisted_nic(self, mock_os, mock_ifcfg):
Line 91: mock_os.path.isfile.return_value = True
If it is not costing a line, try to add the return_value in the patch
definition.
Line 92: mock_ifcfg.ifdown.return_value = None
Line 93:
Line 94: mock_file = PersistingStringIO(NIC_IFCFG)
Line 95: with mock.patch('vdsm.network.acquire.open',
return_value=mock_file,
Line 90: def test_acquire_ifcfg_persisted_nic(self, mock_os, mock_ifcfg):
Line 91: mock_os.path.isfile.return_value = True
Line 92: mock_ifcfg.ifdown.return_value = None
Line 93:
Line 94: mock_file = PersistingStringIO(NIC_IFCFG)
Will this work?
http://www.voidspace.org.uk/python/mock/helpers.html#mock-open
Do we have the correct version of mock for it to work?
(For EL7)
Line 95: with mock.patch('vdsm.network.acquire.open',
return_value=mock_file,
Line 96: create=True):
Line 97:
Line 98: acquire.acquire_unowned_ifaces(
Line 93:
Line 94: mock_file = PersistingStringIO(NIC_IFCFG)
Line 95: with mock.patch('vdsm.network.acquire.open',
return_value=mock_file,
Line 96: create=True):
Line 97:
Why new line?
Line 98: acquire.acquire_unowned_ifa
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 5: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 4: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 3: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 2: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Edward Haas Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
gerrit-hooks has posted comments on this change. Change subject: net: introduce acquire module .. Patch Set 1: * #1195208::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1195208::OK, public bug * Check Product::#1195208::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- 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: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Petr Horáček Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: net: introduce acquire module
Petr Horáček has uploaded a new change for review.
Change subject: net: introduce acquire module
..
net: introduce acquire module
This module will be used to acquire external devices, both persisted
by ifcfg and not persisted. It is needed for OVS switch (and eventually
also for iproute2 and pyroute2).
This patch also introduces PersistingStringIO which is needed for files
mocking.
Change-Id: I180cfd7d69c0ae0a24188bc3d909b9d3d7c12145
Bug-Url: https://bugzilla.redhat.com/1195208
Signed-off-by: Petr Horáček
---
M lib/vdsm/network/Makefile.am
A lib/vdsm/network/acquire.py
A tests/network/acquire_ifaces_test.py
M tests/testlib.py
M vdsm.spec.in
5 files changed, 206 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/74/60974/1
diff --git a/lib/vdsm/network/Makefile.am b/lib/vdsm/network/Makefile.am
index 3889776..52a5719 100644
--- a/lib/vdsm/network/Makefile.am
+++ b/lib/vdsm/network/Makefile.am
@@ -24,6 +24,7 @@
vdsmnetworkdir = $(vdsmpylibdir)/network
dist_vdsmnetwork_PYTHON = \
__init__.py \
+ acquire.py \
api.py \
errors.py \
canonicalize.py \
diff --git a/lib/vdsm/network/acquire.py b/lib/vdsm/network/acquire.py
new file mode 100644
index 000..3bd4cda
--- /dev/null
+++ b/lib/vdsm/network/acquire.py
@@ -0,0 +1,78 @@
+# Copyright 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+# Refer to the README and COPYING files for full details of the license
+#
+from __future__ import absolute_import
+
+import itertools
+import os
+
+import six
+
+from .configurators import ifcfg
+from .ip import address
+from .ip import dhclient
+
+
+ACQUIRED_IFCFG_SUFFIX = (
+u'\n'
+'# This device is now owned by VDSM\n'
+'ONBOOT=no\n'
+'NM_CONTROLLED=no\n'
+'# end of VDSM suffix.'
+)
+
+
+def acquire_unowned_ifaces(netinfo_nets, external_ifaces):
+"""Acquire external ifaces which are not owned by us."""
+used_ports = frozenset(itertools.chain.from_iterable(
+[attrs['ports'] for attrs in six.itervalues(netinfo_nets)]))
+for iface in external_ifaces:
+if iface not in used_ports:
+_acquire_external_iface(iface)
+
+
+def _acquire_external_iface(iface):
+is_ifcfg_controlled = os.path.isfile(ifcfg.NET_CONF_PREF + iface)
+if is_ifcfg_controlled:
+_acquire_external_ifcfg_iface(iface)
+else:
+_acquire_external_non_persistent_iface(iface)
+
+
+def _acquire_external_ifcfg_iface(iface):
+# TODO: Test that NM does not turn iface UP again.
+with open(ifcfg.NET_CONF_PREF + iface, 'r+') as f:
+lines = f.readlines()
+_comment_out_parameters(('ONBOOT', 'NM_CONTROLLED'), lines)
+f.seek(0)
+f.writelines(lines)
+f.write(ACQUIRED_IFCFG_SUFFIX)
+ifcfg.ifdown(iface)
+
+
+def _comment_out_parameters(parameters, lines):
+for i, line in enumerate(lines):
+if any([line.startswith(parameter) for parameter in parameters]):
+lines[i] = '#' + line
+
+
+def _acquire_external_non_persistent_iface(iface):
+# TODO: Tell NM to ignore this iface if it is needed.
+dhclient.kill(iface, family=4)
+dhclient.kill(iface, family=6)
+address.flush(iface)
diff --git a/tests/network/acquire_ifaces_test.py
b/tests/network/acquire_ifaces_test.py
new file mode 100644
index 000..815adb2
--- /dev/null
+++ b/tests/network/acquire_ifaces_test.py
@@ -0,0 +1,104 @@
+# Copyright 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301 USA
+#
+# Refer to the README and COPYING files fo
