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.