Re: [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
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)