Dan Kenigsberg has posted comments on this change.
Change subject: New before_device_create/before_nic_hotplug hook: privatevlan ......................................................................
Patch Set 5:
(5 comments)
Thanks for the fixes, few minor questions remain.
http://gerrit.ovirt.org/#/c/24195/5/vdsm_hooks/privatevlan/README File vdsm_hooks/privatevlan/README:
Line 27: Line 28: privatevlan_guest_learn=none Line 29: This is an optional parameter. It is recommended that you set this Line 30: to none, or leave unset. DHCP snooping functionality is claimed to Line 31: be available ("dhcp"), but seems broken with qemu guests. The Is there a libvirt bug about this? If not, please open one! Line 32: default setting is "any", which means that if the guest IP isn't Line 33: provided, the filter is not applied until an outbound IP packet is Line 34: sent, then the IP is locked in until shutdown or migrate. Line 35:
Line 46: set for other hooks. Please adjust accordingly. Line 47: Line 48: NOTE: Line 49: the filter will be applied only after restart of libvirtd on the host system. Line 50: # service libvirtd restart I am sorry, but Toni has misled you. On el6 one must NOT call `service libvirtd restart` since it is NOT redirected to upstart. Line 51: Line 52: to see libvirt filters use: Line 53: # ebtables -t nat -L Line 54:
http://gerrit.ovirt.org/#/c/24195/5/vdsm_hooks/privatevlan/before_device_cre... File vdsm_hooks/privatevlan/before_device_create.py:
Line 24: import sys Line 25: import hooking Line 26: import traceback Line 27: Line 28: # Name of the predefined nwfilter This comment did not help me. If you think that the use of LIBVIRT_FILTER is not clear (it's clear to me), find a better name for it. Line 29: LIBVIRT_FILTER = 'privatevlan-vdsm' Line 30: Line 31: Line 32: def addInterfaceFilter(document, interface, allowed_macs, ipaddresses, learn):
Line 65: Line 66: Line 67: def main(): Line 68: allowed_macs = os.environ.get('privatevlan_allowed_macs', None) Line 69: if allowed_macs is None: Interpreting the empty string in the same way would make the hook a bit more usable - unless you think that passing the empty string is indeed an error. Line 70: return Line 71: Line 72: iplist = os.environ.get('privatevlan_guest_ip', None) Line 73: learn = os.environ.get('privatevlan_guest_learn', None)
Line 111: Line 112: try: Line 113: main() Line 114: except Exception: Line 115: sys.stderr.write('privatevlan: [unexpected error]: %s\n' % why did you opt not to use hooking.exit_hook()? Line 116: traceback.format_exc())
vdsm-patches@lists.fedorahosted.org