[libvirt] [PATCH V5 04/10] Use scripting for cleaning and renaming of chains

[Eric Blake]

On 11/17/2011 01:11 PM, Stefan Berger wrote:
> Use scripts for the renaming and cleaning up of chains. This allows us to get
> rid of some of the code that is only capable of renaming and removing chains
> whose names are hardcoded.
> 
> A shell function 'collect_chains' is introduced that is given the name
> of an ebtables chain and then recursively determines the names of all
> chains that are accessed from this chain and its sub-chains using 'jumps'.
> 
> The resulting list of chain names is then used to delete all the found
> chains by first flushing and then deleting them.
> 
> The same function is also used for renaming temporary filters to their final
> names.
> 
> I tested this with the bash and dash as script interpreters.
> 
> 
> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
> 
> ---
> 
> v5:
>  - followng Eric Blake's suggestion on a major overhaul of the embedded
>    scripts

Thanks for bearing with me, and the results look a lot nicer!  I guess
it pays to have one of the autoconf maintainers as a reviewer, since
that means you have feedback from someone who knows shell inside and out ;)

> +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
> @@ -92,6 +92,59 @@ static char *gawk_cmd_path;
>  #define PRINT_CHAIN(buf, prefix, ifname, suffix) \
>      snprintf(buf, sizeof(buf), "%c-%s-%s", prefix, ifname, suffix)
>  
> +/* The collect_chains() script recursively determines all names
> + * of ebtables (nat) chains that are 'children' of a given 'root' chain.
> + * The typical out of an ebtables call is as follows:

s/out/output/

> + *
> + * #> ebtables -t nat -L libvirt-I-tck-test205002
> + * Bridge table: nat
> + *
> + * Bridge chain: libvirt-I-tck-test205002, entries: 5, policy: ACCEPT
> + * -p IPv4 -j I-tck-test205002-ipv4
> + * -p ARP -j I-tck-test205002-arp
> + * -p 0x8035 -j I-tck-test205002-rarp
> + * -p 0x835 -j ACCEPT
> + * -j DROP
> + */

The comments go a long way to help.  Thanks.

> +static const char ebtables_script_func_collect_chains[] =
> +    "collect_chains()\n"

Food for thought, but not strictly necessary, and certainly not worth
delaying this patch any further:
 One of the interesting things learned in autoconf is that you want two
levels of comments: those that explain how the generator works (and the
generated file does not have them), and those that explain how the
generated code works (those are useful both in the generator and for
anyone reading the generated output).  Right now, you are only using
level one comments (nwfilter_ebiptables_driver.c has comments about what
it is generating, but the generated script has no comments at all);
depending on how complex the generated script is getting (and it _is_
getting more complex as we add more stuff), it may be worth starting to
add level two comments so that someone reading the generated script can
understand it without referring back to the C code.  For example, this
would mean doing:

+static const char ebtables_script_func_collect_chains[] =
+    "# collect_chains CHAIN...\n"
+    "# for each CHAIN, recursively output the list of sub-chains\n"
+    "# that it references through jump rules\n"
+    "collect_chains()\n"

> +
> +static const char ebiptables_script_set_ifs[] =
> +    "tmp='\n'\n"
> +    "IFS=' ''\t'$tmp\n";

Much nicer ;)

ACK.

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20111117/7e9b0cfb/attachment-0001.sig>