10:20 a.m.
When a filter is updated, compare it and the original one for equality
so that unnecessary instantiations of rules can be avoided.
v2:
- added a test case for testing virHashEqual
Regards,
Stefan
10:20 a.m.
New subject: [libvirt] [PATCH v2 1/3] Compare two hash tables for equality
Add function to compare two hash tables for equality.
---
src/util/hash.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
src/util/hash.h | 12 ++++++++++++
2 files changed, 58 insertions(+)
Index: libvirt-iterator/src/util/hash.c
===================================================================
--- libvirt-iterator.orig/src/util/hash.c
+++ libvirt-iterator/src/util/hash.c
@@ -663,3 +663,49 @@ virHashKeyValuePairPtr virHashGetItems(v
return iter.sortArray;
}
+
+struct virHashEqualData
+{
+ bool equal;
+ const virHashTablePtr table2;
+ virHashValueComparator compar;
+};
+
+static int virHashEqualSearcher(const void *payload, const void *name,
+ const void *data)
+{
+ struct virHashEqualData *vhed = (void *)data;
+ const void *value;
+
+ value = virHashLookup(vhed->table2, name);
+ if (!value ||
+ vhed->compar(value, payload) != 0) {
+ /* key is missing in 2nd table or values are different */
+ vhed->equal = false;
+ /* stop 'iteration' */
+ return 1;
+ }
+ return 0;
+}
+
+bool virHashEqual(const virHashTablePtr table1,
+ const virHashTablePtr table2,
+ virHashValueComparator compar)
+{
+ struct virHashEqualData data = {
+ .equal = true,
+ .table2 = table2,
+ .compar = compar,
+ };
+
+ if (table1 == table2)
+ return true;
+
+ if (!table1 || !table2 ||
+ virHashSize(table1) != virHashSize(table2))
+ return false;
+
+ virHashSearch(table1, virHashEqualSearcher, &data);
+
+ return data.equal;
+}
Index: libvirt-iterator/src/util/hash.h
===================================================================
--- libvirt-iterator.orig/src/util/hash.h
+++ libvirt-iterator/src/util/hash.h
@@ -154,6 +154,18 @@ virHashKeyValuePairPtr virHashGetItems(v
virHashKeyComparator compar);
/*
+ * Compare two tables for equality: the lookup of a key's value in
+ * both tables must result in an equivalent value.
+ * The caller must pass in a comparator function for comparing the values
+ * of two keys.
+ */
+typedef int (*virHashValueComparator)(const void *value1, const void *value2);
+bool virHashEqual(const virHashTablePtr table1,
+ const virHashTablePtr table2,
+ virHashValueComparator compar);
+
+
+/*
* Iterators
*/
int virHashForEach(virHashTablePtr table, virHashIterator iter, void *data);
6:13 p.m.
New subject: [libvirt] [PATCH v2 1/3] Compare two hash tables for equality
On 01/18/2012 09:20 AM, Stefan Berger wrote:
Add function to compare two hash tables for equality.
---
src/util/hash.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
src/util/hash.h | 12 ++++++++++++
2 files changed, 58 insertions(+)
ACK. Pretty straightforward.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
10:20 a.m.
New subject: [libvirt] [PATCH v2 2/3] Add test case for virHashEqual function
Add a test case to test the virHashEqual function.
---
tests/hashtest.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)
Index: libvirt-iterator/tests/hashtest.c
===================================================================
--- libvirt-iterator.orig/tests/hashtest.c
+++ libvirt-iterator/tests/hashtest.c
@@ -576,6 +576,83 @@ cleanup:
static int
+testHashEqualCompValue(const void *value1, const void *value2)
+{
+ return strcmp (value1, value2);
+}
+
+
+static int
+testHashEqual(const void *data ATTRIBUTE_UNUSED)
+{
+ virHashTablePtr hash1, hash2;
+ int ret = -1;
+ char keya[] = "a";
+ char keyb[] = "b";
+ char keyc[] = "c";
+ char value1[] = "1";
+ char value2[] = "2";
+ char value3[] = "3";
+ char value4[] = "4";
+
+ if (!(hash1 = virHashCreate(0, NULL)) ||
+ !(hash2 = virHashCreate(0, NULL)) ||
+ virHashAddEntry(hash1, keya, value1) < 0 ||
+ virHashAddEntry(hash1, keyb, value2) < 0 ||
+ virHashAddEntry(hash1, keyc, value3) < 0 ||
+ virHashAddEntry(hash2, keya, value1) < 0 ||
+ virHashAddEntry(hash2, keyb, value2) < 0) {
+ if (virTestGetVerbose()) {
+ testError("\nfailed to create hashes");
+ }
+ goto cleanup;
+ }
+
+ if (virHashEqual(hash1, hash2, testHashEqualCompValue)) {
+ if (virTestGetVerbose()) {
+ testError("\nfailed equal test for different number of elements");
+ }
+ goto cleanup;
+ }
+
+ if (virHashAddEntry(hash2, keyc, value4) < 0) {
+ if (virTestGetVerbose()) {
+ testError("\nfailed to add element to hash2");
+ }
+ goto cleanup;
+ }
+
+ if (virHashEqual(hash1, hash2, testHashEqualCompValue)) {
+ if (virTestGetVerbose()) {
+ testError("\nfailed equal test for same number of elements");
+ }
+ goto cleanup;
+ }
+
+ if (virHashUpdateEntry(hash2, keyc, value3) < 0) {
+ if (virTestGetVerbose()) {
+ testError("\nfailed to update element in hash2");
+ }
+ goto cleanup;
+ }
+
+ if (!virHashEqual(hash1, hash2, testHashEqualCompValue)) {
+ if (virTestGetVerbose()) {
+ testError("\nfailed equal test for equal hash tables");
+ }
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ virHashFree(hash1);
+ virHashFree(hash2);
+ return ret;
+}
+
+
+static int
mymain(void)
{
int ret = 0;
@@ -612,6 +689,7 @@ mymain(void)
DO_TEST("RemoveSet", RemoveSet);
DO_TEST("Search", Search);
DO_TEST("GetItems", GetItems);
+ DO_TEST("Equal", Equal);
return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
}
6:16 p.m.
New subject: [libvirt] [PATCH v2 2/3] Add test case for virHashEqual function
On 01/18/2012 09:20 AM, Stefan Berger wrote:
Add a test case to test the virHashEqual function.
---
tests/hashtest.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 78 insertions(+)
Even better - you test your new API. I would have squashed this into
1/3, but separate patches is okay as well.
static int
+testHashEqualCompValue(const void *value1, const void *value2)
+{
+ return strcmp (value1, value2);
Style: no space before (.
Suggestion - maybe use strcasecmp, and pass in strings that differ in
case to prove that definitions of 'equal' data values is user-defined.
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:41 p.m.
New subject: [libvirt] [PATCH v2 2/3] Add test case for virHashEqual function
On 01/20/2012 07:16 PM, Eric Blake wrote:
On 01/18/2012 09:20 AM, Stefan Berger wrote:
> Add a test case to test the virHashEqual function.
>
> ---
> tests/hashtest.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 78 insertions(+)
Even better - you test your new API. I would have squashed this into
1/3, but separate patches is okay as well.
> static int
> +testHashEqualCompValue(const void *value1, const void *value2)
> +{
> + return strcmp (value1, value2);
Style: no space before (.
c&p from the test case above...
Suggestion - maybe use strcasecmp, and pass in strings that differ
in
case to prove that definitions of 'equal' data values is user-defined.
I changed that to use c_strcasecmp() with values having different case.
Pushed. Thanks.
Stefan
5:19 a.m.
New subject: [libvirt] [PATCH v2 2/3] Add test case for virHashEqual function
On 18.01.2012 17:20, Stefan Berger wrote:
+
+static int
+testHashEqual(const void *data ATTRIBUTE_UNUSED)
+{
+ virHashTablePtr hash1, hash2;
+ int ret = -1;
+ char keya[] = "a";
+ char keyb[] = "b";
+ char keyc[] = "c";
+ char value1[] = "1";
+ char value2[] = "2";
+ char value3[] = "3";
+ char value4[] = "4";
+
+ if (!(hash1 = virHashCreate(0, NULL)) ||
+ !(hash2 = virHashCreate(0, NULL)) ||
Actually, if the first virHashCreate() returns NULL, hash2 remains
uninitialized and we jump
+ virHashAddEntry(hash1, keya, value1) < 0 ||
+ virHashAddEntry(hash1, keyb, value2) < 0 ||
+ virHashAddEntry(hash1, keyc, value3) < 0 ||
+ virHashAddEntry(hash2, keya, value1) < 0 ||
+ virHashAddEntry(hash2, keyb, value2) < 0) {
+ if (virTestGetVerbose()) {
+ testError("\nfailed to create hashes");
+ }
+ goto cleanup;
+ }
over here and do free() on uninitialized pointer.
+
+cleanup:
+ virHashFree(hash1);
+ virHashFree(hash2);
+ return ret;
+}
Therefore I am pushing this under trivial and build-breaker rules (yeah,
one thing - and perhaps the only one - i like about 4.6 gcc is enhanced
static analysis as I spotted warning while compiling current HEAD):
Author: Michal Privoznik <mprivozn(a)redhat.com>
Date: Tue Jan 24 12:09:42 2012 +0100
hashtest: Initialize variable in virHashEqual test
One of latest patches (b7bcb22ce2) enhanced testing for virHashEqual.
However, hash2 variable might be used uninitialized.
diff --git a/tests/hashtest.c b/tests/hashtest.c
index 6c45b01..441672c 100644
--- a/tests/hashtest.c
+++ b/tests/hashtest.c
@@ -583,7 +583,7 @@ testHashEqualCompValue(const void *value1, const
void *value2)
static int
testHashEqual(const void *data ATTRIBUTE_UNUSED)
{
- virHashTablePtr hash1, hash2;
+ virHashTablePtr hash1, hash2 = NULL;
int ret = -1;
char keya[] = "a";
char keyb[] = "b";
5:22 a.m.
New subject: [libvirt] [PATCH v2 2/3] Add test case for virHashEqual function
On 01/24/2012 06:19 AM, Michal Privoznik wrote:
Therefore I am pushing this under trivial and build-breaker rules (yeah,
one thing - and perhaps the only one - i like about 4.6 gcc is enhanced
static analysis as I spotted warning while compiling current HEAD):
Author: Michal Privoznik<mprivozn(a)redhat.com>
Date: Tue Jan 24 12:09:42 2012 +0100
hashtest: Initialize variable in virHashEqual test
One of latest patches (b7bcb22ce2) enhanced testing for virHashEqual.
However, hash2 variable might be used uninitialized.
diff --git a/tests/hashtest.c b/tests/hashtest.c
index 6c45b01..441672c 100644
--- a/tests/hashtest.c
+++ b/tests/hashtest.c
@@ -583,7 +583,7 @@ testHashEqualCompValue(const void *value1, const
void *value2)
static int
testHashEqual(const void *data ATTRIBUTE_UNUSED)
{
- virHashTablePtr hash1, hash2;
+ virHashTablePtr hash1, hash2 = NULL;
int ret = -1;
char keya[] = "a";
char keyb[] = "b";
ACK.
10:20 a.m.
New subject: [libvirt] [PATCH v2 3/3] nwfilter: Rebuild filters only if new filter is different than current
Compare two filter definitions for equality and only rebuild/instantiate
the new filter if the two filters are found to be different. This improves
performance during an update of a filter with no obvious change or the reloading
of filters during a 'kill -SIGHUP'
Unforuntately this more involved than a mere memcmp() on the structures.
---
src/Makefile.am | 1
src/conf/nwfilter_conf.c | 213 ++++++++++++++++++++
src/conf/nwfilter_params.c | 18 +
src/conf/nwfilter_params.h | 2
src/conf/nwfilter_protocols.c | 447 ++++++++++++++++++++++++++++++++++++++++++
src/conf/nwfilter_protocols.h | 56 +++++
6 files changed, 737 insertions(+)
Index: libvirt-iterator/src/conf/nwfilter_conf.c
===================================================================
--- libvirt-iterator.orig/src/conf/nwfilter_conf.c
+++ libvirt-iterator/src/conf/nwfilter_conf.c
@@ -42,6 +42,7 @@
#include "memory.h"
#include "virterror_internal.h"
#include "datatypes.h"
+#include "nwfilter_protocols.h"
#include "nwfilter_params.h"
#include "nwfilter_conf.h"
#include "domain_conf.h"
@@ -140,6 +141,10 @@ static const struct int_map chain_priori
INTMAP_ENTRY_LAST,
};
+static bool virNWFilterDefEqual(const virNWFilterDefPtr def1,
+ const virNWFilterDefPtr def2,
+ bool cmpUUIDs);
+
/*
* only one filter update allowed
*/
@@ -407,6 +412,134 @@ virNWFilterRuleDefAddString(virNWFilterR
return nwf->strings[nwf->nstrings-1];
}
+static bool
+virNWFilterRuleDefEqual(const virNWFilterRuleDefPtr def1,
+ const virNWFilterRuleDefPtr def2)
+{
+ unsigned int i, j;
+ bool equal;
+
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (def1->priority != def2->priority ||
+ def1->flags != def2->flags ||
+ def1->action != def2->action ||
+ def1->tt != def2->tt ||
+ def1->prtclType != def2->prtclType)
+ return false;
+
+ switch (def1->prtclType) {
+ case VIR_NWFILTER_RULE_PROTOCOL_NONE:
+ return true;
+ case VIR_NWFILTER_RULE_PROTOCOL_MAC:
+ equal = virNWFilterEthHdrFilterDefEqual(&def1->p.ethHdrFilter,
+ &def2->p.ethHdrFilter);
+ break;
+ case VIR_NWFILTER_RULE_PROTOCOL_VLAN:
+ equal = virNWFilterVlanHdrFilterDefEqual(&def1->p.vlanHdrFilter,
+ &def2->p.vlanHdrFilter);
+ break;
+ case VIR_NWFILTER_RULE_PROTOCOL_STP:
+ equal = virNWFilterStpHdrFilterDefEqual(&def1->p.stpHdrFilter,
+ &def2->p.stpHdrFilter);
+ case VIR_NWFILTER_RULE_PROTOCOL_ARP:
+ case VIR_NWFILTER_RULE_PROTOCOL_RARP:
+ equal = virNWFilterArpHdrFilterDefEqual(&def1->p.arpHdrFilter,
+ &def2->p.arpHdrFilter);
+ break;
+ case VIR_NWFILTER_RULE_PROTOCOL_IP:
+ equal = virNWFilterIpHdrFilterDefEqual(&def1->p.ipHdrFilter,
+ &def2->p.ipHdrFilter);
+ break;
+ case VIR_NWFILTER_RULE_PROTOCOL_IPV6:
+ equal = virNWFilterIpv6HdrFilterDefEqual(&def1->p.ipv6HdrFilter,
+ &def2->p.ipv6HdrFilter);
+ break;
+ case VIR_NWFILTER_RULE_PROTOCOL_TCP:
+ case VIR_NWFILTER_RULE_PROTOCOL_TCPoIPV6:
+ equal = virNWFilterTcpHdrFilterDefEqual(&def1->p.tcpHdrFilter,
+ &def2->p.tcpHdrFilter);
+ break;
+ case VIR_NWFILTER_RULE_PROTOCOL_ICMP:
+ case VIR_NWFILTER_RULE_PROTOCOL_ICMPV6:
+ equal = virNWFilterIcmpHdrFilterDefEqual(&def1->p.icmpHdrFilter,
+ &def2->p.icmpHdrFilter);
+ break;
+ case VIR_NWFILTER_RULE_PROTOCOL_IGMP:
+ equal = virNWFilterIgmpHdrFilterDefEqual(&def1->p.igmpHdrFilter,
+ &def2->p.igmpHdrFilter);
+ break;
+ case VIR_NWFILTER_RULE_PROTOCOL_UDP:
+ case VIR_NWFILTER_RULE_PROTOCOL_UDPoIPV6:
+ equal = virNWFilterUdpHdrFilterDefEqual(&def1->p.udpHdrFilter,
+ &def2->p.udpHdrFilter);
+ break;
+ case VIR_NWFILTER_RULE_PROTOCOL_UDPLITE:
+ case VIR_NWFILTER_RULE_PROTOCOL_UDPLITEoIPV6:
+ equal = virNWFilterUdpliteHdrFilterDefEqual(&def1->p.udpliteHdrFilter,
+ &def2->p.udpliteHdrFilter);
+ break;
+ case VIR_NWFILTER_RULE_PROTOCOL_ESP:
+ case VIR_NWFILTER_RULE_PROTOCOL_ESPoIPV6:
+ equal = virNWFilterEspHdrFilterDefEqual(&def1->p.espHdrFilter,
+ &def2->p.espHdrFilter);
+ break;
+ case VIR_NWFILTER_RULE_PROTOCOL_AH:
+ case VIR_NWFILTER_RULE_PROTOCOL_AHoIPV6:
+ equal = virNWFilterAhHdrFilterDefEqual(&def1->p.ahHdrFilter,
+ &def2->p.ahHdrFilter);
+ break;
+ case VIR_NWFILTER_RULE_PROTOCOL_SCTP:
+ case VIR_NWFILTER_RULE_PROTOCOL_SCTPoIPV6:
+ equal = virNWFilterSctpHdrFilterDefEqual(&def1->p.sctpHdrFilter,
+ &def2->p.sctpHdrFilter);
+ break;
+ case VIR_NWFILTER_RULE_PROTOCOL_ALL:
+ case VIR_NWFILTER_RULE_PROTOCOL_ALLoIPV6:
+ equal = virNWFilterAllHdrFilterDefEqual(&def1->p.allHdrFilter,
+ &def2->p.allHdrFilter);
+ break;
+ case VIR_NWFILTER_RULE_PROTOCOL_LAST:
+ return false;
+ }
+
+ if (!equal)
+ return false;
+
+ if (def1->nVarAccess != def2->nVarAccess)
+ return false;
+
+ for (i = 0; i < def1->nVarAccess; i++) {
+ equal = false;
+ for (j = 0; j < def2->nVarAccess; j++) {
+ equal = virNWFilterVarAccessEqual(def1->varAccess[i],
+ def2->varAccess[j]);
+ if (equal)
+ break;
+ }
+ if (!equal)
+ return false;
+ }
+
+ if (def1->nstrings != def2->nstrings)
+ return false;
+
+ for (i = 0; i < def1->nstrings; i++) {
+ equal = false;
+ for (j = 0; j < def2->nstrings; j++) {
+ equal = STREQ(def1->strings[i], def2->strings[j]);
+ if (equal)
+ break;
+ }
+ if (!equal)
+ return false;
+ }
+ return true;
+}
void
virNWFilterObjRemove(virNWFilterObjListPtr nwfilters,
@@ -2817,6 +2950,14 @@ virNWFilterObjAssignDef(virConnectPtr co
virNWFilterLockFilterUpdates();
if ((nwfilter = virNWFilterObjFindByName(nwfilters, def->name))) {
+
+ if (virNWFilterDefEqual(def, nwfilter->def, false)) {
+ virNWFilterDefFree(nwfilter->def);
+ nwfilter->def = def;
+ virNWFilterUnlockFilterUpdates();
+ return nwfilter;
+ }
+
nwfilter->newDef = def;
/* trigger the update on VMs referencing the filter */
if (virNWFilterTriggerVMFilterRebuild(conn)) {
@@ -3226,6 +3367,22 @@ virNWFilterIncludeDefFormat(virNWFilterI
return virBufferContentAndReset(&buf);
}
+static bool
+virNWFilterIncludeDefEqual(const virNWFilterIncludeDefPtr def1,
+ const virNWFilterIncludeDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!STREQ(def1->filterref, def2->filterref) ||
+ !virNWFilterHashTableEqual(def1->params, def2->params))
+ return false;
+
+ return true;
+}
static char *
virNWFilterEntryFormat(virNWFilterEntryPtr entry)
@@ -3235,6 +3392,22 @@ virNWFilterEntryFormat(virNWFilterEntryP
return virNWFilterIncludeDefFormat(entry->include);
}
+static bool
+virNWFilterEntryEqual(const virNWFilterEntryPtr def1,
+ const virNWFilterEntryPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!virNWFilterRuleDefEqual(def1->rule, def2->rule) ||
+ !virNWFilterIncludeDefEqual(def1->include, def2->include))
+ return false;
+
+ return true;
+}
char *
virNWFilterDefFormat(virNWFilterDefPtr def)
@@ -3278,6 +3451,46 @@ virNWFilterDefFormat(virNWFilterDefPtr d
return NULL;
}
+/* Compare two filter definitions for equality.
+ * Both filters must have the same content and all content must appear in the
+ * same order.
+ *
+ * @def1: the 1st filter
+ * @def2: the 2nd filter
+ * @cmpUUIDs: whether to compare the UUIDs of the two filters
+ *
+ * Returns true if both filters are equal, false otherwise.
+ */
+static bool
+virNWFilterDefEqual(const virNWFilterDefPtr def1, const virNWFilterDefPtr def2,
+ bool cmpUUIDs)
+{
+ unsigned int i;
+ bool equal;
+
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!STREQ(def1->name, def2->name) ||
+ (cmpUUIDs && memcmp(def1->uuid, def2->uuid, sizeof(def1->uuid))
!= 0) ||
+ !STREQ(def1->chainsuffix, def2->chainsuffix) ||
+ def1->chainPriority != def2->chainPriority ||
+ def1->nentries != def2->nentries)
+ return false;
+
+ /* the order of the filter entries must be the same in both */
+ for (i = 0; i < def1->nentries; i++) {
+ equal = virNWFilterEntryEqual(def1->filterEntries[i],
+ def2->filterEntries[i]);
+ if (!equal)
+ return false;
+ }
+
+ return true;
+}
char *virNWFilterConfigFile(const char *dir,
const char *name)
Index: libvirt-iterator/src/conf/nwfilter_params.c
===================================================================
--- libvirt-iterator.orig/src/conf/nwfilter_params.c
+++ libvirt-iterator/src/conf/nwfilter_params.c
@@ -747,6 +747,19 @@ err_exit:
return -1;
}
+bool
+virNWFilterHashTableEqual(const virNWFilterHashTablePtr hash1,
+ const virNWFilterHashTablePtr hash2)
+{
+ if (hash1 == hash2)
+ return true;
+
+ if (!hash1 || !hash2)
+ return false;
+
+ return virHashEqual(hash1->hashTable, hash2->hashTable,
+ (virHashValueComparator)strcmp);
+}
static bool
isValidVarName(const char *var)
@@ -896,6 +909,11 @@ bool
virNWFilterVarAccessEqual(const virNWFilterVarAccessPtr a,
const virNWFilterVarAccessPtr b)
{
+ if (a == b)
+ return true;
+ if (!a || !b)
+ return false;
+
if (a->accessType != b->accessType)
return false;
Index: libvirt-iterator/src/conf/nwfilter_params.h
===================================================================
--- libvirt-iterator.orig/src/conf/nwfilter_params.h
+++ libvirt-iterator/src/conf/nwfilter_params.h
@@ -84,6 +84,8 @@ void *virNWFilterHashTableRemoveEntry(vi
const char *name);
int virNWFilterHashTablePutAll(virNWFilterHashTablePtr src,
virNWFilterHashTablePtr dest);
+bool virNWFilterHashTableEqual(const virNWFilterHashTablePtr hash1,
+ const virNWFilterHashTablePtr hash2);
# define VALID_VARNAME \
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_"
Index: libvirt-iterator/src/Makefile.am
===================================================================
--- libvirt-iterator.orig/src/Makefile.am
+++ libvirt-iterator/src/Makefile.am
@@ -152,6 +152,7 @@ NETWORK_CONF_SOURCES = \
# Network filter driver generic impl APIs
NWFILTER_PARAM_CONF_SOURCES = \
conf/nwfilter_params.c conf/nwfilter_params.h \
+ conf/nwfilter_protocols.c conf/nwfilter_protocols.h \
conf/nwfilter_conf.h
NWFILTER_CONF_SOURCES = \
Index: libvirt-iterator/src/conf/nwfilter_protocols.c
===================================================================
--- /dev/null
+++ libvirt-iterator/src/conf/nwfilter_protocols.c
@@ -0,0 +1,447 @@
+/*
+ * nwfilter_protocols.c: protocol handling
+ *
+ * Copyright (C) 2011 IBM Corporation
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Author: Stefan Berger <stefanb(a)us.ibm.com>
+ */
+
+#include <config.h>
+
+#include "internal.h"
+
+#include "uuid.h"
+#include "virterror_internal.h"
+#include "datatypes.h"
+#include "nwfilter_params.h"
+#include "nwfilter_conf.h"
+#include "nwfilter_protocols.h"
+
+#define VIR_FROM_THIS VIR_FROM_NWFILTER
+
+#define NWITEMDESCEQUAL(ITEM) \
+ virNWFilterNWItemDescEqual(&def1->ITEM, &def2->ITEM)
+
+static bool
+virNWFilterNWItemDescEqual(const nwItemDescPtr def1,
+ const nwItemDescPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (def1->flags != def2->flags ||
+ !virNWFilterVarAccessEqual(def1->varAccess, def2->varAccess) ||
+ def1->datatype != def2->datatype)
+ return false;
+
+ switch (def1->datatype) {
+ case DATATYPE_STRINGCOPY:
+ if (strcmp(def1->u.string, def2->u.string))
+ return false;
+ break;
+ case DATATYPE_UINT16:
+ case DATATYPE_UINT8:
+ case DATATYPE_UINT16_HEX:
+ case DATATYPE_UINT8_HEX:
+ case DATATYPE_MACADDR:
+ case DATATYPE_MACMASK:
+ case DATATYPE_IPADDR:
+ case DATATYPE_IPMASK:
+ case DATATYPE_IPV6ADDR:
+ case DATATYPE_IPV6MASK:
+ case DATATYPE_STRING:
+ case DATATYPE_BOOLEAN:
+ case DATATYPE_UINT32:
+ case DATATYPE_UINT32_HEX:
+ if (memcmp(&def1->u, &def2->u, sizeof(def1->u)))
+ return false;
+ break;
+ case DATATYPE_LAST:
+ return false;
+ }
+
+ return true;
+}
+
+static bool
+virNWFilterEthHdrDataDefEqual(const ethHdrDataDefPtr def1,
+ const ethHdrDataDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!NWITEMDESCEQUAL(dataSrcMACAddr) ||
+ !NWITEMDESCEQUAL(dataSrcMACMask) ||
+ !NWITEMDESCEQUAL(dataDstMACAddr) ||
+ !NWITEMDESCEQUAL(dataDstMACMask))
+ return false;
+
+ return true;
+}
+
+bool
+virNWFilterEthHdrFilterDefEqual(const ethHdrFilterDefPtr def1,
+ const ethHdrFilterDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!virNWFilterEthHdrDataDefEqual(&def1->ethHdr, &def2->ethHdr) ||
+ !NWITEMDESCEQUAL(dataProtocolID) ||
+ !NWITEMDESCEQUAL(dataComment))
+ return false;
+
+ return true;
+}
+
+bool
+virNWFilterVlanHdrFilterDefEqual(const vlanHdrFilterDefPtr def1,
+ const vlanHdrFilterDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!virNWFilterEthHdrDataDefEqual(&def1->ethHdr, &def2->ethHdr) ||
+ !NWITEMDESCEQUAL(dataVlanID) ||
+ !NWITEMDESCEQUAL(dataVlanEncap) ||
+ !NWITEMDESCEQUAL(dataComment))
+ return false;
+
+ return true;
+}
+
+bool
+virNWFilterStpHdrFilterDefEqual(const stpHdrFilterDefPtr def1,
+ const stpHdrFilterDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!virNWFilterEthHdrDataDefEqual(&def1->ethHdr, &def2->ethHdr) ||
+ !NWITEMDESCEQUAL(dataType) ||
+ !NWITEMDESCEQUAL(dataFlags) ||
+ !NWITEMDESCEQUAL(dataRootPri) ||
+ !NWITEMDESCEQUAL(dataRootPriHi) ||
+ !NWITEMDESCEQUAL(dataRootAddr) ||
+ !NWITEMDESCEQUAL(dataRootAddrMask) ||
+ !NWITEMDESCEQUAL(dataRootCost) ||
+ !NWITEMDESCEQUAL(dataRootCostHi) ||
+ !NWITEMDESCEQUAL(dataSndrPrio) ||
+ !NWITEMDESCEQUAL(dataSndrPrioHi) ||
+ !NWITEMDESCEQUAL(dataSndrAddr) ||
+ !NWITEMDESCEQUAL(dataSndrAddrMask) ||
+ !NWITEMDESCEQUAL(dataPort) ||
+ !NWITEMDESCEQUAL(dataPortHi) ||
+ !NWITEMDESCEQUAL(dataAge) ||
+ !NWITEMDESCEQUAL(dataAgeHi) ||
+ !NWITEMDESCEQUAL(dataMaxAge) ||
+ !NWITEMDESCEQUAL(dataMaxAgeHi) ||
+ !NWITEMDESCEQUAL(dataHelloTime) ||
+ !NWITEMDESCEQUAL(dataHelloTimeHi) ||
+ !NWITEMDESCEQUAL(dataFwdDelay) ||
+ !NWITEMDESCEQUAL(dataFwdDelayHi) ||
+ !NWITEMDESCEQUAL(dataComment))
+ return false;
+
+ return true;
+}
+
+bool
+virNWFilterArpHdrFilterDefEqual(const arpHdrFilterDefPtr def1,
+ const arpHdrFilterDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!virNWFilterEthHdrDataDefEqual(&def1->ethHdr, &def2->ethHdr) ||
+ !NWITEMDESCEQUAL(dataHWType) ||
+ !NWITEMDESCEQUAL(dataProtocolType) ||
+ !NWITEMDESCEQUAL(dataOpcode) ||
+ !NWITEMDESCEQUAL(dataARPSrcMACAddr) ||
+ !NWITEMDESCEQUAL(dataARPSrcIPAddr) ||
+ !NWITEMDESCEQUAL(dataARPDstMACAddr) ||
+ !NWITEMDESCEQUAL(dataARPDstIPAddr) ||
+ !NWITEMDESCEQUAL(dataGratuitousARP) ||
+ !NWITEMDESCEQUAL(dataComment))
+ return false;
+
+ return true;
+}
+
+static bool
+virNWFilterIpHdrDataDefEqual(const ipHdrDataDefPtr def1,
+ const ipHdrDataDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!NWITEMDESCEQUAL(dataIPVersion) ||
+ !NWITEMDESCEQUAL(dataSrcIPAddr) ||
+ !NWITEMDESCEQUAL(dataSrcIPMask) ||
+ !NWITEMDESCEQUAL(dataDstIPAddr) ||
+ !NWITEMDESCEQUAL(dataDstIPMask) ||
+ !NWITEMDESCEQUAL(dataProtocolID) ||
+ !NWITEMDESCEQUAL(dataSrcIPFrom) ||
+ !NWITEMDESCEQUAL(dataSrcIPTo) ||
+ !NWITEMDESCEQUAL(dataDstIPFrom) ||
+ !NWITEMDESCEQUAL(dataDstIPTo) ||
+ !NWITEMDESCEQUAL(dataDSCP) ||
+ !NWITEMDESCEQUAL(dataState) ||
+ !NWITEMDESCEQUAL(dataConnlimitAbove) ||
+ !NWITEMDESCEQUAL(dataComment))
+ return false;
+
+ return true;
+}
+
+static bool
+virNWFilterPortDataDefEqual(const portDataDefPtr def1,
+ const portDataDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!NWITEMDESCEQUAL(dataSrcPortStart) ||
+ !NWITEMDESCEQUAL(dataSrcPortEnd) ||
+ !NWITEMDESCEQUAL(dataDstPortStart) ||
+ !NWITEMDESCEQUAL(dataDstPortEnd))
+ return false;
+
+ return true;
+}
+
+bool
+virNWFilterIpHdrFilterDefEqual(const ipHdrFilterDefPtr def1,
+ const ipHdrFilterDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!virNWFilterEthHdrDataDefEqual(&def1->ethHdr, &def2->ethHdr) ||
+ !virNWFilterIpHdrDataDefEqual(&def1->ipHdr, &def2->ipHdr) ||
+ !virNWFilterPortDataDefEqual(&def1->portData, &def2->portData))
+ return false;
+
+ return true;
+}
+
+bool
+virNWFilterIpv6HdrFilterDefEqual(const ipv6HdrFilterDefPtr def1,
+ const ipv6HdrFilterDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!virNWFilterEthHdrDataDefEqual(&def1->ethHdr, &def2->ethHdr) ||
+ !virNWFilterIpHdrDataDefEqual(&def1->ipHdr, &def2->ipHdr) ||
+ !virNWFilterPortDataDefEqual(&def1->portData, &def2->portData))
+ return false;
+
+ return true;
+}
+
+bool
+virNWFilterIcmpHdrFilterDefEqual(const icmpHdrFilterDefPtr def1,
+ const icmpHdrFilterDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!NWITEMDESCEQUAL(dataSrcMACAddr) ||
+ !virNWFilterIpHdrDataDefEqual(&def1->ipHdr, &def2->ipHdr) ||
+ !NWITEMDESCEQUAL(dataICMPType) ||
+ !NWITEMDESCEQUAL(dataICMPCode) ||
+ !NWITEMDESCEQUAL(dataStateFlags))
+ return false;
+
+ return true;
+}
+
+bool
+virNWFilterAllHdrFilterDefEqual(const allHdrFilterDefPtr def1,
+ const allHdrFilterDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!NWITEMDESCEQUAL(dataSrcMACAddr) ||
+ !virNWFilterIpHdrDataDefEqual(&def1->ipHdr, &def2->ipHdr))
+ return false;
+
+ return true;
+}
+
+bool
+virNWFilterIgmpHdrFilterDefEqual(const igmpHdrFilterDefPtr def1,
+ const igmpHdrFilterDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!NWITEMDESCEQUAL(dataSrcMACAddr) ||
+ !virNWFilterIpHdrDataDefEqual(&def1->ipHdr, &def2->ipHdr))
+ return false;
+
+ return true;
+}
+
+bool
+virNWFilterTcpHdrFilterDefEqual(const tcpHdrFilterDefPtr def1,
+ const tcpHdrFilterDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!NWITEMDESCEQUAL(dataSrcMACAddr) ||
+ !virNWFilterIpHdrDataDefEqual(&def1->ipHdr, &def2->ipHdr) ||
+ !virNWFilterPortDataDefEqual(&def1->portData, &def2->portData) ||
+ !NWITEMDESCEQUAL(dataTCPOption) ||
+ !NWITEMDESCEQUAL(dataTCPFlags))
+ return false;
+
+ return true;
+}
+
+bool
+virNWFilterUdpHdrFilterDefEqual(const udpHdrFilterDefPtr def1,
+ const udpHdrFilterDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!NWITEMDESCEQUAL(dataSrcMACAddr) ||
+ !virNWFilterIpHdrDataDefEqual(&def1->ipHdr, &def2->ipHdr) ||
+ !virNWFilterPortDataDefEqual(&def1->portData, &def2->portData))
+ return false;
+
+ return true;
+}
+
+bool
+virNWFilterSctpHdrFilterDefEqual(const sctpHdrFilterDefPtr def1,
+ const sctpHdrFilterDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!NWITEMDESCEQUAL(dataSrcMACAddr) ||
+ !virNWFilterIpHdrDataDefEqual(&def1->ipHdr, &def2->ipHdr) ||
+ !virNWFilterPortDataDefEqual(&def1->portData, &def2->portData))
+ return false;
+
+ return true;
+}
+
+bool
+virNWFilterEspHdrFilterDefEqual(const espHdrFilterDefPtr def1,
+ const espHdrFilterDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!NWITEMDESCEQUAL(dataSrcMACAddr) ||
+ !virNWFilterIpHdrDataDefEqual(&def1->ipHdr, &def2->ipHdr))
+ return false;
+
+ return true;
+}
+
+bool
+virNWFilterAhHdrFilterDefEqual(const ahHdrFilterDefPtr def1,
+ const ahHdrFilterDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!NWITEMDESCEQUAL(dataSrcMACAddr) ||
+ !virNWFilterIpHdrDataDefEqual(&def1->ipHdr, &def2->ipHdr))
+ return false;
+
+ return true;
+}
+
+bool
+virNWFilterUdpliteHdrFilterDefEqual(const udpliteHdrFilterDefPtr def1,
+ const udpliteHdrFilterDefPtr def2)
+{
+ if (def1 == def2)
+ return true;
+
+ if (!def1 || !def2)
+ return false;
+
+ if (!NWITEMDESCEQUAL(dataSrcMACAddr) ||
+ !virNWFilterIpHdrDataDefEqual(&def1->ipHdr, &def2->ipHdr))
+ return false;
+
+ return true;
+}
+
Index: libvirt-iterator/src/conf/nwfilter_protocols.h
===================================================================
--- /dev/null
+++ libvirt-iterator/src/conf/nwfilter_protocols.h
@@ -0,0 +1,56 @@
+/*
+ * nwfilter_protocols.h: protocol handling
+ *
+ * Copyright (C) 2011 IBM Corporation
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Author: Stefan Berger <stefanb(a)us.ibm.com>
+ */
+
+#include "datatypes.h"
+#include "nwfilter_params.h"
+#include "nwfilter_conf.h"
+
+bool virNWFilterEthHdrFilterDefEqual(const ethHdrFilterDefPtr def1,
+ const ethHdrFilterDefPtr def2);
+bool virNWFilterVlanHdrFilterDefEqual(const vlanHdrFilterDefPtr def1,
+ const vlanHdrFilterDefPtr def2);
+bool virNWFilterStpHdrFilterDefEqual(const stpHdrFilterDefPtr def1,
+ const stpHdrFilterDefPtr def2);
+bool virNWFilterArpHdrFilterDefEqual(const arpHdrFilterDefPtr def1,
+ const arpHdrFilterDefPtr def2);
+bool virNWFilterIpHdrFilterDefEqual(const ipHdrFilterDefPtr def1,
+ const ipHdrFilterDefPtr def2);
+bool virNWFilterIpv6HdrFilterDefEqual(const ipv6HdrFilterDefPtr def1,
+ const ipv6HdrFilterDefPtr def2);
+bool virNWFilterIcmpHdrFilterDefEqual(const icmpHdrFilterDefPtr def1,
+ const icmpHdrFilterDefPtr def2);
+bool virNWFilterAllHdrFilterDefEqual(const allHdrFilterDefPtr def1,
+ const allHdrFilterDefPtr def2);
+bool virNWFilterIgmpHdrFilterDefEqual(const igmpHdrFilterDefPtr def1,
+ const igmpHdrFilterDefPtr def2);
+bool virNWFilterTcpHdrFilterDefEqual(const tcpHdrFilterDefPtr def1,
+ const tcpHdrFilterDefPtr def2);
+bool virNWFilterUdpHdrFilterDefEqual(const udpHdrFilterDefPtr def1,
+ const udpHdrFilterDefPtr def2);
+bool virNWFilterSctpHdrFilterDefEqual(const sctpHdrFilterDefPtr def1,
+ const sctpHdrFilterDefPtr def2);
+bool virNWFilterEspHdrFilterDefEqual(const espHdrFilterDefPtr def1,
+ const espHdrFilterDefPtr def2);
+bool virNWFilterAhHdrFilterDefEqual(const ahHdrFilterDefPtr def1,
+ const ahHdrFilterDefPtr def2);
+bool virNWFilterUdpliteHdrFilterDefEqual(const udpliteHdrFilterDefPtr def1,
+ const udpliteHdrFilterDefPtr def2);
3:59 p.m.
New subject: [libvirt] [PATCH v2 3/3] nwfilter: Rebuild filters only if new filter is different than current
On 01/18/2012 09:20 AM, Stefan Berger wrote:
Compare two filter definitions for equality and only
rebuild/instantiate
the new filter if the two filters are found to be different. This improves
performance during an update of a filter with no obvious change or the reloading
of filters during a 'kill -SIGHUP'
Unforuntately this more involved than a mere memcmp() on the structures.
s/Unforuntately/Unfortunately/
---
src/Makefile.am | 1
src/conf/nwfilter_conf.c | 213 ++++++++++++++++++++
src/conf/nwfilter_params.c | 18 +
src/conf/nwfilter_params.h | 2
src/conf/nwfilter_protocols.c | 447 ++++++++++++++++++++++++++++++++++++++++++
src/conf/nwfilter_protocols.h | 56 +++++
6 files changed, 737 insertions(+)
Fails 'make syntax-check':
prohibit_empty_lines_at_EOF
src/conf/nwfilter_protocols.c
maint.mk: empty line(s) or no newline at EOF
And while fixing that, I noticed that you didn't bump any copyright
years. Emacs users can get this for free with this in ~/.emacs (not
sure if vim has a comparable hook):
(require 'copyright)
(defun my-copyright-update (&optional arg)
"My improvements to `copyright-update'."
(interactive "*P")
(and (not (eq major-mode 'fundamental-mode))
(copyright-update arg))
nil)
(add-hook 'before-save-hook 'my-copyright-update)
Index: libvirt-iterator/src/conf/nwfilter_params.c
===================================================================
--- libvirt-iterator.orig/src/conf/nwfilter_params.c
+++ libvirt-iterator/src/conf/nwfilter_params.c
@@ -747,6 +747,19 @@ err_exit:
return -1;
}
+bool
+virNWFilterHashTableEqual(const virNWFilterHashTablePtr hash1,
+ const virNWFilterHashTablePtr hash2)
+{
+ if (hash1 == hash2)
+ return true;
+
+ if (!hash1 || !hash2)
+ return false;
+
+ return virHashEqual(hash1->hashTable, hash2->hashTable,
+ (virHashValueComparator)strcmp);
Indentation - I'd remove 3 spaces, so that the case lines up with hash1
on the line before.
Index: libvirt-iterator/src/conf/nwfilter_protocols.c
===================================================================
--- /dev/null
+++ libvirt-iterator/src/conf/nwfilter_protocols.c
@@ -0,0 +1,447 @@
+/*
+ * nwfilter_protocols.c: protocol handling
+ *
+ * Copyright (C) 2011 IBM Corporation
It's 2012, now :)
+
+#define VIR_FROM_THIS VIR_FROM_NWFILTER
+
+#define NWITEMDESCEQUAL(ITEM) \
+ virNWFilterNWItemDescEqual(&def1->ITEM, &def2->ITEM)
You don't have to change if you don't want, but it took me a while to
parse that name. Maybe NW_ITEM_DESC_EQUAL would be more readable.
Index: libvirt-iterator/src/conf/nwfilter_protocols.h
===================================================================
--- /dev/null
+++ libvirt-iterator/src/conf/nwfilter_protocols.h
@@ -0,0 +1,56 @@
+/*
+ * nwfilter_protocols.h: protocol handling
+ *
+ * Copyright (C) 2011 IBM Corporation
2012
Big, but looks like a rather mechanical conversion of the various
protocols into a highly repetitive algorithm for comparisons. I didn't
notice any major problems, so:
ACK with nits fixed.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:33 a.m.
New subject: [libvirt] [PATCH v2 3/3] nwfilter: Rebuild filters only if new filter is different than current
On 01/23/2012 04:59 PM, Eric Blake wrote:
On 01/18/2012 09:20 AM, Stefan Berger wrote:
> Compare two filter definitions for equality and only rebuild/instantiate
> the new filter if the two filters are found to be different. This improves
> performance during an update of a filter with no obvious change or the reloading
> of filters during a 'kill -SIGHUP'
>
> Unforuntately this more involved than a mere memcmp() on the structures.
2012
Big, but looks like a rather mechanical conversion of the various
protocols into a highly repetitive algorithm for comparisons. I didn't
notice any major problems, so:
ACK with nits fixed.
After some consideration I decided not to push this patch but go a
different route: have the new XML parsed, convert new and old internal
representation of the filters to XML and compare those. This will be
much less code.
Stefan
4689
days inactive
4696
days old
10 comments
3 participants
participants (3)
-
Eric Blake -
Michal Privoznik -
Stefan Berger