Re: [libvirt] [PATCH 0/13] [RFC] Network filtering (ACL) extensions for libvirt

"Daniel P. Berrange" <berrange(a)redhat.com&gt; wrote on 03/17/2010 10:40:36 AM: > > On Thu, Mar 11, 2010 at 08:06:04AM -0500, Stefan Berger wrote: > > Hi! > > > > The following set of patches add network filtering (ACL) extensions to > > libvirt and enable network traffic filtering for VMs using ebtables and, > > depending on the networking technology being used (tap, but not > > macvtap), also iptables. Usage of either is optional and controlled > > through filters that a VM is referencing. > > > > The ebtables-level filtering is based on the XML derived from the CIM > > network slide 10 (filtering) from the DMTF website > > (http://www.dmtf.org/standards/cim/cim_schema_v2230/CIM_Network.pdf). > > The XML we derived from this was discussed on the list before. On the > > ebtables level we currently handle filtering of IPv4 and ARP traffic. > > It is planned to cover IPv6 too, either at ebtables or ip6tables > level ? Well, the code should be able to handle it and we at least thought about it. I am not an IPv6 expert myself ... currently ... yet. :-) > > > I've not looked at the actual code in detail yet, but the public API, > virsh commands, XML etc all looks generally good to me. I'll try and > get you a detailed code review friday/monday once I get through my > current work items. Let me post another series later today with the fixes that I have made following your reviews and changes I did make myself. > > > One comment about the instantiation of the rules: Since the XML allows > > to create nearly any possible combination of parameters to ebtables or > > iptables commands, I haven't used the ebtables or iptables wrappers. > > Instead, I am writing ebtables/iptables command into a buffer, add > > command line options to each one of them as described in the rule's XML, > > write the buffer into a file and run it as a script. For those commands > > that are not allowed to fail I am using the following format to run > > them: > > > > cmd="ebtables <some options>" > > r=`${cmd}` > > if [ $? -ne 0 ]; then > > echo "Failure in command ${cmd}." > > exit 1 > > fi > > > > cmd="..." > > [...] > > > > If one of the command fails in such a batch, the libvirt code is going > > pick up the error code '1', tear down anything previously established > > and report an error back. The actual error message shown above is > > currently not reported back, but can be later on with some changes to > > the commands running external programs that need to read the script's > > stdout. > > I understand why you can't use the ebtables/iptables APIs we currently > have there, but I'm not much of a fan of using the shell script. Isn't > it just as easy to directly call virRun() with each set of ARGV to be > exec'd ? I hadn't thought about calling that function... I would want to call a function that can handle something like bash scripts, i.e., multiple concatenated fragments as those shown above just to be more 'efficient'.

If virRun() can handle that and $? for example would be treated there as the return value (which I think is bash-dependent), I'd be happy to call it as well.