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

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



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


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