11:07 a.m.
This patch series builds on the previously posted patch series
https://www.redhat.com/archives/libvir-list/2011-October/msg00912.html
and introduces the capability to assign a list to a variable and
have multiple rules instantiated, one for each item in the list.
This means, that if for example a variable like IP has been assigned
a list of values
IP = [1.2.3.4, 5.6.7.8, 10.0.0.1]
it will instantiate 3 rules (on the ebtables/iptables level) for a single
XML filtering rule. This in turn allows us to build filters that can
evaluate multiple possible values per field, i.e., allow the filtering for
multiple IP addresses (per interface). It would then need David Steven's
patch for support of 'RETURN' (and 'CONTINUE') target(s).
v3:
- following Daniel Berrange's comment regarding how a list of items
should be represented in the XML
v2:
- reimplementation of iterator
- other nits
Regards,
Stefan
11:07 a.m.
New subject: [libvirt] [PATCH V3 1/4] Rework value part of name-value pairs
NWFilters can be provided name-value pairs using the following
XML notiation:
<filterref filter='xyz'>
<parameter name='PORT' value='80'/>
<parameter name='VAL' value='abc'/>
</filterref>
The internal representation currently is so that a name is stored as a
string and the value as well. This patch now addresses the value part of it
and introduces a data structure for storing a value either as a simple
value or as an array for later support of lists (provided in python-like
notation ( [a,b,c] ).
This patch adjusts all code that was handling the values in hash tables
and makes it use the new data type.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/conf/domain_conf.c | 2
src/conf/nwfilter_params.c | 288 ++++++++++++++++++++++++++++--
src/conf/nwfilter_params.h | 38 +++
src/libvirt_private.syms | 3
src/nwfilter/nwfilter_ebiptables_driver.c | 15 +
src/nwfilter/nwfilter_gentech_driver.c | 27 ++
src/nwfilter/nwfilter_learnipaddr.c | 13 +
7 files changed, 365 insertions(+), 21 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===================================================================
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -200,14 +200,25 @@ printVar(virNWFilterHashTablePtr vars,
*done = 0;
if ((item->flags & NWFILTER_ENTRY_ITEM_FLAG_HAS_VAR)) {
- char *val = (char *)virHashLookup(vars->hashTable, item->var);
- if (!val) {
+ virNWFilterVarValuePtr varval;
+ const char *val;
+
+ varval = virHashLookup(vars->hashTable, item->var);
+ if (!varval) {
virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot find value for '%s'"),
item->var);
return 1;
}
+ val = virNWFilterVarValueGetSimple(varval);
+ if (!val) {
+ virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot get simple value of
'%s'"),
+ item->var);
+ return 1;
+ }
+
if (!virStrcpy(buf, val, bufsize)) {
virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
_("Buffer to small to print MAC address "
Index: libvirt-acl/src/conf/nwfilter_params.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_params.c
+++ libvirt-acl/src/conf/nwfilter_params.c
@@ -30,14 +30,268 @@
#include "datatypes.h"
#include "nwfilter_params.h"
#include "domain_conf.h"
+#include "c-ctype.h"
#define VIR_FROM_THIS VIR_FROM_NWFILTER
+static bool isValidVarValue(const char *value);
+
+
+static void
+virNWFilterVarValueFree(virNWFilterVarValuePtr val)
+{
+ unsigned i;
+
+ if (!val)
+ return;
+
+ switch (val->valType) {
+ case NWFILTER_VALUE_TYPE_SIMPLE:
+ VIR_FREE(val->u.simple.value);
+ break;
+ case NWFILTER_VALUE_TYPE_ARRAY:
+ for (i = 0; i < val->u.array.nValues; i++)
+ VIR_FREE(val->u.array.values[i]);
+ VIR_FREE(val->u.array.values);
+ break;
+ case NWFILTER_VALUE_TYPE_LAST:
+ break;
+ }
+ VIR_FREE(val);
+}
+
+static virNWFilterVarValuePtr
+virNWFilterVarValueCopy(const virNWFilterVarValuePtr val)
+{
+ virNWFilterVarValuePtr res;
+ unsigned i;
+ char *str;
+
+ if (VIR_ALLOC(res) < 0) {
+ virReportOOMError();
+ return NULL;
+ }
+ res->valType = val->valType;
+
+ switch (res->valType) {
+ case NWFILTER_VALUE_TYPE_SIMPLE:
+ if (val->u.simple.value) {
+ res->u.simple.value = strdup(val->u.simple.value);
+ if (!res->u.simple.value)
+ goto err_exit;
+ }
+ break;
+ case NWFILTER_VALUE_TYPE_ARRAY:
+ if (VIR_ALLOC_N(res->u.array.values, val->u.array.nValues))
+ goto err_exit;
+ res->u.array.nValues = val->u.array.nValues;
+ for (i = 0; i < val->u.array.nValues; i++) {
+ str = strdup(val->u.array.values[i]);
+ if (!str)
+ goto err_exit;
+ res->u.array.values[i] = str;
+ }
+ break;
+ case NWFILTER_VALUE_TYPE_LAST:
+ break;
+ }
+
+ return res;
+
+err_exit:
+ virReportOOMError();
+ virNWFilterVarValueFree(res);
+ return NULL;
+}
+
+void
+virNWFilterVarValuePrint(virNWFilterVarValuePtr val, virBufferPtr buf)
+{
+ unsigned i;
+ char *item;
+
+ switch (val->valType) {
+ case NWFILTER_VALUE_TYPE_SIMPLE:
+ virBufferAdd(buf, val->u.simple.value, -1);
+ break;
+ case NWFILTER_VALUE_TYPE_ARRAY:
+ virBufferAddLit(buf, "[");
+ for (i = 0; i < val->u.array.nValues; i++) {
+ if (i > 0)
+ virBufferAddLit(buf, ", ");
+ item = val->u.array.values[i];
+ if (item) {
+ bool quote = false;
+ if (c_isspace(item[0]) || c_isspace(item[strlen(item)- 1 ]))
+ quote = true;
+ if (quote)
+ virBufferEscapeString(buf, "%s", "'");
+ virBufferAdd(buf, val->u.array.values[i], -1);
+ if (quote)
+ virBufferEscapeString(buf, "%s", "'");
+ }
+ }
+ virBufferAddLit(buf, "]");
+ break;
+ case NWFILTER_VALUE_TYPE_LAST:
+ break;
+ }
+}
+
+virNWFilterVarValuePtr
+virNWFilterVarValueCreateSimple(const char *value, bool copy_value)
+{
+ virNWFilterVarValuePtr val;
+
+ if (!isValidVarValue(value))
+ return NULL;
+
+ if (VIR_ALLOC(val) < 0) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ val->valType = NWFILTER_VALUE_TYPE_SIMPLE;
+ if (copy_value) {
+ val->u.simple.value = strdup(value);
+ if (!val->u.simple.value) {
+ VIR_FREE(val);
+ virReportOOMError();
+ return NULL;
+ }
+ } else
+ val->u.simple.value = (char *)value;
+
+ return val;
+}
+
+const char *
+virNWFilterVarValueGetSimple(virNWFilterVarValuePtr val)
+{
+ if (val->valType == NWFILTER_VALUE_TYPE_SIMPLE)
+ return val->u.simple.value;
+ return NULL;
+}
+
+const char *
+virNWFilterVarValueGetNthValue(virNWFilterVarValuePtr val, unsigned int idx)
+{
+ const char *res = NULL;
+
+ switch (val->valType) {
+ case NWFILTER_VALUE_TYPE_SIMPLE:
+ if (idx == 0)
+ res = val->u.simple.value;
+ break;
+ case NWFILTER_VALUE_TYPE_ARRAY:
+ if (idx < val->u.array.nValues)
+ res = val->u.array.values[idx];
+ break;
+ case NWFILTER_VALUE_TYPE_LAST:
+ break;
+ }
+
+ return res;
+}
+
+unsigned int
+virNWFilterVarValueGetCardinality(virNWFilterVarValuePtr val)
+{
+ switch (val->valType) {
+ case NWFILTER_VALUE_TYPE_SIMPLE:
+ return 1;
+ break;
+ case NWFILTER_VALUE_TYPE_ARRAY:
+ return val->u.array.nValues;
+ break;
+ case NWFILTER_VALUE_TYPE_LAST:
+ return 0;
+ }
+ return 0;
+}
+
+bool
+virNWFilterVarValueDelValue(virNWFilterVarValuePtr val, const char *value)
+{
+ unsigned int i;
+
+ switch (val->valType) {
+ case NWFILTER_VALUE_TYPE_SIMPLE:
+ return false;
+
+ case NWFILTER_VALUE_TYPE_ARRAY:
+ for (i = 0; i < val->u.array.nValues; i++) {
+ if (STREQ(value, val->u.array.values[i])) {
+ VIR_FREE(val->u.array.values[i]);
+ val->u.array.nValues--;
+ val->u.array.values[i] =
+ val->u.array.values[val->u.array.nValues];
+ return true;
+ }
+ }
+ break;
+
+ case NWFILTER_VALUE_TYPE_LAST:
+ break;
+ }
+
+ return false;
+}
+
+bool
+virNWFilterVarValueAddValue(virNWFilterVarValuePtr val, const char *value,
+ bool make_copy)
+{
+ char *tmp;
+ bool rc = false;
+
+ if (make_copy) {
+ value = strdup(value);
+ if (!value) {
+ virReportOOMError();
+ return false;
+ }
+ }
+
+ switch (val->valType) {
+ case NWFILTER_VALUE_TYPE_SIMPLE:
+ /* switch to array */
+ tmp = val->u.simple.value;
+ if (VIR_ALLOC_N(val->u.array.values, 2) < 0) {
+ val->u.simple.value = tmp;
+ virReportOOMError();
+ return false;
+ }
+ val->valType = NWFILTER_VALUE_TYPE_ARRAY;
+ val->u.array.nValues = 2;
+ val->u.array.values[0] = tmp;
+ val->u.array.values[1] = (char *)value;
+ rc = true;
+ break;
+
+ case NWFILTER_VALUE_TYPE_ARRAY:
+ if (VIR_REALLOC_N(val->u.array.values,
+ val->u.array.nValues + 1) < 0) {
+ virReportOOMError();
+ return false;
+ }
+ val->u.array.values[val->u.array.nValues] = (char *)value;
+ val->u.array.nValues += 1;
+ rc = true;
+ break;
+
+ case NWFILTER_VALUE_TYPE_LAST:
+ break;
+ }
+
+ return rc;
+}
+
static void
hashDataFree(void *payload, const void *name ATTRIBUTE_UNUSED)
{
- VIR_FREE(payload);
+ virNWFilterVarValueFree(payload);
}
@@ -56,7 +310,7 @@ hashDataFree(void *payload, const void *
int
virNWFilterHashTablePut(virNWFilterHashTablePtr table,
const char *name,
- char *val,
+ virNWFilterVarValuePtr val,
int copyName)
{
if (!virHashLookup(table->hashTable, name)) {
@@ -160,12 +414,12 @@ static void
addToTable(void *payload, const void *name, void *data)
{
struct addToTableStruct *atts = (struct addToTableStruct *)data;
- char *val;
+ virNWFilterVarValuePtr val;
if (atts->errOccurred)
return;
- val = strdup((char *)payload);
+ val = virNWFilterVarValueCopy((virNWFilterVarValuePtr)payload);
if (!val) {
virReportOOMError();
atts->errOccurred = 1;
@@ -177,7 +431,7 @@ addToTable(void *payload, const void *na
_("Could not put variable '%s' into
hashmap"),
(const char *)name);
atts->errOccurred = 1;
- VIR_FREE(val);
+ virNWFilterVarValueFree(val);
}
}
@@ -215,11 +469,17 @@ isValidVarValue(const char *value)
return value[strspn(value, VALID_VARVALUE)] == 0;
}
+static virNWFilterVarValuePtr
+virNWFilterParseVarValue(const char *val)
+{
+ return virNWFilterVarValueCreateSimple(val, true);
+}
virNWFilterHashTablePtr
virNWFilterParseParamAttributes(xmlNodePtr cur)
{
char *nam, *val;
+ virNWFilterVarValuePtr value;
virNWFilterHashTablePtr table = virNWFilterHashTableCreate(0);
if (!table) {
@@ -234,20 +494,24 @@ virNWFilterParseParamAttributes(xmlNodeP
if (xmlStrEqual(cur->name, BAD_CAST "parameter")) {
nam = virXMLPropString(cur, "name");
val = virXMLPropString(cur, "value");
+ value = NULL;
if (nam != NULL && val != NULL) {
if (!isValidVarName(nam))
goto skip_entry;
- if (!isValidVarValue(nam))
+ value = virNWFilterParseVarValue(val);
+ if (!value)
goto skip_entry;
- if (virNWFilterHashTablePut(table, nam, val, 1)) {
+ if (virNWFilterHashTablePut(table, nam, value, 1)) {
VIR_FREE(nam);
VIR_FREE(val);
+ virNWFilterVarValueFree(value);
virNWFilterHashTableFree(table);
return NULL;
}
- val = NULL;
+ value = NULL;
}
skip_entry:
+ virNWFilterVarValueFree(value);
VIR_FREE(nam);
VIR_FREE(val);
}
@@ -268,11 +532,13 @@ static void
_formatParameterAttrs(void *payload, const void *name, void *data)
{
struct formatterParam *fp = (struct formatterParam *)data;
+ virNWFilterVarValuePtr value = payload;
- virBufferAsprintf(fp->buf, "%s<parameter name='%s'
value='%s'/>\n",
+ virBufferAsprintf(fp->buf, "%s<parameter name='%s'
value='",
fp->indent,
- (const char *)name,
- (char *)payload);
+ (const char *)name);
+ virNWFilterVarValuePrint(value, fp->buf);
+ virBufferAddLit(fp->buf, "'/>\n");
}
Index: libvirt-acl/src/conf/nwfilter_params.h
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_params.h
+++ libvirt-acl/src/conf/nwfilter_params.h
@@ -23,6 +23,42 @@
# define NWFILTER_PARAMS_H
# include "hash.h"
+# include "buf.h"
+
+enum virNWFilterVarValueType {
+ NWFILTER_VALUE_TYPE_SIMPLE,
+ NWFILTER_VALUE_TYPE_ARRAY,
+
+ NWFILTER_VALUE_TYPE_LAST
+};
+
+typedef struct _virNWFilterVarValue virNWFilterVarValue;
+typedef virNWFilterVarValue *virNWFilterVarValuePtr;
+struct _virNWFilterVarValue {
+ enum virNWFilterVarValueType valType;
+ union {
+ struct {
+ char *value;
+ } simple;
+ struct {
+ char **values;
+ unsigned nValues;
+ } array;
+ } u;
+};
+
+virNWFilterVarValuePtr virNWFilterVarValueCreateSimple(const char *,
+ bool copy_value);
+const char *virNWFilterVarValueGetSimple(virNWFilterVarValuePtr val);
+const char *virNWFilterVarValueGetNthValue(virNWFilterVarValuePtr val,
+ unsigned int idx);
+unsigned int virNWFilterVarValueGetCardinality(virNWFilterVarValuePtr val);
+bool virNWFilterVarValueDelValue(virNWFilterVarValuePtr val,
+ const char *value);
+bool virNWFilterVarValueAddValue(virNWFilterVarValuePtr val,
+ const char *value,
+ bool make_copy);
+void virNWFilterVarValuePrint(virNWFilterVarValuePtr val, virBufferPtr buf);
typedef struct _virNWFilterHashTable virNWFilterHashTable;
typedef virNWFilterHashTable *virNWFilterHashTablePtr;
@@ -42,7 +78,7 @@ virNWFilterHashTablePtr virNWFilterHashT
void virNWFilterHashTableFree(virNWFilterHashTablePtr table);
int virNWFilterHashTablePut(virNWFilterHashTablePtr table,
const char *name,
- char *val,
+ virNWFilterVarValuePtr val,
int freeName);
int virNWFilterHashTableRemoveEntry(virNWFilterHashTablePtr table,
const char *name);
Index: libvirt-acl/src/conf/domain_conf.c
===================================================================
--- libvirt-acl.orig/src/conf/domain_conf.c
+++ libvirt-acl/src/conf/domain_conf.c
@@ -3152,7 +3152,7 @@ virDomainNetDefParseXML(virCapsPtr caps,
event_idx = virXMLPropString(cur, "event_idx");
} else if (xmlStrEqual (cur->name, BAD_CAST "filterref")) {
filter = virXMLPropString(cur, "filter");
- VIR_FREE(filterparams);
+ virNWFilterHashTableFree(filterparams);
filterparams = virNWFilterParseParamAttributes(cur);
} else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) &&
xmlStrEqual(cur->name, BAD_CAST "state")) {
Index: libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c
===================================================================
--- libvirt-acl.orig/src/nwfilter/nwfilter_gentech_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_gentech_driver.c
@@ -147,10 +147,17 @@ virNWFilterVarHashmapAddStdValues(virNWF
char *macaddr,
char *ipaddr)
{
+ virNWFilterVarValue *val;
+
if (macaddr) {
+ val = virNWFilterVarValueCreateSimple(macaddr, false);
+ if (!val) {
+ virReportOOMError();
+ return 1;
+ }
if (virHashAddEntry(table->hashTable,
NWFILTER_STD_VAR_MAC,
- macaddr) < 0) {
+ val) < 0) {
virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Could not add variable
'MAC' to hashmap"));
return 1;
@@ -158,9 +165,14 @@ virNWFilterVarHashmapAddStdValues(virNWF
}
if (ipaddr) {
+ val = virNWFilterVarValueCreateSimple(ipaddr, false);
+ if (!val) {
+ virReportOOMError();
+ return 1;
+ }
if (virHashAddEntry(table->hashTable,
NWFILTER_STD_VAR_IP,
- ipaddr) < 0) {
+ val) < 0) {
virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Could not add variable
'IP' to hashmap"));
return 1;
@@ -491,6 +503,7 @@ virNWFilterDetermineMissingVarsRec(virCo
int rc = 0;
int i, j;
virNWFilterDefPtr next_filter;
+ virNWFilterVarValuePtr val;
for (i = 0; i < filter->nentries; i++) {
virNWFilterRuleDefPtr rule = filter->filterEntries[i]->rule;
@@ -499,10 +512,18 @@ virNWFilterDetermineMissingVarsRec(virCo
/* check all variables of this rule */
for (j = 0; j < rule->nvars; j++) {
if (!virHashLookup(vars->hashTable, rule->vars[j])) {
+ val = virNWFilterVarValueCreateSimple("1", true);
+ if (!val) {
+ virReportOOMError();
+ rc = 1;
+ break;
+ }
virNWFilterHashTablePut(missing_vars, rule->vars[j],
- strdup("1"), 1);
+ val, 1);
}
}
+ if (rc)
+ break;
} else if (inc) {
VIR_DEBUG("Following filter %s\n", inc->filterref);
obj = virNWFilterObjFindByName(&driver->nwfilters,
inc->filterref);
Index: libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c
===================================================================
--- libvirt-acl.orig/src/nwfilter/nwfilter_learnipaddr.c
+++ libvirt-acl/src/nwfilter/nwfilter_learnipaddr.c
@@ -313,10 +313,14 @@ virNWFilterDeregisterLearnReq(int ifinde
static int
virNWFilterAddIpAddrForIfname(const char *ifname, char *addr) {
int ret;
+ virNWFilterVarValuePtr val = virNWFilterVarValueCreateSimple(addr, false);
+
+ if (!val)
+ return 1;
virMutexLock(&ipAddressMapLock);
- ret = virNWFilterHashTablePut(ipAddressMap, ifname, addr, 1);
+ ret = virNWFilterHashTablePut(ipAddressMap, ifname, val, 1);
virMutexUnlock(&ipAddressMapLock);
@@ -339,7 +343,7 @@ virNWFilterDelIpAddrForIfname(const char
const char *
virNWFilterGetIpAddrForIfname(const char *ifname) {
- const char *res;
+ virNWFilterVarValuePtr res;
virMutexLock(&ipAddressMapLock);
@@ -347,7 +351,10 @@ virNWFilterGetIpAddrForIfname(const char
virMutexUnlock(&ipAddressMapLock);
- return res;
+ if (res)
+ return virNWFilterVarValueGetSimple(res);
+
+ return NULL;
}
Index: libvirt-acl/src/libvirt_private.syms
===================================================================
--- libvirt-acl.orig/src/libvirt_private.syms
+++ libvirt-acl/src/libvirt_private.syms
@@ -881,6 +881,9 @@ virNWFilterHashTableFree;
virNWFilterHashTablePut;
virNWFilterHashTablePutAll;
virNWFilterHashTableRemoveEntry;
+virNWFilterVarValueCreateSimple;
+virNWFilterVarValueGetSimple;
+virNWFilterVarValuePrint;
# pci.h
5:13 a.m.
New subject: [libvirt] [PATCH V3 1/4] Rework value part of name-value pairs
On Mon, Oct 24, 2011 at 12:07:27PM -0400, Stefan Berger wrote:
NWFilters can be provided name-value pairs using the following
XML notiation:
<filterref filter='xyz'>
<parameter name='PORT' value='80'/>
<parameter name='VAL' value='abc'/>
</filterref>
The internal representation currently is so that a name is stored as a
string and the value as well. This patch now addresses the value part of it
and introduces a data structure for storing a value either as a simple
value or as an array for later support of lists (provided in python-like
notation ( [a,b,c] ).
This patch adjusts all code that was handling the values in hash tables
and makes it use the new data type.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/conf/domain_conf.c | 2
src/conf/nwfilter_params.c | 288 ++++++++++++++++++++++++++++--
src/conf/nwfilter_params.h | 38 +++
src/libvirt_private.syms | 3
src/nwfilter/nwfilter_ebiptables_driver.c | 15 +
src/nwfilter/nwfilter_gentech_driver.c | 27 ++
src/nwfilter/nwfilter_learnipaddr.c | 13 +
7 files changed, 365 insertions(+), 21 deletions(-)
+bool
+virNWFilterVarValueDelValue(virNWFilterVarValuePtr val, const char *value)
+{
+ unsigned int i;
+
+ switch (val->valType) {
+ case NWFILTER_VALUE_TYPE_SIMPLE:
+ return false;
+
+ case NWFILTER_VALUE_TYPE_ARRAY:
+ for (i = 0; i < val->u.array.nValues; i++) {
+ if (STREQ(value, val->u.array.values[i])) {
+ VIR_FREE(val->u.array.values[i]);
+ val->u.array.nValues--;
+ val->u.array.values[i] =
+ val->u.array.values[val->u.array.nValues];
+ return true;
This doesn't look right. Consider
| A | B | C | D | E |
And you're deleting 'B'. This code will result in a list
| A | C | C | D |
because you're only shuffling down the immediate neighbour
of the element being deleted, rather than all following
elements.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
6:41 a.m.
New subject: [libvirt] [PATCH V3 1/4] Rework value part of name-value pairs
On 10/27/2011 06:13 AM, Daniel P. Berrange wrote:
On Mon, Oct 24, 2011 at 12:07:27PM -0400, Stefan Berger wrote:
> NWFilters can be provided name-value pairs using the following
> XML notiation:
>
> <filterref filter='xyz'>
> <parameter name='PORT' value='80'/>
> <parameter name='VAL' value='abc'/>
> </filterref>
>
> The internal representation currently is so that a name is stored as a
> string and the value as well. This patch now addresses the value part of it
> and introduces a data structure for storing a value either as a simple
> value or as an array for later support of lists (provided in python-like
> notation ( [a,b,c] ).
>
> This patch adjusts all code that was handling the values in hash tables
> and makes it use the new data type.
>
> Signed-off-by: Stefan Berger<stefanb(a)linux.vnet.ibm.com>
>
> ---
> src/conf/domain_conf.c | 2
> src/conf/nwfilter_params.c | 288 ++++++++++++++++++++++++++++--
> src/conf/nwfilter_params.h | 38 +++
> src/libvirt_private.syms | 3
> src/nwfilter/nwfilter_ebiptables_driver.c | 15 +
> src/nwfilter/nwfilter_gentech_driver.c | 27 ++
> src/nwfilter/nwfilter_learnipaddr.c | 13 +
> 7 files changed, 365 insertions(+), 21 deletions(-)
> +bool
> +virNWFilterVarValueDelValue(virNWFilterVarValuePtr val, const char *value)
> +{
> + unsigned int i;
> +
> + switch (val->valType) {
> + case NWFILTER_VALUE_TYPE_SIMPLE:
> + return false;
> +
> + case NWFILTER_VALUE_TYPE_ARRAY:
> + for (i = 0; i< val->u.array.nValues; i++) {
> + if (STREQ(value, val->u.array.values[i])) {
> + VIR_FREE(val->u.array.values[i]);
> + val->u.array.nValues--;
> + val->u.array.values[i] =
> + val->u.array.values[val->u.array.nValues];
> + return true;
This doesn't look right. Consider
| A | B | C | D | E |
And you're deleting 'B'. This code will result in a list
| A | C | C | D |
We had nValues = 5 here. We remove item at i=1. nValues = 4 now.
array[1] = array[4] = 'E' -> A | E | C | D.
But it's wrong since I should preserve the ordering of the elements. I
fixed it. Will post a v4. Thanks for the review.
Stefan
11:07 a.m.
New subject: [libvirt] [PATCH V3 2/4] Create rules for each member of a list
This patch extends the NWFilter driver for Linux (ebiptables) to create
rules for each member of a previously introduced list. If for example
an attribute value (internally) looks like this:
IP = [10.0.0.1, 10.0.0.2, 10.0.0.3]
then 3 rules will be generated for a rule accessing the variable 'IP',
one for each member of the list. The effect of this is that this now
allows for filtering for multiple values in one field. This can then be
used to support for filtering/allowing of multiple IP addresses per
interface.
An interator is introduced that extracts each member of a list and
puts it into a hash table which then is passed to the function creating
a rule. For the above example the iterator would cause 3 loops.
v2:
- pass the iterator all the way to the function that accesses the
hash table and provide a function to pick the value of a variable
that is reflected by the current state of the iterator
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/conf/nwfilter_params.c | 129 ++++++++++++++++++++++++++++++
src/conf/nwfilter_params.h | 25 +++++
src/libvirt_private.syms | 4
src/nwfilter/nwfilter_ebiptables_driver.c | 84 +++++++++++++------
4 files changed, 215 insertions(+), 27 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===================================================================
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -192,7 +192,7 @@ static const struct ushort_map l3_protoc
static int
-printVar(virNWFilterHashTablePtr vars,
+printVar(virNWFilterVarCombIterPtr vars,
char *buf, int bufsize,
nwItemDescPtr item,
int *done)
@@ -200,22 +200,11 @@ printVar(virNWFilterHashTablePtr vars,
*done = 0;
if ((item->flags & NWFILTER_ENTRY_ITEM_FLAG_HAS_VAR)) {
- virNWFilterVarValuePtr varval;
const char *val;
- varval = virHashLookup(vars->hashTable, item->var);
- if (!varval) {
- virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot find value for '%s'"),
- item->var);
- return 1;
- }
-
- val = virNWFilterVarValueGetSimple(varval);
+ val = virNWFilterVarCombIterGetVarValue(vars, item->var);
if (!val) {
- virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot get simple value of
'%s'"),
- item->var);
+ /* error has been reported */
return 1;
}
@@ -234,7 +223,7 @@ printVar(virNWFilterHashTablePtr vars,
static int
-_printDataType(virNWFilterHashTablePtr vars,
+_printDataType(virNWFilterVarCombIterPtr vars,
char *buf, int bufsize,
nwItemDescPtr item,
bool asHex)
@@ -329,7 +318,7 @@ _printDataType(virNWFilterHashTablePtr v
static int
-printDataType(virNWFilterHashTablePtr vars,
+printDataType(virNWFilterVarCombIterPtr vars,
char *buf, int bufsize,
nwItemDescPtr item)
{
@@ -338,7 +327,7 @@ printDataType(virNWFilterHashTablePtr va
static int
-printDataTypeAsHex(virNWFilterHashTablePtr vars,
+printDataTypeAsHex(virNWFilterVarCombIterPtr vars,
char *buf, int bufsize,
nwItemDescPtr item)
{
@@ -406,7 +395,7 @@ ebiptablesAddRuleInst(virNWFilterRuleIns
static int
ebtablesHandleEthHdr(virBufferPtr buf,
- virNWFilterHashTablePtr vars,
+ virNWFilterVarCombIterPtr vars,
ethHdrDataDefPtr ethHdr,
bool reverse)
{
@@ -884,7 +873,7 @@ iptablesInstCommand(virBufferPtr buf,
static int
iptablesHandleSrcMacAddr(virBufferPtr buf,
- virNWFilterHashTablePtr vars,
+ virNWFilterVarCombIterPtr vars,
nwItemDescPtr srcMacAddr,
int directionIn,
bool *srcmacskipped)
@@ -921,7 +910,7 @@ err_exit:
static int
iptablesHandleIpHdr(virBufferPtr buf,
virBufferPtr afterStateMatch,
- virNWFilterHashTablePtr vars,
+ virNWFilterVarCombIterPtr vars,
ipHdrDataDefPtr ipHdr,
int directionIn,
bool *skipRule, bool *skipMatch,
@@ -1095,7 +1084,7 @@ err_exit:
static int
iptablesHandlePortData(virBufferPtr buf,
- virNWFilterHashTablePtr vars,
+ virNWFilterVarCombIterPtr vars,
portDataDefPtr portData,
int directionIn)
{
@@ -1201,7 +1190,7 @@ _iptablesCreateRuleInstance(int directio
virNWFilterDefPtr nwfilter,
virNWFilterRuleDefPtr rule,
const char *ifname,
- virNWFilterHashTablePtr vars,
+ virNWFilterVarCombIterPtr vars,
virNWFilterRuleInstPtr res,
const char *match, bool defMatch,
const char *accept_target,
@@ -1687,7 +1676,7 @@ static int
iptablesCreateRuleInstanceStateCtrl(virNWFilterDefPtr nwfilter,
virNWFilterRuleDefPtr rule,
const char *ifname,
- virNWFilterHashTablePtr vars,
+ virNWFilterVarCombIterPtr vars,
virNWFilterRuleInstPtr res,
bool isIPv6)
{
@@ -1812,7 +1801,7 @@ static int
iptablesCreateRuleInstance(virNWFilterDefPtr nwfilter,
virNWFilterRuleDefPtr rule,
const char *ifname,
- virNWFilterHashTablePtr vars,
+ virNWFilterVarCombIterPtr vars,
virNWFilterRuleInstPtr res,
bool isIPv6)
{
@@ -1937,7 +1926,7 @@ ebtablesCreateRuleInstance(char chainPre
virNWFilterDefPtr nwfilter,
virNWFilterRuleDefPtr rule,
const char *ifname,
- virNWFilterHashTablePtr vars,
+ virNWFilterVarCombIterPtr vars,
virNWFilterRuleInstPtr res,
bool reverse)
{
@@ -2429,7 +2418,7 @@ ebiptablesCreateRuleInstance(virConnectP
virNWFilterDefPtr nwfilter,
virNWFilterRuleDefPtr rule,
const char *ifname,
- virNWFilterHashTablePtr vars,
+ virNWFilterVarCombIterPtr vars,
virNWFilterRuleInstPtr res)
{
int rc = 0;
@@ -2513,6 +2502,47 @@ ebiptablesCreateRuleInstance(virConnectP
return rc;
}
+static int
+ebiptablesCreateRuleInstanceCombinations(
+ virConnectPtr conn ATTRIBUTE_UNUSED,
+ enum virDomainNetType nettype ATTRIBUTE_UNUSED,
+ virNWFilterDefPtr nwfilter,
+ virNWFilterRuleDefPtr rule,
+ const char *ifname,
+ virNWFilterHashTablePtr vars,
+ virNWFilterRuleInstPtr res)
+{
+ int rc = 0;
+ virNWFilterVarCombIterPtr vciter;
+ unsigned int loop_limit = 10;
+
+ /* rule->vars holds all the variables names that this rule will access.
+ * iterate over all combinations of the variables' values and instantiate
+ * the filtering rule with each combination.
+ */
+ vciter = virNWFilterVarCombIterCreate(vars, rule->vars, rule->nvars,
+ loop_limit);
+ if (!vciter) {
+ return 1;
+ }
+
+ do {
+ rc = ebiptablesCreateRuleInstance(conn,
+ nettype,
+ nwfilter,
+ rule,
+ ifname,
+ vciter,
+ res);
+ if (rc)
+ break;
+ vciter = virNWFilterVarCombIterNext(vciter);
+ } while (vciter != NULL);
+
+ virNWFilterVarCombIterFree(vciter);
+
+ return rc;
+}
static int
ebiptablesFreeRuleInstance(void *_inst)
@@ -3896,7 +3926,7 @@ virNWFilterTechDriver ebiptables_driver
.init = ebiptablesDriverInit,
.shutdown = ebiptablesDriverShutdown,
- .createRuleInstance = ebiptablesCreateRuleInstance,
+ .createRuleInstance = ebiptablesCreateRuleInstanceCombinations,
.applyNewRules = ebiptablesApplyNewRules,
.tearNewRules = ebiptablesTearNewRules,
.tearOldRules = ebiptablesTearOldRules,
Index: libvirt-acl/src/conf/nwfilter_params.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_params.c
+++ libvirt-acl/src/conf/nwfilter_params.c
@@ -288,6 +288,135 @@ virNWFilterVarValueAddValue(virNWFilterV
return rc;
}
+void
+virNWFilterVarCombIterFree(virNWFilterVarCombIterPtr ci)
+{
+ if (!ci)
+ return;
+
+ VIR_FREE(ci);
+}
+
+virNWFilterVarCombIterPtr
+virNWFilterVarCombIterCreate(virNWFilterHashTablePtr hash,
+ char * const *vars, unsigned int nVars,
+ unsigned int loop_limit)
+{
+ virNWFilterVarCombIterPtr res;
+ virNWFilterVarValuePtr varVal;
+ unsigned int i;
+ unsigned int loops = 1;
+
+ if (VIR_ALLOC_VAR(res, virNWFilterVarCombEntry, nVars) < 0) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ res->nEntries = nVars;
+ res->hashTable = hash;
+
+ for (i = 0; i < nVars; i++) {
+ varVal = virHashLookup(hash->hashTable, vars[i]);
+ if (varVal == NULL) {
+ virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not find value for variable
'%s'"),
+ vars[i]);
+ goto err_exit;
+ }
+
+ res->entry[i].key = vars[i];
+ res->entry[i].cardinality = virNWFilterVarValueGetCardinality(varVal);
+
+ loops *= res->entry[i].cardinality;
+ if (loop_limit && loops > loop_limit) {
+ virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Lists sizes are too big and would cause
"
+ "the iterator to exceep the maximum
number"
+ "of loops (%u)"), loop_limit);
+ goto err_exit;
+ }
+ }
+
+ return res;
+
+err_exit:
+ virNWFilterVarCombIterFree(res);
+ return NULL;
+}
+
+virNWFilterVarCombIterPtr
+virNWFilterVarCombIterNext(virNWFilterVarCombIterPtr ci)
+{
+ unsigned int i;
+
+ if ((ci->nEntries == 0) ||
+ (ci->nEntries == 1 && ci->entry[0].cardinality == 1)) {
+ virNWFilterVarCombIterFree(ci);
+ return NULL;
+ }
+
+ for (i = 0; i < ci->nEntries; i++) {
+ if (ci->entry[i].cardinality == 1)
+ continue;
+
+ ci->entry[i].idx++;
+ if (ci->entry[i].idx < ci->entry[i].cardinality)
+ break;
+ else
+ ci->entry[i].idx = 0;
+ }
+
+ if (ci->nEntries == i) {
+ virNWFilterVarCombIterFree(ci);
+ return NULL;
+ }
+
+ return ci;
+}
+
+const char *
+virNWFilterVarCombIterGetVarValue(virNWFilterVarCombIterPtr ci,
+ const char *varname)
+{
+ unsigned int i;
+ bool found = false;
+ const char *res = NULL;
+ virNWFilterVarValuePtr value;
+
+ for (i = 0; i < ci->nEntries; i++) {
+ if (STREQ(ci->entry[i].key, varname)) {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found) {
+ virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not find variable '%s' in
iterator"),
+ varname);
+ return NULL;
+ }
+
+ value = virHashLookup(ci->hashTable->hashTable, varname);
+ if (!value) {
+ virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not find value for variable
'%s'"),
+ varname);
+ return NULL;
+ }
+
+ res = virNWFilterVarValueGetNthValue(value, ci->entry[i].idx);
+ if (!res) {
+ virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not get nth (%u) value of "
+ "variable '%s'"),
+ ci->entry[i].idx, varname);
+ return NULL;
+ }
+
+ return res;
+}
+
static void
hashDataFree(void *payload, const void *name ATTRIBUTE_UNUSED)
{
Index: libvirt-acl/src/conf/nwfilter_params.h
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_params.h
+++ libvirt-acl/src/conf/nwfilter_params.h
@@ -91,4 +91,29 @@ int virNWFilterHashTablePutAll(virNWFilt
# define VALID_VARVALUE \
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_.:"
+typedef struct _virNWFilterVarCombEntry virNWFilterVarCombEntry;
+struct _virNWFilterVarCombEntry {
+ const char *key;
+ unsigned int idx;
+ unsigned int cardinality;
+};
+
+typedef struct _virNWFilterVarCombIter virNWFilterVarCombIter;
+typedef virNWFilterVarCombIter *virNWFilterVarCombIterPtr;
+struct _virNWFilterVarCombIter {
+ virNWFilterHashTablePtr hashTable;
+ unsigned int nEntries;
+ virNWFilterVarCombEntry entry[1];
+};
+virNWFilterVarCombIterPtr virNWFilterVarCombIterCreate(
+ virNWFilterHashTablePtr hash,
+ char * const *vars, unsigned int nVars,
+ unsigned int loop_limit);
+
+void virNWFilterVarCombIterFree(virNWFilterVarCombIterPtr ci);
+virNWFilterVarCombIterPtr virNWFilterVarCombIterNext(
+ virNWFilterVarCombIterPtr ci);
+const char *virNWFilterVarCombIterGetVarValue(virNWFilterVarCombIterPtr ci,
+ const char *varname);
+
#endif /* NWFILTER_PARAMS_H */
Index: libvirt-acl/src/libvirt_private.syms
===================================================================
--- libvirt-acl.orig/src/libvirt_private.syms
+++ libvirt-acl/src/libvirt_private.syms
@@ -881,6 +881,10 @@ virNWFilterHashTableFree;
virNWFilterHashTablePut;
virNWFilterHashTablePutAll;
virNWFilterHashTableRemoveEntry;
+virNWFilterVarCombIterCreate;
+virNWFilterVarCombIterFree;
+virNWFilterVarCombIterGetVarValue;
+virNWFilterVarCombIterNext;
virNWFilterVarValueCreateSimple;
virNWFilterVarValueGetSimple;
virNWFilterVarValuePrint;
5:14 a.m.
New subject: [libvirt] [PATCH V3 2/4] Create rules for each member of a list
On Mon, Oct 24, 2011 at 12:07:28PM -0400, Stefan Berger wrote:
This patch extends the NWFilter driver for Linux (ebiptables) to
create
rules for each member of a previously introduced list. If for example
an attribute value (internally) looks like this:
IP = [10.0.0.1, 10.0.0.2, 10.0.0.3]
then 3 rules will be generated for a rule accessing the variable 'IP',
one for each member of the list. The effect of this is that this now
allows for filtering for multiple values in one field. This can then be
used to support for filtering/allowing of multiple IP addresses per
interface.
An interator is introduced that extracts each member of a list and
puts it into a hash table which then is passed to the function creating
a rule. For the above example the iterator would cause 3 loops.
v2:
- pass the iterator all the way to the function that accesses the
hash table and provide a function to pick the value of a variable
that is reflected by the current state of the iterator
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
src/conf/nwfilter_params.c | 129 ++++++++++++++++++++++++++++++
src/conf/nwfilter_params.h | 25 +++++
src/libvirt_private.syms | 4
src/nwfilter/nwfilter_ebiptables_driver.c | 84 +++++++++++++------
4 files changed, 215 insertions(+), 27 deletions(-)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
12:03 p.m.
New subject: [libvirt] [PATCH V3 2/4] Create rules for each member of a list
On 10/27/2011 06:14 AM, Daniel P. Berrange wrote:
On Mon, Oct 24, 2011 at 12:07:28PM -0400, Stefan Berger wrote:
> This patch extends the NWFilter driver for Linux (ebiptables) to create
> rules for each member of a previously introduced list. If for example
> an attribute value (internally) looks like this:
>
> IP = [10.0.0.1, 10.0.0.2, 10.0.0.3]
>
> then 3 rules will be generated for a rule accessing the variable 'IP',
> one for each member of the list. The effect of this is that this now
> allows for filtering for multiple values in one field. This can then be
> used to support for filtering/allowing of multiple IP addresses per
> interface.
>
> An interator is introduced that extracts each member of a list and
> puts it into a hash table which then is passed to the function creating
> a rule. For the above example the iterator would cause 3 loops.
>
> v2:
> - pass the iterator all the way to the function that accesses the
> hash table and provide a function to pick the value of a variable
> that is reflected by the current state of the iterator
>
> Signed-off-by: Stefan Berger<stefanb(a)linux.vnet.ibm.com>
>
> ---
> src/conf/nwfilter_params.c | 129 ++++++++++++++++++++++++++++++
> src/conf/nwfilter_params.h | 25 +++++
> src/libvirt_private.syms | 4
> src/nwfilter/nwfilter_ebiptables_driver.c | 84 +++++++++++++------
> 4 files changed, 215 insertions(+), 27 deletions(-)
ACK
I now modified the iterator to NOT create every combination of the items
of multiple lists, but have all lists processed in parallel. I think
this is for now the needed behaviour. So if someone has a rule
containing $IP and $MAC, then both lists have to have the same size and
their elements will be accessed $IP[m] and $MAC[m] to instantiate the
rule. To have them independently processed we'll need to go through how
the variables are accessed and then maybe a notation of $IP[@1] and
$MAC[@2] will create all possible combinations. Sorry for the confusion.
Stefan
Daniel
11:07 a.m.
New subject: [libvirt] [PATCH V3 3/4] Extend NWFilter parameter parser to cope with lists of values
This patch modifies the NWFilter parameter parser to support multiple
elements with the same name and to internally build a list of items.
An example of the XML looks like this:
<parameter name='TEST' value='10.1.2.3'/>
<parameter name='TEST' value='10.2.3.4'/>
<parameter name='TEST' value='10.1.1.1'/>
The list of values is then stored in the newly introduced data type
virNWFilterVarValue.
The XML formatter is also adapted to print out all items in alphabetical
order sorted by 'name'.
This patch als fixes a bug in the XML schema on the way.
v3:
- Follow Daniel Berrange's suggestion of parsing a list as shown above
- Rewrote XML formatter to print out parameters in alphabetical order
v2:
- check that each item in the list only contains allowed characters
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
docs/schemas/nwfilter.rng | 18 +++++----
src/conf/nwfilter_params.c | 86 ++++++++++++++++++++++++++++-----------------
2 files changed, 64 insertions(+), 40 deletions(-)
Index: libvirt-acl/src/conf/nwfilter_params.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_params.c
+++ libvirt-acl/src/conf/nwfilter_params.c
@@ -595,7 +595,7 @@ isValidVarName(const char *var)
static bool
isValidVarValue(const char *value)
{
- return value[strspn(value, VALID_VARVALUE)] == 0;
+ return (value[strspn(value, VALID_VARVALUE)] == 0) && (strlen(value) != 0);
}
static virNWFilterVarValuePtr
@@ -627,15 +627,22 @@ virNWFilterParseParamAttributes(xmlNodeP
if (nam != NULL && val != NULL) {
if (!isValidVarName(nam))
goto skip_entry;
- value = virNWFilterParseVarValue(val);
- if (!value)
+ if (!isValidVarValue(val))
goto skip_entry;
- if (virNWFilterHashTablePut(table, nam, value, 1)) {
- VIR_FREE(nam);
- VIR_FREE(val);
- virNWFilterVarValueFree(value);
- virNWFilterHashTableFree(table);
- return NULL;
+ value = virHashLookup(table->hashTable, nam);
+ if (value) {
+ /* add value to existing value -> list */
+ if (!virNWFilterVarValueAddValue(value, val, false)) {
+ value = NULL;
+ goto err_exit;
+ }
+ val = NULL;
+ } else {
+ value = virNWFilterParseVarValue(val);
+ if (!value)
+ goto skip_entry;
+ if (virNWFilterHashTablePut(table, nam, value, 1))
+ goto err_exit;
}
value = NULL;
}
@@ -648,40 +655,55 @@ skip_entry:
cur = cur->next;
}
return table;
-}
-
-
-struct formatterParam {
- virBufferPtr buf;
- const char *indent;
-};
+err_exit:
+ VIR_FREE(nam);
+ VIR_FREE(val);
+ virNWFilterVarValueFree(value);
+ virNWFilterHashTableFree(table);
+ return NULL;
+}
-static void
-_formatParameterAttrs(void *payload, const void *name, void *data)
+static int
+virNWFilterFormatParameterNameSorter(const virHashKeyValuePairPtr a,
+ const virHashKeyValuePairPtr b)
{
- struct formatterParam *fp = (struct formatterParam *)data;
- virNWFilterVarValuePtr value = payload;
-
- virBufferAsprintf(fp->buf, "%s<parameter name='%s'
value='",
- fp->indent,
- (const char *)name);
- virNWFilterVarValuePrint(value, fp->buf);
- virBufferAddLit(fp->buf, "'/>\n");
+ return strcmp((const char *)a->key, (const char *)b->key);
}
-
char *
virNWFilterFormatParamAttributes(virNWFilterHashTablePtr table,
const char *indent)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- struct formatterParam fp = {
- .buf = &buf,
- .indent = indent,
- };
+ char **keys, *key;
+ int i, j, card, numKeys;
+ virNWFilterVarValuePtr value;
+
+ if (!table)
+ return NULL;
+
+ keys = (char **)virHashGetKeys(table->hashTable,
+ virNWFilterFormatParameterNameSorter);
+ if (!keys)
+ return NULL;
+
+ numKeys = virHashSize(table->hashTable);
+
+ for (i = 0; i < numKeys; i++) {
+ value = virHashLookup(table->hashTable, keys[i]);
+ card = virNWFilterVarValueGetCardinality(value);
+
+ for (j = 0; j < card; j++) {
+ virBufferAsprintf(&buf,
+ "%s<parameter name='%s'
value='%s'/>\n",
+ indent, keys[i],
+ virNWFilterVarValueGetNthValue(value, j));
+ }
+ key = keys[i];
+ }
- virHashForEach(table->hashTable, _formatParameterAttrs, &fp);
+ virHashFreeKeys(table->hashTable, (void **)keys);
if (virBufferError(&buf)) {
virReportOOMError();
Index: libvirt-acl/docs/schemas/nwfilter.rng
===================================================================
--- libvirt-acl.orig/docs/schemas/nwfilter.rng
+++ libvirt-acl/docs/schemas/nwfilter.rng
@@ -313,14 +313,16 @@
<data type="NCName"/>
</attribute>
<optional>
- <element name="parameter">
- <attribute name="name">
- <ref name="filter-param-name"/>
- </attribute>
- <attribute name="value">
- <ref name="filter-param-value"/>
- </attribute>
- </element>
+ <zeroOrMore>
+ <element name="parameter">
+ <attribute name="name">
+ <ref name="filter-param-name"/>
+ </attribute>
+ <attribute name="value">
+ <ref name="filter-param-value"/>
+ </attribute>
+ </element>
+ </zeroOrMore>
</optional>
</define>
11:31 a.m.
New subject: [libvirt] [PATCH V3 3/4] Extend NWFilter parameter parser to cope with lists of values
2011/10/24 Stefan Berger <stefanb(a)linux.vnet.ibm.com>:
This patch modifies the NWFilter parameter parser to support
multiple
elements with the same name and to internally build a list of items.
An example of the XML looks like this:
<parameter name='TEST' value='10.1.2.3'/>
<parameter name='TEST' value='10.2.3.4'/>
<parameter name='TEST' value='10.1.1.1'/>
The list of values is then stored in the newly introduced data type
virNWFilterVarValue.
The XML formatter is also adapted to print out all items in alphabetical
order sorted by 'name'.
This patch als fixes a bug in the XML schema on the way.
v3:
- Follow Daniel Berrange's suggestion of parsing a list as shown above
- Rewrote XML formatter to print out parameters in alphabetical order
The parameters are stored in a hash and their name is used as key.
This means that the parameters aren't guaranteed to be printed to XML
again in the same order as they where parsed from XML, doesn't it?
This might be no problem when all parameters have a unique name. But
now with multiple parameters having the same name this might be an
issue, assuming that a given element order has a meaning attached to
it. If
<parameter name='TEST' value='10.1.2.3'/>
<parameter name='TEST' value='10.2.3.4'/>
<parameter name='TEST' value='10.1.1.1'/>
and
<parameter name='TEST' value='10.1.1.1'/>
<parameter name='TEST' value='10.1.2.3'/>
<parameter name='TEST' value='10.2.3.4'/>
aren't the same from the NWFilter's point-of-view then this needs to
be fixed to have NWFilter preserve the exact order of elements.
If it's the same then there's no problem here.
--
Matthias Bolte
http://photron.blogspot.com
12:18 p.m.
New subject: [libvirt] [PATCH V3 3/4] Extend NWFilter parameter parser to cope with lists of values
On 10/24/2011 12:31 PM, Matthias Bolte wrote:
2011/10/24 Stefan Berger<stefanb(a)linux.vnet.ibm.com>:
> This patch modifies the NWFilter parameter parser to support multiple
> elements with the same name and to internally build a list of items.
> An example of the XML looks like this:
>
> <parameter name='TEST' value='10.1.2.3'/>
> <parameter name='TEST' value='10.2.3.4'/>
> <parameter name='TEST' value='10.1.1.1'/>
>
> The list of values is then stored in the newly introduced data type
> virNWFilterVarValue.
>
> The XML formatter is also adapted to print out all items in alphabetical
> order sorted by 'name'.
>
> This patch als fixes a bug in the XML schema on the way.
>
> v3:
> - Follow Daniel Berrange's suggestion of parsing a list as shown above
> - Rewrote XML formatter to print out parameters in alphabetical order
The parameters are stored in a hash and their name is used as key.
This means that the parameters aren't guaranteed to be printed to XML
again in the same order as they where parsed from XML, doesn't it?
This might be no problem when all parameters have a unique name. But
now with multiple parameters having the same name this might be an
issue, assuming that a given element order has a meaning attached to
it. If
<parameter name='TEST' value='10.1.2.3'/>
<parameter name='TEST' value='10.2.3.4'/>
<parameter name='TEST' value='10.1.1.1'/>
and
<parameter name='TEST' value='10.1.1.1'/>
<parameter name='TEST' value='10.1.2.3'/>
<parameter name='TEST' value='10.2.3.4'/>
aren't the same from the NWFilter's point-of-view then this needs to
be fixed to have NWFilter preserve the exact order of elements.
If it's the same then there's no problem here.
Internally there's an entry TEST in the hash table that holds the values
in the order they were parsed (increasing index in a string array,
char**). In the XML formatter the list is then printed in that same
order. So the order is preserved and maybe some time in the future one
could select a single value using $TEST[1] and be sure to always get
that same value.
Stefan
5:09 a.m.
New subject: [libvirt] [PATCH V3 3/4] Extend NWFilter parameter parser to cope with lists of values
On Mon, Oct 24, 2011 at 12:07:29PM -0400, Stefan Berger wrote:
This patch modifies the NWFilter parameter parser to support
multiple
elements with the same name and to internally build a list of items.
An example of the XML looks like this:
<parameter name='TEST' value='10.1.2.3'/>
<parameter name='TEST' value='10.2.3.4'/>
<parameter name='TEST' value='10.1.1.1'/>
The list of values is then stored in the newly introduced data type
virNWFilterVarValue.
The XML formatter is also adapted to print out all items in alphabetical
order sorted by 'name'.
This patch als fixes a bug in the XML schema on the way.
v3:
- Follow Daniel Berrange's suggestion of parsing a list as shown above
- Rewrote XML formatter to print out parameters in alphabetical order
v2:
- check that each item in the list only contains allowed characters
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
docs/schemas/nwfilter.rng | 18 +++++----
src/conf/nwfilter_params.c | 86 ++++++++++++++++++++++++++++-----------------
2 files changed, 64 insertions(+), 40 deletions(-)
Index: libvirt-acl/src/conf/nwfilter_params.c
===================================================================
--- libvirt-acl.orig/src/conf/nwfilter_params.c
+++ libvirt-acl/src/conf/nwfilter_params.c
@@ -595,7 +595,7 @@ isValidVarName(const char *var)
static bool
isValidVarValue(const char *value)
{
- return value[strspn(value, VALID_VARVALUE)] == 0;
+ return (value[strspn(value, VALID_VARVALUE)] == 0) && (strlen(value) != 0);
}
static virNWFilterVarValuePtr
@@ -627,15 +627,22 @@ virNWFilterParseParamAttributes(xmlNodeP
if (nam != NULL && val != NULL) {
if (!isValidVarName(nam))
goto skip_entry;
- value = virNWFilterParseVarValue(val);
- if (!value)
+ if (!isValidVarValue(val))
goto skip_entry;
- if (virNWFilterHashTablePut(table, nam, value, 1)) {
- VIR_FREE(nam);
- VIR_FREE(val);
- virNWFilterVarValueFree(value);
- virNWFilterHashTableFree(table);
- return NULL;
+ value = virHashLookup(table->hashTable, nam);
+ if (value) {
+ /* add value to existing value -> list */
+ if (!virNWFilterVarValueAddValue(value, val, false)) {
+ value = NULL;
+ goto err_exit;
+ }
+ val = NULL;
+ } else {
+ value = virNWFilterParseVarValue(val);
+ if (!value)
+ goto skip_entry;
+ if (virNWFilterHashTablePut(table, nam, value, 1))
+ goto err_exit;
}
value = NULL;
}
@@ -648,40 +655,55 @@ skip_entry:
cur = cur->next;
}
return table;
-}
-
-
-struct formatterParam {
- virBufferPtr buf;
- const char *indent;
-};
+err_exit:
+ VIR_FREE(nam);
+ VIR_FREE(val);
+ virNWFilterVarValueFree(value);
+ virNWFilterHashTableFree(table);
+ return NULL;
+}
-static void
-_formatParameterAttrs(void *payload, const void *name, void *data)
+static int
+virNWFilterFormatParameterNameSorter(const virHashKeyValuePairPtr a,
+ const virHashKeyValuePairPtr b)
{
- struct formatterParam *fp = (struct formatterParam *)data;
- virNWFilterVarValuePtr value = payload;
-
- virBufferAsprintf(fp->buf, "%s<parameter name='%s'
value='",
- fp->indent,
- (const char *)name);
- virNWFilterVarValuePrint(value, fp->buf);
- virBufferAddLit(fp->buf, "'/>\n");
+ return strcmp((const char *)a->key, (const char *)b->key);
}
-
char *
virNWFilterFormatParamAttributes(virNWFilterHashTablePtr table,
const char *indent)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
- struct formatterParam fp = {
- .buf = &buf,
- .indent = indent,
- };
+ char **keys, *key;
+ int i, j, card, numKeys;
+ virNWFilterVarValuePtr value;
+
+ if (!table)
+ return NULL;
+
+ keys = (char **)virHashGetKeys(table->hashTable,
+ virNWFilterFormatParameterNameSorter);
+ if (!keys)
+ return NULL;
+
+ numKeys = virHashSize(table->hashTable);
+
+ for (i = 0; i < numKeys; i++) {
+ value = virHashLookup(table->hashTable, keys[i]);
+ card = virNWFilterVarValueGetCardinality(value);
+
+ for (j = 0; j < card; j++) {
+ virBufferAsprintf(&buf,
+ "%s<parameter name='%s'
value='%s'/>\n",
+ indent, keys[i],
+ virNWFilterVarValueGetNthValue(value, j));
+ }
+ key = keys[i];
+ }
- virHashForEach(table->hashTable, _formatParameterAttrs, &fp);
+ virHashFreeKeys(table->hashTable, (void **)keys);
if (virBufferError(&buf)) {
virReportOOMError();
Index: libvirt-acl/docs/schemas/nwfilter.rng
===================================================================
--- libvirt-acl.orig/docs/schemas/nwfilter.rng
+++ libvirt-acl/docs/schemas/nwfilter.rng
@@ -313,14 +313,16 @@
<data type="NCName"/>
</attribute>
<optional>
- <element name="parameter">
- <attribute name="name">
- <ref name="filter-param-name"/>
- </attribute>
- <attribute name="value">
- <ref name="filter-param-value"/>
- </attribute>
- </element>
+ <zeroOrMore>
+ <element name="parameter">
+ <attribute name="name">
+ <ref name="filter-param-name"/>
+ </attribute>
+ <attribute name="value">
+ <ref name="filter-param-value"/>
+ </attribute>
+ </element>
+ </zeroOrMore>
</optional>
</define>
Since you have <zeroOrMore> I think you can remove the parent <optional>
element now
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
6:23 a.m.
New subject: [libvirt] [PATCH V3 3/4] Extend NWFilter parameter parser to cope with lists of values
On 10/27/2011 06:09 AM, Daniel P. Berrange wrote:
On Mon, Oct 24, 2011 at 12:07:29PM -0400, Stefan Berger wrote:
> Index: libvirt-acl/docs/schemas/nwfilter.rng
> ===================================================================
> --- libvirt-acl.orig/docs/schemas/nwfilter.rng
> +++ libvirt-acl/docs/schemas/nwfilter.rng
> @@ -313,14 +313,16 @@
> <data type="NCName"/>
> </attribute>
> <optional>
> -<element name="parameter">
> -<attribute name="name">
> -<ref name="filter-param-name"/>
> -</attribute>
> -<attribute name="value">
> -<ref name="filter-param-value"/>
> -</attribute>
> -</element>
> +<zeroOrMore>
> +<element name="parameter">
> +<attribute name="name">
> +<ref name="filter-param-name"/>
> +</attribute>
> +<attribute name="value">
> +<ref name="filter-param-value"/>
> +</attribute>
> +</element>
> +</zeroOrMore>
> </optional>
> </define>
Since you have<zeroOrMore> I think you can remove the parent<optional>
element now
Fixed.
Stefan
11:07 a.m.
New subject: [libvirt] [PATCH V3 4/4] Add test cases for parsing of list values
This patch adds test cases for parsing of parameters with
multiple occurrances of the same name.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
tests/nwfilterxml2xmlin/attr-value-test.xml | 23 +++++++++++++++++++++++
tests/nwfilterxml2xmlout/attr-value-test.xml | 18 ++++++++++++++++++
tests/nwfilterxml2xmltest.c | 2 ++
3 files changed, 43 insertions(+)
Index: libvirt-acl/tests/nwfilterxml2xmlin/attr-value-test.xml
===================================================================
--- /dev/null
+++ libvirt-acl/tests/nwfilterxml2xmlin/attr-value-test.xml
@@ -0,0 +1,23 @@
+<filter name='testcase'>
+ <uuid>83011800-f663-96d6-8841-fd836b4318c6</uuid>
+ <filterref filter='clean-traffic'>
+ <parameter name='a' value='1.2.3.4'/>
+ <parameter name='a' value='10.1.2.3'/>
+ <parameter name='a' value='10.3.3.3'/>
+ <parameter name='b' value='1.2.3.4'/>
+ </filterref>
+ <rule action='accept' direction='out'>
+ <mac srcmacaddr='1:2:3:4:5:6' srcmacmask='ff:ff:ff:ff:ff:ff'
+ protocolid='arp'/>
+ </rule>
+ <rule action='accept' direction='out'>
+ <tcp srcmacaddr='1:2:3:4:5:6'
+ dstipaddr='10.1.2.3' dstipmask='255.255.255.255'
+ dscp='2'/>
+ </rule>
+ <rule action='accept' direction='out'>
+ <udp-ipv6 srcmacaddr='1:2:3:4:5:6'
+ dstipaddr='a:b:c::d:e:f' dstipmask='128'
+ dscp='2'/>
+ </rule>
+</filter>
Index: libvirt-acl/tests/nwfilterxml2xmlout/attr-value-test.xml
===================================================================
--- /dev/null
+++ libvirt-acl/tests/nwfilterxml2xmlout/attr-value-test.xml
@@ -0,0 +1,18 @@
+<filter name='testcase' chain='root'>
+ <uuid>83011800-f663-96d6-8841-fd836b4318c6</uuid>
+ <filterref filter='clean-traffic'>
+ <parameter name='a' value='1.2.3.4'/>
+ <parameter name='a' value='10.1.2.3'/>
+ <parameter name='a' value='10.3.3.3'/>
+ <parameter name='b' value='1.2.3.4'/>
+ </filterref>
+ <rule action='accept' direction='out' priority='500'>
+ <mac srcmacaddr='01:02:03:04:05:06' srcmacmask='ff:ff:ff:ff:ff:ff'
protocolid='arp'/>
+ </rule>
+ <rule action='accept' direction='out' priority='500'>
+ <tcp srcmacaddr='01:02:03:04:05:06' dstipaddr='10.1.2.3'
dstipmask='32' dscp='2'/>
+ </rule>
+ <rule action='accept' direction='out' priority='500'>
+ <udp-ipv6 srcmacaddr='01:02:03:04:05:06' dstipaddr='a:b:c::d:e:f'
dstipmask='128' dscp='2'/>
+ </rule>
+</filter>
Index: libvirt-acl/tests/nwfilterxml2xmltest.c
===================================================================
--- libvirt-acl.orig/tests/nwfilterxml2xmltest.c
+++ libvirt-acl/tests/nwfilterxml2xmltest.c
@@ -150,6 +150,8 @@ mymain(void)
DO_TEST("chain_prefixtest1", true); /* derived from arp-test */
+ DO_TEST("attr-value-test", false);
+
return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
}
5:14 a.m.
New subject: [libvirt] [PATCH V3 4/4] Add test cases for parsing of list values
On Mon, Oct 24, 2011 at 12:07:30PM -0400, Stefan Berger wrote:
This patch adds test cases for parsing of parameters with
multiple occurrances of the same name.
Signed-off-by: Stefan Berger <stefanb(a)linux.vnet.ibm.com>
---
tests/nwfilterxml2xmlin/attr-value-test.xml | 23 +++++++++++++++++++++++
tests/nwfilterxml2xmlout/attr-value-test.xml | 18 ++++++++++++++++++
tests/nwfilterxml2xmltest.c | 2 ++
3 files changed, 43 insertions(+)
ACK
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
4808
days inactive
4811
days old
13 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Matthias Bolte -
Stefan Berger