[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] add ebtables wrapper



On Tue, Oct 27, 2009 at 12:36:09PM +0100, Gerhard Stenzel wrote:
> This patch adds the files which implement the ebtables wrapper.
> 
> Signed-off-by: Gerhard Stenzel <gerhard stenzel de ibm com>

> +++ b/src/libvirt_private.syms
> @@ -234,6 +234,13 @@ iptablesRemoveUdpInput;
>  iptablesSaveRules;
>  
>  
> +# ebtables.h
> +ebtablesRemoveForwardAllowIn;
> +ebtablesAddForwardAllowIn;
> +ebtablesAddForwardPolicyReject;
> +ebtablesContextNew;
> +ebtablesSaveRules;
> +

  Okay I just moved it before events.h to keep module sorting and
also sorted the entry points.

> +enum {
> +    ADD = 0,
> +    REMOVE,
> +    CREATE,
> +    POLICY,
> +    INSERT
> +};

  Somehow an enum without a name/type is all the problems of enums
without the usefulness (compared to #define) but I must be biased :-)

> +static void
> +ebtRulesSave(ebtRules *rules)
> +{
> +    (void) rules;

  I assume this means /* TODO */

> +}
> +
[...]
> +
> +    if (virRun(NULL, argv, NULL) < 0) {
> +        retval = errno;
> +        goto error;
> +    }
> +
> +    if (action == ADD || action == CREATE || action == POLICY || action == INSERT) {

  need a long line break

> +        retval = ebtRulesAppend(rules, rule, argv, command_idx);
> +        rule = NULL;
> +        argv = NULL;
> +    } else {
> +        retval = ebtRulesRemove(rules, rule);
> +    }
> +
[...]
> +/**
> + * ebtablesSaveRules:
> + * @ctx: pointer to the EB table context
> + *
> + * Saves all the EB table rules associated with a context
> + * to disk so that if ebtables is restarted, the rules
> + * will automatically be reload.
> + */
> +void
> +ebtablesSaveRules(ebtablesContext *ctx)
> +{
> +    ebtRulesSave(ctx->input_filter);
> +    ebtRulesSave(ctx->forward_filter);
> +    ebtRulesSave(ctx->nat_postrouting);
> +}

  Hum, and where ? Under /etc/libvirt/ebtables/.... ?

Are the table and chain names provided in ebtRulesNew() sufficient
to uniquely name the set ? I hope so otherwise we're gonna have trouble
with persistance. It would be good to have ebtRulesSave() documented if
not fully finished before next release.

  I'm gonna commit this, but I think we need to double-check that the
current APIs won't be a problem when we want to implement saving (didn't
checked the second patch yet).

 I also think the spec file should add a Requires to ebtables as this
is not installed systematically (it wasn't present on my workstation
by default).

  I will push this tonight,

    thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]