Moti Asayag has uploaded a new change for review.
Change subject: BZ#811807 Define network filter on libvirt ......................................................................
BZ#811807 Define network filter on libvirt
The patch defines custom network filter on libvirt in order to control the VM traffic. The custom filter is being set when VDSM service is being started.
Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Signed-off-by: Moti Asayag masayag@redhat.com --- M vdsm.spec.in M vdsm/Makefile.am A vdsm/nwfilter.py M vdsm/vdsmd.init.in 4 files changed, 120 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/54/7354/1
diff --git a/vdsm.spec.in b/vdsm.spec.in index 892eee3..6af8a14 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -577,6 +577,7 @@ %{_datadir}/%{vdsm_name}/vdsm-store-net-config %{_datadir}/%{vdsm_name}/vm.py* %{_datadir}/%{vdsm_name}/write-net-config +%attr (755,vdsm,kvm) %{_datadir}/%{vdsm_name}/nwfilter.py* %config(noreplace) %{_sysconfdir}/%{vdsm_name}/logger.conf %config(noreplace) %{_sysconfdir}/%{vdsm_name}/mom.conf %config(noreplace) %{_sysconfdir}/%{vdsm_name}/mom.policy diff --git a/vdsm/Makefile.am b/vdsm/Makefile.am index 5992cfa..1a6d446 100644 --- a/vdsm/Makefile.am +++ b/vdsm/Makefile.am @@ -52,7 +52,8 @@ tc.py \ vdsmDebugPlugin.py \ vmChannels.py \ - vm.py + vm.py \ + nwfilter.py
dist_vdsmpylib_PYTHON = \ __init__.py \ diff --git a/vdsm/nwfilter.py b/vdsm/nwfilter.py new file mode 100755 index 0000000..0be7568 --- /dev/null +++ b/vdsm/nwfilter.py @@ -0,0 +1,101 @@ +#! /usr/bin/python +# Copyright 2012 IBM, 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 +# + +import libvirt +import logging +from vdsm import libvirtconnection + + +def main(): + """ + Defines network filters on libvirt + """ + conn = libvirtconnection.get() + defineNwFilter(conn, NoMacSpoofingFilter()) + try: + conn.close() + except: + pass + +def defineNwFilter(conn, nwFilter): + """ + define vdsm network filter on libvirt to control VM traffic + """ + + filter = None + filterName = nwFilter.filterName + filterXml = nwFilter.buildFilterXml() + + try: + filter = conn.nwfilterLookupByName(filterName) + except libvirt.libvirtError: + logging.debug("No such filter: %s" % (filterName)) + + if filter: + logging.debug("Undefine filter %s" % (filter.name())) + try: + filter.undefine() + except libvirt.libvirtError: + # Ignore failure if filter already exists. Failure might occur when + # attempting to remove a filter which is being used by running VMs + pass + try: + nwFilter = conn.nwfilterDefineXML(filterXml) + except libvirt.libvirtError: + logging.error("Failed to define filter %s" % (filterName)) + raise + else: + logging.debug("Filter %s was defined" % (nwFilter.name())) + + +class VdsmNwFilter(object): + """ + Base class for custom network filters + """ + + def __init__(self, name): + self.filterName = name + + def getFilterXml(self): + raise NotImplementedError("Should have implemented this") + + def buildFilterXml(self): + return self.getFilterXml() % self.filterName + + +class NoMacSpoofingFilter(VdsmNwFilter): + """ + Class defines the vdsm-no-mac-spoofing filter which is comprised of + two libvirt OOB filters: no-mac-spoofing and no-arp-mac-spoofing + """ + + def __init__(self): + VdsmNwFilter.__init__(self, 'vdsm-no-mac-spoofing') + + def getFilterXml(self): + return '''<filter name='%s' chain='root'> + <!-- preventing MAC spoofing --> + <filterref filter='no-mac-spoofing'/> + <!-- preventing ARP MAC spoofing --> + <filterref filter='no-arp-mac-spoofing'/> + </filter> ''' + +if __name__ == '__main__': + main() diff --git a/vdsm/vdsmd.init.in b/vdsm/vdsmd.init.in index d34b48d..70d4cd9 100755 --- a/vdsm/vdsmd.init.in +++ b/vdsm/vdsmd.init.in @@ -440,6 +440,20 @@ startout=`/sbin/initctl start libvirtd 2>&1` if [[ "$?" -eq 0 || "$startout" =~ .*already\ running.* ]]; then + + # Begin workaround + # Libvirt reports its service status as responsive despite its socket + # is not yet ready. Once issue fixed on libvirt, this WA shouls be removed. + for i in {1..20} + do + if pgrep libvirtd > /dev/null 2>&1; + then + return 0 + fi + sleep 1 + done + # End workaround + return 0 else echo "$startout" >&2 @@ -469,6 +483,8 @@ return $ret_val fi
+ @VDSMDIR@/nwfilter.py + @VDSMDIR@/vdsm-restore-net-config load_needed_modules mk_data_center
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com
oVirt Jenkins CI Server has posted comments on this change.
Change subject: BZ#811807 Define network filter on libvirt ......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/543/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Igor Lvovsky has posted comments on this change.
Change subject: BZ#811807 Define network filter on libvirt ......................................................................
Patch Set 1: (1 inline comment)
Dan asked to put it in tools, isn't he?
.................................................... File vdsm/vdsmd.init.in Line 452: fi Line 453: sleep 1 Line 454: done Line 455: # End workaround Line 456: What is this? Why now? How this related to nwfilter? If we really need it, I think it should be in different patch Line 457: return 0 Line 458: else Line 459: echo "$startout" >&2 Line 460: return 1
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Moti Asayag has posted comments on this change.
Change subject: BZ#811807 Define network filter on libvirt ......................................................................
Patch Set 1: (1 inline comment)
Tools isn't good enough since it fails to establish the libvirt connection. In order to achieve it some additional work should be made.
The error is: File "/usr/lib64/python2.6/site-packages/vdsm/libvirtconnection.py", line 25, in <module> import libvirtev ImportError: No module named libvirtev
.................................................... File vdsm/vdsmd.init.in Line 452: fi Line 453: sleep 1 Line 454: done Line 455: # End workaround Line 456: This is required specifically now since we attempt to connect to libvirt immediately after the libvirt service is started. It takes 2-3 seconds for the service to become responsive. Without this workaround, first attempt to configure nwfilter will fail.
I'll add this part as a separate patch, at the head of this patch series. Line 457: return 0 Line 458: else Line 459: echo "$startout" >&2 Line 460: return 1
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: BZ#811807 Define network filter on libvirt ......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/552/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: BZ#811807 Define network filter on libvirt ......................................................................
Patch Set 2: I would prefer that you didn't submit this
(9 inline comments)
.................................................... File vdsm/nwfilter.py Line 1: #! /usr/bin/python Line 2: # Copyright 2012 IBM, Inc. should I have heard about the changes in your affiliation? Line 3: # Line 4: # This program is free software; you can redistribute it and/or modify Line 5: # it under the terms of the GNU General Public License as published by Line 6: # the Free Software Foundation; either version 2 of the License, or
Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # Line 20: Line 21: import libvirt Line 22: import logging start with standard lib import
then site-packages (such as libvirt)
then internal project. Line 23: from vdsm import libvirtconnection Line 24: Line 25: Line 26: def main():
Line 30: conn = libvirtconnection.get() Line 31: defineNwFilter(conn, NoMacSpoofingFilter()) Line 32: try: Line 33: conn.close() Line 34: except: catch-all-exception is generally evil. why is it needed now? Line 35: pass Line 36: Line 37: def defineNwFilter(conn, nwFilter): Line 38: """
Line 39: define vdsm network filter on libvirt to control VM traffic Line 40: """ Line 41: Line 42: filter = None Line 43: filterName = nwFilter.filterName seems like defineNwFilter could have been a method of VdsmNwFilter Line 44: filterXml = nwFilter.buildFilterXml() Line 45: Line 46: try: Line 47: filter = conn.nwfilterLookupByName(filterName)
Line 43: filterName = nwFilter.filterName Line 44: filterXml = nwFilter.buildFilterXml() Line 45: Line 46: try: Line 47: filter = conn.nwfilterLookupByName(filterName) it would be much cleaner to have
filter = conn.nwfilterLookupByName(filterName) filter.undefine()
in one blow. or even
conn.nwfilterLookupByName(filterName).undefine()
as no one cares for the "filter variable" Line 48: except libvirt.libvirtError: Line 49: logging.debug("No such filter: %s" % (filterName)) Line 50: Line 51: if filter:
Line 56: # Ignore failure if filter already exists. Failure might occur when Line 57: # attempting to remove a filter which is being used by running VMs Line 58: pass Line 59: try: Line 60: nwFilter = conn.nwfilterDefineXML(filterXml) no need to set a variable that is never used. Doesn't pyflake cry about this? Line 61: except libvirt.libvirtError: Line 62: logging.error("Failed to define filter %s" % (filterName)) Line 63: raise Line 64: else:
Line 59: try: Line 60: nwFilter = conn.nwfilterDefineXML(filterXml) Line 61: except libvirt.libvirtError: Line 62: logging.error("Failed to define filter %s" % (filterName)) Line 63: raise exceptions should be caught at the top-most level possible. you can log the whole traceback up there. Line 64: else: Line 65: logging.debug("Filter %s was defined" % (nwFilter.name())) Line 66: Line 67:
Line 64: else: Line 65: logging.debug("Filter %s was defined" % (nwFilter.name())) Line 66: Line 67: Line 68: class VdsmNwFilter(object): we all know this is Vdsm here. No need for the Vdsm prefix. Line 69: """ Line 70: Base class for custom network filters Line 71: """ Line 72:
.................................................... File vdsm/vdsmd.init.in Line 482: log_failure_msg "$prog: one of the dependent services did not start, error code $ret_val" Line 483: return $ret_val Line 484: fi Line 485: Line 486: @VDSMDIR@/nwfilter.py > /dev/null 2>&1 do we really want to hide stderr? in the rare case of an error, I'd like to see its nature. Line 487: ret_val=$? Line 488: if [ $ret_val -ne 0 ] Line 489: then Line 490: log_failure_msg "$prog: Failed to define network filters on libvirt"
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Moti Asayag has posted comments on this change.
Change subject: BZ#811807 Define network filter on libvirt ......................................................................
Patch Set 2: (9 inline comments)
.................................................... File vdsm/nwfilter.py Line 1: #! /usr/bin/python Line 2: # Copyright 2012 IBM, Inc. A honest 'copy & paste' mistake. Line 3: # Line 4: # This program is free software; you can redistribute it and/or modify Line 5: # it under the terms of the GNU General Public License as published by Line 6: # the Free Software Foundation; either version 2 of the License, or
Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # Line 20: Line 21: import libvirt Line 22: import logging sadly the 'organize imports' of pydev plugin (ctrl+shift+o) can't save the 2 seconds on this and ends with:
from vdsm import libvirtconnection
import libvirt
import logging Line 23: from vdsm import libvirtconnection Line 24: Line 25: Line 26: def main():
Line 30: conn = libvirtconnection.get() Line 31: defineNwFilter(conn, NoMacSpoofingFilter()) Line 32: try: Line 33: conn.close() Line 34: except: Added specific libvirtError exception Line 35: pass Line 36: Line 37: def defineNwFilter(conn, nwFilter): Line 38: """
Line 39: define vdsm network filter on libvirt to control VM traffic Line 40: """ Line 41: Line 42: filter = None Line 43: filterName = nwFilter.filterName Done Line 44: filterXml = nwFilter.buildFilterXml() Line 45: Line 46: try: Line 47: filter = conn.nwfilterLookupByName(filterName)
Line 43: filterName = nwFilter.filterName Line 44: filterXml = nwFilter.buildFilterXml() Line 45: Line 46: try: Line 47: filter = conn.nwfilterLookupByName(filterName) the only issue is that nwfilterLookupByName raises an exception if no matching filter was found, but if putting all in same try-except block - should be fine. Line 48: except libvirt.libvirtError: Line 49: logging.debug("No such filter: %s" % (filterName)) Line 50: Line 51: if filter:
Line 56: # Ignore failure if filter already exists. Failure might occur when Line 57: # attempting to remove a filter which is being used by running VMs Line 58: pass Line 59: try: Line 60: nwFilter = conn.nwfilterDefineXML(filterXml) it is being referred on the log message below (5 lines under) Line 61: except libvirt.libvirtError: Line 62: logging.error("Failed to define filter %s" % (filterName)) Line 63: raise Line 64: else:
Line 59: try: Line 60: nwFilter = conn.nwfilterDefineXML(filterXml) Line 61: except libvirt.libvirtError: Line 62: logging.error("Failed to define filter %s" % (filterName)) Line 63: raise Done Line 64: else: Line 65: logging.debug("Filter %s was defined" % (nwFilter.name())) Line 66: Line 67:
Line 64: else: Line 65: logging.debug("Filter %s was defined" % (nwFilter.name())) Line 66: Line 67: Line 68: class VdsmNwFilter(object): Done Line 69: """ Line 70: Base class for custom network filters Line 71: """ Line 72:
.................................................... File vdsm/vdsmd.init.in Line 482: log_failure_msg "$prog: one of the dependent services did not start, error code $ret_val" Line 483: return $ret_val Line 484: fi Line 485: Line 486: @VDSMDIR@/nwfilter.py > /dev/null 2>&1 In that case we should face some libvirt errors which are being report occasionally, such as failure to close connection:
"Exception in thread libvirtEventLoop (most likely raised during interpreter shutdown)"
It isn't nice to print messages during service startup as the one above.
The case of failure to define a filter an exception is raised and reported by the error below. Line 487: ret_val=$? Line 488: if [ $ret_val -ne 0 ] Line 489: then Line 490: log_failure_msg "$prog: Failed to define network filters on libvirt"
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: BZ#811807 Define network filter on libvirt ......................................................................
Patch Set 3:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/566/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: BZ#811807 Define network filter on libvirt ......................................................................
Patch Set 3: I would prefer that you didn't submit this
(6 inline comments)
I think we have more simplifications to perform, I hope you do not mind.
.................................................... File vdsm/Makefile.am Line 52: tc.py \ Line 53: vdsmDebugPlugin.py \ Line 54: vmChannels.py \ Line 55: vm.py \ Line 56: nwfilter.py please keep sorted. and while at it, would you add
$(NULL)
to the end of the list? Line 57: Line 58: dist_vdsmpylib_PYTHON = \ Line 59: __init__.py \ Line 60: define.py \
.................................................... File vdsm/nwfilter.py Line 34: Line 35: try: Line 36: NoMacSpoofingFilter().defineNwFilter(conn) Line 37: except libvirt.libvirtError, e: Line 38: traceback.print_exc() sorry, I still do not understand why you bother to catch an exception only to log and re-raise it. think positive. exceptions should simplify code, not complicate it. Line 39: logging.error("Failed to define network filter: %s" % e.message) Line 40: raise Line 41: finally: Line 42: try:
Line 39: logging.error("Failed to define network filter: %s" % e.message) Line 40: raise Line 41: finally: Line 42: try: Line 43: conn.close() I know that some people do not trust the garbage collector to close such objects. fine. I understand that. but why worry about the exception? Line 44: except libvirt.libvirtError: Line 45: pass Line 46: Line 47: class NwFilter(object):
Line 71: # is being used by running VMs Line 72: pass Line 73: Line 74: try: Line 75: nwFilter = conn.nwfilterDefineXML(self.buildFilterXml()) again, you assign a value to a variable that no one care about. Line 76: except libvirt.libvirtError: Line 77: logging.error("Failed to define filter %s" % (self.filterName)) Line 78: raise Line 79: else:
Line 73: Line 74: try: Line 75: nwFilter = conn.nwfilterDefineXML(self.buildFilterXml()) Line 76: except libvirt.libvirtError: Line 77: logging.error("Failed to define filter %s" % (self.filterName)) again, I see no point in this explicit log. the default traceback produced by unhandled exception is good enough. Line 78: raise Line 79: else: Line 80: logging.debug("Filter %s was defined" % (nwFilter.name())) Line 81:
Line 90: NwFilter.__init__(self, 'vdsm-no-mac-spoofing') Line 91: Line 92: def getFilterXml(self): Line 93: return '''<filter name='%s' chain='root'> Line 94: <!-- preventing MAC spoofing --> I this kind of programmer that actually needs comments. However, I find these two as adding very very little over the finely-named filters. Line 95: <filterref filter='no-mac-spoofing'/> Line 96: <!-- preventing ARP MAC spoofing --> Line 97: <filterref filter='no-arp-mac-spoofing'/> Line 98: </filter> '''
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Moti Asayag has posted comments on this change.
Change subject: Define network filter on libvirt ......................................................................
Patch Set 3: (6 inline comments)
.................................................... File vdsm/Makefile.am Line 52: tc.py \ Line 53: vdsmDebugPlugin.py \ Line 54: vmChannels.py \ Line 55: vm.py \ Line 56: nwfilter.py Done Line 57: Line 58: dist_vdsmpylib_PYTHON = \ Line 59: __init__.py \ Line 60: define.py \
.................................................... File vdsm/nwfilter.py Line 34: Line 35: try: Line 36: NoMacSpoofingFilter().defineNwFilter(conn) Line 37: except libvirt.libvirtError, e: Line 38: traceback.print_exc() Done Line 39: logging.error("Failed to define network filter: %s" % e.message) Line 40: raise Line 41: finally: Line 42: try:
Line 39: logging.error("Failed to define network filter: %s" % e.message) Line 40: raise Line 41: finally: Line 42: try: Line 43: conn.close() i wouldn't like to exit this program with exit code <> 0 just due to connection close error.
How would the interpreter behave if the GC fails to close the connection ? Line 44: except libvirt.libvirtError: Line 45: pass Line 46: Line 47: class NwFilter(object):
Line 71: # is being used by running VMs Line 72: pass Line 73: Line 74: try: Line 75: nwFilter = conn.nwfilterDefineXML(self.buildFilterXml()) if wishes to remove the debug info from the else-part of this block - will do so.
I think for debug purpose this info could be useful to the user to know the filter was defined as he expected it (instead 'virsh nwfilter-list | grep my_filter_name) Line 76: except libvirt.libvirtError: Line 77: logging.error("Failed to define filter %s" % (self.filterName)) Line 78: raise Line 79: else:
Line 73: Line 74: try: Line 75: nwFilter = conn.nwfilterDefineXML(self.buildFilterXml()) Line 76: except libvirt.libvirtError: Line 77: logging.error("Failed to define filter %s" % (self.filterName)) Done Line 78: raise Line 79: else: Line 80: logging.debug("Filter %s was defined" % (nwFilter.name())) Line 81:
Line 90: NwFilter.__init__(self, 'vdsm-no-mac-spoofing') Line 91: Line 92: def getFilterXml(self): Line 93: return '''<filter name='%s' chain='root'> Line 94: <!-- preventing MAC spoofing --> Done Line 95: <filterref filter='no-mac-spoofing'/> Line 96: <!-- preventing ARP MAC spoofing --> Line 97: <filterref filter='no-arp-mac-spoofing'/> Line 98: </filter> '''
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Define network filter on libvirt ......................................................................
Patch Set 4:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/676/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Moti Asayag has posted comments on this change.
Change subject: Define network filter on libvirt ......................................................................
Patch Set 4: Verified
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Define network filter on libvirt ......................................................................
Patch Set 4: I would prefer that you didn't submit this
(2 inline comments)
only the tuple thingy is a must-fix.
.................................................... File vdsm/nwfilter.py Line 34: try: Line 35: NoMacSpoofingFilter().defineNwFilter(conn) Line 36: finally: Line 37: try: Line 38: conn.close() I would have trusted libvirt to be doing the cleanup automatically when the process exists. But I suppose that you are right about explicitly closing.
However, why would a close fail? If it ever happens, it means that something really bad has occurred, and I would have liked to see the error.
In any case, my position here is not very strong.
BTW, you know that I have a soft spot for contextmanagers. So for future reference:
with contextlib.closing(libvirtconnection.get()) as conn: NoMacSpoofingFilter().defineNwFilter(conn) Line 39: except libvirt.libvirtError: Line 40: pass Line 41: Line 42: class NwFilter(object):
Line 66: # is being used by running VMs Line 67: pass Line 68: Line 69: nwFilter = conn.nwfilterDefineXML(self.buildFilterXml()) Line 70: logging.debug("Filter %s was defined" % (nwFilter.name())) the parens around
(nwFilter.name())
are meaningless. use
(nwFilter.name(), )
to be uber-holy. Line 71: Line 72: Line 73: class NoMacSpoofingFilter(NwFilter): Line 74: """
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Define network filter on libvirt ......................................................................
Patch Set 5:
Build Successful
http://jenkins.ovirt.info/job/patch_vdsm_unit_tests/686/ : SUCCESS
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has posted comments on this change.
Change subject: Define network filter on libvirt ......................................................................
Patch Set 5: Looks good to me, approved
(1 inline comment)
Thanks!
.................................................... File vdsm/nwfilter.py Line 30: Defines network filters on libvirt Line 31: """ Line 32: conn = libvirtconnection.get() Line 33: NoMacSpoofingFilter().defineNwFilter(conn) Line 34: conn.close() someone told me that it is not uncommon for conn.close() to fail. we may need to either fix the issue, or swallow it. but let us postpone this to a future patch. Line 35: Line 36: class NwFilter(object): Line 37: """ Line 38: Base class for custom network filters
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Moti Asayag has posted comments on this change.
Change subject: Define network filter on libvirt ......................................................................
Patch Set 4: (2 inline comments)
.................................................... File vdsm/nwfilter.py Line 34: try: Line 35: NoMacSpoofingFilter().defineNwFilter(conn) Line 36: finally: Line 37: try: Line 38: conn.close() The error i got is: "Exception in thread libvirtEventLoop (most likely raised during interpreter shutdown)"
I so it has no impact on the return value of the program therefore this block can be removed. (left on the conn.close()) Line 39: except libvirt.libvirtError: Line 40: pass Line 41: Line 42: class NwFilter(object):
Line 66: # is being used by running VMs Line 67: pass Line 68: Line 69: nwFilter = conn.nwfilterDefineXML(self.buildFilterXml()) Line 70: logging.debug("Filter %s was defined" % (nwFilter.name())) Done Line 71: Line 72: Line 73: class NoMacSpoofingFilter(NwFilter): Line 74: """
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Moti Asayag has posted comments on this change.
Change subject: Define network filter on libvirt ......................................................................
Patch Set 5: (1 inline comment)
.................................................... File vdsm/nwfilter.py Line 30: Defines network filters on libvirt Line 31: """ Line 32: conn = libvirtconnection.get() Line 33: NoMacSpoofingFilter().defineNwFilter(conn) Line 34: conn.close() I've added a comment for those changes on the previous patch-set but seems not to submit those comments (now those comments are submitted).
In brief, despite the error, no error exit status is being returned. Line 35: Line 36: class NwFilter(object): Line 37: """ Line 38: Base class for custom network filters
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Igor Lvovsky has posted comments on this change.
Change subject: Define network filter on libvirt ......................................................................
Patch Set 5: (1 inline comment)
.................................................... File vdsm/nwfilter.py Line 59: # Failure might occur when attempting to remove a filter which Line 60: # is being used by running VMs Line 61: pass Line 62: Line 63: nwFilter = conn.nwfilterDefineXML(self.buildFilterXml()) Is this will work if filter already defined and you swallowed the exception in lines above? Line 64: logging.debug("Filter %s was defined" % nwFilter.name()) Line 65: Line 66: Line 67: class NoMacSpoofingFilter(NwFilter):
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Meni Yakove has posted comments on this change.
Change subject: Define network filter on libvirt ......................................................................
Patch Set 5: Verified
vdsm-no-mac-spoofing.xml exist under /etc/libvirt/nwfilter/ VM start with <filterref filter="vdsm-no-mac-spoofing"/> When VM start ebtables have the relevant rules.
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Meni Yakove myakove@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Define network filter on libvirt ......................................................................
Define network filter on libvirt
The patch defines custom network filter on libvirt in order to control the VM traffic. The custom filter is being set when VDSM service is being started.
Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Signed-off-by: Moti Asayag masayag@redhat.com --- M vdsm.spec.in M vdsm/Makefile.am A vdsm/nwfilter.py M vdsm/vdsmd.init.in 4 files changed, 95 insertions(+), 1 deletion(-)
Approvals: Meni Yakove: Verified Dan Kenigsberg: Looks good to me, approved
-- To view, visit http://gerrit.ovirt.org/7354 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: merged Gerrit-Change-Id: I9f1708385dec6a87bc404e4ab25c4da8ab8a8acc Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Moti Asayag masayag@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Livnat Peer lpeer@redhat.com Gerrit-Reviewer: Meni Yakove myakove@redhat.com Gerrit-Reviewer: Moti Asayag masayag@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server
vdsm-patches@lists.fedorahosted.org