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

[Eric Blake <eblake redhat com>]

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.

> @@ -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.

ACK.

--
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org