Change in vdsm[master]: Fix ipwrapper Monitor for deleted links and oneline input
Antoni Segura Puimedon has posted comments on this change. Change subject: Fix ipwrapper Monitor for deleted links and oneline input .. Patch Set 3: (3 comments) File lib/vdsm/ipwrapper.py Line 544: self.proc.kill() Line 545: Line 546: @classmethod Line 547: def _parse(cls, text): Line 548: deletionText = 'Deleted' Done Line 549: changes = [] Line 550: for line in text.splitlines(): Line 551: state = None Line 552: if line.startswith(deletionText): Line 549: changes = [] Line 550: for line in text.splitlines(): Line 551: state = None Line 552: if line.startswith(deletionText): Line 553: state = deletionText.upper() Done Line 554: line = line[len(deletionText):] Line 555: Line 556: _, device, data = [el.strip() for el in line.split(':', 2)] Line 557: flagVal, _ = data.split('\\', 1) # We don't parse link/ether Line 552: if line.startswith(deletionText): Line 553: state = deletionText.upper() Line 554: line = line[len(deletionText):] Line 555: Line 556: _, device, data = [el.strip() for el in line.split(':', 2)] Done Line 557: flagVal, _ = data.split('\\', 1) # We don't parse link/ether Line 558: Line 559: flags, values = data.split('>') Line 560: flags = frozenset(flags[1:].split(',')) -- To view, visit http://gerrit.ovirt.org/21411 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1c593467d328c39654eae723126f889b51ff39d2 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg 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
Change in vdsm[master]: Fix ipwrapper Monitor for deleted links and oneline input
Dan Kenigsberg has posted comments on this change. Change subject: Fix ipwrapper Monitor for deleted links and oneline input .. Patch Set 3: Code-Review-1 (3 comments) File lib/vdsm/ipwrapper.py Line 544: self.proc.kill() Line 545: Line 546: @classmethod Line 547: def _parse(cls, text): Line 548: deletionText = 'Deleted' That should be named like a CONSTANT. Line 549: changes = [] Line 550: for line in text.splitlines(): Line 551: state = None Line 552: if line.startswith(deletionText): Line 549: changes = [] Line 550: for line in text.splitlines(): Line 551: state = None Line 552: if line.startswith(deletionText): Line 553: state = deletionText.upper() I'm not sure we should tie the reported state to the found string via upper(). I think that it would be cleaner to have two constants: _DELETED_TEXT = 'Deleted' LINK_STATE_DELETED = 'DELETED' Line 554: line = line[len(deletionText):] Line 555: Line 556: _, device, data = [el.strip() for el in line.split(':', 2)] Line 557: flagVal, _ = data.split('\\', 1) # We don't parse link/ether Line 552: if line.startswith(deletionText): Line 553: state = deletionText.upper() Line 554: line = line[len(deletionText):] Line 555: Line 556: _, device, data = [el.strip() for el in line.split(':', 2)] for a review of better quality, would you please separate the move to -d -o from the handling of "Deleted"? Verifying correctness of parsing is harder as it is. Line 557: flagVal, _ = data.split('\\', 1) # We don't parse link/ether Line 558: Line 559: flags, values = data.split('>') Line 560: flags = frozenset(flags[1:].split(',')) -- To view, visit http://gerrit.ovirt.org/21411 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1c593467d328c39654eae723126f889b51ff39d2 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg 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
Change in vdsm[master]: Fix ipwrapper Monitor for deleted links and oneline input
oVirt Jenkins CI Server has posted comments on this change. Change subject: Fix ipwrapper Monitor for deleted links and oneline input .. Patch Set 3: Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4752/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5552/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_network_functional_tests/827/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5635/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/21411 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1c593467d328c39654eae723126f889b51ff39d2 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix ipwrapper Monitor for deleted links and oneline input
oVirt Jenkins CI Server has posted comments on this change. Change subject: Fix ipwrapper Monitor for deleted links and oneline input .. Patch Set 3: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4752/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5552/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_network_functional_tests/827/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5632/ : FAILURE -- To view, visit http://gerrit.ovirt.org/21411 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1c593467d328c39654eae723126f889b51ff39d2 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix ipwrapper Monitor for deleted links and oneline input
oVirt Jenkins CI Server has posted comments on this change. Change subject: Fix ipwrapper Monitor for deleted links and oneline input .. Patch Set 2: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4718/ : FAILURE http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5518/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_network_functional_tests/824/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5598/ : FAILURE -- To view, visit http://gerrit.ovirt.org/21411 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1c593467d328c39654eae723126f889b51ff39d2 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix ipwrapper Monitor for deleted links and oneline input
oVirt Jenkins CI Server has posted comments on this change. Change subject: Fix ipwrapper Monitor for deleted links and oneline input .. Patch Set 1: Verified-1 Build Failed http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/4717/ : FAILURE http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/5517/ : SUCCESS http://jenkins.ovirt.org/job/vdsm_network_functional_tests/823/ : FAILURE http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/5597/ : FAILURE -- To view, visit http://gerrit.ovirt.org/21411 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1c593467d328c39654eae723126f889b51ff39d2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: Fix ipwrapper Monitor for deleted links and oneline input
Antoni Segura Puimedon has uploaded a new change for review. Change subject: Fix ipwrapper Monitor for deleted links and oneline input .. Fix ipwrapper Monitor for deleted links and oneline input The previous parsing would not work when a link was deleted. This patch fixes it and sets the state as DELETED on those cases. It also changes the ip monitor command to use the more machine readable "ip -d -o monitor link" (one line per link event). Change-Id: I1c593467d328c39654eae723126f889b51ff39d2 Signed-off-by: Antoni S. Puimedon --- M lib/vdsm/ipwrapper.py M tests/ipwrapperTests.py 2 files changed, 32 insertions(+), 17 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/11/21411/1 diff --git a/lib/vdsm/ipwrapper.py b/lib/vdsm/ipwrapper.py index 69be2aa..85cd919 100644 --- a/lib/vdsm/ipwrapper.py +++ b/lib/vdsm/ipwrapper.py @@ -527,34 +527,42 @@ MonitorEvent = namedtuple('MonitorEvent', ['device', 'flags', 'state']) -class Monitor(): +class Monitor(object): """Minimal wrapper over `ip monitor link`""" def __init__(self): self.proc = None def start(self): -self.proc = execCmd([_IP_BINARY.cmd, 'monitor', 'link'], sync=False) +self.proc = execCmd([_IP_BINARY.cmd, 'monitor', '-d', '-o', 'link'], +sync=False) def stop(self): self.proc.kill() @classmethod def _parse(cls, text): +deletionText = 'Deleted' changes = [] for line in text.splitlines(): -if line.startswith(' '): -continue +state = None +if line.startswith(deletionText): +state = deletionText.upper() +line = line[len(deletionText):] -tokens = line.split() -if not tokens[1].endswith(':'): -continue +_, device, data = [el.strip() for el in line.split(':', 2)] +flagVal, _ = data.split('\\', 1) # We don't parse link/ether -device = tokens[1][:-1] -flags = frozenset(tokens[2][1:-1].split(',')) -values = dict(tokens[i:i + 2] for i in range(3, len(tokens), 2)) +flags, values = data.split('>') +flags = frozenset(flags[1:].split(',')) +if state is None: +values = (el for el in values.strip().split(' ') if el) +for key, value in pairwise(values): +if key == 'state': +state = value +break -changes.append(MonitorEvent(device, flags, values.get('state'))) +changes.append(MonitorEvent(device, flags, state)) return changes diff --git a/tests/ipwrapperTests.py b/tests/ipwrapperTests.py index fb6bc02..d85deb3 100644 --- a/tests/ipwrapperTests.py +++ b/tests/ipwrapperTests.py @@ -190,11 +190,14 @@ self.assertRaises(ValueError, Rule.fromText, text) def testMonitorEvents(self): -out = ("273: bond0: " - "mtu 1500 qdisc noqueue state DOWN \n" - "link/ether 33:44:55:66:77:88 brd ff:ff:ff:ff:ff:ff \n" - "4: wlp3s0: \n" - "link/ether \n") +out = ('273: bond0: mtu 1500 qdisc ' + 'noqueue state DOWN \\link/ether 33:44:55:66:77:88 brd ' + 'ff:ff:ff:ff:ff:ff \n' + '4: wlp3s0: \\' + 'link/ether \n' + 'Deleted 418: foo: mtu 1500 qdisc noop ' + 'state DOWN group default \\link/ether ba:2c:7b:68:b8:77 ' + 'brd ff:ff:ff:ff:ff:ff\n') expected = [ MonitorEvent( 'bond0', @@ -203,6 +206,10 @@ MonitorEvent( 'wlp3s0', frozenset(['BROADCAST', 'MULTICAST', 'UP', 'LOWER_UP']), -None)] +None), +MonitorEvent( +'foo', +frozenset(['BROADCAST', 'MULTICAST']), +'DELETED')] self.assertEqual(Monitor._parse(out), expected) -- To view, visit http://gerrit.ovirt.org/21411 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1c593467d328c39654eae723126f889b51ff39d2 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Antoni Segura Puimedon ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches