1:44 p.m.
On 01/10/2012 12:40 PM, Eric Blake wrote:
s/possiblity/possibility/ in the subject
Fixed.
On 12/09/2011 08:07 AM, Stefan Berger wrote:
> This patch introduces the capability to use a different iterator per
> variable.
>
> The currently supported notation of variables in a filtering rule like
>
> <rule action='accept' direction='out'>
> <tcp srcipaddr='$A' srcportstart='$B'/>
> </rule>
>
> processes the two lists 'A' and 'B' in parallel. This means that A
and B
> must have the same number of 'N' elements and that 'N' rules will be
> instantiated (assuming all tuples from A and B are unique).
>
> In this patch we now introduce the assignment of variables to different
> iterators. Therefore a rule like
>
> <rule action='accept' direction='out'>
> <tcp srcipaddr='$A[@1]' srcportstart='$B[@2]'/>
> </rule>
>
> will now create every combination of elements in A with elements in B since
> A has been assigned to an iterator with Id '1' and B has been assigned to an
> iterator with Id '2', thus processing their value independently.
>
> The first rule has an equivalent notation of
>
> <rule action='accept' direction='out'>
> <tcp srcipaddr='$A[@0]' srcportstart='$B[@0]'/>
> </rule>
>
> ---
> docs/schemas/nwfilter.rng | 71 ++------
This needs something in docs/formatnwfilter.html.in as well.
In 6/6.
> src/conf/nwfilter_conf.c | 35 ++--
> src/conf/nwfilter_conf.h | 6
> src/conf/nwfilter_params.c | 239 +++++++++++++++++++++++++++---
> src/conf/nwfilter_params.h | 38 ++++
> src/libvirt_private.syms | 1
> src/nwfilter/nwfilter_ebiptables_driver.c | 13 +
> src/nwfilter/nwfilter_gentech_driver.c | 8 -
> 8 files changed, 307 insertions(+), 104 deletions(-)
>
>
> - if (VIR_REALLOC_N(nwf->vars, nwf->nvars+1)< 0) {
> - virReportOOMError();
> - return -1;
> - }
> -
> - nwf->vars[nwf->nvars] = strdup(var);
> -
> - if (!nwf->vars[nwf->nvars]) {
> + if (VIR_REALLOC_N(nwf->varAccess, nwf->nVarAccess + 1)< 0) {
I'm okay with this as-is, but you might want to switch to VIR_EXPAND_N
for slightly easier usage.
Fixed.
> @@ -3069,7 +3069,8 @@ virNWFilterRuleDefDetailsFormat(virBuffe
> goto err_exit;
> }
> } else if ((flags& NWFILTER_ENTRY_ITEM_FLAG_HAS_VAR)) {
> - virBufferAsprintf(buf, "$%s", item->var);
> + virBufferAddLit(buf, "$");
I prefer virBufferAddChar(buf, '$'); but under the hood, it's
practically the same amount of work.
Changed.
> + if (parseError) {
> + const char *what = "iterator id";
> + if (dest->accessType == VIR_NWFILTER_VAR_ACCESS_ELEMENT)
> + what = "array index";
"iterator id" and "array index" should probably be marked for
translation, but see my next comment...
> + virNWFilterReportError(VIR_ERR_INVALID_ARG,
> + _("Malformatted %s"), what);
That doesn't make life easy for translators (in languages where nouns
can be masculine or feminine, the adjective translation for
"malformatted" may have a different spelling for the two possilibities
for what). Rather, I'd write:
if (dest->accessType == VIR_NWFILTER_VAR_ACCESS_ELEMENT)
virNWFilterReportError(VIR_ERR_INVALID_ARG,
_("Malformatted array index"));
else
virNWFilterReportError(VIR_ERR_INVALID_ARG,
_("Malformatted iterator id"));
Fixed. Thanks.
> Index: libvirt-iterator/src/conf/nwfilter_params.h
> ===================================================================
> --- libvirt-iterator.orig/src/conf/nwfilter_params.h
> +++ libvirt-iterator/src/conf/nwfilter_params.h
> @@ -91,6 +91,38 @@ int virNWFilterHashTablePutAll(virNWFilt
> # define VALID_VARVALUE \
> "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.:"
>
> +enum virNWFilterVarAccessType {
> + VIR_NWFILTER_VAR_ACCESS_ELEMENT = 0,
> + VIR_NWFILTER_VAR_ACCESS_ITERATOR = 1,
> +
> + VIR_NWFILTER_VAR_ACCESS_LAST,
> +};
> +
> +typedef struct _virNWFilterVarAccess virNWFilterVarAccess;
Is the double-space intentional?
No, fixed.
> Index: libvirt-iterator/docs/schemas/nwfilter.rng
> ===================================================================
> --- libvirt-iterator.orig/docs/schemas/nwfilter.rng
> +++ libvirt-iterator/docs/schemas/nwfilter.rng
> @@ -811,12 +811,15 @@
> </choice>
> </define>
>
> +<define name="variable-name-type">
> +<data type="string">
> +<param name="pattern">$[a-zA-Z0-9_]+(\[[ ]*[@]?[0-9]+[
]*\])?</param>
Does this need to be \$ instead of $, to avoid the meta-meaning of $ as EOL?
It has to be just $. With \$ the test cases don't pass anymore.
> +</data>
> +</define>
> +
> <define name="addrMAC">
> <choice>
> -<!-- variable -->
> -<data type="string">
> -<param name="pattern">$[a-zA-Z0-9_]+</param>
Then again, you were previously using it unquoted, so maybe this is a
latent bug. My guess is that we need to add some sample files somewhere
under tests/*data/*.xml, and ensure that they get schema-tested as part
of 'make check', to prove that our schema makes sense. A quick
git grep 'srcmacaddr=.\$' tests
didn't turn up any existing xml files that would exercise this part of
the rng file.
Stefan