Change in vdsm[master]: tc: refine unsetPortMirroring
Igor Lvovsky has posted comments on this change. Change subject: tc: refine unsetPortMirroring .. Patch Set 6: Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/7299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
Dan Kenigsberg has submitted this change and it was merged. Change subject: tc: refine unsetPortMirroring .. tc: refine unsetPortMirroring unsetPortMirroring should not remove all reference of the mirrored bridge device. Instead, remove only the relevant mirred actions on each call to unsetPortMirroring. Promiscuous mode should be unset only when the last action is removed. Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Signed-off-by: Dan Kenigsberg dan...@redhat.com --- M tests/tcTests.py M vdsm/libvirtvm.py M vdsm/supervdsmServer.py M vdsm/tc.py 4 files changed, 64 insertions(+), 16 deletions(-) Approvals: Antoni Segura Puimedon: Verified; Looks good to me, but someone else must approve Dan Kenigsberg: Igor Lvovsky: Looks good to me, approved -- To view, visit http://gerrit.ovirt.org/7299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
Dan Kenigsberg has posted comments on this change. Change subject: tc: refine unsetPortMirroring .. Patch Set 5: I would prefer that you didn't submit this (3 inline comments) File vdsm/tc.py Line 55: Line 56: Line 57: def setPortMirroring(network, target): Line 58: qdisc_replace_ingress(network) Line 59: _addTarget(network, ':', target) it may be nicer to define QDISC_INGRESS = ':' Line 60: Line 61: qdisc_replace_prio(network) Line 62: devid = qdisc_get_devid(network) Line 63: _addTarget(network, devid, target) Line 56: Line 57: def setPortMirroring(network, target): Line 58: qdisc_replace_ingress(network) Line 59: _addTarget(network, ':', target) Line 60: Igor notes that we have a problem if Vdsm dies right here. We would then have a half-defined mirroring, that unsetPortMirroring would not be able remove. Line 61: qdisc_replace_prio(network) Line 62: devid = qdisc_get_devid(network) Line 63: _addTarget(network, devid, target) Line 64: Line 58: qdisc_replace_ingress(network) Line 59: _addTarget(network, ':', target) Line 60: Line 61: qdisc_replace_prio(network) Line 62: devid = qdisc_get_devid(network) devid is a wrong and misleading name. it is the qdisc_id of the first egress qdisc of the device. Line 63: _addTarget(network, devid, target) Line 64: Line 65: set_promisc(network, True) Line 66: -- To view, visit http://gerrit.ovirt.org/7299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
Antoni Segura Puimedon has posted comments on this change. Change subject: tc: refine unsetPortMirroring .. Patch Set 5: No score (4 inline comments) File vdsm/tc.py Line 34: Exception.__init__(self, self.errCode, self.message, self.command) Line 35: Line 36: Line 37: def _addTarget(network, parent, target): Line 38: fs = list(filters(network, parent)) Ok, you know better the author of the filters ;-). Line 39: if fs: Line 40: filt = fs[0] Line 41: else: Line 42: filt = Filter(prio=None, handle=None, actions=[]) Line 55: Line 56: Line 57: def setPortMirroring(network, target): Line 58: qdisc_replace_ingress(network) Line 59: _addTarget(network, ':', target) Definitely. It had been about 6 years since I last played with traffic control commands and the magic number left me initially scratching my head. Line 60: Line 61: qdisc_replace_prio(network) Line 62: devid = qdisc_get_devid(network) Line 63: _addTarget(network, devid, target) Line 56: Line 57: def setPortMirroring(network, target): Line 58: qdisc_replace_ingress(network) Line 59: _addTarget(network, ':', target) Line 60: Some proposal? Doing it in some sort of transaction that rolls back in case of error/death? Line 61: qdisc_replace_prio(network) Line 62: devid = qdisc_get_devid(network) Line 63: _addTarget(network, devid, target) Line 64: Line 58: qdisc_replace_ingress(network) Line 59: _addTarget(network, ':', target) Line 60: Line 61: qdisc_replace_prio(network) Line 62: devid = qdisc_get_devid(network) Yes. I understood what it meant, but the new name is indeed more suitable. But aren't we normally using camelCase? qdiscId? Line 63: _addTarget(network, devid, target) Line 64: Line 65: set_promisc(network, True) Line 66: -- To view, visit http://gerrit.ovirt.org/7299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
oVirt Jenkins CI Server has posted comments on this change. Change subject: tc: refine unsetPortMirroring .. Patch Set 6: Build Successful http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/714/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/7299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
Antoni Segura Puimedon has posted comments on this change.
Change subject: tc: refine unsetPortMirroring
..
Patch Set 6: (1 inline comment)
File vdsm/tc.py
Line 120:'parent', 'root', 'prio']
Line 121: _process_request(command)
Line 122:
Line 123: def qdisc_get_devid(dev):
Line 124: Return qdisc_id of the first qdisc associated with dev
Would it be possible to change the method name instead, or is it already fixed
in the API?
Line 125:
Line 126: command = [EXT_TC, 'qdisc', 'show', 'dev', dev]
Line 127: out = _process_request(command)
Line 128: return out.split(' ')[2]
--
To view, visit http://gerrit.ovirt.org/7299
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
Dan Kenigsberg has posted comments on this change.
Change subject: tc: refine unsetPortMirroring
..
Patch Set 6: (1 inline comment)
File vdsm/tc.py
Line 120:'parent', 'root', 'prio']
Line 121: _process_request(command)
Line 122:
Line 123: def qdisc_get_devid(dev):
Line 124: Return qdisc_id of the first qdisc associated with dev
how about
qdisc_get_first_per_dev()
? That's rather lame. Do you have a better suggestion?
Line 125:
Line 126: command = [EXT_TC, 'qdisc', 'show', 'dev', dev]
Line 127: out = _process_request(command)
Line 128: return out.split(' ')[2]
--
To view, visit http://gerrit.ovirt.org/7299
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
Antoni Segura Puimedon has posted comments on this change.
Change subject: tc: refine unsetPortMirroring
..
Patch Set 6: (1 inline comment)
File vdsm/tc.py
Line 120:'parent', 'root', 'prio']
Line 121: _process_request(command)
Line 122:
Line 123: def qdisc_get_devid(dev):
Line 124: Return qdisc_id of the first qdisc associated with dev
The fact that it is the first makes a short and nice-sounding name difficult. I
guess that if it would return all the qdisc ids and then the caller would pick
just the first would make it easier (but it would be cheating on your question).
What I found weird is the grammatical inversion of direct object and verb,
qdisc_get_devid suggests to me that what we get as a return is a devid, not a
qdisc_id. Maybe get_qdisc_of_devid (the 'of' sounds lame, I know, it's what we
get by not having object (OO) or dictionary(FP) representations of the network
that would allow for nicer queries).
Line 125:
Line 126: command = [EXT_TC, 'qdisc', 'show', 'dev', dev]
Line 127: out = _process_request(command)
Line 128: return out.split(' ')[2]
--
To view, visit http://gerrit.ovirt.org/7299
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
Antoni Segura Puimedon has posted comments on this change. Change subject: tc: refine unsetPortMirroring .. Patch Set 5: Looks good to me, but someone else must approve (1 inline comment) File vdsm/tc.py Line 34: Exception.__init__(self, self.errCode, self.message, self.command) Line 35: Line 36: Line 37: def _addTarget(network, parent, target): Line 38: fs = list(filters(network, parent)) Just nitpicking: If the most usual use case has some filter already present, the most pythonic way to do lines 38-42 would be without asking for permission: try: filt = list(filters(network, parent))[0] except IndexError: filt = Filter(prio=None, handle=None, actions=[]) Line 39: if fs: Line 40: filt = fs[0] Line 41: else: Line 42: filt = Filter(prio=None, handle=None, actions=[]) -- To view, visit http://gerrit.ovirt.org/7299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
Antoni Segura Puimedon has posted comments on this change. Change subject: tc: refine unsetPortMirroring .. Patch Set 5: Verified -- To view, visit http://gerrit.ovirt.org/7299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
Dan Kenigsberg has posted comments on this change. Change subject: tc: refine unsetPortMirroring .. Patch Set 5: (1 inline comment) File vdsm/tc.py Line 34: Exception.__init__(self, self.errCode, self.message, self.command) Line 35: Line 36: Line 37: def _addTarget(network, parent, target): Line 38: fs = list(filters(network, parent)) This has its beauty, I must admit. Yet I have a little worry of bugs in filter() [I know who wrote that function ;-) ] and I do not want an IndexError there being considered OK here. a safer construct would be fs = list(filters(network, parent)) try: filt = fs[0] except IndexError: filt = Filter(prio=None, handle=None, actions=[]) which is no longer as pretty. I would consider it anyway. Line 39: if fs: Line 40: filt = fs[0] Line 41: else: Line 42: filt = Filter(prio=None, handle=None, actions=[]) -- To view, visit http://gerrit.ovirt.org/7299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
oVirt Jenkins CI Server has posted comments on this change. Change subject: tc: refine unsetPortMirroring .. Patch Set 3: Build Successful http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/613/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/7299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
oVirt Jenkins CI Server has posted comments on this change. Change subject: tc: refine unsetPortMirroring .. Patch Set 5: Build Successful http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/627/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/7299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
oVirt Jenkins CI Server has posted comments on this change. Change subject: tc: refine unsetPortMirroring .. Patch Set 2: Build Failed http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/596/ : FAILURE -- To view, visit http://gerrit.ovirt.org/7299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
Antoni Segura Puimedon has posted comments on this change. Change subject: tc: refine unsetPortMirroring .. Patch Set 2: (1 inline comment) I'm in the process of reviewing it, but I honestly could not find what mirred is, so I might miss some semantics of the change. Commit Message Line 6: Line 7: tc: refine unsetPortMirroring Line 8: Line 9: unsetPortMirroring should not remove all reference of the mirrored Line 10: bridge device. Instead, remove only the relevant mirred actions on each What is mirred? Line 11: call to unsetPortMirroring. Promiscuous mode should be unset only when Line 12: the last action is removed. Line 13: Line 14: Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 -- To view, visit http://gerrit.ovirt.org/7299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
Antoni Segura Puimedon has posted comments on this change. Change subject: tc: refine unsetPortMirroring .. Patch Set 2: (1 inline comment) Dismiss my previous comment. Will complete review later. Commit Message Line 6: Line 7: tc: refine unsetPortMirroring Line 8: Line 9: unsetPortMirroring should not remove all reference of the mirrored Line 10: bridge device. Instead, remove only the relevant mirred actions on each Found out in the tc documentation. Nevermind. Line 11: call to unsetPortMirroring. Promiscuous mode should be unset only when Line 12: the last action is removed. Line 13: Line 14: Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 -- To view, visit http://gerrit.ovirt.org/7299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Antoni Segura Puimedon asegu...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
Dan Kenigsberg has uploaded a new change for review. Change subject: tc: refine unsetPortMirroring .. tc: refine unsetPortMirroring unsetPortMirroring should not remove all reference of the mirrored bridge device. Instead, remove only the relevant filters on each call to unsetPortMirroring. Promiscuous mode should be unset only when the last filter is removed. Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Signed-off-by: Dan Kenigsberg dan...@redhat.com --- M tests/tcTests.py M vdsm/libvirtvm.py M vdsm/supervdsmServer.py M vdsm/tc.py 4 files changed, 28 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/99/7299/1 diff --git a/tests/tcTests.py b/tests/tcTests.py index c8facd2..327049b 100644 --- a/tests/tcTests.py +++ b/tests/tcTests.py @@ -258,6 +258,6 @@ self.assertTrue(self._sendPing(), Bridge received no mirrored ping requests.) -tc.unsetPortMirroring(self._bridge0.devName) +tc.unsetPortMirroring(self._bridge0.devName, self._bridge1.devName) self.assertFalse(self._sendPing(), Bridge received mirrored ping requests, but mirroring is unset.) diff --git a/vdsm/libvirtvm.py b/vdsm/libvirtvm.py index 3b7cfc5..532dbb1 100644 --- a/vdsm/libvirtvm.py +++ b/vdsm/libvirtvm.py @@ -2068,7 +2068,7 @@ for nic in self._devices[vm.NIC_DEVICES]: if hasattr(nic, 'portMirroring'): for network in nic.portMirroring: -supervdsm.getProxy().unsetPortMirroring(network) +supervdsm.getProxy().unsetPortMirroring(network, nic.name) # delete the payload devices for drive in self._devices[vm.DISK_DEVICES]: diff --git a/vdsm/supervdsmServer.py b/vdsm/supervdsmServer.py index 535a660..371dc04 100755 --- a/vdsm/supervdsmServer.py +++ b/vdsm/supervdsmServer.py @@ -257,14 +257,16 @@ tc.setPortMirroring(networkName, ifaceName) @logDecorator -def unsetPortMirroring(self, networkName): +def unsetPortMirroring(self, networkName, target): ''' -Release captured mirror networkName traffic from networkName bridge +Release captured mirror networkName traffic from networkName bridge :param networkName: networkName to release the traffic capture :type networkName: string +:param target: target device to release +:type target: string ''' -tc.unsetPortMirroring(networkName) +tc.unsetPortMirroring(networkName, target) @logDecorator def mkFloppyFs(self, vmId, files): diff --git a/vdsm/tc.py b/vdsm/tc.py index 835ae2c..9091d69 100644 --- a/vdsm/tc.py +++ b/vdsm/tc.py @@ -41,10 +41,22 @@ add_filter(network, target, devid) set_promisc(network, True) -def unsetPortMirroring(network): -qdisc_del(network, 'root') -qdisc_del(network, 'ingress') -set_promisc(network, False) +def unsetPortMirroring(network, target): +rootfilters = tuple(filters(network, ':')) +devid = qdisc_get_devid(network) +priofilters = tuple(filters(network, devid)) + +for f in rootfilters: +if f.target == target: +del_filter(network, target, ':', f.prio) +for f in priofilters: +if f.target == target: +del_filter(network, target, devid, f.prio) + +if not [f for f in rootfilters + priofilters if f.target != target]: +qdisc_del(network, 'root') +qdisc_del(network, 'ingress') +set_promisc(network, False) def _process_request(command): retcode, out, err = storage.misc.execCmd(command, raw=True, sudo=False) @@ -63,6 +75,11 @@ 'action', 'mirred', 'egress', 'mirror', 'dev', target] _process_request(command) +def del_filter(dev, target, parent, prio): +command = [EXT_TC, 'filter', 'del', 'dev', dev, 'parent', parent, + 'prio', prio] +_process_request(command) + def qdisc_replace_parent(dev): command = [EXT_TC, 'qdisc', 'replace', 'dev', dev, 'parent', 'root', 'prio'] -- To view, visit http://gerrit.ovirt.org/7299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg dan...@redhat.com ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: tc: refine unsetPortMirroring
oVirt Jenkins CI Server has posted comments on this change. Change subject: tc: refine unsetPortMirroring .. Patch Set 1: Build Successful http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/498/ : SUCCESS -- To view, visit http://gerrit.ovirt.org/7299 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I09b315e7398c2293dc6f84ab63853b70308b2875 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
