[libvirt] [PATCHv2 3/4] conf: add listenNetwork attribute to domain graphics element

[Laine Stump]

On 07/21/2011 07:32 PM, Eric Blake wrote:
> On 07/20/2011 02:11 AM, Laine Stump wrote:
>> Once it's plugged in, listenNetwork will be an optional replacement
>> for the "listen" attribute. While listen can be a host name or IP
>> address, listenNetwork names one of the networks managed by libvirt
>> (with virNetwork*()/visrh net-*).
>> ---
>>   docs/schemas/domain.rng                            |   33 ++++++++---
>>   src/conf/domain_conf.c                             |   60 
>> +++++++++++++++++--
>>   src/conf/domain_conf.h                             |    3 +
>>   .../qemuxml2argv-graphics-listenNetwork.xml        |   30 ++++++++++
>>   tests/qemuxml2xmltest.c                            |    1 +
>>   5 files changed, 111 insertions(+), 16 deletions(-)
>>   create mode 100644 
>> tests/qemuxml2argvdata/qemuxml2argv-graphics-listenNetwork.xml
>>
>
>> diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
>> index 07c63bd..42f3eb2 100644
>> --- a/docs/schemas/domain.rng
>> +++ b/docs/schemas/domain.rng
>> @@ -1241,9 +1241,14 @@
>> </attribute>
>> </optional>
>> <optional>
>> -<attribute name="listen">
>> -<ref name="addrIPorName"/>
>> -</attribute>
>> +<choice>
>> +<attribute name="listen">
>> +<ref name="addrIPorName"/>
>> +</attribute>
>> +<attribute name="listenNetwork">
>> +<text/>
>
> [stupid thunderbird whitespace corruption]
>
> Shouldn't this be something more specific than <text/>?  A <ref 
> name=.../> to something that matches valid network names might be more 
> appropriate.  Then again, network.rng uses <text/> for the name 
> element, so I guess we're okay here.


Right. We have to accept any valid network name, and currently 
(according to the RNG) a network name can be anything.


>
>> @@ -1300,9 +1305,14 @@
>> </attribute>
>> </optional>
>> <optional>
>> -<attribute name="listen">
>> -<ref name="addrIPorName"/>
>> -</attribute>
>> +<choice>
>> +<attribute name="listen">
>> +<ref name="addrIPorName"/>
>> +</attribute>
>> +<attribute name="listenNetwork">
>> +<text/>
>> +</attribute>
>> +</choice>
>
> We repeat this <choice> enough times; maybe it's worth factorizing 
> into a macro and using it by <ref> in all three call sites?  Up to you 
> - it's not a show-stopper to the patch as-is.

I think I actually prefer them separate, that makes it easier for 
someone reading the rng to see just what attributes are in each element, 
without needing to traipse up and down the file.

I'll push this one as is (after re-basing.)