[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH V5 06/10] Extend the filter XML to support priorities of chains



On 11/17/2011 06:15 PM, Eric Blake wrote:
On 11/17/2011 01:11 PM, Stefan Berger wrote:
This patch extends the filter XML to support priorities of chains
in the XML. An example would be:

<filter name='allow-arpxyz' chain='arp-xyz' priority='200'>
[...]
</filter>

The permitted values for priorities are [-1000, 1000].
By setting the pririty of a chain the order in which it is accessed
from the interface root chain can be influenced.

Signed-off-by: Stefan Berger<stefanb linux vnet ibm com>

---
  docs/schemas/nwfilter.rng |    7 ++++++-
Missing documentation in docs/formatnwfilter.html.in.  I'll live up to
my hard-line reputation on this one, and request a v6 with documentation
(for example, it's worth documenting whether priority 100 is accessed
before or after priority 200).

I have such a patch much further down the queue. I'll pull the relevant parts into v6.
But as to the code...

@@ -2028,6 +2030,26 @@ virNWFilterDefParseXML(xmlXPathContextPt
          goto cleanup;
      }

+    chain_pri_s = virXPathString("string(./@priority)", ctxt);
+    if (chain_pri_s) {
+        if (sscanf(chain_pri_s, "%d",&chain_priority) != 1) {
Let's use virStrToLong_i() instead of sscanf(); much nicer on the error
handling aspect.

Done.
@@ -2036,11 +2058,16 @@ virNWFilterDefParseXML(xmlXPathContextPt
              goto cleanup;
          }
          ret->chainsuffix = chain;
-        /* assign an implicit priority -- support XML attribute later */
-        if (intMapGetByString(chain_priorities, chain, 0,
-&ret->chainPriority) == false) {
-            ret->chainPriority = (NWFILTER_MAX_FILTER_PRIORITY +
-                                  NWFILTER_MIN_FILTER_PRIORITY) / 2;
+
+        if (chain_pri_s) {
+            ret->chainPriority = chain_priority;
+        } else {
+            /* assign an implicit priority -- support XML attribute later */
Is this comment still accurate, now that you have an XML attribute?

Fixed.
@@ -2852,6 +2881,9 @@ virNWFilterDefFormat(virNWFilterDefPtr d
      virBufferAsprintf(&buf, "<filter name='%s' chain='%s'",
                        def->name,
                        def->chainsuffix);
+    if (def->chainPriority != 0)
+        virBufferAsprintf(&buf, " priority='%d'",
+                          def->chainPriority);
That means an explicit pririoty='0' by the user is eaten and does not
appear on the output.  But that's not too bad, and as long as we
document that priority is 0 unless explicitly specified, we're covered
(hence my plea for documentation...)

Everything else looks okay.



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]