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