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 asegu...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
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 asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
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: 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 asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com 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 asegu...@redhat.com
---
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: BROADCAST,MULTICAST,MASTER
- 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: BROADCAST,MULTICAST,UP,LOWER_UP \n
- link/ether \n)
+out = ('273: bond0: BROADCAST,MULTICAST,MASTER mtu 1500 qdisc '
+ 'noqueue state DOWN \\link/ether 33:44:55:66:77:88 brd '
+ 'ff:ff:ff:ff:ff:ff \n'
+ '4: wlp3s0: BROADCAST,MULTICAST,UP,LOWER_UP \\'
+ 'link/ether \n'
+ 'Deleted 418: foo: BROADCAST,MULTICAST 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 asegu...@redhat.com
___
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 asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com 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 asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com 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
