On Fri, Oct 02, 2009 at 03:48:11PM +0200, Gerhard Stenzel wrote: > + > + > +#ifdef ENABLE_ebtabLES_LOKKIT Something went a little wrong with upper/lower case there I think > +static void > +notifyRulesUpdated(const char *table, > + const char *path) > +{ > + char arg[PATH_MAX]; > + const char *argv[4]; > + > + snprintf(arg, sizeof(arg), "--custom-rules=ipv4:%s:%s", table, path); > + > + argv[0] = (char *) LOKKIT_PATH; > + argv[1] = (char *) "--nostart"; > + argv[2] = arg; > + argv[3] = NULL; It is desirable not to cast away const-ness, but rather to declare it correctly - you can also initialize at the same time as declaring to avoid needing to hardcode array indexes, eg const char * const argv[] = { LOKKIT_PATH, "--nostart", arg, NULL, }; > +ebtablesContext * > +ebtablesContextNew(void) > +{ > + ebtablesContext *ctx; > + > + if (VIR_ALLOC(ctx) < 0) > + return NULL; > + > + if (!(ctx->input_filter = ebtRulesNew("filter", "INPUT"))) > + goto error; > + > + if (!(ctx->forward_filter = ebtRulesNew("filter", "FORWARD"))) > + goto error; > + > + if (!(ctx->nat_postrouting = ebtRulesNew("nat", "POSTROUTING"))) > + goto error; > + > + return ctx; > + > + error: > + ebtablesContextFree(ctx); > + return NULL; > +} I know you're just following the existing style/pattern used by the iptables.c/h file here by directly manipulating the main INPUT/FORWARD/POSTROUTING chains, but I think this could be improved a little Add a custom 'libvirt_INPUT' chain as the first entry in the main 'INPUT' chain, and then put all our rules on this custom chain. This will make it clearer for admins where all these extra rules are coming from, and make it slightly safer for management since there willbe less risk of rules in our custom chain getting blown away / mis-ordered wrt other rules I think it would even be desirable to have a custom chain per libvirt driver, since multiple drivers might want todo this MAC filtering. Thus I'd suggest adding a 'const char *driverName' parameter to ebtablesContextNew(const char *driver), and then creating chains named with that, eg so if QEMU, LXC and UML drivers all uses this ebtables code, we'd end up with the INPUT chain having references to 3 custom chains libvirt_qemu_INPUT libvirt_lxc_INPUT libvirt_uml_INPUT Likewise for FORWARD/POSTROUTING. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list