[libvirt] [PATCH V14 5/5] nwfilter: Display detected IP address in domain XML

[Laine Stump]

On 06/08/2012 07:36 AM, Stefan Berger wrote:
> On 06/01/2012 07:41 PM, Stefan Berger wrote:
>> On 06/01/2012 03:05 PM, Eric Blake wrote:
>>> On 05/25/2012 05:56 AM, Stefan Berger wrote:
>>>> An example of a displayed XML may then be:
>>>>
>>>> <interface type='bridge'>
>>>> <mac address='52:54:00:68:e3:90'/>
>>>> <source bridge='virbr0'/>
>>>> <target dev='vnet1'/>
>>>> <model type='virtio'/>
>>>> <filterref filter='clean-traffic'>
>>>> <parameter name='CTRL_IP_LEARNING' value='dhcp'/>
>>>> <parameter name='IP_LEASE' value='192.168.122.210,100'/>
>>> It's a shame that we are making the user parse the single XML entity to
>>> get two pieces of information.  I wonder if it would be better to have:
>>>
>>> <parameter name='IP_LEASE_ADDR' value='192.168.122.210'/>
>>> <parameter name='IP_LEASE_TIMEOUT' value='100'/>
>>>
>>> and reserve two variable names rather than one.
>>>
>>> I'll review the code as is, but I would get a second opinion from
>>> one of
>>> the Dan's on whether we should change the XML to make xpath queries
>>> easier (basically, XML that crams 2 pieces of information into one
>>> attributes is always harder to use).
>>
>> Ok, I'll wait for their comment(s).
>
> Any comments regarding the splitting mentioned above ?
>
> I would prefer *not* to split them mainly because I don't like the
> 'interleaving' of an IP address then its lease timeout then again
> another IP address and lease timeout etc., but if for XML purposes
> it's better/cleaner, I'll do it. If split, I think we would *always*
> have to display both as well. I am thinking of the case of a static IP
> address having been detected that of course does not have a timeout
> value, thus displaying '-1'.

In a way it's unfortunate that this is exposed in a generic element that
has two generic attributes. The most useful format would be a single
element with 3 attributes, like this (but with different names):

    <parameter name='IP_LEASE' address='192.168.122.210' timeout='100'/>

Obviously this doesn't fit with the existing pattern of "name=...
value=...", though, so it introduces its own problems.

Perhaps someone has a suggestion of how to expand the <parameter>
element to allow multiple values in a way that is both consistent with
current usage of <parameter> as well as allowing enough context to make
the meanings of everything understandable (maybe a <parameter> could
have some sort of sub-element when there are multiple values associated
with a parameter (only half kidding)). Or possibly that's just a dead-end.

At any rate, I think the format of the XML should make it possible for
an application to extract usable information out of the XML directly
using, e.g., libxml2, without needing to do extra work by hand.